From 7e5e505c0c647392f47a40f325dfc49660b4ddeb Mon Sep 17 00:00:00 2001 From: Kate Date: Mon, 24 Jan 2022 13:05:14 +0000 Subject: [PATCH] Remove unnecessary copies/move when fetching archives Co-authored-by: Raja Boujbel --- master_changes.md | 1 + src/repository/opamRepository.ml | 43 +++++++++++++++++++------------- tests/reftests/download.test | 32 ++++++++++-------------- tests/reftests/swhid.unix.test | 4 +-- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/master_changes.md b/master_changes.md index ca710438b6a..b0a1dd8ed71 100644 --- 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 index f321669e707..1ff3eddcca4 100644 --- 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 index f5e27ca965e..7a2f14d86c8 100644 --- 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 index 3b0d6ced65b..eb6e04d170d 100644 --- 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 <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>