search

ocaml / opam

opam is a source-based package manager. It supports multiple simultaneous compiler installations, flexible package constraints, and a Git-friendly development workflow.
https://opam.ocaml.org
Other
1.21k stars 348 forks source link

Fix downloading URLs with invalid characters #5921

Closed dra27 closed 2 months ago

dra27 commented 3 months ago

OpamDownload.download downloads a given URL using the basename of the URL as the filename. On Windows, where are there are many restrictions on valid filenames, this causes a problem if the URL includes any query string. Since OpamDownload passes the name used to the continuation, on Windows the illegal characters are simply replaced with underscores.

dra27 commented 2 months ago

It's worth the test case regardless, but this isn't disambiguating the final name - it's the intermediate name which if I traced the code through correctly is always done in an empty temporary directory. For example, in:

patches: ["zstd-detection.patch"]
extra-source "zstd-detection.patch" {
  src: "https://github.com/ocaml/ocaml/commit/baf65b91c51bb04b09ecc98b94ddd4ba3b446912.patch?full_index=1"
  checksum: "sha256=958e061bc3b967e32a5606d5109ed7faacb9b793fe2de0e8f8697c23a178c5cf"
}

the final name (zstd-detection.patch) must be Windows-compatible and there's no automatic renaming (ever) taking place there. The issue is that while downloading, opam downloads https://github.com/ocaml/ocaml/commit/baf65b91c51bb04b09ecc98b94ddd4ba3b446912.patch?full_index=1 to a file called baf65b91c51bb04b09ecc98b94ddd4ba3b446912.patch?full_index=1 before moving it to zstd-detection.patch

kit-ty-kate commented 2 months ago

oh i see! Then I'm wondering, does the name have to be similar as the original? Why not simply have one static name that's always going to be the same?