You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2024/09/08 09:42:10 UTC
[PR] Feature/2.x/2929 remove transitive static (logging-log4j2)
ppkarwasz opened a new pull request, #2930:
URL: https://github.com/apache/logging-log4j2/pull/2930
This PR:
- Add a Groovy script that checks the generated `module-info.class` files for `static transitive` dependencies.
- Modifies the BND configuration not to mark `org.jspecify` as `transitive`.
We remove the `transitive` modifier from all optional dependencies.
Closes #2929.
**Note**: Once these changes are accepted, I'll move the Groovy script to `logging-parent` and implement apache/logging-parent#196 too.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove `transitive` modifier from optional dependencies (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #2930:
URL: https://github.com/apache/logging-log4j2/pull/2930#discussion_r1749797735
##########
log4j-parent/pom.xml:
##########
@@ -995,6 +996,55 @@
</executions>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.gmavenplus</groupId>
+ <artifactId>gmavenplus-plugin</artifactId>
+ <version>${gmavenplus-plugin.version}</version>
+ <configuration>
Review Comment:
Fixed in https://github.com/apache/logging-log4j2/pull/2930/commits/c3db320ab279ece57e97a36a8b7cae52cdf2f338
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove `transitive` modifier from optional dependencies (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2930:
URL: https://github.com/apache/logging-log4j2/pull/2930#discussion_r1749323792
##########
log4j-parent/pom.xml:
##########
@@ -995,6 +996,55 @@
</executions>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.gmavenplus</groupId>
+ <artifactId>gmavenplus-plugin</artifactId>
+ <version>${gmavenplus-plugin.version}</version>
+ <configuration>
Review Comment:
Could you move `configuration` inside the `execution`, please?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove `transitive` modifier from optional dependencies (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz merged PR #2930:
URL: https://github.com/apache/logging-log4j2/pull/2930
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove `transitive` modifier from optional dependencies (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #2930:
URL: https://github.com/apache/logging-log4j2/pull/2930#issuecomment-2339763345
> Question: Shouldn't we instead
>
> * enforce a whitelist of `static` dependencies (so every optional will need to be a conscious decision, not a mistake) and
I am not sure whether to whitelist our optional dependencies or all our **runtime** dependencies. In general there is a mismatch between the scope of Maven dependencies and the OSGi and JPMS optionality.
In general the agreement is that BND should not automatically mark as optional the packages in optional Maven dependencies (see bndtools/bnd#2713), but I believe it could at least warn about **required** packages, which are optional Maven dependencies.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove `transitive` modifier from optional dependencies (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #2930:
URL: https://github.com/apache/logging-log4j2/pull/2930#discussion_r1756633102
##########
log4j-parent/pom.xml:
##########
@@ -995,6 +996,68 @@
</executions>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.gmavenplus</groupId>
+ <artifactId>gmavenplus-plugin</artifactId>
+ <version>${gmavenplus-plugin.version}</version>
+ <dependencies>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-ant</artifactId>
+ <version>${groovy.version}</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy</artifactId>
+ <version>${groovy.version}</version>
+ <scope>runtime</scope>
+ </dependency>
+ </dependencies>
+ <executions>
+ <execution>
+ <id>ban-static-transitive</id>
+ <goals>
+ <goal>execute</goal>
+ </goals>
+ <phase>verify</phase>
+ <configuration>
+ <continueExecuting>false</continueExecuting>
+ <scripts>
+ <script><![CDATA[
+ import java.io.StringWriter
+ import java.util.spi.ToolProvider
+
+ if ("jar" != project.packaging) {
+ log.info('Skipping module descriptor check, since project type is not `jar`.')
+ return
+ }
+ String jarFile = project.build.directory + '/' + project.build.finalName + '.jar';
+ if (!new File(jarFile).exists()) {
+ log.info('Skipping module descriptor check, since `' + project.build.finalName + '.jar` is missing.')
+ return
+ }
+ StringWriter out = new StringWriter()
+ StringWriter err = new StringWriter()
+ ToolProvider jar = ToolProvider.findFirst("jar").orElseThrow()
+ int result = jar.run(new PrintWriter(out), new PrintWriter(err), '-d', '-f', jarFile)
+ if (result != 0) {
+ throw new RuntimeException('Failed to decompile the module descriptor in ' + jarFile + ':\n' + err)
+ }
+ log.debug('Module descriptor: ' + out)
+ for (String line : out.toString().split('\r?\n', -1)) {
+ if (line.contains('static') && line.contains('transitive')) {
+ throw new RuntimeException("The 'static' and 'transitive' modifiers should not be use together: " + line)
+ }
+ }
+ log.info('Successfully verified module descriptor in `' + jarFile + '`.')
Review Comment:
Improve consistency of double-quote usage for string literals:
```suggestion
log.info("Skipping module descriptor check, since the project type is not `jar`.")
return
}
String jarFile = project.build.directory + "/" + project.build.finalName + ".jar";
if (!new File(jarFile).exists()) {
log.info("Skipping module descriptor check, since `" + jarFile + "` is missing.")
return
}
StringWriter out = new StringWriter()
StringWriter err = new StringWriter()
ToolProvider jar = ToolProvider.findFirst("jar").orElseThrow()
int result = jar.run(new PrintWriter(out), new PrintWriter(err), "-d", "-f", jarFile)
if (result != 0) {
throw new RuntimeException("Failed to decompile the module descriptor in `" + jarFile + "`:\n" + err)
}
log.debug("Module descriptor: " + out)
for (String line : out.toString().split("\r?\n", -1)) {
if (line.contains("static") && line.contains("transitive")) {
throw new RuntimeException("The `static` and `transitive` modifiers should not be use together: " + line)
}
}
log.info("Successfully verified module descriptor in `" + jarFile + "`.")
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org