Allow `dune subst` to work inside a subfolder of a Git repository #11760

Richard-Degenne · 2025-05-05T13:54:38Z

I gave my best shot at fixing #11045, but tests fail with a runtime error that I have a hard time understanding. If anyone could give me some pointers or complete the fix, that would be awesome.

Internal error, please report upstream including the contents of _build/log.
Description:
  ("Fdecl.get: not set", {})
Raised at Stdune__Code_error.raise in file
  "otherlibs/stdune/src/code_error.ml", line 10, characters 30-62
Called from Dune_rules__Only_packages.Clflags.t in file
  "src/dune_rules/only_packages.ml" (inlined), line 25, characters 13-24
Called from Dune_rules__Only_packages.filter_packages_in_project in file
  "src/dune_rules/only_packages.ml", line 68, characters 8-20
Called from Fiber__Core.O.(>>|).(fun) in file "vendor/fiber/src/core.ml",
  line 253, characters 36-41
Called from Fiber__Scheduler.exec in file "vendor/fiber/src/scheduler.ml",
  line 76, characters 8-11
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/src/exn.ml", line 38, characters 27-56
Called from Fiber__Scheduler.exec in file "vendor/fiber/src/scheduler.ml",
  line 76, characters 8-11
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/src/exn.ml", line 38, characters 27-56
Called from Fiber__Scheduler.exec in file "vendor/fiber/src/scheduler.ml",
  line 76, characters 8-11
-> required by ("find-dir-raw", In_source_tree ".")

Alizter · 2025-05-05T14:22:27Z

I believe the issue is coming from (ln 309 in subst.ml):

  (* We have to do this because scanning the source tree evaluates [-p].
    That's because [-p] is needed to interpret packages in dune projects
    correctly. It should not be necessary, so we should probably make the
    package loading lazier. *)
  Dune_rules.Only_packages.Clflags.set No_restriction;

It looks to me like that this doesn't get set if there is a vcs present. We probably want that to be set in both cases by moving this before the match if we want Source_tree to work correctly. Not sure if this will fix all the issues however.

Edit: Yes, that appears to have worked. I would move it (together with the comment) to the term function where we set other stuff.

Alizter · 2025-05-05T14:29:12Z

Another comment, it would be great if you could include a commit before with a reproduction cram test. That we can be sure this commit will fix the issue and have a nice commit history.

You want to add the test in test/blackbox-tests/test-cases/subst/.

Let me know if you need any help.

Richard-Degenne · 2025-05-05T14:30:31Z

Awesome, thank you so much for the quick reply. I'll see what I can do for the test.

Richard-Degenne · 2025-05-05T18:10:46Z

This is my first time using cram, but the test seems to be working: it fails without the fix, but passes afterwards.

Let me know if there is anything I should have done differently.

Side question: is there a way to run a single test file? I've been running make test every time, it's kind of sluggish 😅

Alizter · 2025-05-05T21:21:16Z

You can run a single cram test by building the alias given by its name. Simply do ./dune.exe build @git-subfolder to build the test. Another tip is to use watch mode -w and --auto-promote which will automatically promote the current output of the test so that it succeeds. When you update the test, dune will make sure the expected output is updated.

Good first try, just a couple of comments:

Usually we want to test to always succeed, even if it outputs an error in the test itself.
We try to keep the reproduction case small so it is clearer which parts are causing issues. dune init does a bit more than is necessary so I've gone ahead and stripped out the important parts.

You should update the first commit to have this cram test:

Running `dune subst` in a subfolder of a Git repository should work.
Regression test for https://github.com/ocaml/dune/issues/11045

  $ git init --quiet

  $ mkdir subfolder
  $ cat > subfolder/dune-project <<EOF
  > (lang dune 3.19)
  > (name subfolder)
  > EOF

  $ cat > subfolder/subfolder.opam

`dune subst` requires at least one commit in the repository.

  $ git add .
  $ git commit --quiet --message "Initial commit"
  $ cd subfolder
  $ dune subst 2>&1 | grep "Internal error: Uncaught exception"
  | Internal error: Uncaught exception.

In the second commit, you will of course have the updated output.

Richard-Degenne · 2025-05-07T13:16:03Z

@rgrinberg: Thank you for completing the fix! Is there anything else that needs to be done before the PR can be considered for merging?

Signed-off-by: Richard Degenne <richard.degenne+github@gmail.com>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Fixes ocaml#11045 Signed-off-by: Richard Degenne <richard.degenne+github@gmail.com>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

@Alizter

CHANGES: ### Fixed - Fixed a bug that was causing cram tests attached to multiple aliases to be run multiple times. (ocaml/dune#11547, @Alizter) - Fix: pass pkg-config (extra) args in all pkgconfig invocations. A missing --personality flag would result in pkgconf not finding libraries in some contexts. (ocaml/dune#11619, @MisterDA) - Fix: Evaluate `enabled_if` when computing the stubs for stanzas such as `foreign_library` (ocaml/dune#11707, @Alizter, @rgrinberg) - Fix $ dune describe pp for libraries in the presence of `(include_subdirs unqualified)` (ocaml/dune#11729, fixes ocaml/dune#10999, @rgrinberg) - Fix `$ dune subst` in sub directories of a git repository (ocaml/dune#11760, fixes ocaml/dune#11045, @Richard-Degenne) - Fix a crash involving `Path.drop_prefix` when using Melange on Windows (ocaml/dune#11767, @nojb) ### Added - Added detection and warning for common typos in package dependency constraints (ocaml/dune#11600, fixes ocaml/dune#11575, @kemsguy7) - Added `(extra_objects)` field to `(foreign_library)` stanza with `(:include)` support. (ocaml/dune#11683, @Alizter) ### Changed - Allow build RPC messages to be handled by dune's RPC server in eager watch mode (ocaml/dune#11622, @gridbugs) - Allow concurrent build with RPC server (ocaml/dune#11712, @gridbugs)

@Alizter

CHANGES: ### Fixed - Fixed a bug that was causing cram tests attached to multiple aliases to be run multiple times. (ocaml/dune#11547, @Alizter) - Fix: pass pkg-config (extra) args in all pkgconfig invocations. A missing --personality flag would result in pkgconf not finding libraries in some contexts. (ocaml/dune#11619, @MisterDA) - Fix: Evaluate `enabled_if` when computing the stubs for stanzas such as `foreign_library` (ocaml/dune#11707, @Alizter, @rgrinberg) - Fix $ dune describe pp for libraries in the presence of `(include_subdirs unqualified)` (ocaml/dune#11729, fixes ocaml/dune#10999, @rgrinberg) - Fix `$ dune subst` in sub directories of a git repository (ocaml/dune#11760, fixes ocaml/dune#11045, @Richard-Degenne) - Fix a crash involving `Path.drop_prefix` when using Melange on Windows (ocaml/dune#11767, @nojb) ### Added - Added detection and warning for common typos in package dependency constraints (ocaml/dune#11600, fixes ocaml/dune#11575, @kemsguy7) - Added `(extra_objects)` field to `(foreign_library)` stanza with `(:include)` support. (ocaml/dune#11683, @Alizter) ### Changed - Allow build RPC messages to be handled by dune's RPC server in eager watch mode (ocaml/dune#11622, @gridbugs) - Allow concurrent build with RPC server (ocaml/dune#11712, @gridbugs)

Richard-Degenne force-pushed the fix-11045-git-subfolders branch from a99a9d7 to 729be42 Compare May 5, 2025 13:55

Richard-Degenne force-pushed the fix-11045-git-subfolders branch from e8bc5cb to 8866b05 Compare May 5, 2025 18:07

Richard-Degenne force-pushed the fix-11045-git-subfolders branch from 8866b05 to 0fc16dc Compare May 5, 2025 18:12

Richard-Degenne marked this pull request as ready for review May 5, 2025 19:42

rgrinberg force-pushed the fix-11045-git-subfolders branch 2 times, most recently from 1f7e88a to 544b2e7 Compare May 5, 2025 21:17

rgrinberg force-pushed the fix-11045-git-subfolders branch 4 times, most recently from af0d920 to 70782f9 Compare May 6, 2025 23:37

Richard-Degenne and others added 4 commits May 7, 2025 20:59

test: Added regression test for ocaml#11045

07fb215

Signed-off-by: Richard Degenne <richard.degenne+github@gmail.com>

fix: error messages when unable to substitute version

222c306

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

fix: $ dune subst i 8000 n sub-directories of a repo

56a6a79

Fixes ocaml#11045 Signed-off-by: Richard Degenne <richard.degenne+github@gmail.com>

fix: correctly list vcs files in subdirectory

16f442b

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg force-pushed the fix-11045-git-subfolders branch from 70782f9 to 16f442b Compare May 7, 2025 20:04

rgrinberg merged commit d556757 into ocaml:main May 7, 2025
10 of 13 checks passed

Richard-Degenne deleted the fix-11045-git-subfolders branch May 9, 2025 15:37

maiste mentioned this pull request May 20, 2025

[new release] dune (15 packages) (3.19.0~alpha0) ocaml/opam-repository#27905

Closed

maiste mentioned this pull request May 21, 2025

[new release] dune (15 packages) (3.19.0) ocaml/opam-repository#27910

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow `dune subst` to work inside a subfolder of a Git repository #11760

Allow `dune subst` to work inside a subfolder of a Git repository #11760

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Allow dune subst to work inside a subfolder of a Git repository #11760

Allow dune subst to work inside a subfolder of a Git repository #11760

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Allow `dune subst` to work inside a subfolder of a Git repository #11760

Allow `dune subst` to work inside a subfolder of a Git repository #11760