Merge pull request #606 from qmfrederik/fix587

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

Author: AArnott

src/NerdBank.GitVersioning.Tests/VersionOracleTests.cs

@@ -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));
     }
@@ -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()
     {

src/NerdBank.GitVersioning/FilterPath.cs

@@ -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('/');

src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs

@@ -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)
@@ -194,26 +185,6 @@ bool ContainsRelevantChanges(IEnumerable<TreeEntryChanges> changes) =>
                         }
                     }
 
-                    /*
-                    // 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;
@@ -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));