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/08/28 16:32:43 UTC

[GitHub] [lucene-solr] ErickErickson opened a new pull request #1796: LUCENE-9475: Enhance the Gradle build as necessary after removing Ant…

ErickErickson opened a new pull request #1796:
URL: https://github.com/apache/lucene-solr/pull/1796


   This PR is just a starting point. I grepped through the source code to try to find all the references to "ant". Do you have any idea how many words in our code have "ant" in them? Like constant, spanTerm, .... ;) Over 7,000 if you're curious.
   
   Anyway, I pared all those down and there are  a few files (23 or so) that reference ant legitimately. I fixed some that were simple, things like how to run a particular test, but for a number of them I just added //nocommit so we can find them easily. Here are some examples of things I punted on and just added //nocommits for:
   
   - The lucene/benchmarks.
   - UnicodeProps.java references an "ant unicode-data" target.
   - TokebnInfoDictionaryTest has a reference to an "ant test-tools" target
   etc.
   
   I'll check against the 8x version for the targets I didn't immediately see a corresponding Gradle task for, and if the are missing on ant I'll feel safe to remove them. Also I haven't poked around (yet) in the gradle build for tasks that weren't named the same but did the same thing.
   
   I'll start working on some of the nocommits, any hints welcome. 


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



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


[GitHub] [lucene-solr] ErickErickson closed pull request #1796: LUCENE-9475: Enhance the Gradle build as necessary after removing Ant…

Posted by GitBox <gi...@apache.org>.
ErickErickson closed pull request #1796:
URL: https://github.com/apache/lucene-solr/pull/1796


   


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



---------------------------------------------------------------------
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 #1796: LUCENE-9475: Enhance the Gradle build as necessary after removing Ant…

Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #1796:
URL: https://github.com/apache/lucene-solr/pull/1796#discussion_r479496521



##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/util/UnicodeProps.java
##########
@@ -1,4 +1,5 @@
 // DO NOT EDIT THIS FILE! Use "ant unicode-data" to recreate.
+//nocommit do we have such a target?

Review comment:
       I don't think so. If we don't then did we miss it somehow?

##########
File path: solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java
##########
@@ -210,7 +210,7 @@ public void write(OutputStream os) throws IOException {
     // That causes the totalHits and export entries in the context to _not_ get set.
     // The only time that really matters is when we search against an _empty_ set. That's too obscure
     // a condition to handle as part of this patch, if someone wants to pursue it it can be reproduced with:
-    // ant test  -Dtestcase=StreamingTest -Dtests.method=testAllValidExportTypes -Dtests.seed=10F13879D0D1D6AD -Dtests.slow=true -Dtests.locale=es-PA -Dtests.timezone=America/Bahia_Banderas -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
+    // gradlew test  --tests *testAllValidExportTypes* -Ptests.seed=10F13879D0D1D6AD -Ptests.slow=true -Ptests.locale=es-PA -Ptests.timezone=America/Bahia_Banderas -Ptests.asserts=true -Ptests.file.encoding=ISO-8859-1

Review comment:
       I think this should be a qualified class.method reference (much like I showed above).

##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/index/TestManyPointsInOldIndex.java
##########
@@ -36,7 +36,7 @@
 //
 // Compile:
 //   1) temporarily remove 'extends LuceneTestCase' above (else java doesn't see our static void main)

Review comment:
       I'd look as to what this class actually does... seems weird. Classpath below (under "run") is wrong for gradle.

##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
##########
@@ -168,7 +169,7 @@ public void testCreateMoreTermsIndex() throws Exception {
     Thread.sleep(100000);
   }
 
-  // ant test -Dtestcase=TestBackwardsCompatibility -Dtestmethod=testCreateSortedIndex -Dtests.codec=default -Dtests.useSecurityManager=false -Dtests.bwcdir=/tmp/sorted
+  // gradlew -p lucene/backward-codecs test --tests *testCreateSortedIndex* -Ptests.bwcdir=/tmp/sorted -Ptests.codec=default -Ptests.useSecurityManager=false

Review comment:
       should be --tests TestBackwardsCompatibility.testCreateSortedIndex.
   A prefix wildcard may be needed (*TestBackwards...)? 

##########
File path: lucene/analysis/kuromoji/src/test/org/apache/lucene/analysis/ja/dict/TokenInfoDictionaryTest.java
##########
@@ -35,6 +35,7 @@
 
 import static org.apache.lucene.analysis.ja.dict.BinaryDictionary.ResourceScheme;
 
+//nocommit

Review comment:
       "test-tools" shouldn't be a task. I don't know what it was doing but it should be a regular test, perhaps grouped under a test group (and enabled by a property).

##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
##########
@@ -130,6 +131,7 @@ public static void checkSyspropForceBeforeClassAssumptionFailure() {
    */
   @Before
   public void checkSyspropForceBeforeAssumptionFailure() {
+    // nocommit, don't see an equivalent Gradle parameter

Review comment:
       I think -Ptests.jvmargs=-Dtests.force...=true should work (didn't check).

##########
File path: lucene/core/src/test/org/apache/lucene/index/Test2BPoints.java
##########
@@ -31,7 +31,7 @@
 import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;
 
 // e.g. run like this: ant test -Dtestcase=Test2BPoints -Dtests.nightly=true -Dtests.verbose=true -Dtests.monster=true
-// 
+// nocommit, I don't see how to set the heap size, set in gradle.properties? Set to 4G?

Review comment:
       -Ptests.heapsize=6g

##########
File path: lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
##########
@@ -459,6 +459,7 @@
   public static final boolean LEAVE_TEMPORARY;
   static {
     boolean defaultValue = false;
+    //nocommit

Review comment:
       You can remove reference to ant, but keep the rest.

##########
File path: solr/core/src/test/org/apache/solr/search/facet/SpatialHeatmapFacetsTest.java
##########
@@ -39,6 +39,7 @@ public static void beforeSuperClass() throws Exception {
     schemaString = "schema-spatial.xml";
     configString = "solrconfig-basic.xml";
 
+    //nocommit are we sure we do this in Gradle

Review comment:
       We do. See defaults-tests.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



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