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

Path filters: Recurse into directories if child entries may be included by filters #606

Merged
merged 4 commits into from May 29, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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