Remove unnecessary copies/move when fetching archives

ocaml · Jan 21, 2022 · 981e534 · 981e534
1 parent f70548b
commit 981e534
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 13 deletions.
diff --git a/master_changes.md b/master_changes.md
@@ -29,6 +29,7 @@ users)
 
 ## Install
   * Make the status of pinned packages more explicit during installation [#4987 @kit-ty-kate - fix #4925]
+  * Remove unnecessary copies/move when fetching archives [#5018 @kit-ty-kate]
 
 ## Remove
   *

diff --git a/src/repository/opamDownload.ml b/src/repository/opamDownload.ml
@@ -138,36 +138,34 @@ let really_download
     ?(quiet=false) ~overwrite ?(compress=false) ?checksum ?(validate=true)
     ~url ~dst () =
   assert (url.OpamUrl.backend = `http);
-  let tmp_dst = dst ^ ".part" in
-  if Sys.file_exists tmp_dst then OpamSystem.remove tmp_dst;
+  if Sys.file_exists dst then OpamSystem.remove dst;
   OpamProcess.Job.catch
     (function
       | Failure s as e ->
-        OpamSystem.remove tmp_dst;
+        OpamSystem.remove dst;
         if not quiet then OpamConsole.error "%s" s;
         raise e
       | e ->
-        OpamSystem.remove tmp_dst;
+        OpamSystem.remove dst;
         OpamStd.Exn.fatal e;
         log "Could not download file at %s." (OpamUrl.to_string url);
         raise e)
   @@ fun () ->
-  download_command ~compress ?checksum ~url ~dst:tmp_dst ()
+  download_command ~compress ?checksum ~url ~dst ()
   @@+ fun () ->
-  if not (Sys.file_exists tmp_dst) then
+  if not (Sys.file_exists dst) then
     fail (Some "Downloaded file not found",
           "Download command succeeded, but resulting file not found")
   else if Sys.file_exists dst && not overwrite then
     OpamSystem.internal_error "The downloaded file will overwrite %s." dst;
   if validate &&
      OpamRepositoryConfig.(!r.force_checksums <> Some false) then
     OpamStd.Option.iter (fun cksum ->
-        if not (OpamHash.check_file tmp_dst cksum) then
+        if not (OpamHash.check_file dst cksum) then
           fail (Some "Bad checksum",
                     Printf.sprintf "Bad checksum, expected %s"
                       (OpamHash.to_string cksum)))
       checksum;
-  OpamSystem.mv tmp_dst dst;
   Done ()
 
 let download_as ?quiet ?validate ~overwrite ?compress ?checksum url dst =

diff --git a/src/repository/opamRepository.ml b/src/repository/opamRepository.ml
@@ -285,15 +285,13 @@ let pull_tree
           Some ("missing checksum"),
           label ^ ": Missing checksum, and `--require-checksums` was set."))
     else
-      pull_from_mirrors label ?working_dir ?subpath cache_dir local_dirname checksums
+      OpamFilename.with_tmp_dir_job @@ fun tmpdir ->
+      pull_from_mirrors label ?working_dir ?subpath cache_dir tmpdir checksums
         remote_urls
       @@+ function
       | _, Up_to_date None -> Done (Up_to_date "no changes")
       | url, (Up_to_date (Some archive) | Result (Some archive)) ->
-        OpamFilename.with_tmp_dir_job @@ fun tmpdir ->
-        let tmp_archive = OpamFilename.(create tmpdir (basename archive)) in
-        OpamFilename.move ~src:archive ~dst:tmp_archive;
-        extract_archive tmp_archive (OpamUrl.to_string url)
+        extract_archive archive (OpamUrl.to_string url)
       | url, Result None -> Done (Result (OpamUrl.to_string url))
       | _, (Not_available _ as na) -> Done na