Skip to content

dotnet package list command should restore first #47400

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

dotnet package list command should restore first #47400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
95722a9
Implicit restore dotnet list package
Nigusu-Allehu Mar 9, 2025
62c14c7
undo
Nigusu-Allehu Mar 9, 2025
bf0216a
test
Nigusu-Allehu Mar 9, 2025
ddd951c
completion tests
Nigusu-Allehu Mar 10, 2025
7601bdf
seperate tests for dotnet package list
Nigusu-Allehu Mar 10, 2025
bb5c801
test restore
Nigusu-Allehu Mar 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...Cli/dotnet/commands/dotnet-list/dotnet-list-package/ListPackageReferencesCommandParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ private static CliCommand ConstructCommand()
command.Options.Add(PackageListCommandParser.InteractiveOption);
command.Options.Add(PackageListCommandParser.FormatOption);
command.Options.Add(PackageListCommandParser.OutputVersionOption);
command.Options.Add(PackageListCommandParser.NoRestore);

command.SetAction((parseResult) => new ListPackageReferencesCommand(parseResult).Execute());

Expand Down
44 changes: 40 additions & 4 deletions src/Cli/dotnet/commands/dotnet-package/list/ListPackageReferencesCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.CommandLine;
using System.Globalization;
using Microsoft.DotNet.Cli;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Tools.Common;
using Microsoft.DotNet.Tools.MSBuild;
using Microsoft.DotNet.Tools.NuGet;

namespace Microsoft.DotNet.Tools.Package.List
Expand All @@ -20,7 +22,7 @@ public ListPackageReferencesCommand(
_fileOrDirectory = GetAbsolutePath(Directory.GetCurrentDirectory(),
parseResult.HasOption(PackageCommandParser.ProjectOption) ?
parseResult.GetValue(PackageCommandParser.ProjectOption) :
parseResult.GetValue(ListCommandParser.SlnOrProjectArgument));
parseResult.GetValue(ListCommandParser.SlnOrProjectArgument) ?? "");
}

private static string GetAbsolutePath(string currentDirectory, string relativePath)
Expand All @@ -30,7 +32,41 @@ private static string GetAbsolutePath(string currentDirectory, string relativePa

public override int Execute()
{
return NuGetCommand.Run(TransformArgs());
string projectFile = GetProjectOrSolution();
bool noRestore = _parseResult.HasOption(PackageListCommandParser.NoRestore);
int restoreExitCode = 0;

if (!noRestore )
{
restoreExitCode = RunRestore(projectFile);
}

return restoreExitCode == 0
? NuGetCommand.Run(TransformArgs(projectFile))
: restoreExitCode;
}

private int RunRestore(string projectOrSolution)
{
MSBuildForwardingApp restoringCommand = new MSBuildForwardingApp(argsToForward: ["-target:restore", projectOrSolution, "-noConsoleLogger"]);

int exitCode = 0;

try
{
exitCode = restoringCommand.Execute();
}
catch (Exception)
{
exitCode = 1;
}

if (exitCode != 0)
{
Console.WriteLine(String.Format(CultureInfo.CurrentCulture, LocalizableStrings.Error_restore));
}

return exitCode;
}

internal static void EnforceOptionRules(ParseResult parseResult)
Expand All @@ -45,15 +81,15 @@ internal static void EnforceOptionRules(ParseResult parseResult)
}
}

private string[] TransformArgs()
private string[] TransformArgs(string projectOrSolution)
{
var args = new List<string>
{
"package",
"list",
};

args.Add(GetProjectOrSolution());
args.Add(projectOrSolution);

args.AddRange(_parseResult.OptionValuesToBeForwarded(PackageListCommandParser.GetCommand()));

Expand Down
7 changes: 7 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/LocalizableStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,11 @@
<data name="CmdOutputVersionDescription" xml:space="preserve">
<value>Specifies the version of machine-readable output. Requires the '--format json' option.</value>
</data>
<data name="CmdNoRestoreDescription" xml:space="preserve">
<value>Do not restore before running the command.</value>
</data>
<data name="Error_restore" xml:space="preserve">
<value>Restore failed. Please try running `dotnet restore`.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if the Please try running dotnet restore`` is the right action to suggest to the user.
I'd expect that restore fails as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I see how the message could be misleading, as it might imply that running dotnet restore would necessarily fix the issue. A more accurate message could be:
"Restore failed. Run dotnet restore for more details on the issue."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change, how does the restore error appear?

Basically, can you show us a full output of a dotnet list package call that has a failed restore (say becausie a package does not exist).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would look as follows

Restore failed. Please try running dotnet restore.

Restore is executed with the "-noConsoleLogger" option to suppress all logging from the restore process. This ensures that restore messages do not interfere with the package listing logs. If restore fails, we detect it via the exit code and log an error message.

The primary reason for suppressing restore logs is to prevent them from disrupting structured outputs, such as when the user specifies --format json, where additional messages could result in invalid JSON. Given this, the approach is to avoid logging any restore output and, in case of failure, simply inform the user and recommend running dotnet restore to investigate the issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user specifies --format json and we put this custom error, doesn't that still break the structured log?
It's going to be pretty challenging to chain the commands and get the output.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that complicates the integration effectively, cause we'd need to write that from this code right, not the NuGet side code.

Is there a way we can "capture" the logs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have it on the nuget side by forwarding through an option like
--restore-failed and do the logging from xplat

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dotnet/nuget-team thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My opinion is that dotnet add package already knows how to restore, dotnet package update is going to need to as well, and dotnet nuget why also needs it, in addition to dotnet list package. I think we should make the effort to do everything in the NuGet.Client repo, which also makes it easier for us to test. Splitting our business logic across multiple repos increases tech debt.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all these commands, we could move their invocation methods into the NuGet.Client repo, allowing the SDK to call these methods directly instead of forwarding command arguments to xplat.

By doing so, we can introduce a wrapper on the SDK side—let’s call it RestoringCommandsWrapper. This class would ensure that restore operations execute before any dependent command runs.

Example:
Instead of directly executing commands like dotnet nuget why, we could wrap them in a RestoringCommandsWrapper:

public class RestoringCommandsWrapper
{
    public static void Execute()
    {
        // Execute restore first
        RestoreCommand.Execute();

        // Execute the actual command
        command(argss);
    }
}

Then, for commands like dotnet nuget why, we would update the invocation to:

RestoringCommandsWrapper.Execute(() => 
    NuGet.CommandLine.XPlat.Commands.Why.WhyCommand.RunWhyCommand(whyArgs), whyArgs);

Currently, for dotnet nuget why, we invoke a method on the client side that adds nuget why to the root command:

NuGet.CommandLine.XPlat.Commands.Why.WhyCommand.GetWhyCommand(command);

Instead of this approach, we could perform all parsing on the SDK side and use:

NuGet.CommandLine.XPlat.Commands.Why.WhyCommand.RunWhyCommand(whyArgs);

If why is a restoring command, we could use the RestoringCommandsWrapper to ensure restore runs before executing the command.

<comment>do not translate `dotnet restore`</comment>
</data>
</root>
7 changes: 7 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/PackageListCommandParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ internal static class PackageListCommandParser
Arity = ArgumentArity.Zero
}.ForwardAs("--interactive");

public static readonly CliOption NoRestore = new CliOption<bool>("--no-restore")
{
Description = LocalizableStrings.CmdNoRestoreDescription,
Arity = ArgumentArity.Zero
};

public static readonly CliOption VerbosityOption = new ForwardedOption<VerbosityOptions>("--verbosity", "-v")
{
Description = CommonLocalizableStrings.VerbosityOptionDescription,
Expand Down Expand Up @@ -119,6 +125,7 @@ private static CliCommand ConstructCommand()
command.Options.Add(InteractiveOption);
command.Options.Add(FormatOption);
command.Options.Add(OutputVersionOption);
command.Options.Add(NoRestore);
command.Options.Add(PackageCommandParser.ProjectOption);

command.SetAction((parseResult) => new ListPackageReferencesCommand(parseResult).Execute());
Expand Down
10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.de.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.es.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.fr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.it.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.ja.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.ko.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.pl.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Cli/dotnet/commands/dotnet-package/list/xlf/LocalizableStrings.pt-BR.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading