From 79e718ed91ec0732686e5f2818b837b9bdd4e49c Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 19 Jul 2023 19:44:45 +0200
Subject: [PATCH] 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 257f80afc58..4043210b8b2 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -53,6 +53,7 @@ users)
   * Fix extra-files handling when linting packages from repositories, see #5068 [#5639 @rjbou]
 
 ## 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 <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>