dune

dune copied to clipboard

Fixes #10229

Done in https://github.com/ocaml/dune/commit/f39754471b057b26cc4c211a472142257be77533

Can you add a test case reproducing bug in a separate PR?

Can you add a test case reproducing bug in a separate PR?

Do we have those kind of tests ? The setup_roots are hard-coded in src/dune_rules/setup.ml.

I don't think this solve the issue. From a clean checkout of this branch:

(build commands are taken from dune.opam; I'm not sure which ones pkgsrc uses)

$ ./configure --mandir=share/man
$ ocaml boot/bootstrap.ml
$ ./_boot/dune.exe build dune.install --release --profile dune-bootstrap
$ PREFIX=fake_root make install
$ find fake_root share

This installs share/man outside fake_root, which is definitely not right. (these instructions in main create just fake_root with man directly in it).

build commands are taken from dune.opam; I'm not sure which ones pkgsrc uses)

I was missing this detail in the context of the PR. It's fixable. I'll add a test reproducing the bug in a separate PR and rework the PR. Thanks @emillon.

I don't know what's the right solution TBH. the original command uses a relative path for mandir, does that mean it should be interpreted relative to prefix? it would be fine to say that we don't accept relative paths here, for example. before jumping to a new implementation and look for a fix at all costs, I think we should answer these questions in order:

• is the original request something we want to support?
• how do we want to support that?
• how do implement that?

I don't know what's the right solution TBH. the original command uses a relative path for mandir, does that mean it should be interpreted relative to prefix? it would be fine to say that we don't accept relative paths here, for example. before jumping to a new implementation and look for a fix at all costs, I think we should answer these questions in order:

• is the original request something we want to support?
• how do we want to support that?
• how do implement that?

Considering the original request/issue https://us-central.manta.mnx.io/pkgsrc/public/reports/upstream-trunk/20240305.1526/ocaml-dune-3.11.1/install.log

I think if we give the ability to have relative dir for other arguments(--mandir,--docdir,...) when --destdir is mentioned. This could solve the issue.

I've tried this:

 |  $ echo "(lang dune 2.0)" > dune-project
 |  $ touch foo.opam manfile.1
 |  $ cat >dune <<EOF
 |  > (install
 |  >  (section man)
 |  >  (files manfile))
 |  > EOF
 |  $ dune build @install
 |  $ mkdir dest-dir
 |  $ dune install --prefix foo --destdir $PWD/dest-dir --mandir $PWD/dest-dir/foo/share/man
 |  $ tree . -a
+|  .
+|  |-- _build
+|  |   |-- .db
+|  |   |-- .digest-db
+|  |   |-- .filesystem-clock
+|  |   |-- .lock
+|  |   |-- default
+|  |   |   |-- .dune
+|  |   |   |   |-- configurator
+|  |   |   |   `-- configurator.v2
+|  |   |   |-- META.foo
+|  |   |   |-- foo.dune-package
+|  |   |   |-- foo.install
+|  |   |   |-- foo.opam
+|  |   |   `-- manfile.1
+|  |   |-- install
+|  |   |   `-- default
+|  |   |       |-- lib
+|  |   |       |   `-- foo
+|  |   |       |       |-- META -> ../../../../default/META.foo
+|  |   |       |       |-- dune-package -> ../../../../default/foo.dune-package
+|  |   |       |       `-- opam -> ../../../../default/foo.opam
+|  |   |       `-- man
+|  |   |           `-- man1
+|  |   |               `-- manfile.1 -> ../../../../default/manfile.1
+|  |   `-- log
+|  |-- dest-dir
+|  |   |-- foo
+|  |   |   `-- lib
+|  |   |       `-- foo
+|  |   |           |-- META
+|  |   |           |-- dune-package
+|  |   |           `-- opam
+|  |   `-- home
+|  |       `-- alpha
+|  |           `-- hack
+|  |               `-- moyodiallo
+|  |                   `-- dune
+|  |                       `-- _build
+|  |                           `-- .sandbox
+|  |                               `-- f16b8a100c24456a3e62670d954d5b3c
+|  |                                   `-- default
+|  |                                       `-- test
+|  |                                           `-- blackbox-tests
+|  |                                               `-- test-cases
+|  |                                                   `-- install-dir
+|  |                                                       `-- dest-dir
+|  |                                                           `-- foo
+|  |                                                               `-- share
+|  |                                                                   `-- man
+|  |                                                                       `-- man1
+|  |                                                                           `-- manfile.1
+|  |-- dune
+|  |-- dune-project
+|  |-- foo.opam
+|  `-- manfile.1 

home/../../ ../_build/.../.. share/man/man1/manfile directory appears in dest-dir what is following the logic because the destination is known. In the other hand --mandir with relative dir gives :

dune: option '--mandir': the path must be absolute to avoid ambiguity
+|  Usage: dune install [OPTION]… [PACKAGE]…
+|  Try 'dune install --help' or 'dune --help' for more information.

EDITED for more contextualization.

I remember this being discussed in the team meeting. The verdict was: all paths passed in the command line should be interpreted to relative to CWD