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

Repeatable build using Project lock file implementation #2342

Merged
merged 1 commit into from
Sep 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -363,7 +363,11 @@ private async Task<PackageSpec> GetPackageSpecAsync(ISettings settings)
ProjectWideWarningProperties = WarningProperties.GetWarningProperties(
treatWarningsAsErrors: _vsProjectAdapter.TreatWarningsAsErrors,
noWarn: _vsProjectAdapter.NoWarn,
warningsAsErrors: _vsProjectAdapter.WarningsAsErrors)
warningsAsErrors: _vsProjectAdapter.WarningsAsErrors),
RestoreLockProperties = new RestoreLockProperties(
await _vsProjectAdapter.GetRestorePackagesWithLockFileAsync(),
await _vsProjectAdapter.GetNuGetLockFilePathAsync(),
Copy link
Member

Choose a reason for hiding this comment

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

Are you going to change these as well to align with the eventual NuGetLockFile name change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, these are fine since the MSBuild property name itself is NuGetLockFilePath

await _vsProjectAdapter.IsRestoreLockedAsync())
Copy link
Member

Choose a reason for hiding this comment

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

We might be repeating the same conversation that we've had before, but do we really want this?

What's the precedence order if you use a reevaluate switch from the UI?

I'm not arguing for or against yet, I'm just trying to understand the behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is RestoreLockedMode property which is to restrict restore updating lock file. This will primarily be for CI machines so that restore doesn't update lock file but dev boxes should always allow which is why default is allowed. But if this property is set even on dev boxes then we have to respect that, hence this code.

This flag is mentioned in the spec with explanation. If you think this is not clear then may be we can ask Anand to make it more clear. - https://github.com/NuGet/Home/wiki/Enable-repeatable-package-restore-using-lock-file

}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,31 @@ public async Task<NuGetFramework> GetTargetFrameworkAsync()
return NuGetFramework.UnsupportedFramework;
}

public async Task<string> GetRestorePackagesWithLockFileAsync()
{
await _threadingService.JoinableTaskFactory.SwitchToMainThreadAsync();

var value = await BuildProperties.GetPropertyValueAsync(ProjectBuildProperties.RestorePackagesWithLockFile);

return value;
}

public async Task<string> GetNuGetLockFilePathAsync()
{
await _threadingService.JoinableTaskFactory.SwitchToMainThreadAsync();

return await BuildProperties.GetPropertyValueAsync(ProjectBuildProperties.NuGetLockFilePath);
}

public async Task<bool> IsRestoreLockedAsync()
{
await _threadingService.JoinableTaskFactory.SwitchToMainThreadAsync();

var value = await BuildProperties.GetPropertyValueAsync(ProjectBuildProperties.RestoreLockedMode);

return MSBuildStringUtility.IsTrue(value);
}

private async Task<string> GetTargetFrameworkStringAsync()
{
await _threadingService.JoinableTaskFactory.SwitchToMainThreadAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,15 +329,17 @@ await _logger.RunWithProgressAsync(
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
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public sealed class VsSolutionRestoreService : IVsSolutionRestoreService, IVsSol
private const string TreatWarningsAsErrors = nameof(TreatWarningsAsErrors);
private const string WarningsAsErrors = nameof(WarningsAsErrors);
private const string NoWarn = nameof(NoWarn);

private const string RestorePackagesWithLockFile = nameof(RestorePackagesWithLockFile);
private const string NuGetLockFilePath = nameof(NuGetLockFilePath);
private const string RestoreLockedMode = nameof(RestoreLockedMode);

private static readonly Version Version20 = new Version(2, 0, 0, 0);

Expand Down Expand Up @@ -286,7 +288,11 @@ private static PackageSpec ToPackageSpec(ProjectNames projectNames, IVsProjectRe
treatWarningsAsErrors: GetSingleOrDefaultPropertyValue(projectRestoreInfo.TargetFrameworks, TreatWarningsAsErrors, e => e),
warningsAsErrors: GetSingleOrDefaultNuGetLogCodes(projectRestoreInfo.TargetFrameworks, WarningsAsErrors, e => MSBuildStringUtility.GetNuGetLogCodes(e)),
noWarn: GetSingleOrDefaultNuGetLogCodes(projectRestoreInfo.TargetFrameworks, NoWarn, e => MSBuildStringUtility.GetNuGetLogCodes(e))),
CacheFilePath = NoOpRestoreUtilities.GetProjectCacheFilePath(cacheRoot: outputPath, projectPath: projectFullPath)
CacheFilePath = NoOpRestoreUtilities.GetProjectCacheFilePath(cacheRoot: outputPath, projectPath: projectFullPath),
RestoreLockProperties = new RestoreLockProperties(
GetRestorePackagesWithLockFile(projectRestoreInfo.TargetFrameworks),
GetNuGetLockFilePath(projectRestoreInfo.TargetFrameworks),
IsLockFileFreezeOnRestore(projectRestoreInfo.TargetFrameworks))
},
RuntimeGraph = GetRuntimeGraph(projectRestoreInfo),
RestoreSettings = new ProjectRestoreSettings() { HideWarningsAndErrors = true }
Expand Down Expand Up @@ -316,6 +322,21 @@ private static string GetRestoreProjectPath(IVsTargetFrameworks tfms)
return GetSingleNonEvaluatedPropertyOrNull(tfms, RestorePackagesPath, e => e);
}

private static string GetRestorePackagesWithLockFile(IVsTargetFrameworks tfms)
{
return GetSingleNonEvaluatedPropertyOrNull(tfms, RestorePackagesWithLockFile, v => v);
}

private static string GetNuGetLockFilePath(IVsTargetFrameworks tfms)
{
return GetSingleNonEvaluatedPropertyOrNull(tfms, NuGetLockFilePath, v => v);
}

private static bool IsLockFileFreezeOnRestore(IVsTargetFrameworks tfms)
{
return GetSingleNonEvaluatedPropertyOrNull(tfms, RestoreLockedMode, MSBuildStringUtility.IsTrue);
}

/// <summary>
/// The result will contain CLEAR and no sources specified in RestoreSources if the clear keyword is in it.
/// If there are additional sources specified, the value AdditionalValue will be set in the result and then all the additional sources will follow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,23 @@ public interface IVsProjectAdapter
/// Project's target framework
/// </summary>
Task<NuGetFramework> GetTargetFrameworkAsync();

/// <summary>
/// RestorePackagesWithLockFile project property.
/// </summary>
/// <returns></returns>
Task<string> GetRestorePackagesWithLockFileAsync();

/// <summary>
/// NuGetLockFilePath project property.
/// </summary>
/// <returns></returns>
Task<string> GetNuGetLockFilePathAsync();

/// <summary>
/// RestoreLockedMode project property.
/// </summary>
/// <returns></returns>
Task<bool> IsRestoreLockedAsync();
}
}
6 changes: 5 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ Copyright (c) .NET Foundation. All rights reserved.
RestoreRecursive="$(RestoreRecursive)"
RestoreForce="$(RestoreForce)"
HideWarningsAndErrors="$(HideWarningsAndErrors)"
Interactive="$(NuGetInteractive)"/>
Interactive="$(NuGetInteractive)"
ReevaluateRestoreGraph="$(ReevaluateRestoreGraph)"/>
Copy link
Member

Choose a reason for hiding this comment

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

spec: This is not documented anywhere in the specs.

This is the reevaluate lock file, hose everything (well almost) right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the implementation part of --reevaluate flag passed with dotnet.exe restore Since this is not explicitly allowed or needed for msbuild hence not mentioned in the spec.

And yes, it just skip existing lock file and re-evaluate all the dependencies.

</Target>

<!--
Expand Down Expand Up @@ -657,6 +658,9 @@ Copyright (c) .NET Foundation. All rights reserved.
<TreatWarningsAsErrors>$(TreatWarningsAsErrors)</TreatWarningsAsErrors>
<WarningsAsErrors>$(WarningsAsErrors)</WarningsAsErrors>
<NoWarn>$(NoWarn)</NoWarn>
<RestorePackagesWithLockFile>$(RestorePackagesWithLockFile)</RestorePackagesWithLockFile>
<NuGetLockFilePath>$(NuGetLockFilePath)</NuGetLockFilePath>
<RestoreLockedMode>$(RestoreLockedMode)</RestoreLockedMode>
</_RestoreGraphEntry>
</ItemGroup>

Expand Down
9 changes: 8 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks/RestoreTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public class RestoreTask : Microsoft.Build.Utilities.Task, ICancelableTask, IDis
/// </summary>
public bool Interactive { get; set; }

/// <summary>
/// Reevaluate resotre graph even with a lock file, skip no op as well.
/// </summary>
public bool ReevaluateRestoreGraph { get; set; }

public override bool Execute()
{
var log = new MSBuildLogger(Log);
Expand All @@ -86,6 +91,7 @@ public override bool Execute()
log.LogDebug($"(in) RestoreRecursive '{RestoreRecursive}'");
log.LogDebug($"(in) RestoreForce '{RestoreForce}'");
log.LogDebug($"(in) HideWarningsAndErrors '{HideWarningsAndErrors}'");
log.LogDebug($"(in) Reevaluate '{ReevaluateRestoreGraph}'");

try
{
Expand Down Expand Up @@ -159,7 +165,8 @@ private async Task<bool> ExecuteAsync(Common.ILogger log)
PreLoadedRequestProviders = providers,
CachingSourceProvider = sourceProvider,
AllowNoOp = !RestoreForce,
HideWarningsAndErrors = HideWarningsAndErrors
HideWarningsAndErrors = HideWarningsAndErrors,
ReevaluateRestoreGraph = ReevaluateRestoreGraph
};

if (restoreContext.DisableParallel)
Expand Down
93 changes: 93 additions & 0 deletions src/NuGet.Core/NuGet.Commands/PackagesLockFileBuilder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// 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;
using NuGet.Packaging.Core;
using NuGet.ProjectModel;
using NuGet.Shared;

namespace NuGet.Commands
{
public class PackagesLockFileBuilder
{
public PackagesLockFile CreateNuGetLockFile(LockFile assetsFile)
{
var lockFile = new PackagesLockFile();

var libraryLookup = assetsFile.Libraries.Where(e => e.Type == LibraryType.Package)
.ToDictionary(e => new PackageIdentity(e.Name, e.Version));

foreach (var target in assetsFile.Targets)
{
var nuGettarget = new PackagesLockFileTarget()
{
TargetFramework = target.TargetFramework,
RuntimeIdentifier = target.RuntimeIdentifier
};

var framework = assetsFile.PackageSpec.TargetFrameworks.FirstOrDefault(
f => EqualityUtility.EqualsWithNullCheck(f.FrameworkName, target.TargetFramework));

var libraries = target.Libraries;

// check if this is RID-based graph then only add those libraries which differ from original TFM.
if (!string.IsNullOrEmpty(target.RuntimeIdentifier))
{
var onlyTFM = assetsFile.Targets.First(t => EqualityUtility.EqualsWithNullCheck(t.TargetFramework, target.TargetFramework));

libraries = target.Libraries.Where(lib => !onlyTFM.Libraries.Any(tfmLib => tfmLib.Equals(lib))).ToList();
}

foreach (var library in libraries.Where(e => e.Type == LibraryType.Package))
{
var identity = new PackageIdentity(library.Name, library.Version);

var dependency = new LockFileDependency()
{
Id = library.Name,
ResolvedVersion = library.Version,
Sha512 = libraryLookup[identity].Sha512,
Dependencies = library.Dependencies
};

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

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

nuGettarget.Dependencies.Add(dependency);
}

foreach (var projectReference in libraries.Where(e => e.Type == LibraryType.Project || e.Type == LibraryType.ExternalProject))
{
var dependency = new LockFileDependency()
{
Id = projectReference.Name,
Dependencies = projectReference.Dependencies,
Type = PackageDependencyType.Project
};

nuGettarget.Dependencies.Add(dependency);
}

nuGettarget.Dependencies = nuGettarget.Dependencies.OrderBy(d => d.Type).ToList();

lockFile.Targets.Add(nuGettarget);
}

return lockFile;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@ private RestoreSummaryRequest Create(
// Project.json is special cased to put assets file and generated .props and targets in the project folder
RestoreOutputPath = project.PackageSpec.RestoreMetadata.ProjectStyle == ProjectStyle.ProjectJson ? rootPath : project.PackageSpec.RestoreMetadata.OutputPath,
DependencyGraphSpec = projectDgSpec,
MSBuildProjectExtensionsPath = projectPackageSpec.RestoreMetadata.OutputPath,
ParentId = restoreArgs.ParentId
Copy link
Member

Choose a reason for hiding this comment

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

Why is the ParentId removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is being done through ApplyStandardProperties

MSBuildProjectExtensionsPath = projectPackageSpec.RestoreMetadata.OutputPath
};

var restoreLegacyPackagesDirectory = project.PackageSpec?.RestoreMetadata?.LegacyPackagesDirectory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class NoOpRestoreResult : RestoreResult
public NoOpRestoreResult(bool success, LockFile lockFile, LockFile previousLockFile, string lockFilePath, CacheFile cacheFile, string cacheFilePath, ProjectStyle projectStyle, TimeSpan elapsedTime) :
base(success : success, restoreGraphs : null, compatibilityCheckResults : new List<CompatibilityCheckResult>() ,
msbuildFiles : null, lockFile : lockFile, previousLockFile : previousLockFile, lockFilePath: lockFilePath,
cacheFile: cacheFile, cacheFilePath: cacheFilePath, projectStyle: projectStyle, elapsedTime: elapsedTime)
cacheFile: cacheFile, cacheFilePath: cacheFilePath, packagesLockFilePath:null, packagesLockFile:null, projectStyle: projectStyle, elapsedTime: elapsedTime)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public class RestoreArgs

public Guid ParentId { get; set; }

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

public bool ReevaluateRestoreGraph { get; set; }

// Cache directory -> ISettings
private ConcurrentDictionary<string, ISettings> _settingsCache
= new ConcurrentDictionary<string, ISettings>(StringComparer.Ordinal);
Expand Down Expand Up @@ -227,6 +231,9 @@ public void ApplyStandardProperties(RestoreRequest request)

request.AllowNoOp = !request.CacheContext.NoCache && AllowNoOp;
request.HideWarningsAndErrors = HideWarningsAndErrors;
request.ParentId = ParentId;
request.IsRestoreOriginalAction = IsRestoreOriginalAction;
request.ReevaluateRestoreGraph = ReevaluateRestoreGraph;
}
}
}
Loading