Fix parsing of GVFS-aware Git version numbers that contain - (for PR validation builds)#167
Conversation
…the NuGet spec for what defines a package
|
The fact that this same regex shows up in 3 separate scripts could end up being a maintenance issue. Can it be isolated into just one script that returns the appropriate version number? |
Scripts/Mac/BuildGVFSForMac.sh
Outdated
| $SCRIPTDIR/DownloadGVFSGit.sh || exit 1 | ||
| GVFSPROPS=$SRCDIR/GVFS/GVFS.Build/GVFS.props | ||
| GITVERSION="$(cat $GVFSPROPS | grep GitPackageVersion | grep -Eo '[0-9.]{1,}')" | ||
| GITVERSION="$(cat $GVFSPROPS | grep GitPackageVersion | grep -Eo '[0-9.]+(-\w+)?')" |
There was a problem hiding this comment.
I'm not familiar with bash regex syntax, what does +(-\w+)? match against?
There was a problem hiding this comment.
The - here is matching the a literal '-' character, which has to precede any text string in a NuGet version number (according to their spec).
The move from {1,} to + is from me realizing that egrep supports notation like + and ?, it should've been a + for readability to begin with.
There was a problem hiding this comment.
What does the \w match? (also where did you find the spec?)
There was a problem hiding this comment.
The \w matches any word (alphanumeric) character, so we're matching any number of words after the -. So -pr, -rc, -release1 would all be valid values, but something like -release-1 wouldn't be.
The 'spec' of sorts is here: https://docs.microsoft.com/en-us/nuget/reference/package-versioning
|
@sanoursa I agree that it's no good to have this code duplicated in three places. In the past we talked about writing out the version number once to the build directory and having other scripts just read it from that file. Our intention was to make sure that we weren't using a stale version number (imagine the version has changed since your last build, and you're running the Prep script before ever running the Build script that would create the file), but I'm not sure if that scenario is going to ever be common. I can make that change here. |
|
@nickgra isn't it possible to call a script that returns a value? |
|
@sanoursa Yup, that approach works too. I took that approach for commit 2. |
Correct regex used to match GVFS-aware Git package to actually match the NuGet spec for what defines a package. See https://docs.microsoft.com/en-us/nuget/reference/package-versioning
This fixes #175