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

[Bug] Tag: useBranchName isn't working in some cases #2201

Closed
zhuravelsv opened this issue Mar 23, 2020 · 13 comments · Fixed by #2649
Closed

[Bug] Tag: useBranchName isn't working in some cases #2201

zhuravelsv opened this issue Mar 23, 2020 · 13 comments · Fixed by #2649
Labels
Milestone

Comments

@zhuravelsv
Copy link

zhuravelsv commented Mar 23, 2020

Describe the bug
I faced issue with property "tag: useBranchName", I'm trying to use it, but it's not working in some cases, for example even if I use tag "tag.{BranchName}" I got "FullSemVer":"3.0.0-tag..1+8".

I'm not sure is it bug or not, but all details below

Configuration:

assembly-versioning-scheme: MajorMinorPatchTag
assembly-file-versioning-scheme: MajorMinorPatchTag
assembly-informational-format: '{InformationalVersion}'
tag-prefix: '[vV]'
legacy-semver-padding: 4
build-metadata-padding: 4
commits-since-version-source-padding: 4
commit-message-incrementing: Disabled
continuous-delivery-fallback-tag: prerelease
branches:
  master:
    regex: ^master
    mode: ContinuousDelivery
    tag: useBranchName
    increment: None
    prevent-increment-of-merged-branch-version: true
    track-merge-target: false
    tracks-release-branches: false
    is-release-branch: false
  feature:
    regex: .*
    mode: ContinuousDelivery
    tag: useBranchName
    increment: None
    prevent-increment-of-merged-branch-version: false
    track-merge-target: false
    tracks-release-branches: false
    is-release-branch: false

Expected Behavior

master - tag is "master"
custom/foo - tag is "foo"
foo - tag is "foo"
feature/name - tag is "name"
feature - tag is "feature"

Actual Behavior

Tag: useBranchName isn't working for master branch and some other branches:
master, custom/foo, foo, feature/name, feature - no tag
(
useBranchName: "FullSemVer":"3.0.0+16",
tag.{BranchName}: "FullSemVer":"3.0.0+16",
tag.{BranchName}: "FullSemVer":"3.0.0-tag..1+16" (if we set tag: tag.{BranchName})
)

But, if we change regex in feature branch config to this "^features?[/-]" it will work because of this:
http://regexstorm.net/tester?p=%5efeatures%3f%5b%2f-%5d&i=features%2ffoo
regex cut all matched part, and remnants are used for tag, but if we configure regex to match all branches (like in case above) it will not work correctly

Context

I would like to add branch name tag to any branch (except master)

Possible Fix

BaseVersionCalculator.GetBaseVersion return incorrect version

Steps to Reproduce

Just try to use "useBranchName" or "{BranchName}" in master or configure to match all branch names

Your Environment

Azure devops (pipelines, nuget/artifacts), nect core 3.1

  • Version Used: 5.2.4
  • Operating System and version (Windows 10, Ubuntu 18.04): Ubuntu 18.04
@zhuravelsv zhuravelsv added the bug label Mar 23, 2020
@asbjornu
Copy link
Member

Interesting. Would it be possible for you to submit a PR that reproduces this problem in a RepositoryFixture test?

@manicprogrammer
Copy link

I just ran across this as well. I haven't looked at the code yet but I have a suspicion of what is happening.
It seems to occur on any branch that is not namespaced, So develop, master, my-branch but not on any branch like feature/my-feature

The BranchName variable outputs correctly but I suspect how the variable is being parsed when used in a tag is wrong. I suspect it is looking for the last / and using the value after that to put into the tag - but if your branch does not have a namespace it parses the branchname for the purpose of the tag as an empty string.

I'll try to look at the code this weekend and see if my suspicions are correct and if so put in a test and a fix.

@TechWatching
Copy link

Any news on this topic ?

@dejoost
Copy link

dejoost commented Nov 17, 2020

I also ran into this issue today with v5.5.1. As soon as the regex matches the full branch name the BranchName variable returns empty. In my case branch is called "cicd". As proof of concept I tried "regex: ^random/gitversion" and as expected the BranchName returned again as an empty string in the SemVer

@asbjornu
Copy link
Member

Please submit a failing RepositoryFixture test in a PR so it's possible to see exactly what is failing.

@bddckr
Copy link
Contributor

bddckr commented Dec 30, 2020

Looks like the suspicion raised above is correct:

The branch name is stripped of BranchPrefixToTrim:

if (!string.IsNullOrWhiteSpace(configuration.BranchPrefixToTrim))
{
branchName = branchName.RegexReplace(configuration.BranchPrefixToTrim, string.Empty, RegexOptions.IgnoreCase);
}

That property is set to the Regex specified for a branch (via branches in the YAML):

The examples and defaults work because they do not match anything after / or - in the regex (e.g. ^features?[/-]).


Fixing this looks like a breaking change to me... To not have it break existing usages I had to check for whether the branch name contains / or -. That doesn't seem to align with the interest of fixing this - it should be left up to the user to define what gets captured for replacement IMO. Perhaps the best way moving forward is to add BranchPrefixToTrim to the BranchConfig and set it to BranchConfig.Regex by default to keep backwards compatibility?

Here's what I stopped at:

diff --git a/src/GitVersionCore.Tests/IntegrationTests/FeatureBranchScenarios.cs b/src/GitVersionCore.Tests/IntegrationTests/FeatureBranchScenarios.cs
index b9129d88..69a8c4f1 100644
--- a/src/GitVersionCore.Tests/IntegrationTests/FeatureBranchScenarios.cs
+++ b/src/GitVersionCore.Tests/IntegrationTests/FeatureBranchScenarios.cs
@@ -220,6 +220,38 @@ public void ShouldUseConfiguredTag(string tag, string featureName, string preRel
             fixture.AssertFullSemver(expectedFullSemVer, config);
         }
 
+        [TestCase("alpha", "JIRA-123", "alpha")]
+        [TestCase("useBranchName", "JIRA-123", "JIRA-123")]
+        [TestCase("alpha.{BranchName}", "JIRA-123", "alpha.JIRA-123")]
+        public void ConfiguredTagIsBranchNameForBranchesWithoutPrefix(string tag, string branchName, string preReleaseTagName)
+        {
+            var config = new Config
+            {
+                Branches =
+                {
+                    {
+                        "other",
+                        new BranchConfig
+                        {
+                            Increment = IncrementStrategy.Patch,
+                            Regex = ".*",
+                            SourceBranches = new HashSet<string>(),
+                            Tag = tag
+                        }
+                    }
+                }
+            };
+
+            using var fixture = new EmptyRepositoryFixture();
+            fixture.Repository.MakeATaggedCommit("1.0.0");
+            fixture.Repository.CreateBranch(branchName);
+            Commands.Checkout(fixture.Repository, branchName);
+            fixture.Repository.MakeCommits(5);
+
+            var expectedFullSemVer = $"1.0.1-{preReleaseTagName}.1+5";
+            fixture.AssertFullSemver(expectedFullSemVer, config);
+        }
+
         [Test]
         public void BranchCreatedAfterFinishReleaseShouldInheritAndIncrementFromLastMasterCommitTag()
         {
diff --git a/src/GitVersionCore/Configuration/ConfigExtensions.cs b/src/GitVersionCore/Configuration/ConfigExtensions.cs
index 987795e2..e9e39ef2 100644
--- a/src/GitVersionCore/Configuration/ConfigExtensions.cs
+++ b/src/GitVersionCore/Configuration/ConfigExtensions.cs
@@ -129,7 +129,7 @@ public static string GetBranchSpecificTag(this EffectiveConfiguration configurat
                 log.Info("Using branch name to calculate version tag");
 
                 var branchName = branchNameOverride ?? branchFriendlyName;
-                if (!string.IsNullOrWhiteSpace(configuration.BranchPrefixToTrim))
+                if (!string.IsNullOrWhiteSpace(configuration.BranchPrefixToTrim) && (branchName.Contains('/') || branchName.Contains('-')))
                 {
                     branchName = branchName.RegexReplace(configuration.BranchPrefixToTrim, string.Empty, RegexOptions.IgnoreCase);
                 }

@asbjornu
Copy link
Member

asbjornu commented Jan 4, 2021

Wow, what a mess. A PR that fixes this properly would be appreciated for v6. What we should do until then, I'm not sure. Perhaps adding another configuration property is the way to go, but I'd like to avoid it. If we add the check for - or / in the branch name, that would remove some flexibility but would it be a workable compromise until the breaking change and proper fix is introduced in v6?

@bddckr
Copy link
Contributor

bddckr commented Jan 4, 2021

If we add the check for - or / in the branch name, that would remove some flexibility but would it be a workable compromise until the breaking change and proper fix is introduced in v6?

I'm personally using some branch names that use - but do not use /, so I'm afraid that won't work for me.

I do like the idea of making this less weird to resolve as I've done in the linked PR, though - a breaking change in v6 sounds like a good idea to me. What's your idea of how this should work moving forward? Should we offer an additional config property but just remove the relationship to the existing property Regex perhaps so it's all explicit?

@dejoost
Copy link

dejoost commented Jan 5, 2021

as workaround: just add an extra check, if the "trimmed" branchname returns empty, use the untrimmed version (and maybe a boolean to disable the trimming altogether).

for a (breaking) permanent: fix i would make it explicit, add the branchPrefixToTrim to the config and keep it empty by default. This would also allow ppl to apply more exotic transforms.

@carlwoodhouse
Copy link
Contributor

just ran into this :( any progress ?

@asbjornu
Copy link
Member

asbjornu commented Apr 8, 2021

as workaround: just add an extra check, if the "trimmed" branchname returns empty, use the untrimmed version (and maybe a boolean to disable the trimming altogether).

I think this is a nice compromise. A pull request implementing this would be accepted and appreciated. 🙏🏼

@carlwoodhouse
Copy link
Contributor

as workaround: just add an extra check, if the "trimmed" branchname returns empty, use the untrimmed version (and maybe a boolean to disable the trimming altogether).

I think this is a nice compromise. A pull request implementing this would be accepted and appreciated. 🙏🏼

Have opened a PR that implements this suggested compromise i hope :)

@asbjornu asbjornu added this to the 5.6.9 milestone Apr 9, 2021
asbjornu added a commit that referenced this issue Apr 9, 2021
use tag branch name even when the branch has no namespace fixes #2201
@github-actions
Copy link

🎉 This issue has been resolved in version 5.6.9 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants