You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/08/30 14:38:11 UTC

[GitHub] [solr] dweiss commented on a change in pull request #277: SOLR-15603: Activate Gradle build cache

dweiss commented on a change in pull request #277:
URL: https://github.com/apache/solr/pull/277#discussion_r698541218



##########
File path: build.gradle
##########
@@ -20,7 +20,7 @@ import java.time.format.DateTimeFormatter
 
 plugins {
   id "base"
-  id "com.palantir.consistent-versions" version "1.14.0"

Review comment:
       There is a patch upgrading this plugin (and all of gradle infrastructure) here - https://github.com/apache/solr/pull/275. It'll conflict with your PR, once applied. I'll cherry pick relevant areas if you don't want to spend any more time on it.

##########
File path: gradle/generate-defaults.gradle
##########
@@ -42,6 +42,7 @@ configure(rootProject) {
                 "systemProp.file.encoding=UTF-8",
                 "org.gradle.jvmargs=-Xmx3g", // TODO figure out why "gradlew check" runs out of memory if 2g
                 "org.gradle.parallel=true",
+                "org.gradle.caching=true",

Review comment:
       This enables local caches, right? I'm not sure folks will be enthusiastic about their home folders increasing in size so dramatically - solr build outputs are pretty heavy.

##########
File path: gradle/validation/check-broken-links.gradle
##########
@@ -40,11 +41,13 @@ class CheckBrokenLinksTask extends DefaultTask {
   @InputFile
   File script = project.rootProject.file("dev-tools/scripts/checkJavadocLinks.py")
 
+  @OutputFile

Review comment:
       Yup, this is good, thanks.

##########
File path: gradle/wrapper/gradle-wrapper.properties
##########
@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-6.8.3-all.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-6.9.1-bin.zip

Review comment:
       this alone won't work - requires a new checksum too (dont in that other PR).

##########
File path: solr/solr-ref-guide/build.gradle
##########
@@ -215,20 +216,21 @@ check.dependsOn checkLocalJavadocLinksSite, checkSite
 // Hook site building to assemble.
 assemble.dependsOn buildSite
 
+@CacheableTask
 abstract class PrepareSources extends DefaultTask {
     // Original Source files we'll be syncing <b>FROM</b>
     @InputDirectory
-    public DirectoryProperty srcDir = project.objects.directoryProperty()
+    abstract DirectoryProperty getSrcDir()

Review comment:
       Are these abstract properties automatically injected if you declare them like this? Where is the gradle docs that explains this (and which properties can be injected like so)? Thanks.

##########
File path: solr/solr-ref-guide/build.gradle
##########
@@ -184,6 +184,7 @@ ext {
         group "Documentation"
         description "Builds the ${details.desc}"
 
+        outputs.cacheIf { true }

Review comment:
       I'm not sure what it does here?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org