Rewrite install root based paths

[richardlford]

To achieve reproducible builds, currently Dune rewrites paths based on the current build directory. However, it misses paths based on the OCaml installation directory (usually ~/.opam/switch).

This change also maps paths in the installation tree to abstract paths headed by "/workspace_root".

[richardlford]

I promoted more tests, squashed, and rebased.

[richardlford]

I'm trying to get a local reproduction of the failing windows tests. I think I found what should be the formula for setting up to test dune on Windows 10. In a checked-out dune repository, I'm doing these commands in powershell:

First to setup ocaml on my PC (actually a virtual machine running on my Linux box):

./ci/setup-dkml/pc/setup-dkml-windows_x86_64.ps1

The above appears to run successfully. When the above has run it recommends doing the following to continue, which I did:

$env:CHERE_INVOKING = "yes"
$env:MSYSTEM = "CLANG64"
$env:dkml_host_abi = "windows_x86_64"
$env:abi_pattern = "win32-windows_x86_64"
$env:opam_root = "C:\Users\richard\e\dune/.ci/o"
$env:exe_ext = ".exe"

It then also recommends doing the following which I did:

msys64\usr\bin\bash -lc 'sh ci/build-test.sh'

The above runs OK until it gets to the "Build boot dune.exe" step which fails like this:

======== Build boot dune.exe
Running: opam exec -- make _boot/dune.exe
ocamlc -output-complete-exe -w -24 -g -o .duneboot.exe -I boot unix.cma boot/libs.ml boot/duneboot.ml
.\.duneboot.exe
unhandled external library threads.posix
make: *** [Makefile:49: _boot/dune.exe] Error 2

Any suggestions?

[Alizter]

Once you have diskuv installed, what you need to do is run the bootstrap command with with-dkml infront of it. I did this recently and did something like:

with-dkml ocaml boot/bootstrap.ml

and then

.\dune.exe

should work as normal for building the dune repo.

[richardlford]

with-dkml ocaml boot/bootstrap.ml

Here is what I get:

C:\Users\richard\e\dune>with-dkml ocaml boot/bootstrap.ml
ocamlc -output-complete-exe -w -24 -g -o .duneboot.exe -I boot unix.cma boot/libs.ml boot/duneboot.ml
.\.duneboot.exe
unhandled external library threads.posix

I had first used the following link to install diskuv:

https://github.com/diskuv/dkml-installer-ocaml/releases/download/v1.2.0/setup-diskuv-ocaml-windows_x86_64-1.2.0.exe

Here is the with-dkml version:

C:\Users\richard\e\dune>with-dkml --version
env (GNU coreutils) 8.32

But I was having problems with that so decided to replicate the CI exactly by using

./ci/setup-dkml/pc/setup-dkml-windows_x86_64.ps1

although the CI itself uses

ci/setup-dkml/gh-windows/pre/acation.yml

but I was hoping they would be equivalent.

I'm wondering if there is something installed on the CI Windows images but not on my machine that is causing this error.

[Alizter]

You mentioned you were doing this in powershell. I wonder if just doing it in cmd will be different? When I setup windows a few weeks ago, I couldn't get it to work in powershell but it did work in cmd.

[richardlford]

You mentioned you were doing this in powershell. I wonder if just doing it in cmd will be different? When I setup windows a few weeks ago, I couldn't get it to work in powershell but it did work in cmd.

I was using powershell, but the failure I showed above was with cmd.

Do you using the link I mentioned to install diskuv, or do you try to replicate what the CI is doing?

[Alizter]

@richardlford I believe I installed diskuv using the installer you linked. Creating the correct environement however required the with-dkml wrapper. Perhaps it is the case that you need to use that for running dune.exe too?

Edit: Yes, trying it again myself, I needed to use with-dkml for both commands.

[Alizter]

In MSVC I am seeing:

+  >> Fatal error: Invalid value for the environment variable BUILD_PATH_PREFIX_MAP: invalid key/value pair "D", no '=' separator

That makes me think that your bppm code is separating on : in D:\... rather than on ; for windows.

[richardlford]

That makes me think that your bppm code is separating on : in D:\... rather than on ; for windows.

I'm pretty sure you are correct that a path with "D:" is the source of the problem. But the way things are supposed to work is that Build_path_prefix_map.map's are constructed abstractly, and then when they are encoded any paths in the map are encoded such that ":" and "=" get encoded in a way that they no longer have ":" or "=". That lets ":" and "=" be used as the delimiters, regardless of whether you are on Windows or not.

What I think is that there are places that are not doing it that way. In particular, I know that there are test scripts that are setting the BUILD_PATH_PREFIX_MAP directly in ways that would break on Windows. But this failing test is not one of those.

To make those other tests more robust we need a way in scripts to do the appropriate encoding. If the CRAM scripts are either running on a *nix system, or on MSYS, then it shouldn't be too hard to make a sed script that will do the appropriate encoding.

But, as I said, this test does not do that. In particular, for the windows-diff2.t test that I'm using to diagnose the problem, I've even turned off mapping but the problem is still persisting. Currently, I'm thinking that the Cram sanitizing might be the problem so I'm looking at that code.

[Alizter]

Yes, cram tests will understand and attempt to sanitize using bppm.

[Alizter]

Yes, cram tests will understand and attempt to sanitize using bppm.

[Alizter]

empty

[richardlford]

Not sure what you mean, but here is the latest code:

  $ ENCODED_SH=$(dune_cmd encode-prefix "`command -v sh`")
  $ export BUILD_PATH_PREFIX_MAP="SH=$ENCODED_SH"

I've gone through all the places that set BUILD_PATH_PREFIX_MAP manually, and made sure that all of the paths are properly encoded using the new "dune_cmd encode-prefix" command.

When I run command -v sh on either Linux or MSYS2's bash, I get /usr/bin/sh. But perhaps on some Windows environments this could give a Windows-like path that would need encoding.

[richardlford]

I've pushed to reflect your latest review comments. Here is the status of debugging the Windows failures.

1. As mentioned above, I've added a "dune_cmd encode-prefix" subcommand to do the encoding of paths that are to be manually inserted into BUILD_PATH_PREFIX_MAP, and modified all the places that need to use it.
2. In the windows-diff.t/run.t test, I've inserted an echo of the BUILD_PATH_PREFIX_MAP value at the start of the test. It shows that all the Windows paths are properly encoded (so no bare "D:").
3. That test passes on "windows-latest*", but not on "MSVC*", even though for both, the BUILD_PATH_PREFIX_MAP shows as being properly encoded. The "windows-latest*" phase only fails because of the extra echo, but the "MSVC*" phases are getting the invalid map message. What is the difference between the "windows-latest" and the "MSVC" environments? Do you have any idea why this would fail on one but not the other?**
4. One thing that makes this mapping somewhat confusing is that it can be nested. BUILD_PATH_PREFIX_MAP is already set on entry to a CRAM test, partly due to the CRAM code, and partly due to the code in action_exec.ml. Then, inside a CRAM test there are nested calls to dune which may add to the mapping.
5. If BUILD_PATH_PREFIX_MAP is unset at the beginning of windows-diff.t/run.t, the test passes. That makes it possible to get the tests to all pass, but if we don't understand why it fails, that is probably not an acceptable alternative.

[Alizter]

IINM the plain windows job is really using cygwin (therefore emulating a unix system). MSVC is native windows and so the paths will be windows based.

[christinerose]

one very minor grammar suggestion

[christinerose]

Suggested change

	appear in the paths (e.g. on Windows). For example:
	appear in the paths (e.g., on Windows). For example:

[richardlford]

I have made your suggested correction in the latest push.

[rgrinberg]

@gridbugs could you review this one?

[rgrinberg]

This change also maps paths in the installation tree to abstract paths headed by "/workspace_root".

Why did you choose to reuse /workspace_root? I would imagine choosing a different root name such as /install_root would be a possibility.

[richardlford]

Why did you choose to reuse /workspace_root? I would imagine choosing a different root name such as /install_root would be a possibility.

Sorry to take so long to respond (I've started a new job). Using a different root name is definitely a possibility. And in my more complete PR, #7540, files that would be installed (by dune install) were given abstract names of the form /install_root/<package-name>/<relative-path>. In that PR, only files that would not be installed were given /workspace_root/<relative-path> abstract paths.

In the separate PR, #7750, that introduces the dune ocaml debug command, it generates this inverse mapping:

• /workspace_root -> source root
• /workspace_root -> build root
• /workspace_root -> install root

because a source file in the workspace may be installed when the debugger looks at it later. If we used /install_root for files that are already known to be installed, as you suggest, then our inverse mapping would need to be:

• /workspace_root -> source root
• /workspace_root -> build root
• /workspace_root -> install root
• /install_root -> install root

So it would be slightly more complex. I would be happy to use /install_root, if you prefer. The current approach is slightly simpler.

[richardlford]

I have rebased, squashed, and force-pushed.