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

Fix Gradle builds with -Werror #670

Merged
merged 5 commits into from
Dec 5, 2023
Merged

Fix Gradle builds with -Werror #670

merged 5 commits into from
Dec 5, 2023

Conversation

keynmol
Copy link
Contributor

@keynmol keynmol commented Dec 5, 2023

Problem

It turns out that under -Werror, the warning that javac emits when encountering our hacky -Arandomtimestamp=<bla> parameter causes the indexing to fail.

For an example of that, consider https://github.com/palantir/refreshable:

  • Run cs launch scip-java:0.9.7 --contrib --scala 2.13.12 -- index where cs is Coursier
  • Bunch of output, but the main bit is:
> Task :refreshable:compileTestJava FAILED
warning: The following options were not recognized by any processor: '[randomtimestamp]'
error: warnings found and -Werror specified
1 error
1 warning

Why do we have this -Arandomtimestamp at all?

It's there to ensure that Gradle doesn't cache the compilation results at all, as it can lead to a very frustrating experience of not seeing semanticdb files produced despite making changes to the build.

Note - gradle's clean command doesn't clean the particular cache affected by this, neither does --no-daemon option - we already have both of those in auto-indexing and caches still persist between runs without modifying javac options

Alternatives rejected

  • Attempting to remove the -Werror flag from the compilation arguments

    Rejected because historically any modifications to gradle javac flags have been brittle and required hacks. Additionally, there may be builds that assert that the code is compiled with certain flags in CI, and we don't want to make any further modifications to the build
    unless it's necessary

  • Making CLI argument parsing in scip-java-semanticdb more lenient to accept anything

    Rejected because we want to be strict in what arguments are being passed into the plugin, given how already difficult it is to debug compiler plugin pipeline and how much damage one can do by misspelling a parameter (i.e. putting files silently in wrong path)

  • Removing this entirely and instead realying on a magical env variable

    Weakly rejected because magical env variables will require documentation for development - as opposed to a magical "always on" parameter that affects nothing but the plugin and doesn't exist anywhere but during the single gradle run for auto-indexing. The latter seems better.

Test plan

  • new minimal reproduction test was added, tested on Gradle 6/7/8

The warning in question is only triggered if there are annotation
processors in the build
@keynmol keynmol marked this pull request as ready for review December 5, 2023 11:55
@keynmol keynmol merged commit 7749657 into main Dec 5, 2023
15 checks passed
@keynmol keynmol deleted the gradle-w-error branch December 5, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants