#2909 closed defect (fixed)

magic-folder .backup files are wrong

Reported by: meejah Owned by: daira
Priority: normal Milestone: undecided
Component: code-frontend-magic-folder Version: 1.12.1
Keywords: Cc:
Launchpad Bug:

Description

The behavior of producing .backup files is wrong (and I believe the spec is correct). Currently, whenever we decide to download a file, a .backup is produced if there was already a file there.

But, consider the case where Alice and Bob share a folder, and Alice makes a series of updates to a file foo: on the first update, Bob downloads it and writes it; on the second, Bob downloads but already has one so makes a foo.backup as well; on the third, Bob downloads but already has one *and* a foo.backup so produces foo.conflict.

Change History (3)

comment:1 Changed at 2018-03-21T21:55:54Z by meejah

I'm fairly sure that (amongst other problems) also this conflict-case is wrong: https://github.com/tahoe-lafs/tahoe-lafs/blob/master/src/allmydata/frontends/magic_folder.py#L1279

We just blindly write .backup files all the time (i.e. anything that's downloaded) which is obviously wrong; but the conflicted cases don't seem to follow the spec either..

comment:2 Changed at 2019-03-20T16:33:45Z by exarkun

I think the link in meejah's previous comment was probably to https://github.com/tahoe-lafs/tahoe-lafs/blob/797932244da3036e0dd2463961a30cf2d319d99b/src/allmydata/frontends/magic_folder.py#L1279 (before new changes landed on master).

It looks like the logic has changed since this comment was made. I think meejah fixed the issue in affb80e39e33417abc2935c2da4e577173913f91 (or at least changed the code).

comment:3 Changed at 2019-03-20T16:39:03Z by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

It also looks like the .backup logic has changed. Previously, thanks to https://github.com/tahoe-lafs/tahoe-lafs/blob/797932244da3036e0dd2463961a30cf2d319d99b/src/allmydata/frontends/magic_folder.py#L958, any non-conflicted file being written would result in a backup file, including the case described in this ticket's description.

However, it looks like meejah fixed this behavior in 47b17876339443145e95081e9581327229a6f064 by removing the backup_path_u argument to fileutil.replace_file. This causes fileutil.replace_file to not create a backup file. Thus, backups are only created when metadata indicates the new version of the file is a deletion.

The merge for this was c9e00a988ae90ad5e63897607c70404974d370b2:

commit c9e00a988ae90ad5e63897607c70404974d370b2
Merge: 458889682 541493018
Author: meejah <meejah@meejah.ca>
Date:   Tue May 1 15:52:10 2018 -0600

    Merge pull request #475 from meejah/2909.backup-behavior.0
    
    #2909 fix .backup file behavior and (some of) the incorrect .conflict cases (#2911)

Therefore, closing as fixed.

Note: See TracTickets for help on using tickets.