upload plugin should silently ignore deleted files on remote server

Bug #215612 reported by James
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Breezy
Triaged
Wishlist
Unassigned
bzr Upload plugin
Confirmed
Wishlist
Unassigned

Bug Description

Currently if a file is deleted on the remote FTP server AND removed from the local bzr branch (with bzr remove) then changes are uploaded, the upload plugin exits with an error message (an ftp error) saying it could not delete the remote file as it no longer exists.

As a real world example, someone (not using bzr) deleted some files from the ftp server and then notified me that these needed to be deleted, so I removed them from the branch with bzr remove. In this instance, the upload plugin should just ignore this error and continue with the upload. The solution to this is doing a --full upload from scratch.

If the file is removed from bzr branch and from the remote server, it should be ok to ignore and continue?

Tags: upload
Martin Albisetti (beuno)
Changed in bzr-upload:
importance: Undecided → High
status: New → Confirmed
Revision history for this message
Vincent Ladeuil (vila) wrote :

I see your point.

But the plugin relies on the assumption that the .bzr-upload.revid file on the remote site identify a snapshot: a set of files and directories for which the content is precisely known.

Whatever modification is done on the remote site breacks that assumption.

In that particular case, it will be innocuous to ignore the errors when deleting a file or a directory, since what the plugin want to do is... already done.

I'm a bit hesitant to accept that exception though as it can give a false sense of security regarding remote modifications.

May be a --force option may be added to address your case and some minor other ones.

This needs discussion.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Dowgrading importance to wishlist since a workaround exists (using full upload).

Changed in bzr-upload:
importance: High → Wishlist
Revision history for this message
James (james-ellis-gmail) wrote :

Hi

I understand your point. Maybe the upload plugin should, by default, mirror what happens for instance when one bzr branch is merged into another -- in the case where the source branch has had a file bzr removed and the target branch has had the same file manually removed.

I'm not sure if --force would be the correct verb to describe the process - maybe "--ignore" to sliently ignore upload errors ?

Finally, the workaround described is not really valid. The point of the plugin is to upload changed files - if I wanted to upload the entire tree every time, then I'd use plain old FTP. If I have a 300MB website to upload to a remote server but only 30kb of changes, that's not an efficient process.

I'd suggest implementing the --ignore flag would solve the issue in hand and provide a generic way of ignoring upload errors. I'm just unsure at the moment of the scope of an "--ignore" - it may ignore things that really ought to cause an upload failure.

Revision history for this message
Vincent Ladeuil (vila) wrote : Re: [Bug 215612] Re: upload plugin should silently ignore deleted files on remote server
Download full text (3.3 KiB)

>>>>> "James" == James <email address hidden> writes:

    James> I understand your point. Maybe the upload plugin
    James> should, by default, mirror what happens for instance
    James> when one bzr branch is merged into another -- in the
    James> case where the source branch has had a file bzr
    James> removed and the target branch has had the same file
    James> manually removed.

This is where the plugin shows its limitations. bzr fully knows
the content of both branches, whereas the plugin *suppose* it
knows the content of the remote working tree via the revid.

That's why I say the remote working tree is write-only,

    James> I'm not sure if --force would be the correct verb to describe the
    James> process - maybe "--ignore" to sliently ignore upload errors ?

The more I think about it, the more I prefer to use
'--catch-up'. The idea being: ok, we told you not to update your
remote working tree, you did it anyway, but you are pretty sure
that ignoring some errors will be enough to get back in sync, so
you told me: go ahead, trust me, all will be fine ;-)

    James> Finally, the workaround described is not really
    James> valid. The point of the plugin is to upload changed
    James> files - if I wanted to upload the entire tree every
    James> time, then I'd use plain old FTP. If I have a 300MB
    James> website to upload to a remote server but only 30kb of
    James> changes, that's not an efficient process.

Exactly. That also why trying to manage the remote working tree
as a bzr manage the local one is a dead end.

You realize that uploading the whole working tree is a waste of
resources. In the other way, establishing the modifications made
on the working tree requires comparing that working tree with the
last revision. And to do that, you need to download the whole
remote working tree.

    James> I'd suggest implementing the --ignore flag would solve
    James> the issue in hand and provide a generic way of
    James> ignoring upload errors. I'm just unsure at the moment
    James> of the scope of an "--ignore" - it may ignore things
    James> that really ought to cause an upload failure.

I need to think more about it to clearly establish which use
cases we can safely handle with a '--catch-up' option, obviously
the deleted files are one, some others will be more problematic:

- added files (no way to discover them short of walking the whole
  remote working tree, filtering with .bzrignore (which is not
  required so far) for all log files or whatever working files
  existing only on the remote site,

- renamed files or dirs (the magic involved is of a higher level
  without knowledge of the content, and even then...)

- modified files (obviously need to download them)

The more important point that makes me still hesitate to implement
that option is that once we lose the synchronization between the
revid and the remote working tree, there way to get it back.

That may requires addressing bug #206770 by providing a command
to compare the remote and local working trees. May be that
command may work in two stages:

- compare tree shapes (i.e. only the names of files and directories),
- compare tree content (dow...

Read more...

Revision history for this message
Roumano (roumano-dinoutoo) wrote :

Not exactly the same because it's related with the ssh ( not ftp ) :

bzr upload
Using saved location: sftp://castor/
Uploading bin/root/reboot_calcul.bash
bzr: ERROR: File exists: '/home': Failure: unable to mkdir

& because it's make a error, the bzr upload always re-push the file reboot_calcul.bash

Revision history for this message
James (james-ellis-gmail) wrote :

@roumanu

From the looks of it, that's more to do with getting target upload path correct in your bzr upload - should you not be doing "bzr upload sftp://castor/path/to/target/" ? Also, if you don't want reboot_calcul.bash to be uploaded, then add it to the ignore file .bzrignore using "bzr ignore"

Additionally, you seem to have ssh access to the remote system, unless bzr is not installed on that system you are probably better off using "bzr send" to send a patch file to the remote system. Have a look at https://bugs.launchpad.net/bzr/+bug/221021 for a method of sending a patch file to a remote server.

Revision history for this message
Petr Viktorin (encukou) wrote :

There is one more use case:

Suppose I lose the connection in the middle of an upload. Then, I won't be able to do bzr-upload again, because some files were already deleted, and I'm be forced to do a full upload (which is longer, so it's even more likely to fail due to connection problems). I think bzr-upload should be robust in this respect, especially since it's targetting low-end hosting services.

My own use case is a bit different: I added symlinks to my branch, and bzr-upload blew on them (Bug #214825). A silly mistake like that shouldn't force me to upload the whole tree again.

Revision history for this message
Vincent Ladeuil (vila) wrote :

@Petr

Excellent !

Thanks a ton, highlighting use cases is the best thing to do to help us enhance bzr-upload.

You rightly point out that bzr-upload should be more robust against *transient* failures, because its' needed for both incremental and full upload.

This is a known problem and mostly concern the ftp protocol (see bug #127164).

For truly lost connections (like network cable unplugged say), we need a journaled upload which is orders of magnitude more complex, so don't hold your breadth on that one (but patches welcome :).

Regarding your use case (symlinks), you're right about bug #214825, but that's only a symptom of a deeper problem: the plugin expects that the revid it uploaded (in the .bzr-upload.revid file on the remote site) guarantees that the uploaded tree is *exactly* the revision tree it can refer to locally. Any difference introduced for one reason or another is a source of problems (any modification involving a file *not* uploaded can't be handled anymore).

Revision history for this message
Andrew Fenn (andrewfenn) wrote :

Anyone annoyed by this can use the following fix in __init__.py Replace the old function with the same name with the following.

def delete_remote_file(self, relpath):
    if not self.quiet:
        self.outf.write('Deleting %s\n' % relpath)
    try:
        self.to_transport.delete(relpath)
    except errors.NoSuchFile:
        self.outf.write('Error: %s does not exist\n' % relpath)

Revision history for this message
Andrew Fenn (andrewfenn) wrote :

Just to add something..

If you add a file and commit it, then delete the file and commit it. Does bzr upload know the file doesn't exist? I ask because that's the problem we're having right now.

Revision history for this message
Andrew Fenn (andrewfenn) wrote :

I have made a patch that includes a "force" command.

The functionality of this patch is that when the --force parameter is given it:
- ignores errors about files not existing upon delete
- ignores errors about folders not existing upon delete
- ignores errors on attempts to make a directory when it already exists

I used the latest version to make this patch.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Can you push a branch with your patch included and do a merge proposal from there ?
That will make it easier to track progress.

Also, are the tests still passing with your patch ?
Would you add some to ensure your implementation of the --force option will not regress ?

Revision history for this message
Vincent Ladeuil (vila) wrote :

Bah, damn new keyboard.

Whatever you decide, thanks for your work and your feedback !

Revision history for this message
Andrew Fenn (andrewfenn) wrote :

I made the branch "force-option" and proposed it for merging.

https://code.launchpad.net/~andrewfenn/bzr-upload/force-option

Jelmer Vernooij (jelmer)
tags: added: upload
Changed in brz:
status: New → Triaged
importance: Undecided → Wishlist
milestone: none → 3.0.0
Jelmer Vernooij (jelmer)
Changed in brz:
milestone: 3.0.0 → 3.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.