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

diff --git a/Annex/YoutubeDl.hs b/Annex/YoutubeDl.hs
index c6d0f2253eb527789bb52a07a8a972416dcf7d62..854acb9dadbffc7889bfa2d558abe135da73ad25 100644 (file)
--- a/Annex/YoutubeDl.hs
+++ b/Annex/YoutubeDl.hs
@@ -144,7 +144,7 @@ youtubeDlTo key url dest p = do
        res <- withTmpWorkDir key $ \workdir ->
                youtubeDl url (fromRawFilePath workdir) p >>= \case
                        Right (Just mediafile) -> do
-                               liftIO $ renameFile mediafile dest
+                               liftIO $ moveFile (toRawFilePath mediafile) (toRawFilePath dest)
                                return (Just True)
                        Right Nothing -> return (Just False)
                        Left msg -> do

diff --git a/CHANGELOG b/CHANGELOG
index 709790fdb5564cbc10c1b0d2a3655e2759a9ffc5..75d1b6eaa44ee74a6d3bbfce9acac2c2a22d1de8 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,9 @@ git-annex (10.20221213) UNRELEASED; urgency=medium
   * Change --metadata comparisons < > <= and >= to fall back to
     lexicographical comparisons when one or both values being compared
     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
 

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

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

diff --git a/doc/internals.mdwn b/doc/internals.mdwn
index 6a1cf9f2d6eb99437782deba9dab7ee59f80a2f3..47fd0b55cb462734573ca67d9c074a61d025a086 100644 (file)
--- a/doc/internals.mdwn
+++ b/doc/internals.mdwn
@@ -28,10 +28,6 @@ This directory contains partially transferred objects.
 
 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 fsck puts any bad objects it finds in here.

diff --git a/doc/internals/comment_18_1adce7945940b9c384c2383261388dd9._comment b/doc/internals/comment_18_1adce7945940b9c384c2383261388dd9._comment
new file mode 100644 (file)
index 0000000..533b703
--- /dev/null
+++ b/doc/internals/comment_18_1adce7945940b9c384c2383261388dd9._comment
@@ -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..
+"""]]