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

bazel: fix config_setting visibility issues #24237

Merged

Conversation

keith
Copy link
Member

@keith keith commented Nov 29, 2022

This pulls in a googleurl patch that's waiting on upstream, and updates rules_go for compatibility with this flag. This also flips it early so we don't regress before it's flipped upstream.

Fixes #24183

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

This pulls in a googleurl patch that's waiting on upstream, and updates
rules_go for compatibility with this flag. This also flips it early so
we don't regress before it's flipped upstream.

Fixes envoyproxy#24183

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #24237 was opened by keith.

see: more, trace.

@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @RyanTheOptimist

🐱

Caused by: #24237 was opened by keith.

see: more, trace.

Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

LGTM, though this PR is marked as draft so maybe it's not actually ready?

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 29, 2022
@keith keith marked this pull request as ready for review November 29, 2022 17:47
@keith keith merged commit eb41b78 into envoyproxy:main Nov 29, 2022
@keith keith deleted the ks/bazel-fix-config_setting-visibility-issues branch November 29, 2022 17:47
@keith
Copy link
Member Author

keith commented Nov 29, 2022

thanks! was just a draft in case of more surprises

@@ -31,6 +31,9 @@ build --action_env=PATH --host_action_env=PATH
build --enable_platform_specific_config
build --test_summary=terse

# TODO(keith): Remove once this is the default
build --incompatible_config_setting_private_default_visibility
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably have to enable --incompatible_enforce_config_setting_visibility as well to make sure this actually take effect.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flag --incompatible_config_setting_private_default_visibility will break Envoy in Bazel 7.0
3 participants