Skip to content
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

Replace uses of cfg = "host" with cfg = "exec" #15785

Closed
wants to merge 1 commit into from

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Jul 1, 2022

This increases the compatibility for builds where host and exec platform
are different (e.g., with remote execution).

This increases the compatibility for builds where host and exec platform
are different (e.g., with remote execution).
@Yannic Yannic marked this pull request as ready for review July 1, 2022 11:47
@Yannic Yannic requested review from a team, gregestren and fweikert as code owners July 1, 2022 11:47
@Yannic
Copy link
Contributor Author

Yannic commented Jul 1, 2022

@comius @susinmotion @gregestren PTAL

@Yannic
Copy link
Contributor Author

Yannic commented Jul 1, 2022

@bazel-io flag

(not a hard blocker for 5.3, but users can't test whether they are compatible with --incompatible_disable_starlark_host_transitions so we should cherry pick to make migration easier)

@Yannic
Copy link
Contributor Author

Yannic commented Jul 1, 2022

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jul 1, 2022
@ckolli5
Copy link

ckolli5 commented Jul 5, 2022

@bazel-io fork 5.3.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Jul 5, 2022
@meteorcloudy
Copy link
Member

@gregestren @comius Can you help review this one?

@meteorcloudy meteorcloudy requested review from comius and removed request for fweikert July 13, 2022 15:34
@aiuto aiuto requested a review from katre July 15, 2022 01:31
@aiuto
Copy link
Contributor

aiuto commented Jul 15, 2022

AFAIK, this is all fine.
I was concerned about the potential of fallout from //tools/builds_defs:hash changing config, but I don't think it will matter in any substantial way.

Copy link
Member

@katre katre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because cfg = "host" is treated identically to cfg = "exec", this change is effectively a no-op but makes the correct usage much more clear.

@katre katre added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jul 15, 2022
@benjaminp benjaminp deleted the yannic-cfg-exec branch July 19, 2022 17:54
ckolli5 pushed a commit to ckolli5/bazel that referenced this pull request Jul 19, 2022
This increases the compatibility for builds where host and exec platform
are different (e.g., with remote execution).

Closes bazelbuild#15785.

PiperOrigin-RevId: 461897893
Change-Id: I734ccdd1ae13e7c625cc22c894bc482fe080d70b
aranguyen pushed a commit to aranguyen/bazel that referenced this pull request Jul 20, 2022
This increases the compatibility for builds where host and exec platform
are different (e.g., with remote execution).

Closes bazelbuild#15785.

PiperOrigin-RevId: 461897893
Change-Id: I734ccdd1ae13e7c625cc22c894bc482fe080d70b
aranguyen pushed a commit to aranguyen/bazel that referenced this pull request Jul 20, 2022
This increases the compatibility for builds where host and exec platform
are different (e.g., with remote execution).

Closes bazelbuild#15785.

PiperOrigin-RevId: 461897893
Change-Id: I734ccdd1ae13e7c625cc22c894bc482fe080d70b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants