Skip to content

Commit

Permalink
feat: improves Directory.Packages.props evaluation
Browse files Browse the repository at this point in the history
  • Loading branch information
shlomiassaf authored Sep 20, 2022
1 parent 9b9aa61 commit c5b244b
Show file tree
Hide file tree
Showing 15 changed files with 1,011 additions and 87 deletions.
19 changes: 9 additions & 10 deletions src/DotnetAffected.Abstractions/IChangesProvider.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using Microsoft.Build.Evaluation;
using System.Collections.Generic;

namespace DotnetAffected.Abstractions
{
Expand All @@ -17,18 +18,16 @@ public interface IChangesProvider
IEnumerable<string> GetChangedFiles(string directory, string from, string to);

/// <summary>
/// Uses the underlying changes provider to get
/// the text contents of a file at <paramref name="from"/> and at <paramref name="to"/>.
/// Uses the underlying changes provider to load a <see cref="Project"/> file at <paramref name="commitRef"/>.
/// </summary>
/// <param name="directory"></param>
/// <param name="pathToFile"></param>
/// <param name="from"></param>
/// <param name="to"></param>
/// <param name="commitRef"></param>
/// <param name="fallbackToHead">When true, uses the HEAD as the default commit, otherwise uses the current working directory. <br/>
/// Applicable only when <paramref name="commitRef"/> is null or empty.</param>
/// <returns></returns>
(string? FromText, string? ToText) GetTextFileContents(
string directory,
string pathToFile,
string from,
string to);
Project? LoadProject(string directory, string pathToFile, string? commitRef, bool fallbackToHead);

Project? LoadDirectoryPackagePropsProject(string directory, string pathToFile, string? commitRef, bool fallbackToHead);
}
}
18 changes: 8 additions & 10 deletions src/DotnetAffected.Core/AffectedExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,18 @@ private IEnumerable<PackageChange> FindChangedNugetPackages(
IEnumerable<string> changedFiles)
{
// Try to find a Directory.Packages.props file in the list of changed files
// We try to take the deepest file, assuming they import up
var packagePropsPath = changedFiles
.SingleOrDefault(f => f.EndsWith("Directory.Packages.props"));
.Where(f => f.EndsWith("Directory.Packages.props"))
.OrderByDescending(p => p.Length)
.Take(1)
.FirstOrDefault();

if (packagePropsPath is null)
{
return Enumerable.Empty<PackageChange>();
}

// Get the contents of the file at from/to revisions
var (fromFile, toFile) = _changesProvider
.GetTextFileContents(
_repositoryPath,
packagePropsPath,
_fromRef,
_toRef);
var fromFile = _changesProvider.LoadDirectoryPackagePropsProject(_repositoryPath, packagePropsPath, _fromRef, false);
var toFile = _changesProvider.LoadDirectoryPackagePropsProject(_repositoryPath, packagePropsPath, _toRef, true);

// Parse props files into package and version dictionary
var fromPackages = NugetHelper.ParseDirectoryPackageProps(fromFile);
Expand All @@ -148,5 +145,6 @@ private IEnumerable<PackageChange> FindChangedNugetPackages(
// Compare both dictionaries
return NugetHelper.DiffPackageDictionaries(fromPackages, toPackages);
}

}
}
12 changes: 9 additions & 3 deletions src/DotnetAffected.Core/AssumptionChangesProvider.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using DotnetAffected.Abstractions;
using LibGit2Sharp;
using Microsoft.Build.Evaluation;
using Microsoft.Build.Graph;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -34,10 +36,14 @@ public IEnumerable<string> GetChangedFiles(string directory, string @from, strin
.FindNodesByName(_assumptions)
.Select(n => n.ProjectInstance.FullPath);
}

/// <inheritdoc />
public (string FromText, string ToText) GetTextFileContents(string directory, string pathToFile, string from,
string to)
public Project? LoadProject(string directory, string pathToFile, string? commitRef, bool fallbackToHead)
{
throw new System.InvalidOperationException("--assume-changes should not try to access file contents");
}

public Project? LoadDirectoryPackagePropsProject(string directory, string pathToFile, string? commitRef, bool fallbackToHead)
{
throw new System.InvalidOperationException("--assume-changes should not try to access file contents");
}
Expand Down
8 changes: 4 additions & 4 deletions src/DotnetAffected.Core/DotnetAffected.Core.csproj
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<Project Sdk="Microsoft.NET.Sdk">
<Import Project="$(PackageDefaultsPropsPath)"/>
<Import Project="$(PackageDefaultsPropsPath)" />

<PropertyGroup>
<RootNamespace>DotnetAffected.Core</RootNamespace>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include="$(SourcesPath)/DotnetAffected.Abstractions/DotnetAffected.Abstractions.csproj"/>
<ProjectReference Include="$(SourcesPath)/DotnetAffected.Abstractions/DotnetAffected.Abstractions.csproj" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Build.Prediction"/>
<PackageReference Include="LibGit2Sharp"/>
<PackageReference Include="Microsoft.Build.Prediction" />
<PackageReference Include="LibGit2Sharp" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using LibGit2Sharp;
using Microsoft.Build.Evaluation;
using System;
using System.Collections.Generic;
using System.Linq;

namespace DotnetAffected.Core.FileSystem
{
/// <summary>
/// A wrapper around the git filesystem that works around the MSBuild issue https://github.com/dotnet/msbuild/issues/7956 <br/>
/// The build process does not use the file system to ge the content of a file when it needs to load a nested project. <br/>
/// It will only use the file system to query if a file exists or not, if it exist it will just load the file using the file system <para/>
///
/// To workaround it we simply listen to all FileExists requests and if the file exists and it's part of the <see cref="MsBuildGitFileSystem.UseFileSystem">commit</see>
/// we will eager load it on the spot, before the build will try to load it, so by the time the build loads it, it's in the cache. <br/>
///
/// We only listen before we process the root file and stop right after the root is loaded.
/// </summary>
internal class EagerCachingMsBuildGitFileSystem : MsBuildGitFileSystem, IDisposable
{
private readonly Commit? _commit;
private ProjectFactory? _projectFactory;
private Action<string>? _onEagerCacheRequired;

public EagerCachingMsBuildGitFileSystem(Repository repository, Commit? commit) : base(repository, commit)
{
_commit = commit;
}

/// <summary>
/// Use this for File.Exists(path)
/// </summary>
public override bool FileExists(string path)
{
var result = base.FileExists(path);
if (result && !UseFileSystem(path))
_onEagerCacheRequired?.Invoke(path);
return result;
}

public Project CreateProjectAndEagerLoadChildren(string path)
{
var projects = new List<Project>();
_projectFactory ??= new ProjectFactory(this, new ProjectCollection());

_onEagerCacheRequired = s => projects.Add(_projectFactory.CreateProject(s));
projects.Add(_projectFactory.CreateProject(path));
_onEagerCacheRequired = null;

return projects.Last();
}

public void Dispose()
{
_projectFactory?.Dispose();
}
}
}
Loading

0 comments on commit c5b244b

Please sign in to comment.