dune icon indicating copy to clipboard operation
dune copied to clipboard

dune 3.0 cannot find file through relative path during ppx preprocessing

Open bn-d opened this issue 3 years ago • 3 comments
trafficstars

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

  1. Clone https://github.com/johnwhitington/ppx_blob
  2. 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))
  1. Change dune version to 2.9 in dune-project and run dune build. It should finish without any error.
  2. 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+?

bn-d avatar Aug 25 '22 19:08 bn-d

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)

let-def avatar Sep 05 '22 08:09 let-def

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

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

rgrinberg avatar Sep 05 '22 20:09 rgrinberg

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

let-def avatar Sep 06 '22 00:09 let-def

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

rlepigre avatar Oct 14 '22 22:10 rlepigre

@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.

rlepigre avatar Nov 25 '22 13:11 rlepigre

@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.

bn-d avatar Nov 25 '22 16:11 bn-d

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?

rlepigre avatar Nov 25 '22 16:11 rlepigre

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.

bn-d avatar Nov 25 '22 17:11 bn-d

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.

rgrinberg avatar Nov 26 '22 01:11 rgrinberg

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.

rlepigre avatar Nov 29 '22 17:11 rlepigre

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

rgrinberg avatar Feb 23 '23 17:02 rgrinberg