diff options
author | Joey Hess <joey@kitenet.net> | 2013-11-07 15:18:54 -0400 |
---|---|---|
committer | Joey Hess <joey@kitenet.net> | 2013-11-07 15:18:54 -0400 |
commit | cbd2133974a3a848fd25a95b52f1a17d220d5938 (patch) | |
tree | 7b47ad580869e636d27332e1fb171918ee5dc2d4 | |
parent | 39933fd1b6408e08549af1f9e852bf711ccf4ad8 (diff) |
watcher: Avoid loop when adding a file owned by someone else fails in indirect mode because its permissions cannot be modified.
Adding the file moved it to the annex, and then tried to set the mode.
Error unwind then moved the file back, and so the watcher saw the file get
deleted and then added back, and so tried again..
-rw-r--r-- | Command/Add.hs | 28 | ||||
-rw-r--r-- | debian/changelog | 2 | ||||
-rw-r--r-- | doc/bugs/web_app_loops_over_a_non-addable_file.mdwn | 2 |
3 files changed, 21 insertions, 11 deletions
diff --git a/Command/Add.hs b/Command/Add.hs index e0a8269aa..90a9f561b 100644 --- a/Command/Add.hs +++ b/Command/Add.hs @@ -26,7 +26,6 @@ import qualified Annex.Queue #ifndef __ANDROID__ import Utility.Touch #endif -import Utility.FileMode import Config import Utility.InodeCache import Annex.FileMatcher @@ -86,11 +85,6 @@ start file = ifAnnexed file addpresent add - So a KeySource is returned. Its inodeCache can be used to detect any - changes that might be made to the file after it was locked down. - - - In indirect mode, the write bit is removed from the file as part of lock - - down to guard against further writes, and because objects in the annex - - have their write bit disabled anyway. This is not done in direct mode, - - because files there need to remain writable at all times. - - - When possible, the file is hard linked to a temp directory. This guards - against some changes, like deletion or overwrite of the file, and - allows lsof checks to be done more efficiently when adding a lot of files. @@ -103,16 +97,28 @@ lockDown file = ifM crippledFileSystem , do tmp <- fromRepo gitAnnexTmpDir createAnnexDirectory tmp - unlessM isDirect $ - void $ liftIO $ tryIO $ preventWrite file - liftIO $ catchMaybeIO $ do + eitherToMaybe <$> tryAnnexIO (go tmp) + ) + where + {- In indirect mode, the write bit is removed from the file as part + - of lock down to guard against further writes, and because objects + - in the annex have their write bit disabled anyway. + - + - Freezing the content early also lets us fail early when + - someone else owns the file. + - + - This is not done in direct mode, because files there need to + - remain writable at all times. + -} + go tmp = do + unlessM isDirect $ + freezeContent file + liftIO $ do (tmpfile, h) <- openTempFile tmp $ relatedTemplate $ takeFileName file hClose h nukeFile tmpfile withhardlink tmpfile `catchIO` const nohardlink - ) - where nohardlink = do cache <- genInodeCache file return KeySource diff --git a/debian/changelog b/debian/changelog index 6b50b697f..73b3c3b6b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -13,6 +13,8 @@ git-annex (5.20131102) UNRELEASED; urgency=low git-annex, but it is passed along to every git command git-annex runs. * sync: When there are merge conflicts that get automatically resolved, exit nonzero at the end. + * watcher: Avoid loop when adding a file owned by someone else fails + in indirect mode because its permissions cannot be modified. -- Joey Hess <joeyh@debian.org> Sat, 02 Nov 2013 14:54:36 -0400 diff --git a/doc/bugs/web_app_loops_over_a_non-addable_file.mdwn b/doc/bugs/web_app_loops_over_a_non-addable_file.mdwn index 3773bc02f..a1f490061 100644 --- a/doc/bugs/web_app_loops_over_a_non-addable_file.mdwn +++ b/doc/bugs/web_app_loops_over_a_non-addable_file.mdwn @@ -52,3 +52,5 @@ failed Turns out that file is owned by root, so it's an expected failure. But the webapp should simply skip this file, not loop over it. The workaround is obviously to give this file to the proper user. --[[anarcat]] + +> [[fixed|done]] --[[Joey]] |