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 configuration cache issues #54

Closed
C-Otto opened this issue Nov 26, 2020 · 22 comments
Closed

Fix configuration cache issues #54

C-Otto opened this issue Nov 26, 2020 · 22 comments

Comments

@C-Otto
Copy link
Contributor

C-Otto commented Nov 26, 2020

The new incubating feature "configuration cache" seems to be incompatible with the plugin:

$ ./gradlew --configuration-cache help
Configuration cache is an incubating feature.
Calculating task graph as no configuration cache is available for tasks: help

[...]
FAILURE: Build failed with an exception.

* What went wrong:
Configuration cache problems found in this build.

2 problems were found storing the configuration cache, 1 of which seems unique.
- Plugin 'de.aaschmid.cpd': read system property 'file.encoding'
  See https://docs.gradle.org/6.8-rc-1/userguide/configuration_cache.html#config_cache:requirements:undeclared_sys_prop_read

See the complete report at file:///home/cotto/IdeaProjects/playground/build/reports/configuration-cache/bry60ktvel38k530sjei30vdb/configuration-cache-report.html
> Read system property 'file.encoding'
> Read system property 'file.encoding'

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 1s
1 actionable task: 1 executed
Configuration cache entry discarded with 2 problems.

https://docs.gradle.org/6.8-rc-1/userguide/configuration_cache.html

Stacktrace snippet:

org.gradle.api.InvalidUserCodeException: Read system property 'file.encoding'
	at org.gradle.configurationcache.SystemPropertyAccessListener.systemPropertyQueried(SystemPropertyAccessListener.kt:92)
	at org.gradle.internal.classpath.Instrumented.systemProperty(Instrumented.java:83)
	at org.gradle.internal.classpath.Instrumented.systemProperty(Instrumented.java:77)
	at de.aaschmid.gradle.plugins.cpd.CpdExtension.<init>(CpdExtension.java:26)
	at de.aaschmid.gradle.plugins.cpd.CpdExtension_Decorated.<init>(Unknown Source)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
@aaschmid
Copy link
Owner

Thanks for this incompatibility report @C-Otto. I think the solution is making the encoding property a Property. Using 3.2-SNAPSHOT on https://oss.sonatype.org/content/repositories/snapshots/ works for my other builds. You can use id("de.aaschmid.cpd") version "3.2-SNAPSHOT" by enhancing the settings.xml with

pluginManagement {
    repositories {
        maven { 
            url "https://oss.sonatype.org/content/repositories/snapshots/"
        }
        gradlePluginPortal()
    }
}

Would be helpful if you could try it :-)

@C-Otto
Copy link
Contributor Author

C-Otto commented Nov 26, 2020

That works with the help command above, but fails with "clean build":

> Task 'cpdCheck' requires 'minimumTokenCount' to be greater than zero.

Feel free to give it a try in my playground: https://github.com/C-Otto/playground/

I had to edit buildSrc/build.gradle (adding the snapshots repository).

@aaschmid
Copy link
Owner

Do you also have another change unknown to me as "work on my machine"(TM)...
(But I at least saw a deprecation warning which I will also fix)

BTW as I accidentally saw "architecture-tests": Do you know https://www.archunit.org/ (open sourcen and build by the company I work for. Also on https://www.thoughtworks.com/radar/tools/archunit ;-))

aaschmid added a commit that referenced this issue Nov 26, 2020
Signed-off-by: Andreas Schmid <service@aaschmid.de>
@C-Otto
Copy link
Contributor Author

C-Otto commented Nov 27, 2020

I open issues as soon as I see the need :)

I know about ArchUnit, have used it in projects - it's a great tool! It's on my TODO list, I want to use it in the playground project.

@aaschmid
Copy link
Owner

Is your > Task 'cpdCheck' requires 'minimumTokenCount' to be greater than zero. error resolved / explainable? I still cannot reproduce it...

@C-Otto
Copy link
Contributor Author

C-Otto commented Nov 27, 2020

Yes, I still see that issue. Reproducible with ./gradlew --configuration-cache --no-build-cache clean build and ./gradlew --configuration-cache --no-build-cache cpdCheck. Although the error message indicates :subproject-one:cpdCheck, I am able to run ./gradlew --configuration-cache --no-build-cache subproject-one:cpdCheck.

@aaschmid
Copy link
Owner

On main branch, I don't have the issue with neither of your commands above :-(

I have the following changes:

diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle
index 440fa03..ee189ea 100644
--- a/buildSrc/build.gradle
+++ b/buildSrc/build.gradle
@@ -3,11 +3,15 @@ plugins {
 }
 
 repositories {
+    maven {
+        url "https://oss.sonatype.org/content/repositories/snapshots/"
+    }
     gradlePluginPortal()
 }
 
 dependencies {
-    implementation 'de.aaschmid:gradle-cpd-plugin:3.1'
+    implementation 'de.aaschmid:gradle-cpd-plugin:3.2-SNAPSHOT'
     implementation 'gradle.plugin.com.github.spotbugs.snom:spotbugs-gradle-plugin:4.6.0'
     implementation 'net.ltgt.gradle:gradle-errorprone-plugin:1.3.0'
     implementation 'net.ltgt.gradle:gradle-nullaway-plugin:1.0.2'

@C-Otto
Copy link
Contributor Author

C-Otto commented Nov 28, 2020

I had to run the ./gradlew --configuration-cache --no-build-cache cpdCheck twice to see the error. The diff is identical to yours.

aaschmid added a commit that referenced this issue Mar 27, 2021
* #54.configuration.cache:
  make encoding a property (#54)
@C-Otto
Copy link
Contributor Author

C-Otto commented Mar 27, 2021

As reported on November 28, building the second time triggers a failure. Reproduced with Gradle 7.0 RC1, Java 16, CPD 3.2-SNAPSHOT.

@C-Otto
Copy link
Contributor Author

C-Otto commented Mar 27, 2021

To reproduce:
git clone https://github.com/C-Otto/playground && cd playground && git checkout gradle-cpd-plugin-54 && ./gradlew --configuration-cache --no-build-cache clean build && ./gradlew --configuration-cache --no-build-cache clean build

Reusing configuration cache.
> Task :application:cpdCheck FAILED
> Task :subproject-two:cpdCheck FAILED
> Task :subproject-one:compileJava FAILED

FAILURE: Build completed with 3 failures.

1: Task failed with an exception.
-----------
* What went wrong:
A problem was found with the configuration of task ':application:cpdCheck' (type 'Cpd').
  - Type 'Cpd' property 'encoding' doesn't have a configured value.
    
    Reason: This property isn't marked as optional and no value has been configured.
    
    Possible solutions:
      1. Assign a value to 'encoding'.
      2. Mark property 'encoding' as optional.
    
    Please refer to https://docs.gradle.org/7.0-rc-1/userguide/validation_problems.html#value_not_set for more details about this problem.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
==============================================================================
[...]

@aaschmid
Copy link
Owner

Yes, I have the same issue ;-)
And currently I am thinking about why the properties value is not stored in the configuration cache ...

@aaschmid
Copy link
Owner

Unfortunately, I don't see any easy fixes for this, rather I need to rewrite a good part of the properties / configuration stuff of the plugin :-(

aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
@aaschmid
Copy link
Owner

Another yet strange thing is that my test application works with bfe0311, however your project does now complain about a minimumTokenCount again. Most likely I need to change all the properties to Property :-(

aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 27, 2021
* to be able to use `--configuration-cache` in tests
* because APIs changed again, see `forUseAtConfigurationTime()`

Signed-off-by: Andreas Schmid <service@aaschmid.de>
@aaschmid
Copy link
Owner

At least I found the problem with minimumTokenCount, see gradle/gradle#16657

@aaschmid
Copy link
Owner

aaschmid commented Mar 27, 2021

Remaining problem is as far as I can see not resolvable by me while I am doing the same as Gradle internally does with the pmd or checkstyle plugin :-(
grafik

@aaschmid
Copy link
Owner

@C-Otto with the newest 3.2-SNAPSHOT and ./gradlew --configuration-cache --no-build-cache --configuration-cache-problems=warn clean cpdCheck it should work on the second run (at least it does on my machine once I ensured the 3.2-SNAPSHOT is re-downloaded as described in https://stackoverflow.com/a/62085606.

However the warning from above is skipped and I still have no clue where it comes from :-(

@C-Otto
Copy link
Contributor Author

C-Otto commented Mar 27, 2021

Thank you! For Google's sake, here's the warning as text:

cannot deserialize object of type 'org.gradle.api.Project' as these are not supported with the configuration cache.
  See https://docs.gradle.org/7.0-rc-1/userguide/configuration_cache.html#config_cache:requirements:disallowed_types

From that page:

Gradle model types (e.g. Gradle, Settings, Project, SourceSet, Configuration etc…​) are usually used to carry some task input that should be explicitly and precisely declared instead.

For example, if you reference a Project in order to get the project.version at execution time, you should instead directly declare the project version as an input to your task using a Property. Another example would be to reference a SourceSet to later get the source files, the compilation classpath or the outputs of the source set. You should instead declare these as a FileCollection input and reference just that.

I don't know how that relates to the plugin, though.

@aaschmid
Copy link
Owner

aaschmid commented Mar 27, 2021

Unfortunately, I never directly relate to Project, however indirectly through the stack in the screenshot.

As far as I could analyse it, I inherit from org.gradle.api.reporting.internal.TaskReportContainer which inherited from ´org.gradle.api.reporting.internal.DefaultReportContainerwhich has a fieldenabledof typeNamedDomainObjectSetwhich is instantiated viaDefaultNamedDomainObjectSet`.

My reports are built upon the API described in https://docs.gradle.org/current/javadoc/org/gradle/api/reporting/Reporting.html.

@C-Otto
Copy link
Contributor Author

C-Otto commented Mar 27, 2021

Maybe this helps? https://github.com/gradle/gradle/pull/14628/files#diff-239c622f1af32f1bee008aa813579ded9fffeb072f25557cf8eb27801268c770
(got it as a suggestion in the Gradle slack)

@aaschmid
Copy link
Owner

aaschmid commented Mar 28, 2021

Thank you @C-Otto. To be honest I was professionally blinkered in a way that project was contained in a lambda configuring report.getOutputLocation() in the CpdPlugin which I indeed copied from Gradles PmdPlugin however not from the latest version (facepalm).

The PR from above let me rethink it and now I could copy and adjust from the correct place - first tests look promising :-)

aaschmid added a commit that referenced this issue Mar 28, 2021
* as it would break configuration-cache
* Thanks to @C-Otto for the decisive link
  (#54 (comment))

Signed-off-by: Andreas Schmid <service@aaschmid.de>
aaschmid added a commit that referenced this issue Mar 28, 2021
* and by the way update spock to used groovy version

Signed-off-by: Andreas Schmid <service@aaschmid.de>
@aaschmid aaschmid added this to the next version milestone Mar 28, 2021
@aaschmid aaschmid self-assigned this Mar 28, 2021
@aaschmid
Copy link
Owner

Merged to master, preparing for next release...

aaschmid added a commit that referenced this issue Mar 28, 2021
* #54.configuration.cache:
  check-in acceptance test for configuration cache (#54)
  do not use Project within configuration lambda (#54)
  align report methods with internal quality tasks like PMD
  upgrade to Gradle 6.6 (#54)
  make encoding property compatible with configuration cache (#54)
  use primitive wrapper as otherwise it is not configuration cached (#54)
  fix configuration cache problem with reports (#54)
  upgrade to Gradle 6.1
@aaschmid
Copy link
Owner

aaschmid commented Mar 28, 2021

Configuration cache working with v3.2 of gradle-cpd-plugin, FYI gradle/gradle#13490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants