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

Release 0.21 - December 2018 #6493

Closed
laurentlb opened this issue Oct 24, 2018 · 39 comments
Closed

Release 0.21 - December 2018 #6493

laurentlb opened this issue Oct 24, 2018 · 39 comments
Assignees
Labels

Comments

@laurentlb
Copy link
Contributor

Target RC date: December 3rd

@laurentlb laurentlb self-assigned this Oct 24, 2018
@laurentlb laurentlb changed the title Release 0.20 - December 2018 Release 0.21 - December 2018 Oct 24, 2018
@laurentlb
Copy link
Contributor Author

laurentlb commented Dec 3, 2018

Candidate created with:
$ scripts/release/release.sh create --force_rc=2 0.21.0 cb9b2afbba3f8d3a1db8bf68e65d06f1b36902f5
https://releases.bazel.build/0.21.0/rc2/index.html

However some tests are failing: #6832

@davido
Copy link
Contributor

davido commented Dec 3, 2018

Gerrit Code Review cannot be built with this RC, because of outdated protobuf version in rules_closure. Is this expected, to flip the default for any option, until some core bazel rules still depend on those options?

This is the issue in Gerrit issue tracker.
This is the PR for the rules_closure.
This is the fork of rules_closure with this PR included.
This is the CL, that switched to the forked rules_clsoure for Gerrit project.

@jin
Copy link
Member

jin commented Dec 3, 2018

@davido I think everyone's waiting for 3.6.1.2 to be released protocolbuffers/protobuf#4650 (comment)

@davido
Copy link
Contributor

davido commented Dec 4, 2018

@jin Thanks for the pointer, but why Bazel is flipping the bit before 3.6.1.2 was released, and before the update to consume protobuf from HEAD is merged? (Not to mention, that even when protobuf 3.6.1.2 is released, reviews for PRs for rules_closure project take ages.)

IOW: until core Bazel rules still depend on deprecated options, Bazel must not flip the bit.

"Nothing causes more pain, frustration, and disappointment than unfulfilled expectations."

I upgrade to Bazel: 0.21.0rc2, and I cannot build my project any more, and even worse: I cannot fix it either.

@jin
Copy link
Member

jin commented Dec 4, 2018

@davido I agree, and this has caused me a fair bit of trouble with other projects, which was only really alleviated by passing the incompatible flag.

@meteorcloudy I remember reading somewhere that there’ll be a CI pipeline that tests with all_incompatible_changes. Is that the case, and will that catch a breakage like this? It feels like a really bad user experience to flip a default when upstream dependencies haven’t even remotely caught up yet.

cc @dslomov

@davido
Copy link
Contributor

davido commented Dec 4, 2018

It feels like a really bad user experience to flip a default when upstream dependencies haven’t even remotely caught up yet.

Right, so can we avoid destroying other people work? I reverted 86013a0 here: [1]. Can this CL be merged and cherry-picked on 0.21 branch?

@laurentlb
Copy link
Contributor Author

Hi,
We test a number of downstream projects on the CI. Protobuf was updated 6 months ago, other rules were updated more recently. The only exception is rules_closure, but there are pending PRs (it's one-click from being fixed). I think Gerrit just need to update its dependencies too.

I agree that the slow releases of Protobuf are annoying, which is why we want to separate the repository (protocolbuffers/protobuf#5402).

What else shall we update?

@meteorcloudy
Copy link
Member

We now have a new pipeline for testing Bazel@Release + incompatible flags + downstream projects@last_green_commit
See this document for more details

It will catch those failures earlier and help the downstream projects to migrate.

@jin
Copy link
Member

jin commented Dec 4, 2018 via email

@lberki
Copy link
Contributor

lberki commented Dec 5, 2018

Can we cherry-pick 7fc967c and 798b9a9 ? It solves Android Studio performance issues that arose while migrating to 0.18 (!)

@lberki
Copy link
Contributor

lberki commented Dec 5, 2018

/cc @meteorcloudy @dslomov

@davido
Copy link
Contributor

davido commented Dec 5, 2018

@laurentlb

I agree that the slow releases of Protobuf are annoying, which is why we want to separate the repository (protocolbuffers/protobuf#5402).

Protobuf v3.6.1.2 was released today. I've upgraded my PR to bump protobuf to this version in rules_closure.

The only exception is rules_closure, but there are pending PRs (it's one-click from being fixed).

Can someone push merge button then, or what are we missing?

I think Gerrit just need to update its dependencies too.

How can Gerrit update rules_closure, if there is no commit upstream that updated protobuf dependency, and Gerrit maintainers prefer not to fork third party dependencies? In fact my CL that upgraded rules_closure to forked rules_closure version wasn't even reviewed.

@jin

It’ll not be cherrypicked because there’s a workaround by flipping the incompatible_* flags, which is the mechanism to deal with unintended breaking changes.

I disagree. Gerrit Code Review is used by hundreds of core and plugins developers in the wild and we should try very hard to avoid the situation where Gerrit master (and even tip of stable branches) cannot be built with most recent Bazel releases, without passing some obscure --incompatible_foo_bar_baz=false flags.

@davido
Copy link
Contributor

davido commented Dec 6, 2018

So, there is a huge complication with upgrade of protobuf version in rules_closure, unfortunately. I filed this issue.

As pointed out by @acozzette:

We don't have a set date for the 3.7 release, but I'm guessing it should be ready in a month or so.

Why can't we re-consider to revert 86013a0, just merge this CL, and re-visit this deprecation after protobuf 3.7 s released, and Bazel itself, rules_clsoure and Gerrit itself are upgraded to use this full protobuf release?

@laurentlb
Copy link
Contributor Author

@lberki

It solves Android Studio performance issues that arose while migrating to 0.18 (!)

If the same issue is in 0.18, 0.19, and 0.20, it shouldn't be considered as critical for 0.21.

bazel-io pushed a commit that referenced this issue Dec 6, 2018
To support building bazel offline just out of the distribution archive,
all dependencies that would normally be downloaded during the build
are bundled in the distribution archive. So when updating skylib, the
@addition_distfiles repository needs to be updated as well. Do so now.

To be cherry-picked for #6493, i.e., the bazel 0.21.0 release.

Change-Id: I6bf813324298a9b2ca3cb2cac15a871eef9e46f9
PiperOrigin-RevId: 224323158
@meteorcloudy
Copy link
Member

If the same issue is in 0.18, 0.19, and 0.20, it shouldn't be considered as critical for 0.21.

I don't think I can fully agree. Sounds like as long as a regression exists for a time long enough, it's no longer critical. But there are different reasons for a regression to exist so long:

#6806 and #6731 have respectively caused Android Studio and TensorFlow not being able to upgrade Bazel to any version later than 0.18.0. So, I believe they are indeed critical.

Until we can make sure people are actively testing Bazel release candidate (obviously they don't), we should distinguish such regressions.

@jin
Copy link
Member

jin commented Dec 7, 2018

@davido I have written a postmortem on how we can prevent this from happening again, using the timeline of the Android projects breakages. I also mentioned the issues surrounding Gerrit and rules_closure, as we both faced similar breakages.

@davido
Copy link
Contributor

davido commented Dec 7, 2018

@jin Thanks, looks good! I commented on this document.

@laszlocsomor
Copy link
Contributor

Potential release-blocker: #6859

@dslomov
Copy link
Contributor

dslomov commented Dec 7, 2018

Sorry for not replying earlier, I was on sick leave.

Re protobuf issue: looks like the world (including Bazel itself, rules_closure and other things) need to move to protobuf 3.6.1.2, and that is an acceptable solution to everyone, is that correct?

Re #6806: this fix should be cherry-picked under a provision for critical, high impact issues. What happened there is that it took as a while to reproduce and investigate this issue, that is why we only got the fix at the date we got it - the issue was reported quite a while ago.

@davido
Copy link
Contributor

davido commented Dec 7, 2018

Re protobuf issue: looks like the world (including Bazel itself, rules_closure and other things) need to move to protobuf 3.6.1.2, and that is an acceptable solution to everyone, is that correct?

Unfortunately, no. Can you see my comments on @jin's postmortem document linked in his previous comment? To summarize:

  1. 3.6.1.2 is broken, as it missed at least one commit compared to 3.5.1 protobuf release. That broke rules_closure integrations tests for protobuf_js. I uploaded a PR to fix that. After this PR is merged, new tag 3.6.1.3 must be created. ETA is unknown.
  2. 3.6.1.2 is not a complete protobuf release, but just a tag. Particularly protobuf-java wasn't published on Maven Central
  3. Bazel itself still depends on Protobuf 3.6.1
  4. Gerrit Code Review depends on both: rules_closure and on protobuf-java. That why protoc and protobuf-java versions must match, otherwise we are hitting this problem.
  5. Can someone please explain me, why all that mess? Why it is that important to flip the bit: --incompatible_package_name_is_a_function before all that happened:

a) Protobuf 3.7 officially released and all artifacts are published on Maven Central
b) Bazel itself switched to using Protobuf 3.7
c) rules_closure and all other core Bazel rules_xxx projects are adapted to protobuf 3.7 and released

@laurentlb
Copy link
Contributor Author

Cherrypicking 12b9646, 7fc967c, and 798b9a9

@aehlig
Copy link
Contributor

aehlig commented Dec 15, 2018

rc3 still fails to build offline; can we please cherry-pick dbe05df as requested?

Thanks.

laurentlb pushed a commit that referenced this issue Dec 17, 2018
To support building bazel offline just out of the distribution archive,
all dependencies that would normally be downloaded during the build
are bundled in the distribution archive. So when updating skylib, the
@addition_distfiles repository needs to be updated as well. Do so now.

To be cherry-picked for #6493, i.e., the bazel 0.21.0 release.

Change-Id: I6bf813324298a9b2ca3cb2cac15a871eef9e46f9
PiperOrigin-RevId: 224323158
@laurentlb
Copy link
Contributor Author

Done.

scripts/release/release.sh create --force_rc=4 0.21.0 cb9b2af 12b9646 7fc967c 798b9a9 dbe05df

https://releases.bazel.build/0.21.0/rc4/index.html

@laurentlb
Copy link
Contributor Author

@dslomov When can we release it?
I believe it can be released tomorrow, is it okay? (or shall I wait after new year?)

@dslomov
Copy link
Contributor

dslomov commented Dec 18, 2018

I think tomorrow is fine. Could you include links to migration-0.21 and breaking-change-0.21 in release announcement?

@laurentlb
Copy link
Contributor Author

@laurentlb
Copy link
Contributor Author

@vbatts
Copy link

vbatts commented Dec 19, 2018 via email

@jin
Copy link
Member

jin commented Dec 19, 2018

0.21.0 documentation is up: https://docs.bazel.build/versions/0.21.0/bazel-overview.html

@vbatts
Copy link

vbatts commented Dec 19, 2018

@sergiocampama
Copy link
Contributor

when will this hit homebrew?

@laurentlb
Copy link
Contributor Author

It should be updated (bazelbuild/homebrew-tap#35)

@petemounce
Copy link
Contributor

Published to chocolatey.

@ittaiz
Copy link
Member

ittaiz commented Dec 21, 2018

is there a clear guideline on what to do about protobuf? Like @davido said 3.6.1.3 isn't even released.

@davido
Copy link
Contributor

davido commented Dec 23, 2018

Like @davido said 3.6.1.3 isn't even released.

@ittaiz I said this 16 days ago. Meantime protobuf v3.6.1.3 was released. However, it is not a complete protobuf release. Particularly, there is no protobuf-java on Maven Central that tagged with that version. Nevertheless, rules_closure and Gerrit were upgraded to protobuf v3.6.1.3 and it seems to work. So the hope is that protobuf 3.6.1.3 and protobuf-java 3.6.1 are binary compatible.

@ittaiz
Copy link
Member

ittaiz commented Dec 23, 2018 via email

@davido
Copy link
Contributor

davido commented Dec 23, 2018

Where was this released?

Check this PR that updated rules_closure to protobuf 3.6.1.3: [1]:

http_archive(
        name = "com_google_protobuf",
        strip_prefix = "protobuf-3.6.1.3",
        sha256 = "73fdad358857e120fd0fa19e071a96e15c0f23bb25f85d3f7009abfd4f264a2a",
        urls = [
            "https://mirror.bazel.build/github.com/google/protobuf/archive/v3.6.1.3.tar.gz",
            "https://github.com/protocolbuffers/protobuf/archive/v3.6.1.3.tar.gz",
        ],
    )

Important: protobuf-java must still point to 3.6.1, like it's still currently done in gerrit:

maven_jar(
    name = "protobuf",
    artifact = "com.google.protobuf:protobuf-java:3.6.1",
    sha1 = "0d06d46ecfd92ec6d0f3b423b4cd81cb38d8b924",
)

@davido
Copy link
Contributor

davido commented Jan 9, 2019

Bazel 0.21 package for Alpine Linux is here: https://github.com/davido/bazel-alpine-package/releases/tag/0.21.0.

@petemounce
Copy link
Contributor

Belatedly, pushed to chocolatey.

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

No branches or pull requests