summaryrefslogtreecommitdiff
path: root/Command
diff options
context:
space:
mode:
authorGravatar Joey Hess <joey@kitenet.net>2014-08-20 17:06:51 -0400
committerGravatar Joey Hess <joey@kitenet.net>2014-08-20 17:25:30 -0400
commit00d6acb7268239162a9ebd9386f7ca1271c3cc7d (patch)
treec97c753bd523679b7622c9db5bea8f3822d204ad /Command
parent7165e4035e9b6cfeaa5d659341749cc957b27e14 (diff)
avoid trying to create a content file in order to lock it
The nice refactoring in 7165e4035e9b6cfeaa5d659341749cc957b27e14 highlighted a bug in lockContent -- when the content is not present, this incorrectly created an empty lock file, using the same filename as the content file. This seems like it could result in empty objects, which fsck would detect and complain about. Both drop and move --to call lockContent, as does Remote.Git.dropKey -- I think we got lucky and this bug didn't show up because both all of those only operate on files that are present. So this bug could only manifest if there was a race, and a file's content was dropped at just the wrong time, just as another process was about to drop it. (And then only if the other process's dropping failed, otherwise it'd delete the empty object file.) Hmm, move --from also called lockContent. Unnecessarily, since the content is not being removed from the local annex. In this case, the combination of the 2 bugs could result in an empty lock file being written, and then if the download of the content failed, left in the object directory as the content. This commit also optimises lockContent, avoiding an unncessary doesFileExist test and instead just catching the exception that's thrown when the file doesn't exist. This commit was sponsored by Justine Lam.
Diffstat (limited to 'Command')
-rw-r--r--Command/Move.hs14
1 files changed, 7 insertions, 7 deletions
diff --git a/Command/Move.hs b/Command/Move.hs
index 3d9646dea..f70608a6f 100644
--- a/Command/Move.hs
+++ b/Command/Move.hs
@@ -150,11 +150,10 @@ fromOk src key = go =<< Annex.getState Annex.force
return $ u /= Remote.uuid src && elem src remotes
fromPerform :: Remote -> Bool -> Key -> AssociatedFile -> CommandPerform
-fromPerform src move key afile = moveLock move key $
- ifM (inAnnex key)
- ( dispatch move True
- , dispatch move =<< go
- )
+fromPerform src move key afile = ifM (inAnnex key)
+ ( dispatch move True
+ , dispatch move =<< go
+ )
where
go = notifyTransfer Download afile $
download (Remote.uuid src) key afile noRetry $ \p -> do
@@ -166,8 +165,9 @@ fromPerform src move key afile = moveLock move key $
ok <- Remote.removeKey src key
next $ Command.Drop.cleanupRemote key src ok
-{- Locks a key in order for it to be moved.
- - No lock is needed when a key is being copied. -}
+{- Locks a key in order for it to be moved away from the current repository.
+ - No lock is needed when a key is being copied, or moved to the current
+ - repository. -}
moveLock :: Bool -> Key -> Annex a -> Annex a
moveLock True key a = lockContent key a
moveLock False _ a = a