-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix parsing of gfortran version #47352
Conversation
I'll return to this, probably want to rewrite the regex more completely to make sure it's sane, but this is enough to solve my issue for now. |
Could you add your example |
This is what
|
Mine is:
|
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.
It is still selecting the last thing that matches, I believe.
IMO it’s a little unreasonable to parse the version number 10-posix. There’s no way we can realistically disambiguate whether 10-posix is the version number, or 20220324 is. I think we should require at least one period in the version number. |
If it can’t be auto detected, we should have a good error message, or just default to libgfortran5, since that is by far the most common these days. |
Could we anchor the version search by finding
|
Looked into this more, and it appears that
or
|
Possibly not even non-trivially haha:
|
If we don't need |
Because if they have a |
Wow, who would have thought this too could still have so many problems with it For example: Or in other cases trying to interpret the Apple version of clang as a gfortan compiler |
Apparently supporting newer versions need |
The various dump flags are defined here (for their current form, after getting renamed in gcc-8) https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Developer-Options.html#Developer-Options If all else fails, we can get the all cpp macros with -dM -E and search for |
Is there a case where the major version number would ever not be the first number in |
Yes, the docs clarify that this was changed from being the fullversion to being the filepath name |
332b28c
to
7e9327b
Compare
Co-authored-by: Jameson Nash <[email protected]>
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.
Looks good to me!
Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 626f3b2)
Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 626f3b2)
Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 626f3b2)
No description provided.