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.

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

Jump to bottom

Fixes to add package command #1086

Merged
merged 5 commits into from
Jan 4, 2017
Merged

Fixes to add package command #1086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b7f2ddb
Fixing update on tfm
Dec 23, 2016
e2633c5
Adding comments to msbuildAPIUtility and fixing the way we look up re…
Jan 4, 2017
95efc10
Addressing parts of PR feedback
Jan 4, 2017
1a7649e
Fixing log strings on PR comments
Jan 4, 2017
bca4328
Fixing strings
Jan 4, 2017
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
4 changes: 1 addition & 3 deletions ...e/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ namespace NuGet.CommandLine.XPlat
{
public static class AddPackageReferenceCommand
{
private const string MSBuildExeName = "MSBuild.dll";

public static void Register(CommandLineApplication app, Func<ILogger> getLogger,
Func<IAddPackageReferenceCommandRunner> getCommandRunner)
{
Expand Down Expand Up @@ -91,7 +89,7 @@ public static void Register(CommandLineApplication app, Func<ILogger> getLogger,
NoVersion = noVersion,
DgFilePath = dgFilePath.Value()
};
var msBuild = new MSBuildAPIUtility();
var msBuild = new MSBuildAPIUtility(logger);
var addPackageRefCommandRunner = getCommandRunner();
return addPackageRefCommandRunner.ExecuteCommand(packageRefArgs, msBuild);
});
Expand Down
78 changes: 51 additions & 27 deletions ...t.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,16 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using NuGet.Commands;
using NuGet.Common;
using NuGet.Configuration;
using NuGet.Frameworks;
using NuGet.Packaging.Core;
using NuGet.ProjectModel;
using NuGet.Protocol;
using NuGet.Protocol.Core.Types;
using NuGet.Versioning;

Expand Down Expand Up @@ -77,8 +72,6 @@ public async Task<int> ExecuteCommand(PackageReferenceArgs packageReferenceArgs,
.Where(t => t.Success)
.Select(t => t.Graph.Framework));

var x = restorePreviewResult.Result.GetAllUnresolved();

if (packageReferenceArgs.Frameworks?.Any() == true)
{
// If the user has specified frameworks then we intersect that with the compatible frameworks.
Expand All @@ -98,6 +91,7 @@ public async Task<int> ExecuteCommand(PackageReferenceArgs packageReferenceArgs,
packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture,
Strings.Error_AddPkgIncompatibleWithAllFrameworks,
packageReferenceArgs.PackageDependency.Id,
packageReferenceArgs.Frameworks?.Any() == true ? Strings.AddPkg_UserSpecified : Strings.AddPkg_All,
packageReferenceArgs.ProjectPath));

return 1;
Expand Down Expand Up @@ -141,7 +135,7 @@ public async Task<int> ExecuteCommand(PackageReferenceArgs packageReferenceArgs,
return 0;
}

private async Task<RestoreResultPair> PreviewAddPackageReference(PackageReferenceArgs packageReferenceArgs,
private static async Task<RestoreResultPair> PreviewAddPackageReference(PackageReferenceArgs packageReferenceArgs,
DependencyGraphSpec dgSpec,
PackageSpec originalPackageSpec)
{
Expand Down Expand Up @@ -186,7 +180,7 @@ private async Task<RestoreResultPair> PreviewAddPackageReference(PackageReferenc
}
}

private DependencyGraphSpec ReadProjectDependencyGraph(PackageReferenceArgs packageReferenceArgs)
private static DependencyGraphSpec ReadProjectDependencyGraph(PackageReferenceArgs packageReferenceArgs)
{
DependencyGraphSpec spec = null;

Expand All @@ -198,30 +192,60 @@ private DependencyGraphSpec ReadProjectDependencyGraph(PackageReferenceArgs pack
return spec;
}

private void UpdatePackageVersionIfNeeded(RestoreResultPair restorePreviewResult,
private static void UpdatePackageVersionIfNeeded(RestoreResultPair restorePreviewResult,
PackageReferenceArgs packageReferenceArgs)
{
// If the user did not specify a version then write the exact resolved version
if (packageReferenceArgs.NoVersion)
{
// Get the flattened restore graph
var flattenedRestoreGraph = restorePreviewResult
.Result
.RestoreGraphs
.First()
.Flattened;

// Get the package entry and version from the graph
var packageEntry = flattenedRestoreGraph
.Single(p => p.Key.Name.Equals(packageReferenceArgs.PackageDependency.Id, StringComparison.OrdinalIgnoreCase));
var resolvedVersion = packageEntry
.Key
.Version;

//Update the packagedependency with the new version
packageReferenceArgs.PackageDependency = new PackageDependency(packageReferenceArgs.PackageDependency.Id,
VersionRange.Parse(resolvedVersion.ToString()));
// Get the package version from the graph
var resolvedVersion = GetPackageVersionFromRestoreResult(restorePreviewResult, packageReferenceArgs);

if (resolvedVersion != null)
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where null is being handled. If finding the highest version comes back as null it looks like this just keeps going

{
//Update the packagedependency with the new version
packageReferenceArgs.PackageDependency = new PackageDependency(packageReferenceArgs.PackageDependency.Id,
new VersionRange(resolvedVersion));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

It might make the code more clear to just return the version here, it would be easier to handle errors as needed from a higher level.


private static NuGetVersion GetPackageVersionFromRestoreResult(RestoreResultPair restorePreviewResult,
PackageReferenceArgs packageReferenceArgs)
{
// Get the restore graphs from the restore result
var restoreGraphs = restorePreviewResult
.Result
.RestoreGraphs;

if (packageReferenceArgs.Frameworks?.Any() == true)
{
// If the user specified frameworks then we get the flattened graphs only from the compatible frameworks.
var userSpecifiedFrameworks = new HashSet<NuGetFramework>(
packageReferenceArgs
.Frameworks
.Select(f => NuGetFramework.Parse(f)));

restoreGraphs = restoreGraphs
.Where(r => userSpecifiedFrameworks.Contains(r.Framework));
Copy link
Member

Choose a reason for hiding this comment

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

What if the user accidentally specified a framework that doesn't exist in the project? a error message for that scenario might help, looks like it would just go on and possibly fail with an unrelated error message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here

}

foreach (var restoreGraph in restoreGraphs)
{
var matchingPackageEntries = restoreGraph
.Flattened
.Select(p => p)
.Where(p => p.Key.Name.Equals(packageReferenceArgs.PackageDependency.Id, StringComparison.OrdinalIgnoreCase));

if (matchingPackageEntries.Any())
{
return matchingPackageEntries
.First()
.Key
.Version;
}
}
return null;
}
}
}
62 changes: 58 additions & 4 deletions src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs
View file
Open in desktop

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

42 changes: 37 additions & 5 deletions src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,22 +342,23 @@ For more information, visit http://docs.nuget.org/docs/reference/command-line-re
<comment>{0} - Project/csproj path</comment>
</data>
<data name="Error_AddPkgIncompatibleWithAllFrameworks" xml:space="preserve">
<value>Package : '{0}' is incompatible with all the frameworks in project : '{1}'</value>
<value>Package '{0}' is incompatible with '{1}' frameworks in project '{2}'</value>
<comment>{0} - Package Id
{1} - Project Path</comment>
{1} - all / user specified
{2} - Project Path</comment>
</data>
<data name="Info_AddPkgAddingReference" xml:space="preserve">
<value>Adding PackageReference for package : '{0}', into project : '{1}'</value>
<value>Adding PackageReference for package '{0}', into project '{1}'</value>
<comment>{0} - Package Id
{1} - Project Path</comment>
</data>
<data name="Info_AddPkgCompatibleWithAllFrameworks" xml:space="preserve">
<value>Package : '{0}' is compatible with all the specified frameworks in project : '{1}'</value>
<value>Package '{0}' is compatible with all the specified frameworks in project '{1}'</value>
<comment>{0} - Package Id
{1} - Project Path</comment>
</data>
<data name="Info_AddPkgCompatibleWithSubsetFrameworks" xml:space="preserve">
<value>Package : '{0}' is compatible with a subset of the specified frameworks in project : '{1}'</value>
<value>Package '{0}' is compatible with a subset of the specified frameworks in project '{1}'</value>
<comment>{0} - Package Id
{1} - Project Path</comment>
</data>
Expand Down Expand Up @@ -398,4 +399,35 @@ For more information, visit http://docs.nuget.org/docs/reference/command-line-re
<data name="Error_NoDgSpec" xml:space="preserve">
<value>None or invalid DgSpec was passed to NuGet add package command.</value>
</data>
<data name="Error_AddPkgErrorStringForImportedEdit" xml:space="preserve">
<value>Item '{0}' for '{1}' in Imported file '{2}'</value>
<comment>{0} - ItemType
{1} - Package Id
{2} - Imported File Path</comment>
</data>
<data name="Error_AddPkgFailOnImportEdit" xml:space="preserve">
<value>Error while performing '{0}' for package '{1}'. Cannot edit items in imported files - {2}{3}</value>
<comment>{0} - Operation Name
{1} - packageId
{2} - New line
{3} - Error string</comment>
</data>
<data name="Info_AddPkgAdded" xml:space="preserve">
<value>PackageReference for package '{0}' version '{1}' added to file '{2}'</value>
<comment>{0} - Package Id
{1} - package version
{2} - project file path</comment>
</data>
<data name="Info_AddPkgUpdated" xml:space="preserve">
<value>PackageReference for package '{0}' version '{1}' updated in file '{2}'</value>
<comment>{0} - Package Id
{1} - package version
{2} - project file path</comment>
</data>
<data name="AddPkg_All" xml:space="preserve">
<value>all</value>
</data>
<data name="AddPkg_UserSpecified" xml:space="preserve">
<value>user specified</value>
</data>
</root>
Loading