Upgrade upgrader python 3 and abseil (from gflags)

[achew22]

No description provided.

[ittaiz]

I’ll take a look first thing tomorrow

On Sun, 28 Jul 2019 at 21:17 Andrew Z Allen ***@***.***> wrote: @achew22 <https://github.com/achew22> requested your review on: #145 <#145> Upgrade upgrader python 3 and abseil (from gflags). — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#145?email_source=notifications&email_token=AAKQQF4QUBCGOPAHN3AGK63QBXPC5A5CNFSM4IHNDQG2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSXSZH6Y#event-2514850811>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKQQF6D5QTL4KXCI27RW4LQBXPC5ANCNFSM4IHNDQGQ> .

-- *Ittai Zeidman* Cell: 054-6735021 40 Hanamal street, Tel Aviv, Israel <http://www.wix.com>

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[ittaiz]

@googlebot I consent

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[ittaiz]

Amazing! Googlebot cla finally works with multiple committers, yay!

[achew22]

Hold up on this, I was about to push a change that makes this better. Please hold

[achew22]

Do you know what's up with the RBE tests? Why are they not passing?

[ittaiz]

For reasons I don't remember I needed to explicitly specify the bazel versions for the RBE test. In the original PR I said it relates to toolchains.
Maybe if we update bazel_toolchains_test to something up-to-date they will just pass.
I think that in the presubmit you can't exclude a path and this is why I needed to explicitly specify the versions in the exclude
https://github.com/achew22/bazel-integration-testing/blob/updater/javatests/build/bazel/tests/integration/BUILD#L62
https://github.com/achew22/bazel-integration-testing/blob/updater/javatests/build/bazel/tests/integration/BUILD#L58

[achew22]

That brings up another question. Do you know why you explicitly are setting the version numbers here? Is it okay if I set that to GET_LATEST_BAZEL_VERSIONS()?

[ittaiz]

For reasons I don't remember I needed to explicitly specify the bazel versions for the RBE test. In the original PR I said it relates to toolchains.
Maybe if we update bazel_toolchains_test to something up-to-date they will just pass.

What I'd suggest is to update the bazel_toolchains_test to something up to date and remove the versions from the BUILD.bazel and see. I think that the toolchains required by RBE didn't work with earlier bazel versions but don't remember exactly

[achew22]

What I'd suggest is to update the bazel_toolchains_test to something up to date and remove the versions from the BUILD.bazel and see. I think that the toolchains required by RBE didn't work with earlier bazel versions but don't remember exactly

Could I ask you to append another commit to this to achieve that. I've never touched bazel_toolchains before and I am not sure midnight is the best time to learn about them.

[ittaiz]

Definitely. I stopped because you asked me to hold on.

[achew22]

@ittaiz Ah, sorry about that. Feel free to muck with this as you deem necessary. I was just typing a git push command when you happened to push and I didn't want a conflict.

[ittaiz]

@philwo any idea how I can see this test log? It's a buildkite job running on RBE. https://buildkite.com/bazel/bazel-integration-testing/builds/1121#b5a6f2aa-f045-489a-b436-4fa411db263a/259-337

[achew22]

@ittaiz you can click on "artifacts" at the top of the run. Here is the link to the one you wanted https://buildkite.com/organizations/bazel/pipelines/bazel-integration-testing/builds/1121/jobs/b5a6f2aa-f045-489a-b436-4fa411db263a/artifacts/539d4c11-4daf-4613-97a7-6e4ee5366201

[ittaiz]

now it's failing because of remote_java_tools_linux...
I wonder if I can find a bazel-toolchains target that doesn't require it. Would rather dodge the bullet just now.

[ittaiz]

and previous toolchain doesn't work with latest bazel @bazel_toolchains//configs/ubuntu16_04_clang/1.1/bazel_0.22.0/default:cc-compiler-k8: missing value for mandatory attribute 'toolchain_config' in 'cc_toolchain' rule.
What I was able to do is get the non RBE tests to fail only on non RBE issues (bazel added implicit external deps which we need to define like remote_coverage_tools) and get the RBE test to fail only on RBE related toolchains. As well as remove tech debt of the presubmit file.
I hope to be able to continue with this tonight but this won't be short :(
Towards 1.0 many stuff have changed (and I'm leaving for 12 days vacation on Wednesday...)

[ittaiz]

@achew22 what we can do for now if I'm not able to fix it is support versions from 0.23.2 and have the java test fixed to that version (add versions = ["0.23.2"], to each target).
This is not a good solution but if you're blocked and severely need to make headway this is an option.
In that case we should probably note this mismatch in the readme if someone stumbles in

[achew22]

@ittaiz, yeah I think I'm okay with that as a temporary fix. Do you have an ETA on being able to really fix up the RBE stuff? I'm concerned about leaving it to bitrot and 0.23 is a pretty old release at this point.

[ittaiz]

1. It’s not only the RBE stuff but everything java related is broken. This is because they’re splitting away remote_java_tools and remote_coverage_tools and now I need to simulate that. 2. I don’t have. I’m returning from vacation on the 12th and I guess I’ll make it a high priority. Wdyt?

On Tue, 30 Jul 2019 at 3:19 Andrew Z Allen ***@***.***> wrote: @ittaiz <https://github.com/ittaiz>, yeah I think I'm okay with that as a temporary fix. Do you have an ETA on being able to really fix up the RBE stuff? I'm concerned about leaving it to bitrot and 0.23 is a pretty old release at this point. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#145?email_source=notifications&email_token=AAKQQF3DK5PLIA3IKO3B7KLQB6CI5A5CNFSM4IHNDQG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3CLZTA#issuecomment-516209868>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKQQFZ7VGQJ4W7KNO2DU63QB6CI5ANCNFSM4IHNDQGQ> .

-- *Ittai Zeidman* Cell: 054-6735021 40 Hanamal street, Tel Aviv, Israel <http://www.wix.com>

[achew22]

1. It’s not only the RBE stuff but everything java related is broken. This is because they’re splitting way remote_java_tools and remote_coverage_tools and now I need to simulate that.

That's deeply unfortunate...

2. I don’t have. I’m returning from vacation on the 12th and I guess I’ll make it a high priority.
   Wdyt?

I think we should use this as an opportunity to talk with the Bazel team. I'm willing to spend some time rewriting the Bazel integration tests for the core of Bazel to run on something like this. If we don't bite the bullet and have that conversation we are perpetually going to be broken by upstream changes. We can make their testing story better, and improve the ability of tools systems to test.

WDYT?

[ittaiz]

I'd love that and I even tried pushing for this in the past but @jmmv wasn't interested / started building his own infra inside bazel repo for this.
How do you think we should discuss this with the bazel team? Issue? bazel-dev? VC?
(Keep in mind I'll be offline during the next two weeks)