feat(dune): support `+` prefixes for PPX CLI flags by anmonteiro · Pull Request #11234 · ocaml/dune

anmonteiro · 2024-12-20T09:26:18Z

this is useful for cases like melange.ppx which emit alerts and allow to configure them via -alert ....

rgrinberg · 2024-12-20T13:27:20Z

I think this needs to be versioned. You can version this under dune's own language or melange's. Depending on how stable you'd like to keep this feature.

anmonteiro · 2024-12-20T23:12:49Z

Why do you think this needs to be versioned?

I believe the previous behavior would just plainly return an error, so we shouldn't have anyone depending on these features, right?

rgrinberg · 2024-12-21T00:59:48Z

Suppose that somebody would try to use this feature in a publicly released project. That person would need to remember to update the constraints in their project file to require at least dune 3.18. Without the version check, it is very likely that the person would forget to add this constraint, or they might even do it incorrectly, if they misremember which version of dune introduced this feature. Downstream user of this project might then encounter build failures should they build with an older version of dune. If we add a version check, we make this scenario more or less impossible.

It's true that the above argument is applicable to basically any user visible change in dune, but I think it works best for features. As for bug fixes, it's good that users can benefit from those without opting in. I think that this PR is certainly more of a feature than a bug fix.

anmonteiro · 2024-12-21T04:59:10Z

absolutely, thanks for the explainer!

anmonteiro · 2024-12-21T05:54:26Z

@rgrinberg ready for another look.

rgrinberg · 2024-12-22T16:46:49Z

src/dune_rules/preprocess.ml

+        with
+        | Yes, _ -> Right s
+        | _, Yes when syntax_version >= (3, 18) -> Right s
+        | (No | Unknown _), _ ->


I think you could improve the error message here with a hint for prefix = "+" when syntax_version < (3, 18)

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

* test(ppx): show flags starting with `+` are interpreted as library names Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * feat(dune): support `+` prefixes for PPX CLI flags Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * cr: only enabled for Dune versions starting with 3.18 Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * cr: add a better error message for `prefix = "+"` Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> --------- Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

* test(ppx): show flags starting with `+` are interpreted as library names Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * feat(dune): support `+` prefixes for PPX CLI flags Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * cr: only enabled for Dune versions starting with 3.18 Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * cr: add a better error message for `prefix = "+"` Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> --------- Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> Signed-off-by: Chris Armstrong <chris-armstrong@users.noreply.github.com>

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> Signed-off-by: Chris Armstrong <chris-armstrong@users.noreply.github.com>

@Sylvain78

CHANGES: ### Fixed - Support HaikuOS: don't call `execve` since it's not allowed if other pthreads have been created. The fact that Haiku can't call `execve` from other threads than the principal thread of a process (a team in haiku jargon), is a discrepancy to POSIX and hence there is a [bug about it](https://dev.haiku-os.org/ticket/18665). (@Sylvain78, ocaml/dune#10953) - Fix flag ordering in generated Merlin configurations (ocaml/dune#11503, @voodoos, fixes ocaml/merlin#1900, reported by @vouillon) ### Added - Add `(format-dune-file <src> <dst>)` action. It provides a replacement to `dune format-dune-file` command. (ocaml/dune#11166, @nojb) - Allow the `--prefix` flag when configuring dune with `ocaml configure.ml`. This allows to set the prefix just like `$ dune install --prefix`. (ocaml/dune#11172, @rgrinberg) - Allow arguments starting with `+` in preprocessing definitions (starting with `(lang dune 3.18)`). (@amonteiro, ocaml/dune#11234) - Support for opam `(maintenance_intent ...)` in dune-project (ocaml/dune#11274, @art-w) - Validate opam `maintenance_intent` (ocaml/dune#11308, @art-w) - Support `not` in package dependencies constraints (ocaml/dune#11404, @art-w, reported by @hannesm) ### Changed - Warn when failing to discover root due to reads failing. The previous behavior was to abort. (@KoviRobi, ocaml/dune#11173) - Use shorter path for inline-tests artifacts. (@hhugo, ocaml/dune#11307) - Allow dash in `dune init` project name (ocaml/dune#11402, @art-w, reported by @saroupille) - On Windows, under heavy load, file delete operations can sometimes fail due to AV programs, etc. Guard against it by retrying the operation up to 30x with a 1s waiting gap (ocaml/dune#11437, fixes ocaml/dune#11425, @MSoegtropIMC) - Cache: we now only store the executable permission bit for files (ocaml/dune#11541, fixes ocaml/dune#11533, @ElectreAAS) - Display negative error codes on Windows in hex which is the more customary way to display `NTSTATUS` codes (ocaml/dune#11504, @MisterDA)

@Sylvain78

CHANGES: ### Fixed - Support HaikuOS: don't call `execve` since it's not allowed if other pthreads have been created. The fact that Haiku can't call `execve` from other threads than the principal thread of a process (a team in haiku jargon), is a discrepancy to POSIX and hence there is a [bug about it](https://dev.haiku-os.org/ticket/18665). (@Sylvain78, ocaml/dune#10953) - Fix flag ordering in generated Merlin configurations (ocaml/dune#11503, @voodoos, fixes ocaml/merlin#1900, reported by @vouillon) ### Added - Add `(format-dune-file <src> <dst>)` action. It provides a replacement to `dune format-dune-file` command. (ocaml/dune#11166, @nojb) - Allow the `--prefix` flag when configuring dune with `ocaml configure.ml`. This allows to set the prefix just like `$ dune install --prefix`. (ocaml/dune#11172, @rgrinberg) - Allow arguments starting with `+` in preprocessing definitions (starting with `(lang dune 3.18)`). (@amonteiro, ocaml/dune#11234) - Support for opam `(maintenance_intent ...)` in dune-project (ocaml/dune#11274, @art-w) - Validate opam `maintenance_intent` (ocaml/dune#11308, @art-w) - Support `not` in package dependencies constraints (ocaml/dune#11404, @art-w, reported by @hannesm) ### Changed - Warn when failing to discover root due to reads failing. The previous behavior was to abort. (@KoviRobi, ocaml/dune#11173) - Use shorter path for inline-tests artifacts. (@hhugo, ocaml/dune#11307) - Allow dash in `dune init` project name (ocaml/dune#11402, @art-w, reported by @saroupille) - On Windows, under heavy load, file delete operations can sometimes fail due to AV programs, etc. Guard against it by retrying the operation up to 30x with a 1s waiting gap (ocaml/dune#11437, fixes ocaml/dune#11425, @MSoegtropIMC) - Cache: we now only store the executable permission bit for files (ocaml/dune#11541, fixes ocaml/dune#11533, @ElectreAAS) - Display negative error codes on Windows in hex which is the more customary way to display `NTSTATUS` codes (ocaml/dune#11504, @MisterDA)

* test(ppx): show flags starting with `+` are interpreted as library names Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * feat(dune): support `+` prefixes for PPX CLI flags Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * cr: only enabled for Dune versions starting with 3.18 Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> * cr: add a better error message for `prefix = "+"` Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com> --------- Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

anmonteiro changed the title ~~feat(dune): support + prefixes for PPX CLI flags~~ feat(dune): support + prefixes for PPX CLI flags Dec 20, 2024

anmonteiro requested review from jchavarri and rgrinberg December 20, 2024 09:27

anmonteiro force-pushed the anmonteiro/melange-test-use-another-flag branch 2 times, most recently from 8cdabdc to 493b6f9 Compare December 21, 2024 05:54

rgrinberg reviewed Dec 22, 2024

View reviewed changes

anmonteiro added 4 commits December 22, 2024 18:34

test(ppx): show flags starting with + are interpreted as library names

1c788ab

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

feat(dune): support + prefixes for PPX CLI flags

e940f22

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

cr: only enabled for Dune versions starting with 3.18

1936037

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

cr: add a better error message for prefix = "+"

e57cc15

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

anmonteiro force-pushed the anmonteiro/melange-test-use-another-flag branch from 493b6f9 to e57cc15 Compare December 23, 2024 02:39

maiste added melange Melange rules and generator enhancement and removed melange Melange rules and generator labels Dec 23, 2024

rgrinberg approved these changes Dec 23, 2024

View reviewed changes

anmonteiro enabled auto-merge (squash) December 25, 2024 09:10

Merge branch 'main' into anmonteiro/melange-test-use-another-flag

f0dd399

anmonteiro merged commit 30af715 into ocaml:main Dec 25, 2024

anmonteiro deleted the anmonteiro/melange-test-use-another-flag branch December 25, 2024 09:10

anmonteiro added a commit to anmonteiro/dune that referenced this pull request Dec 25, 2024

chore: add changelog for ocaml#11234

bdd06bd

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

anmonteiro added a commit that referenced this pull request Dec 25, 2024

chore: add changelog for #11234 (#11242)

c973f16

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

ElectreAAS pushed a commit to ElectreAAS/dune that referenced this pull request Jan 27, 2025

chore: add changelog for ocaml#11234 (ocaml#11242)

6d536fe

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

maiste mentioned this pull request Mar 31, 2025

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

Closed

maiste mentioned this pull request Apr 3, 2025

[new release] dune (15 packages) (3.18.0) ocaml/opam-repository#27704

Merged

Sudha247 pushed a commit to Sudha247/dune that referenced this pull request Jul 23, 2025

chore: add changelog for ocaml#11234 (ocaml#11242)

3d849de

Signed-off-by: Antonio Nuno Monteiro <anmonteiro@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dune): support `+` prefixes for PPX CLI flags #11234

feat(dune): support `+` prefixes for PPX CLI flags #11234
anmonteiro merged 5 commits intoocaml:mainfrom
anmonteiro:anmonteiro/melange-test-use-another-flag

anmonteiro commented Dec 20, 2024 •

edited

Loading

Uh oh!

rgrinberg commented Dec 20, 2024

Uh oh!

anmonteiro commented Dec 20, 2024

Uh oh!

rgrinberg commented Dec 21, 2024 •

edited

Loading

Uh oh!

anmonteiro commented Dec 21, 2024

Uh oh!

anmonteiro commented Dec 21, 2024

Uh oh!

rgrinberg Dec 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

anmonteiro commented Dec 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgrinberg commented Dec 20, 2024

Uh oh!

anmonteiro commented Dec 20, 2024

Uh oh!

rgrinberg commented Dec 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anmonteiro commented Dec 21, 2024

Uh oh!

anmonteiro commented Dec 21, 2024

Uh oh!

rgrinberg Dec 22, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anmonteiro commented Dec 20, 2024 •

edited

Loading

rgrinberg commented Dec 21, 2024 •

edited

Loading