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

buildDunePackage: use dune install instead of opaline #106150

Merged
merged 1 commit into from Dec 20, 2020

Conversation

sternenseemann
Copy link
Member

Motivation for this change

By using dune install over opaline, we make dune (and of course ocaml and findlib) the sole "static" dependency of any dune package and thus get rid of circular dependency (or bootstrapping) issues because of opaline. More specifically: currently we need to build opaline and all its dependencies opam-file-format without dune. Also we can't enable tests for opam-file-format since alcotest is built using buildDunePackage. This would be no issue if we use dune's install implementation.

I've tested this change using nixpkgs-review so far and it doesn't break anything new in the standard ocamlPackages set.

I'm wondering whether this should be PR'd against staging rather than master since it changes around 600 ocamlPackages which all exist in multiple versions.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@vbgl
Copy link
Contributor

vbgl commented Dec 13, 2020

The JSoO stuff should go away once #106343 is merged.

Also, I don’t think that merging into staging first makes much sense, as hydra does not build so many OCaml packages.

@sternenseemann
Copy link
Member Author

Alright! I'll rebase the js_of_ocaml PR as soon as the janeStreet stuff has been merged and then we can progress to this PR and so on.

@sternenseemann
Copy link
Member Author

Rebased against master, js_of_ocaml commits are removed now.

@vbgl vbgl merged commit 640d925 into NixOS:master Dec 20, 2020
@sternenseemann sternenseemann deleted the dune-test branch December 20, 2020 23:44
@sternenseemann sternenseemann restored the dune-test branch July 24, 2021 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants