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

Go 1.14 version check fails on Windows/amd64 #10507

Closed
filipnavara opened this issue Feb 27, 2020 · 18 comments · Fixed by #10508
Closed

Go 1.14 version check fails on Windows/amd64 #10507

filipnavara opened this issue Feb 27, 2020 · 18 comments · Fixed by #10508
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail

Comments

@filipnavara
Copy link
Contributor

  • Gitea version (or commit ref): a924a90
  • Git version: 2.25
  • Operating system: Windows

Description

The check in make go-check fails because the regular expression for version extraction doesn't match the output of go version.

go version output is go version go1.14 windows/amd64 which cannot be matched with grep -Eo '[0-9]+\.?[0-9]+?\.?[0-9]?\s'.

@silverwind
Copy link
Member

The regex does match and it looks to be able to cope with 2-number versions too. I'm not sure what the issue exactly is.

$ printf "%03d%03d%03d" $(echo 'go version go1.13.4 linux/amd64' | grep -Eo '[0-9]+\.?[0-9]+?\.?[0-9]?\s' | tr '.' ' ')
001013004
$ printf "%03d%03d%03d" $(echo 'go version go1.14 windows/amd64' | grep -Eo '[0-9]+\.?[0-9]+?\.?[0-9]?\s' | tr '.' ' ')
001014000

@jolheiser jolheiser added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Feb 27, 2020
@filipnavara
Copy link
Contributor Author

filipnavara commented Feb 27, 2020

Interesting, in MSYS I get this:

bash-3.1$ printf "%03d%03d%03d" $(echo 'go version go1.14 windows/amd64' | grep -Eo '[0-9]+\.?[0-9]+?\.?[0-9]?\s' | tr '.' ' ')
000000000
bash-3.1$

Perhaps an issue with escaping in the pattern vs command line escaping?

@jolheiser
Copy link
Member

In Git Bash on Windows I get 001014000

@filipnavara
Copy link
Contributor Author

I get the same garbage in Git Bash, let me check various tool versions to pinpoint which one breaks it.

@filipnavara
Copy link
Contributor Author

Seems to be an issue with the grep in MSYS. I'll just close the issue, thanks for help.

@filipnavara
Copy link
Contributor Author

For the record, the version of grep in Git Bash is 3.1. The one I had in my MSYS installation was version 2.5.4. The older version doesn't understand the \s syntax apparently, or interprets it incorrectly.

@zeripath
Copy link
Contributor

hmm... I think that means we should switch away from using \s as it won't work on non-GNU systems too.

@zeripath
Copy link
Contributor

We should probably use \b

@filipnavara
Copy link
Contributor Author

filipnavara commented Feb 27, 2020

\b doesn't work either. With Git Bash it matches both the version and "64" at the end. With the old MSYS grep it doesn't match anything. Putting a dumb (space) instead of \s works with both but not sure if it's reliable and future proof.

Given that version 2.5.4 is over a decade old and we don't have reports from any other system I'd just hope for the best and keep the current code.

@zeripath
Copy link
Contributor

The BSD users will complain about it too.

@guillep2k
Copy link
Member

guillep2k commented Feb 27, 2020

We should use egrep (a shortcut for grep -e) that supports the whole set of regexp syntax (grep's own, but all of it). grep without -e has a reduced set. BTW egrep has been around since Unix System V Release 4.

@silverwind
Copy link
Member

Maybe replace \s with (a single space). Will break on tab thought.

@silverwind
Copy link
Member

We should use egrep

Our grep -E is equivalent to egrep, at least on Linux.

@zeripath
Copy link
Contributor

can do [ \t] as far as I can see. I'm just trying to find the appropriate manpages.

@guillep2k
Copy link
Member

Our grep -E is equivalent to egrep, at least on Linux.

Oh, doh! 🤦‍♂ Of course. I just felt like remembering the good old times, that's all.

@guillep2k
Copy link
Member

The problem is that we need to account for shell and make escaping our characters.

@zeripath
Copy link
Contributor

OK POSIX says that [:space:] should be supported in all locales so I think we can use that.

@zeripath
Copy link
Contributor

as in grep -Eo '[0-9]+\.?[0-9]+?\.?[0-9]?[[:space:]]'

@zeripath zeripath reopened this Feb 27, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants