You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "colvinco (via GitHub)" <gi...@apache.org> on 2023/01/31 09:27:57 UTC
[GitHub] [solr] colvinco opened a new pull request, #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
colvinco opened a new pull request, #1320:
URL: https://github.com/apache/solr/pull/1320
https://issues.apache.org/jira/browse/SOLR-16641
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
This fixes a circular dependency in the gradle task tree that occurs when gradle.properties doesn't already exist and certain tasks are run that depend on the generation of the gradle.properties, while other tasks depend on that task.
Also added notes on the need to have `perl` and `python3` for `gradlew check` to succeed.
# Solution
I've made the gradle.properties generation run as part of the gradle script directly, rather than wrapping it in a task that everything else has to depend on.
# Tests
Without this PR
1. Delete gradle.properties
2. Run `./gradlew updateLicenses` and it will always result in a circular dependency and fail.
3. Apply the PR and run `./gradlew updateLicenses` again and it will generate the gradle.properties and succeed.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093794953
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,106 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
+org.gradle.java.installations.paths=(custom paths)
Review Comment:
Agree, see https://docs.gradle.org/current/userguide/toolchains.html#sec:custom_loc
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094535705
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
I only put it there because it's adjacent to the https://github.com/apache/solr/blob/1d76b053c4a6fb742a97f4250b332d755680d00e/buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.java that's also being used in gradlew. In actual fact, neither of them belong in buildSrc, because they aren't used by the gradle scripts themselves. I just didn't want to do something different.
I could move them both to https://github.com/apache/solr/tree/1d76b053c4a6fb742a97f4250b332d755680d00e/gradle
Either into a new directory within it, or put the WrapperDownloader in https://github.com/apache/solr/tree/1d76b053c4a6fb742a97f4250b332d755680d00e/gradle/wrapper and the new generator in https://github.com/apache/solr/tree/1d76b053c4a6fb742a97f4250b332d755680d00e/gradle/generation?
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093334476
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
Review Comment:
```suggestion
cp gradle.properties.template "$APP_HOME/gradle.properties"
```
Also, could the `gradle.properties.template` file be inside `gradle/` folder to avoid littering the root git folder?
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093190383
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
I've had another look. I think I can make it work for windows as well without it being too horrible.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093050442
##########
gradle.properties.template:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=8
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=8
Review Comment:
Can set some different defaults, but they will only affect Windows, as they will get overridden on Linux.
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094153859
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,106 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
+org.gradle.java.installations.paths=(custom paths)
Review Comment:
Yes this needs to go away, see Lucene build.
If this is enabled Gradle prits a warning everytime as soon as you access any toolchain API.
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
+ }
+
+ // Approximate a common-sense default for running gradle/tests with parallel
+ // workers: half the count of available cpus but not more than 12.
+ var cpus = Runtime.getRuntime().availableProcessors();
+ var maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+ var testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+
+ var replacements = Map.of("org.gradle.workers.max", maxWorkers, "tests.jvms", testsJvms);
+
+ // Java properties doesn't preserve comments, so going line by line instead
+ // The properties file isn't big, nor is the map of replacements, so not worrying about speed
Review Comment:
I am also not really happy with this parsing.
My idea would be: As the template is always applied (no problem with windows or any othe roperating system), we can make the template a real template and add two search/replace markers in the file. We should then load the file into a String (UTF-8 encoding!!!) and then do String#replace('PLACEHOLDER', value). Those replaces can be done using `replacements.entries().forEach(e -> fileContent = fileContent.replace(e.getKey(), e.getValue())` (don't use replaceAll, as it uses regex and we have no forbiddenapis here to check this!!!!).
So I would add `@MAX_WORKERS@` and `@TEST_JVMS@` as map keys and replace those tags. This is like Ant did it for long time :-)
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094212834
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
+ }
+
+ // Approximate a common-sense default for running gradle/tests with parallel
+ // workers: half the count of available cpus but not more than 12.
+ var cpus = Runtime.getRuntime().availableProcessors();
+ var maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+ var testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+
+ var replacements = Map.of("org.gradle.workers.max", maxWorkers, "tests.jvms", testsJvms);
+
+ // Java properties doesn't preserve comments, so going line by line instead
+ // The properties file isn't big, nor is the map of replacements, so not worrying about speed
Review Comment:
I am also not really happy with this parsing.
My idea would be: As the template is always applied (no problem with windows or any othe roperating system), we can make the template a real template and add two search/replace markers in the file. We should then load the file into a String (UTF-8 encoding!!!) and then do `String#replace('@PLACEHOLDER@', value)`. Those replaces can be done using `replacements.entries().forEach(e -> fileContent = fileContent.replace(e.getKey(), e.getValue())` (don't use replaceAll, as it uses regex and we have no forbiddenapis here to check this!!!!).
So I would add `@MAX_WORKERS@` and `@TEST_JVMS@` as map keys and replace those tags. This is like Ant did it for long time :-)
--
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
[GitHub] [solr] dweiss commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1412153419
> We can absolutely put a real default safe (albeit slow) gradle.properties here and the existing .gitignore rule that already references gradle.properties will prevent someone from accidentally committing their changes.
I'm not sure what you mean. When you have a file that's versioned and also part of .gitignore, this file is versioned - any changes to it will be indicated in git status, git add, etc. It's also committed as if .gitignore didn't include that file. This rules out any "ignored local modifications", unfortunately.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093588158
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
For now I've put it into a new class, but I can easily move it in the wrapper downloader
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094623506
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
Remove the package completely. As this is a command line class only and it is not even compiled, theres no need to have a package.
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094212834
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
+ }
+
+ // Approximate a common-sense default for running gradle/tests with parallel
+ // workers: half the count of available cpus but not more than 12.
+ var cpus = Runtime.getRuntime().availableProcessors();
+ var maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+ var testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+
+ var replacements = Map.of("org.gradle.workers.max", maxWorkers, "tests.jvms", testsJvms);
+
+ // Java properties doesn't preserve comments, so going line by line instead
+ // The properties file isn't big, nor is the map of replacements, so not worrying about speed
Review Comment:
I am also not really happy with this parsing.
My idea would be: As the template is always applied (no problem with windows or any othe roperating system), we can make the template a real template and add two search/replace markers in the file. We should then load the *whole* file into a String (UTF-8 encoding!!!) and then do `String#replace('@PLACEHOLDER@', value)`. Those replaces can be done using `replacements.entries().forEach(e -> fileContent = fileContent.replace(e.getKey(), e.getValue())` (don't use replaceAll, as it uses regex and we have no forbiddenapis here to check this!!!!).
So I would add `@MAX_WORKERS@` and `@TEST_JVMS@` as map keys and replace those tags. This is like Ant did it for long time :-)
--
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
[GitHub] [solr] dweiss commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410977811
> I'm surprised to see the "localSettings" gradle task live on. Surely it could be replaced with a bash script?
Depends. Some values there are computed from Java defaults. I agree a modification to launch scripts is probably easier to work with though, especially because it can pick up those defaults on the first run.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093704315
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,106 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
+org.gradle.java.installations.paths=(custom paths)
Review Comment:
Actually I suppose this one should be commented out. I assume `(custom paths)` isn't a special value, it's a placeholder?
```suggestion
#org.gradle.java.installations.paths=(custom paths)
```
--
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
[GitHub] [solr] dweiss commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410944434
> In reality there should be an option in gradle to apply settings at runtime (e.g., using the global build script which sets up project structure) and allow to load custom/default settings and/or our own settings.
In reality, gradle folks are not really responsive to such requests... I know. I wish it was an option too.
--
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
[GitHub] [solr] magibney commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410950319
btw I wasn't intending to throw the scope of this PR by mentioning extracting `gradle.properties` generation into the gradlew script. I think it's still an open question whether it makes sense to pursue that. But it definitely makes sense to fix the circular dependency (as you did in aabacb56472acb573ad60b8b5d21913e3a8d0af5).
Maybe it'd make sense to proceed with a different PR for the gradlew script quesiton? (sorry for the mixed messages).
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410955343
np. I don't mind doing a separate PR if it makes it easier to discuss and get the https://github.com/apache/solr/commit/aabacb56472acb573ad60b8b5d21913e3a8d0af5 fix back in this one, or just let this one brew.
I don't think there's a huge rush to fix this so happy to do it either way
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093330100
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
+ if [ $NUM_CORES -ge 12 ]; then
+ NUM_CORES=12
+ fi
+ sed -i "s/org.gradle.workers.max=[[:digit:]]\+/org.gradle.workers.max=${NUM_CORES}/g;s/tests.jvms=[[:digit:]]\+/tests.jvms=${NUM_CORES}/g" gradle.properties
Review Comment:
This modification could be wrapped in a test for whether $NUM_CORES is in fact a positive integer, to avoid replacing default with invalid value.
--
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
[GitHub] [solr] dweiss commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411899975
> Windows as it's used much less and the need for this file is iffy. We're doing something rather unusual in Lucene & Solr -- insisting on gradle.properties when countless Gradle projects have no such mandate.
Well, depends on how you look at it. There is no way to configure reasonable static defaults for a 2-CPU laptop and a 128-core beast machine. And these "sane" defaults is what this file was originally about. If it were static, we could just commit it in to the repository (many projects do that, actually). But then folks wouldn't be able to tweak it and there's no way to split this configuration into multiple files (as far as I know, at least).
gradle.properties can also be the only (required) option to, for example, open up jdk/gradle internals to plugins like spotless (it's where the JVM setup lives).
I'd love those things to be configurable via different mechanism, but it's not something I invented or have much influence over. For portability, I'd leave those defaults inside gradle and call the generation task from gradlew if this file doesn't exist. I don't see any harm in it, to be honest.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094246681
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
+ }
+
+ // Approximate a common-sense default for running gradle/tests with parallel
+ // workers: half the count of available cpus but not more than 12.
+ var cpus = Runtime.getRuntime().availableProcessors();
+ var maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+ var testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+
+ var replacements = Map.of("org.gradle.workers.max", maxWorkers, "tests.jvms", testsJvms);
+
+ // Java properties doesn't preserve comments, so going line by line instead
+ // The properties file isn't big, nor is the map of replacements, so not worrying about speed
Review Comment:
Done
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,108 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
Review Comment:
Done
--
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
[GitHub] [solr] dweiss commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094174337
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
+ }
+
+ // Approximate a common-sense default for running gradle/tests with parallel
+ // workers: half the count of available cpus but not more than 12.
+ var cpus = Runtime.getRuntime().availableProcessors();
+ var maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+ var testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+
+ var replacements = Map.of("org.gradle.workers.max", maxWorkers, "tests.jvms", testsJvms);
+
+ // Java properties doesn't preserve comments, so going line by line instead
+ // The properties file isn't big, nor is the map of replacements, so not worrying about speed
Review Comment:
This will work for simple values (and property names) but not when property values are more complex characters - these should (in theory at least) be escaped and/or encoded to plain ascii. I don't think it's a problem for now.
##########
help/localSettings.txt:
##########
@@ -1,62 +0,0 @@
-Local developer settings
-========================
-
-The first invocation of any task in Solr gradle build will generate
Review Comment:
I think there is some value in leaving (some content) of this file under help/ - so that folks know that this file can be tweaked and is generated?
--
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
[GitHub] [solr] risdenk merged pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk merged PR #1320:
URL: https://github.com/apache/solr/pull/1320
--
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
[GitHub] [solr] uschindler commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410184811
Hi @dweiss,
I am not fully sure if this is a good idea to do it like that.
In reality there should be an option in gradle to apply settings at runtime (e.g., using the global build script which sets up project structure) and allow to load custom/default settings and/or our own settings.
--
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
[GitHub] [solr] dweiss commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092015488
##########
gradle/validation/jar-checks.gradle:
##########
@@ -313,7 +313,9 @@ configure(plist) {
.collectMany { task -> [task, task.dependsOn]}
.flatten()
.each { task ->
- task.mustRunAfter updateLicenses
+ if (task.name != 'localSettings') {
Review Comment:
Perhaps use the full path 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
[GitHub] [solr] risdenk commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092366743
##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@
Review Comment:
Not sure I understand this line with what is on lines 273 and 282 - it looks like "$@" is still used in both places? Also it doesn't look like we do the same TASK_ARGS trick for Windows if its needed?
Also does $@ need to be quoted?
--
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
[GitHub] [solr] risdenk commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093591631
##########
gradlew:
##########
@@ -163,11 +163,16 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# START OF SOLR CUSTOMIZATION
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ "$JAVACMD" $JAVA_OPTS --source 11 "$APP_HOME/buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java" "$APP_HOME/gradle/template.gradle.properties" "$APP_HOME/gradle.properties"
Review Comment:
`buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java` I think this file wasn't checked in?
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
Review Comment:
Not sure this comment is still valid.
##########
gradlew:
##########
@@ -163,11 +163,16 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# START OF SOLR CUSTOMIZATION
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ "$JAVACMD" $JAVA_OPTS --source 11 "$APP_HOME/buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java" "$APP_HOME/gradle/template.gradle.properties" "$APP_HOME/gradle.properties"
+ GENERATOR_STATUS=$?
+ if [ "$WRAPPER_STATUS" -ne 0 ]; then
+ exit $WRAPPER_STATUS
+ fi
Review Comment:
Should use `GENERATOR_STATUS`?
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092385694
##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@
Review Comment:
I'm not going to claim to be an expert at bash, so I could be wrong on the details of this. I have tested it though.
The positional parameters referenced by `$@` are mutated by the `set` command at various points in the script
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L226
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L236-L241
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L268-L273
which push more arguments onto the parameters. That's described in https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L45-L48
Initially `$@` will refer to the parameters passed in when execution of `gradlew` began, i.e. any parameters supplied by the user `./gradlew updateLicenses check -x test --dry-run` -> `updateLicenses check -x test --dry-run`. The set commands are pushing additional arguments into that, for the JRE, for the main class `org.gradle.wrapper.GradleWrapperMain`, etc.
So when https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L275 is called it contains all the arguments for executing gradle, including the user supplied arguments. However, for calling `localSettings` I don't want the user supplied arguments, so the easiest way I could see to do it without rewriting the whole script was to stash them and not append them into the arguments being built with `set`.
The Windows .bat file doesn't do any complex argument building, so I didn't need to mess around in it to get the same effect.
> Also does $@ need to be quoted?
Quoting it here appeared to quote the entire string of arguments rather than quoting the individual arguments. Maybe I'm missing a trick as that sounds like it would be the normal expansion behavior https://stackoverflow.com/a/3898681
##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@
Review Comment:
I'm not going to claim to be an expert at bash, so I could be wrong on the details of this. I have tested it though.
The positional parameters referenced by `$@` are mutated by the `set` command at various points in the script
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L226
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L236-L241
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L268-L273
which push more arguments onto the parameters. That's described in https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L45-L48
Initially `$@` will refer to the parameters passed in when execution of `gradlew` began, i.e. any parameters supplied by the user `./gradlew updateLicenses check -x test --dry-run` -> `updateLicenses check -x test --dry-run`. The set commands are pushing additional arguments into that, for the JRE, for the main class `org.gradle.wrapper.GradleWrapperMain`, etc.
So when https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L275 is called it contains all the arguments for executing gradle, including the user supplied arguments. However, for calling `localSettings` I don't want the user supplied arguments, so the easiest way I could see to do it without rewriting the whole script was to stash them and not append them into the arguments being built with `set`.
The Windows .bat file doesn't do any complex argument building, so I didn't need to mess around in it to get the same effect.
> Also does $@ need to be quoted?
Quoting it here appeared to quote the entire string of arguments rather than quoting the individual arguments. Maybe I'm missing a trick as that sounds like it would be the normal expansion behavior https://stackoverflow.com/a/3898681
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094188634
##########
help/localSettings.txt:
##########
@@ -1,62 +0,0 @@
-Local developer settings
-========================
-
-The first invocation of any task in Solr gradle build will generate
Review Comment:
And keep it wired into `gradlew :helpLocalSettings`?
--
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
[GitHub] [solr] dweiss commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1412160755
If you're concerned about startup time of that one-time "gradle localSettings" process then I'd suggest implementing this template logic/ retrieval of what's necessary as a java class - much more lightweight compared to full gradle. It's pretty much the same trick that's done with WrapperDownloader.java currently.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093688816
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.URLConnection;
+import java.nio.channels.Channels;
+import java.nio.channels.FileChannel;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.*;
Review Comment:
Yep it didn't run on them. I've run it manually for next commit.
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1412537208
It's been a bit of a rollercoaster ;)
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094528658
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
I thought about it too but thought we could leave it for another day. Although it would be natural to change when we're touching this area.
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410921599
I've moved the generation of the localSettings to gradlew as discussed on the Jira issue.
I've also removed the task dependencies between localSettings and other tasks as they're redundant when executed through gradlew.
I've also removed the CI steps that called localSettings
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094219523
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,108 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
Review Comment:
Is this the JVM that gradle itself runs under? I suppose Solr build itself will run under whatever JVM is active in the shell that starts the build?
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411849127
Hopefully we're getting there now :)
I didn't bother with trying to do variable substitution from batch as I didn't see a good + easy option. But bash will set the two parameters that were being set before.
--
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
[GitHub] [solr] dweiss commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093336382
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
I really don't want to stand in the way here but I think a java-file based solution (like WrapperDownloader) could prove to be a good middle ground between the needs to support different platforms (windows, mac, linux) and to speed up that initial gradle.properties defaults.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093554041
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
I must admit, I hadn't realized Java 11+ supported execution without compilation, I didn't pick up on that being used by gradlew. That's cool (even if it is limited to self contained source files)
I will switch to that, that's much better.
> My idea would be to modify this script/class (executed without compiling directly with java) to write the properties file.
I see [WrapperDownloader takes an arg](https://github.com/apache/solr/blob/main/buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.java#L48-L51) for the destination it downloads the gradle wrapper jar to.
Do you think it's preferable to add this functionality to the WrapperDownloader and it just grows a little, or create a separate class and invoke it on its own?
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092385694
##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@
Review Comment:
I'm not going to claim to be an expert at bash, so I could be wrong on the details of this. I have tested it though.
The positional parameters referenced by `$@` are mutated by the `set` cmd at various points in the script
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L226
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L236
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L268
which push more arguments onto the parameters. That's described in https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L45-L48
Initially `$@` will refer to the parameters passed in when execution of `gradlew` began, i.e. any parameters supplied by the user `./gradlew updateLicenses check -x test --dry-run` -> `updateLicenses check -x test --dry-run`. The set commands are pushing additional arguments into that, for the JRE, for the main class `org.gradle.wrapper.GradleWrapperMain`, etc.
So when https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L275 is called it contains all the arguments for executing gradle, including the user supplied arguments. However, for calling `localSettings` I don't want the user supplied arguments, so the easiest way I could see to do it without rewriting the whole script was to stash them and not append them into the arguments being built with `set`.
The Windows .bat file doesn't do any complex argument building, so I didn't need to mess around in it to get the same effect.
> Also does $@ need to be quoted?
Quoting it here appeared to quote the entire string of arguments rather than quoting the individual arguments. Maybe I'm missing a trick as that sounds like it would be the normal expansion behavior https://stackoverflow.com/a/3898681
--
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
[GitHub] [solr] risdenk commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1419253143
Thanks @colvinco!
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094219442
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,106 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
+org.gradle.java.installations.paths=(custom paths)
Review Comment:
This needs to be commented out (see Lucene build). It hurts when you actually want to use toolkits.
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411855713
The check on crave.io is failing to apply my commit... I'm guessing that's because I've been force pushing to my branch and it's now a bit out of sync? https://github.com/apache/solr/actions/runs/4063490165/jobs/6995782967
```
error: patch failed: gradlew.bat:80
error: gradlew.bat: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: SOLR-16632: Add core name to periodic delete related log messages (#1311)
Applying: SOLR-16641 - Generate gradle.properties from template
Patch failed at 0002 SOLR-16641 - Generate gradle.properties from template
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
```
I can create a clean PR on a fresh branch to test that, unless there's another reason for it to happen?
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094250157
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
Review Comment:
This line prints a message that it skips generation, but there is no exit, so it continues generate and overwrite...
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094650323
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
Can we spin that refactoring out in a separate PR and merge this one, now that it is stable?
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094222984
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,108 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
Review Comment:
No thats only needed for Toolkits. It is unused by Solr ant moment. IMHO, this can go away for Solr, because when you set it up you need to change it anyways.
Lucene uses it heavily to find JDK19 and JDK20 to build the MR-JAR.
--
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
[GitHub] [solr] magibney commented on pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1410290158
There was some related discussion recently on [SOLR-16634](https://issues.apache.org/jira/browse/SOLR-16634?focusedCommentId=17680614#comment-17680614) -- at the moment I'm inclined to favor @dsmiley's suggestion to possibly move localSettings into the `gradlew` wrapper script.
--
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
[GitHub] [solr] dsmiley commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411092588
I suggest for the Windows side, just check if there is no gradle.properties and log a warning pointing at the unix script to help the user craft one themselves. This is just an idea; I think we can get away with being less thorough in some fashion for Windows as it's used much less and the need for this file is iffy. We're doing something rather unusual in Lucene & Solr -- insisting on gradle.properties when countless Gradle projects have no such mandate.
A simplifying alternative would be to check gradle.properties into the project (but still marked ignored in .gitignore) and with Gradle's own defaults for number of workers & tests, or choose low ones. Advise users to edit them according to their machine. We are overthinking this stuff today! This has dev maintenance costs we pay right now. Generating this file is over-engineered IMO.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092385694
##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@
Review Comment:
I'm not going to claim to be an expert at bash, so I could be wrong on the details of this. I have tested it though.
The positional parameters referenced by `$@` are mutated by the `set` command at various points in the script
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L226
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L236-L241
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L268-L273
which push more arguments onto the parameters. That's described in https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L45-L48
Initially `$@` will refer to the parameters passed in when execution of `gradlew` began, i.e. any parameters supplied by the user `./gradlew updateLicenses check -x test --dry-run` -> `updateLicenses check -x test --dry-run`. The set commands are pushing additional arguments into that, for the JRE, for the main class `org.gradle.wrapper.GradleWrapperMain`, etc.
So when https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L275 is called it contains all the arguments for executing gradle, including the user supplied arguments. However, for calling `localSettings` I want the arguments it has built up, but I don't want the user supplied arguments, so the easiest way I could see to do it without rewriting the whole script was to stash them somewhere else and not append them into the arguments being built with `set`.
The Windows .bat file doesn't do any complex argument building, so I didn't need to mess around in it to get the same effect.
> Also does $@ need to be quoted?
Quoting it here appeared to quote the entire string of arguments rather than quoting the individual arguments. Maybe I'm missing a trick as that sounds like it would be the normal expansion behavior https://stackoverflow.com/a/3898681
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Fix circular task dependency for gradle.properties
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1092385694
##########
gradlew:
##########
@@ -163,11 +163,8 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
-if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
-fi
+# Hold on to the original arguments, don't want to pass them in when generating gradle.properties
+TASK_ARGS=$@
Review Comment:
I'm not going to claim to be an expert at bash, so I could be wrong on the details of this. I have tested it though.
The positional parameters referenced by `$@` are mutated by the `set` command at various points in the script
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L226
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L236
https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L268
which push more arguments onto the parameters. That's described in https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L45-L48
Initially `$@` will refer to the parameters passed in when execution of `gradlew` began, i.e. any parameters supplied by the user `./gradlew updateLicenses check -x test --dry-run` -> `updateLicenses check -x test --dry-run`. The set commands are pushing additional arguments into that, for the JRE, for the main class `org.gradle.wrapper.GradleWrapperMain`, etc.
So when https://github.com/apache/solr/blob/90acc20ef22680966e6708f8985f7e110e6a7700/gradlew#L275 is called it contains all the arguments for executing gradle, including the user supplied arguments. However, for calling `localSettings` I don't want the user supplied arguments, so the easiest way I could see to do it without rewriting the whole script was to stash them and not append them into the arguments being built with `set`.
The Windows .bat file doesn't do any complex argument building, so I didn't need to mess around in it to get the same effect.
> Also does $@ need to be quoted?
Quoting it here appeared to quote the entire string of arguments rather than quoting the individual arguments. Maybe I'm missing a trick as that sounds like it would be the normal expansion behavior https://stackoverflow.com/a/3898681
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411718416
> A simplifying alternative would be to check gradle.properties into the project (but still marked ignored in .gitignore) and with Gradle's own defaults for number of workers & tests, or choose low ones. Advise users to edit them according to their machine. We are overthinking this stuff today! This has dev maintenance costs we pay right now. Generating this file is over-engineered IMO.
I would create a `gradle.properties.template` file that lives in source control and copy it to the ignored `gradle.properties` path. That way it's easy to update the template without messing around with gitignore.
I've taken a closer look at the properties file. Currently I see that only two of the settings are generated dynamically, and the rest are all hardcoded.
```
// Approximate a common-sense default for running gradle/tests with parallel
// workers: half the count of available cpus but not more than 12.
def cpus = Runtime.runtime.availableProcessors()
def maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12))
def testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12))
...
# Maximum number of parallel gradle workers.
org.gradle.workers.max=${maxWorkers}
# Maximum number of test JVMs forked per test task.
tests.jvms=${testsJvms}
```
Getting the number of "processors" (depending on whether hyperthreading counts...) shouldn't be a problem on bash, https://stackoverflow.com/questions/6481005/how-to-obtain-the-number-of-cpus-cores-in-linux-from-the-command-line, so I could still seed in those values.
I'm using Ubuntu in WSL:
```
me:~$ getconf _NPROCESSORS_ONLN
16
me:~$ nproc
16
me:~$ grep -c ^processor /proc/cpuinfo
16
me:~$ grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}'
8
```
For Windows it's also possible: https://pureinfotech.com/check-many-cores-processor-windows-10/
```
wmic cpu get NumberOfCores,NumberOfLogicalProcessors /format:value
NumberOfCores=8
NumberOfLogicalProcessors=16
```
`Runtime.runtime.availableProcessors()` gives the same answer as `nproc`, but it was halved (and capped to 12). So I could just use the number of cores, or divide `nproc`.
Doing the string replacement when copying the file in bash shouldn't be a problem, I'll see if I can do it sensibly in batch as well.
--
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
[GitHub] [solr] risdenk commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "risdenk (via GitHub)" <gi...@apache.org>.
risdenk commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093601498
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.URLConnection;
+import java.nio.channels.Channels;
+import java.nio.channels.FileChannel;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.*;
Review Comment:
nit: I'm guessing that spotless doesn't run against these files (thats fine) but usually we avoid wildcard imports.
##########
gradle/generation/local-settings.gradle:
##########
@@ -45,83 +45,4 @@ configure(rootProject) {
}
}
}
-
Review Comment:
@colvinco I think this is still outstanding
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.net.URLConnection;
+import java.nio.channels.Channels;
+import java.nio.channels.FileChannel;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.*;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+import static java.nio.file.StandardOpenOption.APPEND;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ * <p>
+ * Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
+ }
+
+ // Approximate a common-sense default for running gradle/tests with parallel
+ // workers: half the count of available cpus but not more than 12.
+ var cpus = Runtime.getRuntime().availableProcessors();
+ var maxWorkers = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+ var testsJvms = (int) Math.max(1d, Math.min(cpus * 0.5d, 12));
+
+ var replacements = Map.of("org.gradle.workers.max", maxWorkers, "tests.jvms", testsJvms);
+
+ // Java properties doesn't preserve comments, so going line by line instead
+ // The properties file isn't big, nor is the map of replacements, so not worrying about speed here.
+ System.out.println("Generating gradle.properties");
+ final var lines = Files.readAllLines(source).stream().map(line -> {
+ if (!line.startsWith("#") && line.contains("=")) {
+ final String key = line.split("=")[0];
+ var value = replacements.get(key);
+ if (value != null) {
+ final String newLine = key + '=' + value;
+ System.out.println("Setting " + newLine);
+ return newLine;
+ }
Review Comment:
nit: indent is off
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
Review Comment:
Yea my comment was about `configuring this file` - agree the concepts are good. I'm not saying we need to change this completely, but it would be good to consolidate to avoid having to go look another place.
##########
.gitignore:
##########
@@ -38,6 +38,7 @@ __pycache__
# Ignore lucene included build
lucene/
Review Comment:
so I thought the `lucene/` entry was in case of having local Lucene development? It might be worth changing to `/lucene/` or something similar that matches top level only?
I think its a bit surprising that `lucene/` matches any folder in the file structure.
--
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
[GitHub] [solr] madrob commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "madrob (via GitHub)" <gi...@apache.org>.
madrob commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094480535
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
Just because this was based on a Lucene class doesn't mean this is a Lucene class. Should probably be packaged under o.a.solr.gradle?
--
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
[GitHub] [solr] uschindler commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411867969
> The check on crave.io is failing to apply my commit... I'm guessing that's because I've been force pushing to my branch and it's now a bit out of sync? https://github.com/apache/solr/actions/runs/4063490165/jobs/6995782967
>
> ```
> error: patch failed: gradlew.bat:80
> error: gradlew.bat: patch does not apply
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> Applying: SOLR-16632: Add core name to periodic delete related log messages (#1311)
> Applying: SOLR-16641 - Generate gradle.properties from template
> Patch failed at 0002 SOLR-16641 - Generate gradle.properties from template
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> ```
>
> I can create a clean PR on a fresh branch to test that, unless there's another reason for it to happen?
Don't force-push in PRs :-)
PRs are anyways squash merged, so no need to squash them before. It also makes it hard to review when you do changes.
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411869912
> Don't force-push in PRs :-)
>
> PRs are anyways squash merged, so no need to squash them before. It also makes it hard to review when you do changes.
Yep, I normally I wouldn't. But because the solution changed so drastically it seemed better this time around.
Good to hear that squash merge is now the default/only option on here. I had a previous PR that got merged in with a standard merge last year, which was a bit messy :)
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093452719
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
oh did not see Dawid's comment. Two persons, same idea.
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094255817
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
Review Comment:
It would never happen, since in `gradlew` you only run this if it does not exist... Anyway...
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1411028354
It's also having to "write it twice" for the Windows .bat as long as it's still supported for dev. (Though with WSL becoming more prevalent that's a problem that will solve itself eventually).
--
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
[GitHub] [solr] dsmiley commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1412104815
> without messing around with gitignore
What many people don't realize about `.gitignore` is that it does *not* prevent the file being a part of the repo (i.e. committed & pulled). We can absolutely put a real default safe (albeit slow) `gradle.properties` here and the *existing* `.gitignore` rule that *already* references `gradle.properties` will prevent someone from _accidentally_ committing their changes.
--
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
[GitHub] [solr] dweiss commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094191997
##########
help/localSettings.txt:
##########
@@ -1,62 +0,0 @@
-Local developer settings
-========================
-
-The first invocation of any task in Solr gradle build will generate
Review Comment:
Sorry - I thought I removed that comment. This is duplicated in the template now so I think it's fine to be removed.
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093417783
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
aren't we not executing a small java script anyways from Gradlew? I think it mainly downloads gradle distribution.
My idea would be to modify this script/class (executed without compiling directly with java) to write the properties file.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093704315
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,106 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
+org.gradle.java.installations.paths=(custom paths)
Review Comment:
Actually I suppose this one should be commented out. I assume `(custom paths)` isn't a special value, it's a placeholder?
```suggestion
# org.gradle.java.installations.paths=(custom paths)
```
--
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
[GitHub] [solr] magibney commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094488554
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
+1 -- analogous to [this namespace issue](https://github.com/apache/solr/pull/1323/commits/600732f558e3052f5569ab3d189365f9b1edb5d3). Not sure whether there are practical implications to the naming in this case, but regardless, solr package makes sense 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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093596944
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
Review Comment:
That command will still give the help output from https://github.com/apache/solr/blob/main/help/localSettings.txt which describes the concepts, rather than the original gradle task.
The content could be moved into the template file as a comment instead though
--
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
[GitHub] [solr] dweiss commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094645456
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
I think the reason it was originally placed there was so that it lived together with other "sources" - so that any validation checks, formatting, etc. would apply. I don't think we actually do it to buildSrc (haven't checked) but we could, in which case it'd be easier to maintain it as part of buildSrc than in some ad-hoc location.
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094232024
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,108 @@
+#############################
+# Local developer settings #
+#############################
+#
+# The first invocation of any task in Solr gradlew will generate and save this project-local 'gradle.properties' file.
+# This file contains the defaults you may (but don't have to) tweak for your particular hardware (or taste). Note there
+# are certain settings in that file that may be _required_ at runtime for certain plugins (an example is the spotless/
+# google java format plugin, which requires adding custom exports to JVM modules). gradlew only generates this file
+# if it's not already present (it never overwrites the defaults) -- occasionally you may have to manually delete (or move)
+# this file and regenerate from scratch.
+#
+# This is an overview of some of these settings.
+#
+###############
+# Parallelism #
+###############
+#
+# Gradle build can run tasks in parallel but by default it consumes all CPU cores which
+# is too optimistic a default for Solr tests. You can disable the parallelism
+# entirely or assign it a 'low' priority with these properties:
+#
+# org.gradle.parallel=[true, false]
+# org.gradle.priority=[normal, low]
+#
+# The default level of parallelism is computed based on the number of cores on
+# your machine (on the first run of gradle build). By default these are fairly conservative
+# settings (half the number of cores for workers, for example):
+#
+# org.gradle.workers.max=[X]
+# tests.jvms=[N <= X]
+#
+# The number of test JVMs can be lower than the number of workers: this just means
+# that two projects can run tests in parallel to saturate all the workers. The I/O and memory
+# bandwidth limits will kick in quickly so even if you have a very beefy machine bumping
+# it too high may not help.
+#
+# You can always override these settings locally using command line as well:
+# gradlew -Ptests.jvms=N --max-workers=X
+#
+#############
+# Test JVMS #
+#############
+#
+# Test JVMs have their own set of arguments which can be customized. These are configured
+# separately from the gradle workers, for example:
+#
+# tests.jvms=3
+# tests.heapsize=512m
+# tests.minheapsize=512m
+# tests.jvmargs=-XX:+UseParallelGC -XX:TieredStopAtLevel=1 -XX:ActiveProcessorCount=1
+#
+#################
+# Gradle Daemon #
+#################
+#
+# The gradle daemon is a background process that keeps an evaluated copy of the project
+# structure, some caches, etc. It speeds up repeated builds quite a bit but if you don't
+# like the idea of having a (sizeable) background process running in the background,
+# disable it.
+#
+# org.gradle.daemon=[true, false]
+# org.gradle.jvmargs=...
+#############################################################################################
+
+# UTF-8 as standard file encoding
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=2
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=2
+
+# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
+org.gradle.java.installations.auto-download=false
+
+# Set these to enable automatic JVM location discovery.
+org.gradle.java.installations.fromEnv=JDK11,JDK12,JDK13,JDK14,JDK15,JDK16,JDK17
Review Comment:
Yea, I was unaware of toolchains and read up a bit. If unused by Solr the lines should just go away until we start using it...
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094691343
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
I would keep the lucene namespace as at moment this makes merging cross source trees easier (I would like to port this over to Lucene, so cherry picking this commit works from there if the path names are similar). At moment I try to port most lucene and solr gradle build imüprovements to the other project, so aligned path names makes this easier.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1095682367
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,72 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
Review Comment:
Does someone want to merge it then? :)
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094251846
##########
buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * 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.
+ */
+package org.apache.lucene.gradle;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+
+/**
+ * Standalone class that generates a populated gradle.properties from a template.
+ *
+ * <p>Has no dependencies outside of standard java libraries
+ */
+public class GradlePropertiesGenerator {
+ public static void main(String[] args) {
+ if (args.length != 2) {
+ System.err.println("Usage: java GradlePropertiesGenerator.java <source> <destination>");
+ System.exit(2);
+ }
+
+ try {
+ new GradlePropertiesGenerator().run(Paths.get(args[0]), Paths.get(args[1]));
+ } catch (Exception e) {
+ System.err.println("ERROR: " + e.getMessage());
+ System.exit(3);
+ }
+ }
+
+ public void run(Path source, Path destination) throws IOException {
+ if (!Files.exists(source)) {
+ throw new IOException("template file not found: " + source);
+ }
+ if (Files.exists(destination)) {
+ System.out.println(destination + " already exists, skipping generation.");
Review Comment:
Woops :)
--
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
[GitHub] [solr] uschindler commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1419329442
Thanks, I created https://github.com/apache/lucene/pull/12131 for Apache Lucene (cherrypicking was easy). Please have a look.
--
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
[GitHub] [solr] janhoy commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1416039622
Please add a line to `CHANGES.txt` for 9.2 release. It can be useful for folks to get alerted about this change.
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093324267
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
Also, won't work on macOS :(
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093595480
##########
gradlew:
##########
@@ -163,11 +163,16 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# START OF SOLR CUSTOMIZATION
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ "$JAVACMD" $JAVA_OPTS --source 11 "$APP_HOME/buildSrc/src/main/java/org/apache/lucene/gradle/GradlePropertiesGenerator.java" "$APP_HOME/gradle/template.gradle.properties" "$APP_HOME/gradle.properties"
Review Comment:
The gitignore was ignoring everything lucene/. I've updated it to not ignore buildSrc\src\main\java\org\apache\lucene now
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093596944
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
Review Comment:
That command will still give the help output from https://github.com/apache/solr/blob/main/help/localSettings.txt which describes the concepts, rather than the original gradle task.
It could be moved into the template file as a comment instead though
--
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
[GitHub] [solr] colvinco commented on pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1320:
URL: https://github.com/apache/solr/pull/1320#issuecomment-1412153070
Yep I used to do that, but it makes it (slightly) harder to update the defaults in the future and can be confusing because people don't expect gitignored files to be in git - as you say people don't realize that it's an option. It also means that if you copy the source of a repo (e.g. from https://github.com/apache/solr/archive/refs/heads/main.zip), and then create a new repo from it you lose the ignored files.
So that's why I copy templates instead. But I don't mind doing whatever people prefer 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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093698018
##########
gradle/template.gradle.properties:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
Review Comment:
I've moved the help content into the template so it's all self contained
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093064393
##########
gradle/generation/local-settings.gradle:
##########
@@ -45,83 +45,4 @@ configure(rootProject) {
}
}
}
-
Review Comment:
Please keep the "deprecated" localSettings task fora while as all jenkins/github jobs execute them. We can remove it later, but for now add a "fake" task for compatibility.
##########
gradle.properties.template:
##########
@@ -0,0 +1,43 @@
+# See gradlew :helpLocalSettings for more information on configuring this file.
+systemProp.file.encoding=UTF-8
+
+# Set up gradle JVM defaults.
+#
+# We also open up internal compiler modules for spotless/ google java format.
+org.gradle.jvmargs=-Xmx1g -XX:TieredStopAtLevel=1 -XX:+UseParallelGC -XX:ActiveProcessorCount=1 \
+ --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
+ --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+
+# Run at normal priority, in parallel
+org.gradle.parallel=true
+org.gradle.priority=normal
+
+# This setting enables local task output caches. This will speed up
+# your local builds in most cases but will also consume disk space in your
+# gradle home. See SOLR-15603 for details.
+# org.gradle.caching=true
+
+# Silence gradle warnings. We'll deal with them when we upgrade the wrapper.
+org.gradle.warning.mode=none
+
+# You may disable the background daemon if it consumes too much memory.
+org.gradle.daemon=true
+# timeout after 15 mins of inactivity.
+org.gradle.daemon.idletimeout=900000
+
+# Maximum number of parallel gradle workers.
+org.gradle.workers.max=8
+
+# Maximum number of test JVMs forked per test task.
+tests.jvms=8
Review Comment:
Please use a low value like "2" as default (also for max workers). If I start gradle on my laptop i dont want the machine to crush.
--
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
[GitHub] [solr] uschindler commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093064393
##########
gradle/generation/local-settings.gradle:
##########
@@ -45,83 +45,4 @@ configure(rootProject) {
}
}
}
-
Review Comment:
Please keep the "deprecated" localSettings task for a while as all jenkins/github jobs execute them. We can remove it later, but for now add a "fake" task for compatibility.
--
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
[GitHub] [solr] dweiss commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "dweiss (via GitHub)" <gi...@apache.org>.
dweiss commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1093100197
##########
gradlew:
##########
@@ -163,10 +163,18 @@ fi
CLASSPATH=$GRADLE_WRAPPER_JAR
-# Don't fork a daemon mode on initial run that generates local defaults.
-GRADLE_DAEMON_CTRL=
+# Generate gradle.properties if they don't exist
if [ ! -e "$APP_HOME/gradle.properties" ]; then
- GRADLE_DAEMON_CTRL=--no-daemon
+ echo "Populating gradle.properties from gradle.properties.template."
+ # Do the copy first. If the next steps fail, the gradle.properties will still be usable
+ cp gradle.properties.template gradle.properties
+ NUM_CORES=$(grep ^cpu\\scores /proc/cpuinfo | uniq | awk '{print $4}')
Review Comment:
Honestly, and speaking also as a Windows user, I don't see how putting all this logic into gradlew makes it a better solution. I preferred one code that generated those defaults - it could still be externalized and pattern-replaced but it'd at least work for everyone the same way.
--
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
[GitHub] [solr] colvinco commented on a diff in pull request #1320: SOLR-16641 - Generate gradle.properties from gradlew
Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1320:
URL: https://github.com/apache/solr/pull/1320#discussion_r1094197600
##########
help/localSettings.txt:
##########
@@ -1,62 +0,0 @@
-Local developer settings
-========================
-
-The first invocation of any task in Solr gradle build will generate
Review Comment:
Ah bad timing. I can revert that last commit I just put in, or leave it. It might still be a useful signpost
--
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