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/09/01 09:20:42 UTC

[GitHub] [solr] madrob commented on a change in pull request #275: SOLR-15602: upgrade to gradle 7.2, clean up gradle code a bit.

madrob commented on a change in pull request #275:
URL: https://github.com/apache/solr/pull/275#discussion_r699392924



##########
File path: build.gradle
##########
@@ -20,15 +20,15 @@ import java.time.format.DateTimeFormatter
 
 plugins {
   id "base"
-  id "com.palantir.consistent-versions" version "1.14.0"
+  id "com.palantir.consistent-versions" version "2.0.0" 
   id "org.owasp.dependencycheck" version "5.3.0"
   id 'de.thetaphi.forbiddenapis' version '3.1' apply false
   id "de.undercouch.download" version "4.0.2" apply false
   id "net.ltgt.errorprone" version "2.0.2" apply false
-  id 'com.diffplug.spotless' version "5.8.2" apply false

Review comment:
       nit: end of line whitespace

##########
File path: build.gradle
##########
@@ -183,6 +184,7 @@ apply from: file('gradle/hacks/solr.findbugs.gradle')
 apply from: file('gradle/hacks/wipe-temp.gradle')
 apply from: file('gradle/hacks/hashmapAssertions.gradle')
 apply from: file('gradle/hacks/turbocharge-jvm-opts.gradle')
+apply from: file('gradle/hacks/dummy-outputs.gradle')

Review comment:
       can we call this "placeholder-outputs" instead?

##########
File path: gradle/generation/javacc.gradle
##########
@@ -129,17 +115,33 @@ configure(project(":solr:core")) {
 
 // We always regenerate, no need to declare outputs.
 class JavaCCTask extends DefaultTask {
-  @Input
+  @InputFile
   File javaccFile

Review comment:
       should this be `RegularFile`?

##########
File path: gradle/generation/local-settings.gradle
##########
@@ -35,26 +35,48 @@ configure(rootProject) {
         def testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12))
 
         // Write the defaults for this machine.
-        rootProject.file("gradle.properties").write(
-            [
-                "# These settings have been generated automatically on the first run.",
-                "# See gradlew :helpLocalSettings for more information.",
-                "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.priority=normal",
-                "org.gradle.warning.mode=none", // Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
-                "",
-                "# You may disable the background daemon if it consumes too much memory.",
-                "org.gradle.daemon=true",
-                "org.gradle.daemon.idletimeout=900000", // timeout after 15 mins.
-                "",
-                "# Maximum number of parallel gradle workers.",
-                "org.gradle.workers.max=${maxWorkers}",
-                "",
-                "# Maximum number of test JVMs forked per test task.",
-                "tests.jvms=${testsJvms}"
-            ].join("\n"), "UTF-8")
+        rootProject.file("gradle.properties").write("""

Review comment:
       Do we need some kind of flag to regenerate this file if it exists but users are upgrading from an older gradle/jvm version?

##########
File path: gradle/java/jar-manifest.gradle
##########
@@ -61,8 +61,9 @@ subprojects {
                             "Specification-Version" : project.baseVersion,
                             "Specification-Title"   : title,
 
-                            "X-Compile-Source-JDK"  : "${project.sourceCompatibility}",
-                            "X-Compile-Target-JDK"  : "${project.targetCompatibility}",
+                            // Evaluate these properties lazily so that the defaults are applied properly.
+                            "X-Compile-Source-JDK"  : "${-> project.sourceCompatibility}",

Review comment:
       I'm not familiar with this syntax and am having trouble searching for it. Can you help me find a reference?

##########
File path: gradle/globals.gradle
##########
@@ -0,0 +1,153 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+allprojects {
+  apply plugin: 'base'
+
+  group "org.apache"
+
+  def lucenePrereleaseBuild = '5'

Review comment:
       @markrmiller was updating this in #225 just in case that gets merged first




-- 
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