Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken use of curl in 2.2.0~alpha3 on native Windows #5681

Closed
jonahbeckford opened this issue Sep 20, 2023 · 8 comments
Closed

Broken use of curl in 2.2.0~alpha3 on native Windows #5681

jonahbeckford opened this issue Sep 20, 2023 · 8 comments

Comments

@jonahbeckford
Copy link
Contributor

jonahbeckford commented Sep 20, 2023

Assets

Problem

Context: curl has always been broken as a download method on native Windows. So after opam init I always do a opam option --yes --global download-command=wget in DkML.

With opam.2.2.0~alpha0 (commits until 2022-12-28) I could do the following without a problem:

$ with-dkml opam init --yes --no-setup --bare --disable-sandboxing default https://opam.ocaml.org

# The following command is executed only if repo/default or repo/default.tar.gz was not created
# - It is created in 2.2.0~alpha0 so this won't execute. Some earlier versions of opam didn't though.
$ with-dkml opam repository add default https://opam.ocaml.org --yes --dont-select --rank=3

$ with-dkml opam option --yes --global download-command=wget

Now with opam.2.2.0~alpha3 (commits until 2023-09-18) I have to use --no-cygwin-setup (which is fine) but I get an error:

$ with-dkml opam init --yes --no-setup --bare --disable-sandboxing --no-cygwin-setup
[WARNING] Flag --no-cygwin-setup is experimental, there is no guarantee that it will be kept; avoid using it in scripts.
No configuration file found, using built-in defaults.
Checking for available remotes: rsync and local, git.
  - you won't be able to use mercurial repositories unless you install the hg command on your system.
  - you won't be able to use darcs repositories unless you install the darcs command on your system.


<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><>  🐫
[ERROR] Could not update repository "default": OpamDownload.Download_fail(_, "curl: code %http_coden while downloading https://opam.ocaml.org/index.tar.gz")
[ERROR] Initial download of repository failed.
@jonahbeckford
Copy link
Contributor Author

Blocks verification of #5673

@jonahbeckford
Copy link
Contributor Author

jonahbeckford commented Sep 20, 2023

Root cause

The opam.2.2.0~alpha0 version had debug logs:

00:00.218  PARALLEL               Next task in job 0: C:\Users\beckf\AppData\Local\Programs\DKMLNA~1\tools\MSYS2\usr\bin\curl.exe --write-out %{http_code}\n --retry 3 --retry-delay 2 --user-agent opam/2.2.0~alpha0~20221228 -L -o C:\Users\beckf\AppData\Local\Temp\opam-32908-84161f\index.tar.gz.part -- https://opam.ocaml.org/index.tar.gz
00:00.835  PROC                   cygvoke(glob): "C:\Users\beckf\AppData\Local\Programs\DKMLNA~1\tools\MSYS2\usr\bin\curl.exe" "--write-out" "%{http_code}\n" "--retry" "3" "--retry-delay" "2" "--user-agent" "opam/2.2.0~alpha0~20221228" "-L" "-o" "C:\Users\beckf\AppData\Local\Temp\opam-32908-84161f\index.tar.gz.part" "--" "https://opam.ocaml.org/index.tar.gz"
Processing  1/1: [default: http]
00:03.573  PARALLEL               Collected task for job 0 (ret:0)

But opam.2.2.0~alpha3 has:

00:00.102  PARALLEL               Next task in job 0: C:\Users\beckf\AppData\Local\Programs\DKMLNA~1\tools\MSYS2\usr\bin\curl.exe --write-out %{http_code}\n --retry 3 --retry-delay 2 --user-agent opam/2.2.0~alpha3~dev -L -o C:\Users\beckf\AppData\Local\Temp\opam-14628-ac7d23\index.tar.gz.part -- https://opam.ocaml.org/index.tar.gz
Processing  1/1: [default: http]
00:02.702  PARALLEL               Collected task for job 0 (ret:0)
...
[ERROR] Could not update repository "default": OpamDownload.Download_fail(_, "curl: code %http_coden while downloading https://opam.ocaml.org/index.tar.gz")

I guess that is unsurprising since for DkML I have to use --no-cygwin-setup, and that is needed for the cygvoke(glob).

The \n in %{http_code}\n won't be interpreted as a newline in the Command Prompt.

@jonahbeckford
Copy link
Contributor Author

jonahbeckford commented Sep 20, 2023

So ... can we do one of the following (or some or all):

  1. Provide a command line option to use wget rather than curl in opam init?
  2. Provide a command line option to skip the download of the initial repository in opam init?
  3. Fix curl so that it can reliably pass the newline character on Windows, or remove the need for the newline character?
  4. Provide a opam init --msys2-location=DIR option, just like there is a opam init --cygwin-location=DIR option.

@kit-ty-kate
Copy link
Member

I use curl on Windows and the master branch and I haven't seen this issue. Is the MSYS2 version of curl known to be more buggy compared to Cygwin?

Actually re-reading myself as I'm writing makes me think mixing MSYS2 and Cygwin is the problem.
Could you try again with the Cygwin version installed?
I think opam automatically puts cygwin into the PATH first but I'm not entirely sure. @rjbou is this what we are doing?

@jonahbeckford
Copy link
Contributor Author

jonahbeckford commented Sep 20, 2023

I definitely can't use the Cygwin version ... there is no Cygwin in DkML, and Cygwin would cause fatal conflicts with MSYS2 anyway.

Hence my use of --no-cygwin-setup.

But you just made me realize that if there was a --msys2-location=DIR option then this could be avoided. @rjbou : I remember we discussed something like that, but unclear if that was on the plate for 2.2. I'll add it as the fourth option in my list of solutions.

Edit: Also added "Assets" to issue description so can repeat what I did.

@rjbou
Copy link
Collaborator

rjbou commented Sep 20, 2023

I think opam automatically puts cygwin into the PATH first but I'm not entirely sure. @rjbou is this what we are doing?

For init, it is Windows curl that is used. Once cygwin configured, it is set at the beginning of the path, yes.

Some comments and answers on what you proposed.

  1. Provide a command line option to use wget rather than curl in opam init?

It is possible to do that with 2 ways:

  • Set OPAMFETCH=wget variable, opam should take it even for init, but it will be needed for each invocation (fixed in master, see Fix OPAMFETCH/OPAMCURL handling #5607).
  • Provide a custom opamrc with download-command: "wget" at init with option --config. It will have the same effect than setting via opam option afterwards.
  1. Provide a command line option to skip the download of the initial repository in opam init?

It is not possible for the moment to have a repository without its content. There is some work on that, see #4617.

  1. Fix curl so that it can reliably pass the newline character on Windows, or remove the need for the newline character?

I tried to update curl quoting for some windows run, i ended up having something that worked. I'm not thrilled by the solution, but we could add a conditioned curl invocation: if cywgin || unix, current quoting, otherwise windows quoting.

  1. Provide a opam init --msys2-location=DIR option, just like there is a opam init --cygwin-location=DIR option.
    But you just made me realize that if there was a --msys2-location=DIR option then this could be avoided. @rjbou : I remember we discussed something like that, but unclear if that was on the plate for 2.2. I'll add it as the fourth option in my list of solutions.

Yes, we discussed that, but i don't remember the conclusion (we didn't took notes:)). We began with that, and end up on a maybe wrapper as with-dkml fits is better ? If in you path, you have msys2 binary path available, opam should take that curl.
Anyway, it is something that can be done in the same way than Cygwin one.

@jonahbeckford
Copy link
Contributor Author

I am quite fine with OPAMFETCH=wget ... it is only for the initial opam init.

And it gets past the curl issue, so technically this issue can be closed.

However, opam init fails at a later point, and I do now think that --msys2-location=DIR is unavoidable. I'll open a new issue for that.

@jonahbeckford
Copy link
Contributor Author

Resolved with OPAMFETCH=wget. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants