fix: virtual libraries bug #10581
Merged
fix: virtual libraries bug #10581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When compiling an implementation of a virtual library, there's a check that makes sure we don't the virtual library doesn't exist in the closure of the implementation. This check tried to compute the linking closure of the library to do so. However, the linking closure might not be complete if the implementation contains other virtual library. To fix the issue, we use a "partial" linking closure that tries to compute the closure as much as possible, but doesn't fail on missing implementation. Fix #10460 Signed-off-by: Rudi Grinberg <me@rgrinberg.com> <!-- ps-id: eed8cded-17bc-425f-95b8-3e0337e2881d -->
anmonteiro
approved these changes
May 25, 2024
MA0010
pushed a commit
to MA0010/dune
that referenced
this pull request
Jun 5, 2024
When compiling an implementation of a virtual library, there's a check that makes sure we don't the virtual library doesn't exist in the closure of the implementation. This check tried to compute the linking closure of the library to do so. However, the linking closure might not be complete if the implementation contains other virtual library. To fix the issue, we use a "partial" linking closure that tries to compute the closure as much as possible, but doesn't fail on missing implementation. Fix ocaml#10460 Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
emillon
added a commit
to emillon/dune
that referenced
this pull request
Jun 11, 2024
Signed-off-by: Etienne Millon <me@emillon.org>
emillon
added a commit
that referenced
this pull request
Jun 11, 2024
emillon
added a commit
to emillon/opam-repository
that referenced
this pull request
Jun 17, 2024
CHANGES: ### Added - allow libraries with the same `(name ..)` in projects as long as they don't conflict during resolution (via `enabled_if`). (ocaml/dune#10307, @anmonteiro, @jchavarri) - `dune describe pp` now finds the exact module and the stanza it belongs to, instead of guessing the name of the preprocessed file. (ocaml/dune#10321, @anmonteiro) - Print the result of `dune describe pp` with the respective dialect printer. (ocaml/dune#10322, @anmonteiro) - Add new flag `--context` to `dune ocaml-merlin`, which allows to select a Dune context when requesting Merlin config. Add `dune describe contexts` subcommand. Introduce a field `generate_merlin_rules` for contexts declared in the workspace, that allows to optionally produce Merlin rules for other contexts besides the one selected for Merlin (ocaml/dune#10324, @jchavarri) - melange: add include paths for private library `.cmj` files during JS emission. (ocaml/dune#10416, @anmonteiro) - `dune ocaml-merlin`: communicate additional directives `SOURCE_ROOT`, `UNIT_NAME` (the actual name with wrapping) and `INDEX` with the paths to the index(es). (ocaml/dune#10422, @voodoos) - Add a new alias `@ocaml-index` that uses the `ocaml-index` binary to generate indexes that can be read by tools such as Merlin to provide project-wide references search. (ocaml/dune#10422, @voodoos) - merlin: add optional `(merlin_reader CMD)` construct to `(dialect)` stanza to configure a merlin reader (ocaml/dune#8567, @andreypopp) ### Changed - melange: treat private libraries with `(package ..)` as public libraries, fixing an issue where `import` paths were wrongly emitted. (ocaml/dune#10415, @anmonteiro) - install `.glob` files for Coq theories too (ocaml/dune#10602, @ejgallego) ### Fixed - Don't try to document non-existent libraries in doc-new target (ocaml/dune#10319, fixes ocaml/dune#10056, @jonludlam) - Make `dune-site`'s `load_all` function look for `META` files so that it doesn't fail on empty directories in the plugin directory (ocaml/dune#10458, fixes ocaml/dune#10457, @shym) - Fix incorrect warning for libraries defined inside non-existant directories using `(subdir ..)` and used by executables using `dune-build-info` (ocaml/dune#10525, @rgrinberg) - Don't try to take build lock when running `coq top --no-build` (ocaml/dune#10547, fixes ocaml/dune#7671, @lzy0505) - Make sure to truncate dune's lock file after locking and unlocking so that users cannot observe incorrect pid's (ocaml/dune#10575, @rgrinberg) - mdx: link mdx binary with `byte_complete`. This fixes `(libraries)` with foreign archives on Linux. (ocaml/dune#10586, fixes ocaml/dune#10582, @anmonteiro) - virtual libraries: fix an issue where linking an executable involving several virtual libries would cause an error. (ocaml/dune#10581, fixes ocaml/dune#10460, @rgrinberg)
avsm
pushed a commit
to avsm/opam-repository
that referenced
this pull request
Sep 5, 2024
CHANGES: ### Added - allow libraries with the same `(name ..)` in projects as long as they don't conflict during resolution (via `enabled_if`). (ocaml/dune#10307, @anmonteiro, @jchavarri) - `dune describe pp` now finds the exact module and the stanza it belongs to, instead of guessing the name of the preprocessed file. (ocaml/dune#10321, @anmonteiro) - Print the result of `dune describe pp` with the respective dialect printer. (ocaml/dune#10322, @anmonteiro) - Add new flag `--context` to `dune ocaml-merlin`, which allows to select a Dune context when requesting Merlin config. Add `dune describe contexts` subcommand. Introduce a field `generate_merlin_rules` for contexts declared in the workspace, that allows to optionally produce Merlin rules for other contexts besides the one selected for Merlin (ocaml/dune#10324, @jchavarri) - melange: add include paths for private library `.cmj` files during JS emission. (ocaml/dune#10416, @anmonteiro) - `dune ocaml-merlin`: communicate additional directives `SOURCE_ROOT`, `UNIT_NAME` (the actual name with wrapping) and `INDEX` with the paths to the index(es). (ocaml/dune#10422, @voodoos) - Add a new alias `@ocaml-index` that uses the `ocaml-index` binary to generate indexes that can be read by tools such as Merlin to provide project-wide references search. (ocaml/dune#10422, @voodoos) - merlin: add optional `(merlin_reader CMD)` construct to `(dialect)` stanza to configure a merlin reader (ocaml/dune#8567, @andreypopp) ### Changed - melange: treat private libraries with `(package ..)` as public libraries, fixing an issue where `import` paths were wrongly emitted. (ocaml/dune#10415, @anmonteiro) - install `.glob` files for Coq theories too (ocaml/dune#10602, @ejgallego) ### Fixed - Don't try to document non-existent libraries in doc-new target (ocaml/dune#10319, fixes ocaml/dune#10056, @jonludlam) - Make `dune-site`'s `load_all` function look for `META` files so that it doesn't fail on empty directories in the plugin directory (ocaml/dune#10458, fixes ocaml/dune#10457, @shym) - Fix incorrect warning for libraries defined inside non-existant directories using `(subdir ..)` and used by executables using `dune-build-info` (ocaml/dune#10525, @rgrinberg) - Don't try to take build lock when running `coq top --no-build` (ocaml/dune#10547, fixes ocaml/dune#7671, @lzy0505) - Make sure to truncate dune's lock file after locking and unlocking so that users cannot observe incorrect pid's (ocaml/dune#10575, @rgrinberg) - mdx: link mdx binary with `byte_complete`. This fixes `(libraries)` with foreign archives on Linux. (ocaml/dune#10586, fixes ocaml/dune#10582, @anmonteiro) - virtual libraries: fix an issue where linking an executable involving several virtual libries would cause an error. (ocaml/dune#10581, fixes ocaml/dune#10460, @rgrinberg)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When compiling an implementation of a virtual library, there's a check that makes sure we don't the virtual library doesn't exist in the closure of the implementation.
This check tried to compute the linking closure of the library to do so. However, the linking closure might not be complete if the implementation contains other virtual library.
To fix the issue, we use a "partial" linking closure that tries to compute the closure as much as possible, but doesn't fail on missing implementation.
Fix #10460
Signed-off-by: Rudi Grinberg me@rgrinberg.com