Skip to content

Feature: Support ignoring (filtering) paths in git tree #4420

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.

Sign up for GitHub

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
+197 −33
Open

Feature: Support ignoring (filtering) paths in git tree #4420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e45f63c
feat(paths-filter): first commit that supports paths filtering (ignor…
Feb 13, 2025
9d9c5dd
refactor(paths-filter): implement path find logic in gitrepository class
Feb 13, 2025
ad8bc2f
refactor: apply suggestions from code review
Roei-Levi Feb 16, 2025
354790e
fix: patchCache
Feb 16, 2025
1152953
refactor: restore VersionFilters in EffectiveConfiguration
Feb 16, 2025
a3003ef
refactor: add VersionIncrementSourceType to distinguish between versi…
Feb 16, 2025
b3a10eb
refactor: prefix local field access with 'this'
Feb 19, 2025
0e5359e
fix
Feb 19, 2025
6c0e49f
refactor: prefix local field access with 'this'
Feb 19, 2025
a397bf5
refactor(paths-filter): use regex in paths filter, remove filtering m…
Feb 27, 2025
e84e12b
refactor: compile and cache regexes
Mar 14, 2025
001b673
revert: mainline defer filtering of commits
Mar 14, 2025
ec630ba
refactor: patch finding
Mar 14, 2025
dd643e2
build
Mar 14, 2025
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
1 change: 1 addition & 0 deletions docs/input/docs/workflows/GitFlow/v1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ branches:
is-main-branch: false
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
1 change: 1 addition & 0 deletions docs/input/docs/workflows/GitHubFlow/v1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ branches:
is-main-branch: false
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
1 change: 1 addition & 0 deletions docs/input/docs/workflows/TrunkBased/preview1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ branches:
pre-release-weight: 30000
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
7 changes: 7 additions & 0 deletions schemas/6.1/GitVersion.configuration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,13 @@
"null"
]
},
"paths": {
"description": "A sequence of file paths to be excluded from the version calculations.",
"type": "array",
"items": {
"type": "string"
}
},
"sha": {
"description": "A sequence of SHAs to be excluded from the version calculations.",
"$ref": "#/$defs/hashSetOfString"
Expand Down
1 change: 1 addition & 0 deletions ...s/Configuration/ConfigurationProviderTests.CanWriteOutEffectiveConfiguration.approved.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ branches:
is-main-branch: false
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
1 change: 1 addition & 0 deletions src/GitVersion.Configuration.Tests/Workflows/approved/GitFlow/v1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ branches:
is-main-branch: false
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
1 change: 1 addition & 0 deletions src/GitVersion.Configuration.Tests/Workflows/approved/GitHubFlow/v1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ branches:
is-main-branch: false
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
1 change: 1 addition & 0 deletions src/GitVersion.Configuration.Tests/Workflows/approved/TrunkBased/preview1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ branches:
pre-release-weight: 30000
ignore:
sha: []
paths: []
mode: ContinuousDelivery
label: '{BranchName}'
increment: Inherit
Expand Down
8 changes: 8 additions & 0 deletions src/GitVersion.Configuration/IgnoreConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.ObjectModel;
using GitVersion.Configuration.Attributes;

namespace GitVersion.Configuration;
Expand All @@ -22,4 +23,11 @@ public string? BeforeString
[JsonPropertyName("sha")]
[JsonPropertyDescription("A sequence of SHAs to be excluded from the version calculations.")]
public HashSet<string> Shas { get; init; } = [];

[JsonIgnore]
IReadOnlyCollection<string> IIgnoreConfiguration.Paths => Paths;

[JsonPropertyName("paths")]
[JsonPropertyDescription("A sequence of file paths to be excluded from the version calculations.")]
public Collection<string> Paths { get; init; } = [];
}
11 changes: 11 additions & 0 deletions src/GitVersion.Core.Tests/VersionCalculation/PathFilterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using GitVersion.Core.Tests.Helpers;
using GitVersion.VersionCalculation;

namespace GitVersion.Core.Tests;

[TestFixture]
public class PathFilterTests : TestBase
{
[Test]
public void VerifyNullGuard() => Should.Throw<ArgumentNullException>(() => new PathFilter(null!, null!, null!));
}
5 changes: 4 additions & 1 deletion src/GitVersion.Core/Configuration/EffectiveConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using GitVersion.Extensions;
using GitVersion.Git;
using GitVersion.VersionCalculation;

namespace GitVersion.Configuration;
Expand All @@ -12,6 +13,8 @@ public record EffectiveConfiguration
public EffectiveConfiguration(
IGitVersionConfiguration configuration,
IBranchConfiguration branchConfiguration,
IGitRepository? repository = null,
Lazy<GitVersionContext>? versionContext = null,
EffectiveConfiguration? fallbackConfiguration = null)
{
configuration.NotNull();
Expand Down Expand Up @@ -67,7 +70,7 @@ public EffectiveConfiguration(
PatchVersionBumpMessage = configuration.PatchVersionBumpMessage;
NoBumpMessage = configuration.NoBumpMessage;
CommitMessageIncrementing = branchConfiguration.CommitMessageIncrementing.Value;
VersionFilters = configuration.Ignore.ToFilters();
VersionFilters = versionContext != null && repository != null ? configuration.Ignore.ToFilters(repository, versionContext.Value) : [];
Ignore = configuration.Ignore;
TracksReleaseBranches = branchConfiguration.TracksReleaseBranches ?? false;
IsReleaseBranch = branchConfiguration.IsReleaseBranch ?? false;
Expand Down
4 changes: 3 additions & 1 deletion src/GitVersion.Core/Configuration/IIgnoreConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@ public interface IIgnoreConfiguration

IReadOnlySet<string> Shas { get; }

bool IsEmpty => Before == null && Shas.Count == 0;
IReadOnlyCollection<string> Paths { get; }

bool IsEmpty => Before == null && Shas.Count == 0 && Paths.Count == 0;
}
5 changes: 3 additions & 2 deletions src/GitVersion.Core/Extensions/ConfigurationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public static EffectiveConfiguration GetEffectiveConfiguration(
{
fallbackConfiguration = parentConfiguration;
}
return new EffectiveConfiguration(configuration, branchConfiguration, fallbackConfiguration);
return new EffectiveConfiguration(configuration, branchConfiguration, fallbackConfiguration: fallbackConfiguration);
}

public static IBranchConfiguration GetBranchConfiguration(this IGitVersionConfiguration configuration, IBranch branch)
Expand All @@ -38,12 +38,13 @@ public static IBranchConfiguration GetBranchConfiguration(this IGitVersionConfig
return branchConfiguration;
}

public static IEnumerable<IVersionFilter> ToFilters(this IIgnoreConfiguration source)
public static IEnumerable<IVersionFilter> ToFilters(this IIgnoreConfiguration source, IGitRepository repository, GitVersionContext versionContext)
{
source.NotNull();

if (source.Shas.Count != 0) yield return new ShaVersionFilter(source.Shas);
if (source.Before.HasValue) yield return new MinDateVersionFilter(source.Before.Value);
if (source.Paths.Count != 0) yield return new PathFilter(repository, versionContext, source.Paths);
}

private static IEnumerable<IBranchConfiguration> GetBranchConfigurations(IGitVersionConfiguration configuration, string branchName)
Expand Down
1 change: 1 addition & 0 deletions src/GitVersion.Core/Git/IGitRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public interface IGitRepository : IDisposable
IRemoteCollection Remotes { get; }

ICommit? FindMergeBase(ICommit commit, ICommit otherCommit);
IReadOnlyList<string>? FindPatchPaths(ICommit commit, string? tagPrefix);
int UncommittedChangesCount();
void DiscoverRepository(string? gitDirectory);
}
4 changes: 0 additions & 4 deletions src/GitVersion.Core/PublicAPI.Shipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ GitVersion.Configuration.EffectiveConfiguration.AssemblyVersioningScheme.get ->
GitVersion.Configuration.EffectiveConfiguration.CommitDateFormat.get -> string?
GitVersion.Configuration.EffectiveConfiguration.CommitMessageIncrementing.get -> GitVersion.VersionCalculation.CommitMessageIncrementMode
GitVersion.Configuration.EffectiveConfiguration.DeploymentMode.get -> GitVersion.VersionCalculation.DeploymentMode
GitVersion.Configuration.EffectiveConfiguration.EffectiveConfiguration(GitVersion.Configuration.IGitVersionConfiguration! configuration, GitVersion.Configuration.IBranchConfiguration! branchConfiguration, GitVersion.Configuration.EffectiveConfiguration? fallbackConfiguration = null) -> void
GitVersion.Configuration.EffectiveConfiguration.Ignore.get -> GitVersion.Configuration.IIgnoreConfiguration!
GitVersion.Configuration.EffectiveConfiguration.Increment.get -> GitVersion.IncrementStrategy
GitVersion.Configuration.EffectiveConfiguration.IsMainBranch.get -> bool
Expand All @@ -88,7 +87,6 @@ GitVersion.Configuration.EffectiveConfiguration.TrackMergeMessage.get -> bool
GitVersion.Configuration.EffectiveConfiguration.TrackMergeTarget.get -> bool
GitVersion.Configuration.EffectiveConfiguration.TracksReleaseBranches.get -> bool
GitVersion.Configuration.EffectiveConfiguration.UpdateBuildNumber.get -> bool
GitVersion.Configuration.EffectiveConfiguration.VersionFilters.get -> System.Collections.Generic.IEnumerable<GitVersion.VersionCalculation.IVersionFilter!>!
GitVersion.Configuration.EffectiveConfiguration.VersionInBranchPattern.get -> string?
GitVersion.Configuration.EffectiveConfiguration.VersionStrategy.get -> GitVersion.VersionCalculation.VersionStrategies
GitVersion.Configuration.IBranchConfiguration
Expand Down Expand Up @@ -602,7 +600,6 @@ GitVersion.Settings.OnlyTrackedBranches -> bool
GitVersion.VersionCalculation.BaseVersion
GitVersion.VersionCalculation.BaseVersion.BaseVersion() -> void
GitVersion.VersionCalculation.BaseVersion.BaseVersion(GitVersion.VersionCalculation.BaseVersionOperand! Operand) -> void
GitVersion.VersionCalculation.BaseVersion.BaseVersion(string! source, GitVersion.SemanticVersion! semanticVersion, GitVersion.Git.ICommit? baseVersionSource = null) -> void
GitVersion.VersionCalculation.BaseVersion.BaseVersionSource.get -> GitVersion.Git.ICommit?
GitVersion.VersionCalculation.BaseVersion.GetIncrementedVersion() -> GitVersion.SemanticVersion!
GitVersion.VersionCalculation.BaseVersion.Operand.get -> GitVersion.VersionCalculation.BaseVersionOperand!
Expand All @@ -614,7 +611,6 @@ GitVersion.VersionCalculation.BaseVersion.ShouldIncrement.get -> bool
GitVersion.VersionCalculation.BaseVersion.Source.get -> string!
GitVersion.VersionCalculation.BaseVersionOperand
GitVersion.VersionCalculation.BaseVersionOperand.BaseVersionOperand() -> void
GitVersion.VersionCalculation.BaseVersionOperand.BaseVersionOperand(string! Source, GitVersion.SemanticVersion! SemanticVersion, GitVersion.Git.ICommit? BaseVersionSource = null) -> void
GitVersion.VersionCalculation.BaseVersionOperand.BaseVersionSource.get -> GitVersion.Git.ICommit?
GitVersion.VersionCalculation.BaseVersionOperand.BaseVersionSource.init -> void
GitVersion.VersionCalculation.BaseVersionOperand.SemanticVersion.get -> GitVersion.SemanticVersion!
Expand Down
17 changes: 17 additions & 0 deletions src/GitVersion.Core/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
#nullable enable
GitVersion.Configuration.EffectiveConfiguration.EffectiveConfiguration(GitVersion.Configuration.IGitVersionConfiguration! configuration, GitVersion.Configuration.IBranchConfiguration! branchConfiguration, GitVersion.Git.IGitRepository? repository = null, System.Lazy<GitVersion.GitVersionContext!>? versionContext = null, GitVersion.Configuration.EffectiveConfiguration? fallbackConfiguration = null) -> void
GitVersion.Configuration.EffectiveConfiguration.VersionFilters.get -> System.Collections.Generic.IEnumerable<GitVersion.VersionCalculation.IVersionFilter!>!
GitVersion.Configuration.IIgnoreConfiguration.Paths.get -> System.Collections.Generic.IReadOnlyCollection<string!>!
GitVersion.Extensions.FileSystemExtensions
GitVersion.Git.IGitRepository.FindPatchPaths(GitVersion.Git.ICommit! commit, string? tagPrefix) -> System.Collections.Generic.IReadOnlyList<string!>?
GitVersion.VersionCalculation.BaseVersion.BaseVersion(string! source, GitVersion.SemanticVersion! semanticVersion, GitVersion.Git.ICommit? baseVersionSource = null, GitVersion.VersionCalculation.VersionIncrementSourceType sourceType = GitVersion.VersionCalculation.VersionIncrementSourceType.Tree) -> void
GitVersion.VersionCalculation.BaseVersion.SourceType.get -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.BaseVersion.SourceType.init -> void
GitVersion.VersionCalculation.BaseVersionOperand.BaseVersionOperand(string! Source, GitVersion.SemanticVersion! SemanticVersion, GitVersion.Git.ICommit? BaseVersionSource = null, GitVersion.VersionCalculation.VersionIncrementSourceType SourceType = GitVersion.VersionCalculation.VersionIncrementSourceType.Tree) -> void
GitVersion.VersionCalculation.BaseVersionOperand.SourceType.get -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.BaseVersionOperand.SourceType.init -> void
GitVersion.VersionCalculation.BaseVersionOperator.SourceType.get -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.IBaseVersionIncrement.SourceType.get -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.VersionIncrementSourceType.Fallback = 2 -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.VersionIncrementSourceType.NextVersionConfig = 3 -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.VersionIncrementSourceType.Tag = 1 -> GitVersion.VersionCalculation.VersionIncrementSourceType
GitVersion.VersionCalculation.VersionIncrementSourceType.Tree = 0 -> GitVersion.VersionCalculation.VersionIncrementSourceType
static GitVersion.Extensions.FileSystemExtensions.GetLastDirectoryWrite(this System.IO.Abstractions.IFileSystem! fileSystem, string! path) -> long
17 changes: 10 additions & 7 deletions src/GitVersion.Core/VersionCalculation/IncrementStrategyFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

namespace GitVersion.VersionCalculation;

internal class IncrementStrategyFinder(IRepositoryStore repositoryStore, ITaggedSemanticVersionRepository taggedSemanticVersionRepository)
internal class IncrementStrategyFinder(
IRepositoryStore repositoryStore,
Lazy<GitVersionContext> versionContext,
IGitRepository repository,
ITaggedSemanticVersionRepository taggedSemanticVersionRepository)
: IIncrementStrategyFinder
{
private readonly Dictionary<string, VersionField?> commitIncrementCache = [];
Expand Down Expand Up @@ -51,15 +55,14 @@ public VersionField DetermineIncrementedField(
var minorRegex = TryGetRegexOrDefault(configuration.MinorVersionBumpMessage, RegexPatterns.VersionCalculation.DefaultMinorRegex);
var patchRegex = TryGetRegexOrDefault(configuration.PatchVersionBumpMessage, RegexPatterns.VersionCalculation.DefaultPatchRegex);
var noBumpRegex = TryGetRegexOrDefault(configuration.NoBumpMessage, RegexPatterns.VersionCalculation.DefaultNoBumpRegex);
var pathFilters = configuration.Ignore.ToFilters(repository, versionContext.Value).OfType<PathFilter>();
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not the business of the IncrementStrategyFinder to filter the commits by the specified ignore settings. At this point only commits who are part of the calculation should be passed into the method. That means the exclusion of the commits have happend much earlier.

Copy link
Author

Choose a reason for hiding this comment

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

I see.
Let me just clarify my concern, that if filtering based on paths is done eagerly, this will cause performance issues, especially for large commit history, due to the costly patch calculation (from what I saw on my machine, every patch calc takes as long as ~100ms).
I believe that current implementation defers the filtering well till it is required.
This issue applies generally whenever filtering needs to be done.

With that in mind, my suggestion is not to filter earlier, but rather later.
Would that make sense?

Copy link
Contributor

@HHobeck HHobeck Mar 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I see. Let me just clarify my concern, that if filtering based on paths is done eagerly, this will cause performance issues, especially for large commit history, due to the costly patch calculation (from what I saw on my machine, every patch calc takes as long as ~100ms). I believe that current implementation defers the filtering well till it is required. This issue applies generally whenever filtering needs to be done.

Regardless of the point if it is evaluated earlier in the execution or not: Isn't it necessary in almost every case to decide whether a commit (beginning from the last tagged commit) should be ignored or not? For instance the variable CommitsSinceTag and CommitsSinceVersionSource can be calculated only if you evaluate it for every commit.

GitVersion/src/GitVersion.Core/VersionCalculation/VersionCalculators/VersionCalculatorBase.cs

Lines 17 to 26 in dd9434b

protected SemanticVersionBuildMetaData CreateVersionBuildMetaData(ICommit? baseVersionSource)
{
var commitLogs = this.repositoryStore.GetCommitLog(
baseVersionSource: baseVersionSource,
currentCommit: Context.CurrentCommit,
ignore: Context.Configuration.Ignore
);
int commitsSinceTag = commitLogs.Count;
this.log.Info($"{commitsSinceTag} commits found between {baseVersionSource} and {Context.CurrentCommit}");

Of course we need to ensure that for each commit the evaluation happens only one time and onyl if it is really necessary. Would it be an idea to extend the ICommit interface by providing a property with the following structure?

bool NeedsToBeIgnored { get; }

This property can be implemented with delayed loading and the result can be cached as well. At the moment we have some problems with caching (please keep this in mind):

Copy link
Contributor

@HHobeck HHobeck Mar 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have tought about it and it is indeed very bad from the performance perspective, if in a potential scenario the CommitsSinceTag and CommitsSinceVersionSource variable will be calculated but the user don't use it.

This applies for ManualDeployment and ContinuousDeployment mode if you are only interested in e.g. the SemVer variable.

GitVersion/src/GitVersion.Core/VersionCalculation/VersionCalculators/ManualDeploymentVersionCalculator.cs

Lines 23 to 26 in dd9434b

return new SemanticVersion(semanticVersion)
{
BuildMetaData = buildMetaData
};

and

GitVersion/src/GitVersion.Core/VersionCalculation/VersionCalculators/ContinuousDeploymentVersionCalculator.cs

Lines 23 to 31 in dd9434b

return new SemanticVersion(semanticVersion)
{
PreReleaseTag = SemanticVersionPreReleaseTag.Empty,
BuildMetaData = new SemanticVersionBuildMetaData(buildMetaData)
{
CommitsSinceVersionSource = buildMetaData.CommitsSinceTag!.Value,
CommitsSinceTag = null
}
};

Edit: I think the performance problem we have in this scenario needs to be addressed maybe later by changing the business logic (out of scope of this issue). For instance we could introduce a new configuration to skip the calculation of the CommitsSinceTag and CommitsSinceVersionSource variable.


var increments = commits
.Select(c => GetIncrementFromCommit(c, majorRegex, minorRegex, patchRegex, noBumpRegex))
.Where(v => v != null)
.ToList();
.Select(c => (commit: c, increment: GetIncrementFromCommit(c, majorRegex, minorRegex, patchRegex, noBumpRegex)))
.Where(pair => pair.increment != null)
.OrderByDescending(pair => pair.increment);

return increments.Count != 0
? increments.Max()
: null;
return increments.FirstOrDefault(pair => !pathFilters.Any(f => f.Exclude(pair.commit, out _)), (null!, null)).increment;
}

private VersionField? FindCommitMessageIncrement(
Expand Down
51 changes: 51 additions & 0 deletions src/GitVersion.Core/VersionCalculation/PathFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System.Text.RegularExpressions;
using GitVersion.Git;

namespace GitVersion.VersionCalculation;

internal class PathFilter(IGitRepository repository, GitVersionContext context, IEnumerable<string> paths) : IVersionFilter
{
private readonly GitVersionContext context = context;
private readonly List<Regex> pathsRegexes = paths.Select(path => new Regex(path, RegexOptions.IgnoreCase | RegexOptions.Compiled)).ToList();
private readonly Dictionary<string, bool> pathMatchCache = [];

public bool Exclude(IBaseVersion baseVersion, out string? reason)
{
ArgumentNullException.ThrowIfNull(baseVersion);

reason = null;
if (baseVersion.SourceType != VersionIncrementSourceType.Tree) return false;

return Exclude(baseVersion.BaseVersionSource, out reason);
}

public bool Exclude(ICommit? commit, out string? reason)
{
reason = null;

if (commit != null)
{
var patchPaths = repository.FindPatchPaths(commit, this.context.Configuration.TagPrefixPattern);

if (patchPaths != null)
{
foreach (var path in patchPaths)
{
if (!pathMatchCache.TryGetValue(path, out var isMatch))
{
isMatch = this.pathsRegexes.Any(regex => regex.IsMatch(path));
pathMatchCache[path] = isMatch;
}

if (isMatch)
{
reason = "Source was ignored due to commit path matching ignore regex";
return true;
}
}
}
}

return false;
}
}
18 changes: 12 additions & 6 deletions src/GitVersion.Core/VersionCalculation/VersionCalculators/NextVersionCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace GitVersion.VersionCalculation;
internal class NextVersionCalculator(
ILog log,
Lazy<GitVersionContext> versionContext,
IGitRepository repository,
IEnumerable<IDeploymentModeCalculator> deploymentModeCalculators,
IEnumerable<IVersionStrategy> versionStrategies,
IEffectiveBranchConfigurationFinder effectiveBranchConfigurationFinder,
Expand Down Expand Up @@ -155,10 +156,10 @@ private SemanticVersion CalculateSemanticVersion(

private NextVersion CalculateNextVersion(IBranch branch, IGitVersionConfiguration configuration)
{
var nextVersions = GetNextVersions(branch, configuration);
var nextVersions = GetNextVersions(branch, configuration).OrderByDescending(v => v.IncrementedVersion);
Copy link
Contributor

@HHobeck HHobeck Mar 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry if I need to tell you this: Also this code change needs to be reverted. The filtering has been done already in:

private bool IncludeVersion(IBaseVersion baseVersion, IIgnoreConfiguration ignoreConfiguration)

It makes aboslute no sense to check it again.

Just a side note: Generally I think the ignore implementation in this class is only a safe guard implementation... Normally I would expect that only base versions from the versionStrategy.GetBaseVersions(..) method are returned who are part of the execution (the ignore business logic has been applied and not filtered).

Copy link
Author

Choose a reason for hiding this comment

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

The idea here is similar to the one before - defer the filtering of commits. Without these changes, the filtering would have happened on every single commit which is considered as a base version for the calculation.
On the other hand, since it handles the base versions only, maybe many of commits won't be flowing through this filtering, therefore performance is less of an issue here IDK.

Regarding your comment about it being a safe guard - my understanding is that this part is responsible for filtering the base versions, which are only partially filtered by the commit SHA alone in versionStrategy.GetBaseVersions implementations.

Let me know what you think we should do here, and I'll adjust the logic accordingly.

Copy link
Contributor

@HHobeck HHobeck Mar 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It makes the code absolutely unmaintainable and error prone if we have a ignore configuration with different filtering methods, which are executed on different code level. I see your motivation... But we need to find a way where both aspects (performance/delayed loading/caching and BL where it is expected and intenionally) are considered. Does this make sense?

log.Separator();

var maxVersion = nextVersions.Max()
var maxVersion = nextVersions.FirstOrDefault(v => ShouldIncludeVersion(configuration, v))
?? throw new GitVersionException("No base versions determined on the current branch.");

ICommit? latestBaseVersionSource;
Expand All @@ -167,6 +168,7 @@ private NextVersion CalculateNextVersion(IBranch branch, IGitVersionConfiguratio
.Where(
element => element.BaseVersion.BaseVersionSource != null
&& element.IncrementedVersion == maxVersion.IncrementedVersion
&& ShouldIncludeVersion(configuration, element)
).ToArray();
if (matchingVersionsOnceIncremented.Length > 1)
{
Expand All @@ -192,11 +194,11 @@ private NextVersion CalculateNextVersion(IBranch branch, IGitVersionConfiguratio
.Where(v => v.BaseVersion.BaseVersionSource != null)
.OrderByDescending(v => v.IncrementedVersion)
.ThenByDescending(v => v.BaseVersion.BaseVersionSource?.When)
.FirstOrDefault();
.FirstOrDefault(v => ShouldIncludeVersion(configuration, v));

version ??= versions.Where(v => v.BaseVersion.BaseVersionSource == null)
.OrderByDescending(v => v.IncrementedVersion)
.First();
.First(v => ShouldIncludeVersion(configuration, v));
latestBaseVersionSource = version.BaseVersion.BaseVersionSource;
}

Expand All @@ -216,6 +218,9 @@ private NextVersion CalculateNextVersion(IBranch branch, IGitVersionConfiguratio
return new(maxVersion.IncrementedVersion, calculatedBase, maxVersion.BranchConfiguration);
}

private bool ShouldIncludeVersion(IGitVersionConfiguration configuration, NextVersion version) =>
configuration.Ignore.ToFilters(repository, this.Context).All(filter => !filter.Exclude(version.BaseVersion, out _));

private static NextVersion CompareVersions(NextVersion version1, NextVersion version2)
{
if (version1.BaseVersion.BaseVersionSource == null)
Expand Down Expand Up @@ -264,7 +269,8 @@ IEnumerable<NextVersion> GetNextVersionsInternal()
foreach (var baseVersion in versionStrategy.GetBaseVersions(effectiveBranchConfiguration))
{
log.Info(baseVersion.ToString());
if (IncludeVersion(baseVersion, configuration.Ignore))
if (versionStrategy is not FallbackVersionStrategy || IncludeVersion(baseVersion, configuration.Ignore))
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert the logic to exclude the FallbackVersionStrategy.The FallackVersionStrategy class is like every other version strategy class and should not handled explicit.

Copy link
Author

Choose a reason for hiding this comment

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

The reason for this, is the deferral of filtering to all other strategies other than FallbackVersionStrategy, simply because I wasn't sure how to handle the case where atLeastOneBaseVersionReturned=false. Ideally, if deferring the filtering, this condition won't be here and the IncludeVersion will not be invoked here.
I'll change it after resolving previous comments.

Copy link
Contributor

@HHobeck HHobeck Mar 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You see, this is not a good design (and makes it hard to understand) if you apply a implicit knowledge about the fact that the base version fo FallbackVersionStrategy is already evaluated. You are skipping it because of performance reason.. We need to find a design on an abstraction level e.g. ICommit interface where we ensure this aspect. We need to have transparent calls in the business logic without implicit assumptions.

Edit: Anyway, an idea would be to remove the filtering in NextVersionCalculator generally and ensure that the IVersionStrategy implementation classes only returning evaluated base versions.

//Defer the version inclusion check to the caller, for strategies other than FallbackVersionStrategy
{
atLeastOneBaseVersionReturned = true;

Expand All @@ -284,7 +290,7 @@ IEnumerable<NextVersion> GetNextVersionsInternal()

private bool IncludeVersion(IBaseVersion baseVersion, IIgnoreConfiguration ignoreConfiguration)
{
foreach (var versionFilter in ignoreConfiguration.ToFilters())
foreach (var versionFilter in ignoreConfiguration.ToFilters(repository, Context))
{
if (versionFilter.Exclude(baseVersion, out var reason))
{
Expand Down
Loading