From d32216c95d4765c122c042c6573e60f3a206d450 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 00:37:36 -0800 Subject: [PATCH 01/18] Bump gradle to 6.0.1 --- gradle/wrapper/gradle-wrapper.jar | Bin 55190 -> 55616 bytes gradle/wrapper/gradle-wrapper.properties | 2 +- gradlew | 22 +++++++++++++++++++--- gradlew.bat | 18 +++++++++++++++++- 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index 87b738cbd051603d91cc39de6cb000dd98fe6b02..5c2d1cf016b3885f6930543d57b744ea8c220a1a 100755 GIT binary patch delta 3320 zcmai0c|2768`iN!wwN(!Oxeo5?`tVU3{m#%jC~noTx!q_nHtNnR`zAgWC@krB#b55 znJk4YA);()+(!K-w|npJuix)IpYu7-^SqzuJ>T~|?;j_-ma(;-@!<_I_B>B@4FVej z11CRtM@$8afpkN^v*te{ycR9yTldxXJbmio?@}x{9}zaw&=aQt(a^ZXN9S3i8a+Z% zGc@&(5}jplZjJKk2wNlTp(mbeKL5J9Gjo==yT{-eVKj?*rT1%bQ@%#Xce~~1f{19^ zoD75QEoSzDVh@!9qG4yl`;9=Ysp?rRX=(8$VDRz=R+oA3>jLxjW-H!-2biNSYuy)U z7-B-qC5l;>qjMTg!DbWPY}h7qxi6xp)_T)_O2+*&NDg?v;RyY@5XtWHx%(ImQ_3E% zA%$s3xrxE0Fk>DhG!pG)4}I!pWJl~QtV_3Jl2W4PuWWssMq^UpGatK+4CING9pB#5 z_NDc)aonVrZuXsr5!RcE#?aXFZQjt2VMd)-p00K$EheT?H!m_D2Mdqq;0moaO=C&y zgJnvzgUn!wkx^{r049pU#gsIMhl`%{MDNl;}JRbneC zSTB=5f;o9=2Rt24_lt&%%f~m{Ts)zu8H9j`INrgMp>l-|k%Kj%U`OXL1J2e+CJHJxreHLD_#o*ZeuXE4uGDQAJS_PpEGt7hmd7psmLEBL^h zD#JbHiklZEXkk9(6uF$ErsUu^jg7c~1oRS&CuTq*Xg_cOvGw~FZ&1#p(6|jz9lJnP zSIJ)sX_W2$PSksX&}*_ejz+t*X)xK|JcakaMRGd%c*R)cQcT|?sM^#{fdjh5_I$iK zBX_d;wz+cf>b}r!i3yo6eaua)d`|Mi_|Q3mAz5Qn?#~xgE9In<;TwYN^~mtaYy#WU z*ffWtxwlk&!e@UfqQ$bn23RDFV3o-H_WM}44yQpYw;JuRf$at#XX-qmuVnKqg-Bo# zJjZE39)!{i$qJh?oJzVzWFDlSW;{Wf`Z)33Y$Fh^+qasrsEJsfy9yhyTFe?Lej&3n zEAS(D8WCt(ew(SGD z-J#7@l?KI*ZbS)AVQ23qV&{c=$@zUp0@6=kZp+5by+gnAWdB||7e=!yJ|WTpG0OC7 zKlKWFv6#(>nrEq@d1i-#L9SVxTDNb1DaY%2$=@)`k&3s8wz$M*;THa&!2Isj%6CQS zY>A4HtmWY3@9e@F)mCHJQzBz~Lt(wcJE{!CAr=wxn4|5n(jslTy)~IF?tNK zD^2#hTM0d6MDg>`9;s5*(4W1V8y}F8OT6Xap{`=h1XVKO3zrBh=;JnIs*RB>@7t5T zwV=G^T)L=(9P7tS={6`tEBBBm^u~_!-#m75G*h}y_Jj7|STtiY_LDR5UUHI@awWmB zDn6q9{2M-EHaTm53ln%ENJ$HpLwRcL>7^hUrM=}&`qmWTgtr{Ul*Lqcd_9S0xZ1s>F2dVd(s)3&$`gxFAu6jXYIS ze#M~w@=X@lm)sFI4EEiqKh7JxN=_?+}D=iHCc&S2<^VPZ6 zYKXZgvi(Yne9}k6o=ezgquABVB77}x$nKXh`@LjH&lQPqm_;MTL>4RGO|E#_7AS4@43rz=ij?gcMZalnd-JK4ILhL)Ee(3G zN}g99HmhxoBjHR~y@b>-7{f+`p zIZ<^8%d;wCA#xfwSc6$DNVPjAX6FCkb|MQ|6hFyz9UhoLF0^xUd#*^2Ofn zOJgmwDyb1=Z8T)ArRy|VQOM+BrhZ>W_ELJ6u(d^JTu|j%*6g8JKZ-ewoj)sXJCdS= zHOo?HscL;Z`H18}%WnE1&o42KZ+=fg(*VN>t>kRkcd{mP9NF6;MnzH&m2WsD)sX~h zbhv|Ux$w2avQwoI`IKiGMLrL;Z>R}Y_0K*L=63V z)ut+5tM74Glzb?92kbu5@3M#1Hi7K3$c)?TL$}`aKf0hC3`r!>Xy3!f{ z`}Y#@$`|mG1JlKzVE!vD04aX}x#hV*+AC>bQ|%XJ1<&;=0?uX!RM?CIB=+!tgkB-w zu*HF--^U4#nG1mXz0v^0@|UCs1lt}!1zTaTwoe+k?sPym`pyB-F25ivXx)#1|1%|e zJ7Vpujkk#Lu%U{v6xiQ5LW2`~QXrR`ja@*L=b0ejT977v%C)0WAik0gV7U z6a-7##p#p>>>3a{^Z}e3Z~?A|foBFU12bqaEE*0vqdCCVLFq%{;F%$Dkb6i8;Qo!C z&;zkU(!i5zbSMd)zQzg8(kU^HPQ^flVIzR)<^jwbwget09YD?zV*rx+mx@0IN{#S< zsB|8Ve>>sJI7sHE!@=(((ttqL0ks%C4M^r5!0H?rJ;MV|jtT)1cMl{|9xo_Okp@Ka ze^CzbCPf?IDFWLlE`V1FDDpZ0C@7~VMZt%!6%SFtxz{!Tb1UfBDEg~49x!4|2#_L! zX=6UXeh28_?VY*suC^Sy!?XXp?9-G{ zEbF`ELqycMcTK-$-pw|Jox9S^<_NX$7{PI7aX1p5N>aOyj&D01H#;3?=q^!=_mq@k zUHheWO_|CDYA~8r<-%q8&Gm$uPSx4S`reKPnv?Nif4kS)^smTg&m@kLYT87txGxGxw+Qc zTAi=`vzavOlyLrgf2A~;1~Gx$jcb|fkhfctRt6CjRooL|#wr)(*8D4n;2cBe>p9_T zCeJf!IgCH0h1m)UPLk3hZz120oe5YH$oXjSMHcPv@#wX;OP5bBSJMavm2}5Q8(V&# zXGA!+dAwOiXuQ)|+XwF2HW1@_MPm3*v{M86V_~+xk1K7cI7mxBKU5#bofCjZqqjs$ z(sipv#Ul%KJ)h?ua}a3Dg(6yaxeJ(HD-&`AT9kZJVLJTz?WIfgao$bYwEhXh+&GA= zkpI03HVxtWc*H!~z~9%DC;;Qej=WppOD!i1$MO1`&8LW%IWd2sbnS7j+<0b`v1%qx!owUU+ZIHJFp1yH9BFvUYI^up=ZYX$K_YM|Bn2fCG3sq#(EpRB$|A9~9*^M%Sq)EAjr0&W`hHyz96Z9h*odHK|Ju$JQ0c zO9oayZQv;2b{pLJo`T)C%yS@sAKO*WC%22XDmrdRTd;uFr*sb_{GDl=*Y`l*;>lNWh=XCbn#V}C&jmw3>t zNH(fnG%j@AI$TSggf(e3DxrpHjnpeKExsb|hC`kxjD4HUSmu)&aJNt&DtCWh#51*} zS!qfplP(f0`hJ)VHrXFD_uB7ia4#%U)3S8lGY9^(T1)M8xQxP*3w4&QJr~O`$A&N5 z_taom$34zt+reJDV?oZ*qr5ERUH7#~xm7)D(u#q#m`~~-F+TZ6Q*L)s_#T3GZUuZM zhCH9!{qXnD)9jln$|GDeDPqo=+D6#vQkAjdHtT>{VxU#AQJW-je=UWN5*R>v5vWF6 zK_6z?#thq>&%@fu5epvO$rfx`v9GojdOLGFaQ2V8?Ri z(?L2JBK(;G)bIF7r5T6Ahzst5k4j#hvhl3a`@Ksfyj3^Cx}zGE)vm$ecB$?~2`S&e zE)Nx6TiDO*JO6UmWWc+zLDmnII+)ROEvW3_{*%Fjs8Q^k4+Z&cJ0lp=@p*N!fw0>L zPSWrxar=HPDCwZnmN%orA-K2142{bJ0el>N{KM(xoHJu_HWSQihq^y%SEmj>CsBjl zj6)jxqm7NwiVHh-xQ`ex^02-y_ZO`A`P(1UwLK5G_T8=uI8@e%Kh31Xay z>H$7OG8cQ%>c_RjXhRA|Yh=93MnM)V0JlD#yP-1YNx}5`sg}-vE%slfve&}e$*L>+ zSAq_CMc5SYx6N)5h%-)?JOAhiVM5`TWT7?<9 zKKxMMb9GXHpQ1ajAr?!hxcauobJLf{IpvJ=9ny}FwdGCYmwgj?0qhIG{5zbTTVc2b zo+3h|{F_Yg96k{?rVn`m`%d??#avI-eh^XnTH2r*o>5n>`UuIsuCIeN5Br62W!Yy#8)0uWcVG%-QnMHczpWoe zftoSf-WJq~x8`|ws<-9{Va9@s#SoH3uw`>4!~uyB-(lV)SD9f(TPNa!o7JLL%!a)@gUmedno%~}$ z#zZLYah$5mf@Z2}a(oDDM^$qq>*nb;?aVn?D`($Om=?j+T%S?eSgR1t=zzwGw|kvM zt~WiOO&UVW=7N=8ERxM<4?Wbj4bPIP4z3=hjp(uuT}ne*E9ct0)Lsk?bG=1nNo=oB z0JEoKzAw45q-lB!IbJKsY=Lpru48qY6ql!Z#J13ywC&7??l&AtxiowZ|Cg(k*UE#@ zrJm|m^EV_6jz}f($PrOb`S;imdEwtu`#cCu3aMXBgUUH4t2j_qu=KmOO645(v(_DL z^G5PF%RR0@X5D{(V%x5L{xD1Sa>^wR+$0j(DeVfwk;tp3<@i$~qOsvx^uUy!zV8G0~0`$f?VV=?vm zOwYnZB>UV_b#sh6ibtN`5I+l%mTE9T%*J!xaz}cWisUNLg@>nEiKv4hgmv`5C)GIDbBOgq{?5K-!=>z{CLJ$wIBkL-~yV{}~e*^#eZ1f%)RR;DgcM zfOqnA#42!t$D;@!QT3n50ve1d0$Zl^m}ABc){bz2HDhq#o&{ZLlQ=*lO9Alv7y_uW z`bTL2KkVsP<{%6$`1yeL}DmCZuxPZRJp*( z*Kk1M23@g@UjhQ6PEZ{58CL@Aqv>cB0|#ltT;SR`95{}ptMe0@zz&v<>j{GNDt-bE zn5EFw?u0e)Ee+J0^aq@C>E_j>A%MyU^@?Rcohe{^TCd{d<=ub5$bWAh Date: Tue, 31 Dec 2019 00:37:51 -0800 Subject: [PATCH 02/18] Remove the build-scan plugin because it doesn't work on 6. --- build.gradle | 7 ------- 1 file changed, 7 deletions(-) diff --git a/build.gradle b/build.gradle index 106a6ab335..ae0aa5e42d 100644 --- a/build.gradle +++ b/build.gradle @@ -5,8 +5,6 @@ plugins { id 'com.diffplug.gradle.eclipse.resourcefilters' version '3.18.1' // https://plugins.gradle.org/plugin/com.gradle.plugin-publish id 'com.gradle.plugin-publish' version '0.10.1' apply false - // https://plugins.gradle.org/plugin/com.gradle.build-scan - id 'com.gradle.build-scan' version '2.4' // https://github.com/bintray/gradle-bintray-plugin/releases id 'com.jfrog.bintray' version '1.8.4' apply false // https://github.com/mnlipp/jdrupes-mdoclet/releases @@ -17,11 +15,6 @@ plugins { id "com.github.spotbugs" version "2.0.0" apply false } -buildScan { - termsOfServiceUrl = 'https://gradle.com/terms-of-service' - apply from: 'gradle/build-scans.gradle' -} - // root eclipse project apply plugin: 'com.diffplug.gradle.eclipse.resourcefilters' eclipseResourceFilters { From 957c0f49d80caffa890a2999fb455a96401c2e56 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 00:47:19 -0800 Subject: [PATCH 03/18] Fix compile-deprecation in plugin-maven. --- plugin-maven/build.gradle | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugin-maven/build.gradle b/plugin-maven/build.gradle index c5a3c87759..dd7d58bda0 100644 --- a/plugin-maven/build.gradle +++ b/plugin-maven/build.gradle @@ -55,17 +55,18 @@ def mvnw(String args) { } } +apply plugin: 'java-library' dependencies { if (!project.versionMaven.endsWith('-SNAPSHOT') && project.versionLib.endsWith('-SNAPSHOT')) { // gradle = release, lib = snapshot, therefore gradle should depend on the last stable lib - compile "com.diffplug.spotless:spotless-lib:${project.stableLib}" - compile "com.diffplug.spotless:spotless-lib-extra:${project.stableLib}" + api "com.diffplug.spotless:spotless-lib:${project.stableLib}" + api "com.diffplug.spotless:spotless-lib-extra:${project.stableLib}" } else { - compile project(':lib') - compile project(':lib-extra') + api project(':lib') + api project(':lib-extra') } - compile "org.codehaus.plexus:plexus-resources:${VER_PLEXUS_RESOURCES}" + api "org.codehaus.plexus:plexus-resources:${VER_PLEXUS_RESOURCES}" compileOnly "org.apache.maven:maven-plugin-api:${VER_MAVEN_API}" compileOnly "org.apache.maven.plugin-tools:maven-plugin-annotations:${VER_MAVEN_API}" @@ -135,7 +136,7 @@ libs.each { task createPomXml(dependsOn: installLocalDependencies) { doLast { - def additionalDependencies = project.configurations.compile.resolvedConfiguration.resolvedArtifacts.findAll { + def additionalDependencies = project.configurations.compileClasspath.resolvedConfiguration.resolvedArtifacts.findAll { return !libs.contains(it.moduleVersion.id.name) }.collect { return " \n" + From d9397a1b0e20b9c6cc4e9413a826311d174e2874 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 01:57:36 -0800 Subject: [PATCH 04/18] Add `spotless.resolveDependenciesIn` as a workaround Gradle 6 constraints --- .../gradle/spotless/GradleProvisioner.java | 58 +++++++++++++--- .../gradle/spotless/SpotlessExtension.java | 14 ++++ .../gradle/spotless/SpotlessPlugin.java | 56 ++++++++++++++++ .../gradle/spotless/DefaultRepoConfig.java | 67 +++++++++++++++++++ 4 files changed, 184 insertions(+), 11 deletions(-) create mode 100644 plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java index 57ee601de9..66bd011a24 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java @@ -23,15 +23,30 @@ import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ConfigurationContainer; import org.gradle.api.artifacts.Dependency; +import org.gradle.util.GradleVersion; import com.diffplug.common.base.StringPrinter; +import com.diffplug.common.base.Unhandled; import com.diffplug.spotless.Provisioner; /** Gradle integration for Provisioner. */ public class GradleProvisioner { + /** Determines where Spotless will resolve its dependencies. */ + public enum ResolveDependenciesIn { + /** Spotless dependencies will be resolved from the root buildscript (deprecated for multi-project builds in 6.0). */ + ROOT_BUILDSCRIPT, + /** Spotless dependencies will be resolved from the project buildscript (by default subprojects don't have even a single repository). */ + PROJECT_BUILDSCRIPT, + /** Spotless dependencies will be resolved from the normal repositories of this project. */ + PROJECT + } + + static final GradleVersion STRICT_CONFIG_ACCESS = GradleVersion.version("6.0"); + private GradleProvisioner() {} public static Provisioner fromProject(Project project) { + ResolveDependenciesIn mode = project.getPlugins().getPlugin(SpotlessPlugin.class).getExtension().getResolveDependenciesIn(); Objects.requireNonNull(project); return (withTransitives, mavenCoords) -> { try { @@ -39,27 +54,48 @@ public static Provisioner fromProject(Project project) { .map(project.getBuildscript().getDependencies()::create) .toArray(Dependency[]::new); - // #372 workaround: Accessing rootProject.configurations from multiple projects is not thread-safe ConfigurationContainer configContainer; - synchronized (project.getRootProject()) { - configContainer = project.getRootProject().getBuildscript().getConfigurations(); + if (mode == null || mode == ResolveDependenciesIn.ROOT_BUILDSCRIPT) { + // #372 workaround: Accessing rootProject.configurations from multiple projects is not thread-safe + synchronized (project.getRootProject()) { + configContainer = project.getRootProject().getBuildscript().getConfigurations(); + } + } else if (mode == ResolveDependenciesIn.PROJECT_BUILDSCRIPT) { + configContainer = project.getBuildscript().getConfigurations(); + } else if (mode == ResolveDependenciesIn.PROJECT) { + configContainer = project.getConfigurations(); + } else { + throw Unhandled.enumException(mode); } Configuration config = configContainer.detachedConfiguration(deps); config.setDescription(mavenCoords.toString()); config.setTransitive(withTransitives); return config.resolve(); } catch (Exception e) { - logger.log(Level.SEVERE, - StringPrinter.buildStringFromLines("You probably need to add a repository containing the '" + mavenCoords + "' artifact in the 'build.gradle' of your root project.", - "E.g.: 'buildscript { repositories { mavenCentral() }}'", - "Note that included buildscripts (using 'apply from') do not share their buildscript repositories with the underlying project.", - "You have to specify the missing repository explicitly in the buildscript of the root project."), - e); + String errorMsg; + if (mode == null || mode == ResolveDependenciesIn.ROOT_BUILDSCRIPT) { + errorMsg = StringPrinter.buildStringFromLines( + "You need to add a repository containing the '" + mavenCoords + "' artifact to the `buildscript{}` block of the build.gradle of your root project.", + "E.g.: 'buildscript { repositories { mavenCentral() }}'", + "Note that included buildscripts (using 'apply from') do not share their buildscript repositories with the underlying project.", + "You have to specify the missing repository explicitly in the buildscript of the root project."); + } else if (mode == ResolveDependenciesIn.PROJECT_BUILDSCRIPT) { + errorMsg = StringPrinter.buildStringFromLines( + "You need to add a repository containing the '" + mavenCoords + "' artifact to the `buildscript{}` block of the build.gradle of this project.", + "E.g.: 'buildscript { repositories { mavenCentral() }}'", + "By default, subprojects dont have any repositories in their buildscript whatsoever."); + } else if (mode == ResolveDependenciesIn.PROJECT) { + errorMsg = StringPrinter.buildStringFromLines( + "You need to add a repository containing the '" + mavenCoords + "' artifact to the `repositories{}` block of the build.gradle of this project.", + "E.g.: 'repositories { mavenCentral() }'"); + } else { + throw Unhandled.enumException(mode); + } + logger.log(Level.SEVERE, errorMsg); throw e; } }; } - private static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); - + static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index 7d34a93caa..04e224975a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -23,6 +23,8 @@ import java.util.LinkedHashMap; import java.util.Map; +import javax.annotation.Nullable; + import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; @@ -60,6 +62,18 @@ public SpotlessExtension(Project project) { rootApplyTask.setDescription(APPLY_DESCRIPTION); } + /** Repository mode. */ + GradleProvisioner.ResolveDependenciesIn resolveDependenciesIn = null; + + @Nullable + public GradleProvisioner.ResolveDependenciesIn getResolveDependenciesIn() { + return resolveDependenciesIn; + } + + public void setResolveDependenciesIn(GradleProvisioner.ResolveDependenciesIn repositoryMode) { + this.resolveDependenciesIn = requireNonNull(repositoryMode); + } + /** Line endings (if any). */ LineEnding lineEndings = LineEnding.GIT_ATTRIBUTES; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java index 4d0be9cb53..4c5ca41f71 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java @@ -18,9 +18,13 @@ import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.Task; +import org.gradle.api.artifacts.dsl.RepositoryHandler; +import org.gradle.api.artifacts.repositories.ArtifactRepository; +import org.gradle.api.artifacts.repositories.MavenArtifactRepository; import org.gradle.api.plugins.BasePlugin; import org.gradle.util.GradleVersion; +import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.SpotlessCache; public class SpotlessPlugin implements Plugin { @@ -56,9 +60,61 @@ public void apply(Project project) { SpotlessPluginLegacy.enforceCheck(spotlessExtension, project); } } + + // the user hasn't specified where to resolve deps and they're going to get a deprecation warning + if (spotlessExtension.resolveDependenciesIn == null + && project != project.getRootProject() + && GradleVersion.current().compareTo(GradleProvisioner.STRICT_CONFIG_ACCESS) >= 0) { + boolean safeToForceProjectLocal = isMavenCentralSuperset(project.getRootProject().getBuildscript().getRepositories()) && + isMavenCentralSuperset(project.getRepositories()); + if (safeToForceProjectLocal) { + // if the root buildscript is just `gradlePluginPortal()` (a superset of mavenCental) + // and the project repositories are either empty or some superset of mavenCentral + // then there's no harm in defaulting them to project-local resolution + spotlessExtension.resolveDependenciesIn = GradleProvisioner.ResolveDependenciesIn.PROJECT; + } else { + GradleProvisioner.logger.severe(StringPrinter.buildStringFromLines( + "The way that Spotless resolves formatter dependencies was deprecated in Gradle 6.0.", + "To silence this warning, you need to set X: `spotless { resolveDependenciesIn 'X' }`", + " 'ROOT_BUILDSCRIPT' - current behavior, causes gradle to emit deprecation warnings.", + " 'PROJECT' *recommended* - uses this project's repositories to resolve dependencies.", + " Only drawback is for the uncommon case that you want your", + " build tools to draw from a different set of respositories than ", + " than your build artifacts.", + " 'PROJECT_BUILDSCRIPT' - uses this project's buildscript to resolve dependencies.", + " You will probably need to add this:", + " `buildscript { repositories { mavenCentral() } }`", + " to the top of the `build.gradle` in this subproject.", + "We're hoping to find a better resolution in the future, see issue below for more info", + " https://github.com/diffplug/spotless/issues/502")); + } + } }); } + private boolean isMavenCentralSuperset(RepositoryHandler repositories) { + return repositories.stream().allMatch(SpotlessPlugin::isMavenCentralSuperset); + } + + private static boolean isMavenCentralSuperset(ArtifactRepository repository) { + if (!(repository instanceof MavenArtifactRepository)) { + return false; + } + MavenArtifactRepository maven = (MavenArtifactRepository) repository; + if ("MavenRepo".equals(maven.getName()) + && "https://repo.maven.apache.org/maven2/".equals(maven.getUrl().toString())) { + return true; + } else if ("BintrayJCenter".equals(maven.getName()) + && "https://jcenter.bintray.com/".equals(maven.getUrl().toString())) { + return true; + } else if ("Gradle Central Plugin Repository".equals(maven.getName()) + && "https://plugins.gradle.org/m2".equals(maven.getUrl().toString())) { + return true; + } else { + return false; + } + } + /** The extension for this plugin. */ public SpotlessExtension getExtension() { return spotlessExtension; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java new file mode 100644 index 0000000000..0827d3db7d --- /dev/null +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java @@ -0,0 +1,67 @@ +/* + * Copyright 2016 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.IOException; + +import org.assertj.core.api.Assertions; +import org.gradle.testkit.runner.GradleRunner; +import org.junit.Test; + +import com.diffplug.spotless.ResourceHarness; + +public class DefaultRepoConfig extends ResourceHarness { + @Test + public void integration() throws IOException { + setFile("build.gradle").toLines( + "repositories {", + " mavenCentral()", + " jcenter()", + " gradlePluginPortal()", + "}", + "", + "println('buildscriptSize=' + buildscript.repositories.size())", + "for (repo in buildscript.repositories) {", + " println(repo.toString())", + " println(repo.class)", + "}", + "println('projectSize=' + repositories.size())", + "for (repo in repositories) {", + " println(repo.name)", + " println(repo.url)", + " println(repo.artifactUrls)", + "}"); + + String output = GradleRunner.create() + .withGradleVersion("6.0") + .withProjectDir(rootFolder()) + .build().getOutput(); + Assertions.assertThat(output.replace("\r\n", "\n")).startsWith("\n" + + "> Configure project :\n" + + "buildscriptSize=0\n" + + "projectSize=3\n" + + "MavenRepo\n" + + "https://repo.maven.apache.org/maven2/\n" + + "[]\n" + + "BintrayJCenter\n" + + "https://jcenter.bintray.com/\n" + + "[]\n" + + "Gradle Central Plugin Repository\n" + + "https://plugins.gradle.org/m2\n" + + "[]\n" + + ""); + } +} From 3b412df7a679a5c23377de38cf5fe75f58068d74 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 23:04:57 -0800 Subject: [PATCH 05/18] Revert "Add `spotless.resolveDependenciesIn` as a workaround Gradle 6 constraints" This reverts commit d9397a1b0e20b9c6cc4e9413a826311d174e2874. --- .../gradle/spotless/GradleProvisioner.java | 58 +++------------- .../gradle/spotless/SpotlessExtension.java | 14 ---- .../gradle/spotless/SpotlessPlugin.java | 56 ---------------- .../gradle/spotless/DefaultRepoConfig.java | 67 ------------------- 4 files changed, 11 insertions(+), 184 deletions(-) delete mode 100644 plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java index 66bd011a24..57ee601de9 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java @@ -23,30 +23,15 @@ import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ConfigurationContainer; import org.gradle.api.artifacts.Dependency; -import org.gradle.util.GradleVersion; import com.diffplug.common.base.StringPrinter; -import com.diffplug.common.base.Unhandled; import com.diffplug.spotless.Provisioner; /** Gradle integration for Provisioner. */ public class GradleProvisioner { - /** Determines where Spotless will resolve its dependencies. */ - public enum ResolveDependenciesIn { - /** Spotless dependencies will be resolved from the root buildscript (deprecated for multi-project builds in 6.0). */ - ROOT_BUILDSCRIPT, - /** Spotless dependencies will be resolved from the project buildscript (by default subprojects don't have even a single repository). */ - PROJECT_BUILDSCRIPT, - /** Spotless dependencies will be resolved from the normal repositories of this project. */ - PROJECT - } - - static final GradleVersion STRICT_CONFIG_ACCESS = GradleVersion.version("6.0"); - private GradleProvisioner() {} public static Provisioner fromProject(Project project) { - ResolveDependenciesIn mode = project.getPlugins().getPlugin(SpotlessPlugin.class).getExtension().getResolveDependenciesIn(); Objects.requireNonNull(project); return (withTransitives, mavenCoords) -> { try { @@ -54,48 +39,27 @@ public static Provisioner fromProject(Project project) { .map(project.getBuildscript().getDependencies()::create) .toArray(Dependency[]::new); + // #372 workaround: Accessing rootProject.configurations from multiple projects is not thread-safe ConfigurationContainer configContainer; - if (mode == null || mode == ResolveDependenciesIn.ROOT_BUILDSCRIPT) { - // #372 workaround: Accessing rootProject.configurations from multiple projects is not thread-safe - synchronized (project.getRootProject()) { - configContainer = project.getRootProject().getBuildscript().getConfigurations(); - } - } else if (mode == ResolveDependenciesIn.PROJECT_BUILDSCRIPT) { - configContainer = project.getBuildscript().getConfigurations(); - } else if (mode == ResolveDependenciesIn.PROJECT) { - configContainer = project.getConfigurations(); - } else { - throw Unhandled.enumException(mode); + synchronized (project.getRootProject()) { + configContainer = project.getRootProject().getBuildscript().getConfigurations(); } Configuration config = configContainer.detachedConfiguration(deps); config.setDescription(mavenCoords.toString()); config.setTransitive(withTransitives); return config.resolve(); } catch (Exception e) { - String errorMsg; - if (mode == null || mode == ResolveDependenciesIn.ROOT_BUILDSCRIPT) { - errorMsg = StringPrinter.buildStringFromLines( - "You need to add a repository containing the '" + mavenCoords + "' artifact to the `buildscript{}` block of the build.gradle of your root project.", - "E.g.: 'buildscript { repositories { mavenCentral() }}'", - "Note that included buildscripts (using 'apply from') do not share their buildscript repositories with the underlying project.", - "You have to specify the missing repository explicitly in the buildscript of the root project."); - } else if (mode == ResolveDependenciesIn.PROJECT_BUILDSCRIPT) { - errorMsg = StringPrinter.buildStringFromLines( - "You need to add a repository containing the '" + mavenCoords + "' artifact to the `buildscript{}` block of the build.gradle of this project.", - "E.g.: 'buildscript { repositories { mavenCentral() }}'", - "By default, subprojects dont have any repositories in their buildscript whatsoever."); - } else if (mode == ResolveDependenciesIn.PROJECT) { - errorMsg = StringPrinter.buildStringFromLines( - "You need to add a repository containing the '" + mavenCoords + "' artifact to the `repositories{}` block of the build.gradle of this project.", - "E.g.: 'repositories { mavenCentral() }'"); - } else { - throw Unhandled.enumException(mode); - } - logger.log(Level.SEVERE, errorMsg); + logger.log(Level.SEVERE, + StringPrinter.buildStringFromLines("You probably need to add a repository containing the '" + mavenCoords + "' artifact in the 'build.gradle' of your root project.", + "E.g.: 'buildscript { repositories { mavenCentral() }}'", + "Note that included buildscripts (using 'apply from') do not share their buildscript repositories with the underlying project.", + "You have to specify the missing repository explicitly in the buildscript of the root project."), + e); throw e; } }; } - static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); + private static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); + } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index 04e224975a..7d34a93caa 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -23,8 +23,6 @@ import java.util.LinkedHashMap; import java.util.Map; -import javax.annotation.Nullable; - import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; @@ -62,18 +60,6 @@ public SpotlessExtension(Project project) { rootApplyTask.setDescription(APPLY_DESCRIPTION); } - /** Repository mode. */ - GradleProvisioner.ResolveDependenciesIn resolveDependenciesIn = null; - - @Nullable - public GradleProvisioner.ResolveDependenciesIn getResolveDependenciesIn() { - return resolveDependenciesIn; - } - - public void setResolveDependenciesIn(GradleProvisioner.ResolveDependenciesIn repositoryMode) { - this.resolveDependenciesIn = requireNonNull(repositoryMode); - } - /** Line endings (if any). */ LineEnding lineEndings = LineEnding.GIT_ATTRIBUTES; diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java index 4c5ca41f71..4d0be9cb53 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessPlugin.java @@ -18,13 +18,9 @@ import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.Task; -import org.gradle.api.artifacts.dsl.RepositoryHandler; -import org.gradle.api.artifacts.repositories.ArtifactRepository; -import org.gradle.api.artifacts.repositories.MavenArtifactRepository; import org.gradle.api.plugins.BasePlugin; import org.gradle.util.GradleVersion; -import com.diffplug.common.base.StringPrinter; import com.diffplug.spotless.SpotlessCache; public class SpotlessPlugin implements Plugin { @@ -60,61 +56,9 @@ public void apply(Project project) { SpotlessPluginLegacy.enforceCheck(spotlessExtension, project); } } - - // the user hasn't specified where to resolve deps and they're going to get a deprecation warning - if (spotlessExtension.resolveDependenciesIn == null - && project != project.getRootProject() - && GradleVersion.current().compareTo(GradleProvisioner.STRICT_CONFIG_ACCESS) >= 0) { - boolean safeToForceProjectLocal = isMavenCentralSuperset(project.getRootProject().getBuildscript().getRepositories()) && - isMavenCentralSuperset(project.getRepositories()); - if (safeToForceProjectLocal) { - // if the root buildscript is just `gradlePluginPortal()` (a superset of mavenCental) - // and the project repositories are either empty or some superset of mavenCentral - // then there's no harm in defaulting them to project-local resolution - spotlessExtension.resolveDependenciesIn = GradleProvisioner.ResolveDependenciesIn.PROJECT; - } else { - GradleProvisioner.logger.severe(StringPrinter.buildStringFromLines( - "The way that Spotless resolves formatter dependencies was deprecated in Gradle 6.0.", - "To silence this warning, you need to set X: `spotless { resolveDependenciesIn 'X' }`", - " 'ROOT_BUILDSCRIPT' - current behavior, causes gradle to emit deprecation warnings.", - " 'PROJECT' *recommended* - uses this project's repositories to resolve dependencies.", - " Only drawback is for the uncommon case that you want your", - " build tools to draw from a different set of respositories than ", - " than your build artifacts.", - " 'PROJECT_BUILDSCRIPT' - uses this project's buildscript to resolve dependencies.", - " You will probably need to add this:", - " `buildscript { repositories { mavenCentral() } }`", - " to the top of the `build.gradle` in this subproject.", - "We're hoping to find a better resolution in the future, see issue below for more info", - " https://github.com/diffplug/spotless/issues/502")); - } - } }); } - private boolean isMavenCentralSuperset(RepositoryHandler repositories) { - return repositories.stream().allMatch(SpotlessPlugin::isMavenCentralSuperset); - } - - private static boolean isMavenCentralSuperset(ArtifactRepository repository) { - if (!(repository instanceof MavenArtifactRepository)) { - return false; - } - MavenArtifactRepository maven = (MavenArtifactRepository) repository; - if ("MavenRepo".equals(maven.getName()) - && "https://repo.maven.apache.org/maven2/".equals(maven.getUrl().toString())) { - return true; - } else if ("BintrayJCenter".equals(maven.getName()) - && "https://jcenter.bintray.com/".equals(maven.getUrl().toString())) { - return true; - } else if ("Gradle Central Plugin Repository".equals(maven.getName()) - && "https://plugins.gradle.org/m2".equals(maven.getUrl().toString())) { - return true; - } else { - return false; - } - } - /** The extension for this plugin. */ public SpotlessExtension getExtension() { return spotlessExtension; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java deleted file mode 100644 index 0827d3db7d..0000000000 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/DefaultRepoConfig.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright 2016 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.gradle.spotless; - -import java.io.IOException; - -import org.assertj.core.api.Assertions; -import org.gradle.testkit.runner.GradleRunner; -import org.junit.Test; - -import com.diffplug.spotless.ResourceHarness; - -public class DefaultRepoConfig extends ResourceHarness { - @Test - public void integration() throws IOException { - setFile("build.gradle").toLines( - "repositories {", - " mavenCentral()", - " jcenter()", - " gradlePluginPortal()", - "}", - "", - "println('buildscriptSize=' + buildscript.repositories.size())", - "for (repo in buildscript.repositories) {", - " println(repo.toString())", - " println(repo.class)", - "}", - "println('projectSize=' + repositories.size())", - "for (repo in repositories) {", - " println(repo.name)", - " println(repo.url)", - " println(repo.artifactUrls)", - "}"); - - String output = GradleRunner.create() - .withGradleVersion("6.0") - .withProjectDir(rootFolder()) - .build().getOutput(); - Assertions.assertThat(output.replace("\r\n", "\n")).startsWith("\n" + - "> Configure project :\n" + - "buildscriptSize=0\n" + - "projectSize=3\n" + - "MavenRepo\n" + - "https://repo.maven.apache.org/maven2/\n" + - "[]\n" + - "BintrayJCenter\n" + - "https://jcenter.bintray.com/\n" + - "[]\n" + - "Gradle Central Plugin Repository\n" + - "https://plugins.gradle.org/m2\n" + - "[]\n" + - ""); - } -} From ef65960ad5c09d09db90dc5c3d387f4b04a36e01 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 22:34:43 -0800 Subject: [PATCH 06/18] Improve the error message in JavaExtension if there's no target. --- .../main/java/com/diffplug/gradle/spotless/JavaExtension.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java index e435b46f45..cb9effbfd5 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavaExtension.java @@ -178,7 +178,7 @@ protected void setupTask(SpotlessTask task) { if (target == null) { JavaPluginConvention javaPlugin = getProject().getConvention().findPlugin(JavaPluginConvention.class); if (javaPlugin == null) { - throw new GradleException("You must apply the java plugin before the spotless plugin if you are using the java extension."); + throw new GradleException("You must either specify 'target' manually or apply the 'java' plugin."); } FileCollection union = getProject().files(); for (SourceSet sourceSet : javaPlugin.getSourceSets()) { From 28d8962a63a3c694ca3f2790cc144035dcac37fc Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 22:35:38 -0800 Subject: [PATCH 07/18] Make it possible for code within a lazy configuration to figure out which step is being configured. --- .../main/java/com/diffplug/spotless/FormatterStep.java | 9 +++++++++ .../java/com/diffplug/spotless/FormatterStepImpl.java | 10 +++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index 93e747aa40..1131c20a24 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java @@ -142,4 +142,13 @@ public static FormatterStep createNeverUpToDate( Objects.requireNonNull(function, "function"); return createNeverUpToDateLazy(name, () -> function); } + + /** + * If we are currently inside the `Supplier` of a lazy FormatterStep, + * this method will return the FormatterStep whose state is being calculated. + * Returns null if we are not. + */ + public static @Nullable FormatterStep lazyStepBeingResolvedInThisThread() { + return FormatterStepImpl.lazyStepBeingResolvedInThisThread.get(); + } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java index 4f25d3c1e4..da1210900a 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java @@ -53,9 +53,17 @@ public String getName() { @Override protected State calculateState() throws Exception { - return stateSupplier.get(); + try { + lazyStepBeingResolvedInThisThread.set(this); + return stateSupplier.get(); + } finally { + lazyStepBeingResolvedInThisThread.set(null); + } } + /** Stores the step name in a ThreadLocal for the purpose of debugging errors triggered in calculateState. */ + static final ThreadLocal lazyStepBeingResolvedInThisThread = new ThreadLocal<>(); + static final class Standard extends FormatterStepImpl { private static final long serialVersionUID = 1L; From 892203d499685397c28e1d70a1c76dd554e3fdda Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 31 Dec 2019 22:39:02 -0800 Subject: [PATCH 08/18] Create a :spotlessRegisterDependencies task, and use it to resolve the Gradle 6+ deprecation warnings. --- .../gradle/spotless/FormatExtension.java | 15 ++ .../gradle/spotless/GradleProvisioner.java | 16 +- .../spotless/RegisterDependenciesInRoot.java | 182 ++++++++++++++++++ .../spotless/RegisterDependenciesTask.java | 82 ++++++++ .../gradle/spotless/SpotlessExtension.java | 14 ++ .../RegisterDependenciesInRootTest.java | 113 +++++++++++ 6 files changed, 420 insertions(+), 2 deletions(-) create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java create mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java create mode 100644 plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 3723d89526..940f5b0041 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -145,6 +145,11 @@ public void target(Object... targets) { this.target = parseTargetsIsExclude(targets, false); } + /** Sets the target to be empty without a warning. */ + public void targetEmptyForDeclaration() { + this.target = getProject().files(); + } + /** * Sets which files will be excluded from formatting. Files to be formatted = (target - targetExclude). * @@ -603,6 +608,16 @@ protected void setupTask(SpotlessTask task) { } task.setSteps(steps); task.setLineEndingsPolicy(getLineEndings().createPolicy(getProject().getProjectDir(), () -> task.target)); + if (root.registerDependenciesTask != null) { + // if we have a register dependencies task + if (root.project == root.project.getRootProject()) { + // :spotlessRegisterDependencies depends on every SpotlessTask in the root + root.registerDependenciesTask.dependsOn(task); + } else { + // and every SpotlessTask in a subproject depends on :spotlessRegisterDependencies + task.dependsOn(root.registerDependenciesTask); + } + } } /** Returns the project that this extension is attached to. */ diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java index 57ee601de9..8737a46ae1 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java @@ -32,6 +32,19 @@ public class GradleProvisioner { private GradleProvisioner() {} public static Provisioner fromProject(Project project) { + RegisterDependenciesTask task = project.getPlugins().apply(SpotlessPlugin.class).getExtension().registerDependenciesTask; + if (task == null) { + return fromRootBuildscript(project); + } else { + if (project.getRootProject() == project) { + return task.rootProvisioner; + } else { + return new RegisterDependenciesInRoot.SubProvisioner(task.rootProvisioner, project); + } + } + } + + static Provisioner fromRootBuildscript(Project project) { Objects.requireNonNull(project); return (withTransitives, mavenCoords) -> { try { @@ -60,6 +73,5 @@ public static Provisioner fromProject(Project project) { }; } - private static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); - + static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java new file mode 100644 index 0000000000..60079a19fd --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java @@ -0,0 +1,182 @@ +/* + * Copyright 2016 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.File; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import javax.annotation.Nullable; + +import org.gradle.api.Project; +import org.gradle.util.GradleVersion; + +import com.diffplug.common.base.Preconditions; +import com.diffplug.common.collect.ImmutableList; +import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.Provisioner; + +class RegisterDependenciesInRoot { + static final GradleVersion STRICT_CONFIG_ACCESS_WARNING = GradleVersion.version("6.0"); + + static final String ENABLE_KEY = "spotless_register_dependencies_in_root"; + static final String TASK_NAME = "spotlessRegisterDependencies"; + + /** Determines if the "spotless_register_dependencies_in_root" mode is enabled. */ + public static boolean isEnabled(Project project) { + Object enable = project.getRootProject().findProperty(ENABLE_KEY); + if (Boolean.TRUE.equals(enable) || "true".equals(enable)) { + return true; + } + boolean onlyOneProjectInEntireBuild = project == project.getRootProject() + && project.getChildProjects().isEmpty(); + if (onlyOneProjectInEntireBuild) { + return false; + } + if (GradleVersion.current().compareTo(STRICT_CONFIG_ACCESS_WARNING) >= 0) { + return true; + } + return false; + } + + /** Models a request to the provisioner. */ + private static class Request { + final boolean withTransitives; + final ImmutableList mavenCoords; + + public Request(boolean withTransitives, Collection mavenCoords) { + this.withTransitives = withTransitives; + this.mavenCoords = ImmutableList.copyOf(mavenCoords); + } + + @Override + public int hashCode() { + return withTransitives ? mavenCoords.hashCode() : ~mavenCoords.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } else if (obj instanceof Request) { + Request o = (Request) obj; + return o.withTransitives == withTransitives && o.mavenCoords.equals(mavenCoords); + } else { + return false; + } + } + + @Override + public String toString() { + String coords = mavenCoords.toString(); + StringBuilder builder = new StringBuilder(); + builder.append(coords.substring(1, coords.length() - 1)); // strip off [] + if (withTransitives) { + builder.append(" with transitives"); + } else { + builder.append(" no transitives"); + } + return builder.toString(); + } + } + + /** The provisioner used for all sub-projects. */ + static class SubProvisioner implements Provisioner { + private final RootProvisioner root; + private final Project project; + + public SubProvisioner(RootProvisioner root, Project project) { + this.root = Objects.requireNonNull(root); + this.project = Objects.requireNonNull(project); + } + + @Override + public Set provisionWithTransitives(boolean withTransitives, Collection mavenCoordinates) { + return root.provisionForSub(project, withTransitives, mavenCoordinates); + } + } + + /** The provisioner used for the root project. */ + static class RootProvisioner implements Provisioner { + private final Project rootProject; + + RootProvisioner(Project rootProject) { + Preconditions.checkArgument(rootProject == rootProject.getRootProject()); + this.rootProject = rootProject; + } + + @Override + public Set provisionWithTransitives(boolean withTransitives, Collection mavenCoordinates) { + return doProvision(new Request(withTransitives, mavenCoordinates), true); + } + + private Map> cache = new HashMap<>(); + + /** Guaranteed to return non-null for internal requests, but might return null for an external request which isn't cached already. */ + private synchronized @Nullable Set doProvision(Request req, boolean isRoot) { + Set result = cache.get(req); + if (result != null) { + return result; + } + if (isRoot) { + result = GradleProvisioner.fromRootBuildscript(rootProject).provisionWithTransitives(req.withTransitives, req.mavenCoords); + cache.put(req, result); + return result; + } else { + return null; + } + } + + private Set provisionForSub(Project project, boolean withTransitives, Collection mavenCoordinates) { + Request req = new Request(withTransitives, mavenCoordinates); + Set result = doProvision(req, false); + if (result != null) { + return result; + } else { + // if it wasn't cached, complain loudly and use the crappy workaround + GradleProvisioner.logger.severe(warningMsg(req)); + return GradleProvisioner.fromRootBuildscript(project).provisionWithTransitives(withTransitives, mavenCoordinates); + } + } + } + + private static String warningMsg(Request requestedDeps) { + FormatterStep beingResolved = FormatterStep.lazyStepBeingResolvedInThisThread(); + return String.format( + "This subproject is using a formatter that was not used in the root project. To enable%n" + + "performance optimzations (and avoid Gradle 7 deprecation warnings), you must declare%n" + + "all of your formatters within the root project. For example, if your subproject has%n" + + "a `java {}` block but your root project does not, just add a matching `java {}` block to%n" + + "your root project. If you want to make it clear that it is intentional that the target%n" + + "is empty, you can do this in your root build.gradle:%n" + + "%n" + + " spotless {%n" + + " java {%n" + + " targetEmptyForDeclaration()%n" + + " [...same steps as subproject...]%n" + + " }%n" + + " }%n" + + "%n" + + "To help you figure out which block is missing, the step you are missing is%n" + + " step name: %s%n" + + " requested: %s%n", + beingResolved == null ? "(unknown)" : beingResolved.getName(), + requestedDeps); + } +} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java new file mode 100644 index 0000000000..7cdd7298b0 --- /dev/null +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -0,0 +1,82 @@ +/* + * Copyright 2016 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.gradle.api.DefaultTask; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Internal; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.TaskAction; + +import com.diffplug.common.collect.Sets; +import com.diffplug.common.io.Files; +import com.diffplug.spotless.FormatterStep; + +/** + * The minimal task required to force all SpotlessTasks in the root + * project to trigger their dependency resolution, so that they will + * be cached for subproject tasks to slurp from. + */ +public class RegisterDependenciesTask extends DefaultTask { + @Input + public List getSteps() { + List allSteps = new ArrayList<>(); + Set alreadyAdded = Sets.newIdentityHashSet(); + for (Object dependsOn : getDependsOn()) { + // in Gradle 2.14, we can have a single task listed as a dep twice, + // and we can also have non-tasks listed as a dep + if (dependsOn instanceof SpotlessTask) { + SpotlessTask task = (SpotlessTask) dependsOn; + if (alreadyAdded.add(task)) { + allSteps.addAll(task.getSteps()); + } + } + } + return allSteps; + } + + File unitOutput; + + @OutputFile + public File getUnitOutput() { + return unitOutput; + } + + RegisterDependenciesInRoot.RootProvisioner rootProvisioner; + + @Internal + public RegisterDependenciesInRoot.RootProvisioner getRootProvisioner() { + return rootProvisioner; + } + + void setup() { + unitOutput = new File(getProject().getBuildDir(), "tmp/spotless-register-dependencies"); + rootProvisioner = new RegisterDependenciesInRoot.RootProvisioner(getProject()); + } + + @TaskAction + public void trivialFunction() throws IOException { + Files.createParentDirs(unitOutput); + Files.write("unit", unitOutput, StandardCharsets.UTF_8); + } +} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index 7d34a93caa..8b6421142b 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -23,6 +23,8 @@ import java.util.LinkedHashMap; import java.util.Map; +import javax.annotation.Nullable; + import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; @@ -39,6 +41,7 @@ public class SpotlessExtension { final Project project; final Task rootCheckTask, rootApplyTask; + final @Nullable RegisterDependenciesTask registerDependenciesTask; static final String EXTENSION = "spotless"; static final String CHECK = "Check"; @@ -58,6 +61,17 @@ public SpotlessExtension(Project project) { rootApplyTask = project.task(EXTENSION + APPLY); rootApplyTask.setGroup(TASK_GROUP); rootApplyTask.setDescription(APPLY_DESCRIPTION); + boolean registerDependenciesInRoot = RegisterDependenciesInRoot.isEnabled(project); + if (registerDependenciesInRoot) { + if (project.getRootProject() == project) { + registerDependenciesTask = project.getTasks().create(RegisterDependenciesInRoot.TASK_NAME, RegisterDependenciesTask.class); + registerDependenciesTask.setup(); + } else { + registerDependenciesTask = project.getRootProject().getPlugins().apply(SpotlessPlugin.class).spotlessExtension.registerDependenciesTask; + } + } else { + registerDependenciesTask = null; + } } /** Line endings (if any). */ diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java new file mode 100644 index 0000000000..d49f6ead2f --- /dev/null +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java @@ -0,0 +1,113 @@ +/* + * Copyright 2016 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.gradle.spotless; + +import java.io.IOException; + +import org.assertj.core.api.Assertions; +import org.junit.Test; + +public class RegisterDependenciesInRootTest extends GradleIntegrationTest { + @Test + public void registerDependencies() throws IOException { + setFile("settings.gradle") + .toLines("include 'sub'"); + setFile("build.gradle").toLines( + "buildscript { repositories { mavenCentral() } }", + "plugins { id 'com.diffplug.gradle.spotless' }"); + setFile("sub/build.gradle").toLines( + "apply plugin: 'com.diffplug.gradle.spotless'", + "", + "spotless {", + " java {", + " target 'src/main/java/**/*.java'", + " googleJavaFormat('1.2')", + " }", + "}"); + + // works fine on old versions + String oldestSupported = gradleRunner() + .withArguments("spotlessCheck").build().getOutput(); + Assertions.assertThat(oldestSupported.replace("\r", "")).startsWith( + ":spotlessCheck UP-TO-DATE\n" + + ":sub:spotlessJava\n" + + ":sub:spotlessJavaCheck\n" + + ":sub:spotlessCheck\n" + + "\n" + + "BUILD SUCCESSFUL"); + + // generates a warning in 6.0 + setFile("gradle.properties").toLines(); + String warningStarts = gradleRunner().withGradleVersion("6.0") + .withArguments("spotlessCheck").build().getOutput(); + assertWarning(warningStarts, true); + + // we can make the old version generate the warning with spotless_register_dependencies_in_root + setFile("gradle.properties").toLines( + "spotless_register_dependencies_in_root=true"); + String oldestSupportedWithRegisterDependencies = gradleRunner() + .withArguments("spotlessCheck").build().getOutput(); + assertWarning(oldestSupportedWithRegisterDependencies, true); + + // fix the root project + setFile("build.gradle").toLines( + "buildscript { repositories { mavenCentral() } }", + "plugins { id 'com.diffplug.gradle.spotless' }", + "spotless {", + " java {", + " targetEmptyForDeclaration()", + " googleJavaFormat('1.2')", + " }", + "}"); + assertWarning(warningStarts, true); + + setFile("gradle.properties").toLines( + "spotless_register_dependencies_in_root=true"); + String oldestSupportedWithRegisterDependenciesFixed = gradleRunner() + .withArguments("spotlessCheck").build().getOutput(); + assertWarning(oldestSupportedWithRegisterDependenciesFixed, false); + + setFile("gradle.properties").toLines(); + String warningStartsFixed = gradleRunner().withGradleVersion("6.0") + .withArguments("spotlessCheck").build().getOutput(); + assertWarning(warningStartsFixed, false); + } + + private void assertWarning(String input, boolean isThere) { + String warning = "This subproject is using a formatter that was not used in the root project. To enable\n" + + "performance optimzations (and avoid Gradle 7 deprecation warnings), you must declare\n" + + "all of your formatters within the root project. For example, if your subproject has\n" + + "a `java {}` block but your root project does not, just add a matching `java {}` block to\n" + + "your root project. If you want to make it clear that it is intentional that the target\n" + + "is empty, you can do this in your root build.gradle:\n" + + "\n" + + " spotless {\n" + + " java {\n" + + " targetEmptyForDeclaration()\n" + + " [...same steps as subproject...]\n" + + " }\n" + + " }\n" + + "\n" + + "To help you figure out which block is missing, the step you are missing is\n" + + " step name: google-java-format\n" + + " requested: com.google.googlejavaformat:google-java-format:1.2 with transitives\n"; + if (isThere) { + Assertions.assertThat(input.replace("\r", "")).contains(warning); + } else { + Assertions.assertThat(input.replace("\r", "")).doesNotContain(warning); + } + } +} From ad869157b64959c1ea3f543ccb409879682c6aa8 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 11:18:33 -0800 Subject: [PATCH 09/18] Fix the maven build. --- plugin-maven/build.gradle | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/plugin-maven/build.gradle b/plugin-maven/build.gradle index dd7d58bda0..c2aa5a4bb1 100644 --- a/plugin-maven/build.gradle +++ b/plugin-maven/build.gradle @@ -55,18 +55,17 @@ def mvnw(String args) { } } -apply plugin: 'java-library' dependencies { if (!project.versionMaven.endsWith('-SNAPSHOT') && project.versionLib.endsWith('-SNAPSHOT')) { // gradle = release, lib = snapshot, therefore gradle should depend on the last stable lib - api "com.diffplug.spotless:spotless-lib:${project.stableLib}" - api "com.diffplug.spotless:spotless-lib-extra:${project.stableLib}" + implementation "com.diffplug.spotless:spotless-lib:${project.stableLib}" + implementation "com.diffplug.spotless:spotless-lib-extra:${project.stableLib}" } else { - api project(':lib') - api project(':lib-extra') + implementation project(':lib') + implementation project(':lib-extra') } - api "org.codehaus.plexus:plexus-resources:${VER_PLEXUS_RESOURCES}" + implementation "org.codehaus.plexus:plexus-resources:${VER_PLEXUS_RESOURCES}" compileOnly "org.apache.maven:maven-plugin-api:${VER_MAVEN_API}" compileOnly "org.apache.maven.plugin-tools:maven-plugin-annotations:${VER_MAVEN_API}" @@ -136,7 +135,7 @@ libs.each { task createPomXml(dependsOn: installLocalDependencies) { doLast { - def additionalDependencies = project.configurations.compileClasspath.resolvedConfiguration.resolvedArtifacts.findAll { + def additionalDependencies = project.configurations.runtimeClasspath.resolvedConfiguration.resolvedArtifacts.findAll { return !libs.contains(it.moduleVersion.id.name) }.collect { return " \n" + From e1980bd2c793788c20858c47d36eae055a271a93 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 13:37:16 -0800 Subject: [PATCH 10/18] Fix build scan bug in our CI. --- .ci/ci.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/ci.sh b/.ci/ci.sh index 5c63f91e10..32937655e4 100755 --- a/.ci/ci.sh +++ b/.ci/ci.sh @@ -1,7 +1,7 @@ #!/bin/bash # Do the Gradle build -./gradlew --scan build --build-cache || exit 1 +./gradlew build --build-cache || exit 1 ./gradlew npmTest --build-cache || exit 1 if [ "$TRAVIS_REPO_SLUG" == "diffplug/spotless" ] && [ "$TRAVIS_PULL_REQUEST" == "false" ] && [ "$TRAVIS_BRANCH" == "master" ]; then From 9f15a669bf131cb16039fa7b395cf97e14b02ad9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 11:58:07 -0800 Subject: [PATCH 11/18] Improve javadoc. --- .../diffplug/gradle/spotless/RegisterDependenciesTask.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index 7cdd7298b0..80e02a7d94 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -33,9 +33,12 @@ import com.diffplug.spotless.FormatterStep; /** + * NOT AN END-USER TASK, DO NOT USE FOR ANYTHING! + * * The minimal task required to force all SpotlessTasks in the root * project to trigger their dependency resolution, so that they will - * be cached for subproject tasks to slurp from. + * be cached for subproject tasks to slurp from. See {@link RegisterDependenciesInRoot} + * for the bigger picture. */ public class RegisterDependenciesTask extends DefaultTask { @Input From 342386c8282a8716d4f2165f8bd5b525d1577a84 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 13:34:35 -0800 Subject: [PATCH 12/18] Figured out a way to do it 100% transparently, without requiring declarations in root. We can infer them without a performance penalty by registering every subproject SpotlessTask with the RegisterDependenciesTask. When we eventually bump our minimum-required gradle to take advantage of task configuration avoidance, this workaround will still work - the only task we'll have to create eagerly is the registerDependencies task. --- .../gradle/spotless/FormatExtension.java | 16 +-- .../gradle/spotless/GradleProvisioner.java | 11 +- .../spotless/RegisterDependenciesInRoot.java | 101 +++--------------- .../spotless/RegisterDependenciesTask.java | 32 +++--- .../gradle/spotless/SpotlessExtension.java | 13 +-- .../RegisterDependenciesInRootTest.java | 69 ++---------- 6 files changed, 49 insertions(+), 193 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java index 940f5b0041..753b3fd280 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java @@ -145,11 +145,6 @@ public void target(Object... targets) { this.target = parseTargetsIsExclude(targets, false); } - /** Sets the target to be empty without a warning. */ - public void targetEmptyForDeclaration() { - this.target = getProject().files(); - } - /** * Sets which files will be excluded from formatting. Files to be formatted = (target - targetExclude). * @@ -608,15 +603,8 @@ protected void setupTask(SpotlessTask task) { } task.setSteps(steps); task.setLineEndingsPolicy(getLineEndings().createPolicy(getProject().getProjectDir(), () -> task.target)); - if (root.registerDependenciesTask != null) { - // if we have a register dependencies task - if (root.project == root.project.getRootProject()) { - // :spotlessRegisterDependencies depends on every SpotlessTask in the root - root.registerDependenciesTask.dependsOn(task); - } else { - // and every SpotlessTask in a subproject depends on :spotlessRegisterDependencies - task.dependsOn(root.registerDependenciesTask); - } + if (root.project != root.project.getRootProject()) { + root.registerDependenciesTask.hookSubprojectTask(task); } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java index 8737a46ae1..e516637bef 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java @@ -32,16 +32,7 @@ public class GradleProvisioner { private GradleProvisioner() {} public static Provisioner fromProject(Project project) { - RegisterDependenciesTask task = project.getPlugins().apply(SpotlessPlugin.class).getExtension().registerDependenciesTask; - if (task == null) { - return fromRootBuildscript(project); - } else { - if (project.getRootProject() == project) { - return task.rootProvisioner; - } else { - return new RegisterDependenciesInRoot.SubProvisioner(task.rootProvisioner, project); - } - } + return project.getPlugins().apply(SpotlessPlugin.class).getExtension().registerDependenciesTask.rootProvisioner; } static Provisioner fromRootBuildscript(Project project) { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java index 60079a19fd..3b9d16100f 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java @@ -19,41 +19,26 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.Set; -import javax.annotation.Nullable; - import org.gradle.api.Project; -import org.gradle.util.GradleVersion; import com.diffplug.common.base.Preconditions; import com.diffplug.common.collect.ImmutableList; -import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.Provisioner; +/** + * This is the problem that we're solving: + * - When a user asks for a formatter, we need to download the jars for that formatter + * - Gradle wants us to resolve all our dependencies in the root project - no new dependencies in subprojects + * - So we make it possible for the root project to create "dummy" tasks whose only function + * @author ntwigg + * + */ class RegisterDependenciesInRoot { - static final GradleVersion STRICT_CONFIG_ACCESS_WARNING = GradleVersion.version("6.0"); - - static final String ENABLE_KEY = "spotless_register_dependencies_in_root"; - static final String TASK_NAME = "spotlessRegisterDependencies"; + private RegisterDependenciesInRoot() {} - /** Determines if the "spotless_register_dependencies_in_root" mode is enabled. */ - public static boolean isEnabled(Project project) { - Object enable = project.getRootProject().findProperty(ENABLE_KEY); - if (Boolean.TRUE.equals(enable) || "true".equals(enable)) { - return true; - } - boolean onlyOneProjectInEntireBuild = project == project.getRootProject() - && project.getChildProjects().isEmpty(); - if (onlyOneProjectInEntireBuild) { - return false; - } - if (GradleVersion.current().compareTo(STRICT_CONFIG_ACCESS_WARNING) >= 0) { - return true; - } - return false; - } + static final String TASK_NAME = "spotlessInternalRegisterDependencies"; /** Models a request to the provisioner. */ private static class Request { @@ -96,25 +81,10 @@ public String toString() { } } - /** The provisioner used for all sub-projects. */ - static class SubProvisioner implements Provisioner { - private final RootProvisioner root; - private final Project project; - - public SubProvisioner(RootProvisioner root, Project project) { - this.root = Objects.requireNonNull(root); - this.project = Objects.requireNonNull(project); - } - - @Override - public Set provisionWithTransitives(boolean withTransitives, Collection mavenCoordinates) { - return root.provisionForSub(project, withTransitives, mavenCoordinates); - } - } - /** The provisioner used for the root project. */ static class RootProvisioner implements Provisioner { private final Project rootProject; + private final Map> cache = new HashMap<>(); RootProvisioner(Project rootProject) { Preconditions.checkArgument(rootProject == rootProject.getRootProject()); @@ -123,60 +93,15 @@ static class RootProvisioner implements Provisioner { @Override public Set provisionWithTransitives(boolean withTransitives, Collection mavenCoordinates) { - return doProvision(new Request(withTransitives, mavenCoordinates), true); - } - - private Map> cache = new HashMap<>(); - - /** Guaranteed to return non-null for internal requests, but might return null for an external request which isn't cached already. */ - private synchronized @Nullable Set doProvision(Request req, boolean isRoot) { + Request req = new Request(withTransitives, mavenCoordinates); Set result = cache.get(req); if (result != null) { return result; - } - if (isRoot) { + } else { result = GradleProvisioner.fromRootBuildscript(rootProject).provisionWithTransitives(req.withTransitives, req.mavenCoords); cache.put(req, result); return result; - } else { - return null; - } - } - - private Set provisionForSub(Project project, boolean withTransitives, Collection mavenCoordinates) { - Request req = new Request(withTransitives, mavenCoordinates); - Set result = doProvision(req, false); - if (result != null) { - return result; - } else { - // if it wasn't cached, complain loudly and use the crappy workaround - GradleProvisioner.logger.severe(warningMsg(req)); - return GradleProvisioner.fromRootBuildscript(project).provisionWithTransitives(withTransitives, mavenCoordinates); } } } - - private static String warningMsg(Request requestedDeps) { - FormatterStep beingResolved = FormatterStep.lazyStepBeingResolvedInThisThread(); - return String.format( - "This subproject is using a formatter that was not used in the root project. To enable%n" + - "performance optimzations (and avoid Gradle 7 deprecation warnings), you must declare%n" + - "all of your formatters within the root project. For example, if your subproject has%n" + - "a `java {}` block but your root project does not, just add a matching `java {}` block to%n" + - "your root project. If you want to make it clear that it is intentional that the target%n" + - "is empty, you can do this in your root build.gradle:%n" + - "%n" + - " spotless {%n" + - " java {%n" + - " targetEmptyForDeclaration()%n" + - " [...same steps as subproject...]%n" + - " }%n" + - " }%n" + - "%n" + - "To help you figure out which block is missing, the step you are missing is%n" + - " step name: %s%n" + - " requested: %s%n", - beingResolved == null ? "(unknown)" : beingResolved.getName(), - requestedDeps); - } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index 80e02a7d94..914963913a 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -20,21 +20,20 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; -import java.util.Set; import org.gradle.api.DefaultTask; +import org.gradle.api.execution.TaskExecutionGraph; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; -import com.diffplug.common.collect.Sets; import com.diffplug.common.io.Files; import com.diffplug.spotless.FormatterStep; /** * NOT AN END-USER TASK, DO NOT USE FOR ANYTHING! - * + * * The minimal task required to force all SpotlessTasks in the root * project to trigger their dependency resolution, so that they will * be cached for subproject tasks to slurp from. See {@link RegisterDependenciesInRoot} @@ -44,20 +43,27 @@ public class RegisterDependenciesTask extends DefaultTask { @Input public List getSteps() { List allSteps = new ArrayList<>(); - Set alreadyAdded = Sets.newIdentityHashSet(); - for (Object dependsOn : getDependsOn()) { - // in Gradle 2.14, we can have a single task listed as a dep twice, - // and we can also have non-tasks listed as a dep - if (dependsOn instanceof SpotlessTask) { - SpotlessTask task = (SpotlessTask) dependsOn; - if (alreadyAdded.add(task)) { - allSteps.addAll(task.getSteps()); - } + TaskExecutionGraph taskGraph = getProject().getGradle().getTaskGraph(); + for (SpotlessTask task : tasks) { + if (taskGraph.hasTask(task)) { + allSteps.addAll(task.getSteps()); } } return allSteps; } + private List tasks = new ArrayList<>(); + + @Internal + public List getTasks() { + return tasks; + } + + void hookSubprojectTask(SpotlessTask task) { + tasks.add(task); + task.dependsOn(this); + } + File unitOutput; @OutputFile @@ -80,6 +86,6 @@ void setup() { @TaskAction public void trivialFunction() throws IOException { Files.createParentDirs(unitOutput); - Files.write("unit", unitOutput, StandardCharsets.UTF_8); + Files.write(Integer.toString(getSteps().size()), unitOutput, StandardCharsets.UTF_8); } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index 8b6421142b..f2a5d8a8ab 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -61,16 +61,11 @@ public SpotlessExtension(Project project) { rootApplyTask = project.task(EXTENSION + APPLY); rootApplyTask.setGroup(TASK_GROUP); rootApplyTask.setDescription(APPLY_DESCRIPTION); - boolean registerDependenciesInRoot = RegisterDependenciesInRoot.isEnabled(project); - if (registerDependenciesInRoot) { - if (project.getRootProject() == project) { - registerDependenciesTask = project.getTasks().create(RegisterDependenciesInRoot.TASK_NAME, RegisterDependenciesTask.class); - registerDependenciesTask.setup(); - } else { - registerDependenciesTask = project.getRootProject().getPlugins().apply(SpotlessPlugin.class).spotlessExtension.registerDependenciesTask; - } + if (project.getRootProject() == project) { + registerDependenciesTask = project.getTasks().create(RegisterDependenciesInRoot.TASK_NAME, RegisterDependenciesTask.class); + registerDependenciesTask.setup(); } else { - registerDependenciesTask = null; + registerDependenciesTask = project.getRootProject().getPlugins().apply(SpotlessPlugin.class).spotlessExtension.registerDependenciesTask; } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java index d49f6ead2f..fe2e7c6793 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java @@ -38,76 +38,27 @@ public void registerDependencies() throws IOException { " }", "}"); - // works fine on old versions String oldestSupported = gradleRunner() .withArguments("spotlessCheck").build().getOutput(); Assertions.assertThat(oldestSupported.replace("\r", "")).startsWith( ":spotlessCheck UP-TO-DATE\n" + + ":spotlessInternalRegisterDependencies\n" + ":sub:spotlessJava\n" + ":sub:spotlessJavaCheck\n" + ":sub:spotlessCheck\n" + "\n" + "BUILD SUCCESSFUL"); - // generates a warning in 6.0 setFile("gradle.properties").toLines(); - String warningStarts = gradleRunner().withGradleVersion("6.0") + String newestSupported = gradleRunner().withGradleVersion("6.0") .withArguments("spotlessCheck").build().getOutput(); - assertWarning(warningStarts, true); - - // we can make the old version generate the warning with spotless_register_dependencies_in_root - setFile("gradle.properties").toLines( - "spotless_register_dependencies_in_root=true"); - String oldestSupportedWithRegisterDependencies = gradleRunner() - .withArguments("spotlessCheck").build().getOutput(); - assertWarning(oldestSupportedWithRegisterDependencies, true); - - // fix the root project - setFile("build.gradle").toLines( - "buildscript { repositories { mavenCentral() } }", - "plugins { id 'com.diffplug.gradle.spotless' }", - "spotless {", - " java {", - " targetEmptyForDeclaration()", - " googleJavaFormat('1.2')", - " }", - "}"); - assertWarning(warningStarts, true); - - setFile("gradle.properties").toLines( - "spotless_register_dependencies_in_root=true"); - String oldestSupportedWithRegisterDependenciesFixed = gradleRunner() - .withArguments("spotlessCheck").build().getOutput(); - assertWarning(oldestSupportedWithRegisterDependenciesFixed, false); - - setFile("gradle.properties").toLines(); - String warningStartsFixed = gradleRunner().withGradleVersion("6.0") - .withArguments("spotlessCheck").build().getOutput(); - assertWarning(warningStartsFixed, false); - } - - private void assertWarning(String input, boolean isThere) { - String warning = "This subproject is using a formatter that was not used in the root project. To enable\n" + - "performance optimzations (and avoid Gradle 7 deprecation warnings), you must declare\n" + - "all of your formatters within the root project. For example, if your subproject has\n" + - "a `java {}` block but your root project does not, just add a matching `java {}` block to\n" + - "your root project. If you want to make it clear that it is intentional that the target\n" + - "is empty, you can do this in your root build.gradle:\n" + - "\n" + - " spotless {\n" + - " java {\n" + - " targetEmptyForDeclaration()\n" + - " [...same steps as subproject...]\n" + - " }\n" + - " }\n" + - "\n" + - "To help you figure out which block is missing, the step you are missing is\n" + - " step name: google-java-format\n" + - " requested: com.google.googlejavaformat:google-java-format:1.2 with transitives\n"; - if (isThere) { - Assertions.assertThat(input.replace("\r", "")).contains(warning); - } else { - Assertions.assertThat(input.replace("\r", "")).doesNotContain(warning); - } + Assertions.assertThat(newestSupported.replace("\r", "")).startsWith( + "> Task :spotlessCheck UP-TO-DATE\n" + + "> Task :spotlessInternalRegisterDependencies\n" + + "> Task :sub:spotlessJava\n" + + "> Task :sub:spotlessJavaCheck\n" + + "> Task :sub:spotlessCheck\n" + + "\n" + + "BUILD SUCCESSFUL"); } } From c08b4c7d06a0ef7ab58a661dccd2ade2ff112e2e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 13:53:50 -0800 Subject: [PATCH 13/18] Reorganized - all provisioner stuff into GradleProvisioner, all the rest into RegisterDependenciesTask. --- .../gradle/spotless/GradleProvisioner.java | 74 +++++++++++- .../spotless/RegisterDependenciesInRoot.java | 107 ------------------ .../spotless/RegisterDependenciesTask.java | 18 +-- .../gradle/spotless/SpotlessExtension.java | 6 +- ...java => RegisterDependenciesTaskTest.java} | 2 +- 5 files changed, 87 insertions(+), 120 deletions(-) delete mode 100644 plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java rename plugin-gradle/src/test/java/com/diffplug/gradle/spotless/{RegisterDependenciesInRootTest.java => RegisterDependenciesTaskTest.java} (96%) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java index e516637bef..d5bbf6a823 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java @@ -15,7 +15,12 @@ */ package com.diffplug.gradle.spotless; +import java.io.File; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -24,7 +29,9 @@ import org.gradle.api.artifacts.ConfigurationContainer; import org.gradle.api.artifacts.Dependency; +import com.diffplug.common.base.Preconditions; import com.diffplug.common.base.StringPrinter; +import com.diffplug.common.collect.ImmutableList; import com.diffplug.spotless.Provisioner; /** Gradle integration for Provisioner. */ @@ -35,6 +42,30 @@ public static Provisioner fromProject(Project project) { return project.getPlugins().apply(SpotlessPlugin.class).getExtension().registerDependenciesTask.rootProvisioner; } + /** The provisioner used for the root project. */ + static class RootProvisioner implements Provisioner { + private final Project rootProject; + private final Map> cache = new HashMap<>(); + + RootProvisioner(Project rootProject) { + Preconditions.checkArgument(rootProject == rootProject.getRootProject()); + this.rootProject = rootProject; + } + + @Override + public Set provisionWithTransitives(boolean withTransitives, Collection mavenCoordinates) { + Request req = new Request(withTransitives, mavenCoordinates); + Set result = cache.get(req); + if (result != null) { + return result; + } else { + result = GradleProvisioner.fromRootBuildscript(rootProject).provisionWithTransitives(req.withTransitives, req.mavenCoords); + cache.put(req, result); + return result; + } + } + } + static Provisioner fromRootBuildscript(Project project) { Objects.requireNonNull(project); return (withTransitives, mavenCoords) -> { @@ -64,5 +95,46 @@ static Provisioner fromRootBuildscript(Project project) { }; } - static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); + private static final Logger logger = Logger.getLogger(GradleProvisioner.class.getName()); + + /** Models a request to the provisioner. */ + private static class Request { + final boolean withTransitives; + final ImmutableList mavenCoords; + + public Request(boolean withTransitives, Collection mavenCoords) { + this.withTransitives = withTransitives; + this.mavenCoords = ImmutableList.copyOf(mavenCoords); + } + + @Override + public int hashCode() { + return withTransitives ? mavenCoords.hashCode() : ~mavenCoords.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } else if (obj instanceof Request) { + Request o = (Request) obj; + return o.withTransitives == withTransitives && o.mavenCoords.equals(mavenCoords); + } else { + return false; + } + } + + @Override + public String toString() { + String coords = mavenCoords.toString(); + StringBuilder builder = new StringBuilder(); + builder.append(coords.substring(1, coords.length() - 1)); // strip off [] + if (withTransitives) { + builder.append(" with transitives"); + } else { + builder.append(" no transitives"); + } + return builder.toString(); + } + } } diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java deleted file mode 100644 index 3b9d16100f..0000000000 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesInRoot.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright 2016 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.gradle.spotless; - -import java.io.File; -import java.util.Collection; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; - -import org.gradle.api.Project; - -import com.diffplug.common.base.Preconditions; -import com.diffplug.common.collect.ImmutableList; -import com.diffplug.spotless.Provisioner; - -/** - * This is the problem that we're solving: - * - When a user asks for a formatter, we need to download the jars for that formatter - * - Gradle wants us to resolve all our dependencies in the root project - no new dependencies in subprojects - * - So we make it possible for the root project to create "dummy" tasks whose only function - * @author ntwigg - * - */ -class RegisterDependenciesInRoot { - private RegisterDependenciesInRoot() {} - - static final String TASK_NAME = "spotlessInternalRegisterDependencies"; - - /** Models a request to the provisioner. */ - private static class Request { - final boolean withTransitives; - final ImmutableList mavenCoords; - - public Request(boolean withTransitives, Collection mavenCoords) { - this.withTransitives = withTransitives; - this.mavenCoords = ImmutableList.copyOf(mavenCoords); - } - - @Override - public int hashCode() { - return withTransitives ? mavenCoords.hashCode() : ~mavenCoords.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } else if (obj instanceof Request) { - Request o = (Request) obj; - return o.withTransitives == withTransitives && o.mavenCoords.equals(mavenCoords); - } else { - return false; - } - } - - @Override - public String toString() { - String coords = mavenCoords.toString(); - StringBuilder builder = new StringBuilder(); - builder.append(coords.substring(1, coords.length() - 1)); // strip off [] - if (withTransitives) { - builder.append(" with transitives"); - } else { - builder.append(" no transitives"); - } - return builder.toString(); - } - } - - /** The provisioner used for the root project. */ - static class RootProvisioner implements Provisioner { - private final Project rootProject; - private final Map> cache = new HashMap<>(); - - RootProvisioner(Project rootProject) { - Preconditions.checkArgument(rootProject == rootProject.getRootProject()); - this.rootProject = rootProject; - } - - @Override - public Set provisionWithTransitives(boolean withTransitives, Collection mavenCoordinates) { - Request req = new Request(withTransitives, mavenCoordinates); - Set result = cache.get(req); - if (result != null) { - return result; - } else { - result = GradleProvisioner.fromRootBuildscript(rootProject).provisionWithTransitives(req.withTransitives, req.mavenCoords); - cache.put(req, result); - return result; - } - } - } -} diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index 914963913a..858a20955d 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -34,12 +34,16 @@ /** * NOT AN END-USER TASK, DO NOT USE FOR ANYTHING! * - * The minimal task required to force all SpotlessTasks in the root - * project to trigger their dependency resolution, so that they will - * be cached for subproject tasks to slurp from. See {@link RegisterDependenciesInRoot} - * for the bigger picture. + * - When a user asks for a formatter, we need to download the jars for that formatter + * - Gradle wants us to resolve all our dependencies in the root project - no new dependencies in subprojects + * - So, whenever a SpotlessTask in a subproject gets configured, we call {@link #hookSubprojectTask(SpotlessTask)}, + * which makes this task a dependency of the SpotlessTask + * - When this "registerDependencies" task does its up-to-date check, it queries the task execution graph to see which + * SpotlessTasks are at risk of being executed, and causes them all to be evaluated safely in the root buildscript. */ public class RegisterDependenciesTask extends DefaultTask { + static final String TASK_NAME = "spotlessInternalRegisterDependencies"; + @Input public List getSteps() { List allSteps = new ArrayList<>(); @@ -71,16 +75,16 @@ public File getUnitOutput() { return unitOutput; } - RegisterDependenciesInRoot.RootProvisioner rootProvisioner; + GradleProvisioner.RootProvisioner rootProvisioner; @Internal - public RegisterDependenciesInRoot.RootProvisioner getRootProvisioner() { + public GradleProvisioner.RootProvisioner getRootProvisioner() { return rootProvisioner; } void setup() { unitOutput = new File(getProject().getBuildDir(), "tmp/spotless-register-dependencies"); - rootProvisioner = new RegisterDependenciesInRoot.RootProvisioner(getProject()); + rootProvisioner = new GradleProvisioner.RootProvisioner(getProject()); } @TaskAction diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java index f2a5d8a8ab..1221872c06 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessExtension.java @@ -23,8 +23,6 @@ import java.util.LinkedHashMap; import java.util.Map; -import javax.annotation.Nullable; - import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; @@ -41,7 +39,7 @@ public class SpotlessExtension { final Project project; final Task rootCheckTask, rootApplyTask; - final @Nullable RegisterDependenciesTask registerDependenciesTask; + final RegisterDependenciesTask registerDependenciesTask; static final String EXTENSION = "spotless"; static final String CHECK = "Check"; @@ -62,7 +60,7 @@ public SpotlessExtension(Project project) { rootApplyTask.setGroup(TASK_GROUP); rootApplyTask.setDescription(APPLY_DESCRIPTION); if (project.getRootProject() == project) { - registerDependenciesTask = project.getTasks().create(RegisterDependenciesInRoot.TASK_NAME, RegisterDependenciesTask.class); + registerDependenciesTask = project.getTasks().create(RegisterDependenciesTask.TASK_NAME, RegisterDependenciesTask.class); registerDependenciesTask.setup(); } else { registerDependenciesTask = project.getRootProject().getPlugins().apply(SpotlessPlugin.class).spotlessExtension.registerDependenciesTask; diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java similarity index 96% rename from plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java rename to plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java index fe2e7c6793..2e4e0f40c0 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesInRootTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/RegisterDependenciesTaskTest.java @@ -20,7 +20,7 @@ import org.assertj.core.api.Assertions; import org.junit.Test; -public class RegisterDependenciesInRootTest extends GradleIntegrationTest { +public class RegisterDependenciesTaskTest extends GradleIntegrationTest { @Test public void registerDependencies() throws IOException { setFile("settings.gradle") From addcf1ec3928d30aad35d066ab87426bf91f6bfd Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 13:56:06 -0800 Subject: [PATCH 14/18] Revert "Make it possible for code within a lazy configuration to figure out which step is being configured." This reverts commit 28d8962a63a3c694ca3f2790cc144035dcac37fc. --- .../main/java/com/diffplug/spotless/FormatterStep.java | 9 --------- .../java/com/diffplug/spotless/FormatterStepImpl.java | 10 +--------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java index 1131c20a24..93e747aa40 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStep.java @@ -142,13 +142,4 @@ public static FormatterStep createNeverUpToDate( Objects.requireNonNull(function, "function"); return createNeverUpToDateLazy(name, () -> function); } - - /** - * If we are currently inside the `Supplier` of a lazy FormatterStep, - * this method will return the FormatterStep whose state is being calculated. - * Returns null if we are not. - */ - public static @Nullable FormatterStep lazyStepBeingResolvedInThisThread() { - return FormatterStepImpl.lazyStepBeingResolvedInThisThread.get(); - } } diff --git a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java index da1210900a..4f25d3c1e4 100644 --- a/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java +++ b/lib/src/main/java/com/diffplug/spotless/FormatterStepImpl.java @@ -53,17 +53,9 @@ public String getName() { @Override protected State calculateState() throws Exception { - try { - lazyStepBeingResolvedInThisThread.set(this); - return stateSupplier.get(); - } finally { - lazyStepBeingResolvedInThisThread.set(null); - } + return stateSupplier.get(); } - /** Stores the step name in a ThreadLocal for the purpose of debugging errors triggered in calculateState. */ - static final ThreadLocal lazyStepBeingResolvedInThisThread = new ThreadLocal<>(); - static final class Standard extends FormatterStepImpl { private static final long serialVersionUID = 1L; From d41ac593b7aea609e893013a9c66384d264aa7f9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 14:04:10 -0800 Subject: [PATCH 15/18] Update changelog. --- plugin-gradle/CHANGES.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index f3801fa46c..8c5a30793b 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,7 +3,9 @@ ### Version 3.27.0-SNAPSHOT - TBD ([javadoc](https://diffplug.github.io/spotless/javadoc/snapshot/), [snapshot](https://oss.sonatype.org/content/repositories/snapshots/com/diffplug/spotless/spotless-plugin-gradle/)) * Added method `FormatExtension.createIndependentTask(String taskName)` which allows creating a Spotless task outside of the `check`/`apply` lifecycle. See [javadoc](https://github.com/diffplug/spotless/blob/91ed7203994e52058ea6c2e0f88d023ed290e433/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java#L613-L639) for details. ([#500](https://github.com/diffplug/spotless/pull/500)) -* Running clean and spotlessCheck during a parallel build could cause exceptions, fixed by ([#501](https://github.com/diffplug/spotless/pull/501)). +* Running `clean` and `spotlessCheck` during a parallel build could cause exceptions, fixed by ([#501](https://github.com/diffplug/spotless/pull/501)). +* Fixed Gradle 7 deprecation warnings that started being emitted in Gradle 6. ([#503](https://github.com/diffplug/spotless/pull/503)) + * Even if you're using a pre-6.0 version of Gradle, you will probably see small performance and stability improvements. The PR above finally fixed the root problems of ([#372](https://github.com/diffplug/spotless/issues/372)). ### Version 3.26.1 - November 27th 2019 ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-plugin-gradle/3.26.1/), [jcenter](https://bintray.com/diffplug/opensource/spotless-plugin-gradle/3.26.1)) From 67fc9d8b4060379b0bfa7e995179bddfdcd64477 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 14:23:54 -0800 Subject: [PATCH 16/18] Remove the #372 workaround, since we don't need it anymore. --- .../gradle/spotless/GradleProvisioner.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java index d5bbf6a823..218dfb2a2f 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/GradleProvisioner.java @@ -26,11 +26,9 @@ import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.ConfigurationContainer; import org.gradle.api.artifacts.Dependency; import com.diffplug.common.base.Preconditions; -import com.diffplug.common.base.StringPrinter; import com.diffplug.common.collect.ImmutableList; import com.diffplug.spotless.Provisioner; @@ -39,6 +37,7 @@ public class GradleProvisioner { private GradleProvisioner() {} public static Provisioner fromProject(Project project) { + // TODO: this method is not necessary - we could remove it entirely for a small speedup return project.getPlugins().apply(SpotlessPlugin.class).getExtension().registerDependenciesTask.rootProvisioner; } @@ -74,21 +73,15 @@ static Provisioner fromRootBuildscript(Project project) { .map(project.getBuildscript().getDependencies()::create) .toArray(Dependency[]::new); - // #372 workaround: Accessing rootProject.configurations from multiple projects is not thread-safe - ConfigurationContainer configContainer; - synchronized (project.getRootProject()) { - configContainer = project.getRootProject().getBuildscript().getConfigurations(); - } - Configuration config = configContainer.detachedConfiguration(deps); + Configuration config = project.getRootProject().getBuildscript().getConfigurations().detachedConfiguration(deps); config.setDescription(mavenCoords.toString()); config.setTransitive(withTransitives); return config.resolve(); } catch (Exception e) { - logger.log(Level.SEVERE, - StringPrinter.buildStringFromLines("You probably need to add a repository containing the '" + mavenCoords + "' artifact in the 'build.gradle' of your root project.", + logger.log( + Level.SEVERE, + "You probably need to add a repository containing the '" + mavenCoords + "' artifact in the 'build.gradle' of your root project.\n" + "E.g.: 'buildscript { repositories { mavenCentral() }}'", - "Note that included buildscripts (using 'apply from') do not share their buildscript repositories with the underlying project.", - "You have to specify the missing repository explicitly in the buildscript of the root project."), e); throw e; } From 062e9e66931f0165028f014c2263b50c02fd225a Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 14:52:18 -0800 Subject: [PATCH 17/18] Minor improvement to the JitPack info. --- CONTRIBUTING.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 699e9df5ae..36a989760b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,6 +145,7 @@ pluginManagement { In Gradle 6.0+, you can use the following snippet in your `settings.gradle`. + ```gradle pluginManagement { repositories { @@ -159,12 +160,15 @@ pluginManagement { resolutionStrategy { eachPlugin { if (requested.id.id == 'com.diffplug.gradle.spotless') { - useModule('com.github.{{user-or-org}}.spotless:spotless-plugin-gradle:SHA_OF_COMMIT_YOU_WANT') + useModule('com.github.{{USER_OR_ORG}}.spotless:spotless-plugin-gradle:{{SHA_OF_COMMIT_YOU_WANT}}') } } } } ``` + +If it doesn't work, you can check the JitPack log at `https://jitpack.io/com/github/{{USER_OR_ORG}}/spotless/{{SHA_OF_COMMIT_YOU_WANT}}/build.log`. + ### Maven Run `./gradlew publishToMavenLocal` to publish this to your local repository. The maven plugin is not published to JitPack due to [jitpack/jitpack.io#4112](https://github.com/jitpack/jitpack.io/issues/4112). From 753590654feaa9377d0146184804f0e8cc92f0c7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Wed, 1 Jan 2020 14:53:34 -0800 Subject: [PATCH 18/18] RegisterDependenciesTask now asserts that it is applied to only the root project. --- .../com/diffplug/gradle/spotless/RegisterDependenciesTask.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java index 858a20955d..1284d7f372 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/RegisterDependenciesTask.java @@ -28,6 +28,7 @@ import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; +import com.diffplug.common.base.Preconditions; import com.diffplug.common.io.Files; import com.diffplug.spotless.FormatterStep; @@ -83,6 +84,7 @@ public GradleProvisioner.RootProvisioner getRootProvisioner() { } void setup() { + Preconditions.checkArgument(getProject().getRootProject() == getProject(), "Can only be used on the root project"); unitOutput = new File(getProject().getBuildDir(), "tmp/spotless-register-dependencies"); rootProvisioner = new GradleProvisioner.RootProvisioner(getProject()); }