convert renameFile to moveFile to support cross-device moves

Improve handling of some .git/annex/ subdirectories being on other
filesystems, in the bittorrent special remote, and youtube-dl integration,
and git-annex addurl.

The only one of these that I've confirmed to be a problem is in the
bittorrent special remote when .git/annex/tmp and .git/annex/othertmp are
on different filesystems.

As well as auditing for renameFile, also audited for createLink, all of
those are ok as are the other remaining renameFile calls. Also audited all
code paths that use .git/annex/othertmp, and did not find any other
cross-device problems. So, removing mention of othertmp needing to be on
the same device.

Sponsored-by: Dartmouth College's Datalad project
This commit is contained in:
Joey Hess 2022-12-20 15:17:50 -04:00
parent 5cbfb74391
commit 9d60385001
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
6 changed files with 26 additions and 14 deletions

View file

@ -144,7 +144,7 @@ youtubeDlTo key url dest p = do
res <- withTmpWorkDir key $ \workdir -> res <- withTmpWorkDir key $ \workdir ->
youtubeDl url (fromRawFilePath workdir) p >>= \case youtubeDl url (fromRawFilePath workdir) p >>= \case
Right (Just mediafile) -> do Right (Just mediafile) -> do
liftIO $ renameFile mediafile dest liftIO $ moveFile (toRawFilePath mediafile) (toRawFilePath dest)
return (Just True) return (Just True)
Right Nothing -> return (Just False) Right Nothing -> return (Just False)
Left msg -> do Left msg -> do

View file

@ -3,6 +3,9 @@ git-annex (10.20221213) UNRELEASED; urgency=medium
* Change --metadata comparisons < > <= and >= to fall back to * Change --metadata comparisons < > <= and >= to fall back to
lexicographical comparisons when one or both values being compared lexicographical comparisons when one or both values being compared
are not numbers. are not numbers.
* Improve handling of some .git/annex/ subdirectories being on other
filesystems, in the bittorrent special remote, and youtube-dl
integration, and git-annex addurl.
-- Joey Hess <id@joeyh.name> Mon, 12 Dec 2022 13:04:54 -0400 -- Joey Hess <id@joeyh.name> Mon, 12 Dec 2022 13:04:54 -0400

View file

@ -467,9 +467,7 @@ addWorkTree _ addunlockedmatcher u url file key mtmp = case mtmp of
-- Move to final location for large file check. -- Move to final location for large file check.
pruneTmpWorkDirBefore tmp $ \_ -> do pruneTmpWorkDirBefore tmp $ \_ -> do
createWorkTreeDirectory (P.takeDirectory file) createWorkTreeDirectory (P.takeDirectory file)
liftIO $ renameFile liftIO $ moveFile tmp file
(fromRawFilePath tmp)
(fromRawFilePath file)
largematcher <- largeFilesMatcher largematcher <- largeFilesMatcher
large <- checkFileMatcher largematcher file large <- checkFileMatcher largematcher file
if large if large
@ -477,9 +475,7 @@ addWorkTree _ addunlockedmatcher u url file key mtmp = case mtmp of
-- Move back to tmp because addAnnexedFile -- Move back to tmp because addAnnexedFile
-- needs the file in a different location -- needs the file in a different location
-- than the work tree file. -- than the work tree file.
liftIO $ renameFile liftIO $ moveFile file tmp
(fromRawFilePath file)
(fromRawFilePath tmp)
go go
else Command.Add.addSmall (DryRun False) file s else Command.Add.addSmall (DryRun False) file s
>>= maybe noop void >>= maybe noop void

View file

@ -217,7 +217,7 @@ downloadTorrentFile u = do
ok <- Url.withUrlOptions $ ok <- Url.withUrlOptions $
Url.download nullMeterUpdate Nothing u f Url.download nullMeterUpdate Nothing u f
when ok $ when ok $
liftIO $ renameFile f (fromRawFilePath torrent) liftIO $ moveFile (toRawFilePath f) torrent
return ok return ok
) )
@ -228,7 +228,7 @@ downloadMagnetLink u metadir dest = ifM download
<$> dirContents metadir <$> dirContents metadir
case ts of case ts of
(t:[]) -> do (t:[]) -> do
renameFile t dest moveFile (toRawFilePath t) (toRawFilePath dest)
return True return True
_ -> return False _ -> return False
, return False , return False
@ -256,7 +256,7 @@ downloadTorrentContent k u dest filenum p = do
showOutput showOutput
ifM (download torrent downloaddir <&&> liftIO (doesFileExist dlf)) ifM (download torrent downloaddir <&&> liftIO (doesFileExist dlf))
( do ( do
liftIO $ renameFile dlf dest liftIO $ moveFile (toRawFilePath dlf) (toRawFilePath dest)
-- The downloaddir is not removed here, -- The downloaddir is not removed here,
-- so if aria downloaded parts of other -- so if aria downloaded parts of other
-- files, and this is called again, it will -- files, and this is called again, it will

View file

@ -28,10 +28,6 @@ This directory contains partially transferred objects.
This is a temp directory for miscellaneous other temp files. This is a temp directory for miscellaneous other temp files.
While .git/annex/objects and .git/annex/tmp can be put on different
filesystems if desired, .git/annex/othertmp
has to be on the same filesystem as the work tree and git repository.
### `.git/annex/bad/` ### `.git/annex/bad/`
git-annex fsck puts any bad objects it finds in here. git-annex fsck puts any bad objects it finds in here.

View file

@ -0,0 +1,17 @@
[[!comment format=mdwn
username="joey"
subject="""re: why othertmp to be on the same file system?"""
date="2022-12-20T18:39:35Z"
content="""
I've audited the code and the only place I could find where it did not work
to have othertmp on a different filesystem is in the bittorrent special
remote when it downloads a torrent file. But that also failed when
`.git/annex/tmp` was on a different filesystem! (Since it was moving between
the two directories.) I've fixed that.
It's still best to keep things on the same filesystem because
cross-filesystem moves can be expensive and it sometimes falls back to less
ideal behavior in other ways too when operating across filesystems. Also
of course, you avoid being the one who gets to find and report breakage
like the above..
"""]]