dune 3.0 cannot find file through relative path during ppx preprocessing

bn-d commented 2 years ago

Expected Behavior

ppx_blob should be able to accept file relative to the code path.

Actual Behavior

Get the following error with dune 3.0 and above.

$ dune build
File "test/test.ml", line 3, characters 52-71:
3 |     Alcotest.(check string) "file contents" "foo\n" [%blob "test_file"]
                                                        ^^^^^^^^^^^^^^^^^^^
Error: [%blob] could not find or load file test_file
File "example/quine.ml", line 1, characters 13-31:
1 | print_string [%blob "quine.ml"]
                 ^^^^^^^^^^^^^^^^^^
Error: [%blob] could not find or load file quine.ml

If change the path from test_file (relative to code) to test/test_file (relative to project root), it will compile with dune 3.0.

Reproduction

Clone https://github.com/johnwhitington/ppx_blob

Modify test/dune to below to be compatible with dune 2.0+

(test
(name test)
(preprocess (pps ppx_blob))
(preprocessor_deps (file test_file))
(libraries alcotest))

Change dune version to 2.9 in dune-project and run dune build. It should finish without any error.
Change dune version to 3.0 in dune-project and run dune build. It should fail with the above error.

Specifications

Version of dune (output of dune --version): 3.3.1
Version of ocaml (output of ocamlc --version): 4.13.2
Operating system (distribution and version): red hat Linux

Additional information

Below is the build command change for the reproduction above. But in some other cases, I see no change in the build command.

- $ (cd _build/default                                           && .ppx/c3461e88ae1503d575d582eb5874e916/ppx.exe -o example/quine.pp.ml --impl example/quine.ml -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)
+ $ (cd _build/.sandbox/31720b06aaaec80b23342b7409ab3115/default && .ppx/c3461e88ae1503d575d582eb5874e916/ppx.exe -o example/quine.pp.ml --impl example/quine.ml -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)

Is this behavior change expected for dune 3.0+?

let-def commented 2 years ago

I had the same problem using dune 3.4.1 in a project using (lang dune 2.7) with non-ppx preprocessors (using this specification: https://dune.readthedocs.io/en/stable/concepts.html?highlight=per_module#per-module-preprocessing-specification)

rgrinberg commented 2 years ago

It's the same issue as https://github.com/johnwhitington/ppx_blob/issues/23

@let-def are you extracting locations from the build artifacts?

let-def commented 2 years ago

No. My filter is a simple CPP-like text processor that removes some chunks of code (based on OCaml version).

rlepigre commented 2 years ago

I'm having the same issue with dune 3.4.1. This makes ppx_blob completely unusable.

rlepigre commented 2 years ago

@rgrinberg any idea what would be needed to fix this? I'd be happy to give it a shot, but some hints on what might be happening and on how to debug this would be greatly appreciated.

bn-d commented 2 years ago

@rgrinberg any idea what would be needed to fix this? I'd be happy to give it a shot, but some hints on what might be happening and on how to debug this would be greatly appreciated.

I believe you can find what went wrong in the ppx_blob issue mentioned previously. There is also a PR open to make ppx_blob to work dune 3.0.

rlepigre commented 2 years ago

OK, thanks. However, I must be missing something: it seemed to me that the failing example I gave in https://github.com/ocaml/dune/pull/6233 shows that there is a dune bug, but you seem to imply that the problem rather comes from ppx_blob?

bn-d commented 2 years ago

The problem is clearly caused by dune behavior changes. But

if it is an intended change, it is a breaking change and should be better documented.
else, it's a bug and should be fixed.

That's why I raised this issue again under dune for clarification.

In my experiences with ppx_blob, the workaround is changing the path relative to the workspace root. I.e. ./pp.sh to ./src/pp.sh in your example.

rgrinberg commented 2 years ago

The change was intended. Dune set an environment variable that makes the compiler produce artifacts that don't depend on the locations of your OCaml sources. This is needed to make effective use of the dune cache. As a consequence, you're no longer allowed to inspect paths from the resulting build artifacts. The workaround you've mentioned is the correct one.

rlepigre commented 2 years ago

Actually, I'm wondering: doesn't the fix completely break dune's compositionality? Isn't it often the case that the dune workspace is not the same as the root of a dune project?

I mean, I currently have a project that lives in a folder project, that does not have a dune-workspace file in any parent directory (including itself). So currently, I do something like [%blob "src/static/file.txt"] (relative path from project). Now, what if I create a dune-workspace file next to project? Will I need to change my blob into [%blob "project/src/static/file.txt"]?

Edit: so yeah, after thinking about it a bit more, this does completely break compositionality @rgrinberg, assuming I'm understanding what is happening correctly.

rgrinberg commented 1 year ago

Yes, you're right. Thinking about this again, perhaps the right thing to do is always expand [%blob ".."] relative to the file that's being preprocessed.

cc @gridbugs

ocaml / dune