-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Add incompatible flag to disable cfg="host" from Starlark #14383
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
Add incompatible flag to disable cfg="host" from Starlark #14383
Conversation
Host transitions are problematic where the host and exec environment are different (e.g., with Remote Execution). This change adds an incompatible flag that fails analysis if Starlark rules use `cfg = "host"`. Migrating is pretty straigt forward (replace `host` with `exec`) and should be automatable by buildifier (although I haven't tried to do so).
|
@katre and/or @gregestren PTAL |
|
@comius is actually working on something similar, so I'm assigning this to him so the two of you can discuss. |
This doesn't really matter now because all tools here are just Bash and Python scripts, but could prevent problems later when an incompatible flag to disable cfg="host" from Starlark lands. bazelbuild/bazel#14383
This doesn't really matter now because all tools here are just Bash and Python scripts, but could prevent problems later when an incompatible flag to disable cfg="host" from Starlark lands. bazelbuild/bazel#14383
|
cc @susinmotion I would attempt the more aggressive path first, where we just change all "host" transitions to "exec". This would reduce the churn substantially. We're already fixing some issues that happen with that plan (within Bazel). If this plan fails, this PR becomes viable alternative. |
|
In my experiments, it seemed like tools build in exec config do get I think we should probably still make Should I send the documentation change in a separate PR? That seems useful regardless of whether we add the flag or not. |
|
@susinmotion 's been focusing more on this recently. @susinmotion - do you have recommendations for @Yannic 's latest comment? |
|
Cool, thanks for thinking about this. A separate PR for documentation sounds great--we're really close to making this change. I'm torn about making cfg="host" an error after the migration. On the one hand, it will make people's rules less confusing in the long term, since they'll say "exec" when they mean "exec". On the other hand, it causes breakages with things that aren't actually a problem, and that's a bummer for users. Can we do some kind of migration script here instead? |
|
@susinmotion thanks for the update!
Yes! Added a |
|
Hey, can you resolve the conflicts? Host transition has been removed now, so a flag to completely disable it is useful. |
comius
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the conflicts.
Friendly ping. |
|
@comius Updated, PTAL |
src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
Outdated
Show resolved
Hide resolved
…/BuildLanguageOptions.java Co-authored-by: Ivo List <[email protected]>
|
@bazel-io flag |
|
@bazel-io fork 5.2.0 |
Host transitions are problematic where the host and exec environment are different (e.g., with Remote Execution). This change adds an incompatible flag that fails analysis if Starlark rules use `cfg = "host"`. Migrating is pretty straigt forward (replace `host` with `exec`) and should be automatable by buildifier (although I haven't tried to do so). Closes #14383. PiperOrigin-RevId: 441253060 Co-authored-by: Yannic <[email protected]>
Host transitions are problematic where the host and exec environment are
different (e.g., with Remote Execution). This change adds an incompatible
flag that fails analysis if Starlark rules use
cfg = "host".Migrating is pretty straigt forward (replace
hostwithexec) andshould be automatable by buildifier (although I haven't tried to do so).