dune
dune copied to clipboard
ocaml
[Vendored Opam] fix partial-match warnings by removing a lot of dead code
Fixes #10216.
The only functions in OpamRepository.ml that are used externally are pull_file and pull_tree. I removed everything else, as we now have our own fetching code in dune_pkg
I am honestly not sure of this. I agree that these warnings are annoying but I am also not fond of modifying the vendored code, since it makes it harder and harder to resync with OPAM upstream, especially if they're not part of our vendored OPAM repo (which is probably also not up to date with current dune).
I think a better way would be to extend the bootstrap process to understand vendored dependencies and make sure they are treated in a (more) similar way to how they are treated in the standard dune build, i.e. do not run with our extended set of warnings.
I agree that these warnings are annoying but I am also not fond of modifying the vendored code, since it makes it harder and harder to resync with OPAM upstream, especially if they're not part of our vendored OPAM repo (which is probably also not up to date with current dune).
For context, we added these warnings by modifying the vendored code. So while I agree with you that we'd need a more principled way to make changes to our vendored opam code, ignoring the warnings is not the solution here.
(in other words, our aim should be decreasing the diff with upstream, not removing code; but in that case that might be aligned)
The code for downloading/extracting diffs isn't going to be shared, so in this case, removal should work. Is there a reason why the entire modules can't be just removed? Or replaced with just stubs.
Hi, this PR is important for opam-health-check-ng to work properly and generally pretty annoying to see when compiling opam from scratch (where dune is vendored), can someone take over this PR and put it over the line?
If nobody has time to do it i can try to do it, but reading the discussion above, I'm not sure to see what the consensus is on what to do.
You can either just revert all of our changes to opam related to this feature or continue along with this PR and remove the code altogether. Both are fine with me. I lean towards removal as there's no plans to share this code anymore and it will reduce the build times for us.