Skip to content
Open
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 @@ -1083,6 +1083,7 @@ private PackageSpec GetPackageSpec(IMSBuildProject project, IReadOnlyDictionary<
restoreMetadata.UsingMicrosoftNETSdk = MSBuildRestoreUtility.GetUsingMicrosoftNETSdk(project.GetProperty("UsingMicrosoftNETSdk"));
restoreMetadata.SdkAnalysisLevel = MSBuildRestoreUtility.GetSdkAnalysisLevel(project.GetProperty("SdkAnalysisLevel"));
restoreMetadata.UseLegacyDependencyResolver = project.IsPropertyTrue("RestoreUseLegacyDependencyResolver");
restoreMetadata.RestoreDoNotWriteDependencyGraphSpec = project.IsPropertyTrue("RestoreDoNotWriteDependencyGraphSpec");

return (restoreMetadata, targetFrameworkInfos);

Expand Down
1 change: 1 addition & 0 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<UsingMicrosoftNETSdk>$(UsingMicrosoftNETSdk)</UsingMicrosoftNETSdk>
<NETCoreSdkVersion>$(NETCoreSdkVersion)</NETCoreSdkVersion>
<RestoreUseLegacyDependencyResolver>$(RestoreUseLegacyDependencyResolver)</RestoreUseLegacyDependencyResolver>
<RestoreDoNotWriteDependencyGraphSpec>$(RestoreDoNotWriteDependencyGraphSpec)</RestoreDoNotWriteDependencyGraphSpec>
</_RestoreGraphEntry>
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ public async Task<RestoreResult> ExecuteAsync(CancellationToken token)
restoreTime.Elapsed)
{
AuditRan = auditRan,
DidDGHashChange = !noOpCacheFileEvaluation
DidDGHashChange = !noOpCacheFileEvaluation,
DoNotWriteDependencyGraphSpec = _request.Project.RestoreMetadata.RestoreDoNotWriteDependencyGraphSpec
};

telemetry.TelemetryEvent[UpdatedAssetsFile] = restoreResult._isAssetsFileDirty.Value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ public class RestoreResult : IRestoreResult
/// </summary>
internal bool DidDGHashChange { get; init; }

/// <summary>
/// If true, the dg spec file should not be written to disk.
/// </summary>
internal bool DoNotWriteDependencyGraphSpec { get; init; }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: extra newline.


private readonly string _dependencyGraphSpecFilePath;

Expand Down Expand Up @@ -317,7 +322,7 @@ await FileUtility.ReplaceWithLock(

private async Task CommitDgSpecFileAsync(ILogger log, bool toolCommit)
{
if (!toolCommit && _dependencyGraphSpecFilePath != null && _dependencyGraphSpec != null && (DidDGHashChange || !File.Exists(_dependencyGraphSpecFilePath)))
if (!toolCommit && !DoNotWriteDependencyGraphSpec && _dependencyGraphSpecFilePath != null && _dependencyGraphSpec != null && (DidDGHashChange || !File.Exists(_dependencyGraphSpecFilePath)))
{
log.LogVerbose($"Persisting dg to {_dependencyGraphSpecFilePath}");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ public static PackageSpec GetPackageSpec(IEnumerable<IMSBuildItem> items)
result.RestoreMetadata.UsingMicrosoftNETSdk = GetUsingMicrosoftNETSdk(specItem.GetProperty("UsingMicrosoftNETSdk"));
result.RestoreMetadata.SdkAnalysisLevel = GetSdkAnalysisLevel(specItem.GetProperty("SdkAnalysisLevel"));
result.RestoreMetadata.UseLegacyDependencyResolver = IsPropertyTrue(specItem, "RestoreUseLegacyDependencyResolver");
result.RestoreMetadata.RestoreDoNotWriteDependencyGraphSpec = IsPropertyTrue(specItem, "RestoreDoNotWriteDependencyGraphSpec");
result.RestoreSettings.SdkVersion = GetSdkAnalysisLevel(specItem.GetProperty("NETCoreSdkVersion"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ private static (ProjectRestoreMetadata? RestoreMetadata, List<TargetFrameworkInf
restoreMetadata.UsingMicrosoftNETSdk = MSBuildRestoreUtility.GetUsingMicrosoftNETSdk(outerBuild.GetProperty("UsingMicrosoftNETSdk"));
restoreMetadata.SdkAnalysisLevel = MSBuildRestoreUtility.GetSdkAnalysisLevel(outerBuild.GetProperty("SdkAnalysisLevel"));
restoreMetadata.UseLegacyDependencyResolver = outerBuild.IsPropertyTrue("RestoreUseLegacyDependencyResolver");
restoreMetadata.RestoreDoNotWriteDependencyGraphSpec = outerBuild.IsPropertyTrue("RestoreDoNotWriteDependencyGraphSpec");

return (restoreMetadata, targetFrameworkInfos);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public partial class JsonPackageSpecReader
private static readonly byte[] SdkAnalysisLevel = Encoding.UTF8.GetBytes("SdkAnalysisLevel");
private static readonly byte[] UsingMicrosoftNETSdk = Encoding.UTF8.GetBytes("UsingMicrosoftNETSdk");
private static readonly byte[] UseLegacyDependencyResolverPropertyName = Encoding.UTF8.GetBytes("restoreUseLegacyDependencyResolver");
private static readonly byte[] RestoreDoNotWriteDependencyGraphSpecPropertyName = Encoding.UTF8.GetBytes("restoreDoNotWriteDependencyGraphSpec");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if this should be a part of the no-op invalidation.

Like in theory, if I went from no to yes, I'd want the dgspec to be regenerated, but I know there's been attempts at using CI data to make local restores more efficient.

My only concern with not adding it though is it becoming out of date.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was worried about special casing any particular property, just in case that little detail is lost in the overall design. I could go either way...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whoever is using CI caching can just not set this anyways.
Let's leave it.

Will this be enabled across all of 1ES or just whenever wanted?

private static readonly byte[] PackagesToPrunePropertyName = Encoding.UTF8.GetBytes("packagesToPrune");

internal static PackageSpec GetPackageSpecUtf8JsonStreamReader(Stream stream, string name, string packageSpecPath, IEnvironmentVariableReader environmentVariableReader, string snapshotValue = null)
Expand Down Expand Up @@ -775,6 +776,7 @@ private static void ReadMSBuildMetadata(ref Utf8JsonStreamReader jsonReader, Pac
bool usingMicrosoftNetSdk = true;
NuGetVersion sdkAnalysisLevel = null;
bool useLegacyDependencyResolver = false;
bool restoreDoNotWriteDependencyGraphSpec = false;

if (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.StartObject)
{
Expand Down Expand Up @@ -1033,6 +1035,10 @@ private static void ReadMSBuildMetadata(ref Utf8JsonStreamReader jsonReader, Pac
{
useLegacyDependencyResolver = jsonReader.ReadNextTokenAsBoolOrThrowAnException(UseLegacyDependencyResolverPropertyName, Strings.Invalid_AttributeValue);
}
else if (jsonReader.ValueTextEquals(RestoreDoNotWriteDependencyGraphSpecPropertyName))
{
restoreDoNotWriteDependencyGraphSpec = jsonReader.ReadNextTokenAsBoolOrThrowAnException(RestoreDoNotWriteDependencyGraphSpecPropertyName, Strings.Invalid_AttributeValue);
}
else
{
jsonReader.Skip();
Expand Down Expand Up @@ -1061,6 +1067,7 @@ private static void ReadMSBuildMetadata(ref Utf8JsonStreamReader jsonReader, Pac
msbuildMetadata.SdkAnalysisLevel = sdkAnalysisLevel;
msbuildMetadata.UsingMicrosoftNETSdk = usingMicrosoftNetSdk;
msbuildMetadata.UseLegacyDependencyResolver = useLegacyDependencyResolver;
msbuildMetadata.RestoreDoNotWriteDependencyGraphSpec = restoreDoNotWriteDependencyGraphSpec;

if (configFilePaths != null)
{
Expand Down
1 change: 1 addition & 0 deletions src/NuGet.Core/NuGet.ProjectModel/PackageSpecWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ private static void WriteMetadataBooleans(IObjectWriter writer, ProjectRestoreMe
SetValueIfTrue(writer, "CentralPackageTransitivePinningEnabled", msbuildMetadata.CentralPackageTransitivePinningEnabled);
SetValueIfFalse(writer, "UsingMicrosoftNETSdk", msbuildMetadata.UsingMicrosoftNETSdk);
SetValueIfTrue(writer, "restoreUseLegacyDependencyResolver", msbuildMetadata.UseLegacyDependencyResolver);
SetValueIfTrue(writer, "restoreDoNotWriteDependencyGraphSpec", msbuildMetadata.RestoreDoNotWriteDependencyGraphSpec);
}


Expand Down
11 changes: 10 additions & 1 deletion src/NuGet.Core/NuGet.ProjectModel/ProjectRestoreMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ public class ProjectRestoreMetadata : IEquatable<ProjectRestoreMetadata>

public bool UseLegacyDependencyResolver { get; set; }

/// <summary>
/// Gets or sets a value indicating whether or not the dependency graph spec file should not be written during restore.
/// The default value is <see langword="false" />.
/// </summary>
public bool RestoreDoNotWriteDependencyGraphSpec { get; set; }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

get; init; maybe?

Get it right from the start since we want these to be immutable eventually.

I wish I had added UseLegacyDependencyResolver as a get init. :D


public override int GetHashCode()
{
StringComparer osStringComparer = PathUtility.GetStringComparerBasedOnOS();
Expand Down Expand Up @@ -182,6 +188,7 @@ public override int GetHashCode()
hashCode.AddObject(UsingMicrosoftNETSdk);
hashCode.AddObject(SdkAnalysisLevel);
hashCode.AddObject(UseLegacyDependencyResolver);
hashCode.AddObject(RestoreDoNotWriteDependencyGraphSpec);

return hashCode.CombinedHash;
}
Expand Down Expand Up @@ -230,7 +237,8 @@ public bool Equals(ProjectRestoreMetadata other)
RestoreAuditProperties == other.RestoreAuditProperties &&
UsingMicrosoftNETSdk == other.UsingMicrosoftNETSdk &&
EqualityUtility.EqualsWithNullCheck(SdkAnalysisLevel, other.SdkAnalysisLevel) &&
UseLegacyDependencyResolver == other.UseLegacyDependencyResolver;
UseLegacyDependencyResolver == other.UseLegacyDependencyResolver &&
RestoreDoNotWriteDependencyGraphSpec == other.RestoreDoNotWriteDependencyGraphSpec;
}

private HashSet<string> GetSources(IList<PackageSource> sources)
Expand Down Expand Up @@ -284,6 +292,7 @@ protected void FillClone(ProjectRestoreMetadata clone)
clone.SdkAnalysisLevel = SdkAnalysisLevel;
clone.UsingMicrosoftNETSdk = UsingMicrosoftNETSdk;
clone.UseLegacyDependencyResolver = UseLegacyDependencyResolver;
clone.RestoreDoNotWriteDependencyGraphSpec = RestoreDoNotWriteDependencyGraphSpec;
}
}
}
2 changes: 2 additions & 0 deletions src/NuGet.Core/NuGet.ProjectModel/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ const NuGet.ProjectModel.LockFileFormat.LegacyVersion = 3 -> int
~static NuGet.ProjectModel.PackageSpecOperations.AddOrUpdateDependency(NuGet.ProjectModel.PackageSpec spec, NuGet.Packaging.Core.PackageIdentity identity, System.Collections.Generic.IEnumerable<string> frameworksToAdd) -> void
~NuGet.ProjectModel.PackagesLockFileTarget.TargetAlias.get -> string
~NuGet.ProjectModel.PackagesLockFileTarget.TargetAlias.set -> void
NuGet.ProjectModel.ProjectRestoreMetadata.RestoreDoNotWriteDependencyGraphSpec.get -> bool
NuGet.ProjectModel.ProjectRestoreMetadata.RestoreDoNotWriteDependencyGraphSpec.set -> void
Original file line number Diff line number Diff line change
Expand Up @@ -4873,6 +4873,52 @@ private IMSBuildItem CreateItems(IDictionary<string, string> properties)
return new MSBuildItem(Guid.NewGuid().ToString(), properties);
}

[Theory]
[InlineData("true", true)]
[InlineData("false", false)]
[InlineData(null, false)]
[InlineData("", false)]
public void MSBuildRestoreUtility_GetPackageSpec_RestoreDoNotWriteDependencyGraphSpec(
string propertyValue,
bool expectedValue)
{
using (var workingDir = TestDirectory.Create())
{
// Arrange
var project1Root = Path.Combine(workingDir, "a");
var project1Path = Path.Combine(project1Root, "a.csproj");

var items = new List<IDictionary<string, string>>();

var properties = new Dictionary<string, string>()
{
{ "Type", "ProjectSpec" },
{ "Version", "2.0.0" },
{ "ProjectName", "a" },
{ "ProjectStyle", "PackageReference" },
{ "ProjectUniqueName", "482C20DE-DFF9-4BD0-B90A-BD3201AA351A" },
{ "ProjectPath", project1Path },
{ "TargetFrameworks", "net46" },
};

if (propertyValue != null)
{
properties["RestoreDoNotWriteDependencyGraphSpec"] = propertyValue;
}

items.Add(properties);

var wrappedItems = items.Select(CreateItems).ToList();

// Act
var dgSpec = MSBuildRestoreUtility.GetDependencySpec(wrappedItems);
var project1Spec = dgSpec.Projects.Single();

// Assert
project1Spec.RestoreMetadata.RestoreDoNotWriteDependencyGraphSpec.Should().Be(expectedValue);
}
}

private Dictionary<string, string> WithUniqueName(Dictionary<string, string> item, string uniqueName)
{
var newItem = new Dictionary<string, string>(item, StringComparer.OrdinalIgnoreCase);
Expand Down
51 changes: 51 additions & 0 deletions test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreResultTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,57 @@ public async Task CommitAsync_WithExistingDGSpecAndDidDGHashChange_WritesDepende
Assert.True(File.Exists(dgSpecPath));
}

[Fact]
public async Task CommitAsync_WhenDoNotWriteDependencyGraphSpecIsTrue_DoesNotWriteDgSpec()
{
// Arrange
using var td = TestDirectory.Create();
var path = Path.Combine(td, "project.assets.json");
var cachePath = Path.Combine(td, "project.csproj.nuget.cache");
var dgSpecPath = Path.Combine(td, "project1.nuget.g.dgspec.json");
var dgSpec = new DependencyGraphSpec();
var configJson = @"
{
""frameworks"": {
""net45"": { }
}
}";

var spec = JsonPackageSpecReader.GetPackageSpec(configJson, "TestProject", Path.Combine(td, "project.csproj")).WithTestRestoreMetadata();
dgSpec.AddProject(spec);
dgSpec.AddRestore(spec.Name);

var logger = new TestLogger();
var result = new RestoreResult(
success: true,
restoreGraphs: null,
compatibilityCheckResults: null,
lockFile: new LockFile(),
previousLockFile: null,
lockFilePath: path,
msbuildFiles: Enumerable.Empty<MSBuildOutputFile>(),
cacheFile: new CacheFile("NotSoRandomString"),
cacheFilePath: cachePath,
packagesLockFilePath: null,
packagesLockFile: null,
dependencyGraphSpecFilePath: dgSpecPath,
dependencyGraphSpec: dgSpec,
projectStyle: ProjectStyle.Unknown,
elapsedTime: TimeSpan.MinValue)
{
DoNotWriteDependencyGraphSpec = true
};

// Act
await result.CommitAsync(logger, CancellationToken.None);

// Assert
Assert.DoesNotContain(
logger.VerboseMessages,
m => m.Contains("Persisting dg"));
Assert.False(File.Exists(dgSpecPath));
}

[Fact]
public void WhenRestoreResult_LogMessagesAreSourcedFromTheAssetsFile()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,62 @@ public void Write_ReadWriteWarningProperties()
VerifyJsonPackageSpecRoundTrip(json);
}

[Fact]
public void Write_ReadWrite_RestoreDoNotWriteDependencyGraphSpec_True()
{
// Arrange
var json = @"{
""restore"": {
""projectUniqueName"": ""projectUniqueName"",
""projectName"": ""projectName"",
""projectPath"": ""projectPath"",
""packagesPath"": ""packagesPath"",
""outputPath"": ""outputPath"",
""projectStyle"": ""PackageReference"",
""restoreDoNotWriteDependencyGraphSpec"": true,
""frameworks"": {
""net45"": {
""framework"": ""net45"",
""projectReferences"": {}
}
}
}
}";
// Act & Assert
VerifyJsonPackageSpecRoundTrip(json);
}

[Fact]
public void Write_ReadWrite_RestoreDoNotWriteDependencyGraphSpec_DefaultFalse_NotWritten()
{
// Arrange - when RestoreDoNotWriteDependencyGraphSpec is false (default), it should not appear in output
var json = @"{
""restore"": {
""projectUniqueName"": ""projectUniqueName"",
""projectName"": ""projectName"",
""projectPath"": ""projectPath"",
""packagesPath"": ""packagesPath"",
""outputPath"": ""outputPath"",
""projectStyle"": ""PackageReference"",
""frameworks"": {
""net45"": {
""framework"": ""net45"",
""projectReferences"": {}
}
}
}
}";
// Act
var spec = JsonPackageSpecReader.GetPackageSpec(json, "TestProject", "project.csproj");

// Assert - default value should be false
spec.RestoreMetadata.RestoreDoNotWriteDependencyGraphSpec.Should().BeFalse();

// And it should not appear in the serialized output
var output = GetJsonString(spec);
output.Should().NotContain("restoreDoNotWriteDependencyGraphSpec");
}

[Fact]
public void Write_SerializesMembersAsJson()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -833,5 +833,40 @@ private static void AssertHashCode(bool expected, ProjectRestoreMetadata leftSid
leftSide.GetHashCode().Should().NotBe(rightSide.GetHashCode());
}
}

[Theory]
[InlineData(true, true, true)]
[InlineData(false, false, true)]
[InlineData(true, false, false)]
[InlineData(false, true, false)]
public void Equals_WithRestoreDoNotWriteDependencyGraphSpec(bool left, bool right, bool expected)
{
var leftSide = new ProjectRestoreMetadata
{
RestoreDoNotWriteDependencyGraphSpec = left
};

var rightSide = new ProjectRestoreMetadata
{
RestoreDoNotWriteDependencyGraphSpec = right
};

AssertEquality(expected, leftSide, rightSide);
AssertHashCode(expected, leftSide, rightSide);
}

[Fact]
public void Clone_WithRestoreDoNotWriteDependencyGraphSpec()
{
var original = new ProjectRestoreMetadata
{
RestoreDoNotWriteDependencyGraphSpec = true
};

var clone = original.Clone();

clone.RestoreDoNotWriteDependencyGraphSpec.Should().Be(true);
AssertEquality(true, original, clone);
}
}
}