-
Notifications
You must be signed in to change notification settings - Fork 663
feat: add support for environment variables in branch labels #4944
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
Changes from all commits
014ae33
91e1cb2
ebe16d0
7fd0dc7
b5f47b2
1f26fa0
b762c23
8d1e896
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| using GitVersion.Core; | ||
| using GitVersion.Extensions; | ||
| using GitVersion.Formatting; | ||
| using GitVersion.Git; | ||
| using GitVersion.VersionCalculation; | ||
|
|
||
|
|
@@ -97,38 +98,25 @@ private static bool ShouldBeIgnored(ICommit commit, IIgnoreConfiguration ignore) | |
|
|
||
| extension(EffectiveConfiguration configuration) | ||
| { | ||
| public string? GetBranchSpecificLabel(ReferenceName branchName, string? branchNameOverride) | ||
| => GetBranchSpecificLabel(configuration, branchName.WithoutOrigin, branchNameOverride); | ||
| public string? GetBranchSpecificLabel(ReferenceName branchName, string? branchNameOverride, IEnvironment environment) | ||
| => GetBranchSpecificLabel(configuration, branchName.WithoutOrigin, branchNameOverride, environment); | ||
|
|
||
| public string? GetBranchSpecificLabel(string? branchName, string? branchNameOverride) | ||
| public string? GetBranchSpecificLabel(string? branchName, string? branchNameOverride, IEnvironment environment) | ||
| { | ||
| configuration.NotNull(); | ||
| environment.NotNull(); | ||
|
|
||
| var label = configuration.Label; | ||
|
|
||
| if (label is null) | ||
| { | ||
| return label; | ||
| } | ||
|
|
||
| var effectiveBranchName = branchNameOverride ?? branchName; | ||
| if (configuration.RegularExpression.IsNullOrWhiteSpace() || effectiveBranchName.IsNullOrEmpty()) return label; | ||
| var regex = RegexPatterns.Cache.GetOrAdd(configuration.RegularExpression); | ||
| var match = regex.Match(effectiveBranchName); | ||
| if (!match.Success) return label; | ||
| foreach (var groupName in regex.GetGroupNames()) | ||
| { | ||
| var groupValue = match.Groups[groupName].Value; | ||
| Lazy<string> escapedGroupValueLazy = new(() => groupValue.RegexReplace(RegexPatterns.SanitizeNameRegexPattern, "-")); | ||
| var placeholder = $"{{{groupName}}}"; | ||
| int index, startIndex = 0; | ||
| while ((index = label.IndexOf(placeholder, startIndex, StringComparison.InvariantCulture)) >= 0) | ||
| { | ||
| var escapedGroupValue = escapedGroupValueLazy.Value; | ||
| label = label.Remove(index, placeholder.Length).Insert(index, escapedGroupValue); | ||
| startIndex = index + escapedGroupValue.Length; | ||
| } | ||
| } | ||
| return label; | ||
| var labelPlaceholders = BuildLabelPlaceholders(configuration.RegularExpression, effectiveBranchName); | ||
|
|
||
| return label.FormatWith(labelPlaceholders, environment); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Throws if environment variable is not found and has no fallback. This is deliberate to stay consistent with other usages of environment variables, for example in assembly file version. |
||
| } | ||
|
|
||
| public TaggedSemanticVersions GetTaggedSemanticVersion() | ||
|
|
@@ -153,5 +141,27 @@ public TaggedSemanticVersions GetTaggedSemanticVersion() | |
| } | ||
| return taggedSemanticVersion; | ||
| } | ||
|
|
||
| private static Dictionary<string, object> BuildLabelPlaceholders(string? regularExpression, string? effectiveBranchName) | ||
| { | ||
| var placeholders = new Dictionary<string, object>(); | ||
|
|
||
| if (regularExpression.IsNullOrWhiteSpace() || effectiveBranchName.IsNullOrEmpty()) | ||
| return placeholders; | ||
|
|
||
| var regex = RegexPatterns.Cache.GetOrAdd(regularExpression); | ||
| var match = regex.Match(effectiveBranchName); | ||
|
|
||
| if (!match.Success) | ||
| return placeholders; | ||
|
|
||
| foreach (var groupName in regex.GetGroupNames()) | ||
| { | ||
| var groupValue = match.Groups[groupName].Value; | ||
| placeholders[groupName] = groupValue.RegexReplace(RegexPatterns.SanitizeNameRegexPattern, "-"); | ||
| } | ||
|
|
||
| return placeholders; | ||
| } | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here are a few suggestions regarding this implementation:
internal static class StringFormatWithExtension
{
extension(string template)
{
public string FormatWith(object? source, IEnvironment environment)
{
object? GetFromSource(string value)
{
// ...
}
object? GetFromEnvironment(string value) => environment.GetEnvironmentVariable(value);
return template.FormatWith(GetFromSource, GetFromEnvironment);
}
public string FormatWith(IDictionary<string, object> source, IEnvironment environment)
{
object? GetFromSource(string value)
{
source.TryGetValue(value, out var result);
return result;
}
object? GetFromEnvironment(string value) => environment.GetEnvironmentVariable(value);
return template.FormatWith(GetFromSource, GetFromEnvironment);
}
private string FormatWith(
Func<string, object?> getFromSource, Func<string, object?> getFromEnvironment)
{
// ...
}
}
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1 and 2, fair enough, will make these changes. For 3, it was written this way (and I'm guessing the original implementor in #4746 did it this way for the same reason) to remain compatible/easy to understand as the existing configuration options for If the longer-term intention is to further enhance the regex/env var to support this conflation, I'd suggest doing this in a follow up change rather than here. Let me know your thoughts on this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any thoughts on this guys?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand the following statement:
From the documentation:
So for assembly-file-versioning-format: '{env:JustAVariable} ?? {JustAProperty} ?? "JustAFallback"`'Should be possible as follows: assembly-file-versioning-format: '{env:JustAVariable ?? JustAProperty ?? "JustAFallback"}'…the difference being the placement of the braces, which IIRC needs to embrace the entire expression. Whether one, two or multiple fallback values are possible already is not something I have tested, but I think it's something that should be supported.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree. The would be the correction:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough, if that's the consensus I'll have a go at implementing it in this PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome! But feel free to create a separate PR if you'd like. |
Uh oh!
There was an error while loading. Please reload this page.