Require implicit restore for commands that require restore #14119
Conversation
| 3. **Handle scenarios where restore is unsuccessful** | ||
| - If implicit restore fails due to network issues, package source unavailability, or conflicting dependencies, the command should fail with a clear error message. | ||
| - If `--no-restore` is specified and the assets file is missing or outdated, the command should either: | ||
| - Proceed with a warning that the results may be inaccurate. |
There was a problem hiding this comment.
Looks like this is missing other options. I think we should detect if restore is out-of-date and log a warning if the user specified --no-restore.
There was a problem hiding this comment.
tbh, I wouldn't mess with the outdated check.
The rest of the implicit commands also don't mess with it.
If you skip the implicit command, we should just assume it's correct until we have a good reason to make it more costly.
In the worst case scenario, this works as well as today's list package does.
|
|
||
| When a user executes a NuGet command that requires restore, the system should: | ||
|
|
||
| 1. **Check if restore has been performed and is up to date** |
There was a problem hiding this comment.
Just call restore, it'll perform it's own up to date check.
That's how build is implemented. It just calls restore, restore determines if anything needs to happen.
| 3. **Handle scenarios where restore is unsuccessful** | ||
| - If implicit restore fails due to network issues, package source unavailability, or conflicting dependencies, the command should fail with a clear error message. | ||
| - If `--no-restore` is specified and the assets file is missing or outdated, the command should either: | ||
| - Proceed with a warning that the results may be inaccurate. |
There was a problem hiding this comment.
tbh, I wouldn't mess with the outdated check.
The rest of the implicit commands also don't mess with it.
If you skip the implicit command, we should just assume it's correct until we have a good reason to make it more costly.
In the worst case scenario, this works as well as today's list package does.
|
|
||
| <!-- Explain the proposal in sufficient detail with implementation details, interaction models, and clarification of corner cases. --> | ||
|
|
||
| The command should invoke restore **only when necessary**, checking the assets file has against the project dgspec has. |
There was a problem hiding this comment.
Determining whether restore is necessary is basically no-op. The first step of the restore command is no-op, so while there's some extra overhead of calling the msbuild process, I think in terms of correctness we're better off not duplicating the restore no-op logic.
Fixes: #13406
rendered https://github.com/NuGet/Home/blob/dev-nyenwork-implicit-restore/accepted/2025/when-to-require-implicit-restore.md