Remove unnecessary copies/move when fetching archives

Co-authored-by: Raja Boujbel <raja.boujbel@ocamlpro.com>
ocaml · Aug 8, 2024 · 7e5e505 · 7e5e505
1 parent 2da9860
commit 7e5e505
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 39 deletions.
diff --git a/master_changes.md b/master_changes.md
@@ -36,6 +36,7 @@ users)
 
 ## Build (package)
   * ◈ Add `--verbose-on` option to enable verbose mode on specified package names [#5682 @desumn @rjbou]
+  * Remove unnecessary copies/move when fetching archives [#5018 @kit-ty-kate @rjbou]
 
 ## Remove
 

diff --git a/src/repository/opamRepository.ml b/src/repository/opamRepository.ml
@@ -355,32 +355,39 @@ let pull_tree_t
           label ^ ": Missing checksum, and `--require-checksums` was set."))
     else
       OpamFilename.with_tmp_dir_job @@ fun tmpdir ->
-      let extract url archive =
-        match dirnames with
-        | [_] ->
-          let tmp_archive = OpamFilename.(create tmpdir (basename archive)) in
-          OpamFilename.move ~src:archive ~dst:tmp_archive;
-          extract_archive tmp_archive url
-        | _ -> extract_archive archive url
-      in
-      let pull label checksums remote_urls =
-        match dirnames with
-        | [ label, local_dirname, subpath ] ->
-          pull_from_mirrors label ?full_fetch ?working_dir ?subpath
-            cache_dir local_dirname checksums remote_urls
+      (* We need to check if the url can be an archive or not, to know if it
+         need to be downloaded directly in the source directory [local_dirname]
+         or in temporary one [tmpdir] to extract it in sources directory *)
+      let pull =
+        let label0, destdir, subpath =
+          match dirnames with
+          | [ label, local_dirname, subpath ] ->
+            let need_local_dirname =
+              List.for_all OpamUrl.(fun u ->
+                  match u.backend with
+                  | #version_control -> true
+                  | `http -> false
+                  | `rsync -> local_dir u <> None)
+                remote_urls
+            in
+            Some label,
+            (if need_local_dirname then local_dirname else tmpdir),
+            subpath
+          | _ -> None, tmpdir, None
+        in
+        fun label checksums remote_urls ->
+          pull_from_mirrors (OpamStd.Option.default label label0)
+            ?full_fetch ?working_dir ?subpath cache_dir destdir
+            checksums remote_urls
           @@| fun (url, res) ->
           (OpamUrl.to_string_w_subpath subpath url),
           res
-        | _ ->
-          pull_from_mirrors label ?full_fetch ?working_dir cache_dir tmpdir
-            checksums remote_urls
-          @@| fun (url, res) -> OpamUrl.to_string url, res
       in
       pull label checksums remote_urls
       @@+ function
       | _, Up_to_date None -> Done (Up_to_date "no changes")
       | url, (Up_to_date (Some archive) | Result (Some archive)) ->
-        extract url archive
+        extract_archive archive url
       | url, Result None -> Done (Result url)
       | _, (Not_available _ as na) -> Done na
 

diff --git a/tests/reftests/download.test b/tests/reftests/download.test
@@ -33,7 +33,7 @@ The following actions will be performed:
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 Processing  1/1: [foo.1: http]
-+ wget "-t" "3" "-O" "${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/v1.0.0.tar.gz.part" "-U" "opam/current" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
++ wget "-t" "3" "-O" "${OPAMTMP}/v1.0.0.tar.gz.part" "-U" "opam/current" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
 Processing  1/1: [foo.1: extract]
 + tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}"
 Done.
@@ -48,7 +48,7 @@ The following actions will be performed:
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 Processing  1/1: [foo.1: http]
-+ curl "--write-out" "%{http_code}\n" "--retry" "3" "--retry-delay" "2" "--user-agent" "opam/current" "-L" "-o" "${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/v1.0.0.tar.gz.part" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
++ curl "--write-out" "%{http_code}\n" "--retry" "3" "--retry-delay" "2" "--user-agent" "opam/current" "-L" "-o" "${OPAMTMP}/v1.0.0.tar.gz.part" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
 Processing  1/1: [foo.1: extract]
 + tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}"
 Done.
@@ -107,10 +107,10 @@ The following actions will be performed:
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 Processing  1/1: [foo.1: http]
-+ curl "--write-out" "%{http_code}\n" "--retry" "3" "--retry-delay" "2" "--user-agent" "opam/current" "-L" "-o" "${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/v1.0.0.tar.gz.part" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
-[ERROR] Failed to get sources of foo.1: curl error code ***The curl is a lie*** [args: --write-out %{http_code}\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/v1.0.0.tar.gz.part -- https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz]
++ curl "--write-out" "%{http_code}\n" "--retry" "3" "--retry-delay" "2" "--user-agent" "opam/current" "-L" "-o" "${OPAMTMP}/v1.0.0.tar.gz.part" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
+[ERROR] Failed to get sources of foo.1: curl error code ***The curl is a lie*** [args: --write-out %{http_code}\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/v1.0.0.tar.gz.part -- https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz]
 
-OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (curl: code ***The curl is a lie*** [args: --write-out %{http_code}\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/v1.0.0.tar.gz.part -- https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz] while downloading https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz)")
+OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (curl: code ***The curl is a lie*** [args: --write-out %{http_code}\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/v1.0.0.tar.gz.part -- https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz] while downloading https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz)")
 
 
 <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -147,7 +147,7 @@ The following actions will be performed:
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 Processing  1/1: [foo.1: http]
-+ wget "-t" "3" "-O" "${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/v1.0.0.tar.gz.part" "-U" "opam/current" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
++ wget "-t" "3" "-O" "${OPAMTMP}/v1.0.0.tar.gz.part" "-U" "opam/current" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
 Processing  1/1: [foo.1: extract]
 + tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}"
 Done.
@@ -186,18 +186,15 @@ The following actions will be performed:
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 SYSTEM                          rmdir ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1
 SYSTEM                          mkdir ${OPAMTMP}
-SYSTEM                          mkdir ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1
 Processing  1/1: [foo.1: rsync]
-+ rsync "-rLptgoDvc" "--exclude" ".git" "--exclude" "_darcs" "--exclude" ".hg" "--exclude" ".#*" "--exclude" "_opam*" "--exclude" "_build" "--delete" "--delete-excluded" "${BASEDIR}/arch.tgz" "${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1"
++ rsync "-rLptgoDvc" "--exclude" ".git" "--exclude" "_darcs" "--exclude" ".hg" "--exclude" ".#*" "--exclude" "_opam*" "--exclude" "_build" "--delete" "--delete-excluded" "${BASEDIR}/arch.tgz" "${OPAMTMP}"
 SYSTEM                          mkdir ${BASEDIR}/OPAM/download-cache/md5-dir/
-SYSTEM                          copy ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/arch.tgz -> ${BASEDIR}/OPAM/download-cache/md5-dir/
-SYSTEM                          mv ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1/arch.tgz -> ${OPAMTMP}/arch.tgz
-SYSTEM                          rmdir ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1
-SYSTEM                          mkdir ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1
+SYSTEM                          copy ${OPAMTMP}/arch.tgz -> ${BASEDIR}/OPAM/download-cache/md5-dir/
 SYSTEM                          mkdir ${OPAMTMP}
 Processing  1/1: [foo.1: extract]
 + tar "xfz" "${OPAMTMP}/arch.tgz" "-C" "${OPAMTMP}"
 SYSTEM                          copydir ${OPAMTMP}/REPO -> ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1
+SYSTEM                          mkdir ${BASEDIR}/OPAM/download/.opam-switch/sources/foo.1
 SYSTEM                          rmdir ${OPAMTMP}
 SYSTEM                          rmdir ${OPAMTMP}
 Done.
@@ -262,19 +259,16 @@ The following actions will be performed:
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 SYSTEM                          rmdir ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1
 SYSTEM                          mkdir ${OPAMTMP}
-SYSTEM                          mkdir ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1
 Processing  1/1: [baz.1: http]
-+ wget "-t" "3" "-O" "${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1/v1.0.0.tar.gz.part" "-U" "opam/current" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
-SYSTEM                          mv ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1/v1.0.0.tar.gz.part -> ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1/v1.0.0.tar.gz
++ wget "-t" "3" "-O" "${OPAMTMP}/v1.0.0.tar.gz.part" "-U" "opam/current" "--" "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz"
+SYSTEM                          mv ${OPAMTMP}/v1.0.0.tar.gz.part -> ${OPAMTMP}/v1.0.0.tar.gz
 SYSTEM                          mkdir ${BASEDIR}/OPAM/download-cache/md5-dir/
-SYSTEM                          copy ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1/v1.0.0.tar.gz -> ${BASEDIR}/OPAM/download-cache/md5-dir/
-SYSTEM                          mv ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1/v1.0.0.tar.gz -> ${OPAMTMP}/v1.0.0.tar.gz
-SYSTEM                          rmdir ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1
-SYSTEM                          mkdir ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1
+SYSTEM                          copy ${OPAMTMP}/v1.0.0.tar.gz -> ${BASEDIR}/OPAM/download-cache/md5-dir/
 SYSTEM                          mkdir ${OPAMTMP}
 Processing  1/1: [baz.1: extract]
 + tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}"
 SYSTEM                          copydir ${OPAMTMP}/get_line-1.0.0 -> ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1
+SYSTEM                          mkdir ${BASEDIR}/OPAM/download/.opam-switch/sources/baz.1
 SYSTEM                          rmdir ${OPAMTMP}
 SYSTEM                          rmdir ${OPAMTMP}
 Done.

diff --git a/tests/reftests/swhid.unix.test b/tests/reftests/swhid.unix.test
@@ -39,7 +39,7 @@ The following actions will be performed:
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 [ERROR] Failed to get sources of snappy-ko.2: Download command failed
 
-OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (Download command failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${BASEDIR}/OPAM/fallback/.opam-switch/sources/snappy-ko.2/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)")
+OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (Download command failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)")
 
 
 <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -68,7 +68,7 @@ The following actions will be performed:
 Processing  1/3: [snappy-swhid-dir.2: http]
 [ERROR] Failed to get sources of snappy-swhid-dir.2: Download command failed
 
-OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (Download command failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${BASEDIR}/OPAM/fallback/.opam-switch/sources/snappy-swhid-dir.2/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)")
+OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (Download command failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)")
 
 
 <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>