Bug 4479 - transaction doesn't clean up old journal.backup.* files
Summary: transaction doesn't clean up old journal.backup.* files
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: 3.2.2
Hardware: PC Mac OS
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-20 00:50 UTC by Durham Goode
Modified: 2015-01-22 15:04 UTC (History)
3 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Durham Goode 2014-12-20 00:50 UTC
If an old journal.backup.* file is still on disk when a new transaction starts, it can interfere with the creation of the new transaction.  When the new transaction is closing and it begins to generate the final files, it uses 'util.copyfiles(filepath, backuppath, hardlink=hardlink)' in transaction.addbackup to copy the original bookmarks/etc to the journal.  If the old journal.backup.bookmarks still exists, the hardlink fails (because os.link doesn't overwrite files) and it resorts to using shutil.copy().  This is undesirable because of the performance, but also because shutil.copy() performs a chmod under the hood, which requires that the file be owned by the user, and not just writable.

This can interfere with server infrastructure where multiple users can all push to the repo by being in the same unix group.
Comment 1 Pierre-Yves David 2015-01-05 15:28 UTC
Allowing overwriting when hardlinking sounds like the way to fix this here.
Comment 2 Pierre-Yves David 2015-01-05 15:42 UTC
(I advertising the overwrite course because I double checked the code and we should be cleaning there up already, So we are reaching a pathological case here were the file exists for some reason.)
Comment 3 Pierre-Yves David 2015-01-05 16:30 UTC
My bad, if the file was there because it of a hard kill of the process, we would have some journal file around too and we would have to run `hg recover` so we must fail to clean it up in some situation. Looking into this now.
Comment 4 HG Bot 2015-01-06 14:01 UTC
Fixed by http://selenic.com/repo/hg/rev/987ef74d8d01
Pierre-Yves David <pierre-yves.david@fb.com>
transaction: use the right location when cleaning up backup file (issue4479)

The location variable fetch from the loop and the one used to actually fetch it
mismatched. We fix the name to ensure file outside of store are cleaned up.

(please test the fix)
Comment 5 Matt Mackall 2015-01-22 15:04 UTC
Bulk testing -> fixed