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

Fix jackson sbt assembly duplicate depends issue #867

Closed
wants to merge 1 commit into from
Closed

Fix jackson sbt assembly duplicate depends issue #867

wants to merge 1 commit into from

Conversation

alikemalocalan
Copy link

Problem
Fixes #855

Explain the context and why you're making that change. What is the
problem you're trying to solve? In some cases there is not a problem
and this can be thought of being the motivation for your change.

Solution

Describe the modifications you've done.

Result

What will change as a result of your pull request? Note that sometimes
this section is unnecessary because it is self-explanatory based on
the solution.

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2020

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2020

Codecov Report

Merging #867 into develop will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #867      +/-   ##
===========================================
- Coverage    77.45%   77.43%   -0.03%     
===========================================
  Files          814      814              
  Lines        24111    24111              
  Branches      1597     1599       +2     
===========================================
- Hits         18676    18671       -5     
- Misses        5435     5440       +5     
Impacted Files Coverage Δ
...n/scala/com/twitter/finagle/http2/Exceptions.scala 0.00% <0.00%> (-50.00%) ⬇️
...gle/http2/transport/client/ClientSessionImpl.scala 79.54% <0.00%> (-11.37%) ⬇️
...gle/http2/transport/client/ClientServiceImpl.scala 81.81% <0.00%> (-9.10%) ⬇️
...com/twitter/finagle/netty4/ConnectionBuilder.scala 75.80% <0.00%> (-1.62%) ⬇️
.../com/twitter/finagle/tracing/BroadcastTracer.scala 66.66% <0.00%> (ø)
...om/twitter/finagle/dispatch/ServerDispatcher.scala 85.10% <0.00%> (+2.12%) ⬆️
...m/twitter/finagle/exp/ConcurrencyLimitFilter.scala 91.42% <0.00%> (+2.85%) ⬆️
...tter/finagle/dispatch/SerialClientDispatcher.scala 100.00% <0.00%> (+16.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80d119f...ec84e1f. Read the comment docs.

@ryanoneill
Copy link
Contributor

Hi @alikemalocalan, thanks for the submission. Sorry that I missed it earlier this week. I'm catching up on the details now.

Copy link
Contributor

@ryanoneill ryanoneill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other projects contain a dependency on jackson for perhaps not an obvious reason. For our open source projects like Finagle (which includes Finatra, Util, Scrooge, and TwitterServer), there are two build systems. One of which we use externally, sbt, while the other we use internally, pants -> eventually bazel. With that said, pants has stricter rules on the use of transitive dependencies, especially when types are used or exported from that dependency. So it's easiest for maintenance and management if our dependencies for our sbt projects match those closely with the ones for pants. That's why it might not be obvious why some of these projects contain a dependency on jackson.

I'm curious as to how changing things in this way fixes your issue.

@@ -497,8 +497,7 @@ lazy val finagleException = Project(
name := "finagle-exception",
libraryDependencies ++= Seq(
util("codec")
) ++ scroogeLibs,
libraryDependencies ++= jacksonLibs
) ++ scroogeLibs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finagle-exception uses jackson directly, so it really should have a dependency on jackson.

@@ -520,7 +519,6 @@ lazy val finagleServersets = Project(
),
"commons-lang" % "commons-lang" % "2.6"
),
libraryDependencies ++= jacksonLibs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finagle-serversets does too, so it should too.

@@ -720,7 +716,7 @@ lazy val finagleMySQL = Project(
util("stats"),
caffeineLib,
jsr305Lib
) ++ jacksonLibs,
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finagle-mysql too.

@bryce-anderson
Copy link
Contributor

@alikemalocalan, are you interested in addressing Ryan's feedback?

@alikemalocalan
Copy link
Author

hi @bryce-anderson , i understood explanation of @ryanoneill , my solution isn't useful for like this project. Maybe next years , if you will find more simple sbt sub-module solution, i can help you :D

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

Successfully merging this pull request may close these issues.

Sbt assembly deduplicate error
5 participants