From 98e8eb9678cb5d22abaaee206c7e7d75daec57d8 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Wed, 19 Jul 2023 18:11:40 +0200 Subject: [PATCH 1/4] gha: cygwin, add wget as needed for a test --- .github/scripts/cygwin.cmd | 3 +++ master_changes.md | 1 + 2 files changed, 4 insertions(+) diff --git a/.github/scripts/cygwin.cmd b/.github/scripts/cygwin.cmd index 9e438661c53..eb02db20b54 100644 --- a/.github/scripts/cygwin.cmd +++ b/.github/scripts/cygwin.cmd @@ -96,6 +96,9 @@ if "%1" equ "x86_64-pc-cygwin" ( :: is found set CYGWIN_PACKAGES=make,patch,curl,diffutils,tar,unzip,git,gcc-g++,libicu-devel%CYGWIN_PACKAGES% +:: wget is needed for download.test OPAMFETCH/OPAMCURL testing +set CYGWIN_PACKAGES=wget,%CYGWIN_PACKAGES% + :: D:\cygwin-packages is specified just to keep the build directory clean; the :: files aren't preserved. %CYGWIN_ROOT%\setup.exe --quiet-mode --no-shortcuts --no-startmenu --no-desktop --only-site --root %CYGWIN_ROOT% --site "%CYGWIN_MIRROR%" --local-package-dir D:\cygwin-packages --packages %CYGWIN_PACKAGES% diff --git a/master_changes.md b/master_changes.md index 1bb7ebe0772..10482a0b06a 100644 --- a/master_changes.md +++ b/master_changes.md @@ -115,6 +115,7 @@ users) ## Github Actions * Add coreutils install for cheksum validation tests [#5560 @rjbou] + * Add `wget` on Cygwin install [#5607 @rjbou] ## Doc From 92b2bba08782099287b125acbbd583713d81ce3f Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Tue, 12 Sep 2023 12:36:11 +0200 Subject: [PATCH 2/4] reftest: fix sed-cmd regexp to handle escaped '\' --- master_changes.md | 2 +- tests/reftests/run.ml | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/master_changes.md b/master_changes.md index 10482a0b06a..2aeb2d1122d 100644 --- a/master_changes.md +++ b/master_changes.md @@ -111,7 +111,7 @@ users) ### Engine * With real path resolved for all opam temp dir, remove `/private` from mac temp dir regexp [#5654 @rjbou] - * Reimplement `sed-cmd` command regexp, to handle prefixed commands with path not only in subprocess, but anywere in output [#5657 @rjbou] + * Reimplement `sed-cmd` command regexp, to handle prefixed commands with path not only in subprocess, but anywere in output [#5657 #5607 @rjbou] ## Github Actions * Add coreutils install for cheksum validation tests [#5560 @rjbou] diff --git a/tests/reftests/run.ml b/tests/reftests/run.ml index 265face0f44..60814411ee9 100644 --- a/tests/reftests/run.ml +++ b/tests/reftests/run.ml @@ -468,8 +468,9 @@ module Parse = struct *) opt_quoted @@ [ alpha; char ':'; - rep1 @@ seq [ char '\\'; rep1 @@ diff any (with_quote_set "\\") ]; - char '\\'; + rep1 @@ seq [ char '\\'; opt @@ char '\\'; + rep1 @@ diff any (with_quote_set "\\") ]; + char '\\'; opt @@ char '\\'; Re.str cmd; opt @@ Re.str ".exe"; ] in From 667b6edf7f598b62ac056b831c91ad9cb1bd4981 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Wed, 19 Jul 2023 12:50:21 +0200 Subject: [PATCH 3/4] reftest: add download test, to test OPAMFETCH & OPAMCURL handling --- master_changes.md | 1 + tests/reftests/download.test | 150 +++++++++++++++++++++++++++++++++++ tests/reftests/dune.inc | 18 +++++ 3 files changed, 169 insertions(+) create mode 100644 tests/reftests/download.test diff --git a/master_changes.md b/master_changes.md index 2aeb2d1122d..0ab90edb83a 100644 --- a/master_changes.md +++ b/master_changes.md @@ -108,6 +108,7 @@ users) * Add several checksum & cache validation checks for archive, extra-source section, and extra-file field [#5560 @rjbou] * Move local-cache into archive-field-checks test [#5560 @rjbou] * Admin: add `admin add-extrafiles` test cases [#5647 @rjbou] + * Add download test, to check `OPAMCURL/OPAMFETCH` handling [#5607 @rjbou] ### Engine * With real path resolved for all opam temp dir, remove `/private` from mac temp dir regexp [#5654 @rjbou] diff --git a/tests/reftests/download.test b/tests/reftests/download.test new file mode 100644 index 00000000000..1155197fd34 --- /dev/null +++ b/tests/reftests/download.test @@ -0,0 +1,150 @@ +N0REP0 +### :::::::::::::::::::::::::::::::::: +### :I: OPAMFETCH & OPAMCURL variables +### :::::::::::::::::::::::::::::::::: +### OPAMVERBOSE=2 +### opam --version >$ OPAMVERSION +### +opam-version: "2.0" +url { + src: "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz" + checksum: "md5=c9c157af4229fbb45d3f59f0d6d75dbe" +} +### opam switch create download --empty +### opam install foo --download-only | grep -v "^+-" | ".*\(curl\|wget\).* \"--\"" -> 'curl-or-wget --' | sed-cmd tar +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [foo.1: http] +curl-or-wget -- "https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz" +Processing 1/1: ++ tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}" +Done. +### opam clean -c +Clearing cache of downloaded files +### :I:a: FETCH wget +### OPAMFETCH=wget +### opam install foo --download-only | grep -v "^+-" | sed-cmd wget | sed-cmd tar | "${OPAMVERSION}" -> "current" +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [foo.1: http] ++ wget "--content-disposition" "-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" +Processing 1/1: ++ tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}" +Done. +### opam clean -c +Clearing cache of downloaded files +### :I:b: FETCH curl +### OPAMFETCH=curl +### opam install foo --download-only | grep -v "^+-" | sed-cmd curl | sed-cmd tar | "${OPAMVERSION}" -> "current" +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> 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" +Processing 1/1: ++ tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}" +Done. +### opam clean -c +Clearing cache of downloaded files +### :I:c: FETCH curl with args +### OPAMFETCH="curl --another-args %{retry}%" +### opam install foo --download-only | grep -v "^+-" | sed-cmd curl +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [foo.1: http] ++ curl "--another-args" "3" +[ERROR] Failed to get sources of foo.1: Curl failed + +OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (Curl failed: \"curl --another-args 3\" exited with code 2)") + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> +'${OPAM} install foo --download-only' failed. +# Return code 40 # +### opam clean -c +Clearing cache of downloaded files +### +#!/usr/bin/env bash +echo "***The curl is a lie*** [args: $@]" +### chmod +x bin/curl +### :I:d: local curl +### OPAMCURL=$BASEDIR/bin/curl +### opam install foo --download-only | grep -v "^+-" | sed-cmd curl +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [foo.1: http] ++ curl "--another-args" "3" +[ERROR] Failed to get sources of foo.1: Curl failed + +OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (Curl failed: \"curl --another-args 3\" exited with code 2)") + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> +'${OPAM} install foo --download-only' failed. +# Return code 40 # +### opam clean -c +Clearing cache of downloaded files +### :I:e: FETCH curl & local curl +### OPAMFETCH=curl OPAMCURL=$BASEDIR/bin/curl +### opam install foo --download-only | grep -v "^+-" | sed-cmd curl | sed-cmd tar | "${OPAMVERSION}" -> "current" +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> 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" +Processing 1/1: ++ tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}" +Done. +### opam clean -c +Clearing cache of downloaded files +### :I:f: FETCH curl with args & local curl +### OPAMFETCH="curl --another-args %{retry}%" OPAMCURL=$BASEDIR/bin/curl +### opam install foo --download-only | grep -v "^+-" | sed-cmd curl +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [foo.1: http] ++ curl "--another-args" "3" +[ERROR] Failed to get sources of foo.1: Curl failed + +OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (Curl failed: \"curl --another-args 3\" exited with code 2)") + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> +'${OPAM} install foo --download-only' failed. +# Return code 40 # +### opam clean -c +Clearing cache of downloaded files +### :I:g: FETCH wget & local curl +### OPAMFETCH=wget OPAMCURL=$BASEDIR/bin/curl +### opam install foo --download-only | grep -v "^+-" | sed-cmd wget | sed-cmd tar | "${OPAMVERSION}" -> "current" +The following actions will be performed: +=== install 1 package + - install foo 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [foo.1: http] ++ wget "--content-disposition" "-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" +Processing 1/1: ++ tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}" +Done. +### opam clean -c +Clearing cache of downloaded files diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index f2d47a3e02a..1261dcb4f7c 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -359,6 +359,24 @@ %{targets} (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:dot-install.test} %{read-lines:testing-env})))) +(rule + (alias reftest-download) + (action + (diff download.test download.out))) + +(alias + (name reftest) + (deps (alias reftest-download))) + +(rule + (targets download.out) + (deps root-N0REP0) + (package opam) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:download.test} %{read-lines:testing-env})))) + (rule (alias reftest-empty-conflicts-001) (action From 75a3a7a1efa10c2dd457f507b4ddac9e9ec1dcd5 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Wed, 19 Jul 2023 19:44:45 +0200 Subject: [PATCH 4/4] repository: fix OPAMFETCH/OPAMCURL handling OPAMCURL is ignored if OPAMFETCH is set. This is changed to handle OPAMCURL specification when OPAMFETCH is set to "curl" (simple command, or new arguments). If OPAMFETCH is set to another tool, OPAMCURL is ignored. --- master_changes.md | 1 + src/repository/opamRepositoryConfig.ml | 14 ++++++++++---- tests/reftests/download.test | 21 +++++++++++++-------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/master_changes.md b/master_changes.md index 0ab90edb83a..3c2b50005b0 100644 --- a/master_changes.md +++ b/master_changes.md @@ -54,6 +54,7 @@ users) * Allow to mark a set of warnings as errors using a new syntax -W @1..9 [#5652 @kit-ty-kate @rjbou - fixes #5651] ## Repository + * Fix `OPAMCURL` and `OPAMFETCH` handling [#5607 @rjbou - fix #5597] ## Lock diff --git a/src/repository/opamRepositoryConfig.ml b/src/repository/opamRepositoryConfig.ml index 373177e9f20..306f3be8237 100644 --- a/src/repository/opamRepositoryConfig.ml +++ b/src/repository/opamRepositoryConfig.ml @@ -122,13 +122,19 @@ let initk k = (CString cmd, None), `Default in let c = cmd :: List.map (fun a -> OpamTypes.CString a, None) a in - Some (lazy (c, kind)) + Some (c, kind) | [] -> None ) - >>+ fun () -> - E.curl () >>| (fun s -> - lazy ([CString s, None], `Curl)) + |> (fun fetch -> + match E.curl (), fetch with + | None, fetch -> OpamStd.Option.map Lazy.from_val fetch + | Some cmd, Some (((CIdent "curl"| CString "curl"), filter)::args, _) -> + Some (lazy ((CString cmd, filter)::args, `Curl)) + | Some cmd, None -> + Some (lazy ([CString cmd, None], `Curl)) + | Some _, _ -> (* ignored *) + OpamStd.Option.map Lazy.from_val fetch) in let validation_hook = E.validationhook () >>| fun s -> diff --git a/tests/reftests/download.test b/tests/reftests/download.test index 1155197fd34..fd7c9de4e21 100644 --- a/tests/reftests/download.test +++ b/tests/reftests/download.test @@ -88,9 +88,9 @@ The following actions will be performed: <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> Processing 1/1: [foo.1: http] + curl "--another-args" "3" -[ERROR] Failed to get sources of foo.1: Curl failed +[ERROR] Failed to get sources of foo.1: curl error code ***The curl is a lie*** [args: --another-args 3] -OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (Curl failed: \"curl --another-args 3\" exited with code 2)") +OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (curl: code ***The curl is a lie*** [args: --another-args 3] while downloading https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz)") <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> @@ -100,7 +100,7 @@ OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.t Clearing cache of downloaded files ### :I:e: FETCH curl & local curl ### OPAMFETCH=curl OPAMCURL=$BASEDIR/bin/curl -### opam install foo --download-only | grep -v "^+-" | sed-cmd curl | sed-cmd tar | "${OPAMVERSION}" -> "current" +### opam install foo --download-only | grep -v "^+-" | sed-cmd curl | "${OPAMVERSION}" -> "current" The following actions will be performed: === install 1 package - install foo 1 @@ -108,9 +108,14 @@ 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" -Processing 1/1: -+ tar "xfz" "${OPAMTMP}/v1.0.0.tar.gz" "-C" "${OPAMTMP}" -Done. +[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] + +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)") + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> +'${OPAM} install foo --download-only' failed. +# Return code 40 # ### opam clean -c Clearing cache of downloaded files ### :I:f: FETCH curl with args & local curl @@ -123,9 +128,9 @@ The following actions will be performed: <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> Processing 1/1: [foo.1: http] + curl "--another-args" "3" -[ERROR] Failed to get sources of foo.1: Curl failed +[ERROR] Failed to get sources of foo.1: curl error code ***The curl is a lie*** [args: --another-args 3] -OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (Curl failed: \"curl --another-args 3\" exited with code 2)") +OpamSolution.Fetch_fail("https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz (curl: code ***The curl is a lie*** [args: --another-args 3] while downloading https://github.com/UnixJunkie/get_line/archive/v1.0.0.tar.gz)") <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>