From 981e534d988dbdc1a4b09d20291ad7fc863d7d96 Mon Sep 17 00:00:00 2001 From: Kate Date: Fri, 21 Jan 2022 22:15:42 +0000 Subject: [PATCH] Remove unnecessary copies/move when fetching archives --- master_changes.md | 1 + src/repository/opamDownload.ml | 14 ++++++-------- src/repository/opamRepository.ml | 8 +++----- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/master_changes.md b/master_changes.md index 3ede4510132..8fbafba49c1 100644 --- 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 index 38e54db8d08..0d59b7740ab 100644 --- a/src/repository/opamDownload.ml +++ b/src/repository/opamDownload.ml @@ -138,23 +138,22 @@ 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 @@ -162,12 +161,11 @@ let really_download 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 index 85a0ea7d56d..553d8a75643 100644 --- 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