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

LUCENE-10328: Module path for compiling and running tests is wrong #571

Merged
merged 48 commits into from Jan 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
e8435cc
Non-functional changes cleaning up code
dweiss Dec 22, 2021
aaae8a6
Cleanups and refactorings without functional changes.
dweiss Dec 22, 2021
7d0b70b
Make sure configurations use jar artifacts instead of split class+res…
dweiss Dec 22, 2021
bd0e82b
Work in progress, to be reverted and flattened.
dweiss Dec 22, 2021
7519283
Convert some more modules to use test framework on module path
dweiss Dec 22, 2021
9fe2855
LUCENE-10337: workaround for xerces and split packages.
dweiss Dec 23, 2021
7743921
LUCENE-10335: IOUtils.getDecodingReader(Class<?>, String) is broken w…
dweiss Dec 23, 2021
95c5754
LUCENE-10338: Scan for tests only by convention file name pattern (Te…
dweiss Dec 23, 2021
b8d445f
Follow-up to test convention moved.
dweiss Dec 23, 2021
0f8678b
Use modular deps in the replicator.
dweiss Dec 23, 2021
7e0f1dc
Separate spatial-extras test features into a separate subproject. The…
dweiss Dec 23, 2021
6d88a74
Force certain projects to compile/run test in classpath mode until ci…
dweiss Dec 23, 2021
605ac4d
Separate spatial-extras test features into a separate subproject.
dweiss Dec 23, 2021
c00fb51
Separate spatial-extras test features into a separate subproject.
dweiss Dec 23, 2021
5904448
Update locks.
dweiss Dec 23, 2021
dd0fba5
Tweak the output from source validation. Remove NOCOMMITs, they're re…
dweiss Dec 23, 2021
42a7472
Revert "LUCENE-10335: IOUtils.getDecodingReader(Class<?>, String) is …
dweiss Dec 23, 2021
a1f5f14
Permit gradle versions with a matching base version (for debugging gr…
dweiss Dec 27, 2021
8f0cbb2
Apply Uwe's patch for LUCENE-10335
dweiss Dec 27, 2021
067c9f4
Merge remote-tracking branch 'origin/main' into LUCENE-10328
dweiss Dec 27, 2021
9b16978
Tidy.
dweiss Dec 27, 2021
0875392
LUCENE-10338: make TestPosition an internal class inside TrivialLooka…
dweiss Dec 27, 2021
d0abc12
Multiple-mode rendering of module-path and classpath.
dweiss Dec 27, 2021
d7ff7e1
Add oddball hacks for ECJ which otherwise fails with errors. Nekohtml…
dweiss Dec 27, 2021
66977bc
Correct javadoc.
dweiss Dec 27, 2021
f561e4f
Remaining fixes to get check to pass
dweiss Dec 27, 2021
3f6069e
Run projects with modular tests with proper modular path - combined c…
dweiss Dec 27, 2021
df58882
Correct jar separation between source sets, add a test validating mod…
dweiss Dec 27, 2021
cb420e2
Add a runtime modular path dependency check.
dweiss Dec 27, 2021
b6ae29b
Don't tweak the paths for intellij.
dweiss Dec 27, 2021
152bb76
Removing the main sourceset module - works with gradle but confuses i…
dweiss Dec 27, 2021
d5a408e
Add an awkward exclusion to break circular dependency in spatial3d
dweiss Dec 29, 2021
f87acbf
Add some initial module-patching infrastructure that tries not to bre…
dweiss Dec 29, 2021
b3b3168
Merging changes from uschindler:jira/LUCENE-10335
dweiss Dec 30, 2021
855694c
Revert "Merging changes from uschindler:jira/LUCENE-10335"
dweiss Jan 1, 2022
4366944
Merging with main.
dweiss Jan 1, 2022
3265a67
LUCENE-10344: force ecj to run in classpath mode for projects/sources…
dweiss Jan 3, 2022
e91d84c
Merge branch 'main' of https://gitbox.apache.org/repos/asf/lucene int…
uschindler Jan 3, 2022
c76d65e
Revert some changes that were better fixed in other PR
uschindler Jan 3, 2022
66509ff
Merge branch 'main' into LUCENE-10328
uschindler Jan 3, 2022
d88f5cf
Edit comment.
dweiss Jan 3, 2022
1aac2dc
Merge remote-tracking branch 'origin/main' into LUCENE-10328
dweiss Jan 4, 2022
c6363fc
Use test source set constant.
dweiss Jan 4, 2022
c175e21
Add decision tree on how to configure classpath/ modular mode.
dweiss Jan 4, 2022
938e01b
Make morfologik.tests extend from (modular) lucene test-framework.
dweiss Jan 4, 2022
d689960
Make core.tests utilize the test framework in a normal, modular way.
dweiss Jan 4, 2022
67ecf7f
Merge remote-tracking branch 'origin/main' into LUCENE-10328
dweiss Jan 4, 2022
d1f3732
Merge branch 'main' into LUCENE-10328
dweiss Jan 5, 2022
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
29 changes: 8 additions & 21 deletions gradle/documentation/render-javadoc.gradle
Expand Up @@ -57,7 +57,7 @@ allprojects {
outputDir = project.javadoc.destinationDir
}

if (project.path == ':lucene:luke' || project.path.endsWith(".tests")) {
if (project.path == ':lucene:luke' || !(project in rootProject.ext.mavenProjects)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this mavenProjects?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah it is inverse. All projects that will land in Maven central. Yeah thats a better check.

Copy link
Contributor

Choose a reason for hiding this comment

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

by the way theres also the better operator project !in rootProject.ext.mavenProjects

I used is elsewhere already.

// These projects are not part of the public API so we don't render their javadocs
// as part of the site's creation. A side-effect of this is that javadocs would not
// be linted for these projects. To avoid this, we connect the regular javadoc task
Expand Down Expand Up @@ -165,26 +165,6 @@ configure(project(":lucene:test-framework")) {
project.tasks.withType(RenderJavadocTask) {
// TODO: fix missing javadocs
javadocMissingLevel = "class"
// TODO: clean up split packages
javadocMissingIgnore = [
"org.apache.lucene.analysis",
"org.apache.lucene.analysis.standard",
"org.apache.lucene.codecs",
"org.apache.lucene.codecs.blockterms",
"org.apache.lucene.codecs.bloom",
"org.apache.lucene.codecs.compressing",
"org.apache.lucene.codecs.uniformsplit",
"org.apache.lucene.codecs.uniformsplit.sharedterms",
"org.apache.lucene.geo",
"org.apache.lucene.index",
"org.apache.lucene.search",
"org.apache.lucene.search.similarities",
"org.apache.lucene.search.spans",
"org.apache.lucene.store",
"org.apache.lucene.util",
"org.apache.lucene.util.automaton",
"org.apache.lucene.util.fst"
]
}
}

Expand All @@ -195,6 +175,13 @@ configure(project(":lucene:sandbox")) {
}
}

configure(project(":lucene:spatial-test-fixtures")) {
project.tasks.withType(RenderJavadocTask) {
// TODO: fix missing javadocs
javadocMissingLevel = "class"
}
}

configure(project(":lucene:misc")) {
project.tasks.withType(RenderJavadocTask) {
// TODO: fix missing javadocs
Expand Down
585 changes: 428 additions & 157 deletions gradle/java/modules.gradle

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion gradle/maven/publications.gradle
Expand Up @@ -38,7 +38,9 @@ configure(rootProject) {
// Exclude the parent container project for analysis modules (no artifacts).
":lucene:analysis",
// Exclude the native module.
":lucene:misc:native"
":lucene:misc:native",
// Exclude test fixtures.
":lucene:spatial-test-fixtures"
]

// Exclude all subprojects that are modular test projects and those explicitly
Expand Down
10 changes: 8 additions & 2 deletions gradle/validation/check-environment.gradle
Expand Up @@ -33,14 +33,20 @@ configure(rootProject) {
def currentJavaVersion = JavaVersion.current()
if (currentJavaVersion < minJavaVersion) {
throw new GradleException("At least Java ${minJavaVersion} is required, you are running Java ${currentJavaVersion} "
+ "[${System.getProperty('java.vm.name')} ${System.getProperty('java.vm.version')}]")
+ "[${System.getProperty('java.vm.name')} ${System.getProperty('java.vm.version')}]")
}

// If we're regenerating the wrapper, skip the check.
if (!gradle.startParameter.taskNames.contains("wrapper")) {
def currentGradleVersion = GradleVersion.current()
if (currentGradleVersion != GradleVersion.version(expectedGradleVersion)) {
throw new GradleException("Gradle ${expectedGradleVersion} is required (hint: use the gradlew script): this gradle is ${currentGradleVersion}")
if (currentGradleVersion.baseVersion == GradleVersion.version(expectedGradleVersion).baseVersion) {
logger.warn("Gradle ${expectedGradleVersion} is required but base version of this gradle matches, proceeding (" +
"this gradle is ${currentGradleVersion})")
} else {
throw new GradleException("Gradle ${expectedGradleVersion} is required (hint: use the gradlew script): " +
"this gradle is ${currentGradleVersion}")
}
}
}
}
49 changes: 24 additions & 25 deletions gradle/validation/ecj-lint.gradle
Expand Up @@ -74,42 +74,36 @@ allprojects {
args += [ "-properties", file("${resources}/ecj.javadocs.prefs").absolutePath ]

// We depend on modular paths.
def modularPaths = sourceSet.modularPaths
dependsOn modularPaths.compileModulePathConfiguration
def modularPaths = sourceSet.modularPathsForEcj
dependsOn modularPaths

// Place input files in an external file to dodge command line argument
// limits. We could pass a directory but ecj seems to be buggy: when it
// encounters a module-info.java file it no longer compiles other source files.
def inputsFile = file("${tmpDst}/ecj-inputs.txt")
// Add modular dependencies and their transitive dependencies to module path.
task.argumentProviders.add(modularPaths.compilationArguments)

// Add classpath, if needed.
task.argumentProviders.add((CommandLineArgumentProvider) {
// Add modular dependencies and their transitive dependencies to module path.
def modularPathFiles = modularPaths.compileModulePathConfiguration.files
def extraArgs = []
if (!modularPathFiles.isEmpty()) {
if (!modularPaths.hasModuleDescriptor()) {
// We're compiling a non-module so we'll bring everything on module path in
// otherwise things wouldn't be part of the resolved module graph.
extraArgs += ["--add-modules", "ALL-MODULE-PATH"]
}

extraArgs += ["--module-path", modularPathFiles.join(File.pathSeparator)]
}

// Add classpath locations in a lazy provider (can't resolve the
// configuration at evaluation time). Filter out non-existing entries
// (output folders for non-existing input source dirs like resources).
def cpath = sourceSet.compileClasspath.filter { p -> p.exists() }
cpath = cpath - modularPathFiles
FileCollection cpath = modularPaths.compilationClasspath.filter { p -> p.exists() }
if (!cpath.isEmpty()) {
extraArgs += ["-classpath", cpath.join(File.pathSeparator)]
return ["-classpath", cpath.join(File.pathSeparator)]
} else {
return []
}
})

extraArgs += ["@" + inputsFile.absolutePath]
return extraArgs
// Place input files in an external file to dodge command line argument
// limits. We could pass a directory but ecj seems to be buggy: when it
// encounters a module-info.java file it no longer compiles other source files.
def inputsFile = file("${tmpDst}/ecj-inputs.txt")
task.argumentProviders.add((CommandLineArgumentProvider) {
return ["@" + inputsFile.absolutePath]
})

doFirst {
modularPaths.logCompilationPaths(logger)

tmpDst.mkdirs()

// escape filename accoring to ECJ's rules:
Expand All @@ -118,7 +112,12 @@ allprojects {
def escapeFileName = { String s -> s.replaceAll(/ +/, /"$0"/) }
inputsFile.setText(
srcDirs.collectMany { dir ->
project.fileTree(dir: dir, include: "**/*.java" ).files
project.fileTree(
dir: dir,
include: "**/*.java",
// Exclude the benchmark class with dependencies on nekohtml, which causes module-classpath conflicts and breaks ecj.
exclude: "**/DemoHTMLParser.java"
).files
}
// Try to sort all input files; a side-effect of this should be that module-info.java
// is placed first on the list, which works around ECJ bug:
Expand Down
14 changes: 7 additions & 7 deletions gradle/validation/validate-source-patterns.gradle
Expand Up @@ -154,12 +154,11 @@ class ValidateSourcePatternsTask extends DefaultTask {
(~$/\n\s*var\s+.*=.*<>.*/$) : 'Diamond operators should not be used with var'
]

def found = 0;
def violations = new TreeSet();
def reportViolation = { f, name ->
logger.error('{}: {}', name, f);
violations.add(name);
found++;
String msg = String.format(Locale.ROOT, "%s: %s", f, name)
uschindler marked this conversation as resolved.
Show resolved Hide resolved
logger.error(msg)
violations.add(msg)
}

def javadocsPattern = ~$/(?sm)^\Q/**\E(.*?)\Q*/\E/$;
Expand Down Expand Up @@ -258,9 +257,10 @@ class ValidateSourcePatternsTask extends DefaultTask {
}
progress.completed()

if (found) {
throw new GradleException(String.format(Locale.ENGLISH, 'Found %d violations in source files (%s).',
found, violations.join(', ')));
if (!violations.isEmpty()) {
throw new GradleException(String.format(Locale.ENGLISH, 'Found %d source violation(s):\n %s',
violations.size(),
violations.join('\n ')))
}
}
}
2 changes: 1 addition & 1 deletion lucene/analysis/common/build.gradle
Expand Up @@ -21,7 +21,7 @@ description = 'Analyzers for indexing content in different languages and domains

dependencies {
moduleApi project(':lucene:core')
testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}

// Fetch the data and enable regression tests against woorm/ libreoffice dictionaries.
Expand Down
2 changes: 1 addition & 1 deletion lucene/analysis/icu/build.gradle
Expand Up @@ -25,5 +25,5 @@ dependencies {

moduleApi 'com.ibm.icu:icu4j'

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
2 changes: 1 addition & 1 deletion lucene/analysis/kuromoji/build.gradle
Expand Up @@ -23,5 +23,5 @@ dependencies {
moduleApi project(':lucene:core')
moduleApi project(':lucene:analysis:common')

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
5 changes: 1 addition & 4 deletions lucene/analysis/morfologik.tests/build.gradle
Expand Up @@ -21,8 +21,5 @@ description = 'Module tests for :lucene:analysis:morfologik'

dependencies {
moduleTestImplementation project(':lucene:analysis:morfologik')
moduleTestImplementation("junit:junit", {
exclude group: "org.hamcrest"
})
moduleTestImplementation "org.hamcrest:hamcrest"
moduleTestImplementation project(':lucene:test-framework')
}
2 changes: 1 addition & 1 deletion lucene/analysis/morfologik.tests/src/test/module-info.java
Expand Up @@ -21,7 +21,7 @@
requires org.apache.lucene.core;
requires org.apache.lucene.analysis.common;
requires org.apache.lucene.analysis.morfologik;
requires junit;
requires org.apache.lucene.test_framework;

exports org.apache.lucene.analysis.morfologik.tests;
}
Expand Up @@ -19,28 +19,24 @@
import org.apache.lucene.analysis.morfologik.MorfologikAnalyzer;
import org.apache.lucene.analysis.uk.UkrainianMorfologikAnalyzer;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.tests.util.LuceneTestCase;
import org.junit.Assert;
import org.junit.Test;

public class TestMorfologikAnalyzer {
@Test
public class TestMorfologikAnalyzer extends LuceneTestCase {
public void testMorfologikAnalyzerLoads() {
var analyzer = new MorfologikAnalyzer();
Assert.assertNotNull(analyzer);
}

@Test
public void testUkrainianMorfologikAnalyzerLoads() {
var analyzer = new UkrainianMorfologikAnalyzer();
Assert.assertNotNull(analyzer);
}

@Test
public void testWeAreModule() {
Assert.assertTrue(this.getClass().getModule().isNamed());
}

@Test
public void testLuceneIsAModule() {
Assert.assertTrue(IndexWriter.class.getModule().isNamed());
}
Expand Down
2 changes: 1 addition & 1 deletion lucene/analysis/morfologik/build.gradle
Expand Up @@ -27,5 +27,5 @@ dependencies {
moduleImplementation 'org.carrot2:morfologik-polish'
moduleImplementation 'ua.net.nlp:morfologik-ukrainian-search'

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
4 changes: 2 additions & 2 deletions lucene/analysis/nori/build.gradle
Expand Up @@ -22,7 +22,7 @@ description = 'Korean Morphological Analyzer'
dependencies {
moduleApi project(':lucene:core')
moduleApi project(':lucene:analysis:common')
testImplementation project(':lucene:test-framework')

moduleTestImplementation project(':lucene:test-framework')
}

2 changes: 1 addition & 1 deletion lucene/analysis/opennlp/build.gradle
Expand Up @@ -24,5 +24,5 @@ dependencies {
moduleApi project(':lucene:analysis:common')
moduleApi 'org.apache.opennlp:opennlp-tools'

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
2 changes: 1 addition & 1 deletion lucene/analysis/phonetic/build.gradle
Expand Up @@ -25,6 +25,6 @@ dependencies {

moduleApi 'commons-codec:commons-codec'

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}

2 changes: 1 addition & 1 deletion lucene/analysis/smartcn/build.gradle
Expand Up @@ -23,5 +23,5 @@ dependencies {
moduleApi project(':lucene:core')
moduleApi project(':lucene:analysis:common')

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
2 changes: 1 addition & 1 deletion lucene/analysis/stempel/build.gradle
Expand Up @@ -23,5 +23,5 @@ dependencies {
moduleApi project(':lucene:core')
moduleApi project(':lucene:analysis:common')

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
2 changes: 1 addition & 1 deletion lucene/backward-codecs/build.gradle
Expand Up @@ -22,5 +22,5 @@ description = 'Codecs for older versions of Lucene'

dependencies {
moduleApi project(':lucene:core')
testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
9 changes: 8 additions & 1 deletion lucene/benchmark/build.gradle
Expand Up @@ -36,11 +36,18 @@ dependencies {
moduleImplementation "org.locationtech.spatial4j:spatial4j"
moduleImplementation ("net.sourceforge.nekohtml:nekohtml", {
exclude module: "xml-apis"
// LUCENE-10337: Exclude xercesImpl from module path because it has split packages with the JDK (!)
exclude module: "xercesImpl"
})

// LUCENE-10337: Include xercesImpl on regular classpath where it won't cause conflicts.
implementation ("xerces:xercesImpl", {
exclude module: "xml-apis"
})

moduleRuntimeOnly project(':lucene:analysis:icu')

testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}

// We add 'conf' to resources because we validate *.alg script correctness in one of the tests.
Expand Down
6 changes: 3 additions & 3 deletions lucene/classification/build.gradle
Expand Up @@ -25,7 +25,7 @@ dependencies {
moduleImplementation project(':lucene:queries')
moduleImplementation project(':lucene:grouping')

testImplementation project(':lucene:test-framework')
testImplementation project(':lucene:analysis:common')
testImplementation project(':lucene:codecs')
moduleTestImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:analysis:common')
moduleTestImplementation project(':lucene:codecs')
}
2 changes: 1 addition & 1 deletion lucene/codecs/build.gradle
Expand Up @@ -21,5 +21,5 @@ description = 'Lucene codecs and postings formats'

dependencies {
moduleImplementation project(':lucene:core')
testImplementation project(':lucene:test-framework')
moduleTestImplementation project(':lucene:test-framework')
}
Expand Up @@ -15,13 +15,14 @@
* limitations under the License.
*/

package org.apache.lucene.codecs.lucene90;
package org.apache.lucene.codecs.lucene90.tests;

/** Test utility class to create mock {@link Lucene90PostingsFormat.IntBlockTermState}. */
public class MockTermStateFactory {
import org.apache.lucene.codecs.lucene90.Lucene90PostingsFormat.IntBlockTermState;

/** Creates an empty {@link Lucene90PostingsFormat.IntBlockTermState}. */
public static Lucene90PostingsFormat.IntBlockTermState create() {
return new Lucene90PostingsFormat.IntBlockTermState();
/** Test utility class to create mock {@link IntBlockTermState}. */
public class MockTermStateFactory {
/** Creates an empty {@link IntBlockTermState}. */
public static IntBlockTermState create() {
return new IntBlockTermState();
}
}
Expand Up @@ -19,7 +19,7 @@

import java.io.IOException;
import java.util.Collections;
import org.apache.lucene.codecs.lucene90.MockTermStateFactory;
import org.apache.lucene.codecs.lucene90.tests.MockTermStateFactory;
import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.FieldInfo;
import org.apache.lucene.index.IndexOptions;
Expand Down
Expand Up @@ -27,7 +27,7 @@
import java.util.Set;
import org.apache.lucene.codecs.BlockTermState;
import org.apache.lucene.codecs.PostingsReaderBase;
import org.apache.lucene.codecs.lucene90.MockTermStateFactory;
import org.apache.lucene.codecs.lucene90.tests.MockTermStateFactory;
import org.apache.lucene.codecs.uniformsplit.BlockHeader;
import org.apache.lucene.codecs.uniformsplit.BlockLine;
import org.apache.lucene.codecs.uniformsplit.FSTDictionary;
Expand Down