search

tweag / opam-nix

Turn opam-based OCaml projects into Nix derivations
MIT License
111 stars 33 forks source link

fake-opam is overriding actual opam #23

Closed rgrinberg closed 2 years ago

rgrinberg commented 2 years ago

Describe the bug

opam-nix provides a fake opam binary. This binary overrides the actual opam binary which is necessary for dune's test suite.

To Reproduce

Consider the base commit in this PR[https://github.com/ocaml/dune/pull/6180] and run

$ nix develop --command opam
  2.0.0
balsoft commented 2 years ago

I have slightly improved the fake opam in bd348d2229f3cbc82a0cb01999dbcd207295ea5d, not sure if that helps.

balsoft commented 2 years ago

I don't think there's a good general way to have fake-opam shadowed by real opam if it's present in the scope, since "real" opam won't work the majority of the time, and providing it may trick packages into thinking it is available even if it's not. That said, I will experiment and see how often this really is a problem.

rgrinberg commented 2 years ago

What's the purpose of faking out opam anyway?

balsoft commented 2 years ago

dune sometimes calls opam --version for whatever reason, and fails if it's not available; Also, some build scripts shell out to e.g. opam subst or opam config var to get some variables. This is annoying, but somewhat unavoidable in a big ecosystem I guess. We have to work around that, by proving a fake opam binary. Random example off the top of my head: https://github.com/mirage/ocaml-gmp/pull/18 (this PR removes the dependency, but it was there)

rgrinberg commented 2 years ago

Why not provide a real opam binary though?

rgrinberg commented 2 years ago

Also, dune does run opam to see if it's available, but it works just fine even if opam isn't available.

balsoft commented 2 years ago

Why not provide a real opam binary though?

Because it won't actually work (since there's no switch or any other state available).

Also, dune does run opam to see if it's available, but it works just fine even if opam isn't available.

Good to know. I think it wasn't the case for some packages, but I can't remember which ones now :( Might have been related to dune-configurator, now that I think about it.

rgrinberg commented 2 years ago

Because it won't actually work (since there's no switch or any other state available).

Right, but that's not the only use case. For example, we need opam in dune's flake to test our opam integration. We don't need the switch state to be setup.

balsoft commented 2 years ago

Would it help if there was a way to disable fake-opam per-derivation?

balsoft commented 2 years ago

You can disable this by doing overrideAttrs (_: { withFakeOpam = false; }) .