Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ extraJavaModuleInfo {
// "org.mycompany.server", "org.mycompany.client")
// or simply export all packages
// exportAllPackages()
// or export all packages except specific named ones
// exportAllPackagesExcept("org.mycompany.notgood1", "org.mycompany.notgood2")
Copy link
Member

Choose a reason for hiding this comment

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

👍


requiresTransitive("org.apache.commons.logging")
requires("java.sql")
Expand Down Expand Up @@ -216,7 +218,9 @@ This needs to be done in all subprojects. You use the `versionsProvidingConfigur

```kotlin
extraJavaModuleInfo {
versionsProvidingConfiguration = "mainRuntimeClasspath"
versionsProvidingConfiguration = project.provider { project.configurations.named("mainRuntimeClasspath").get() }
// or for older Gradle/Kotlin versions
// versionsProvidingConfiguration.set(project.provider { project.configurations.named("mainRuntimeClasspath").get() })
}
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public abstract class ExtraJavaModuleInfoPluginExtension {

public abstract Property<Boolean> getDeriveAutomaticModuleNamesFromFileNames();

public abstract Property<String> getVersionsProvidingConfiguration();
public abstract Property<Configuration> getVersionsProvidingConfiguration();
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should change this. If I understand your reasoning correctly, this actually makes the situation worse. The code at the moment always takes the Configuration out of the current project scope – versionsSource = project.getConfigurations()... in PublishedMetadata.java. This ensures that the user cannot put in a Configuration object obtain from a different context.


To solve the warning in you project, you should never do something like this:

configurations.add(project("platform").configurations.named("allDependencies"))

Note: if you turn on org.gradle.unsafe.isolated-projects=true it will find such things in your setup and give you warnings/errors for it.

Instead, you should make the allDependencies a consumable (not resolvable) in platform.
Then in all other projects, you define a dependency to that. Similar to what is documented here. Something like:

// main/platform/build.gradle.kts
val consistentResolutionAttribute = Attribute.of("consistent-resolution", String::class.java)
configurations.create("allDependencies") {
    isCanBeResolved = false
    ...
    attributes { attribute(consistentResolutionAttribute, "global") }
}
// main/project/build.gradle.kts
val consistentResolutionAttribute = Attribute.of("consistent-resolution", String::class.java)
val allDependenciesPath = configurations.create("allDependenciesPath")) {
    isCanBeConsumed = false
    attributes { attribute(consistentResolutionAttribute, "global") }
}

dependencies {
    allDependenciesPath(project(":platform"))
}

extraJavaModuleInfo {
    versionsProvidingConfiguration = "allDependenciesPath"
}

This is a bit tricky, but I don't have a more compact solution. And because it is quite individual in different project setups, and cross-cutting subprojects, I don't have a good idea for features in this plugin to make it easier atm.


/**
* Add full module information for a given Jar file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ private void addModuleDescriptor(File originalJar, File moduleJar, ModuleInfo mo
packages);
mergeJars(moduleInfo, outputStream, providers, packages);
outputStream.putNextEntry(newReproducibleEntry("module-info.class"));

if (moduleInfo.exportAllPackages) {
moduleInfo.exportAllPackagesExceptions.forEach(it -> packages.remove(packageToPath(it)));
}
outputStream.write(addModuleInfo(
moduleInfo,
providers,
Expand Down Expand Up @@ -390,7 +394,7 @@ private byte[] copyAndExtractProviders(
String packagePath =
i > 0 ? mrJarMatcher.matches() ? mrJarMatcher.group(1) : entryName.substring(0, i) : "";

if (!removedPackages.contains(packagePath.replace('/', '.'))) {
if (!removedPackages.contains(pathToPackage(packagePath))) {
if (entryName.endsWith(".class") && !packagePath.isEmpty()) {
packages.add(packagePath);
}
Expand Down Expand Up @@ -462,6 +466,24 @@ public void visitEnd() {
return classWriter.toByteArray();
}

/**
* Convert a Java package name to a path
* @param packageName The package name
* @return The package name converted to a path
*/
private static String packageToPath(String packageName) {
return packageName.replace('.', '/');
}

/**
* Convert a path to a Java package name
* @param path The path
* @return The path converted to a package name
*/
private static String pathToPackage(String path) {
return path.replace('/', '.');
}
Copy link
Member

Choose a reason for hiding this comment

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

✅ Thanks for introducing these methods!


private void addModuleInfoEntires(
ModuleInfo moduleInfo,
Map<String, List<String>> providers,
Expand All @@ -473,13 +495,13 @@ private void addModuleInfoEntires(
for (Map.Entry<String, Set<String>> entry : moduleInfo.exports.entrySet()) {
String packageName = entry.getKey();
Set<String> modules = entry.getValue();
moduleVisitor.visitExport(packageName.replace('.', '/'), 0, modules.toArray(new String[0]));
moduleVisitor.visitExport(packageToPath(packageName), 0, modules.toArray(new String[0]));
}

for (Map.Entry<String, Set<String>> entry : moduleInfo.opens.entrySet()) {
String packageName = entry.getKey();
Set<String> modules = entry.getValue();
moduleVisitor.visitOpen(packageName.replace('.', '/'), 0, modules.toArray(new String[0]));
moduleVisitor.visitOpen(packageToPath(packageName), 0, modules.toArray(new String[0]));
}

if (moduleInfo.requireAllDefinedDependencies) {
Expand Down Expand Up @@ -524,7 +546,7 @@ private void addModuleInfoEntires(
moduleVisitor.visitRequire(requireName, Opcodes.ACC_STATIC_PHASE | Opcodes.ACC_TRANSITIVE, null);
}
for (String usesName : moduleInfo.uses) {
moduleVisitor.visitUse(usesName.replace('.', '/'));
moduleVisitor.visitUse(packageToPath(usesName));
}
for (Map.Entry<String, List<String>> entry : providers.entrySet()) {
String name = entry.getKey();
Expand All @@ -539,9 +561,9 @@ private void addModuleInfoEntires(
}
if (!implementations.isEmpty()) {
moduleVisitor.visitProvide(
name.replace('.', '/'),
packageToPath(name),
implementations.stream()
.map(impl -> impl.replace('.', '/'))
.map(ExtraJavaModuleInfoTransform::packageToPath)
.toArray(String[]::new));
}
}
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/org/gradlex/javamodule/moduleinfo/ModuleInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
package org.gradlex.javamodule.moduleinfo;

import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.gradle.api.model.ObjectFactory;
Expand All @@ -27,6 +29,7 @@ public class ModuleInfo extends ModuleSpec {
final Set<String> requiresStaticTransitive = new LinkedHashSet<>();
final Map<String, Set<String>> ignoreServiceProviders = new LinkedHashMap<>();
final Set<String> uses = new LinkedHashSet<>();
final Set<String> exportAllPackagesExceptions = new LinkedHashSet<>();

boolean exportAllPackages;
boolean requireAllDefinedDependencies;
Expand Down Expand Up @@ -119,7 +122,24 @@ public String getModuleVersion() {
* Automatically export all packages of the Jar. Can be used instead of individual 'exports()' statements.
*/
public void exportAllPackages() {
exportAllPackagesExcept(Collections.emptyList());
}

/**
* Automatically export all packages of the Jar. Can be used instead of individual 'exports()' statements.
* @param exceptions A list of packages not to export
*/
public void exportAllPackagesExcept(String... exceptions) {
exportAllPackagesExcept(Arrays.asList(exceptions));
}

/**
* Automatically export all packages of the Jar. Can be used instead of individual 'exports()' statements.
* @param exceptions A list of packages not to export
*/
public void exportAllPackagesExcept(List<String> exceptions) {
this.exportAllPackages = true;
exportAllPackagesExceptions.addAll(exceptions);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ public class PublishedMetadata implements Serializable {

@SuppressWarnings({"UnstableApiUsage", "unchecked"})
private List<String> componentVariant(
Provider<String> versionsProvidingConfiguration, Project project, String usage) {
Provider<Configuration> versionsProvidingConfiguration, Project project, String usage) {
Configuration versionsSource;
if (versionsProvidingConfiguration.isPresent()) {
versionsSource = project.getConfigurations()
.named(versionsProvidingConfiguration.get())
.get();
versionsSource = versionsProvidingConfiguration.get();
if (!versionsSource.isCanBeResolved()) {
throw new IllegalArgumentException(
"Configuration '" + versionsSource.getName() + "' must be resolvable");
}
} else {
// version provider is not configured, create on adhoc based on ALL classpaths of the project
versionsSource = maybeCreateDefaultVersionSourcConfiguration(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,69 @@ class EdgeCasesFunctionalTest extends Specification {
run().task(':run').outcome == TaskOutcome.SUCCESS
}

def "can automatically export all packages except specified of a legacy Jar"() {
given:
file("src/main/java/org/gradle/sample/app/Main.java") << """
package org.gradle.sample.app;

import javax.json.JsonString;
import javax.json.JsonValue;

public class Main {
public static void main(String[] args) {
JsonString jsonString = new JsonString() {
@Override
public boolean equals(Object obj) {
return false;
}
@Override
public CharSequence getChars() {
return null;
}
@Override
public String getString() {
return null;
}
@Override
public int hashCode() {
return 0;
}
@Override
public JsonValue.ValueType getValueType() {
return null;
}
};
}
}
"""
file("src/main/java/module-info.java") << """
module org.gradle.sample.app {
exports org.gradle.sample.app;

requires org.glassfish.java.json;
requires java.json;
}
"""
buildFile << """
dependencies {
implementation("org.glassfish:jakarta.json:1.1.6")
implementation("jakarta.json:jakarta.json-api:1.1.6")
}

extraJavaModuleInfo {
module("org.glassfish:jakarta.json", "org.glassfish.java.json") {
exportAllPackagesExcept("javax.json", "javax.json.spi", "javax.json.stream")
overrideModuleName()
}
knownModule("jakarta.json:jakarta.json-api", "java.json")
}
"""

expect:
def result = failRun()
result.output.matches(/(?s).*Package javax\.json[.a-z]* in both.*/)
Comment on lines +215 to +240
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding a test. Right now it does not seem to test the new feature, though. The split-package error will always occur no matter if the packages are exported or not.

I suggest the following adjustment

Suggested change
file("src/main/java/module-info.java") << """
module org.gradle.sample.app {
exports org.gradle.sample.app;
requires org.glassfish.java.json;
requires java.json;
}
"""
buildFile << """
dependencies {
implementation("org.glassfish:jakarta.json:1.1.6")
implementation("jakarta.json:jakarta.json-api:1.1.6")
}
extraJavaModuleInfo {
module("org.glassfish:jakarta.json", "org.glassfish.java.json") {
exportAllPackagesExcept("javax.json", "javax.json.spi", "javax.json.stream")
overrideModuleName()
}
knownModule("jakarta.json:jakarta.json-api", "java.json")
}
"""
expect:
def result = failRun()
result.output.matches(/(?s).*Package javax\.json[.a-z]* in both.*/)
file("src/main/java/module-info.java") << """
module org.gradle.sample.app {
exports org.gradle.sample.app;
requires java.json;
}
"""
buildFile << """
dependencies {
implementation("org.glassfish:jakarta.json:1.1.6")
implementation("jakarta.json:jakarta.json-api:1.1.6")
}
extraJavaModuleInfo {
module("org.glassfish:jakarta.json", "java.json") {
exportAllPackagesExcept("javax.json", "javax.json.spi", "javax.json.stream")
}
}
"""
expect:
def result = failRun()
result.output.contains("error: package javax.json is not visible")

}

def "deriveAutomaticModuleNamesFromFileNames produces a build time error for invalid module names"() {
given:
buildFile << """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ class RequireAllDefinedDependenciesFunctionalTest extends Specification {
given:
def sharedBuildScript = """
extraJavaModuleInfo {
versionsProvidingConfiguration.set("mainRuntimeClasspath")
versionsProvidingConfiguration.set(project.provider { project.configurations.named("mainRuntimeClasspath").get() })
module(${libs.commonsHttpClient}, "org.apache.httpcomponents.httpclient")
module(${libs.commonsLogging}, "org.apache.commons.logging")
knownModule("commons-codec:commons-codec", "org.apache.commons.codec")
Expand Down