[Build] Make ThirdPartyAudit task uptodate more effective #118723
Conversation
breskeby
added
:Delivery/Build
breskeby
added
v9.0.0
v8.17.1
v8.18.0
auto-merge-without-approval
breskeby
force-pushed
the
make-thirdpartycheckinputseffective
branch
from
78d9dfd to
51e229e
Compare
|
Here's a before after
|
breskeby
changed the title
|
Pinging @elastic/es-delivery (Team:Delivery) |
breskeby
changed the title
| * project dependencies that are not shadowed jars. | ||
| * Basically a thirdparty only view of the dependency tree. | ||
| */ | ||
| public static FileCollection thirdPartyDependenciesView(Configuration configuration) { |
There was a problem hiding this comment.
Doesn't calling this effectively trigger dependency resolution? I think we might be implicitly relying on lazy task configuration to not eagerly resolve dependencies when we shouldn't but I think we might need to wrap this in a Provider or something to be totally proper.
There was a problem hiding this comment.
The artifactview handling itself is lazy. But there was indeed an oversight in the util method implementation where I resolved the provider eagerly and the dependency infos (not the artifacts) have been resolved eagerly. fixed this here: 5f507b0
now those configurations are really only resolved at task graph calculation time.
One issue is that I have configuration cache enabled by default for testing locally and this makes dependency resolution timings differ from the normal way that makes it easier to oversee this 🤔
There was a problem hiding this comment.
But doesn't the call to getFiles() immediately trigger resolution? Where's the lazy bit?
There was a problem hiding this comment.
@mark-vieira getFiles only gives you back file collection that is lazy as configuration itself is. the resolution is kicking only in when that file collection is resolved. like a configuration is implementing filecollection too. you might mix that up with FileCollection#getFiles like Configuration#getFiles returning a Set.
Trust me bro...
And in case you don't ....
I double checked manually by throwing in exception using the beforeResolve hook and that shows how only the task graph building triggered the resolution: https://gradle-enterprise.elastic.co/s/n6igninpsi3m4/failure?expanded-stacktrace=WyIwIl0&focused-stack-frame=0-28#1
There was a problem hiding this comment.
I need to think if there is a way we can enforce failing builds or at least have a regular check running to ensure we don't resolve anything too early. One option might be to verify https://gradle-enterprise.elastic.co/s/6bqzp2o7mq6k2/performance/dependency-resolution#dependency-resolution-configuration-user-initiated programatically somehow.
There was a problem hiding this comment.
@mark-vieira getFiles only gives you back file collection that is lazy as configuration itself is. the resolution is kicking only in when that file collection is resolved. like a configuration is implementing filecollection too. you might mix that up with FileCollection#getFiles like Configuration#getFiles returning a Set.
Ah, gotcha. That makes sense.
There was a problem hiding this comment.
One option might be to verify https://gradle-enterprise.elastic.co/s/6bqzp2o7mq6k2/performance/dependency-resolution#dependency-resolution-configuration-user-initiated programatically somehow.
Indeed. I wonder if that's detected during the built itself, or just inferred based on resolution event data after the fact.
Filtering out the project dependencies allows way better uptodate and caching behaviour. We are only interested in thirdparty libs anyhow in this context.
breskeby
force-pushed
the
make-thirdpartycheckinputseffective
branch
from
fade149 to
a2db848
Compare
we do not want to do any dependency resolution preemptively
breskeby
force-pushed
the
make-thirdpartycheckinputseffective
branch
from
a2db848 to
5f507b0
Compare
…8723) We should basically be able to skip the third party dependency audit tasks if no third party dependency has changed. Filtering out the project dependencies allows way better uptodate and caching behaviour. We are only interested in thirdparty libs anyhow in this context. A positive side effect of the reduced class path is a quicker execution of the task
…8723) We should basically be able to skip the third party dependency audit tasks if no third party dependency has changed. Filtering out the project dependencies allows way better uptodate and caching behaviour. We are only interested in thirdparty libs anyhow in this context. A positive side effect of the reduced class path is a quicker execution of the task
We should basically be able to skip the third party dependency audit tasks if no third party dependency has changed.
Filtering out the project dependencies allows way better uptodate and caching behaviour. We are only interested in thirdparty libs anyhow in this context.
A positive side effect of the reduced class path is a quicker execution of the task