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: update to 6.0.0rc1 #23678

Merged
merged 2 commits into from
Nov 1, 2022
Merged

Conversation

keith
Copy link
Member

@keith keith commented Oct 26, 2022

No description provided.

@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: #23678 was opened by keith.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Oct 27, 2022
@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 @phlax

🐱

Caused by: #23678 was synchronize by keith.

see: more, trace.

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith force-pushed the ks/bazel-update-to-6.0.0rc1 branch from 3e17981 to 4cbce1a Compare November 1, 2022 04:46
@keith keith marked this pull request as ready for review November 1, 2022 11:29
@@ -88,7 +88,7 @@ fi

# Complete envoy-static build
if [[ $BUILD_ENVOY_STATIC -eq 1 ]]; then
bazel "${BAZEL_STARTUP_OPTIONS[@]}" build "${BAZEL_BUILD_OPTIONS[@]}" //source/exe:envoy-static
bazel "${BAZEL_STARTUP_OPTIONS[@]}" build --output_groups=+pdb_file "${BAZEL_BUILD_OPTIONS[@]}" //source/exe:envoy-static
Copy link
Member Author

Choose a reason for hiding this comment

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

New requirement to force bazel to download this when using remote exec

Copy link
Member

Choose a reason for hiding this comment

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

i dont entirely understand why this is now required here but wondering if we need to add it anywhere else and/or update relevant docs

Copy link
Member Author

Choose a reason for hiding this comment

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

I moved it to the .bazelrc instead, which should eliminate the need for that for folks not using the scripts. the gist of it is that previously building cc_binary targets would always output the binary and the pdb file, now you have to explicitly request the pdb file, realistically I think this only applies to remote exec, which otherwise skips the download of it.

Copy link
Member

Choose a reason for hiding this comment

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

cool, thanks for explanation

bazel/repository_locations.bzl Show resolved Hide resolved
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith requested a review from phlax November 1, 2022 16:43
Copy link
Member

@phlax phlax left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @keith

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Nov 1, 2022
@keith keith merged commit 84df237 into envoyproxy:main Nov 1, 2022
@keith keith deleted the ks/bazel-update-to-6.0.0rc1 branch November 1, 2022 20:33
project_url = "https://github.com/bazelbuild/rules_license",
version = "0.0.3",
sha256 = "00ccc0df21312c127ac4b12880ab0f9a26c1cff99442dc6c5a331750360de3c3",
urls = ["https://github.com/bazelbuild/rules_license/releases/download/0.0.3/rules_license-0.0.3.tar.gz"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see this one come through - we shouldn't hard code the versions here. Should be
urls = ["https://github.com/bazelbuild/rules_license/releases/download/{version}/rules_license-{version}.tar.gz"],

Copy link
Member Author

Choose a reason for hiding this comment

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

oops thanks! #23780

phlax added a commit to phlax/envoy that referenced this pull request Nov 20, 2022
This reverts commit 84df237.

Signed-off-by: Ryan Northey <ryan@synca.io>
phlax added a commit that referenced this pull request Nov 21, 2022
Signed-off-by: Ryan Northey <ryan@synca.io>
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.

3 participants