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

Upgrade Error Prone 2.10.0 -> 2.14.0 and errorprone-slf4j 0.1.4 -> 0.1.12 #76

Merged
merged 6 commits into from
Jul 31, 2022

Conversation

Picnic-Bot
Copy link
Contributor

@Picnic-Bot Picnic-Bot commented Apr 25, 2022

This PR contains the following updates:

Package Update Change
com.google.errorprone:error_prone_annotations (source) minor 2.10.0 -> 2.13.1

Release Notes

google/error-prone

v2.13.1

Compare Source

What's Changed

Full Changelog: google/error-prone@v2.13.0...v2.13.1

v2.13.0

Compare Source

  • Handle all annotations with the simple name Generated in -XepDisableWarningsInGeneratedCode (#​3094)
  • Reconcile BugChecker#isSuppressed with suppression handling in ErrorProneScanner (#​3094)
  • Fix a bug in enclosingPackage (8fa64d4)
  • Improve performance of fix application (186334b)
  • Implicitly treat @AutoBuilder setter methods as @CanIgnoreReturnValue.
  • Remove some obsolete checks (PublicConstructorForAbstractClass, HashCodeToString)

Release Diff: v2.12.1...v2.13.0.

v2.12.1

Compare Source

v2.12.0

Compare Source

New checks

Fixed issues: #​58, #​65, #​1327, #​1654, #​2858, #​2867, #​2916, #​2951, #​2954, #​3006, #​3008

Full Changelog: google/error-prone@v2.11.0...v2.12.0

v2.11.0

Compare Source

Error Prone now requires JDK 11 or newer (https://github.com/google/error-prone/issues/2730).

New checks

Fixed issues: #​2641, #​2705, #​2776, #​2798, #​2799, #​2819, #​2820, #​2831, #​2833, #​2834, #​2835, #​2861, #​2873, #​2889, #​2892, #​2901

Full Changelog: google/error-prone@v2.10.0...v2.11.0


  • If you want to rebase/retry this PR, click this checkbox.

@Picnic-Bot
Copy link
Contributor Author

Picnic-Bot commented Apr 25, 2022

Suggested commit message:

Upgrade Error Prone 2.10.0 -> 2.14.0 and errorprone-slf4j 0.1.4 -> 0.1.12

See 

@Stephan202
Copy link
Member

This PR is blocked on #83.

@Picnic-Bot Picnic-Bot force-pushed the renovate/version.error-prone-orig branch from eb459bd to 27ca6cc Compare April 25, 2022 11:38
@Stephan202 Stephan202 force-pushed the renovate/version.error-prone-orig branch from 27ca6cc to f87f2ac Compare April 25, 2022 16:46
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and added a comment. Due to a compatibility issue this PR also needs to include the upgrade from #53.

Updated suggested commit message:

Upgrade Error Prone 2.10.0 -> 2.14.0 (#76)

This also requires the upgrade of errorprone-slf4j 0.1.4 -> 0.1.12.

See:
- https://github.com/google/error-prone/releases/tag/v2.11.0
- https://github.com/google/error-prone/releases/tag/v2.12.0
- https://github.com/google/error-prone/releases/tag/v2.12.1
- https://github.com/google/error-prone/releases/tag/v2.13.0
- https://github.com/google/error-prone/releases/tag/v2.13.1
- https://github.com/google/error-prone/releases/tag/v2.14.0
- https://github.com/google/error-prone/compare/v2.10.0...v2.14.0
- https://github.com/PicnicSupermarket/error-prone/compare/v2.10.0-picnic-3...v2.14.0-picnic-2
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.5
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.6
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.7
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.8
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.9
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.10
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.11
- https://github.com/KengoTODA/errorprone-slf4j/releases/tag/v0.1.12
- https://github.com/KengoTODA/errorprone-slf4j/compare/v0.1.4...v0.1.12

This upgrade does come with a caveat: due to PicnicSupermarket/error-prone@614b292 the Refaster templates compiled by stock Error Prone and Picnic's fork are no longer compatible.

Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some context.

pom.xml Outdated
<version.error-prone-orig>2.10.0</version.error-prone-orig>
<version.error-prone-slf4j>0.1.4</version.error-prone-slf4j>
<version.error-prone-orig>2.13.1</version.error-prone-orig>
<version.error-prone-slf4j>0.1.11</version.error-prone-slf4j>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +1472 to +1501
<!-- XXX: Enable this check. -->
-Xep:BugPatternNaming:OFF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed #86 for this, as (a) it introduces quite a lot of changes and (b) it requires more thinking.

@Stephan202 Stephan202 force-pushed the renovate/version.error-prone-orig branch from f87f2ac to fb4c29e Compare June 5, 2022 14:24
@Stephan202
Copy link
Member

Rebased, added a commit and updated the suggested commit message.

@Stephan202 Stephan202 changed the title Upgrade com.google.errorprone:error_prone_annotations 2.10.0 -> 2.13.1 Upgrade Error Prone 2.10.0 -> 2.14.0 and errorprone-slf4j 0.1.4 -> 0.1.12 Jun 5, 2022
@Stephan202
Copy link
Member

Okay, so the reason that the most recent commit doesn't work is as follows. error-prone-contrib adds refaster-compiler to its annotation processor classpath, for which Maven will "simply" use the artifact installed in the local Maven repository (it doesn't use artifacts provided by the reactor build). And because -Perror-prone-fork doesn't influence the set of declared dependencies of the pom.xml files installed in the local Maven repository, the annotation processor classpath used when compiling error-prone-contrib will contain vanilla Error Prone. This in turn means that the Refaster templates are always compiled without the changes of google/error-prone#2706, while (when building with -Perror-prone-fork) RefasterCheckTest will subsequently try to deserialize the templates with the changes of that PR. That causes the InvalidClassException.

Haven't thought of a good solution yet.

@Stephan202 Stephan202 force-pushed the renovate/version.error-prone-orig branch from 4b51792 to 4dcec2a Compare June 6, 2022 07:13
@Stephan202
Copy link
Member

Stephan202 commented Jun 6, 2022

I reverted the .travis.yml changes, because that's not a proper solution direction.

The more I think of it, the more I feel we need to come up with a solution in which RefasterRuleCompiler and RefasterCheck transparently support both vanilla Error Prone and Picnic fork-compatible templates. And (if feasible), we may even want to somehow support multiple versions of Error Prone and the fork. Otherwise we may get into a situation in which Error Prone Support must often be migrated in lockstep with Error Prone itself. This sort of thing may hurt adoption once open-sourced. Put another way: CodeTransformer serialization format changes should not have a disproportionate impact on the rest of the ecosystem we're building.

(Still don't have a good idea on how to take things from here. Starting to get an inkling of some solution directions, but all have uncertainties and each requires quite a bit of work to (in)validate. TBD.)

@rickie
Copy link
Member

rickie commented Jun 17, 2022

Rebased and resolved conflicts.

@rickie rickie added this to the 0.1.0 milestone Jun 17, 2022
@rickie rickie force-pushed the renovate/version.error-prone-orig branch from 5887bbf to 5bb5066 Compare July 6, 2022 11:52
@rickie
Copy link
Member

rickie commented Jul 6, 2022

Rebased.

@rickie
Copy link
Member

rickie commented Jul 7, 2022

Updated the suggested commit message.

Not sure how we should do the commit message. In #120 we used a different format (just copied the example over):

Upgrade Error Prone fork v2.10.0-picnic-1 -> v2.10.0-picnic-3 (#120)

See:
- https://github.com/PicnicSupermarket/error-prone/releases/tag/v2.10.0-picnic-2
- https://github.com/PicnicSupermarket/error-prone/releases/tag/v2.10.0-picnic-3
- https://github.com/PicnicSupermarket/error-prone/compare/v2.10.0-picnic-1...v2.10.0-picnic-3

I think it is more accurate as to what we are actually updating. However, the current format is "closer" to what we the important part of the update is. WDYT?

@Stephan202 Stephan202 force-pushed the renovate/version.error-prone-orig branch from 568bde6 to 9704cb2 Compare July 31, 2022 12:34
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and tweaked the original suggested commit message. The most important update here is the non-fork upgrade, so would focus on that.

I'll merge once built, so that we can also prepare some other things for the upcoming release.

@Stephan202 Stephan202 merged commit 65c4694 into master Jul 31, 2022
@Stephan202 Stephan202 deleted the renovate/version.error-prone-orig branch July 31, 2022 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants