Dune subst now tries to guess the project root

[NathanReb]

Expected Behavior

dune subst should always treat the current working directory as the root of the dune workspace.

Actual Behavior

dune subst which is called for pinned opam packages now tries to guess the root of the project. With pinned packages in a local opam switch, this means that it went all the way back up from inside the ./_opam/.opam-switch/build/<package> to my ./dune-project and failed, for unrelated reasons which allowed me to find out about this bug.

It could potentially modify files that it shouldn't modify, use the wrong git history etc...

Reproduction

mkdir test-project
cd test-project
echo "(lang dune 2.9)" > dune-project
opam switch create ./ 4.11.1
opam pin dune --dev
opam pin csexp --dev

The installation of csexp should fail on the dune subst step because of the local dune-project.

Specifications

• Version of dune (output of dune --version): 2.7.0-428-g00496d33d3
• Version of ocaml (output of ocamlc --version): 4.11.1
• Operating system (distribution and version): 5.5.0-2-amd64 Feature request: generate .merlin files #1 SMP Debian 5.5.17-1 (2020-04-15) x86_64 GNU/Linux

[NathanReb]

I'll try to bisect this later today!

[bobot]

Could we ask that --root . be given to dune for this use case?

[NathanReb]

This will potentially break pinning for every single opam package, including the ones that have been using dune opam file generation so far so I'd advise against this.

We can start generating "dune" "subst" "--root" "./" {pinned} as build instruction from now on but I think the old behaviour should be preserved until reaching 3.x.

I'm not yet quite clear how it will behave depending on the context but I'm pretty sure it can silently mess things up when pinning a package and give the users a hard time figuring out what's going on.

That being said, this decision is not mine to make but I think by doing so we would effictively introduce a universe split as for 2.7.1 and lower, --root is not recognized by dune subst. Meaning a package would have to choose between adding the --root and use dune.2.8+ or using the old command and dune 2.7-.

[bobot]

Oh no, sorry, I should have been clearer. It was really that in the futur we generate with the --root ., (I forgot dune was generating that). It is a regression that must be fixed.

And thank you for pointing that dune subst doesn't have contrary to the other command a --root option.

[rgrinberg]

#4048 should fix this.