Skip to content
Closed
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
2 changes: 1 addition & 1 deletion eng/Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@
Language Server
-->
<PackageVersion Include="Microsoft.VisualStudio.LanguageServer.Client.Implementation" Version="17.10.72-preview" />
<PackageVersion Include="NuGet.ProjectModel" Version="6.8.0-rc.112" />
<PackageVersion Include="NuGet.ProjectModel" Version="6.14.0" />
<PackageVersion Include="Microsoft.TestPlatform.TranslationLayer" Version="$(MicrosoftNETTestSdkVersion)" />
<PackageVersion Include="Microsoft.TestPlatform.ObjectModel" Version="$(MicrosoftNETTestSdkVersion)" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.LanguageServer.Handler;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.Extensions.Logging;
using NuGet.ProjectModel;
using NuGet.Versioning;
Expand All @@ -14,6 +16,8 @@ namespace Microsoft.CodeAnalysis.LanguageServer.HostWorkspace;

internal static class ProjectDependencyHelper
{
private const string NuGetCacheFileName = "project.nuget.cache";

internal static bool NeedsRestore(ProjectFileInfo newProjectFileInfo, ProjectFileInfo? previousProjectFileInfo, ILogger logger)
{
if (previousProjectFileInfo is null)
Expand Down Expand Up @@ -60,6 +64,155 @@ private static bool CheckProjectAssetsForUnresolvedDependencies(ProjectFileInfo
return false;
}

// Fast path: consult the 'project.nuget.cache' file NuGet writes next to the assets file. If it
// confirms the last restore still accounts for every package reference the project declares, we can
// skip parsing the (potentially large) project.assets.json entirely.
if (TryConfirmRestoreUpToDateFromNuGetCache(projectFileInfo, projectAssetsPath))
{
return false;
}

return CheckAssetsFileForUnresolvedReferences(projectFileInfo, projectAssetsPath, logger);

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.

just to check my understanding, what would happen if we just return true; in this path?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This would restore. The risk is that checking the cache file could mis-report a package as missing (we're parsing a version and id from the path). If we just return true here it could lead to a restore loop.

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.

So is the idea that certain project setups, will consistently fail to produce a project.nuget.cache that we can use to verify that restore was complete+successful?

When does that failure happen? Is it limited to old tooling versions, unusual project setups, ...?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Essentially I am not 100% confident we'll always be able to exactly match packages from the cache file, given that we have to parse the package id and versions from the file path.

I haven't been able to come up with an exact scenario where it breaks, but I can easily imagine how it could break (nuget path changes to not include version, some kind of mismatch in package path and package id, casing differences, encoding differences, etc).

So I left the original code in there as a defensive fallback (which is still better than before due using STJ).

One alternative is to instead hand-roll a parser for the project.assets.json and only use that - I think we could get similar allocations as using the cache file, but we'd no longer be using the nuget API.

}

/// <summary>
/// Uses the <c>project.nuget.cache</c> file NuGet writes next to <paramref name="projectAssetsPath"/> to
/// confirm without parsing the (potentially large) <c>project.assets.json</c> that the last restore is still
/// applicable to the project we're loading. The cache must be valid (<see cref="CacheFile.IsValid"/> verifies
/// the format version matches the NuGet library we build against, the last restore succeeded, and a dependency
/// graph hash was recorded) and the packages that restore produced
/// (<see cref="CacheFile.ExpectedPackageFilePaths"/>) must satisfy every package reference the project currently
/// declares. The latter check catches a project whose package set changed since the last restore—for example a
/// package added directly to the project or via a shared <c>Directory.Packages.props</c>—in which case the
/// previously restored assets no longer reflect the project and we can't claim it's up to date.
/// </summary>
/// <returns>
/// <see langword="true"/> only when the cache proves the restore is current for every package reference the
/// project declares; otherwise <see langword="false"/> (the cache is missing, invalid, or doesn't account for
/// all of the project's package references), in which case the caller should inspect the assets file directly.
/// </returns>
/// <remarks>
/// This intentionally does not reproduce NuGet's dependency-graph-hash comparison, which needs the current
/// restore dependency graph that is not available here. Instead it confirms the restored package set covers the
/// project's declared references and otherwise defers to the precise assets-file check, so it can only ever
/// accelerate the up-to-date case and never report a stale restore as current.
Comment on lines +95 to +98
/// </remarks>
private static bool TryConfirmRestoreUpToDateFromNuGetCache(ProjectFileInfo projectFileInfo, string projectAssetsPath)
{
var cachePath = Path.Combine(Path.GetDirectoryName(projectAssetsPath)!, NuGetCacheFileName);
if (!File.Exists(cachePath))
{
return false;
}

var cacheFile = IOUtilities.PerformIO<CacheFile?>(() =>
{
using var stream = File.OpenRead(cachePath);
return CacheFileFormat.Read(stream, NuGet.Common.NullLogger.Instance, cachePath);
});

if (cacheFile is null)
{
return false;
}

// IsValid checks Version == CacheFile.CurrentVersion (the format version this NuGet build understands),
// Success, and that a dependency graph hash was recorded.
if (!cacheFile.IsValid || cacheFile.ExpectedPackageFilePaths is not { Count: > 0 })
{
return false;
}

// Reconstruct the set of packages the last restore produced and confirm every package reference the
// project currently declares is present with a satisfying version. If a reference isn't accounted for,
// the project changed since the restore (or the cache is otherwise incomplete), so we can't confirm it's
// up to date and defer to the assets-file check.
var restoredPackages = TryCreateRestoredPackagesMap(cacheFile.ExpectedPackageFilePaths);
if (restoredPackages is null)
{
return false;
}

foreach (var reference in projectFileInfo.PackageReferences)
Comment thread
dibarbet marked this conversation as resolved.
{
if (!IsPackageReferenceResolved(reference, restoredPackages))
{
return false;
}
}

return true;
}

/// <summary>
/// Builds a map of package id to the versions present in the restore, derived from the
/// <see cref="CacheFile.ExpectedPackageFilePaths"/>. Each entry looks like
/// <c>{packagesFolder}/{id}/{version}/{id}.{version}.nupkg.sha512</c>, so the package id and resolved version
/// are the two directories that contain the file. Returns <see langword="null"/> if no package could be parsed.
/// </summary>
private static Dictionary<string, OneOrMany<NuGetVersion>>? TryCreateRestoredPackagesMap(IList<string> expectedPackageFilePaths)
{
var versionsById = new Dictionary<string, OneOrMany<NuGetVersion>>(StringComparer.OrdinalIgnoreCase);

foreach (var filePath in expectedPackageFilePaths)
{
var versionDirectory = Path.GetDirectoryName(filePath);
var idDirectory = Path.GetDirectoryName(versionDirectory);
if (string.IsNullOrEmpty(versionDirectory) || string.IsNullOrEmpty(idDirectory))
{
continue;
}

var id = Path.GetFileName(idDirectory);
if (string.IsNullOrEmpty(id) || !NuGetVersion.TryParse(Path.GetFileName(versionDirectory), out var version))
{
continue;
}

if (!versionsById.TryGetValue(id, out var versions))
{
versionsById.Add(id, OneOrMany.Create(version));
}
else if (!versions.Contains(version))
{
versionsById[id] = versions.Add(version);
}
}

return versionsById.Count == 0 ? null : versionsById;
}

/// <summary>
/// Determines whether <paramref name="reference"/> is satisfied by a restored package in
/// <paramref name="restoredPackages"/> (a map of package id to the versions present in the restore). A
/// reference is resolved when its name is present and at least one restored version satisfies the requested
/// version range.
/// </summary>
private static bool IsPackageReferenceResolved(PackageReferenceItem reference, IReadOnlyDictionary<string, OneOrMany<NuGetVersion>> restoredPackages)
{
if (!restoredPackages.TryGetValue(reference.Name, out var versions))
{
// The package name isn't in the restore at all.
return false;
}

var requestedVersionRange = VersionRange.TryParse(reference.VersionRange, out var versionRange)
? versionRange
: VersionRange.All;

foreach (var version in versions)
{
if (requestedVersionRange.Satisfies(version))
{
return true;
}
}

return false;
}

private static bool CheckAssetsFileForUnresolvedReferences(ProjectFileInfo projectFileInfo, string projectAssetsPath, ILogger logger)
{
// Iterate the project's package references and check if there is a package with the same name
// and acceptable version in the lock file.

Expand All @@ -71,21 +224,8 @@ private static bool CheckProjectAssetsForUnresolvedDependencies(ProjectFileInfo

foreach (var reference in projectFileInfo.PackageReferences)
{
if (!projectAssetsMap.TryGetValue(reference.Name, out var projectAssetsVersions))
if (!IsPackageReferenceResolved(reference, projectAssetsMap))
{
// If the package name isn't in the lock file then it's unresolved.
unresolved.Add(reference);
continue;
}

var requestedVersionRange = VersionRange.TryParse(reference.VersionRange, out var versionRange)
? versionRange
: VersionRange.All;

var projectAssetsHasVersion = projectAssetsVersions.Any(projectAssetsVersion => SatisfiesVersion(requestedVersionRange, projectAssetsVersion));
if (!projectAssetsHasVersion)
{
// If the package name is in the lock file but none of the versions satisfy the requested version range then it's unresolved.
unresolved.Add(reference);
}
}
Expand All @@ -101,20 +241,15 @@ private static bool CheckProjectAssetsForUnresolvedDependencies(ProjectFileInfo

return false;

static ImmutableDictionary<string, ImmutableArray<NuGetVersion>> CreateProjectAssetsMap(LockFile lockFile)
static ImmutableDictionary<string, OneOrMany<NuGetVersion>> CreateProjectAssetsMap(LockFile lockFile)
{
// Create a map of package names to all versions in the lock file.
var map = lockFile.Libraries
.GroupBy(l => l.Name, l => l.Version, StringComparer.OrdinalIgnoreCase)
.ToImmutableDictionary(g => g.Key, g => g.ToImmutableArray(), StringComparer.OrdinalIgnoreCase);
.ToImmutableDictionary(g => g.Key, g => OneOrMany.Create(g.ToImmutableArray()), StringComparer.OrdinalIgnoreCase);

return map;
}

static bool SatisfiesVersion(VersionRange requestedVersionRange, NuGetVersion projectAssetsVersion)
{
return requestedVersionRange.Satisfies(projectAssetsVersion);
}
}

internal static async Task RestoreProjectsAsync(WorkDoneProgressManager workDoneProgressManager, ImmutableArray<string> projectPaths, bool enableProgressReporting, DotnetCliHelper dotnetCliHelper, ILogger logger, CancellationToken cancellationToken)
Expand Down
Loading