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

Update rules_foreign_cc version #7058

Closed
wants to merge 2 commits into from
Closed

Update rules_foreign_cc version #7058

wants to merge 2 commits into from

Conversation

hlopko
Copy link
Contributor

@hlopko hlopko commented May 23, 2019

It contains forward fixes for Bazel 0.27.0

Description:
Risk Level: Low
Testing: Presubmits

@hlopko
Copy link
Contributor Author

hlopko commented May 23, 2019 •
edited
Loading

It seems Bazel < 0.25.2 is used. Are there plans to bump the version to 0.25.2? @htuch

@htuch
Copy link
Member

htuch commented May 23, 2019

CC @lizan

@lizan
Copy link
Member

lizan commented May 24, 2019

Yeah it needs #6866, which I couldn't make coverage works

@hlopko
Copy link
Contributor Author

hlopko commented May 28, 2019

I can make rules_foreign_cc work with 0.24 to unblock myself (I will update this PR once bazel-contrib/rules_foreign_cc#270 is submitted).

But we will need to speed up other incompatible flags migrations (we - the Bazel team - are in increased pressure because 0.27 will start a 3 month window of no incompatible changes). We will have to potentially temporarily disable envoy on our downstream pipeline.

@hlopko hlopko mentioned this pull request May 28, 2019
Closed
@hlopko hlopko force-pushed the update_rules_foreign_cc branch 3 times, most recently from 0816b1e to cec30d8 Compare May 28, 2019 08:38
@hlopko
Copy link
Contributor Author

hlopko commented May 28, 2019

Hi @lizan @htuch

I've updated this PR and it should be green. Failures on the ci look unrelated (java.io.IOException: Error downloading [https://github.com/google/benchmark/archive/v1.5.tar.gz]).

Can we move forward with this PR?

@moderation
Copy link
Contributor

@hlopko It looks like there is a new release of v1.5.0 of benchmark so they have updated the binary. This makes our sha check fail. To fix this we'll need a PR to correct the sha. I can create this if you like.

@hlopko
Copy link
Contributor Author

hlopko commented May 28, 2019

That'd be great, thanks!

@htuch
Copy link
Member

htuch commented May 28, 2019

@moderation see #7090

@hlopko
Copy link
Contributor Author

hlopko commented May 28, 2019

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: clang_tidy (failed build)
🔨 rebuilding ci/circleci: asan (failed build)
🔨 rebuilding ci/circleci: tsan (failed build)
🔨 rebuilding ci/circleci: ipv6_tests (failed build)
🔨 rebuilding ci/circleci: release (failed build)
🔨 rebuilding ci/circleci: compile_time_options (failed build)

🐱

Caused by: a #7058 (comment) was created by @hlopko.

see: more, trace.

@htuch
Copy link
Member

htuch commented May 28, 2019

@hlopko please merge master to pick up the fix in #7090

bazel-io pushed a commit to bazelbuild/bazel that referenced this pull request May 29, 2019
@hlopko @copybara-github
Flip --incompatible_require_ctx_in_configure_features
1bfedba 
Last run of the CI (https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/124) shows only Tensorflow and Envoy issues. Envoy is fixed by envoyproxy/envoy#7058, and Tensorflow is fixed in Bazel@HEAD by cab65ff.

RELNOTES: Incompatible flag `--incompatible_require_ctx_in_configure_features` has been flipped. See #7793 for more information.
PiperOrigin-RevId: 250458681
@hlopko
Copy link
Contributor Author

hlopko commented May 29, 2019

udp_listener_impl_test failure in envoy-macos seems unrelated (maybe flake?).
ci/circleci:coverage prints
//test/coverage:coverage_tests TIMEOUT in 4015.0s

which also seems unrelated?

@hlopko
Copy link
Contributor Author

hlopko commented May 29, 2019

Oh and to be explicit - I rebased this PR on master, fixes for #7090 are in now.

@htuch
Copy link
Member

htuch commented May 30, 2019

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7058 (comment) was created by @htuch.

see: more, trace.

@htuch
Copy link
Member

htuch commented May 30, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@htuch
Copy link
Member

htuch commented May 30, 2019

@hlopko coverage seems to be broken in rules_foreign_cc in the most recent run at https://circleci.com/gh/envoyproxy/envoy/226403?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@lizan
Copy link
Member

lizan commented Jun 3, 2019

@hlopko Can you merge master to pickup #6866 contains the fix for coverage?

@hlopko
Update rules_foreign_cc version
10e1d18 
Update rules_foreign_cc to bazel-contrib/rules_foreign_cc@604f1e1

Signed-off-by: Marcel Hlopko <hlopko@google.com>
@hlopko
Update rules_foreign_cc to bazel-contrib/rules_foreign_cc@6b2f454
8190124 
Signed-off-by: Marcel Hlopko <hlopko@google.com>
@hlopko
Copy link
Contributor Author

hlopko commented Jun 4, 2019

I just did that. Does #6866 actually fix coverage? https://circleci.com/gh/envoyproxy/envoy/226720?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link seems like a failing build.

In any case, it seems this PR still fails with coverage:

git clone '--shallow-since=1551386336 +0000' https://go.googlesource.com/tools /build/tmp/_bazel_bazel/b570b5ccd0454dc9af9f65ab1833764d/external/org_golang_x_tools
error: unknown option `shallow-since=1551386336 +0000'

@lizan
Copy link
Member

lizan commented Jun 4, 2019

/retest
@hlopko yes it does, the failing CI for this PR seems a transient error with network.

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: api (failed build)
🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7058 (comment) was created by @lizan.

see: more, trace.

@lizan
Copy link
Member

lizan commented Jun 4, 2019

@hlopko also do not rebase + force-push in the future, it mess up GH review history and hard to track last master merge. A regular merge is preferred. The PR will be squashed into one commit at the end anyway.

@lizan lizan mentioned this pull request Jun 4, 2019
Merged
@lizan
Copy link
Member

lizan commented Jun 4, 2019

Closing in favor of #7168

@lizan lizan closed this Jun 4, 2019
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
@hlopko @irengrig
Flip --incompatible_require_ctx_in_configure_features
d1ccc72 
Last run of the CI (https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/124) shows only Tensorflow and Envoy issues. Envoy is fixed by envoyproxy/envoy#7058, and Tensorflow is fixed in Bazel@HEAD by bazelbuild@cab65ff.

RELNOTES: Incompatible flag `--incompatible_require_ctx_in_configure_features` has been flipped. See bazelbuild#7793 for more information.
PiperOrigin-RevId: 250458681
irengrig pushed a commit to irengrig/bazel that referenced this pull request Jul 15, 2019
@hlopko @irengrig
Flip --incompatible_require_ctx_in_configure_features
19aac86 
Last run of the CI (https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/124) shows only Tensorflow and Envoy issues. Envoy is fixed by envoyproxy/envoy#7058, and Tensorflow is fixed in Bazel@HEAD by bazelbuild@cab65ff.

RELNOTES: Incompatible flag `--incompatible_require_ctx_in_configure_features` has been flipped. See bazelbuild#7793 for more information.
PiperOrigin-RevId: 250458681
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this pull request Sep 4, 2022
Flip --incompatible_require_ctx_in_configure_features
2e1a376 
    Last run of the CI (https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/124) shows only Tensorflow and Envoy issues. Envoy is fixed by envoyproxy/envoy#7058, and Tensorflow is fixed in Bazel@HEAD by bazelbuild/bazel@cab65ff.

    RELNOTES: Incompatible flag `--incompatible_require_ctx_in_configure_features` has been flipped. See bazelbuild/bazel#7793 for more information.
    PiperOrigin-RevId: 250458681
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lizan lizan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hlopko @htuch @lizan @moderation