search

apache / shiro

Apache Shiro
https://shiro.apache.org/
Apache License 2.0
4.32k stars 2.31k forks source link

[Bug] [1.12.0] BOM is configured wrong #1014

Closed boris-petrov closed 1 year ago

boris-petrov commented 1 year ago

Search before asking

Environment

Not relevant.

Shiro version

1.12.0

What was the actual outcome?

See below.

What was the expected outcome?

See below.

How to reproduce

Add this in a build.gradle file:

plugins {
        id 'java'
}

repositories {
        mavenCentral()
}

dependencies {
        implementation(platform('org.apache.shiro:shiro-bom:1.12.0'))
        implementation 'org.apache.shiro:shiro-web'
}

And in src/main/java/Test.java:

import org.apache.shiro.subject.Subject;

public class Test {
}

Running gradle clean compileJava will fail. If you run it but first change the version number to 1.11.0 it will work fine.

Run gradle dependencies --configuration compileClasspath. This leads to:

compileClasspath - Compile classpath for source set 'main'.
+--- org.apache.shiro:shiro-bom:1.12.0
|    +--- org.apache.shiro:shiro-web:1.12.0 (c)
|    \--- org.owasp.encoder:encoder:1.2.3 (c)
\--- org.apache.shiro:shiro-web -> 1.12.0
     \--- org.owasp.encoder:encoder:1.2.3

As you see, shiro-core is missing as a dependency and hence the project won't compile. Using version 1.11.0 leads to a correct output. Or so I think. I'm not sure I understand correctly how BOMs are supposed to work but I believe this is a problem in version 1.12.0.

I see in 1.12.0 that a dependencyManagement tag has been added which is not present in 1.11.0. Not sure if that's the problem.

Debug logs

No response

fpapon commented 1 year ago

It's working with maven, you can take a look at the project sample here:

https://github.com/apache/shiro/blob/shiro-root-1.12.0/samples/web-jakarta/pom.xml

You need to use the classifier jakarta for the dependencies.

boris-petrov commented 1 year ago

@fpapon I'm really not sure what you mean. I don't want the Jakarta version of Shiro, I want the old javax world. I updated the comment with a simple reproduction. This simple project doesn't compile on 1.12.0 but does on 1.11.0.

fpapon commented 1 year ago

So you don't need the BOM for the javax namespace, the BOM is provided only for jakarta to avoid including non jakarta Shiro jar.

boris-petrov commented 1 year ago

hm, that's unfortunate. Why not also provide a BOM for the non-jakarta version? It's still useful - allows me to not specify explicitly a version for all the Shiro dependencies.

fpapon commented 1 year ago

Users never ask for that but if you think that it could be useful, feel free to create an issue to add this feature.

boris-petrov commented 1 year ago

Done!.

lprimak commented 1 year ago

Hi, @boris-petrov I have reproduced your issue.

You are correct in diagnosing the Shiro change. However, the Shiro change is correct. <dependencyManagement> is the correct tag for BOM files. It was incorrect before.

First of all, you should remove the BOM line from your gradle file. It's not needed at all. That's the simplest fix to your issue. I tested it and it brings in all transitive dependencies correctly.

If, for some reason, you do need to use BOM (i.e. Jakarta artifacts), the correct usage for maven BOM is:

plugins {
        id 'java'
        id 'org.springframework.boot' version '3.1.2'
}

repositories {
        mavenCentral()
}

apply plugin: 'io.spring.dependency-management'

dependencyManagement {
  imports {
    mavenBom 'org.apache.shiro:shiro-bom:1.12.0'
  }
}

dependencies {
        implementation 'org.apache.shiro:shiro-web'
}

With Gradle, the "BOM" term is overloaded. "Gradle BOM" is not the same as "Maven BOM" which is what Shiro is.

I am going to close this issue, please feel free to reopen if the above solution doesn't work for you for some reason.

boris-petrov commented 1 year ago

@lprimak thanks for all the time you spent on this!

I'm really not sure what you mean though. As far as I understand, BOM files are just a list of dependency-versions. And that's what I used them for. I added the BOM and then in the couple of places I used Shiro, I simply didn't add a version. In addition to that, BOMs define versions for transitive dependencies that are known/needed to work with the library. I definitely could use Shiro without a BOM (that's what I did for 1.12.0) - it's just simpler with it. And DRY-er because I'm not repeating versions.

You're right that maybe the BOM in version 1.11.0 was "wrong" in some way, not sure about that. What I request is pretty much the same BOM as in 1.12.0 to be available to the non-Jakarta version of Shiro - i.e. (I think) just without the <classifier>jakarta</classifier> lines.

Does that make sense or am I mistaking some concepts? :smile:

lprimak commented 1 year ago

I think I was able to get to the bottom of your issue more. Looks like gradle has a bug handling BOMs via implementation platform('XXX') with classifiers (e.g. jakarta) It ignores classifiers. Hence your original gradle build excludes all transitive dependencies from Shiro. My version (via imports { mavenBom {} }) that works with Spring plugin, handles this correctly.

Have you tried the way I described? (i.e. Spring plugin for BOM) It should work.

However I highly discourage the use of BOM this way. There is simply no need for BOM when using javax artifacts. I understand your "wanting" the DRY, but Gradle has it's own method to specify versions in a DRY way, so I would suggest you do that:

plugins {
        id 'java'
}

repositories {
        mavenCentral()
}

configurations.all {
  resolutionStrategy.eachDependency { details ->
    if (details.requested.group == 'org.apache.shiro') {
      details.useVersion "1.12.0"
    }
  }
}

dependencies {
        implementation "org.apache.shiro:shiro-web"
}

results:

> Task :dependencies

------------------------------------------------------------
Root project 'shiro-issue-jul20'
------------------------------------------------------------

compileClasspath - Compile classpath for source set 'main'.
\--- org.apache.shiro:shiro-web -> 1.12.0
     +--- org.apache.shiro:shiro-core:1.12.0
     |    +--- org.apache.shiro:shiro-lang:1.12.0
     |    |    \--- org.slf4j:slf4j-api:1.7.36
     |    +--- org.apache.shiro:shiro-cache:1.12.0
     |    |    \--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    +--- org.apache.shiro:shiro-crypto-hash:1.12.0
     |    |    +--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    |    \--- org.apache.shiro:shiro-crypto-core:1.12.0
     |    |         \--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    +--- org.apache.shiro:shiro-crypto-cipher:1.12.0
     |    |    +--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    |    \--- org.apache.shiro:shiro-crypto-core:1.12.0 (*)
     |    +--- org.apache.shiro:shiro-config-core:1.12.0
     |    |    \--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    +--- org.apache.shiro:shiro-config-ogdl:1.12.0
     |    |    +--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    |    +--- org.apache.shiro:shiro-config-core:1.12.0 (*)
     |    |    +--- org.apache.shiro:shiro-event:1.12.0
     |    |    |    \--- org.apache.shiro:shiro-lang:1.12.0 (*)
     |    |    +--- commons-beanutils:commons-beanutils:1.9.4
     |    |    |    \--- commons-collections:commons-collections:3.2.2
     |    |    \--- org.slf4j:slf4j-api:1.7.36
     |    \--- org.apache.shiro:shiro-event:1.12.0 (*)
     \--- org.owasp.encoder:encoder:1.2.3
boris-petrov commented 1 year ago

Thanks for the detailed explanations! I agree that this way of DRY-ing up dependency versions also works fine and is "good" Gradle style. I just thought that when there's a BOM, I could use it (as is the case with other projects). But it really doesn't matter that much. So thanks for the time!