Correct CARGO_MANIFEST_DIR values for Bazel <3.7.0 #469
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
Bazel 3.7.0 introduced a change (301b96b223b0c0) in the way it reports file paths for external packages. This ensures that rules_rust sets the right value for CARGO_MANIFEST_DIR both for Bazel < 3.7.0 and >= 3.7.0. Note that if this changed once, it might change again, so the right thing to do is to use a bazel API to do that work once it's available.
sitaktif
force-pushed
the
exec-root-rel-path
branch
from
58b017e
to
29b1783
Compare
illicitonion
approved these changes
Oct 30, 2020
There was a problem hiding this comment.
This looks good to me to support multiple versions of bazel in the absence of a stable API to get the exec path of the package directory.
@damienmg do you know if there's an API we've overlooked here, or a supported way to look up the exec path of a package directory? It feels like there should be, if these directories are treated as opaque and subject to change (as we saw in bazel 3.7.0)...
|
@damienmg Hey, pinging this PR. Seems pretty important to get in 😅 |
|
I am not aware of any API. Since the breaking change was not marked as breaking that should have been a reason for rollback I guess. It's probably too late now. |
Merged
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.
Bazel 3.7.0 introduced a change (301b96b223b0c0) in the way it reports
file paths for external packages. This ensures that rules_rust sets the
right value for CARGO_MANIFEST_DIR both for Bazel < 3.7.0 and >= 3.7.0.
Note that if this changed once, it might change again, so the right
thing to do is to use a bazel API to do that work once it's available.