Add findlib toolchain handling for dune install.

[rizo]

Problem

For a given package $PKG, when explicitly providing a findlib toolchain with -x $TOOLCHAIN, dune install does two things that, in my understanding, are incorrect:

1. it will attempt to install targets for the default context, and not exclusively the targets for $TOOLCHAIN; and
2. it attempts to install:
   • _build/default.$TOOLCHAIN/$PKG.install instead of
   • _build/default.$TOOLCHAIN/$PKG-$TOOLCHAIN.install.

These two points can be validated currently on master by updating ./test/blackbox-tests/test-cases/cross-compilation/run.t as follows:

   $ env OCAMLFIND_CONF=$PWD/etc/findlib.conf dune install --prefix=_prefix --dry-run -x foo
+  Error: The following <package>.install are missing:
+  - _build/default/p.install
+  - _build/default.foo/p.install
+  Hint: try running: dune build @install
+  [1]

As stated in point 1, even though -x foo was passed, _build/default/p.install is expected to be present. And, as stated in point 2, the install file location is _build/default.foo/p.install, but should be _build/default.foo/p-foo.install. At least given my understanding of dune's path conventions for non-default toolchains.

Solution

See the proposed behaviour in the first commit 479158e for comparison with the current behaviour.

More specifically, in the presence of the -x option, the implementation of dune install:

• selects only the contexts that match the passed toolchain name; and
• resolves the package install filenames using the toolchain name.

Alternatives

Regarding 1

Keep the current functionality where dune install -x $TOOLCHAIN ... installs targets from the contexts that don't have the $TOOLCHAIN. As a workaround to only install the desired toolchain targets, dune install -x $TOOLCHAIN --context=default.$TOOLCHAIN can be used, assuming the default context.

I think this would be fine, even though the user has explicitly passed a toolchain. This behaviour is somewhat unexpected, but at least can be tuned.

Regarding 2

The naming convention for install files could be changed in the build folder. Currently the generated files (with --promote-install-files) are:

• ./_build/default.$TOOLCHAIN/$PKG-$TOOLCHAIN.install and;
• ./$PKG-$TOOLCHAIN.install

Instead, dune could generate ./_build/default.$TOOLCHAIN/$PKG.install internally. To my knowledge, no collisions are possible, because the default.$TOOLCHAIN context is used as a namespace.

This would also work, but introduces an unnecessary inconsistency for the install filenames.

[rizo]

Related to ocaml/opam#4204 and ocaml/opam#2476.

Am I correct in assuming that there's currently no way to instruct opam to install the $PKG-$TOOLCHAIN.install files?

[ghost]

This was designed for the ocaml-cross repositories where the package names are suffixed with -$TOOLCHAIN. For instance lwt becomes lwt-windows. /cc @toots

Regarding dune install -x TOOLCHAIN installing only the files for TOOLCHAIN, overall that seems reasonable but the behaviour needs to be consistent between dune build and dune install. More precisely, at a high-level the new interpretation seems to be: if you pass -x TOOLCHAIN, then default.TOOLCHAIN becomes the main build context and default becomes an "auxiliary context" that is only used if needed to run things on the current machine.

For instance, IIRC dune build -x windows blah will build both _build/default/blah and _build/default.windows/blah. With the change you propose, dune build -x windows blah should only build _build/default.windows/blah.

BTW, while the new interpretation seems fine, it is also technically a breaking change, so we need to assess the impact before going through it.

[rizo]

Thanks for the explanation, @jeremiedimino!

Indeed, I later noticed that build behaviour is inclusive. I personally prefer the proposed interpretation, where -x filters the context(s) being built. But I'm happy with either: changing the behaviour of build to match the proposed one or correcting the PR to install targets for both contexts.

Let me know what's your preference, I'm happy to implement it.

[ghost]

Well, I also like the proposed interpretation better, though I usually prefer to be strict about backward compatibility. That being said, I was reading the doc and the doc seems to suggest that if you have no dune-workspace file and you pass -x target, the native default build context is an auxiliary context. Given that plus the bug in dune install you pointed, I think I'm Ok considering that the current behaviour is a bug.

Just ccing @avsm and @samoht to make sure this won't impact mirage; right now you if you do dune build -x foo bah, dune will interpret that as requesting both _build/default/blah and _build/default.foo/blah. We'd like to change it to mean just _build/default.foo/blah. Is that better/worse/the same for the mirage workflow?

[avsm]

I appreciate you checking in on the Mirage usecase. I don't believe this would affect it, since we don't use -x (but instead generate a dune-workspace file with our specific flags). @samoht is leading the charge on this, so I'll let him confirm for sure.

[rgrinberg]

@samoht ping?

I'm also in favor of this change. @rizo would you be willing to change $ dune build -x as well?

[avsm]

@samoht is on vacation - I’m 99% sure this is fine, so go ahead and we will work around any (unlikely) issues in Mirage

[rizo]

I'm happy to give it a shot, @rgrinberg. Will try to submit a PR in a few days.

[rgrinberg]

Thanks. Feel free to ask questions if anything is unclear.

…

On Jul 29, 2020, 8:16 AM -0700, Rizo I ***@***.***>, wrote: I'm happy to give it a shot, @rgrinberg. Will try to submit a PR in a few days. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.