-
-
Notifications
You must be signed in to change notification settings - Fork 371
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
Overhaul transitive module handling in dependency resolution #4145
Changes from all commits
95f49f9
a561284
9434d99
ca347cc
b1eff1c
24714ed
e1c1daa
275f54d
5c90bb0
1a8a3d2
c6c8a7e
191bf54
d9195df
6f39c72
31f610d
4f293a3
aa5e418
2098054
feda739
b970a90
e1c02eb
8b8db3c
9c2e634
84d8a4d
10601aa
222efe7
90f93e9
1f46568
7ad6ff4
db7af84
8f8ecb0
c5dc629
e1f5d8f
a122015
30462a2
5527736
847462b
b9ee650
4c0a06f
b5611cf
2c6b737
da17946
7ac0124
0d8d34d
536d958
685fb8d
1ab71c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,10 +44,12 @@ trait Static extends ScalaModule { | |
} | ||
|
||
/** | ||
* webjar dependencies - created from transitive ivy deps | ||
* webjar dependencies - created from ivy deps | ||
*/ | ||
def webJarDeps = Task { | ||
transitiveIvyDeps().filter(_.dep.module.organization.value == "org.webjars") | ||
ivyDeps() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This slightly changes the semantics of the previous code. Before, Alternatively, we could have a look at the whole dependency graph (including external transitive dependencies). But only looking at direct dependencies of the Mill modules looks odd to me (what if the Mill project is split or is a merge of two Mill projects - that changes the value of this task, while it shouldn't IMO). |
||
.filter(_.dep.module.organization.value == "org.webjars") | ||
.map(bindDependency()) | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,10 @@ | |
{ | ||
"name": "org.slf4j:slf4j-api", | ||
"version": "2.0.16" | ||
}, | ||
{ | ||
"name": "org.hamcrest:hamcrest-core", | ||
"version": "1.3" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The changes in this file are a good thing: rather than including only dependencies directly added in Mill modules (via |
||
} | ||
] | ||
}, | ||
|
@@ -44,6 +48,10 @@ | |
{ | ||
"name": "org.jetbrains.kotlin:kotlin-stdlib", | ||
"version": "<kotlin-version>" | ||
}, | ||
{ | ||
"name": "org.jetbrains:annotations", | ||
"version": "13.0" | ||
} | ||
] | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ | |
<orderEntry type="library" name="scala-js-SDK-3.3.1" level="project"/> | ||
<orderEntry type="library" name="portable-scala-reflect_sjs1_2.13-1.1.3.jar" level="project"/> | ||
<orderEntry type="library" name="scala-library-2.13.13.jar" level="project"/> | ||
<orderEntry type="library" name="scala3-library_sjs1_3-3.3.1.jar" level="project"/> | ||
<orderEntry type="library" name="scala3-library_sjs1_3-3.3.3.jar" level="project"/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't investigate why the version is bumped here. That doesn't look like a major issue though… |
||
<orderEntry type="library" name="scalajs-javalib-1.16.0.jar" level="project"/> | ||
<orderEntry type="library" name="scalajs-library_2.13-1.16.0.jar" level="project"/> | ||
<orderEntry type="library" name="scalajs-scalalib_2.13-2.13.13%2B1.16.0.jar" level="project"/> | ||
|
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.
@alexarchambault these turn out not to be safe, we probably need to fix the code to not add calls to
super
since they are not binary compatibleCC @sake92
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.
I'm not sure I said it before, but I suggest to generate the access to
super.<overridden>
from our macros as soon as we can break bin-compat, so that we better prepare the binary code for ad-hoc overriding of tasks. The super-calls can be left unused, so they should not affect functionality, but we at least we trigger the super-accessors to be present in the binary code.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.
Trying to address that in #4380, although the workarounds aren't fully satisfying IMO (they assume users don't add overrides between the trait defining the method and the one overriding it)
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.
Other such changes adding calls to
super
have been introduced in the past, apparently:mill/build.mill
Lines 523 to 580 in 9815c35