-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-53958][BUILD] Simplify Jackson deps management by using BOM #52668
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
Conversation
<scalafmt.validateOnly>true</scalafmt.validateOnly> | ||
<scalafmt.changedOnly>true</scalafmt.changedOnly> | ||
<fasterxml.jackson.version>2.19.2</fasterxml.jackson.version> | ||
<fasterxml.jackson.databind.version>2.19.2</fasterxml.jackson.databind.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case that jackson-databind
has a standalone patched version, the jackson-bom
also has a new release, for example
https://mvnrepository.com/artifact/com.fasterxml.jackson/jackson-bom/2.13.4.20221013
I hit some Jackson deps issues (#52664 (comment)) in upgrading Avro to 1.12.1, and haven't figured out the root cause, so I am looking into this new simple approach to address the issues. cc @LuciferYang @sarutak @dongjoon-hyun WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you revert all elimination of <exclusion>
tags from this PR?
I believe this PR should work without touching the existing exclusion
rules in order to verify that BOM
is actually working correctly, @pan3793 .
@dongjoon-hyun, either w/ or w/o the elimination of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. Thank you for revising this PR, @pan3793 .
Merged to master for Apache Spark 4.1.0-preview3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Late LGTM.
What changes were proposed in this pull request?
Currently, it's relatively complicated with dependency version management due to the different resolution mechanisms between Maven and SBT.
Take Jackson deps as an example, to ensure all Jackson deps have the same version on the classpath, for both Maven and SBT, we must exclude all transitive Jackson deps and re-declare them manually, otherwise SBT may pick the higher version from transitive deps then breaks the building.
This PR proposes to use BOM to simplify the above process, see technical details at https://github.com/FasterXML/jackson-bom
Maven has built-in support for BOM, SBT also has a plugin https://github.com/heremaps/here-sbt-bom
Why are the changes needed?
Simplify dependency management.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass GHA.
Manually checked the output of
build/sbt dependencyTree
, all Jackson deps are correctly resolved to the expected version - 2.19.2.Was this patch authored or co-authored using generative AI tooling?
No.