Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16641 - Generate gradle.properties from gradlew #1320

Merged
merged 11 commits into from Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/bin-solr-test.yml
Expand Up @@ -34,8 +34,6 @@ jobs:
restore-keys: |
${{ runner.os }}-gradle-binsolr-
${{ runner.os }}-gradle-
- name: Initialize gradle settings
run: ./gradlew localSettings
- name: Test the bin/solr script
run: ./gradlew integrationTests
- name: Archive logs
Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/docker-test.yml
Expand Up @@ -42,8 +42,6 @@ jobs:
restore-keys: |
${{ runner.os }}-gradle-docker-
${{ runner.os }}-gradle-
- name: Initialize gradle settings
run: ./gradlew localSettings
- name: Build Docker image with Gradle
run: ./gradlew solr:docker:docker
- name: Run tests on Docker image
Expand Down
3 changes: 0 additions & 3 deletions .github/workflows/gradle-precommit.yml
Expand Up @@ -34,9 +34,6 @@ jobs:
${{ runner.os }}-gradle-precommit-
${{ runner.os }}-gradle-

- name: Initialize gradle settings
run: ./gradlew localSettings

- name: Run gradle check (without tests)
run: ./gradlew check -x test -Ptask.times=true

Expand Down
2 changes: 0 additions & 2 deletions .github/workflows/solrj-test.yml
Expand Up @@ -33,7 +33,5 @@ jobs:
restore-keys: |
${{ runner.os }}-gradle-solrj-
${{ runner.os }}-gradle-
- name: Initialize gradle settings
run: ./gradlew localSettings
- name: Test the SolrJ Package
run: ./gradlew solr:solrj:test
2 changes: 1 addition & 1 deletion .gitignore
Expand Up @@ -37,7 +37,7 @@ __pycache__
*~

# Ignore lucene included build
lucene/
/lucene

# Ignore gradle wrapper
gradle/wrapper/gradle-wrapper.jar
Expand Down
@@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 -- analogous to this namespace issue. Not sure whether there are practical implications to the naming in this case, but regardless, solr package makes sense here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we spin that refactoring out in a separate PR and merge this one, now that it is stable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does someone want to merge it then? :)


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.");
janhoy marked this conversation as resolved.
Show resolved Hide resolved
return;
}

// 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("@MAX_WORKERS@", maxWorkers, "@TEST_JVMS@", testsJvms);

System.out.println("Generating gradle.properties");
String fileContent = Files.readString(source, StandardCharsets.UTF_8);
for (var entry : replacements.entrySet()) {
fileContent = fileContent.replace(entry.getKey(), String.valueOf(entry.getValue()));
}
Files.writeString(
destination, fileContent, StandardCharsets.UTF_8, StandardOpenOption.CREATE_NEW);
}
}
1 change: 1 addition & 0 deletions dev-docs/how-to-contribute.adoc
Expand Up @@ -17,6 +17,7 @@ In order to make a new contribution to Solr you will use the fork you have creat
1. Create a new Jira issue in the Solr project: https://issues.apache.org/jira/projects/SOLR/issues
2. Create a new branch in your Solr fork to provide a PR for your contribution on the newly created issue. Make any necessary changes for the given bug/feature in that branch. You can use additional information in these dev-docs to build and test your code as well as ensure it passes code quality checks.
3. Once you are satisfied with your changes, get your branch ready for a PR by running `./gradlew tidy updateLicenses check -x test`. This will format your source code, update licenses of any dependency version changes and run all pre-commit tests. Commit the changes.
* Note: the `check` command requires `perl` and `python3` to be present on your `PATH` to validate documentation.
4. Open a PR of your branch against the `main` branch of the apache/solr repository. When you open a PR on your fork, this should be the default option.
* The title of your PR should include the Solr Jira issue that you opened, i.e. `SOLR-12345: New feature`.
* The PR description will automatically populate with a pre-set template that you will need to fill out.
Expand Down
2 changes: 2 additions & 0 deletions dev-docs/solr-source-code.adoc
Expand Up @@ -38,6 +38,8 @@ If you want to build the documentation, type `./gradlew -p solr documentation`.

`./gradlew check` will assemble Solr and run all validation tasks unit tests.

NOTE: the `check` command requires `perl` and `python3` to be present on your `PATH` to validate documentation.

To build the final Solr artifacts run `./gradlew assemble`.

Lastly, there is developer oriented documentation in `./dev-docs/README.adoc` that
Expand Down
2 changes: 0 additions & 2 deletions dev-tools/scripts/releaseWizard.yaml
Expand Up @@ -356,8 +356,6 @@ groups:
- !Command
cmd: git pull --ff-only
stdout: true
- !Command
cmd: "{{ gradle_cmd }} localSettings"
- !Command
cmd: "{{ gradle_cmd }} clean check -x test"
- !Todo
Expand Down
81 changes: 3 additions & 78 deletions gradle/generation/local-settings.gradle
Expand Up @@ -41,87 +41,12 @@ configure(rootProject) {
throw new GradleException(
"Certain gradle tasks and plugins require access to jdk.compiler" +
" internals, your gradle.properties might have just been generated or could be" +
" out of sync (see help/localSettings.txt)")
}
}
}

colvinco marked this conversation as resolved.
Show resolved Hide resolved
task localSettings() {
doFirst {
// If we don't have the defaults yet, create them.
if (hasDefaults) {
logger.lifecycle("Local settings already exist, skipping generation.")
} else {
// 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))

// Write the defaults for this machine.
rootProject.file("gradle.properties").write("""
# These settings have been generated automatically on the first run.
# See gradlew :helpLocalSettings for more information.
systemProp.file.encoding=UTF-8

# 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=${maxWorkers}

# Maximum number of test JVMs forked per test task.
tests.jvms=${testsJvms}

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

""", "UTF-8")

logger.log(LogLevel.WARN, "\nIMPORTANT. This is the first time you ran the build. " +
"I wrote some sane defaults (for this machine) to 'gradle.properties', " +
"they will be picked up on consecutive gradle invocations (not this one).\n\n" +
"Run gradlew :helpLocalSettings for more information.")
" out of sync (see gradle/template.gradle.properties)")
}
}
}
}

if (!hasDefaults) {
// Make all tasks depend on local setup to make sure it'll run.
allprojects {
tasks.all { task ->
if (task != rootProject.localSettings) {
task.dependsOn rootProject.localSettings
}
}
}
task localSettings() {
// This is just a placeholder until all references to the localSettings task are removed #SOLR-16641
}
102 changes: 102 additions & 0 deletions gradle/template.gradle.properties
@@ -0,0 +1,102 @@
#############################
# 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=@MAX_WORKERS@

# Maximum number of test JVMs forked per test task.
tests.jvms=@TEST_JVMS@

# Disable auto JVM provisioning (we don't use toolchains yet but want no surprises).
org.gradle.java.installations.auto-download=false
4 changes: 2 additions & 2 deletions gradle/validation/gradlew-scripts-tweaked.gradle
Expand Up @@ -32,8 +32,8 @@ configure(rootProject) {
doFirst {
scripts.each { file ->
def content = new String(file.readBytes(), StandardCharsets.US_ASCII)
if (content.indexOf("Don't fork a daemon mode on initial run that generates local defaults") < 0) {
throw new GradleException("Launch script ${file} does not have a manual daemon tweak (see LUCENE-9232).")
if (content.indexOf("GradlePropertiesGenerator") < 0) {
throw new GradleException("Launch script ${file} does not have a tweak to generate gradle.properties (see SOLR-16641).")
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions gradlew
Expand Up @@ -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"
risdenk marked this conversation as resolved.
Show resolved Hide resolved
GENERATOR_STATUS=$?
if [ "$GENERATOR_STATUS" -ne 0 ]; then
exit $GENERATOR_STATUS
fi
fi
# END OF SOLR CUSTOMIZATION

# Increase the maximum file descriptors if we can.
if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
Expand Down Expand Up @@ -237,7 +242,6 @@ set -- \
"-Dorg.gradle.appname=$APP_BASE_NAME" \
-classpath "$CLASSPATH" \
org.gradle.wrapper.GradleWrapperMain \
$GRADLE_DAEMON_CTRL \
"$@"

# Stop when "xargs" is not available.
Expand Down