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

Add documentation about temporary compile outputs #23251

Merged
merged 8 commits into from
Dec 23, 2022

Conversation

asodja
Copy link
Member

@asodja asodja commented Dec 21, 2022

Documentation for #23066.

We might add improvements that remove the behaviour in 8.1.

@asodja asodja requested review from a team and hythloda as code owners December 21, 2022 13:43
@asodja asodja requested a review from jvandort December 21, 2022 13:43
@bot-gradle bot-gradle added this to the 8.1 RC1 milestone Dec 21, 2022
@asodja asodja self-assigned this Dec 21, 2022
@asodja asodja changed the base branch from master to release December 21, 2022 13:43
@asodja asodja added @execution a:documentation Documentation content labels Dec 21, 2022
@asodja asodja modified the milestones: 8.1 RC1, 8.0 RC1 Dec 21, 2022
@@ -515,6 +515,16 @@ To help you understand how incremental compilation works, the following provides
* Having a source structure that does not match the package names, while legal for compilation, might end up causing trouble in the toolchain.
Even more if annotation processing and <<build_cache.adoc#build_cache,caching>> are involved.

Additionally, Gradle might temporary change the output location while running incremental compilation. This might affect some annotation processors that inspect output locations and which allow users to wire some action to file paths (e.g. `-XepExcludedPaths` in Error Prone). You can disable this Gradle behaviour by setting link:{javadocPath}/org/gradle/api/tasks/compile/CompileOptions.html#getIncrementalAfterFailure--[`options.incrementalAfterFailure`] to `false` on the JavaCompile task. Alternatively you can try specify temporary output path to annotation processor. Output location mapping to temporary output locations is listed in the next table:
Copy link
Member

Choose a reason for hiding this comment

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

❌ Can you do one line per sentence?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be fixed now

subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
@@ -225,6 +225,7 @@ <h3 id="authoring-gradle-builds-java">Authoring JVM Builds</h3>
<li><a href="../userguide/dependency_management_for_java_projects.html">Managing Dependencies</a></li>
<li><a class="nav-dropdown" data-toggle="collapse" href="#jvm-plugins" aria-expanded="false" aria-controls="jvm-plugins">JVM Plugins</a>
<ul id="jvm-plugins">
<li><a href="../userguide/java_plugin.html">Java Plugin</a></li>
Copy link
Member

@wolfs wolfs Dec 21, 2022

Choose a reason for hiding this comment

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

🤔 I wouldn't include this change in the PR. I wonder if there is a reason not to include the Java plugin in this section. For example, you shouldn't apply the java plugin any more, you should only apply the more specialized plugins (I think).

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

asodja and others added 3 commits December 21, 2022 15:32
…_version_7.adoc

Co-authored-by: Stefan Wolf <wolf@gradle.com>
Co-authored-by: Stefan Wolf <wolf@gradle.com>
@gradle gradle deleted a comment from asodja Dec 21, 2022
Copy link
Member

@wolfs wolfs left a comment

Choose a reason for hiding this comment

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

LGTM!

@gradle gradle deleted a comment from asodja Dec 21, 2022
Copy link
Member

@jvandort jvandort left a comment

Choose a reason for hiding this comment

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

Some suggestions and a question

subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
Additionally, Gradle might temporary change the output location while running incremental compilation.
This might affect some annotation processors that inspect output locations and which allow users to wire some action to file paths (e.g. `-XepExcludedPaths` in Error Prone).
You can disable this Gradle behaviour by setting link:{javadocPath}/org/gradle/api/tasks/compile/CompileOptions.html#getIncrementalAfterFailure--[`options.incrementalAfterFailure`] to `false` on the JavaCompile task.
Alternatively, you can try to specify a temporary output path to the annotation processor.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand this sentence properly. Is there some mechanism for users to configure what the temporary output path is? Or is this a more general comment for annotation processors which themselves can be configured with custom file paths?

Copy link
Member Author

@asodja asodja Dec 22, 2022

Choose a reason for hiding this comment

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

Thanks for the review, I believe this might not be clear to a lot of users.

Users can't change that temporary output, but they can make annotation aware of it.
So basically what I want to say here is:
The user has two options:
1.) They can just disable incremental after failure, done.
2.) Or if they want to keep incremental after failure working they can try to pass a temporary output value to the annotation processor as a parameter, so the annotation processor is aware of that path. In case of Error Prone they can pass a value to -XepExcludedPaths. Example:
Before they had: -XepExcludedPaths=.*/build/generated/.*". With the new behavior they might need to do
-XepExcludedPaths=.*/(build/generated|compileJava/compileTransaction/annotation-output)/.*".

Do you have any suggestions on how I can rephrase the paragraph to be more clear?

Somehow related note: I hope we remove these temporary paths in 8.1 with #23226.🤞

Copy link
Contributor

@hythloda hythloda Dec 22, 2022

Choose a reason for hiding this comment

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

let me whack it.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps:

This might affect some annotation processors that inspect output locations or
accept file paths as arguments. For example, Error Prone's `-XepExcludedPaths` 
parameter can be updated to `.*/(build/generated|compileJava/compileTransaction/annotation-output)/.*` 
in order to make the processor aware of the new temporary directory. 

Co-authored-by: Justin Van Dort <jvandort@gradle.com>
@gradle gradle deleted a comment from asodja Dec 22, 2022
@gradle gradle deleted a comment from asodja Dec 22, 2022
…er outputs

Co-authored-by: Amanda L Martin <hythloda@gmail.com>
@asodja asodja force-pushed the asodja/document-compile-outs branch from 9ca3b84 to 26d0048 Compare December 22, 2022 17:18
@asodja
Copy link
Member Author

asodja commented Dec 22, 2022

@jvandort @hythloda is this now good for a merge?

Copy link
Member

@jvandort jvandort left a comment

Choose a reason for hiding this comment

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

Added some suggestions to avoid you and they.

subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc Outdated Show resolved Hide resolved
Co-authored-by: Justin Van Dort <jvandort@gradle.com>
@asodja
Copy link
Member Author

asodja commented Dec 23, 2022

@bot-gradle test BD

@gradle gradle deleted a comment from asodja Dec 23, 2022
@bot-gradle
Copy link
Collaborator

I've triggered the following builds for you:

@asodja asodja requested a review from jvandort December 23, 2022 14:36
@hythloda hythloda self-requested a review December 23, 2022 14:41
@wolfs
Copy link
Member

wolfs commented Dec 23, 2022

@bot-gradle test and merge

@gradle gradle deleted a comment from asodja Dec 23, 2022
@bot-gradle
Copy link
Collaborator

Your PR is queued. See the queue page for details.

@bot-gradle
Copy link
Collaborator

I've triggered a build for you.

@bot-gradle bot-gradle merged commit 4dfa1b0 into release Dec 23, 2022
@bot-gradle bot-gradle deleted the asodja/document-compile-outs branch December 23, 2022 16:26
bot-gradle added a commit that referenced this pull request Jan 26, 2023
This removes text added in #23251, since we changed the behaviour of Java and Groovy compilation in #23226.

Co-authored-by: Anže Sodja <asodja@gradle.com>
asodja added a commit that referenced this pull request Feb 16, 2023
bot-gradle added a commit that referenced this pull request Feb 17, 2023
…modules" to 7.6.1

Backport from #23119 to 7.6.1.

Also backport from #23251 to 7.6.1

Fixes #23224

Co-authored-by: Anže Sodja <asodja@gradle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:documentation Documentation content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Easier debugging of CompileTransaction path selection in incremental task execution
5 participants