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

Moe Sync #95

Merged
merged 4 commits into from
Oct 4, 2019
Merged

Moe Sync #95

merged 4 commits into from
Oct 4, 2019

Conversation

cpovirk
Copy link
Member

@cpovirk cpovirk commented Oct 4, 2019

This code has been reviewed and submitted internally. Feel free to discuss on the PR and we can submit follow-up changes as necessary.

Commits:

Update Travis JDK to JDK11.

This is an attempt to fix the error:

install-jdk.sh 2019-09-17
Expected feature release number in range of 9 to 14, but got: 8
The command "~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"" failed and exited with 3 during .

It is similar to CL 253584474

20ba8fc


Mostly migrate off jsr305.

  • Mostly migrate to Checker Framework declaration annotations.
  • Migrate @GuardedBy to a custom annotation for now. (We would migrate to Error Prone's, but that's causing me problems as I experiment with JPMS.)

Compare to b/69411537 for Guava.

I've left @ParametersAreNonnullByDefault in place for now, but we'd need to remove it to fully eliminate the jsr305 dep.

RELNOTES=Migrated from jsr305 @Nullable to Checker Framework @NullableDecl. In addition to the new dependency on the Checker Framework annotations, we keep the dependency on jsr305 for now so that we can keep using @ParametersAreNonNullByDefault.

ebbb863


Update versions of some plugins and a dependency to prepare for Java 11.

8556f11


Use AutoService as a proper annotation processor.

I was going to say that this also paves the way for including the annotation as a non-optional dependency, should we wish to follow our Guava precedent for annotations:

But I see that it's retention=SOURCE anyway, so there isn't much reason to do that -- except maybe consistency with other annotation packages someday. (Maybe it's still a negative then, as it might still let people rely on our transitive dependency?)

I think the relationship of all this to Java 11 was that I might have to set an Automatic-Module-Name on AutoService, and it makes more sense to set it after we've done the processor-vs.-annotation artifact split. Once I was upgrading, it made sense to set up the annotation processor the Right Away, now that we're using a version in which that works. (Or maybe it always worked but now it's nice that it gets the processor off the classpath?) Or maybe there was some other reason for the change to the annotation-processor setup; once again, I forget. It looks like it might have been that AutoService stops running when I switch how we run Error Prone. Hopefully this was the solution :) But it's probably a good idea in any case.

This CL is basically following the "alternatively" instructions in https://github.com/google/auto/blob/master/value/userguide/index.md#in-pomxml

...even though the AutoService instructions haven't been similarly updated yet: https://github.com/google/auto/tree/master/service#download

9ac60c7

This is an attempt to fix the error:

install-jdk.sh 2019-09-17
Expected feature release number in range of 9 to 14, but got: 8
The command "~/bin/install-jdk.sh --target "/home/travis/oraclejdk8" --workspace "/home/travis/.cache/install-jdk" --feature "8" --license "BCL"" failed and exited with 3 during .

It is similar to CL 253584474

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272528889
- Mostly migrate to Checker Framework declaration annotations.
- Migrate @GuardedBy to a custom annotation for now. (We would migrate to Error Prone's, but that's causing me problems as I experiment with JPMS.)

Compare to b/69411537 for Guava.

I've left @ParametersAreNonnullByDefault in place for now, but we'd need to remove it to fully eliminate the jsr305 dep.

RELNOTES=Migrated from jsr305 `@Nullable` to Checker Framework `@NullableDecl`. In addition to the new dependency on the Checker Framework annotations, we keep the dependency on jsr305 for now so that we can keep using `@ParametersAreNonNullByDefault`.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272713254
- We may want an ICU4J new enough to contain an Automatic-Module-Name.
- Our old maven-surefire-plugin breaks with NullPointerException with JDK10+: https://bugzilla.redhat.com/show_bug.cgi?id=1572708
- Our old maven-compiler-plugin breaks with JDK11: google/error-prone#1136 (comment) (I don't have the error message handy anymore.)

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272714574
I was going to say that this also paves the way for including the annotation as a non-optional dependency, should we wish to follow our Guava precedent for annotations:
- google/guava#2824
- google/guava#2721

But I see that it's retention=SOURCE anyway, so there isn't much reason to do that -- except maybe consistency with other annotation packages someday. (Maybe it's still a negative then, as it might still let people rely on our transitive dependency?)

I think the relationship of all this to Java 11 was that I might have to set an Automatic-Module-Name on AutoService, and it makes more sense to set it after we've done the processor-vs.-annotation artifact split. Once I was upgrading, it made sense to set up the annotation processor the Right Away, now that we're using a version in which that works. (Or maybe it always worked but now it's nice that it gets the processor off the classpath?) Or maybe there was some other reason for the change to the annotation-processor setup; once again, I forget. It looks like it might have been that AutoService stops running when I switch how we run Error Prone. Hopefully this was the solution :) But it's probably a good idea in any case.

This CL is basically following the "alternatively" instructions in https://github.com/google/auto/blob/master/value/userguide/index.md#in-pomxml

...even though the AutoService instructions haven't been similarly updated yet: https://github.com/google/auto/tree/master/service#download

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272720556
@cpovirk
Copy link
Member Author

cpovirk commented Oct 4, 2019

This gets us incrementally closer to a working build. I'm going to submit it, and I'll continue working.

@cpovirk cpovirk merged commit a8489ff into master Oct 4, 2019
@cpovirk cpovirk deleted the sync-master-2019/10/04 branch October 4, 2019 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants