Allow to declare (instrumentation) dependencies by nojb · Pull Request #4210 · ocaml/dune

nojb · 2021-02-10T09:10:59Z

@aantron if you could give this one a spin that would be great. The syntax is as discussed in #3983.

aantron · 2021-02-10T10:42:19Z

Looks good and seems to work.

ghost · 2021-02-11T12:18:40Z

src/dune_rules/lib_rules.ml

  let pp =
    Preprocessing.make sctx ~dir ~dep_kind ~scope ~preprocess ~expander
-      ~preprocessor_deps:lib.buildable.preprocessor_deps
+      ~preprocessor_deps:(lib.buildable.preprocessor_deps @ instrumentation_deps)


It might be worth sharing this code betweem exe_rules.ml and lib_rules.ml.

Good idea. I added an extra argument to Preprocessing.make ~instrumentation_deps and do the concatentation inside that function.

ghost

LGTM.

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb · 2021-02-17T19:43:24Z

Will merge this once CI is green.

@rgrinberg this should go into 2.8.3... to this end, should I change the version guard in the code from (3, 0) to (2, 8) ?

rgrinberg · 2021-02-17T20:20:12Z

This is a new feature, so it cannot go into 2.8.3

nojb · 2021-02-17T20:22:43Z

This is a new feature, so it cannot go into 2.8.3

Right, sorry, I got confused for a moment :)

rgrinberg · 2021-02-17T22:48:11Z

If you'd like we could include it in 2.9 however. The PR will need to be modified slightly though.

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

ejgallego · 2021-06-02T18:40:17Z

Hi folks, deadline for submitting the backport of this PR is June 11th, please do so or remove the 2.9 milestone from this PR if you don't want this in 2.9 anymore.

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb · 2021-06-02T18:48:41Z

PR backport at #4686

ejgallego · 2021-06-02T19:00:23Z

PR backport at #4686

Thanks a lot @nojb !

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com> Co-authored-by: Emilio Jesús Gallego Arias <e+git@x80.org>

@emillon

…ator, dune-private-libs, dune and dune-build-info (2.9-rc1) CHANGES: - Add `(enabled_if ...)` to `(mdx ...)` (ocaml/dune#4434, @emillon) - Add support for instrumentation dependencies (ocaml/dune#4210, fixes ocaml/dune#3983, @nojb) - Add the possibility to use `locks` with the cram tests stanza (ocaml/dune#4480, @voodoos) - Allow to set up merlin in a variant of the default context (ocaml/dune#4145, @TheLortex, @voodoos) - Add `(package ...)` to `(mdx ...)` (ocaml/dune#4691, fixes ocaml/dune#3756, @emillon) - Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01) - Fix generation of merlin configuration when using `(include_subdirs unqualified)` on Windows (ocaml/dune#4745, @nojb) - Fix bug for the install of Coq native files when using `(include_subdirs qualified)` (ocaml/dune#4753, @ejgallego) - Allow users to specify install target directories for `doc` and `etc` sections. We add new options `--docdir` and `--etcdir` to both Dune's configure and `dune install` command. (ocaml/dune#4744, fixes ocaml/dune#4723, @ejgallego, thanks to @JasonGross for reporting this issue) - Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01) - Fix issue where Dune would ignore `(env ... (coq (flags ...)))` declarations appearing in `dune` files (ocaml/dune#4749, fixes ocaml/dune#4566, @ejgallego @rgrinberg) - Disable some warnings on Coq 8.14 and `(lang coq (>= 0.3))` due to the rework of the Coq "native" compilation system (ocaml/dune#4760, @ejgallego)

@emillon

…ator, dune-private-libs, dune and dune-build-info (2.9.0) CHANGES: - Add `(enabled_if ...)` to `(mdx ...)` (ocaml/dune#4434, @emillon) - Add support for instrumentation dependencies (ocaml/dune#4210, fixes ocaml/dune#3983, @nojb) - Add the possibility to use `locks` with the cram tests stanza (ocaml/dune#4480, @voodoos) - Allow to set up merlin in a variant of the default context (ocaml/dune#4145, @TheLortex, @voodoos) - Add `(package ...)` to `(mdx ...)` (ocaml/dune#4691, fixes ocaml/dune#3756, @emillon) - Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01) - Fix generation of merlin configuration when using `(include_subdirs unqualified)` on Windows (ocaml/dune#4745, @nojb) - Fix bug for the install of Coq native files when using `(include_subdirs qualified)` (ocaml/dune#4753, @ejgallego) - Allow users to specify install target directories for `doc` and `etc` sections. We add new options `--docdir` and `--etcdir` to both Dune's configure and `dune install` command. (ocaml/dune#4744, fixes ocaml/dune#4723, @ejgallego, thanks to @JasonGross for reporting this issue) - Fix issue where Dune would ignore `(env ... (coq (flags ...)))` declarations appearing in `dune` files (ocaml/dune#4749, fixes ocaml/dune#4566, @ejgallego @rgrinberg) - Disable some warnings on Coq 8.14 and `(lang coq (>= 0.3))` due to the rework of the Coq "native" compilation system (ocaml/dune#4760, @ejgallego) - Fix a bug where instrumentation flags would be added even if the instrumentatation was disabled (@nojb, ocaml/dune#4770) - Fix ocaml/dune#4682: option `-p` takes now precedence on environement variable `DUNE_PROFILE` (ocaml/dune#4730, ocaml/dune#4774, @bobot, reported by @dra27 ocaml/dune#4632) - Fix installation with opam of package with dune sites. The `.install` file is now produced by a local `dune install` during the build phase (ocaml/dune#4730, ocaml/dune#4645, @bobot, reported by @kit-ty-kate ocaml/dune#4198) - Fix multiple issues in the sites feature (ocaml/dune#4730, ocaml/dune#4645 @bobot, reported by @Lelio-Brun ocaml/dune#4219, by @Kakadu ocaml/dune#4325, by @toots ocaml/dune#4415)

@emillon

…ator, dune-private-libs, dune and dune-build-info (2.9.0) CHANGES: - Add `(enabled_if ...)` to `(mdx ...)` (ocaml/dune#4434, @emillon) - Add support for instrumentation dependencies (ocaml/dune#4210, fixes ocaml/dune#3983, @nojb) - Add the possibility to use `locks` with the cram tests stanza (ocaml/dune#4480, @voodoos) - Allow to set up merlin in a variant of the default context (ocaml/dune#4145, @TheLortex, @voodoos) - Add `(package ...)` to `(mdx ...)` (ocaml/dune#4691, fixes ocaml/dune#3756, @emillon) - Handle renaming of `coq.kernel` library to `coq-core.kernel` in Coq 8.14 (ocaml/dune#4713, @proux01) - Fix generation of merlin configuration when using `(include_subdirs unqualified)` on Windows (ocaml/dune#4745, @nojb) - Fix bug for the install of Coq native files when using `(include_subdirs qualified)` (ocaml/dune#4753, @ejgallego) - Allow users to specify install target directories for `doc` and `etc` sections. We add new options `--docdir` and `--etcdir` to both Dune's configure and `dune install` command. (ocaml/dune#4744, fixes ocaml/dune#4723, @ejgallego, thanks to @JasonGross for reporting this issue) - Fix issue where Dune would ignore `(env ... (coq (flags ...)))` declarations appearing in `dune` files (ocaml/dune#4749, fixes ocaml/dune#4566, @ejgallego @rgrinberg) - Disable some warnings on Coq 8.14 and `(lang coq (>= 0.3))` due to the rework of the Coq "native" compilation system (ocaml/dune#4760, @ejgallego) - Fix a bug where instrumentation flags would be added even if the instrumentatation was disabled (@nojb, ocaml/dune#4770) - Fix ocaml/dune#4682: option `-p` takes now precedence on environement variable `DUNE_PROFILE` (ocaml/dune#4730, ocaml/dune#4774, @bobot, reported by @dra27 ocaml/dune#4632) - Fix installation with opam of package with dune sites. The `.install` file is now produced by a local `dune install` during the build phase (ocaml/dune#4730, ocaml/dune#4645, @bobot, reported by @kit-ty-kate ocaml/dune#4198) - Fix multiple issues in the sites feature (ocaml/dune#4730, ocaml/dune#4645 @bobot, reported by @Lelio-Brun ocaml/dune#4219, by @Kakadu ocaml/dune#4325, by @toots ocaml/dune#4415)

nojb requested review from emillon and rgrinberg as code owners February 10, 2021 09:11

ghost reviewed Feb 11, 2021

View reviewed changes

ghost approved these changes Feb 11, 2021

View reviewed changes

nojb force-pushed the instrumentation_deps branch from 684a4b5 to 3d1cb79 Compare February 17, 2021 19:37

nojb added 4 commits February 17, 2021 20:40

(instrumentation): add support for (deps) field

7ebe68c

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Add test

7de3167

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

Doc

f7afc9b

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

CHANGES.md

7f4a952

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb force-pushed the instrumentation_deps branch from 3d1cb79 to 7f4a952 Compare February 17, 2021 19:41

nojb merged commit fb6ab57 into ocaml:main Feb 17, 2021

nojb deleted the instrumentation_deps branch February 17, 2021 20:23

nojb added a commit to nojb/dune that referenced this pull request Apr 7, 2021

Allow to declare (instrumentation) dependencies (ocaml#4210)

80af4ea

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb added this to the 2.9 milestone Apr 7, 2021

nojb added a commit to nojb/dune that referenced this pull request Jun 2, 2021

Allow to declare (instrumentation) dependencies (ocaml#4210)

4e5f66a

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

nojb added a commit to nojb/dune that referenced this pull request Jun 2, 2021

Allow to declare (instrumentation) dependencies (ocaml#4210)

b046104

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>

ejgallego added a commit that referenced this pull request Jun 10, 2021

Allow to declare (instrumentation) dependencies (#4210) (#4686)

900e7a9

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com> Co-authored-by: Emilio Jesús Gallego Arias <e+git@x80.org>

ejgallego mentioned this pull request Jun 21, 2021

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to declare (instrumentation) dependencies#4210

Allow to declare (instrumentation) dependencies#4210
nojb merged 4 commits intoocaml:mainfrom
nojb:instrumentation_deps

nojb commented Feb 10, 2021

Uh oh!

aantron commented Feb 10, 2021

Uh oh!

ghost Feb 11, 2021

Uh oh!

nojb Feb 17, 2021

Uh oh!

ghost left a comment

Uh oh!

nojb commented Feb 17, 2021

Uh oh!

rgrinberg commented Feb 17, 2021

Uh oh!

nojb commented Feb 17, 2021

Uh oh!

rgrinberg commented Feb 17, 2021

Uh oh!

ejgallego commented Jun 2, 2021

Uh oh!

nojb commented Jun 2, 2021

Uh oh!

ejgallego commented Jun 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

nojb commented Feb 10, 2021

Uh oh!

aantron commented Feb 10, 2021

Uh oh!

ghost Feb 11, 2021

Choose a reason for hiding this comment

Uh oh!

nojb Feb 17, 2021

Choose a reason for hiding this comment

Uh oh!

ghost left a comment

Choose a reason for hiding this comment

Uh oh!

nojb commented Feb 17, 2021

Uh oh!

rgrinberg commented Feb 17, 2021

Uh oh!

nojb commented Feb 17, 2021

Uh oh!

rgrinberg commented Feb 17, 2021

Uh oh!

ejgallego commented Jun 2, 2021

Uh oh!

nojb commented Jun 2, 2021

Uh oh!

ejgallego commented Jun 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants