Skip to content

Add ability to link multiple referenced packages at once #27

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add ability to link multiple referenced packages at once #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2704dff
Initial version of (un)linking by specifying repo root dir and packag…
danieljbfaustino Nov 11, 2019
ac7ba42
Refactoring.
danieljbfaustino Nov 11, 2019
1a46605
Fixing white spaces and unnecessary using statement.
danieljbfaustino Nov 11, 2019
e4b92c2
Refactored usage of the 3 modes for linking.
danieljbfaustino Nov 11, 2019
eaf2fef
Removed the need for a rootDir option for Unlinking. If the user does…
danieljbfaustino Nov 11, 2019
f4828a8
Fixed issue where projects starting with the same name of the package…
Dec 31, 2019
1eae899
Refactored usage of verbosity level for both link and unlink commands.
danieljbfaustino Dec 31, 2019
c099b02
Error won't be reported if performing unlink of all packages and if o…
Feb 11, 2020
385cdc9
Reverting unnecessary changes to solution file.
danieljbfaustino Apr 22, 2020
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
2 changes: 1 addition & 1 deletion source/NuLink.Cli/INuLinkCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ namespace NuLink.Cli
{
public interface INuLinkCommand
{
int Execute(NuLinkCommandOptions options);
void Execute(NuLinkCommandOptions options);
Copy link
Member

Choose a reason for hiding this comment

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

The idea behind the int return value is to return an exit code, which can be useful for scripting. I guess int becomes tricky because of a multi-package operation, which can result in a mix of successes and failures. In such a case, I'd prefer to define the overall success and failure.

This is what I suggest:

  • For link command, the "package already linked" situation, when the link target is the desired location, should be a success rather than an error (print a success message, do nothing).
  • For unlink command, if the package is not linked, it's a success and not an error (print a success message, do nothing).
  • Continue to handle all other errors
  • Let the user choose one of two modes:
    • Fail fast (default): stop on the first error and exit with an error code
    • Ignore errors: if an --ignore-errors flag is specified, we run all packages, report all errors as warnings, and always exit with code 0.

(as for implementation, see comment below about exceptions)

}
}
106 changes: 84 additions & 22 deletions source/NuLink.Cli/LinkCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection.Metadata.Ecma335;
Expand All @@ -15,47 +16,86 @@ public LinkCommand(IUserInterface ui)
_ui = ui;
}

public int Execute(NuLinkCommandOptions options)
public void Execute(NuLinkCommandOptions options)
{
_ui.ReportMedium(() =>
$"Checking package references in {(options.ProjectIsSolution ? "solution" : "project")}: {options.ConsumerProjectPath}");

var requestedPackage = GetPackageInfo();
var allPackages = GetAllPackages(options);
var localProjectPath = options.LocalProjectPath;

if (options.Mode != NuLinkCommandOptions.LinkMode.AllToAll)
Copy link
Member

Choose a reason for hiding this comment

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

A styling hint: in order to keep functions short, I'd extract "then" and "else" into two nested functions.

{
var requestedPackage = GetPackage(allPackages, options.PackageId);

if (options.Mode == NuLinkCommandOptions.LinkMode.SingleToAll)
{
localProjectPath = GetAllProjects(options.RootDirectory).
Copy link
Member

Choose a reason for hiding this comment

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

I'd avoid reassigning a local variable because it reduces readability.

Instead, you can introduce a new variable named effectiveLocalProjectPath and use the ? : operator to assign it either localProjectPath or the result of lookup with GetAllProjects().

FirstOrDefault(proj => proj.Contains(requestedPackage.PackageId + ".csproj"));
}

LinkPackage(requestedPackage, localProjectPath, options.Mode == NuLinkCommandOptions.LinkMode.SingleToSingle);
}
else
{
var allProjectsInRoot = GetAllProjects(options.RootDirectory).ToList();

foreach (var package in allPackages)
{
localProjectPath = allProjectsInRoot.FirstOrDefault(proj => proj.Contains(package.PackageId + ".csproj"));
LinkPackage(package, localProjectPath, false);
}
}
}

private void LinkPackage(PackageReferenceInfo requestedPackage, string localProjectPath, bool singleMode)
{
if (localProjectPath == null)
{
_ui.Report(singleMode ? VerbosityLevel.Error : VerbosityLevel.Low, () =>
$"{(singleMode ? string.Concat(VerbosityLevel.Error.ToString(), ": ") : string.Empty)}" +
$"Cannot find corresponding project to package {requestedPackage.PackageId}");

return;
}

var linkTargetPath = Path.Combine(Path.GetDirectoryName(localProjectPath), "bin", "Debug");
var status = requestedPackage.CheckStatus();
var linkTargetPath = Path.Combine(Path.GetDirectoryName(options.LocalProjectPath), "bin", "Debug");

ValidateOperation();
if (!ValidateOperation())
{
return;
}

PerformOperation();

_ui.ReportSuccess(() => $"Linked {requestedPackage.LibFolderPath}");
_ui.ReportSuccess(() => $" -> {linkTargetPath}", ConsoleColor.Magenta);
return 0;

PackageReferenceInfo GetPackageInfo()
{
var allProjects = new WorkspaceLoader().LoadProjects(options.ConsumerProjectPath, options.ProjectIsSolution);
var referenceLoader = new PackageReferenceLoader(_ui);
var allPackages = referenceLoader.LoadPackageReferences(allProjects);
var package = allPackages.FirstOrDefault(p => p.PackageId == options.PackageId);
return package ?? throw new Exception($"Error: Package not referenced: {options.PackageId}");
}

void ValidateOperation()
bool ValidateOperation()
Copy link
Member

Choose a reason for hiding this comment

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

TL;DR: let's keep throwing exceptions rather than returning a bool.

I was thinking quite a bit about this one. If we want to support both "ignore errors" and "fail fast" modes, the code can become complicated. I mean that using exception results in simpler code. On the other hand, we have "do not use exceptions to steer logic".

In the bottom line, I think that throwing exceptions from LinkPackage() is OK, because LinkPackage should either perform its responsibility or throw. We can implement both "fail fast" and "ignore" by catching at the caller (inside the foreach loop). When in "ignore" mode, report a warning. When in "fail fast" mode, re-throw.

{
if (!status.LibFolderExists)
{
throw new Exception($"Error: Cannot link package {options.PackageId}: 'lib' folder not found, expected {requestedPackage.LibFolderPath}");
_ui.ReportError(() => $"Error: Cannot link package {requestedPackage.PackageId}: 'lib' folder not found, expected {requestedPackage.LibFolderPath}");
return false;
}

if (status.IsLibFolderLinked)
{
throw new Exception($"Error: Package {requestedPackage.PackageId} is already linked to {status.LibFolderLinkTargetPath}");
_ui.Report(singleMode ? VerbosityLevel.Error : VerbosityLevel.Low, () =>
Copy link
Member

Choose a reason for hiding this comment

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

As I wrote above, this should be an error only if the link target path differs from the requested one. Maybe this check should be performed earlier.

$"{(singleMode ? string.Concat(VerbosityLevel.Error.ToString(), ": ") : string.Empty)}" +
$"Package {requestedPackage.PackageId} is already linked to {status.LibFolderLinkTargetPath}");

return false;
}

if (!Directory.Exists(linkTargetPath))
{
throw new Exception($"Error: Target link directory doesn't exist: {linkTargetPath}");
_ui.ReportError(() => $"Error: Target link directory doesn't exist: {linkTargetPath}");
return false;
}

return true;
}

void PerformOperation()
Expand All @@ -66,13 +106,14 @@ void PerformOperation()
}
else
{
_ui.ReportWarning(() => $"Warning: backup folder was not expected to exist: {requestedPackage.LibBackupFolderPath}");
_ui.ReportWarning(() =>
$"Warning: backup folder was not expected to exist: {requestedPackage.LibBackupFolderPath}");
}

try
{
SymbolicLinkWithDiagnostics.Create(
fromPath: requestedPackage.LibFolderPath,
fromPath: requestedPackage.LibFolderPath,
toPath: linkTargetPath);
}
catch
Expand All @@ -96,10 +137,31 @@ void RevertOperation()
_ui.ReportError(() => "--- MANUAL RECOVERY INSTRUCTIONS ---");
_ui.ReportError(() => $"1. Go to {Path.GetDirectoryName(requestedPackage.LibFolderPath)}");
_ui.ReportError(() => $"2. Rename '{Path.GetFileName(requestedPackage.LibBackupFolderPath)}'" +
$" to '{Path.GetFileName(requestedPackage.LibFolderPath)}'");
$" to '{Path.GetFileName(requestedPackage.LibFolderPath)}'");
_ui.ReportError(() => "--- END OF RECOVERY INSTRUCTIONS ---");
}
}
}

private PackageReferenceInfo GetPackage(HashSet<PackageReferenceInfo> allPackages, string packageId)
{
var package = allPackages.FirstOrDefault(p => p.PackageId == packageId);
return package ?? throw new Exception($"Error: Package not referenced: {packageId}");
}

private HashSet<PackageReferenceInfo> GetAllPackages(NuLinkCommandOptions options)
{
var allProjects = new WorkspaceLoader().LoadProjects(options.ConsumerProjectPath, options.ProjectIsSolution);
var referenceLoader = new PackageReferenceLoader(_ui);
var allPackages = referenceLoader.LoadPackageReferences(allProjects);
return allPackages;
}

private IEnumerable<string> GetAllProjects(string rootDir)
{
var slnPaths = Directory.GetFiles(rootDir, "*.sln", SearchOption.AllDirectories);
var allProjects = new WorkspaceLoader().LoadProjects(slnPaths).Select(proj => proj.ProjectFile.Path);
return allProjects;
}
}
}
18 changes: 15 additions & 3 deletions source/NuLink.Cli/NuLinkCommandOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,38 @@ namespace NuLink.Cli
{
public class NuLinkCommandOptions
{
public enum LinkMode
{
SingleToSingle,
SingleToAll,
AllToAll
}

public NuLinkCommandOptions(
string consumerProjectPath,
string packageId = null,
bool dryRun = false,
string consumerProjectPath,
string rootDirectory = null,
string packageId = null,
bool dryRun = false,
bool bareUI = false,
string localProjectPath = null)
{
ConsumerProjectPath = consumerProjectPath;
RootDirectory = rootDirectory;
PackageId = packageId;
DryRun = dryRun;
BareUI = bareUI;
LocalProjectPath = localProjectPath;
ProjectIsSolution = ConsumerProjectPath.EndsWith(".sln", StringComparison.OrdinalIgnoreCase);
Mode = rootDirectory != null ? packageId == null ? LinkMode.AllToAll : LinkMode.SingleToAll : LinkMode.SingleToSingle;
}

public string ConsumerProjectPath { get; }
public bool ProjectIsSolution { get; }
public string RootDirectory { get; }
public string PackageId { get; }
public string LocalProjectPath { get; }
public bool DryRun { get; }
public bool BareUI { get; }
public LinkMode Mode { get; }
}
}
39 changes: 30 additions & 9 deletions source/NuLink.Cli/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ private static RootCommand BuildCommandLine()
Name = "on/off",
Arity = ArgumentArity.ZeroOrOne
});
var rootDirOption = new Option(new[] { "--root-dir", "-r" }, HelpText.RootDirOption, new Argument<string>() {
Name = "root-directory",
Arity = ArgumentArity.ExactlyOne
});

return new RootCommand() {
new Command("status", HelpText.StatusCommand, handler: HandleStatus()) {
Expand All @@ -45,6 +49,7 @@ private static RootCommand BuildCommandLine()
},
new Command("link", HelpText.LinkCommand, handler: HandleLink()) {
consumerOption,
rootDirOption,
packageOption,
localProjectOption,
dryRunOption
Expand All @@ -70,28 +75,31 @@ private static ICommandHandler HandleStatus() =>
});

private static ICommandHandler HandleLink() =>
CommandHandler.Create<string, string, string, bool>((consumer, package, local, dryRun) => {
CommandHandler.Create<string, string, string, string, bool>((consumer, rootDir, package, local, dryRun) => {
var options = new NuLinkCommandOptions(
ValidateConsumerProject(consumer),
rootDir,
package,
localProjectPath: ValidateTargetProject(local),
localProjectPath: local,
dryRun: dryRun);
return ExecuteCommand(
"link",
options,
options.ConsumerProjectPath != null && options.LocalProjectPath != null && options.PackageId != null);
options.ConsumerProjectPath != null && !string.IsNullOrEmpty(rootDir)
? ValidateRootDirectory(rootDir)
: ValidateTargetProject(options.LocalProjectPath) && options.PackageId != null);
});

private static ICommandHandler HandleUnlink() =>
CommandHandler.Create<string, string, bool>((consumer, package, dryRun) => {
var options = new NuLinkCommandOptions(
ValidateConsumerProject(consumer),
package,
packageId: package,
dryRun: dryRun);
return ExecuteCommand(
"unlink",
options,
options.ConsumerProjectPath != null && options.PackageId != null);
options.ConsumerProjectPath != null);
});

private static int ExecuteCommand(
Expand Down Expand Up @@ -145,17 +153,28 @@ private static string ValidateConsumerProject(string filePath)
return filePath;
}

private static string ValidateTargetProject(string filePath)
private static bool ValidateTargetProject(string filePath)
{
if (File.Exists(filePath))
{
return filePath;
return true;
}

FullUI.FatalError(() => $"Error: File does not exist: {filePath}");
return null;
return false;
}


private static bool ValidateRootDirectory(string rootDir)
{
if (Directory.Exists(rootDir))
{
return true;
}

FullUI.FatalError(() => $"Error: Directory does not exist: {rootDir}");
return false;
}

private static INuLinkCommand CreateCommand(string name, NuLinkCommandOptions options)
{
var ui = (options.BareUI ? new BareUI() : new FullUI() as IUserInterface);
Expand Down Expand Up @@ -191,6 +210,8 @@ private static class HelpText
"If specified, list intended actions without executing them";
public const string QuietOption =
"If specified, suppresses all output except data (useful for scripting)";
public const string RootDirOption =
"If specified, executes command on all packages in project/solution";
}
}
}
4 changes: 1 addition & 3 deletions source/NuLink.Cli/StatusCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public StatusCommand(IUserInterface ui)
_ui = ui;
}

public int Execute(NuLinkCommandOptions options)
public void Execute(NuLinkCommandOptions options)
{
_ui.ReportMedium(() =>
$"Checking status of packages in {(options.ProjectIsSolution ? "solution" : "project")}: {options.ConsumerProjectPath}");
Expand All @@ -34,8 +34,6 @@ public int Execute(NuLinkCommandOptions options)
}
}

return 0;

void PrintPackage(PackageReferenceInfo reference, PackageStatusInfo status)
{
var statusColor = (status.IsCorrupt ? ConsoleColor.Red : ConsoleColor.Green);
Expand Down
Loading