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

Update upb to fix compilation error with gcc 10 #12077

Closed
wants to merge 1 commit into from

Conversation

meteorcloudy
Copy link
Member

Fixes #12056

@meteorcloudy
Copy link
Member Author

@vbatts Can you verify if this PR fix the compilation error for you?

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020

The change looks fine, but the distfile tests are still failing in various ways.

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020

https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/6893#23242da4-4c34-4fd8-bf2e-f391ee10d809

It seems to be downloading 9effcbcb27f0a665f9f345030188c0b291e32482 instead of the newer version.
Did this miss the instance at WORKSPACE@641?

@meteorcloudy
Copy link
Member Author

@aiuto Nice catch, fixed!

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020

Last time I tried to update a dependency I had similar pain. I finally filed bug #12081 about it.

@meteorcloudy
Copy link
Member Author

meteorcloudy commented Sep 10, 2020

Hmm, looks like we have to explicitly override upb in the WORKSPACE file.

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020

Now it fails in the non_determinism test. Could it be related to //third_party/grpc:grpc_1.26.0.patch?

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020 via email

@meteorcloudy
Copy link
Member Author

Now it fails in the non_determinism test. Could it be related to //third_party/grpc:grpc_1.26.0.patch?

No, the patch is for something else. The upb is actually introduced in grpc_deps(), I tried to override it by declaring it directly in Bazel's WORKSPACE file. But it doesn't seem to work because the new upb version doesn't work with the old grpc version.. This is not looking good ...

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020

Yeah. I still see the same error with the update patch. But, if you decide you want it.

diff --git a/third_party/grpc/grpc_1.26.0.patch b/third_party/grpc/grpc_1.26.0.patch
index 03b4aed96a..25d0631773 100644
--- a/third_party/grpc/grpc_1.26.0.patch
+++ b/third_party/grpc/grpc_1.26.0.patch
@@ -70,12 +70,14 @@ index 09fcad95a2..9b737e5deb 100644
      if "com_google_absl" not in native.existing_rules():
 @@ -209,7 +212,10 @@ def grpc_deps():
              name = "upb",
-             sha256 = "61d0417abd60e65ed589c9deee7c124fe76a4106831f6ad39464e1525cef1454",
-             strip_prefix = "upb-9effcbcb27f0a665f9f345030188c0b291e32482",
+-            sha256 = "61d0417abd60e65ed589c9deee7c124fe76a4106831f6ad39464e1525cef1454",
+-            strip_prefix = "upb-9effcbcb27f0a665f9f345030188c0b291e32482",
 -            url = "https://github.com/protocolbuffers/upb/archive/9effcbcb27f0a665f9f345030188c0b291e32482.tar.gz",
++            sha256 = "7992217989f3156f8109931c1fc6db3434b7414957cb82371552377beaeb9d6c",
++            strip_prefix = "upb-382d5afc60e05470c23e8de19b19fc5ad231e732",
 +            urls = [
-+                "https://mirror.bazel.build/github.com/protocolbuffers/upb/archive/9effcbcb27f0a665f9f345030188c0b291e32482.tar.gz",
-+                "https://github.com/protocolbuffers/upb/archive/9effcbcb27f0a665f9f345030188c0b291e32482.tar.gz",
++                "https://mirror.bazel.build/github.com/protocolbuffers/upb/archive/382d5afc60e05470c23e8de19b19fc5ad231e732.tar.gz",
++                "https://github.com/protocolbuffers/upb/archive/382d5afc60e05470c23e8de19b19fc5ad231e732.tar.gz",
 +            ],
          )
      if "envoy_api" not in native.existing_rules():

@meteorcloudy
Copy link
Member Author

Thanks for coming up with the patch, unfortunately the newer version isn't compatible with grpc. The best I can come up with is to patch the original upb version, see #12083

@aiuto
Copy link
Contributor

aiuto commented Sep 10, 2020

So, is this PR moot, and we should do 3.5.1 with a cherrpick of #12083?

@meteorcloudy
Copy link
Member Author

Yes, looks like a regression we should fix.

bazel-io pushed a commit that referenced this pull request Sep 11, 2020
Fixes #12056

This is a replacement of #12077 after an unsuccessful attempt to upgrade upb version.

Closes #12083.

PiperOrigin-RevId: 331145667
aiuto pushed a commit that referenced this pull request Sep 12, 2020
Fixes #12056

This is a replacement of #12077 after an unsuccessful attempt to upgrade upb version.

Closes #12083.

PiperOrigin-RevId: 331145667
aiuto pushed a commit that referenced this pull request Sep 25, 2020
Fixes #12056

This is a replacement of #12077 after an unsuccessful attempt to upgrade upb version.

Closes #12083.

PiperOrigin-RevId: 331145667
laurentlb pushed a commit that referenced this pull request Oct 1, 2020
Fixes #12056

This is a replacement of #12077 after an unsuccessful attempt to upgrade upb version.

Closes #12083.

PiperOrigin-RevId: 331145667
laurentlb pushed a commit that referenced this pull request Oct 2, 2020
Fixes #12056

This is a replacement of #12077 after an unsuccessful attempt to upgrade upb version.

Closes #12083.

PiperOrigin-RevId: 331145667
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Oct 22, 2020
Fixes bazelbuild#12056

This is a replacement of bazelbuild#12077 after an unsuccessful attempt to upgrade upb version.

Closes bazelbuild#12083.

PiperOrigin-RevId: 331145667
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Oct 22, 2020
Fixes bazelbuild#12056

This is a replacement of bazelbuild#12077 after an unsuccessful attempt to upgrade upb version.

Closes bazelbuild#12083.

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

Successfully merging this pull request may close these issues.

3.5.0 doesn't compile with gcc 10
3 participants