You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/03/30 18:56:32 UTC
[GitHub] [lucene-solr] madrob opened a new pull request #1390: LUCENE-9266
remove gradle wrapper jar from source
madrob opened a new pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390
ASF Release Policy states that we cannot have binary JAR files checked
in to our source releases, a few other projects have solved this by
modifying their generated gradlew scripts to download a copy of the
wrapper jar.
This implementation is heavily based on the one found in KAFKA-1714
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401829201
##########
File path: solr/build.xml
##########
@@ -459,7 +459,7 @@
fullpath="${fullnamever}/solr/LUCENE_CHANGES.txt" />
<tarfileset dir="${src.export.dir}"
prefix="${fullnamever}"
- excludes="solr/example/**/*.sh solr/example/**/bin/ solr/scripts/**"/>
Review comment:
Indent different for some reason?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401172383
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
Review comment:
I think it should be exactly the same version as the one we "require". Same as with JAR dependencies. If the checksum of the existing (or downloaded) file doesn't match, we can issue a warning and overwrite it with a newly downloaded one (after verifying its checksum). This gives us the liberty to update gradle wrapper on different branches, for example: if you switch branches, the wrapper would be re-downloaded and upgraded for you to match the branch's required version.
I specifically don't *want* people to have the liberty of running arbitrary gradle versions (or wrapper versions). This is where I got burned many times before. If we are to drop versioned wrapper jar it has to be matching exactly what is required by each git revision of the code (so that branch switching works seamlessly and everyone is using the same version on the same git revision).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-607953114
Committed in e25ab4204
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400724627
##########
File path: gradlew.bat
##########
@@ -81,6 +81,12 @@ set CMD_LINE_ARGS=%*
set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar
+@rem LUCENE-9266 Download the gradle wrapper jar if we don't have one
+set XSUM=28b330c20a9a73881dfe9702df78d4d78bf72368e8906c70080ab6932462fe9e
+"%JAVA_EXE%" buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.java 6.0.1 "%CLASSPATH%" %XSUM%
+
+@rem TODO check output of previous command and maybe fail
Review comment:
https://ss64.com/nt/errorlevel.html
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-606473807
I like the direction, Mike. If we can keep gradle wrapper jar in the repository (see that comment on legal jira) then this patch could be easily extended to make the whole download check conditional and only applicable if you're running a stand-alone source release bundle. This is definitely a follow-up work.
I can try to help out with Windows testing and maybe cosmetics; let me know if you want me to file a pr against your repo or attach a patch when you're ready.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401165360
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
Review comment:
I'm afraid to ask what happened. :) I'd bump the limit to a reasonable 500K just in case the wrapper grows and we forget about the check...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401164140
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
+
+ File temp = Files.createTempFile("gradle-wrapper", null).toFile();
+ try (ReadableByteChannel in = Channels.newChannel(url.openStream());
+ FileOutputStream out = new FileOutputStream(temp)) {
+ out.getChannel().transferFrom(in, 0, maxSize);
+ }
+
+ try (FileInputStream fis = new FileInputStream(temp)) {
+ MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
+
+ // Convert binary digest to hex checksum value
+ // This will strip leading zeroes, deal with that when we need to
+ String sha256sum = new BigInteger(1, sha256.digest(fis.readAllBytes())).toString(16);
+
+ if (args[2].equals(sha256sum)) {
+ if (!temp.renameTo(jar)) {
+ System.err.println("Failed renaming wrapper jar, leaving download at " + temp);
Review comment:
The user might be able to manually move it into place since we've already validated the checksum. I'll update the error.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400719401
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
Review comment:
Do you think we really need this check? I'd say simplify and just url.openStream().transferTo(outputStream).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401163433
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
Review comment:
I'd like to have the check, yes. Got burned by something like this once, and never again :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400482417
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
There is this discussion from 2014 https://lists.apache.org/thread.html/e19b21cadfa4efa67b8e9cf6f6425bc4c6714a947d11273c2fe8e9b2%401402431481%40%3Cgeneral.incubator.apache.org%3E
There is extant solution in Apache Groovy to use an already installed gradle to create the wrapper, but only for building src releases https://github.com/apache/groovy#bootstrapping-gradle
And it looks like there is no interest in allowing the wrapper jar in source releases via https://issues.apache.org/jira/browse/LEGAL-288
I'll cross-post this to the JIRA because I think this is important research to show.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400482408
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
But then, see this:
https://github.com/search?q=org%3Aapache+filename%3Agradle-wrapper.jar
Maybe we're just trying to do too much... It really is making a simple and reliable thing a lot more complicated than it needs to be.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400452053
##########
File path: .github/workflows/gradle-precommit.yml
##########
@@ -19,5 +19,8 @@ jobs:
java-version: 11
- name: Grant execute permission for gradlew
run: chmod +x gradlew
+ - name: Run something to download the gradle wrapper jar
Review comment:
It does work out of the box. The reason that I added an intermediate step was so that we can run the validator before running the rest of precommit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400450976
##########
File path: gradlew
##########
@@ -80,6 +80,19 @@ case "`uname`" in
;;
esac
+
+# Loop in case we encounter an error.
Review comment:
The simplest workaround: a simple Java class that can be compiled on demand that would take an URL as an argument and place it in the given location. We do assume javac is available.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob closed pull request #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob closed pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400723323
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
Review comment:
I think we should check the checksum even if the file exists. If the checksum doesn't match then we can try to download again. This sort of solves the wrapper-upgrade issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400481056
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
https://issues.apache.org/jira/browse/LEGAL-288
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-607423166
bq. Your patch moved the *nix invocation of WrapperDownloader to before we set JAVA_CMD, when it still needed to be after. I switched that back, but kept your change of setting the CLASSPATH after we download the new gradle-wrapper.jar
I should have checked, sorry. I had a million things to do today.
bq. In your Windows patch you use backslashes for GRADLE_WRAPPER_JAR but forward slashes for the JAVA_EXE command - is that fine/expected/necessary?
Neither matters on modern Java versions. You can switch them to windows path separators if you wish but you don't have to.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-606832106
Realistically, I don't think anybody will. Rather, they'll retry the
script. If something doesn't work with move it's probably fatal
anyway.
On Tue, Mar 31, 2020 at 9:34 PM Mike Drob <no...@github.com> wrote:
>
> @madrob commented on this pull request.
>
> ________________________________
>
> In buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.java:
>
> > + File temp = Files.createTempFile("gradle-wrapper", null).toFile();
> + try (ReadableByteChannel in = Channels.newChannel(url.openStream());
> + FileOutputStream out = new FileOutputStream(temp)) {
> + out.getChannel().transferFrom(in, 0, maxSize);
> + }
> +
> + try (FileInputStream fis = new FileInputStream(temp)) {
> + MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
> +
> + // Convert binary digest to hex checksum value
> + // This will strip leading zeroes, deal with that when we need to
> + String sha256sum = new BigInteger(1, sha256.digest(fis.readAllBytes())).toString(16);
> +
> + if (args[2].equals(sha256sum)) {
> + if (!temp.renameTo(jar)) {
> + System.err.println("Failed renaming wrapper jar, leaving download at " + temp);
>
> The user might be able to manually move it into place since we've already validated the checksum. I'll update the error.
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400718042
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
Review comment:
NIO maybe? Paths.get(.). Files.exists()... these give a lot more reasonable exception 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400721515
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
+
+ File temp = Files.createTempFile("gradle-wrapper", null).toFile();
+ try (ReadableByteChannel in = Channels.newChannel(url.openStream());
+ FileOutputStream out = new FileOutputStream(temp)) {
+ out.getChannel().transferFrom(in, 0, maxSize);
+ }
+
+ try (FileInputStream fis = new FileInputStream(temp)) {
+ MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
+
+ // Convert binary digest to hex checksum value
+ // This will strip leading zeroes, deal with that when we need to
+ String sha256sum = new BigInteger(1, sha256.digest(fis.readAllBytes())).toString(16);
+
+ if (args[2].equals(sha256sum)) {
+ if (!temp.renameTo(jar)) {
Review comment:
Rename won't work on Windows between drives. I'd do a copy and delete temp. If you want to do a rename then you'd have to create that temp file in the same directory as gradle wrapper is expected to finally be located.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400803187
##########
File path: .github/workflows/gradle-precommit.yml
##########
@@ -19,5 +19,8 @@ jobs:
java-version: 11
- name: Grant execute permission for gradlew
run: chmod +x gradlew
+ - name: Run something to download the gradle wrapper jar
Review comment:
Oh.. now I see which validator you had in mind. Sorry for being dim.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400486453
##########
File path: .github/workflows/gradle-precommit.yml
##########
@@ -19,5 +19,8 @@ jobs:
java-version: 11
- name: Grant execute permission for gradlew
run: chmod +x gradlew
+ - name: Run something to download the gradle wrapper jar
Review comment:
validator will verify the checksum, which I felt was important before we go on to running the rest of precommit. Maybe my threat model is wrong here? We can move validator action to after precommit in that case?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400447186
##########
File path: .github/workflows/gradle-precommit.yml
##########
@@ -19,5 +19,8 @@ jobs:
java-version: 11
- name: Grant execute permission for gradlew
run: chmod +x gradlew
+ - name: Run something to download the gradle wrapper jar
Review comment:
This has to work out of the box for ./gradlew precommit. You shouldn't need to run anything prior to that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400450439
##########
File path: gradlew
##########
@@ -80,6 +80,19 @@ case "`uname`" in
;;
esac
+
+# Loop in case we encounter an error.
Review comment:
This loop should terminate with an error if the wrapper can't be downloaded. A windows version would also be necessary. The problem is that Windows won't have curl so some other workaround is needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400454342
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
We don't strictly need this, but the version to download ends up hard-coded in the wrapper, so upgrading gradle in the future would need us to regenerate. Not a great option, but maybe not the worst. Can remove this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-607356036
Your patch moved the *nix invocation of WrapperDownloader to before we set JAVA_CMD, when it still needed to be after. I switched that back, but kept your change of setting the CLASSPATH after we download the new gradle-wrapper.jar. I'll push a squashed/rebased version of everything here for one last look before committing.
In your Windows patch you use backslashes for GRADLE_WRAPPER_JAR but forward slashes for the JAVA_EXE command - is that fine/expected/necessary?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-607224015
[changes.patch.txt](https://github.com/apache/lucene-solr/files/4415071/changes.patch.txt)
Hi Mike. I've restructured the downloader class a bit so that gradle wrapper version and checksum are aligned with the target file - makes it a single place to change things (instead of messing with the scripts). I also simplified error messaging a bit, hope it's ok.
More interestingly I discovered a hideous problem with CLASSPATH environment variable: this is used by java in single-source mode and was locking the wrapper file (on Windows), effectively preventing any changes (in case of overwriting due to a wrong checksum, for example). Corrected.
Looks good to me to commit this in (removing the gradle-wrapper.jar file). Anything else can be a follow-up.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-607439907
I would really want to limit the number of forked java processes to the absolute minimum in those scripts. Each invocation of java adds a bit of penalty to startup and these accumulate. I would assume people are running with Java 11+ and if it fails for them so be it...
If you really want to be nice here then I would rewrite the Downloader to be compatible with Java 8, then *compile* that class with javac and run it as regular class. The first thing you do in the downloader then is you can System.exit(-1) if java version < 11.
It can be also left for a separate patch/ improvement.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400453166
##########
File path: .github/workflows/gradle-precommit.yml
##########
@@ -19,5 +19,8 @@ jobs:
java-version: 11
- name: Grant execute permission for gradlew
run: chmod +x gradlew
+ - name: Run something to download the gradle wrapper jar
Review comment:
I don't see the point. It either will download gradle jar or it won't (and fail) before it runs precommit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400721876
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
+
+ File temp = Files.createTempFile("gradle-wrapper", null).toFile();
+ try (ReadableByteChannel in = Channels.newChannel(url.openStream());
+ FileOutputStream out = new FileOutputStream(temp)) {
+ out.getChannel().transferFrom(in, 0, maxSize);
+ }
+
+ try (FileInputStream fis = new FileInputStream(temp)) {
+ MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
+
+ // Convert binary digest to hex checksum value
+ // This will strip leading zeroes, deal with that when we need to
+ String sha256sum = new BigInteger(1, sha256.digest(fis.readAllBytes())).toString(16);
+
+ if (args[2].equals(sha256sum)) {
+ if (!temp.renameTo(jar)) {
+ System.err.println("Failed renaming wrapper jar, leaving download at " + temp);
Review comment:
Always cleanup the temp file. What's the use of leaving 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400487392
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
I see we found the same things here.
The other option is to exclude the gradle-wrapper from failing the smoke check? I don't want to take the position that because other projects are doing something which is against apache policy (and we even have a check for it!) that it's ok for us to do so 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400489677
##########
File path: .github/workflows/gradle-precommit.yml
##########
@@ -19,5 +19,8 @@ jobs:
java-version: 11
- name: Grant execute permission for gradlew
run: chmod +x gradlew
+ - name: Run something to download the gradle wrapper jar
Review comment:
The checksum should be checked as part of the script that downloads the wrapper jar... otherwise it's too late and you've executed something you shouldn't have. That's what I meant when I said it adds additional complexity...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400459182
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
Gradle version to download should be in gradle-wrapper.properties. The wrapper JAR itself typically works across versions (in my experience) but I hear your point. There is also the checksum-validation issue...
Maybe we could ask legal for clarification whether this particular JAR can be distributed as part of source releases? This restriction makes things annoyingly complex to maintain for close to zero benefit...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on issue #1390: LUCENE-9266 remove
gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on issue #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#issuecomment-607435284
From @gerlowskija on Slack:
>Initially it failed for me with:
`./gradlew --info tasks
Error: Could not find or load main class buildSrc.src.main.java.org.apache.lucene.gradle.WrapperDownloader.java`
But that turned out to be because I was using Java8. Might be worth doing some sort of java -version check somewhere, but maybe not worth the trouble.
>Sure. But while people are developing on a mix of master/8x, there’s gonna be Java version mixups pretty frequently, so people are gonna start seeing that opaque error message unless we give them a better one.
A better error message would be nice here, but I also don't want to figure out how to parse the output. Maybe the easiest solution is to add another class with an empty main method and if that fails we assume the java launcher is too old?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401163791
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
+ final URL url = new URL("https://github.com/gradle/gradle/raw/v" + args[0] + "/gradle/wrapper/gradle-wrapper.jar");
+
+ // As of v6.0.1 the wrapper is approximately 60K
+ // Can increase this is gradle wrapper ever goes beyond 100K, but keep a safety check
+ final int maxSize = 100 * 1024;
+
+ File temp = Files.createTempFile("gradle-wrapper", null).toFile();
+ try (ReadableByteChannel in = Channels.newChannel(url.openStream());
+ FileOutputStream out = new FileOutputStream(temp)) {
+ out.getChannel().transferFrom(in, 0, maxSize);
+ }
+
+ try (FileInputStream fis = new FileInputStream(temp)) {
+ MessageDigest sha256 = MessageDigest.getInstance("SHA-256");
+
+ // Convert binary digest to hex checksum value
+ // This will strip leading zeroes, deal with that when we need to
+ String sha256sum = new BigInteger(1, sha256.digest(fis.readAllBytes())).toString(16);
+
+ if (args[2].equals(sha256sum)) {
+ if (!temp.renameTo(jar)) {
Review comment:
NIO's `Files.move` checks if it can rename and if not will do a copy+delete. I'll switch to that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400448369
##########
File path: build.gradle
##########
@@ -54,6 +55,7 @@ ext {
// if the build file is incorrectly written and evaluates something
// eagerly).
+apply from: file('gradle/wrapper.gradle')
Review comment:
We don't need to set up the wrapper task. If we can't have the local jar then the launch scripts must take care to download the proper version prior to launching 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r401166452
##########
File path: buildSrc/src/main/java/org/apache/lucene/gradle/WrapperDownloader.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;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.math.BigInteger;
+import java.net.URL;
+import java.nio.channels.Channels;
+import java.nio.channels.ReadableByteChannel;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+
+public class WrapperDownloader {
+ public static void main(String[] args) throws Exception {
+ if (args.length != 3) {
+ System.err.println("Usage: java WrapperDownloader.java <version> <destination> <checksum>");
+ System.exit(1);
+ }
+
+ File jar = new File(args[1]);
+ if (!jar.exists()) {
Review comment:
If the wrapper is from a different version, then the checksum won't match. And the jar itself isn't versioned. So there's a lot more to do here if we want to always compute sums.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1390:
LUCENE-9266 remove gradle wrapper jar from source
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1390: LUCENE-9266 remove gradle wrapper jar from source
URL: https://github.com/apache/lucene-solr/pull/1390#discussion_r400472030
##########
File path: gradlew
##########
@@ -80,6 +80,19 @@ case "`uname`" in
;;
esac
+
+# Loop in case we encounter an error.
Review comment:
This turned out to be pretty straightforward, I'm skipping things like proxy and async and retry and pause/resume for now. I don't know enough about Windows scripting to understand what I need to change, but hopefully somebody else can help me out. Shouldn't be more than two lines.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org