Allow `enabled_if` in executable(s) stanza by voodoos · Pull Request #3137 · ocaml/dune

voodoos · 2020-02-14T15:26:46Z

This PR add support for enabled_if in executable(s) stanza.

Any attempt to build an executable whose enable_if field evaluates to false will raise a "Don't know how to build" error.

I am unsure whether or not that behaviour is the correct one. Other options are:

do nothing
warn the user that the enable_if field is not satisfied.

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

rgrinberg · 2020-02-14T16:46:24Z

Any attempt to build an executable whose enable_if field evaluates to false will raise a "Don't know how to build" error.

Isn't this the same for libraries with enabled_if that evaluates to false?

voodoos · 2020-02-14T16:51:58Z

Yes it is indeed the case.

Current behaviour is: nothing for aliases, "don't know" for rules and libraries, and a more precise error when depending on a disabled library.

From enabled_if/run.t (which is missing the case "explicitly build a disabled library"):

This alias is disabled, building it should do nothing:
  $ dune build @x

This one is enabled:
  $ dune build @y
  Building alias y

This rule is disabled, trying to build a should fail:
  $ dune build a
  Error: Don't know how to build a
  Hint: did you mean b?
  [1]

This one is enabled:
  $ dune build b
  Building file b

Test the enabled_if field for libraries:

  $ dune build main.exe
  File "dune", line 33, characters 12-15:
  33 |  (libraries foo))
                   ^^^
  Error: Library "foo" in _build/default is hidden (unsatisfied 'enabled_if').
  Hint: try: dune external-lib-deps --missing main.exe
  [1]

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

rgrinberg · 2020-02-14T17:15:36Z

i think this is sufficient for now. thanks.

@shonfeder

…lugin, dune-private-libs and dune-glob (2.3.0) CHANGES: - Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes ocaml/dune#3046, @shonfeder) - `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103, fixes ocaml/dune#3088, @shonfeder) - Avoid linear walk to detect children, this should greatly improve performance when a target has a large number of dependencies (ocaml/dune#2959, @ejgallego, @aalekseyev, @Armael) - [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of Coq's stdlib (ocaml/dune#3096, @ejgallego) - [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego) - Better error reporting for "data only" and "vendored" dirs. Using these with anything else than a strict subdirectory or `*` will raise an error. The previous behavior was to just do nothing (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos) - Fix bootstrap on bytecode only switches on windows or where `-j1` is set. (ocaml/dune#3112, @xclerc, @rgrinberg) - Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690 @voodoos) - Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them to be used to fail (ocaml/dune#3138, @rgrinberg) - Introduce a `strict_package_deps` mode that verifies that dependencies between packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117)

@shonfeder

…lugin, dune-private-libs and dune-glob (2.3.0) CHANGES: - Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes ocaml/dune#3046, @shonfeder) - `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103, fixes ocaml/dune#3088, @shonfeder) - Avoid linear walk to detect children, this should greatly improve performance when a target has a large number of dependencies (ocaml/dune#2959, @ejgallego, @aalekseyev, @Armael) - [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of Coq's stdlib (ocaml/dune#3096, @ejgallego) - [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego) - Better error reporting for "data only" and "vendored" dirs. Using these with anything else than a strict subdirectory or `*` will raise an error. The previous behavior was to just do nothing (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos) - Fix bootstrap on bytecode only switches on windows or where `-j1` is set. (ocaml/dune#3112, @xclerc, @rgrinberg) - Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690 @voodoos) - Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them to be used to fail (ocaml/dune#3138, @rgrinberg) - Introduce a `strict_package_deps` mode that verifies that dependencies between packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117) - Make sure the `@all` alias is defined when no `dune` file is present in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)

ghost · 2020-02-17T16:00:58Z

Did you check what happens if the executable is public? From a quick glance over the path, it seems to me that Dune would try to build the executable and fail.

voodoos · 2020-02-17T16:48:45Z

I can confirm that attempting to build @install a project including disabled public executables will trigger a don't know how to build error.

What do you think should be the correct behaviour ?

ghost · 2020-02-19T12:12:09Z

I think we should exclude executables with unsatified enabled_if clauses from installation. And while reading this code I found a very embarrassing bug: #3163

Bascially, you should interpret the enabled_if in the keep_if function that #3163 modifies.

@shonfeder

…lugin, dune-private-libs and dune-glob (2.3.0) CHANGES: - Improve validation and error handling of arguments to `dune init` (ocaml/dune#3103, fixes ocaml/dune#3046, @shonfeder) - `dune init exec NAME` now uses the `NAME` argument for private modules (ocaml/dune#3103, fixes ocaml/dune#3088, @shonfeder) - Avoid linear walk to detect children, this should greatly improve performance when a target has a large number of dependencies (ocaml/dune#2959, @ejgallego, @aalekseyev, @Armael) - [coq] Add `(boot)` option to `(coq.theories)` to enable bootstrap of Coq's stdlib (ocaml/dune#3096, @ejgallego) - [coq] Deprecate `public_name` field in favour of `package` (ocaml/dune#2087, @ejgallego) - Better error reporting for "data only" and "vendored" dirs. Using these with anything else than a strict subdirectory or `*` will raise an error. The previous behavior was to just do nothing (ocaml/dune#3056, fixes ocaml/dune#3019, @voodoos) - Fix bootstrap on bytecode only switches on windows or where `-j1` is set. (ocaml/dune#3112, @xclerc, @rgrinberg) - Allow `enabled_if` fields in `executable(s)` stanzas (ocaml/dune#3137, fixes ocaml/dune#1690 @voodoos) - Do not fail if `ocamldep`, `ocamlmklib`, or `ocaml` are absent. Wait for them to be used to fail (ocaml/dune#3138, @rgrinberg) - Introduce a `strict_package_deps` mode that verifies that dependencies between packages in the workspace are specified correctly. (@rgrinberg, ocaml/dune#3117) - Make sure the `@all` alias is defined when no `dune` file is present in a directory (ocaml/dune#2946, fix ocaml/dune#2927, @diml)

voodoos added 5 commits February 14, 2020 16:16

Add parsing of enable_if in executable(s)

5d5d523

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

Add guard to exe rule generation

4ffc9e5

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

Add test case for enabled_if in executables

5f69d82

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

Update documentation

1dc5277

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

Change CHANGES

354357d

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

voodoos requested review from emillon, nojb and rgrinberg as code owners February 14, 2020 15:27

Add test case for library enabled_if

76f56ea

Signed-off-by: Ulysse Gérard <thevoodoos@gmail.com>

rgrinberg merged commit c6aed04 into ocaml:master Feb 14, 2020

rgrinberg mentioned this pull request Feb 15, 2020

[new release] dune-build-info, dune, dune-configurator, dune-action-plugin, dune-private-libs and dune-glob (2.3.0) ocaml/opam-repository#15845

Merged

voodoos mentioned this pull request Feb 25, 2020

Prevent installation of disabled executables #3195

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `enabled_if` in executable(s) stanza#3137

Allow `enabled_if` in executable(s) stanza#3137
rgrinberg merged 6 commits intoocaml:masterfrom
voodoos:feature/enabled-if

voodoos commented Feb 14, 2020 •

edited

Loading

Uh oh!

rgrinberg commented Feb 14, 2020

Uh oh!

voodoos commented Feb 14, 2020 •

edited

Loading

Uh oh!

rgrinberg commented Feb 14, 2020

Uh oh!

ghost commented Feb 17, 2020

Uh oh!

voodoos commented Feb 17, 2020 •

edited

Loading

Uh oh!

ghost commented Feb 19, 2020 •

edited by ghost

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

voodoos commented Feb 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Feb 14, 2020

Uh oh!

voodoos commented Feb 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Feb 14, 2020

Uh oh!

ghost commented Feb 17, 2020

Uh oh!

voodoos commented Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Feb 19, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

voodoos commented Feb 14, 2020 •

edited

Loading

voodoos commented Feb 14, 2020 •

edited

Loading

voodoos commented Feb 17, 2020 •

edited

Loading

ghost commented Feb 19, 2020 •

edited by ghost

Loading