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

Replace special chars in InformationalVersion with dashes #2014

Merged
merged 9 commits into from Jan 6, 2020
@@ -0,0 +1,32 @@
{
"Major":1,
"Minor":2,
"Patch":3,
"PreReleaseTag":"",
"PreReleaseTagWithDash":"",
"PreReleaseLabel":"",
"PreReleaseNumber":"",
"WeightedPreReleaseNumber":"",
"BuildMetaData":5,
"BuildMetaDataPadded":"0005",
"FullBuildMetaData":"5.Branch.feature-123.Sha.commitSha",
"MajorMinorPatch":"1.2.3",
"SemVer":"1.2.3",
"LegacySemVer":"1.2.3",
"LegacySemVerPadded":"1.2.3",
"AssemblySemVer":"1.2.3.0",
"AssemblySemFileVer":"1.2.3.0",
"FullSemVer":"1.2.3+5",
"InformationalVersion":"1.2.3+5.Branch.feature-123.Sha.commitSha",
"BranchName":"feature/123",
"Sha":"commitSha",
"ShortSha":"commitShortSha",
"NuGetVersionV2":"1.2.3",
"NuGetVersion":"1.2.3",
"NuGetPreReleaseTagV2":"",
"NuGetPreReleaseTag":"",
"VersionSourceSha":"versionSourceSha",
"CommitsSinceVersionSource":5,
"CommitsSinceVersionSourcePadded":"0005",
"CommitDate":"2014-03-06"
}
@@ -0,0 +1,32 @@
{
"Major":1,
"Minor":2,
"Patch":3,
"PreReleaseTag":"",
"PreReleaseTagWithDash":"",
"PreReleaseLabel":"",
"PreReleaseNumber":"",
"WeightedPreReleaseNumber":"",
"BuildMetaData":5,
"BuildMetaDataPadded":"0005",
"FullBuildMetaData":"5.Branch.feature-123.Sha.commitSha",
"MajorMinorPatch":"1.2.3",
"SemVer":"1.2.3",
"LegacySemVer":"1.2.3",
"LegacySemVerPadded":"1.2.3",
"AssemblySemVer":"1.2.3.0",
"AssemblySemFileVer":"1.2.3.0",
"FullSemVer":"1.2.3+5",
"InformationalVersion":"1.2.3+5.Branch.feature-123.Sha.commitShortSha",
"BranchName":"feature/123",
"Sha":"commitSha",
"ShortSha":"commitShortSha",
"NuGetVersionV2":"1.2.3",
"NuGetVersion":"1.2.3",
"NuGetPreReleaseTagV2":"",
"NuGetPreReleaseTag":"",
"VersionSourceSha":"versionSourceSha",
"CommitsSinceVersionSource":5,
"CommitsSinceVersionSourcePadded":"0005",
"CommitDate":"2014-03-06"
}
54 changes: 54 additions & 0 deletions src/GitVersionCore.Tests/VariableProviderTests.cs
Expand Up @@ -229,5 +229,59 @@ public void ProvidesVariablesInContinuousDeploymentModeWithTagSetToUseBranchName

vars.FullSemVer.ShouldBe("1.2.3-feature.5");
}

[Test]
[Category("NoMono")]
[Description("Won't run on Mono due to source information not being available for ShouldMatchApproved.")]
public void ProvidesVariablesInContinuousDeliveryModeForFeatureBranch()
{
var semVer = new SemanticVersion
{
Major = 1,
Minor = 2,
Patch = 3,
BuildMetaData = "5.Branch.feature/123"
};

semVer.BuildMetaData.Branch = "feature/123";
semVer.BuildMetaData.VersionSourceSha = "versionSourceSha";
semVer.BuildMetaData.Sha = "commitSha";
semVer.BuildMetaData.ShortSha = "commitShortSha";
semVer.BuildMetaData.CommitDate = DateTimeOffset.Parse("2014-03-06 23:59:59Z");


var config = new TestEffectiveConfiguration();

var vars = variableProvider.GetVariablesFor(semVer, config, false);

JsonOutputFormatter.ToJson(vars).ShouldMatchApproved(c => c.SubFolder("Approved"));
}

[Test]
[Category("NoMono")]
[Description("Won't run on Mono due to source information not being available for ShouldMatchApproved.")]
public void ProvidesVariablesInContinuousDeliveryModeForFeatureBranchWithCustomAssemblyInformationalFormat()
{
var semVer = new SemanticVersion
{
Major = 1,
Minor = 2,
Patch = 3,
BuildMetaData = "5.Branch.feature/123"
};

semVer.BuildMetaData.Branch = "feature/123";
semVer.BuildMetaData.VersionSourceSha = "versionSourceSha";
semVer.BuildMetaData.Sha = "commitSha";
semVer.BuildMetaData.ShortSha = "commitShortSha";
semVer.BuildMetaData.CommitDate = DateTimeOffset.Parse("2014-03-06 23:59:59Z");


var config = new TestEffectiveConfiguration(assemblyInformationalFormat: "{Major}.{Minor}.{Patch}+{CommitsSinceVersionSource}.Branch.{BranchName}.Sha.{ShortSha}");

var vars = variableProvider.GetVariablesFor(semVer, config, false);

JsonOutputFormatter.ToJson(vars).ShouldMatchApproved(c => c.SubFolder("Approved"));
}
}
}
3 changes: 2 additions & 1 deletion src/GitVersionCore/OutputVariables/VariableProvider.cs
@@ -1,6 +1,7 @@
using System;
using System.Text.RegularExpressions;
using GitVersion.Exceptions;
using GitVersion.Extensions;
using GitVersion.VersionCalculation;
using GitVersion.VersioningModes;
using GitVersion.Configuration;
Expand Down Expand Up @@ -56,7 +57,7 @@ public VersionVariables GetVariablesFor(SemanticVersion semanticVersion, Effecti
var semverFormatValues = new SemanticVersionFormatValues(semanticVersion, config);

var informationalVersion = CheckAndFormatString(config.AssemblyInformationalFormat, semverFormatValues,
environment, semverFormatValues.DefaultInformationalVersion, "AssemblyInformationalVersion");
environment, semverFormatValues.DefaultInformationalVersion, "AssemblyInformationalVersion").RegexReplace("[^0-9A-Za-z-.+]", "-");
Copy link
Member

Choose a reason for hiding this comment

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

Is it not possible for CheckAndFormatString to perform the RegexReplace? The problem can in theory occur for assemblyFileSemVer and assemblySemVer as well if their formats or schemes include the prerelease label (tag)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm.
I see someone removed the fields assembly-versioning-format and assembly-file-versioning-format from the documentation but not from the code.

I would say it not needed to move the regex because for assembly version and assembly file version the rules are way more restricting.
Basicly only the schemes described in

MajorMinorPatchTag,
MajorMinorPatch,
MajorMinor,
Major,
None

and
MajorMinorPatchTag,
MajorMinorPatch,
MajorMinor,
Major,
None

are allowed by Microsoft specs.

I guess that is also the reason why the fields are no longer in the documentation for GitVersion 4.x and 5.x.

Also see Microsoft docs

Copy link
Member

Choose a reason for hiding this comment

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

Good point. Would moving the RegexReplace hurt, though? I would prefer it inside the method if it doesn't actually destroy anything.

Copy link
Contributor Author

@Skoucail Skoucail Jan 3, 2020

Choose a reason for hiding this comment

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

Nop would not hurt i guess.
Just a theoretical performance hit. But i don't think that will be noticeable.

Edit: actually the performance would be better as the RegexReplace will only be executed if a format has been given. I committed the change


var assemblyFileSemVer = CheckAndFormatString(config.AssemblyFileVersioningFormat, semverFormatValues,
environment, semverFormatValues.AssemblyFileSemVer, "AssemblyFileVersioningFormat");
Expand Down