Add Implicit restore to run/build/publish/pack/test #6762
Conversation
|
@emgarten with the current state of the PR, they can pass the restore properties using the msbuild additional arguments. But I intend to add restore options to the commands that do implicit restore as well. |
livarcocc
force-pushed
the
implicit_restore
branch
from
ee032de to
070637d
Compare
|
@dotnet-bot Test Ubuntu x64 Release Build |
livarcocc
changed the title
|
@dotnet-bot Test RHEL7.2 x64 Release Build |
|
@dotnet-bot Test Windows_NT x64 Release Build |
|
@MattGertz for approval Customer scenario This command adds implicit restore invocations to build/publish/pack/run/test. This way, the user won't have to remember to manually run restore whenever he makes a dependency change to the project. Bugs this fixes: https://github.com/dotnet/cli/issues/6105 Workarounds, if any Run restore manually before the commands that use implicit restore. Risk Low. Performance impact We will run restore now before every build/run/publish/pack/test commands. So, there will be a performance impact for these commands, depending on how fast NOOP restore is. We are providing a --no-restore flag as a escape valve. Is this a regression from a previous update? N/A Root cause analysis: N/A How was the bug found? Planned feature. |
|
|
||
| RestoreCommandParser.AddImplicitRestoreOptions(fullBuildOptions); | ||
|
|
||
| return fullBuildOptions.ToArray(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
|
||
| private IEnumerable<string> ArgsToForwardToRestore | ||
| { | ||
| get |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
| CommonOptions.VerbosityOption() | ||
| }; | ||
|
|
||
| RestoreCommandParser.AddImplicitRestoreOptions(fullBuildOptions); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
livarcocc
force-pushed
the
implicit_restore
branch
2 times, most recently
from
b56194a to
ac1ebab
Compare
|
Diff with ?w=1 (ignore whitespace) looks good, but there's some indentation that's off that's making the diff hard to see. Please fix it and rebase/squash. |
… /v was not entirely honored. Adding tests for implicit restore for all the affected commands. Fixing an issue where the command target was being passed to the restore command during implicit restore. Adding restore params to all commands with implicit restore. Also, implicitly set the restore output to quiet. Adding tests for the no-restore option.
livarcocc
force-pushed
the
implicit_restore
branch
4 times, most recently
from
f8247f1 to
3eaf727
Compare
livarcocc
force-pushed
the
implicit_restore
branch
from
3eaf727 to
c896186
Compare
The missing piece here is adding restore options to the commands that support implicit restore, so that you can do things like dotnet build --configfile <path_to_config_file> instead of dotnet build /p:RestoreConfigFile=<path_to_config_file>. Also note that all build arguments will be passed down to restore.
For this, I debated not passing them along, but discussed with @emgarten and he confirmed that only restore relevant arguments will be used to decide on noop, also, there may be properties that would change the restore (like a conditional packagereference) and in this case, we do want to pass that and force a new restore. So, I believe this is the right decision here.
Also, currently, the restore verbosity is the default, even in the implicit case, which means, when doing build, you will see the restore output followed by the build output. I think I may end-up making the implicit restore output "quiet", but I am waiting to see what the output for noop restore looks like to decide.
@dotnet/dotnet-cli