reschke commented on code in PR #2959:
URL: https://github.com/apache/jackrabbit-oak/pull/2959#discussion_r3425681505
##########
oak-blob-cloud-azure/pom.xml:
##########
@@ -285,7 +285,6 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
- <version>33.1.0-jre</version>
Review Comment:
This depencency is embedded intentionally. It matches the version used by
the Azure SDK. Updating it could be possible, but Azure support doesn't have
adequate test coverage over here, so I would leave that to someone more
knowledgeable of the SDK.
For that matter, this dependency will go away when the SDK 8 use is gone.
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
Review Comment:
It doesn't conflict, because these are embeds. One reason for being embeds
is to avoid conflicts.
##########
oak-parent/pom.xml:
##########
@@ -706,6 +724,12 @@
<artifactId>caffeine</artifactId>
<version>3.1.8</version>
</dependency>
+ <!-- guava:33.5.0-jre pulls 2.41.0; caffeine:3.1.8 pulls 2.21.1 -->
Review Comment:
1) Guava will go anyway.
2) It would be good to understand why that shows up as a dependency; given
it's nature as a code checking tool. It might be better to suppress it in the
place where we use Caffeine.
In any case, if it ever disappears as a Caffeine dependeny, this will leave
noise over here.
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
+ Maven uses oak-shaded-guava's original pom.xml rather than
dependency-reduced-pom.xml,
+ so guava:33.5.0-jre also appears as a transitive dep (making guava
optional in
+ oak-shaded-guava would eliminate that false positive, but not the
azure-keyvault conflict). -->
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>33.5.0-jre</version>
+ </dependency>
+ <!-- jackrabbit-core:2.22.3 pulls oak-jackrabbit-api:1.88.0 -->
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-jackrabbit-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.12.0; api-all:2.0.1 pulls 2.8.0
-->
+ <dependency>
+ <groupId>org.apache.commons</groupId>
Review Comment:
This is embedded for good reasons.
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
+ Maven uses oak-shaded-guava's original pom.xml rather than
dependency-reduced-pom.xml,
+ so guava:33.5.0-jre also appears as a transitive dep (making guava
optional in
+ oak-shaded-guava would eliminate that false positive, but not the
azure-keyvault conflict). -->
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>33.5.0-jre</version>
+ </dependency>
+ <!-- jackrabbit-core:2.22.3 pulls oak-jackrabbit-api:1.88.0 -->
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-jackrabbit-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.12.0; api-all:2.0.1 pulls 2.8.0
-->
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-pool2</artifactId>
+ <version>2.12.0</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.1.12; api-all:2.0.1 pulls 2.1.3
-->
+ <dependency>
+ <groupId>org.apache.mina</groupId>
+ <artifactId>mina-core</artifactId>
Review Comment:
embedded.
##########
oak-run-commons/pom.xml:
##########
@@ -141,6 +141,10 @@
<groupId>org.apache.lucene</groupId>
<artifactId>lucene-core</artifactId>
</exclusion>
+ <!-- jackrabbit-core:2.22.3 (via jackrabbit-parent) declares
tika.version=2.4.1,
Review Comment:
Actually, the problem was that there *was* embedding, and that was caused by
a plugin version change (probably fixing a bug).
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
+ Maven uses oak-shaded-guava's original pom.xml rather than
dependency-reduced-pom.xml,
+ so guava:33.5.0-jre also appears as a transitive dep (making guava
optional in
+ oak-shaded-guava would eliminate that false positive, but not the
azure-keyvault conflict). -->
Review Comment:
we are removing Guava.
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
+ Maven uses oak-shaded-guava's original pom.xml rather than
dependency-reduced-pom.xml,
+ so guava:33.5.0-jre also appears as a transitive dep (making guava
optional in
+ oak-shaded-guava would eliminate that false positive, but not the
azure-keyvault conflict). -->
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>33.5.0-jre</version>
+ </dependency>
+ <!-- jackrabbit-core:2.22.3 pulls oak-jackrabbit-api:1.88.0 -->
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-jackrabbit-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.12.0; api-all:2.0.1 pulls 2.8.0
-->
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-pool2</artifactId>
+ <version>2.12.0</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.1.12; api-all:2.0.1 pulls 2.1.3
-->
+ <dependency>
+ <groupId>org.apache.mina</groupId>
+ <artifactId>mina-core</artifactId>
+ <version>2.1.12</version>
+ </dependency>
+ <!-- parsson:1.0.5 pulls 2.0.2; elasticsearch-java:8.19.5 pulls 2.0.1 -->
+ <dependency>
+ <groupId>jakarta.json</groupId>
+ <artifactId>jakarta.json-api</artifactId>
+ <version>2.0.2</version>
+ </dependency>
+
+ <!-- jackrabbit-jcr-server:2.22.3 pulls tika-core:2.4.1; oak-lucene uses
${tika.version} -->
+ <dependency>
+ <groupId>org.apache.tika</groupId>
+ <artifactId>tika-core</artifactId>
+ <version>${tika.version}</version>
+ </dependency>
+ <!-- jetty-annotations (via spring-boot-starter-jetty:2.7.18) pulls 9.6;
tika-parsers:1.28.5 pulls 9.3 -->
+ <dependency>
+ <groupId>org.ow2.asm</groupId>
+ <artifactId>asm</artifactId>
+ <version>9.6</version>
+ </dependency>
+ <!-- poi:5.2.2 and poi-scratchpad:5.2.2 pull 2.17.2; xmlbeans:5.0.3 (via
poi-ooxml) pulls 2.17.1 -->
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-api</artifactId>
+ <version>2.17.2</version>
+ </dependency>
+ <!-- oak-run has a direct dep on 1.3.4; aws-java-sdk-core:1.12.791 pulls
1.1.3, httpclient pulls 1.2 -->
+ <dependency>
+ <groupId>commons-logging</groupId>
+ <artifactId>commons-logging</artifactId>
+ <version>1.3.4</version>
+ </dependency>
+ <!-- azure-identity:1.11.3 pulls 5.13.0; tika-parsers:1.28.5 pulls
5.12.1 -->
+ <dependency>
+ <groupId>net.java.dev.jna</groupId>
+ <artifactId>jna</artifactId>
+ <version>5.13.0</version>
+ </dependency>
+ <!-- jackson-dataformat-xml:2.19.4 (via oak-segment-azure) pulls 7.1.1;
tika-parsers:1.28.5 pulls 6.3.1 -->
+ <dependency>
Review Comment:
we are updating Tika
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
+ Maven uses oak-shaded-guava's original pom.xml rather than
dependency-reduced-pom.xml,
+ so guava:33.5.0-jre also appears as a transitive dep (making guava
optional in
+ oak-shaded-guava would eliminate that false positive, but not the
azure-keyvault conflict). -->
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>33.5.0-jre</version>
+ </dependency>
+ <!-- jackrabbit-core:2.22.3 pulls oak-jackrabbit-api:1.88.0 -->
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-jackrabbit-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.12.0; api-all:2.0.1 pulls 2.8.0
-->
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-pool2</artifactId>
+ <version>2.12.0</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.1.12; api-all:2.0.1 pulls 2.1.3
-->
+ <dependency>
+ <groupId>org.apache.mina</groupId>
+ <artifactId>mina-core</artifactId>
+ <version>2.1.12</version>
+ </dependency>
+ <!-- parsson:1.0.5 pulls 2.0.2; elasticsearch-java:8.19.5 pulls 2.0.1 -->
Review Comment:
I just don't see how this matters. We should discuss this in the ticket.
##########
oak-run/pom.xml:
##########
@@ -53,7 +53,7 @@
+ 41 MB build failing on the release profile (OAK-6250)
+ 38 MB. Initial value. Current 35MB plus a 10%
-->
- <max.jar.size>92274688</max.jar.size>
+ <max.jar.size>93323264</max.jar.size>
Review Comment:
why did it increase? (just trying to understand)
##########
oak-segment-azure/pom.xml:
##########
@@ -330,7 +330,6 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
- <version>33.1.0-jre</version>
</dependency>
Review Comment:
No. Embedded for good reasons.
##########
oak-examples/webapp/pom.xml:
##########
@@ -111,7 +111,7 @@
<dependency>
<groupId>com.googlecode.json-simple</groupId>
<artifactId>json-simple</artifactId>
- <version>1.1</version>
Review Comment:
Yes, but should be a separate ticket; I can do that.
##########
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/tika/CSVFileGenerator.java:
##########
@@ -46,6 +46,8 @@ public void generate(FluentIterable<BinaryResource> binaries)
throws IOException
CSVPrinter printer = new CSVPrinter(new BufferedWriter(new
FileWriter(outFile, StandardCharsets.UTF_8)),
CSVFileBinaryResourceProvider.FORMAT);
closer.register(printer);
+ // commons-csv 1.2+ no longer auto-writes the header on
construction; must be explicit
Review Comment:
When a code change is needed, that's a good indication that it should happen
in a separate ticket.
I can do that.
##########
oak-parent/pom.xml:
##########
@@ -855,6 +879,166 @@
<artifactId>jsr305</artifactId>
<version>3.0.2</version>
</dependency>
+ <!-- azure-keyvault-core:1.2.6 pulls guava:30.1.1-jre, conflicting with
the direct guava
+ deps in oak-blob-cloud-azure and oak-segment-azure. Additionally,
in reactor builds
+ Maven uses oak-shaded-guava's original pom.xml rather than
dependency-reduced-pom.xml,
+ so guava:33.5.0-jre also appears as a transitive dep (making guava
optional in
+ oak-shaded-guava would eliminate that false positive, but not the
azure-keyvault conflict). -->
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>33.5.0-jre</version>
+ </dependency>
+ <!-- jackrabbit-core:2.22.3 pulls oak-jackrabbit-api:1.88.0 -->
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-jackrabbit-api</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.12.0; api-all:2.0.1 pulls 2.8.0
-->
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-pool2</artifactId>
+ <version>2.12.0</version>
+ </dependency>
+ <!-- oak-auth-ldap has a direct dep on 2.1.12; api-all:2.0.1 pulls 2.1.3
-->
+ <dependency>
+ <groupId>org.apache.mina</groupId>
+ <artifactId>mina-core</artifactId>
+ <version>2.1.12</version>
+ </dependency>
+ <!-- parsson:1.0.5 pulls 2.0.2; elasticsearch-java:8.19.5 pulls 2.0.1 -->
+ <dependency>
+ <groupId>jakarta.json</groupId>
+ <artifactId>jakarta.json-api</artifactId>
+ <version>2.0.2</version>
+ </dependency>
+
+ <!-- jackrabbit-jcr-server:2.22.3 pulls tika-core:2.4.1; oak-lucene uses
${tika.version} -->
+ <dependency>
+ <groupId>org.apache.tika</groupId>
+ <artifactId>tika-core</artifactId>
+ <version>${tika.version}</version>
+ </dependency>
+ <!-- jetty-annotations (via spring-boot-starter-jetty:2.7.18) pulls 9.6;
tika-parsers:1.28.5 pulls 9.3 -->
+ <dependency>
+ <groupId>org.ow2.asm</groupId>
+ <artifactId>asm</artifactId>
+ <version>9.6</version>
+ </dependency>
Review Comment:
See above. These are no problems technically. This change just adds noise
here. We would need to maintain the reference here, plus remove it when it's
not needed anymore.
Do we have tooling for 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]