Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record instrumentation backends in dune-package #3735

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

nojb
Copy link
Collaborator

@nojb nojb commented Aug 21, 2020

Embarrassingly, it seems that instrumentation backends are not recorded in dune-package which renders the feature unusable across installed libraries (eg opam). Bug discovered by @patricoferris.

@patricoferris
Copy link

Thanks @nojb -- tested it locally with modified versions of landmarks and bisect_ppx (pinning them in my opam switch) and all seems to be working.

@nojb
Copy link
Collaborator Author

nojb commented Aug 21, 2020

Thanks for the confirmation @patricoferris!

@rgrinberg would it be possible to make a bug fix release with this?

@nojb nojb force-pushed the fix_instrumentation_dune_package branch from 1bd44e3 to 09c4581 Compare August 21, 2020 15:52
@rgrinberg
Copy link
Member

Sounds good. Could you add a CHANGES entry?

@nojb
Copy link
Collaborator Author

nojb commented Aug 21, 2020

Sounds good. Could you add a CHANGES entry?

Thanks, done!

@rgrinberg
Copy link
Member

Do we have a test for this?

If not, I think we should add a test for installed instrumentation backends. See how we do it for inline tests, ppx drivers, or virtual libraries for an example.

@nojb
Copy link
Collaborator Author

nojb commented Aug 22, 2020

Do we have a test for this?

No, we don't.

If not, I think we should add a test for installed instrumentation backends. See how we do it for inline tests, ppx drivers, or virtual libraries for an example.

Will do, thanks.

@nojb nojb force-pushed the fix_instrumentation_dune_package branch 2 times, most recently from cd81c10 to 4c5eccf Compare August 22, 2020 05:26
@nojb
Copy link
Collaborator Author

nojb commented Aug 22, 2020

If not, I think we should add a test for installed instrumentation backends. See how we do it for inline tests, ppx drivers, or virtual libraries for an example.

This is now done.

Signed-off-by: Nicolás Ojeda Bär <n.oje.bar@gmail.com>
@rgrinberg rgrinberg force-pushed the fix_instrumentation_dune_package branch from 4c5eccf to ae15a89 Compare August 25, 2020 19:07
@rgrinberg rgrinberg merged commit c27641d into ocaml:master Aug 25, 2020
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Sep 3, 2020
…lugin, dune-private-libs and dune-glob (2.7.1)

CHANGES:

- configurator: More flexible probing of `#define`. We allow duplicate values in
  the object file, as long as they are the same after parsing. (ocaml/dune#3739, fixes
  ocaml/dune#3736, @rgrinberg)

- Record instrumentation backends in dune-package files. This makes it possible
  to use instrumentation backends defined in installed libraries (eg via OPAM).
  (ocaml/dune#3735, @nojb)

- Add missing `.aux` & `.glob` targets to coq rules (ocaml/dune#3721, fixes ocaml/dune#3437,
  @rgrinberg)

- Fix `dune-package` installation when META templates are present (ocaml/dune#3743, fixes
  ocaml/dune#3746, @rgrinberg)

- Resolve symlinks before running `$ git diff` (ocaml/dune#3750, fixes ocaml/dune#3740, @rgrinberg)

- Cram tests: when checking that all test directories contain a `run.t` file,
  skip empty directories. These can be left around by git. (ocaml/dune#3753, @emillon)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Sep 5, 2020
…lugin, dune-private-libs and dune-glob (2.7.1)

CHANGES:

- configurator: More flexible probing of `#define`. We allow duplicate values in
  the object file, as long as they are the same after parsing. (ocaml/dune#3739, fixes
  ocaml/dune#3736, @rgrinberg)

- Record instrumentation backends in dune-package files. This makes it possible
  to use instrumentation backends defined in installed libraries (eg via OPAM).
  (ocaml/dune#3735, @nojb)

- Add missing `.aux` & `.glob` targets to coq rules (ocaml/dune#3721, fixes ocaml/dune#3437,
  @rgrinberg)

- Fix `dune-package` installation when META templates are present (ocaml/dune#3743, fixes
  ocaml/dune#3746, @rgrinberg)

- Resolve symlinks before running `$ git diff` (ocaml/dune#3750, fixes ocaml/dune#3740, @rgrinberg)

- Cram tests: when checking that all test directories contain a `run.t` file,
  skip empty directories. These can be left around by git. (ocaml/dune#3753, @emillon)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants