-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PackageValidation runs in no-op incremental builds #23517
Comments
I suspect there is not a better target to hook, but this could be avoided with inputs + outputs. Right now it looks like the only output is MSBuild properties/items/warnings/errors, but the target or task can always touch a marker file to create a new timestamped output. |
Agree, inputs/outputs would be more resilient here. We also need to rerun even if the package isn't rebuilt: for example, someone changes a suppression file or changes the left-hand-side package. |
Fixes #23517 By defining inputs and a semaphore file as the output, PackageValidation doesn't run anymore in an no-op incremental build. In the example of dotnet/runtime, this saves nearly two minutes of unnecessary work.
Fixes #23517 By defining inputs and a semaphore file as the output, PackageValidation doesn't run anymore in an no-op incremental build. In the example of dotnet/runtime, this saves nearly two minutes of unnecessary work.
The PackageValidation SDK feature currently runs always, even if the package hasn't changed:
sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Compatibility.Common.targets
Lines 10 to 12 in 69a8180
This is the case because the
RunPackageValidation
target is hooked onto the wrong target (maybe there's one that doesn't run in no-op incremental packaging builds) and because it doesn't define any Inputs and Outputs.When incrementally building the libraries subset of dotnet/runtime for allconfigurations (
build.cmd libs -allconfigurations && build.cmd libs -allconfigurations /bl
), this adds 111 seconds to the overall build.cc @safern @joperezr @ericstj
The text was updated successfully, but these errors were encountered: