Simplify grep regexp to be more compatible

[stblassitude]

The new expression also works with the BSD implementation of grep, instead of just GNU grep.

Also, I believe the expression did not work consistently for all possible
version numbers, for example a two digit patch version would not have
been matched.

[silverwind]

It does not work on single-digit minor version:

echo 'go version go2.0 linux/amd64' | grep -Eo '[0-9]+\.?[0-9]+\.?[0-9]+'

[stblassitude]

That is correct. Both Go and Node report three-part version numbers, so it shouldn't be a problem.

Would you prefer: '[0-9]+\.[0-9]+(\.[0-9]+)?'

$ for v in 1.2 1.2.3 11.2 11.22.3 11.2.33 11.22.33 111.222.333; do echo $v | grep -Eo '[0-9]+\.[0-9]+(\.[0-9]+)?'; done
1.2
1.2.3
11.2
11.22.3
11.2.33
11.22.33
111.222.333

Allowing single-part version numbers would also report 64 for linux/amd64.

[silverwind]

Go reports 2-part version and Node 3-part but I'd like it to work on all variations between 1 and 3. These should all work:

echo 'v14' | grep -Eo '[0-9]+\.?[0-9]*\.?[0-9]*'
echo 'v14.1' | grep -Eo '[0-9]+\.?[0-9]*\.?[0-9]*'
echo 'v14.1.2' | grep -Eo '[0-9]+\.?[0-9]*\.?[0-9]*'

echo 'go version go1 linux/amd64' | grep -Eo '[0-9]\.?[0-9]*\.?[0-9]*[[:space:]]'
echo 'go version go1.2 linux/amd64' | grep -Eo '[0-9]\.?[0-9]*\.?[0-9]*[[:space:]]'
echo 'go version go1.2.3 linux/amd64' | grep -Eo '[0-9]\.?[0-9]*\.?[0-9]*[[:space:]]'

BTW, my version of bsd grep (grep (BSD grep) 2.5.1-FreeBSD) does support [[:space:]]. Also see #10508 about it.

[silverwind]

Blocking for now

[stblassitude]

Why is the trailing space relevant? To deselect amd64? I would find it really surprising for go to report a single-part version number in the future.

[silverwind]

Yes it's for that. I don't like this whole grep business, maybe there are better alternatives to get the version directly. For node, one can do:

$ node -p process.versions.node
12.16.1

Don't know about go, thought.

[stblassitude]

for v in 1 1.2 1.2.3 11.2 11.22.3 11.2.33 11.22.33 111.222.333; do echo "go$v linux/amd64" | bsdgrep -Eo '[0-9]+(\.[0-9]+){0,2}[[:space:]]'; done
1 
1.2 
1.2.3 
11.2 
11.22.3 
11.2.33 
11.22.33 
111.222.333 

[silverwind]

Suggestion to not use grep:

$ go version | cut -c14- | cut -d ' ' -f1
1.14
$ node -p process.versions.node
12.16.1

The go version string is static as per this.

Edited: Updated node to just use process

[codecov-io]

Codecov Report

Merging #10765 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #10765      +/-   ##
==========================================
+ Coverage   43.53%   43.56%   +0.02%     
==========================================
  Files         589      589              
  Lines       82658    82658              
==========================================
+ Hits        35983    36006      +23     
+ Misses      42199    42181      -18     
+ Partials     4476     4471       -5

Impacted Files	Coverage Δ
models/notification.go	64.71% <0%> (+0.83%)	⬆️
modules/log/event.go	65.64% <0%> (+1.02%)	⬆️
modules/queue/workerpool.go	58% <0%> (+1.06%)	⬆️
modules/git/command.go	89.56% <0%> (+2.6%)	⬆️
services/pull/check.go	53.04% <0%> (+3.04%)	⬆️
modules/process/manager.go	78.31% <0%> (+3.61%)	⬆️
modules/avatar/avatar.go	54% <0%> (+6%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2809b3...ed0cb12. Read the comment docs.

[silverwind]

Apparently go's release tags can also have suffixes like go1.14beta1. So I think a regex is still needed to match those as well:

go version | grep -Eo '[0-9]+\.[0-9.]+'

This assumes there will never be a single digit version, but I think that is safe to assume.

[assistcontrol]

It starts with go, so I'd suggest: go[0-9].*[[:space:]] | tr -d a-z | tr '.' ' '

[lafriks]

Please resolve conflict

[lafriks]

Oh, damn, sorry @silverwind I did not see that you requested changes

[silverwind]

It's fine. I should have cleared that anyways.