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

Logbook 3.7.0 pulls in Lombok as transitive dependency #1705

Closed
bwaldvogel opened this issue Dec 6, 2023 · 9 comments
Closed

Logbook 3.7.0 pulls in Lombok as transitive dependency #1705

bwaldvogel opened this issue Dec 6, 2023 · 9 comments
Labels

Comments

@bwaldvogel
Copy link

When upgrading Logbook from 3.6.0 to 3.7.0 we get a new transitive dependency pulled in: org.projectlombok:lombok:1.18.30.
For projects that are not using Lombok, that causes undesired effects. For example, IntelliJ IDEA auto-discovers that Lombok is used in the project and starts suggesting the use of Lombok annotations.

Steps to reproduce

  1. Create a new Gradle / Spring Boot project (use start.spring.io or IntelliJ’s native integration)
  2. Add the following dependency: org.zalando:logbook-spring-boot-starter:3.7.0

Actual behavior

./gradlew -q dependencyInsight --dependency lombok shows that Lombok is pulled in:

org.projectlombok:lombok:1.18.30
\--- org.zalando:logbook-spring-boot-autoconfigure:3.7.0
     \--- org.zalando:logbook-spring-boot-starter:3.7.0

When downgrading from 3.7.0 to 3.6.0, the Lombok dependency disappears.

Expected behavior

The Lombok is not pulled in as it was in version 3.6.0.

@bwaldvogel bwaldvogel added the Bug label Dec 6, 2023
@kasmarian
Copy link
Member

@msdousti it looks like this change started pulling lombok as a compile dependency, because it's declared as such in spring-boot-dependencies. Do you think we can revert to using spring-framework-bom? Alternatively, we'd need to override the scope for lombok (potentially other dependencies?) in logbook-spring-boot-autoconfigure module directly.

@msdousti
Copy link
Collaborator

msdousti commented Dec 6, 2023

@kasmarian
There are several things going on that need a deeper look.

  1. The change you mentioned only touches the dependencyManagement section. By itself, dependencyManagement will not include dependencies in a project; it just says that if a dependency is ever requested, this specific version must be used.

  2. I tried creating a Gradle and a Maven project including the Steps to reproduce. The Gradle project is as mentioned in the ticket, but the Maven project behaves as expected: It does not include Lombok - mvn dependency:tree -Dincludes=org.projectlombok returns nothing. See for yourself here.

As a side note, depending on spring-framework-bom in a Spring Boot project is not right, since the BOM file for Spring Framework often includes newer material that might not be compatible with the current version of Spring Boot. When the project does not have Spring Boot as its parent (like here), it's customary to include spring-boot-dependencies in the dependencyManagement section.

I'll try to dig deeper why Gradle is not behaving as expected.

@kasmarian
Copy link
Member

I also noticed that maven project doesn't pull lombok, still, with gradle this only happens for 3.0.7 version.

The change you mentioned only touches the dependencyManagement section. By itself, dependencyManagement will not include dependencies in a project; it just says that if a dependency is ever requested, this specific version must be used.

In the parent project, we do specify lombok without a specific scope. Can it be that the scope from spring-boot-dependencies is used in case of a gradle project instead of the scope from logbook-parent's dependencyManagement?
What if we define the scope of lombok in parent's pom in <dependencies> directly and not under <dependencyManagement> ? I agree, knowing the root cause for this would be interesting. Just throwing the ideas for now.

@msdousti
Copy link
Collaborator

msdousti commented Dec 6, 2023

@kasmarian I believe the the above PR (#1706) fixes this.

@msdousti
Copy link
Collaborator

msdousti commented Dec 6, 2023

@kasmarian

PS: Th answers to this SO question nail it:

Anyway, save yourself the hassle, and leave scope out of your dependencyManagement.

dependencyManagement is just here to define dependencies version for all project submodules, the only pertinent scope in this section is import for BOMs.
Scope must be defined in dependencies section.

I'd say we should review the parent project based on this observation.

PS: Did this in my PR.

@kasmarian
Copy link
Member

We released 3.7.1 where this should be addressed. May take some time until the release is available in repositories

@msdousti msdousti closed this as completed Dec 8, 2023
@bwaldvogel
Copy link
Author

bwaldvogel commented Dec 11, 2023

Thanks for fixing this so quickly. Version 3.7.1 does not pull in lombok anymore.
However, it pulls in a couple of new dependencies to the runtimeClasspath that shouldn’t be pulled in, such as hamcrest.

./gradlew -q dependencyInsight --dependency hamcrest --configuration runtimeClasspath
org.hamcrest:hamcrest:2.2
\--- org.zalando:logbook-spring-boot-autoconfigure:3.7.1
     \--- org.zalando:logbook-spring-boot-starter:3.7.1
          \--- runtimeClasspath

Shall I create a new issue or reopen this one?

@msdousti
Copy link
Collaborator

@bwaldvogel Thanks a lot for your report!

It will be great if you create a new ticket mentioning all those "badly scoped" dependencies; I'll then fix them in one PR.

@bwaldvogel
Copy link
Author

Please see #1711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants