Skip to content

Commit

Permalink
Merge pull request #606 from qmfrederik/fix587
Browse files Browse the repository at this point in the history
Path filters: Recurse into directories if child entries may be included by filters
  • Loading branch information
AArnott committed May 29, 2021
2 parents 8c11e8d + 0066fa0 commit 19614e2
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 31 deletions.
24 changes: 23 additions & 1 deletion src/NerdBank.GitVersioning.Tests/VersionOracleTests.cs
Expand Up @@ -444,7 +444,7 @@ public void Worktree_Support(bool detachedHead)
var context = this.CreateGitContext(workTreePath);
var oracleWorkTree = new VersionOracle(context);
Assert.Equal(oracleOriginal.Version, oracleWorkTree.Version);

Assert.True(context.TrySelectCommit("HEAD"));
Assert.True(context.TrySelectCommit(this.LibGit2Repository.Head.Tip.Sha));
}
Expand Down Expand Up @@ -731,6 +731,28 @@ public void GetVersionHeight_IncludeRootExcludeSome()
Assert.Equal(2, this.GetVersionHeight(relativeDirectory));
}

[Fact]
public void GetVersion_PathFilterInTwoDeepSubDirAndVersionBump()
{
this.InitializeSourceControl();

const string relativeDirectory = "src/lib";
var versionOptions = new VersionOptions
{
Version = new SemanticVersion("1.1"),
PathFilters = new FilterPath[]
{
new FilterPath(".", relativeDirectory),
},
};
this.WriteVersionFile(versionOptions, relativeDirectory);
Assert.Equal(1, this.GetVersionHeight(relativeDirectory));

versionOptions.Version = new SemanticVersion("1.2");
this.WriteVersionFile(versionOptions, relativeDirectory);
Assert.Equal(1, this.GetVersionHeight(relativeDirectory));
}

[Fact]
public void GetVersionHeight_ProjectDirectoryDifferentToVersionJsonDirectory()
{
Expand Down
25 changes: 25 additions & 0 deletions src/NerdBank.GitVersioning/FilterPath.cs
Expand Up @@ -199,6 +199,31 @@ public bool Includes(string repoRelativePath, bool ignoreCase)
stringComparison);
}

/// <summary>
/// Determines if children of <paramref name="repoRelativePath"/> may be included
/// by this <see cref="FilterPath"/>.
/// </summary>
/// <param name="repoRelativePath">Forward-slash delimited path (repo relative).</param>
/// <param name="ignoreCase">
/// Whether paths should be compared case insensitively.
/// Should be the 'core.ignorecase' config value for the repository.
/// </param>
/// <returns>
/// <see langword="true"/> if this <see cref="FilterPath"/> is an including filter that may match
/// children of <paramref name="repoRelativePath"/>, otherwise <see langword="false"/>.
/// </returns>
public bool IncludesChildren(string repoRelativePath, bool ignoreCase)
{
if (repoRelativePath is null)
throw new ArgumentNullException(nameof(repoRelativePath));

if (!this.IsInclude) return false;
if (this.IsRoot) return true;

var stringComparison = ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal;
return this.RepoRelativePath.StartsWith(repoRelativePath + "/", stringComparison);
}

private static (int dirsToAscend, StringBuilder result) GetRelativePath(string path, string relativeTo)
{
var pathParts = path.Split('/');
Expand Down
32 changes: 2 additions & 30 deletions src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs
Expand Up @@ -165,15 +165,6 @@ bool TryCalculateHeight(GitCommit commit)

var ignoreCase = repository.IgnoreCase;

/*
bool ContainsRelevantChanges(IEnumerable<TreeEntryChanges> changes) =>
excludePaths.Count == 0
? changes.Any()
// If there is a single change that isn't excluded,
// then this commit is relevant.
: changes.Any(change => !excludePaths.Any(exclude => exclude.Excludes(change.Path, ignoreCase)));
*/

int height = 1;

if (pathFilters != null)
Expand All @@ -194,26 +185,6 @@ bool TryCalculateHeight(GitCommit commit)
}
}

/*
// If there are no include paths, or any of the include
// paths refer to the root of the repository, then do not
// filter the diff at all.
var diffInclude =
includePaths.Count == 0 || pathFilters.Any(filter => filter.IsRoot)
? null
: includePaths;
// If the diff between this commit and any of its parents
// does not touch a path that we care about, don't bump the
// height.
var relevantCommit =
commit.Parents.Any()
? commit.Parents.Any(parent => ContainsRelevantChanges(commit.GetRepository().Diff
.Compare<TreeChanges>(parent.Tree, commit.Tree, diffInclude, DiffOptions)))
: ContainsRelevantChanges(commit.GetRepository().Diff
.Compare<TreeChanges>(null, commit.Tree, diffInclude, DiffOptions));
*/

if (!relevantCommit)
{
height = 0;
Expand Down Expand Up @@ -268,7 +239,8 @@ private static bool IsRelevantCommit(GitRepository repository, GitTree tree, Git

bool isRelevant =
// Either there are no include filters at all (i.e. everything is included), or there's an explicit include filter
(!filters.Any(f => f.IsInclude) || filters.Any(f => f.Includes(fullPath, repository.IgnoreCase)))
(!filters.Any(f => f.IsInclude) || filters.Any(f => f.Includes(fullPath, repository.IgnoreCase))
|| (!entry.IsFile && filters.Any(f => f.IncludesChildren(fullPath, repository.IgnoreCase))))
// The path is not excluded by any filters
&& !filters.Any(f => f.Excludes(fullPath, repository.IgnoreCase));

Expand Down

0 comments on commit 19614e2

Please sign in to comment.