add: build_if in test stanza by emillon · Pull Request #7899 · ocaml/dune

emillon · 2023-06-06T11:40:40Z

The semantics of (enabled_if) in (test) can be confusing: (test) can be seen as the combination of (executable) and a (rule (alias runtest)); but (enabled_if) actually only controls the "running" part, not the "building" one.

This adds a new (build_if) field in (test). When it evaluates to false, the test stanza is bypassed (no build is attempted).

emillon · 2023-06-06T11:42:22Z

Remarks:

I'm not sure about the syntax and field name; I also contemplated extending the enabled_if field with something like (enabled_if (build (...)))
this does not allow having a different predicate for "build if" and "test if" but I don't think we need that nuance.

Alizter · 2023-06-06T11:42:37Z

Why don't we want this semantics for enabled_if to begin with?

emillon · 2023-06-06T11:43:33Z

TODO:

test interaction with (package)
confirm that this works on eio and ctypes
write doc

emillon · 2023-06-06T11:46:05Z

xrefs: #5529 #6134 #5505

rgrinberg · 2023-06-06T12:13:26Z

How about build_if? It's a little more in-line with our other names. Also, it gives you strictly more flexibility by allowing you to control both conditions.

emillon · 2023-06-06T12:28:20Z

How about build_if? It's a little more in-line with our other names. Also, it gives you strictly more flexibility by allowing you to control both conditions.

do you mean that build_if would be an extra blang field? I can try that. I think there should be a shorthand for when the condition is the same though. what do you think about (build_if X) and no (enabled_if) be a shortcut for (build_if X) (enabled_if X)?

rgrinberg · 2023-06-06T12:31:09Z

do you mean that build_if would be an extra blang field? I can try that

Exactly.

I think there should be a shorthand for when the condition is the same though. what do you think about (build_if X) and no (enabled_if) be a shortcut for (build_if X) (enabled_if X)?

Honestly, I wouldn't bother. We have the exact same issue in dozens of different places. We should just implement variables and solve it once and for all.

emillon · 2023-06-06T14:50:01Z

Actually, it's not even necessary: (enabled_if) defaults to true so replacing (enabled_if) by (build_if) does the expected thing.

Alizter · 2023-06-06T19:50:35Z

Will you update the docs?

emillon · 2023-06-07T07:33:37Z

Will you update the docs?

Yes, it's literally in the TODO list above :)

emillon · 2023-06-07T13:37:04Z

This works on the ctypes port; I also created an eio branch that uses that and seems to be working.

tmattio · 2023-06-07T16:02:38Z

This is super useful, thanks @emillon! I have the same question as @Alizter: why not change the semantics of enabled_if instead? It seems to me that if you don't want to run your test, it's not worth building it?

rgrinberg · 2023-06-07T21:46:39Z

There's some situations where a test is only executable on a particular platform but it's still useful to make sure it at least builds on every platform for all developers. We have some tests like that in dune.

emillon · 2023-06-08T08:58:39Z

Yes, exactly. For example, something that only runs if another package is available, or if you have an API key available. I would say it depends on what is in the enabled_if clause.

tmattio · 2023-06-08T10:14:26Z

That makes sense, thanks!

Fixes ocaml#6938 The semantics of `(enabled_if)` in `(test)` can be confusing: `(test)` can be seen as the combination of `(executable)` and a `(rule (alias runtest))`; but `(enabled_if)` actually only controls the "running" part, not the "building" one. This adds a new `(build_if)` field in `(test)`. When it evaluates to false, the test stanza is bypassed (no build is attempted). Signed-off-by: Etienne Millon <me@emillon.org>

@3Rafal

CHANGES: - Validate file extension for `$ dune ocaml top-module`. (ocaml/dune#8005, fixes ocaml/dune#8004, @3Rafal) - Include the time it takes to read/write state files when `--trace-file` is enabled (ocaml/dune#7960, @rgrinberg) - Add `dune show` command group which is an alias of `dune describe`. (ocaml/dune#7946, @Alizter) - Include source tree scans in the traces produced by `--trace-file` (ocaml/dune#7937, @rgrinberg) - Cinaps: The promotion rules for cinaps would only offer one file at a time no matter how many promotions were available. Now we offer all the promotions at once (ocaml/dune#7901, @rgrinberg) - Do not re-run OCaml syntax files on every iteration of the watch mode. This is too memory consuming. (ocaml/dune#7894, fix ocaml/dune#6900, @rgrinberg) - Remove some compatibility code for old version of dune that generated `.merlin` files. Now dune will never remove `.merlin` files automatically (ocaml/dune#7562) - Add `dune show env` command and make `dune printenv` an alias of it. (ocaml/dune#7985, @Alizter) - Add additional metadata to the traces provided by `--trace-file` whenever `--trace-extended` is passed (ocaml/dune#7778, @rleshchinskiy) - Extensions used in `(dialect)` can contain periods (e.g., `cppo.ml`). (ocaml/dune#7782, fixes ocaml/dune#7777, @nojb) - Allow `(include_subdirs qualified)` to be used when libraries define a `(modules ...)` field (ocaml/dune#7797, fixes ocaml/dune#7597, @anmonteiro) - `$ dune describe` is now a command group, so arguments to subcommands must be passed after subcommand itself. (ocaml/dune#7919, @Alizter) - The `interface` and `implementation` fields of a `(dialect)` are now optional (ocaml/dune#7757, @gpetiot) - Add commands `dune show targets` and `dune show aliases` that display all the available targets and aliases in a given directory respectively. (ocaml/dune#7770, grants ocaml/dune#265, @Alizter) - Allow multiple globs in library's `(stdlib (internal_modules ..))` (@anmonteiro, ocaml/dune#7878) - Attach melange rules to the default alias (ocaml/dune#7926, @haochenx) - In opam constraints, reject `(and)` and `(or)` with no arguments at parse time (ocaml/dune#7730, @emillon) - Compute digests and manage sandboxes in background threads (ocaml/dune#7947, @rgrinberg) - Add `(build_if)` to the `(test)` stanza. When it evaluates to false, the executable is not built. (ocaml/dune#7899, fixes ocaml/dune#6938, @emillon) - Add necessary parentheses in generated opam constraints (ocaml/dune#7682, fixes ocaml/dune#3431, @Lucccyo)

@3Rafal

CHANGES: - Validate file extension for `$ dune ocaml top-module`. (ocaml/dune#8005, fixes ocaml/dune#8004, @3Rafal) - Include the time it takes to read/write state files when `--trace-file` is enabled (ocaml/dune#7960, @rgrinberg) - Add `dune show` command group which is an alias of `dune describe`. (ocaml/dune#7946, @Alizter) - Include source tree scans in the traces produced by `--trace-file` (ocaml/dune#7937, @rgrinberg) - Cinaps: The promotion rules for cinaps would only offer one file at a time no matter how many promotions were available. Now we offer all the promotions at once (ocaml/dune#7901, @rgrinberg) - Do not re-run OCaml syntax files on every iteration of the watch mode. This is too memory consuming. (ocaml/dune#7894, fix ocaml/dune#6900, @rgrinberg) - Add `--all` option to `dune rpc status` to show all Dune RPC servers running. (ocaml/dune#8011, fix ocaml/dune#7902, @Alizter) - Remove some compatibility code for old version of dune that generated `.merlin` files. Now dune will never remove `.merlin` files automatically (ocaml/dune#7562) - Add `dune show env` command and make `dune printenv` an alias of it. (ocaml/dune#7985, @Alizter) - Add additional metadata to the traces provided by `--trace-file` whenever `--trace-extended` is passed (ocaml/dune#7778, @rleshchinskiy) - Extensions used in `(dialect)` can contain periods (e.g., `cppo.ml`). (ocaml/dune#7782, fixes ocaml/dune#7777, @nojb) - Allow `(include_subdirs qualified)` to be used when libraries define a `(modules ...)` field (ocaml/dune#7797, fixes ocaml/dune#7597, @anmonteiro) - `$ dune describe` is now a command group, so arguments to subcommands must be passed after subcommand itself. (ocaml/dune#7919, @Alizter) - The `interface` and `implementation` fields of a `(dialect)` are now optional (ocaml/dune#7757, @gpetiot) - Add commands `dune show targets` and `dune show aliases` that display all the available targets and aliases in a given directory respectively. (ocaml/dune#7770, grants ocaml/dune#265, @Alizter) - Allow multiple globs in library's `(stdlib (internal_modules ..))` (@anmonteiro, ocaml/dune#7878) - Attach melange rules to the default alias (ocaml/dune#7926, @haochenx) - In opam constraints, reject `(and)` and `(or)` with no arguments at parse time (ocaml/dune#7730, @emillon) - Compute digests and manage sandboxes in background threads (ocaml/dune#7947, @rgrinberg) - Add `(build_if)` to the `(test)` stanza. When it evaluates to false, the executable is not built. (ocaml/dune#7899, fixes ocaml/dune#6938, @emillon) - Add necessary parentheses in generated opam constraints (ocaml/dune#7682, fixes ocaml/dune#3431, @Lucccyo)

emillon requested a review from rgrinberg June 6, 2023 11:40

emillon force-pushed the tests-skip-build branch from b5b5e2a to e1f255c Compare June 6, 2023 14:57

rgrinberg approved these changes Jun 6, 2023

View reviewed changes

rgrinberg added this to the 3.9.0 milestone Jun 6, 2023

emillon force-pushed the tests-skip-build branch from e1f255c to d0d3ddb Compare June 7, 2023 08:19

emillon mentioned this pull request Jun 7, 2023

WIP: use build_if in test stanzas ocaml-multicore/eio#550

Closed

emillon force-pushed the tests-skip-build branch from d0d3ddb to bb0a0c1 Compare June 7, 2023 13:25

emillon changed the title ~~add: skip_build in test stanza~~ add: build_if in test stanza Jun 7, 2023

emillon mentioned this pull request Jun 7, 2023

[wip] Port to Dune yallop/ocaml-ctypes#588

Closed

18 tasks

avsm mentioned this pull request Jun 9, 2023

Mark eio_windows as only available on Windows ocaml-multicore/eio#542

Merged

emillon force-pushed the tests-skip-build branch 3 times, most recently from 09dc529 to e7eba3b Compare June 12, 2023 12:11

nguermond mentioned this pull request Jun 13, 2023

quote open command with Filename.quote_command and return Lwt process smolkaj/ocaml-open#2

Open

emillon force-pushed the tests-skip-build branch from e7eba3b to 8f0183d Compare June 19, 2023 13:35

emillon linked an issue Jun 19, 2023 that may be closed by this pull request

enabled_if not masking a library target #5505

Closed

emillon merged commit 0d482e9 into ocaml:main Jun 19, 2023

emillon deleted the tests-skip-build branch June 19, 2023 13:40

emillon mentioned this pull request Jun 23, 2023

[new release] dune (15 packages) (3.9.0~alpha1) ocaml/opam-repository#23989

Closed

emillon mentioned this pull request Jun 28, 2023

[new release] dune (15 packages) (3.9.0) ocaml/opam-repository#24013

Merged

emillon mentioned this pull request Jan 26, 2024

enabled_if doesn't work in (test) #6938

Closed

jchavarri mentioned this pull request Mar 8, 2024

eif: fix name collision in same folder for exes and melange emits #10220

Merged

Conversation

emillon commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emillon commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alizter commented Jun 6, 2023

Uh oh!

emillon commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emillon commented Jun 6, 2023

Uh oh!

rgrinberg commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emillon commented Jun 6, 2023

Uh oh!

rgrinberg commented Jun 6, 2023

Uh oh!

emillon commented Jun 6, 2023

Uh oh!

Alizter commented Jun 6, 2023

Uh oh!

emillon commented Jun 7, 2023

Uh oh!

emillon commented Jun 7, 2023

Uh oh!

tmattio commented Jun 7, 2023

Uh oh!

rgrinberg commented Jun 7, 2023

Uh oh!

emillon commented Jun 8, 2023

Uh oh!

tmattio commented Jun 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

emillon commented Jun 6, 2023 •

edited

Loading

emillon commented Jun 6, 2023 •

edited

Loading

emillon commented Jun 6, 2023 •

edited

Loading

rgrinberg commented Jun 6, 2023 •

edited

Loading