Skip to content
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

Add dotnet nuget why command #5761

Merged
merged 53 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
708f4e2
applied changes from pragnya17-dotnet-nuget-why
advay26 Mar 28, 2024
10245f8
cleaning up some old code
advay26 Mar 28, 2024
2098b79
wipping
advay26 Apr 3, 2024
3d428bc
fixed duplicate paths bug
advay26 Apr 3, 2024
db9a7ce
trees are beautiful
advay26 Apr 5, 2024
ef25ca2
rip my sleep schedule
advay26 Apr 5, 2024
9170c26
everything works now
advay26 Apr 5, 2024
b1b74f5
refactoring
advay26 Apr 6, 2024
067430a
cleaning up strings
advay26 Apr 6, 2024
08e95fc
fixed redundant traversal bug
advay26 Apr 7, 2024
ae50998
minor string stuff
advay26 Apr 8, 2024
3f153f9
remove (*) deduplication, fix project reference bug
advay26 Apr 10, 2024
8a750f1
wip: recursion -> stack
advay26 Apr 16, 2024
25804a8
recursion -> stack is working now
advay26 Apr 17, 2024
a44299b
wipping
advay26 Apr 18, 2024
073e9b5
removed TODOs to clean up
advay26 Apr 18, 2024
7ed373a
added basic test; failed to run basic test;
advay26 Apr 21, 2024
aaee328
test stubs - commented out
advay26 Apr 22, 2024
ebc4732
cleanup + removed debugging code
advay26 Apr 23, 2024
4a42f3e
Moved files to diff folder
advay26 Apr 23, 2024
d9a6482
experimenting with tests
advay26 Apr 24, 2024
904740e
started addressing feedback, but wow it's a lot of work
advay26 May 3, 2024
69bfb3b
adjusted tests to static class
advay26 May 3, 2024
cb0c043
wip
advay26 May 7, 2024
e55583a
nit
advay26 May 8, 2024
7891af4
temp: working on project refs
advay26 May 9, 2024
fa67ec9
still wipping....
advay26 May 9, 2024
961b7df
added tests, TODOs for targetAlias matching in GetResolvedVersions; r…
advay26 May 10, 2024
cf5771f
added new test files, will fill them out later
advay26 May 10, 2024
efaee79
fixed failing test
advay26 May 10, 2024
0bba3a1
addressin smore feedback
advay26 May 10, 2024
8ee8eba
added func tests + dotnet integration tests
advay26 May 13, 2024
31a6bf3
cleaning up for PR
advay26 May 13, 2024
5c3ad52
cleaning up
advay26 May 13, 2024
852edf0
register dotnet nuget/package * commands separately
advay26 May 15, 2024
923a1df
moved print methods out to new class
advay26 May 15, 2024
527d448
wip: addressing feedback, trialing new method for top level packages/…
advay26 May 15, 2024
506ee92
refactoring
advay26 May 16, 2024
1297105
reverted MSBuildAPIUtility changes
advay26 May 16, 2024
fec9de8
nit
advay26 May 16, 2024
b164b31
fixed tests (at least locally)
advay26 May 16, 2024
2a1c576
whitespace changes?
advay26 May 16, 2024
dccf34c
added unit tests, fixed func tests, + some small nits
advay26 May 16, 2024
50394da
removed unnecessary param
advay26 May 16, 2024
8f3db89
trying to fix 'fully qualified path' error on linux and mac tests
advay26 May 16, 2024
d9c8890
address feedback, try to fix unit tests path issue
advay26 May 29, 2024
6074b25
fixed whitespace issue
advay26 May 29, 2024
7aceac8
fixed unit test path issue for remaining file paths
advay26 May 29, 2024
8aff030
trying to fix integration tests
advay26 May 29, 2024
ee68859
removed WhyCommandUtility sub directory
advay26 May 29, 2024
118735d
oops
advay26 May 29, 2024
7b9a2e9
please work
advay26 May 30, 2024
4a60b13
skip non-sdk stype projects + some more null dereference handling
advay26 May 31, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable enable

using System.Collections.Generic;
using NuGet.Shared;

namespace NuGet.CommandLine.XPlat
{
/// <summary>
/// Represents a node in the package dependency graph.
/// </summary>
internal class DependencyNode
{
public string Id { get; set; }
public string Version { get; set; }
public HashSet<DependencyNode> Children { get; set; }

public DependencyNode(string id, string version)
{
Id = id;
Version = version;
advay26 marked this conversation as resolved.
Show resolved Hide resolved
Children = new HashSet<DependencyNode>();
}

public override int GetHashCode()
{
var hashCodeCombiner = new HashCodeCombiner();
hashCodeCombiner.AddObject(Id);
hashCodeCombiner.AddObject(Version);
hashCodeCombiner.AddUnorderedSequence(Children);
return hashCodeCombiner.CombinedHash;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.CommandLine;
using System.CommandLine.Help;

namespace NuGet.CommandLine.XPlat
{
internal static class WhyCommand
{
private static CliArgument<string> Path = new CliArgument<string>("PROJECT|SOLUTION")
{
Description = Strings.WhyCommand_PathArgument_Description,
Arity = ArgumentArity.ExactlyOne
};

private static CliArgument<string> Package = new CliArgument<string>("PACKAGE")
{
Description = Strings.WhyCommand_PackageArgument_Description,
Arity = ArgumentArity.ExactlyOne
};

private static CliOption<List<string>> Frameworks = new CliOption<List<string>>("--framework", "-f")
{
Description = Strings.WhyCommand_FrameworksOption_Description,
Arity = ArgumentArity.OneOrMore
};

private static HelpOption Help = new HelpOption()
{
Arity = ArgumentArity.Zero
};

internal static void Register(CliCommand rootCommand, Func<ILoggerWithColor> getLogger)
{
var whyCommand = new CliCommand("why", Strings.WhyCommand_Description);

whyCommand.Arguments.Add(Path);
whyCommand.Arguments.Add(Package);
whyCommand.Options.Add(Frameworks);
whyCommand.Options.Add(Help);

whyCommand.SetAction((parseResult) =>
{
ILoggerWithColor logger = getLogger();

try
{
var whyCommandArgs = new WhyCommandArgs(
parseResult.GetValue(Path),
parseResult.GetValue(Package),
parseResult.GetValue(Frameworks),
logger);

int exitCode = WhyCommandRunner.ExecuteCommand(whyCommandArgs);
return exitCode;
}
catch (ArgumentException ex)
{
logger.LogError(ex.Message);
return ExitCodes.InvalidArguments;
}
});

rootCommand.Subcommands.Add(whyCommand);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable enable

using System;
using System.Collections.Generic;

namespace NuGet.CommandLine.XPlat
{
internal class WhyCommandArgs
{
public string Path { get; }
public string Package { get; }
public List<string> Frameworks { get; }
public ILoggerWithColor Logger { get; }

/// <summary>
/// A constructor for the arguments of the 'why' command.
/// </summary>
/// <param name="path">The path to the solution or project file.</param>
/// <param name="package">The package for which we show the dependency graphs.</param>
/// <param name="frameworks">The target framework(s) for which we show the dependency graphs.</param>
/// <param name="logger"></param>
public WhyCommandArgs(
string path,
string package,
List<string> frameworks,
ILoggerWithColor logger)
{
Path = path ?? throw new ArgumentNullException(nameof(path));
Package = package ?? throw new ArgumentNullException(nameof(package));
Frameworks = frameworks ?? throw new ArgumentNullException(nameof(frameworks));
Logger = logger ?? throw new ArgumentNullException(nameof(logger));
}
}
}
advay26 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable enable

using System;
zivkan marked this conversation as resolved.
Show resolved Hide resolved
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using Microsoft.Build.Evaluation;
using NuGet.CommandLine.XPlat.WhyCommandUtility;
using NuGet.ProjectModel;

namespace NuGet.CommandLine.XPlat
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: Just wondering why the namespace is different to the project's default namespace + directory structure?

Copy link
Member Author

Choose a reason for hiding this comment

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

All commands and files in this project fall in the NuGet.CommandLine.XPlat namespace, so I've removed other namespaces like NuGet.CommandLine.XPlat.WhyCommandUtility and moved everything under NuGet.CommandLine.XPlat to keep things consistent. Let me know if you think this needs changes or can be improved upon

Copy link
Member

Choose a reason for hiding this comment

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

IMO the reasons to organize code on the filesystem is identical to the reasons to organize code in namespaces. Therefore, I consider it bad when they're out of sync. But I recognise that NuGet's existing code does this. I just think it's a bad pattern that we should stop copying. But I'm not going to hold up this PR because of this, and I don't know if anyone else in the team share my opinion. But for what it's worth, since this file's class is internal, there's no public API risk of moving it to a "more appropriate" namespace.

{
internal static class WhyCommandRunner
{
private const string ProjectName = "MSBuildProjectName";
private const string ProjectAssetsFile = "ProjectAssetsFile";

/// <summary>
/// Executes the 'why' command.
/// </summary>
/// <param name="whyCommandArgs">CLI arguments for the 'why' command.</param>
public static int ExecuteCommand(WhyCommandArgs whyCommandArgs)
{
try
{
ValidatePathArgument(whyCommandArgs.Path);
ValidatePackageArgument(whyCommandArgs.Package);
}
catch (ArgumentException ex)
{
whyCommandArgs.Logger.LogError(ex.Message);
return ExitCodes.InvalidArguments;
}
advay26 marked this conversation as resolved.
Show resolved Hide resolved

string targetPackage = whyCommandArgs.Package;

IEnumerable<string> projectPaths = Path.GetExtension(whyCommandArgs.Path).Equals(".sln")
? MSBuildAPIUtility.GetProjectsFromSolution(whyCommandArgs.Path).Where(f => File.Exists(f))
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 guessing that MSBuild only returns project files which exist. @jeffkl would know for sure.

: [whyCommandArgs.Path];

foreach (var projectPath in projectPaths)
{
Project project = MSBuildAPIUtility.GetProject(projectPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can enhance the MSBuildAPIUtility to just return the ProjectAndSolution type instead of selecting only the paths in GetProjectsFromSolution. That way, you wouldn't need convert the paths back to projects, here.

I'm not familiar with these types, so let's see if @jeffkl has opinions on this.

LockFile? assetsFile = GetProjectAssetsFile(project, whyCommandArgs.Logger);

if (assetsFile != null)
{
Dictionary<string, List<DependencyNode>?>? dependencyGraphPerFramework = DependencyGraphFinder.GetAllDependencyGraphs(
assetsFile,
whyCommandArgs.Package,
whyCommandArgs.Frameworks);

if (dependencyGraphPerFramework != null)
{
whyCommandArgs.Logger.LogMinimal(
nkolev92 marked this conversation as resolved.
Show resolved Hide resolved
string.Format(
Strings.WhyCommand_Message_DependencyGraphsFoundInProject,
project.GetPropertyValue(ProjectName),
targetPackage));

DependencyGraphPrinter.PrintAllDependencyGraphs(dependencyGraphPerFramework, targetPackage, whyCommandArgs.Logger);
}
else
{
whyCommandArgs.Logger.LogMinimal(
string.Format(
Strings.WhyCommand_Message_NoDependencyGraphsFoundInProject,
project.GetPropertyValue(ProjectName),
targetPackage));
}
}

ProjectCollection.GlobalProjectCollection.UnloadProject(project);
}

return ExitCodes.Success;
}

private static void ValidatePathArgument(string path)
{
if (string.IsNullOrEmpty(path))
{
throw new ArgumentException(
string.Format(CultureInfo.CurrentCulture,
Strings.WhyCommand_Error_ArgumentCannotBeEmpty,
"PROJECT|SOLUTION"));
}

if (!File.Exists(path)
|| (!path.EndsWith("proj", StringComparison.OrdinalIgnoreCase)
&& !path.EndsWith(".sln", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException(
string.Format(CultureInfo.CurrentCulture,
Strings.WhyCommand_Error_PathIsMissingOrInvalid,
path));
}
}

private static void ValidatePackageArgument(string package)
{
if (string.IsNullOrEmpty(package))
{
throw new ArgumentException(
string.Format(CultureInfo.CurrentCulture,
Strings.WhyCommand_Error_ArgumentCannotBeEmpty,
"PACKAGE"));
}
}

/// <summary>
/// Validates and returns the assets file for the given project.
/// </summary>
/// <param name="project">Evaluated MSBuild project</param>
/// <param name="logger">Logger for the 'why' command</param>
/// <returns>Assets file for the given project. Returns null if there was any issue finding or parsing the assets file.</returns>
private static LockFile? GetProjectAssetsFile(Project project, ILoggerWithColor logger)
{
if (!MSBuildAPIUtility.IsPackageReferenceProject(project))
{
logger.LogError(
string.Format(
CultureInfo.CurrentCulture,
Strings.Error_NotPRProject,
project.FullPath));

return null;
}

string assetsPath = project.GetPropertyValue(ProjectAssetsFile);

if (!File.Exists(assetsPath))
{
logger.LogError(
string.Format(
CultureInfo.CurrentCulture,
Strings.Error_AssetsFileNotFound,
project.FullPath));

return null;
}

var lockFileFormat = new LockFileFormat();
LockFile assetsFile = lockFileFormat.Read(assetsPath);

// assets file validation
if (assetsFile.PackageSpec == null
|| assetsFile.Targets == null
|| assetsFile.Targets.Count == 0)
{
logger.LogError(
string.Format(
CultureInfo.CurrentCulture,
Strings.WhyCommand_Error_InvalidAssetsFile,
assetsFile.Path,
project.FullPath));

return null;
}

return assetsFile;
}
}
}
Loading