--no-restore false isn't honored in the dotnet build and other commands

[baronfel]

Describe the bug

The --no-restore flag technically accepts a boolean argument, --no-restore true/--no-restore false. The dotnet build command (and likely several others), only check for the existence of the option, not what the actual value is, when passing noRestore to other parts of the code. An example of this can be found here. We should change this pattern to use GetValueForOption<T> instead.

To Reproduce

In any project, run dotnet build --no-restore false. Note that a restore does not occur.

Exceptions (if any)

None

Further technical details

• Present in 6.0.100, unknown in prior versions

[KalleOlaviNiemitalo]

Note that a restore does occur.

Does not occur.

[KalleOlaviNiemitalo]

Can the option instead be changed to not accept any argument? That would match the --help output.

[baronfel]

Does not occur.

Can you clarify? On my testing the restore did occur, which is the whole problem:

[KalleOlaviNiemitalo]

PowerShell 7.2.1
Copyright (c) Microsoft Corporation.

https://aka.ms/powershell
Type 'help' to get help.

PS C:\Projects> mkdir restore

    Directory: C:\Projects

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d----            6.1.2022    20.42                restore

PS C:\Projects> cd restore
PS C:\Projects\restore> dotnet new classlib
The template "Class Library" was created successfully.

Processing post-creation actions...
Running 'dotnet restore' on C:\Projects\restore\restore.csproj...
  Determining projects to restore...
  Restored C:\Projects\restore\restore.csproj (in 88 ms).
Restore succeeded.

PS C:\Projects\restore> remove-item -recurse obj
PS C:\Projects\restore> dotnet build --no-restore false
Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

C:\Program Files\dotnet\sdk\6.0.101\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(267,5): error NETSDK1004: Assets file 'C:\Projects\restore\obj\project.assets.json' not found. Run a NuGet package restore to generate this file. [C:\Projects\restore\restore.csproj]

Build FAILED.

C:\Program Files\dotnet\sdk\6.0.101\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(267,5): error NETSDK1004: Assets file 'C:\Projects\restore\obj\project.assets.json' not found. Run a NuGet package restore to generate this file. [C:\Projects\restore\restore.csproj]
    0 Warning(s)
    1 Error(s)

Time Elapsed 00:00:00.44
PS C:\Projects\restore> dotnet --version
6.0.101
PS C:\Projects\restore>

This did not restore NuGet packages because --no-restore was given. Even though, because the argument of --no-restore was false, it should have restored the packages. Which matches most of what you wrote in the description, except "Note that a restore does occur".

[baronfel]

ah, you're exactly right. I blame double-negatives and allergies. I'll get the description updated with your change.

[KalleOlaviNiemitalo]

But then I don't understand how your screen shot in #23297 (comment) shows package restore taking place anyway. Is that because of a difference between .NET SDK versions?

[baronfel]

I think that's due to stuff happening in the build of the SDK project I chose to build itself; I chose a bad test sample. Your example with a newly-scaffolded project is a better example.

[BartoszKlonowski]

@baronfel I would like to work on this. Please assign me to this item.
Just to make sure: is the description fully updated according to all the findings in the comments?

[KalleOlaviNiemitalo]

#23297 (comment) has not been answered.

[KalleOlaviNiemitalo]

I don't see a need to let --no-restore take an optional Boolean argument.

The only reason for such a feature would be if a --no-restore setting can be inherited from somewhere, so that a user who wants to restore packages during a build needs to explicitly override that with --no-restore false. However, double negatives are confusing as was already seen in this issue. If this kind of override has to be supported, then it would be better to use just --restore for that. I don't know how to implement mutually overriding options with the command line API, though.

So my preference would be:

• Make --no-restore not accept any argument.
• If a feature like --no-restore false is needed, then implement --restore with no argument. Optionally, make --restore also accept an optional Boolean argument, so that --no-restore is an alias for --restore false.