-
Notifications
You must be signed in to change notification settings - Fork 29
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
test dependencies are not resolved #36
Comments
Yeah, this isn't great and it may get worse - in the 0install working branch (a faster solver with fewer dependencies) I've turned off test dependencies entirely due to infinite recursion (one of dune-configurator's test deps transitively depens on dune-configurator). The current workaround is to add I think eventually it'd be good to have a mapping of packagename:deptype to enable dev / test dependencies or something like that, but it's fairly low priority, I've usually only needed it fore development dependencies at the toplevel, rather than some package deep in the tree. |
That works fine for me for projects that aren't libraries. But, if someone wanted to use opam2nix to build a library that's meant to end up on opam, or where the opam definition shouldn't be changed or its generated like with dune's new feature, should the normal ocaml nix rules be used instead? |
What does "the normal ocaml nix rules" mean? I don't touch the opam packages in upstream nixpkgs, but I assume they're just manually edited. |
Yea I was referring to the ones defined in nixpkgs using buildDunePackage. |
I think that would be pretty impossible to automate - even if the information were perfect I'm not sure how you'd extract it, but if you throw in that the nixpkgs packages might be named slightly differently and only apply to a single version of a package, it would likely just add confusion. Adding a way to specify "these deps should get their test deps" is definitely a more reasonable approach, it just needs... a bunch 'o code ;) |
Opam has issues handling this as well. I'm just spitballing here, and my experience with opam2nix is still minimal, but perhaps it could be possible to automatically derive a |
I've always imagined this would be solved by passing in some kind of mapping to influence the solver, e.g. But now that I think about it, maybe that's a good thing? Maybe you only want to bring test deps into the toplevel solve and not affect any transitive deps 🤔 In which case maybe it's enough to not mess with pseudo package names, but rather just pass |
I've always imagined this would be solved by passing in some kind of
mapping to influence the solver, e.g. `{ foo: { test: true } }`.
Something like `foo-test` feels more hacky, because it could only
affect the top-level solve (i.e. it would pull `foo`'s test
dependencies into the solve, but wouldn't cause
things-that-depend-on-`foo` to also depend on its test dependencies).
Isn't that the point? That's how I intended to break the dependency
cycle. If foo depends on bar, it does not mean that foo should depend on
bar-test.
But now that I think about it, maybe that's a good thing? Maybe you
only want to bring test deps into the toplevel solve and not affect
any transitive deps 🤔
Yes, test deps should not introduce any dependency edges between regular
packages.
In which case maybe it's enough to not mess with pseudo package names,
but rather just pass `--test` to the solver and it'll include the test
dependencies for all toplevel packages.
I'm not very familiar with the solver, but isn't that's essentially the
issue with opam's --with-test? The way I imagine it works is that it
ignores the {with-test} flags, and pretends that test dependencies are
now normal dependencies. This is the source for all the cycles.
|
Is there an extra flag required to resolve dependencies with a
{with-test}
qualifier? If I add the qualifier to a dependency, I get an input likealcotest = selection.alcotest or null;
, so it is considered optional. But the selection file doesn't contain the alcotest dependency itself, so it can't be found by dune.The text was updated successfully, but these errors were encountered: