Merge pull request #5607 from rjbou/opamcurl

Fix `OPAMFETCH`/`OPAMCURL` handling

.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%

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
 
@@ -108,13 +109,15 @@ 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]
-  * 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]
+  * Add `wget` on Cygwin install [#5607 @rjbou]
 
 ## Doc
 

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 ->

tests/reftests/download.test

@@ -0,0 +1,155 @@
+N0REP0
+### ::::::::::::::::::::::::::::::::::
+### :I: OPAMFETCH & OPAMCURL variables
+### ::::::::::::::::::::::::::::::::::
+### OPAMVERBOSE=2
+### opam --version >$ OPAMVERSION
+### <pkg:foo.1>
+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
+### <bin/curl>
+#!/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 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: 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 <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+'${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 | "${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"
+[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
+### 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 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: 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 <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+'${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

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

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