Added the direct dependencies between modules to the output of ``dune describe`` by esope · Pull Request #5412 · ocaml/dune

esope · 2022-02-04T09:34:23Z

When the new --with-deps flag is given to the command dune describe workspace, the output now contains a new field module_deps for each module entry. This new field contains the list of modules of the current library/executable a module immediately depends on.

For example, on a simple library project with two files a.ml and b.ml such that a.ml refers to the module B, with the following dune file:

(library
 (name a))

the command dune describe workspace --with-deps prints:

((library                         
  ((name a)
   (uid 3210b99379777ec5c43564aae2d79282)
   (local true)
   (requires ())
   (source_dir _build/default)
   (modules
    (((name B)
      (impl (_build/default/b.ml))
      (intf ())
      (cmt (_build/default/.a.objs/byte/a__B.cmt))
      (cmti ())
      (module_deps ()))
     ((name A)
      (impl (_build/default/a.ml))
      (intf ())
      (cmt (_build/default/.a.objs/byte/a.cmt))
      (cmti ())
      (module_deps (B)))
     ((name A__)
      (impl (_build/default/a__.ml-gen))
      (intf ())
      (cmt (_build/default/.a.objs/byte/a__.cmt))
      (cmti ())
      (module_deps ()))))
   (include_dirs (_build/default/.a.objs/byte)))))

esope · 2022-02-04T09:41:59Z

This PR results from this discussion thread, and hopefully provides a solution to issue#3425.
CC @nojb @rgrinberg

rgrinberg · 2022-02-06T01:26:00Z

bin/describe.ml

+      union
+        ~f:(fun _ () () -> Some ())
+        (of_list_unit deps_for_impl)
+        (of_list_unit deps_for_intf)


Do you think it's perhaps better to print these sets separately and let callers merge them if needed?

Good idea. Implemented in the recently pushed changes.

rgrinberg · 2022-02-06T01:28:42Z

Could you add tests for this feature?

esope · 2022-02-07T10:22:33Z

@rgrinberg: with your recent commit b5ad3a39b8debb430654665b60433a3acf5b4410 refactor: implement progress with svar I encounter the following error

File "src/dune_engine/build_system.ml", line 214, characters 11-17:
File "src/dune_engine/build_system.ml", line 214, characters 11-17: Assertion
failed
Raised at Dune_engine__Build_system.State.start_rule_exn in file
  "src/dune_engine/build_system.ml", line 214, characters 11-23
Called from Dune_engine__Build_system.Exported.execute_rule_impl in file
  "src/dune_engine/build_system.ml", line 557, characters 51-76
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13
Re-raised at Stdune__Exn.raise_with_backtrace in file
  "otherlibs/stdune/exn.ml", line 36, characters 27-56
Called from Fiber.Scheduler.exec in file "src/fiber/fiber.ml", line 878,
  characters 10-13

I must not crash.  Uncertainty is the mind-killer. Exceptions are the
little-death that brings total obliteration.  I will fully express my cases. 
Execution will pass over me and through me.  And when it has gone past, I
will unwind the stack along its path.  Where the cases are handled there will
be nothing.  Only I will remain.

On line 214 of file src/dune_engine/build_system.ml, the variable current seems to be in the Initializing case.

My code used to work properly without these svar-related changes.

esope · 2022-02-07T10:37:50Z

Could you add tests for this feature?

This is pushed in the recent commits.

But it triggered another (minor) issue: I expanded the current test for dune describe so that it contains an example that uses a PPX rewriter. As a consequence, new dependencies to external libraries appear, and they refer to absolute path names (example here)... This will create spurious mistakes on every developers' machines, since these paths are likely to refer to their own home directories, and can even differ when tests are run with two different versions of the OCaml compiler...

Do you see an easy solution to this issue?

Signed-off-by: Rudi Grinberg <me@rgrinberg.com> ps-id: 52DED725-A17B-4910-8B63-4615B1B4C9A1

rgrinberg · 2022-02-07T16:27:11Z

@esope thanks for the report. I'll fix this issue shortly.

rgrinberg · 2022-02-08T05:14:28Z

Do you see an easy solution to this issue?

Honestly, there's no easy solution. The obvious thing to do is to add the paths to BUILD_PATH_PREFIX_MAP, but unfortunately the formatting is going to depend on the unfiltered length which still makes it not completely reproducible.

How about just writing a simple pre-processor that will redact the fields that aren't reproducible?

esope · 2022-02-08T08:26:08Z

How about just writing a simple pre-processor that will redact the fields that aren't reproducible?

I'll try this solution. I've also noticed that since the UIDs depend on the paths, they are subject to instability too...

esope · 2022-02-10T15:03:36Z

How about just writing a simple pre-processor that will redact the fields that aren't reproducible?

I'll try this solution. I've also noticed that since the UIDs depend on the paths, they are subject to instability too...

I'm trying to write a simple executable that reads the output of dune describe on stdin, parses it, sanitizes it to normalize the non-reproducible parts, and prints its result on stdout. This executable will be part of the test/blackbox-tests/test-cases/describe.t directory.

To do this, I need to parse the contents of stdin to produce something akin to a S-expression, for example using the functions of dune's dune_lang library. It seems I cannot access this library at all from the test/blackbox-tests/test-cases/describe.t directory. @rgrinberg: do you have any idea on how to to this?

An alternative path would be to add a flag to dune describe workspace, that would enable the sanitization before printing, but running dune's internal tests would be this flag's sole purpose...

rgrinberg · 2022-02-10T16:12:23Z

To do this, I need to parse the contents of stdin to produce something akin to a S-expression, for example using the functions of dune's dune_lang library. It seems I cannot access this library at all from the test/blackbox-tests/test-cases/describe.t directory. @rgrinberg: do you have any idea on how to to this?

The easiest way is to introduce a subcommand to dune_cmd. Have a look at how that executable is defined.

An alternative path would be to add a flag to dune describe workspace, that would enable the sanitization before printing, but running dune's internal tests would be this flag's sole purpose...

If you think this is simpler, it is fine by me.

esope · 2022-02-11T19:13:35Z

An alternative path would be to add a flag to dune describe workspace, that would enable the sanitization before printing, but running dune's internal tests would be this flag's sole purpose...

If you think this is simpler, it is fine by me.

Done in commit 72cdde1.

rgrinberg · 2022-02-11T21:18:07Z

CI isn't happy:

Run opam exec -- make dune.exe
[11](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:11)
ocamlc -output-complete-exe -w -24 -g -o .duneboot.exe -I boot unix.cma boot/libs.ml boot/duneboot.ml
[12](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:12)
./.duneboot.exe
[13](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:13)
cd _boot && /home/runner/.opam/4.12.1/bin/ocamldep.opt -modules -map main__.ml -map dune_rpc_impl.ml -map build_info__.ml -map dune_rules__.ml -map dune_engine__.ml -map async_inotify_for_dune__.ml -map ocaml_inotify.ml -map dune_rpc_private__.ml -map dune_meta_parser.ml -map dune_action_plugin__.ml -map dune_glob__.ml -map dune_re__.ml -map dune_cache.ml -map dune_cache_storage__.ml -map dune_util__.ml -map memo__.ml -map dag__.ml -map incremental_cycles__.ml -map dune_lang__.ml -map dune_graph__.ml -map stdune__.ml -args source_files
[14](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:14)

[15](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:15)
File "bin/describe.ml", line 838, characters 28-30:
[16](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:16)
Error: Syntax error: 'end' expected
[17](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:17)
File "bin/describe.ml", line 8[18](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:18), characters 17-23:
18
  This 'struct' might be unmatched
[19](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:19)
make: *** [Makefile:48: dune.exe] Error 2
[20](https://github.com/ocaml/dune/runs/5161075121?check_suite_focus=true#step:8:20)

esope · 2022-02-12T16:25:12Z

CI is much happier now.
I had to reimplement String.ends_with, that is only available in recent version of the standard library. @rgrinberg: I wrote the implementation in describe.ml, but do you think of a better module where this implementation should be?

…mmediate dependencies of a module Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

… of preprocessors that are registered for a library Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

…ules Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

…cribe``. TODO: add a flag to the CLI, and introduce a new dune lang version Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

…dependencies in the output of ``dune describe workspace`` Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

… provided Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

…nterface, and the dependencies of its implementation. We used to return the union of the two. Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

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

This flag does its best to produce stable outputs across different machines, when the output involves absolute paths. The tests for the ``dune describe workspace`` are updated so that they use this flag. Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

…d produce a file that older OCaml versions could not parse Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

…ns of the standard library Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

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

rgrinberg · 2022-02-15T03:57:05Z

Merged but I simplified the sanitation quite a bit. It's not as robust, but good enough for our testing needs.

@rgrinberg

…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info and dune-action-plugin (3.1.0) CHANGES: - Add `sourcehut` as an option for defining project sources in dune-project files. For example, `(source (sourcehut user/repo))`. (ocaml/dune#5564, @rgrinberg) - Add `dune coq top` command for running a Coq toplevel (ocaml/dune#5457, @rlepigre) - Fix dune exec dumping database in wrong directory (ocaml/dune#5544, @bobot) - Always output absolute paths for locations in RPC reported diagnostics (ocaml/dune#5539, @rgrinberg) - Add `(deps <deps>)` in ctype field (ocaml/dune#5346, @bobot) - Add `(include <file>)` constructor to dependency specifications. This can be used to introduce dynamic dependencies (ocaml/dune#5442, @anmonteiro) - Ensure that `dune describe` computes a transitively closed set of libraries (ocaml/dune#5395, @esope) - Add direct dependencies to $ dune describe output (ocaml/dune#5412, @esope) - Show auto-detected concurrency on Windows too (ocaml/dune#5502, @MisterDA) - Fix operations that remove folders with absolute path. This happens when using esy (ocaml/dune#5507, @EduardoRFS) - Dune will not fail if some directories are non-empty when uninstalling. (ocaml/dune#5543, fixes ocaml/dune#5542, @nojb) - `coqdep` now depends only on the filesystem layout of the .v files, and not on their contents (ocaml/dune#5547, helps with ocaml/dune#5100, @ejgallego) - The mdx stanza 0.2 can now be used with `(implicit_transitive_deps false)` (ocaml/dune#5558, fixes ocaml/dune#5499, @emillon) - Fix missing parenthesis in printing of corresponding terminal command for `(with-outputs-to )` (ocaml/dune#5551, fixes ocaml/dune#5546, @Alizter)

esope mentioned this pull request Feb 5, 2022

Generating a file describing the dependencies between ML files ? #3425

Closed

rgrinberg reviewed Feb 6, 2022

View reviewed changes

esope referenced this pull request Feb 7, 2022

refactor: implement progress with svar

b5ad3a3

Signed-off-by: Rudi Grinberg <me@rgrinberg.com> ps-id: 52DED725-A17B-4910-8B63-4615B1B4C9A1

rgrinberg force-pushed the describe_module_deps branch from ef31f56 to 83480fd Compare February 8, 2022 05:06

esope mentioned this pull request Feb 10, 2022

Ensure that dune describe computes a transitively closed set of libraries #5395

Closed

esope and others added 13 commits February 14, 2022 19:20

Added a function to src/dune_rules/ocamldep.{ml,mli} to compute the i…

4275b45

…mmediate dependencies of a module Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Added a function do src/dune_rules/lib.{ml,mli} that returns the list…

87847bd

… of preprocessors that are registered for a library Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Make the Ocamldep and Preprocess modules visible to clients of Dune_r…

a65de77

…ules Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Added direct dependencies between modules in the output of ``dune des…

4bec00d

…cribe``. TODO: add a flag to the CLI, and introduce a new dune lang version Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Added a --with-deps flag to control whether to compute and print …

d20600c

…dependencies in the output of ``dune describe workspace`` Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Print the module_deps field only when the --with-deps flag is…

492caf0

… provided Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Return two sets of dependencies per module: the dependencies of its i…

0b297a0

…nterface, and the dependencies of its implementation. We used to return the union of the two. Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

Added tests for dune describe workspace --wth-deps.

bc3b378

Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

fix: do not run rules outside of Build_system.run

1df686d

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

minor code modification to circumvent ocamlformat's changes that woul…

284bcf8

…d produce a file that older OCaml versions could not parse Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

reimplemented String.ends_with, that is not available in older versio…

f30ec04

…ns of the standard library Signed-off-by: Benoît Montagu <benoit.montagu@inria.fr>

refactor: remove string_ends_with

4498db4

fix: reproduce in a simpler way

5bea609

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

rgrinberg force-pushed the describe_module_deps branch from 192153e to 5bea609 Compare February 15, 2022 02:23

rgrinberg closed this Feb 15, 2022

esope deleted the describe_module_deps branch February 15, 2022 08:56

rgrinberg mentioned this pull request Apr 15, 2022

[new release] xdg, stdune, ordering, ocamlc-loc, fiber, dyn, dune, dune-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info and dune-action-plugin (3.1.0) ocaml/opam-repository#21217

Merged

Conversation

esope commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esope commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg Feb 6, 2022

Choose a reason for hiding this comment

Uh oh!

esope Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgrinberg commented Feb 6, 2022

Uh oh!

esope commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esope commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Feb 7, 2022

Uh oh!

rgrinberg commented Feb 8, 2022

Uh oh!

esope commented Feb 8, 2022

Uh oh!

esope commented Feb 10, 2022

Uh oh!

rgrinberg commented Feb 10, 2022

Uh oh!

esope commented Feb 11, 2022

Uh oh!

rgrinberg commented Feb 11, 2022

Uh oh!

esope commented Feb 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Feb 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

esope commented Feb 4, 2022 •

edited

Loading

esope commented Feb 4, 2022 •

edited

Loading

esope Feb 7, 2022 •

edited

Loading

esope commented Feb 7, 2022 •

edited

Loading

esope commented Feb 7, 2022 •

edited

Loading

esope commented Feb 12, 2022 •

edited

Loading