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.

Sign up for GitHub

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

Jump to bottom

Incompatible: Remove legacy allowlist names #19493

Open
comius opened this issue Sep 12, 2023 · 12 comments
Open

Incompatible: Remove legacy allowlist names #19493

comius opened this issue Sep 12, 2023 · 12 comments
Assignees
@comius
Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts

Comments

@comius
Copy link
Contributor

comius commented Sep 12, 2023

The change removes legacy _whitelist_function_transition.

First merged and rolled back:
10a46b1

The naming is in use in old rules_go, which several projects haven't upgraded.
@comius
Copy link
Contributor Author

comius commented Sep 12, 2023 •
edited
Loading

Help needed:

Pending fixes:

Fixed (not verified):

rules_docker was archived:

Buildfarm depends on rules_docker:

@comius comius self-assigned this Sep 12, 2023
@comius comius added incompatible-change Incompatible/breaking change team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts P2 We'll consider working on this in future. (Assignee optional) labels Sep 12, 2023
@comius
Copy link
Contributor Author

comius commented Sep 12, 2023

cc @fmeum

@comius comius mentioned this issue Sep 12, 2023
Open
4 tasks
@rickeylev rickeylev mentioned this issue Sep 12, 2023
Merged
github-merge-queue bot pushed a commit to bazelbuild/rules_python that referenced this issue Sep 13, 2023
@sgowroji
fix(gazelle): upgrade rules_go: 0.39.1 -> 0.41.0 to work with upcomin…
4769fea 
…g Bazel versions (#1410)

This makes rules_python's usage of Go work with upcoming Bazel versions.
The
`_whitelist_function_transition` attribute in the Go rules is being
removed, per
bazelbuild/bazel#19493. Newer Go rule releases
are compatible
with the upcoming Bazel versions.

Fixes : [1409](#1409)
@comius
Copy link
Contributor Author

comius commented Oct 13, 2023

One month has passed, without maintainers upgrading rules_go. So I believe we can roll forward the change again.

comius added a commit to comius/bazel that referenced this issue Oct 13, 2023
@comius @copybara-github
Rollforward of bazelbuild@10a46b1: Remove legacy allowlist names
003fbff 
NEW: Maintainers of the repos had 1 month to upgrade version of rules Go bazelbuild#19493
NEW: syncing against code changes
PiperOrigin-RevId: 573099651
Change-Id: I517e45a83989bfa24f6487ef6d89d60aeb31e4d3
@comius comius mentioned this issue Oct 13, 2023
Closed
@fmeum
Copy link
Collaborator

fmeum commented Oct 13, 2023

@comius Would you be supportive of a change to remove the allowlist attribute requirement for Bazel, for example via a flag that is unflipped within Google via a .blazerc line? I have never seen anyone specify a non-trivial package_group outside Google, especially not in publicly available rulesets.

@comius
Copy link
Contributor Author

comius commented Oct 13, 2023

Actually I was thinking to change the implementation so that attribute is automatically added when there’s a transition.

The only problem is that I’d need it to either always point to @bazel_tools (simple) or add a flag to configure where it should point.

@comius
Copy link
Contributor Author

comius commented Oct 13, 2023

Cc @gregestren

@fmeum
Copy link
Collaborator

fmeum commented Oct 13, 2023

If users do want to point it to a non-standard location, could they still explicitly add the attribute? If that is the case, then having it always point into @bazel_tools when it is added automatically sounds fine to me.

copybara-service bot pushed a commit that referenced this issue Oct 13, 2023
@comius @copybara-github
Rollforward of 10a46b1: Remove legacy allowlist names
4064d78 
NEW: Maintainers of the repos had 1 month to upgrade version of rules Go #19493
NEW: rebasing the code after 1 month
NEW: Downstream test: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3381
PiperOrigin-RevId: 573199364
Change-Id: I517e45a83989bfa24f6487ef6d89d60aeb31e4d3
@gregestren
Copy link
Contributor

I support removing the requirement from Bazel, whatever the implementation (both sound fine to me).

@comius
Copy link
Contributor Author

comius commented Oct 17, 2023

Ack. Working on the implementation.

copybara-service bot pushed a commit that referenced this issue Oct 17, 2023
@comius @copybara-github
Automatically add function transition allow list when needed
bb7fb2d 
If the allowlist is already present its value is respected (The package paths and label name are fixed/checked. Only repository name can be changed).

Automatically add allowlist with tools repository as defined by rule definition environment.

This fixes the incompatibility introduced in: #19493 (because whilelist is ignored and default allowlist is added)

PiperOrigin-RevId: 574226941
Change-Id: If90f78a610d7bd3c107dcd94a39902c7c939aa7b
@fmeum
Copy link
Collaborator

fmeum commented Oct 17, 2023

@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 Oct 17, 2023
@fmeum
Copy link
Collaborator

fmeum commented Oct 17, 2023

@iancha1992 Could bb7fb2d be marked for cherry-picking to 7.0.0?

@iancha1992
Copy link
Member

@bazel-io fork 7.0.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 Oct 17, 2023
@bazel-io bazel-io mentioned this issue Oct 17, 2023
Closed
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Oct 17, 2023
@comius @bazel-io
Automatically add function transition allow list when needed
7e7e613 
If the allowlist is already present its value is respected (The package paths and label name are fixed/checked. Only repository name can be changed).

Automatically add allowlist with tools repository as defined by rule definition environment.

This fixes the incompatibility introduced in: bazelbuild#19493 (because whilelist is ignored and default allowlist is added)

PiperOrigin-RevId: 574226941
Change-Id: If90f78a610d7bd3c107dcd94a39902c7c939aa7b
@bazel-io bazel-io mentioned this issue Oct 17, 2023
Merged
@iancha1992 iancha1992 mentioned this issue Oct 17, 2023
Closed
iancha1992 pushed a commit that referenced this issue Oct 18, 2023
@bazel-io @comius
[7.0.0] Automatically add function transition allow list when needed (#…
60f3319 
…19855)

If the allowlist is already present its value is respected (The package
paths and label name are fixed/checked. Only repository name can be
changed).

Automatically add allowlist with tools repository as defined by rule
definition environment.

This fixes the incompatibility introduced in:
#19493 (because whilelist is
ignored and default allowlist is added)

Commit
bb7fb2d

PiperOrigin-RevId: 574226941
Change-Id: If90f78a610d7bd3c107dcd94a39902c7c939aa7b

Co-authored-by: Googler <[email protected]>
fweikert pushed a commit that referenced this issue Nov 7, 2023
@comius @fweikert
Automatically add function transition allow list when needed
c387f85 
If the allowlist is already present its value is respected (The package paths and label name are fixed/checked. Only repository name can be changed).

Automatically add allowlist with tools repository as defined by rule definition environment.

This fixes the incompatibility introduced in: #19493 (because whilelist is ignored and default allowlist is added)

PiperOrigin-RevId: 574226941
Change-Id: If90f78a610d7bd3c107dcd94a39902c7c939aa7b
aherrmann added a commit to aherrmann/rules_zig that referenced this issue Dec 1, 2023
@aherrmann
Rename to function_transition_allowlist
5f7fde7 
See bazelbuild/bazel#19493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@comius comius

Labels
incompatible-change Incompatible/breaking change P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@comius @fmeum @gregestren @bazel-io @iancha1992