pkg: Undo change reversing order of env updates #10474
Merged
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
src/dune_rules/pkg_rules.ml
Outdated
| @@ -337,7 +337,7 @@ module Pkg = struct | |||
| appearing earlier can overwrite the values of variables set by | |||
| packages appearing later. *) | |||
There was a problem hiding this comment.
I think this comment needs to be adapted now because it states the reverse of what is happening and that's very confusing.
There was a problem hiding this comment.
Good catch. Fixed!
rgrinberg
approved these changes
May 1, 2024
| Prepended with trailing sep:Prepended 2nd time with sep | ||
| Appended 2nd time without leading sep:Appended without leading sep | ||
| Appended 2nd time with leading sep:Appended with leading sep | ||
| Hello from the second package! |
There was a problem hiding this comment.
Could you leave a comment that this output is incorrect?
In 3ea5444 the order of env updates performed by packages was reversed. This brought the behaviour of dune closer to opam but introduced a regression which prevents ocamlfind from building with dune package management. Here's an issue for tracking investigating why this change caused the regression: ocaml#10473 Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
anmonteiro
pushed a commit
to anmonteiro/dune
that referenced
this pull request
Nov 17, 2024
In 3ea5444 the order of env updates performed by packages was reversed. This brought the behaviour of dune closer to opam but introduced a regression which prevents ocamlfind from building with dune package management. Here's an issue for tracking investigating why this change caused the regression: ocaml#10473 Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
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.
In 3ea5444 the order of env updates performed by packages was reversed. This brought the behaviour of dune closer to opam but introduced a regression which prevents ocamlfind from building with dune package management.
Here's an issue for tracking investigating why this change caused the regression: #10473
I'm proposing we merge this to revert to the old behaviour in the short term and then investigate why changing the order of updates caused the regression.