-
-
Notifications
You must be signed in to change notification settings - Fork 71
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
Adds support for customizing tag formats #72
Conversation
@@ -50,6 +50,14 @@ | |||
in a previous build in a GitInfo.cache for | |||
performance reasons. | |||
Defaults to empty value (no ignoring). | |||
|
|||
$(GitTagRegex): Regular Experssion used with git describe to find the BaseTag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is really needed. We could just run the version regex against the tags in order to filter out the ones to consider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right, but I didn't have an easy way to test, so I kept them separate to make clear what I thought would help.
$(GitTagRegex): Regular Experssion used with git describe to find the BaseTag | ||
Defaults to * (all) | ||
|
||
$(GitBaseVersionExpr): Regular Experssion used with git describe to find the BaseTag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The versions are semver2, I don't think we need to make the regex customizable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're referring to making the GitBaseVersionExpr public instead of private, I do think you should allow user customization. As written, the _GitBaseVersionExpr catches any tag with a semver2 version embedded. Customizing it would allow us to be more specific about the format of the actual tag that we would consider as a "valid" version tag.
@@ -579,7 +587,7 @@ | |||
DependsOnTargets="_GitBranch;_GitCommit" | |||
Condition="'$(GitBaseVersion)' == '' And '$(GitIgnoreTagVersion)' != 'true' "> | |||
|
|||
<Exec Command='$(GitExe) describe --tags --abbrev=0' | |||
<Exec Command='$(GitExe) describe --tags --match=$(GitTagRegex) --abbrev=0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this --match
argument support our current semver2 expression? If so, then we just need to pass it and that's it? If it breaks, do we need a simpler format of the semver2 regex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The answer is, I really don't know for sure (I don't really have a good way to test on my side). I suspect it does. This question was the impetus behind adding a second regex rather than just using the _GitBaseVersionExpr.
Thanks a lot for your contribution! I made a few comments. Looking forward to hearing from you! |
I'll modify the submission to just use GitBaseVersionExpr and resubmit. |
Same problem here: gitlab adds git-references containing additional buildinfo (e.g. 'pipelines') and makes gitinfo take the wrong tag for calculating versioninfo. |
$(GitTagRegex): Regular Experssion used with git describe to find the BaseTag | ||
Defaults to * (all) | ||
|
||
$(GitBaseVersionExpr): Regular Experssion used with git describe to find the BaseTag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in both strings (Experssion
-> Expression
). Also, it's not correct that this regex is used to find BaseTag. It's used to parse the base version too.
It's weird that even if the repo has an |
Fixes #100, brings #72 in. It didn't make a lot of sense to require the entire branch or tag to be semver2, from beginning to end. It's usually the case that branches are named like `rel/vX.Y.Z`, for example, and we were forcing such common naming conventions to customize GitInfo unnecessarily. So to make it more flexible and powerful, we add two things with this commit: 1. We no longer enfore version to match from beginning of string (`^` removed from default regex). We still require the version to be the last part. However, if that still doesn't match the desired behavior, users can now override the default expression by just setting `GitBaseVersionRegex` which is now "public" and documented. 2. We also allow the filter for tags to be specified, so that "weird" tags aren't considered for matching, and can easily be filtered out by providing, for example, a `rel/v*` expression for `GitTagRegex)`.
Fixes #100, brings #72 in. It didn't make a lot of sense to require the entire branch or tag to be semver2, from beginning to end. It's usually the case that branches are named like `rel/vX.Y.Z`, for example, and we were forcing such common naming conventions to customize GitInfo unnecessarily. So to make it more flexible and powerful, we add two things with this commit: 1. We no longer enfore version to match from beginning of string (`^` removed from default regex). We still require the version to be the last part. However, if that still doesn't match the desired behavior, users can now override the default expression by just setting `GitBaseVersionRegex` which is now "public" and documented. 2. We also allow the filter for tags to be specified, so that "weird" tags aren't considered for matching, and can easily be filtered out by providing, for example, a `rel/v*` expression for `GitTagRegex)`.
Superseded by #129 |
Fixes #100, brings #72 in. It didn't make a lot of sense to require the entire branch or tag to be semver2, from beginning to end. It's usually the case that branches are named like `rel/vX.Y.Z`, for example, and we were forcing such common naming conventions to customize GitInfo unnecessarily. So to make it more flexible and powerful, we add two things with this commit: 1. We no longer enfore version to match from beginning of string (`^` removed from default regex). We still require the version to be the last part. However, if that still doesn't match the desired behavior, users can now override the default expression by just setting `GitBaseVersionRegex` which is now "public" and documented. 2. We also allow the filter for tags to be specified, so that "weird" tags aren't considered for matching, and can easily be filtered out by providing, for example, a `rel/v*` expression for `GitTagRegex)`. Bump to 2.1 since it might be breaking for some consumers.
We use tags for our base versions. We ran into an issue where our build system was adding tags that were clobbering the base version. We needed a way to filter the tags by ONLY ones that match the SemVer spec.
We haven't tested this code (this is meant only to give you an example of what we are looking for), however we use similar semantics on the node.js side (using git describe --match=v*).
Thought it might make a good addition to your excellent library.