Skip to content

Commit

Permalink
Addressed feedback and some improvements for better readability
Browse files Browse the repository at this point in the history
  • Loading branch information
jainaashish committed Aug 17, 2018
1 parent a5bc098 commit 22054ce
Show file tree
Hide file tree
Showing 29 changed files with 350 additions and 400 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -315,15 +315,17 @@ private async Task RestoreAsync(bool forceRestore, RestoreOperationSource restor
var providerCache = new RestoreCommandProvidersCache();
Action<SourceCacheContext> cacheModifier = (cache) => { };
var isRestoreOriginalAction = true;
var restoreSummaries = await DependencyGraphRestoreUtility.RestoreAsync(
_solutionManager,
dgSpec,
cacheContext,
providerCache,
cacheModifier,
sources,
_nuGetProjectContext.OperationId,
forceRestore,
dgSpec,
isRestoreOriginalAction,
l,
t);
Expand Down
21 changes: 6 additions & 15 deletions src/NuGet.Core/NuGet.Commands/PackagesLockFileBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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.Linq;
using NuGet.Common;
using NuGet.LibraryModel;
Expand All @@ -12,19 +13,9 @@ namespace NuGet.Commands
{
public class PackagesLockFileBuilder
{
private readonly int _lockFileVersion;

public PackagesLockFileBuilder(int lockFileVersion)
{
_lockFileVersion = lockFileVersion;
}

public PackagesLockFile CreateNuGetLockFile(LockFile assetsFile)
{
var lockFile = new PackagesLockFile()
{
Version = _lockFileVersion
};
var lockFile = new PackagesLockFile();

var libraryLookup = assetsFile.Libraries.Where(e => e.Type == LibraryType.Package)
.ToDictionary(e => new PackageIdentity(e.Name, e.Version));
Expand Down Expand Up @@ -63,16 +54,16 @@ public PackagesLockFile CreateNuGetLockFile(LockFile assetsFile)
};

var framework_dep = framework?.Dependencies.FirstOrDefault(
dep => PathUtility.GetStringComparerBasedOnOS().Equals(dep.Name, library.Name));
dep => StringComparer.OrdinalIgnoreCase.Equals(dep.Name, library.Name));

if (framework_dep != null)
{
dependency.Type = PackageInstallationType.Direct;
dependency.Type = PackageDependencyType.Direct;
dependency.RequestedVersion = framework_dep.LibraryRange.VersionRange;
}
else
{
dependency.Type = PackageInstallationType.Transitive;
dependency.Type = PackageDependencyType.Transitive;
}

nuGettarget.Dependencies.Add(dependency);
Expand All @@ -84,7 +75,7 @@ public PackagesLockFile CreateNuGetLockFile(LockFile assetsFile)
{
Id = projectReference.Name,
Dependencies = projectReference.Dependencies,
Type = PackageInstallationType.Project
Type = PackageDependencyType.Project
};

nuGettarget.Dependencies.Add(dependency);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class RestoreArgs

public Guid ParentId { get; set; }

public bool IsRestore { get; set; } = true;
public bool IsRestoreOriginalAction { get; set; } = true;

// Cache directory -> ISettings
private ConcurrentDictionary<string, ISettings> _settingsCache
Expand Down Expand Up @@ -230,7 +230,7 @@ public void ApplyStandardProperties(RestoreRequest request)
request.AllowNoOp = !request.CacheContext.NoCache && AllowNoOp;
request.HideWarningsAndErrors = HideWarningsAndErrors;
request.ParentId = ParentId;
request.IsRestore = IsRestore;
request.IsRestoreOriginalAction = IsRestoreOriginalAction;
}
}
}
209 changes: 100 additions & 109 deletions src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,6 @@ public class RestoreRequest

public Guid ParentId { get; set;}

public bool IsRestore { get; set; } = true;
public bool IsRestoreOriginalAction { get; set; } = true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public static bool VerifyPackagesOnDisk(RestoreRequest request)
/// This methods handles the deduping of tools
/// Handles the ignoring of RestoreSettings
/// </summary>
public static DependencyGraphSpec GetDGSpec(RestoreRequest request)
public static string GetHash(RestoreRequest request)
{
if (request.Project.RestoreMetadata.ProjectStyle == ProjectStyle.DotnetCliTool || request.Project.RestoreMetadata.ProjectStyle == ProjectStyle.PackageReference)
{
Expand All @@ -222,11 +222,11 @@ public static DependencyGraphSpec GetDGSpec(RestoreRequest request)
}

PersistHashedDGFileIfDebugging(dgSpec, request.Log);
return dgSpec;
return dgSpec.GetHash();
}

PersistHashedDGFileIfDebugging(request.DependencyGraphSpec, request.Log);
return request.DependencyGraphSpec;
return request.DependencyGraphSpec.GetHash();
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public static GraphItem<RemoteResolveResult> CreateUnresolvedMatch(LibraryRange
// This is only applicable when packages has to be resolved from packages.lock.json file
if (lockFileLibraries.TryGetValue(key, out var libraries))
{
var library = libraries.FirstOrDefault(lib => PathUtility.GetStringComparerBasedOnOS().Equals(lib.Name, libraryRange.Name));
var library = libraries.FirstOrDefault(lib => StringComparer.OrdinalIgnoreCase.Equals(lib.Name, libraryRange.Name));

if (library != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ public static class DependencyGraphRestoreUtility
/// </summary>
public static async Task<IReadOnlyList<RestoreSummary>> RestoreAsync(
ISolutionManager solutionManager,
DependencyGraphSpec dgSpec,
DependencyGraphCacheContext context,
RestoreCommandProvidersCache providerCache,
Action<SourceCacheContext> cacheContextModifier,
IEnumerable<SourceRepository> sources,
Guid parentId,
bool forceRestore,
DependencyGraphSpec dgSpec,
bool isRestoreOriginalAction,
ILogger log,
CancellationToken token)
{
Expand All @@ -56,7 +57,7 @@ public static class DependencyGraphRestoreUtility
dgSpec,
parentId,
forceRestore,
isRestore: true);
isRestoreOriginalAction);

var restoreSummaries = await RestoreRunner.RunAsync(restoreContext, token);

Expand All @@ -71,49 +72,6 @@ public static class DependencyGraphRestoreUtility
return new List<RestoreSummary>();
}

/// <summary>
/// Restore a dg spec. This will not update the context cache.
/// </summary>
public static async Task<IReadOnlyList<RestoreSummary>> RestoreAsync(
DependencyGraphSpec dgSpec,
DependencyGraphCacheContext context,
RestoreCommandProvidersCache providerCache,
Action<SourceCacheContext> cacheContextModifier,
IEnumerable<SourceRepository> sources,
Guid parentId,
bool forceRestore,
ILogger log,
CancellationToken token)
{
// Check if there are actual projects to restore before running.
if (dgSpec.Restore.Count > 0)
{
using (var sourceCacheContext = new SourceCacheContext())
{
// Update cache context
cacheContextModifier(sourceCacheContext);

var restoreContext = GetRestoreContext(
context,
providerCache,
sourceCacheContext,
sources,
dgSpec,
parentId,
forceRestore: forceRestore,
isRestore: false);

var restoreSummaries = await RestoreRunner.RunAsync(restoreContext, token);

RestoreSummary.Log(log, restoreSummaries);

return restoreSummaries;
}
}

return new List<RestoreSummary>();
}

private static async Task PersistDGSpec(DependencyGraphSpec dgSpec)
{
try
Expand Down Expand Up @@ -176,7 +134,7 @@ private static async Task PersistDGSpec(DependencyGraphSpec dgSpec)
dgFile,
parentId,
forceRestore: true,
isRestore: false);
isRestoreOriginalAction: false);

var requests = await RestoreRunner.GetRequests(restoreContext);
var results = await RestoreRunner.RunWithoutCommit(requests, restoreContext);
Expand Down Expand Up @@ -289,7 +247,7 @@ public static async Task<PackageSpec> GetProjectSpec(IDependencyGraphProject pro
DependencyGraphSpec dgFile,
Guid parentId,
bool forceRestore,
bool isRestore)
bool isRestoreOriginalAction)
{
var caching = new CachingSourceProvider(new PackageSourceProvider(context.Settings));
foreach( var source in sources)
Expand All @@ -307,7 +265,7 @@ public static async Task<PackageSpec> GetProjectSpec(IDependencyGraphProject pro
AllowNoOp = !forceRestore,
CachingSourceProvider = caching,
ParentId = parentId,
IsRestore = isRestore
IsRestoreOriginalAction = isRestoreOriginalAction
};

return restoreContext;
Expand Down
8 changes: 5 additions & 3 deletions src/NuGet.Core/NuGet.PackageManagement/NuGetPackageManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2892,15 +2892,17 @@ var projectUniqueNamesForBuildIntToUpdate
// Restore and commit the lock file to disk regardless of the result
// This will restore all parents in a single restore
await DependencyGraphRestoreUtility.RestoreAsync(
SolutionManager,
dgSpecForParents,
referenceContext,
GetRestoreProviderCache(),
cacheContextModifier,
projectAction.Sources,
nuGetProjectContext.OperationId,
false, // No need to force restore as the inputs would've changed here anyways
logger,
token);
forceRestore: false, // No need to force restore as the inputs would've changed here anyways
isRestoreOriginalAction: false, // not an explicit restore request instead being done as part of install or update
log: logger,
token: token);
}
}
else
Expand Down
74 changes: 74 additions & 0 deletions src/NuGet.Core/NuGet.ProjectModel/JsonUtility.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
// 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.IO;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using NuGet.Packaging.Core;
using NuGet.Versioning;

namespace NuGet.ProjectModel
{
internal static class JsonUtility
{
internal static readonly char[] PathSplitChars = new[] { LockFile.DirectorySeparatorChar };

/// <summary>
/// JsonLoadSettings with line info and comments ignored.
/// </summary>
Expand Down Expand Up @@ -36,5 +42,73 @@ internal static JObject LoadJson(TextReader reader)
return JObject.Load(jsonReader, DefaultLoadSettings);
}
}

internal static PackageDependency ReadPackageDependency(string property, JToken json)
{
var versionStr = json.Value<string>();
return new PackageDependency(
property,
versionStr == null ? null : VersionRange.Parse(versionStr));
}

internal static JProperty WritePackageDependency(PackageDependency item)
{
return new JProperty(
item.Id,
WriteString(item.VersionRange?.ToLegacyShortString()));
}

internal static TItem ReadProperty<TItem>(JObject jObject, string propertyName)
{
if (jObject != null)
{
JToken value;
if (jObject.TryGetValue(propertyName, out value) && value != null)
{
return value.Value<TItem>();
}
}

return default(TItem);
}

internal static IList<TItem> ReadObject<TItem>(JObject jObject, Func<string, JToken, TItem> readItem)
{
if (jObject == null)
{
return new List<TItem>();
}
var items = new List<TItem>();
foreach (var child in jObject)
{
items.Add(readItem(child.Key, child.Value));
}
return items;
}

internal static JObject WriteObject<TItem>(IEnumerable<TItem> items, Func<TItem, JProperty> writeItem)
{
var array = new JObject();
foreach (var item in items)
{
array.Add(writeItem(item));
}
return array;
}

internal static int ReadInt(JToken cursor, string property, int defaultValue)
{
var valueToken = cursor[property];
if (valueToken == null)
{
return defaultValue;
}
return valueToken.Value<int>();
}

internal static JToken WriteString(string item)
{
return item != null ? new JValue(item) : JValue.CreateNull();
}
}
}
15 changes: 0 additions & 15 deletions src/NuGet.Core/NuGet.ProjectModel/LockFile/LockFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,6 @@ public bool Equals(LockFile other)
&& LogsEqual(other.LogMessages);
}

public LockFile Clone()
{
var lockFile = new LockFile();
lockFile.Version = Version;
lockFile.Path = Path;
lockFile.ProjectFileDependencyGroups = ProjectFileDependencyGroups?.Select(item => item.Clone()).ToList();
lockFile.Libraries = Libraries?.Select(item => item.Clone()).ToList();
lockFile.Targets = Targets?.Select(item => item.Clone()).ToList();
lockFile.PackageFolders = new List<LockFileItem>(PackageFolders);
lockFile.PackageSpec = PackageSpec.Clone();
lockFile.LogMessages = new List<IAssetsLogMessage>(LogMessages);

return lockFile;
}

private bool LogsEqual(IList<IAssetsLogMessage> otherLogMessages)
{
if (ReferenceEquals(LogMessages, otherLogMessages))
Expand Down
Loading

0 comments on commit 22054ce

Please sign in to comment.