Update cargo_build_script to work without runfiles support.
#2871
Conversation
UebelAndre
force-pushed
the
cargo_runfiles
branch
9 times, most recently
from
c279095
to
77a9de9
Compare
UebelAndre
force-pushed
the
cargo_runfiles
branch
from
aa88b9e
to
77a9de9
Compare
There was a problem hiding this comment.
Works for me!
cc @fmeum just as an FYI in case there's anything you think should live more upstream somewhere
cargo/private/runfiles_enabled.bzl
Outdated
| ":{}_enable_runfiles".format(name): True, | ||
| ":{}_disable_runfiles".format(name): False, | ||
| # Otherwise fall back to the system default. | ||
| "@platforms//os:windows": False, |
There was a problem hiding this comment.
I'm surprised Bazel is happy to treat these as independent - doesn't it have a precedence conflict between the @platforms branch and the runfiles ones?
There was a problem hiding this comment.
It complained when you actually specified --noenable_runfiles. I've updated this to work on windows and unix systems where the flag is enabled, disabled, and unset.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
It would be great because I made this a lot more complicated lol |
UebelAndre
force-pushed
the
cargo_runfiles
branch
from
91057a2
to
f4166de
Compare
UebelAndre
force-pushed
the
cargo_runfiles
branch
from
42e40d0
to
bfc741f
Compare
UebelAndre
force-pushed
the
cargo_runfiles
branch
from
bfc741f
to
d277e02
Compare
UebelAndre
force-pushed
the
cargo_runfiles
branch
from
9724497
to
041c478
Compare
| workspace_name = ctx.label.workspace_name | ||
| if not workspace_name: | ||
| workspace_name = ctx.workspace_name |
There was a problem hiding this comment.
| workspace_name = ctx.label.workspace_name | |
| if not workspace_name: | |
| workspace_name = ctx.workspace_name | |
| workspace_name = ctx.workspace_name |
External repos always fall into the ../ branch anyway.
| workspace_name = ctx.label.workspace_name | ||
| if not workspace_name: | ||
| workspace_name = ctx.workspace_name |
There was a problem hiding this comment.
See above, this can be simplified.
| ), | ||
| } | ||
|
|
||
| def runfiles_enabled_build_setting(name, **kwargs): |
There was a problem hiding this comment.
Nice hack ;-)
I think that this can be simplified since you don't need a single build setting to match both the platform and the value of --enable_runfiles. This currently also matches the target platform, but the default is based on the host platform.
How about only matching the flag here and then using ctx.configuration.host_path_separator to filter out a Windows host in runfiles_enabled_build_setting?
There was a problem hiding this comment.
@fmeum This took an embarrassing amount of brain power 😅 , could I get you to slice and dice this code to be the simpler form?
| @@ -0,0 +1,71 @@ | |||
| //! A tool for building runfiles directories for Bazel environments that don't | |||
| //! support runfiles or have runfiles enabled. | |||
…bazelbuild#2871)" This reverts commit c520417.
…uild#2871) Partially addresses bazelbuild#1156
|
This change ended up being more complicated and needs another review but I forgot to disable auto-merge. Revert is here #2885 |
Partially addresses #1156