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 Protobuf to 3.11.3 #10651

Closed
wants to merge 1 commit into from
Closed

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Jan 24, 2020

No description provided.

@Yannic Yannic changed the title Update protobuf to 3.11 Update Protobuf to 3.11 Jan 24, 2020
@irengrig irengrig added the team-Rules-Server Issues for serverside rules included with Bazel label Feb 3, 2020
@lberki
Copy link
Contributor

lberki commented Feb 4, 2020

(I'll hold off reviewing this until #10648 lands)

@philwo
Copy link
Member

philwo commented Feb 6, 2020

I think all dependencies of this are merged now!

@Yannic Yannic changed the title Update Protobuf to 3.11 Update Protobuf to 3.11.3 Feb 6, 2020
@Yannic
Copy link
Contributor Author

Yannic commented Feb 6, 2020

We're mostly there! #10723 adds a patch for Protobuf to use the existing references for zlib, six, guava, ... When that landed, I'll rebase this PR (already verified locally that it works) and merge, and as a last change we get to delete the vendored version of Protobuf 😃.

@Yannic Yannic force-pushed the update_protobuf branch 2 times, most recently from b494f2c to a467720 Compare February 10, 2020 14:12
@philwo
Copy link
Member

philwo commented Feb 10, 2020

Hi @Yannic,

just saw the failed tests here with this message:

ERROR: Analysis of target '//a:c' failed; build aborted: no such package '@bazel_skylib//lib': java.io.IOException: Error downloading [https://github.com/bazelbuild/bazel-skylib/archive/0.9.0.tar.gz] to /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/sandbox/linux-sandbox/2733/execroot/io_bazel/_tmp/53cf69ace19a0796fc4537030d753e89/root/917143f2cfdfb15433399b1574e0bac0/external/bazel_skylib/0.9.0.tar.gz: Unknown host: github.com

You can fix this with a little tweak:

  • Add @bazel_skylib//:WORKSPACE to the test_repos filegroup in src/BUILD.
  • Add bazel_skylib to the repos array in testenv.sh.

Then it should work!

@Yannic Yannic force-pushed the update_protobuf branch 6 times, most recently from d0303aa to a28d598 Compare February 10, 2020 16:00
@Yannic
Copy link
Contributor Author

Yannic commented Feb 10, 2020

@philwo Thanks! That fixed the skylib error. Unfortunately, there is a new error because of zlib being referenced as @io_bazel//third_party/zlib and I can't seem to figure out how to fix that.

@philwo
Copy link
Member

philwo commented Feb 10, 2020

Mhm... yeah, I'm not sure how to solve that. What's the reason that we're patching the references to @zlib and @six in protobuf's BUILD file via https://github.com/bazelbuild/bazel/blob/master/third_party/protobuf/3.11.3.patch?

If we change this: https://github.com/bazelbuild/bazel/blob/master/WORKSPACE#L59-L62 to:

local_repository(
    name = "six",
    path = "./third_party/py/six/",
)

and add one for zlib:

local_repository(
    name = "zlib",
    path = "./third_party/zlib/",
)

Maybe it works?

@Yannic
Copy link
Contributor Author

Yannic commented Feb 10, 2020

Finally figured it out. I think the problem was actually that bash didn't like the use of ` in a comment.

Also, I'm not sure why, but adding local_repository(name = "zlib, path = "...") prohibits from building //third_party/zlib (did you mean @zlib//), while new_local_repository(name = "zlib, path = "...", build_file = ".../BUILD") doesn't. I find that very confusing.

@Yannic Yannic marked this pull request as ready for review February 10, 2020 18:38
@bazel-io bazel-io closed this in 5e571d2 Feb 13, 2020
@philwo
Copy link
Member

philwo commented Feb 13, 2020

@Yannic We're done!! 🎉

Thank you so much for this contribution and your hard work through the little issues and blockers :)

Yannic added a commit to Yannic/bazel that referenced this pull request Feb 14, 2020
These files are no longer needed since Protobuf was upgrade
to 3.11.3 in bazelbuild#10651.
Yannic added a commit to Yannic/bazel that referenced this pull request Feb 14, 2020
These files are no longer needed since Protobuf was upgrade
to 3.11.3 in bazelbuild#10651.
bazel-io pushed a commit that referenced this pull request Feb 14, 2020
These files are no longer needed since Protobuf was upgrade
to 3.11.3 in #10651.

Partial commit for third_party/*, see #10784.

Signed-off-by: Philipp Wollermann <[email protected]>
@Yannic Yannic deleted the update_protobuf branch March 11, 2020 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Rules-Server Issues for serverside rules included with Bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants