Handle non-Apple packaged GCC versions #34
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently if you use a version of GCC packaged from a Linux distro or built from trunk, you'll get an inferred machine name of something like "foo__gcc_None__riscv64".
The None is supposed to be the cc_build part of the inferred version info, but currently we only handle this for Apple packaged GCC versions.
This teaches the version handling to handle these other types of GCC, which will have a version string like
gcc version 16.0.0 20250807 (experimental) (GCC)
or
gcc version 12.2.0 (Debian 12.2.0-14+deb12u1)
lukel97
requested review from
DavidSpickett,
TamarChristinaArm,
asb,
omjavaid and
slydiman
TamarChristinaArm
approved these changes
Aug 13, 2025
| m = re.match(r'(.*) version ([^ ]*) ([0-9]+ )?+(\([^(]*\))(.*)', | ||
| version_ln) | ||
| if m is not None: | ||
| cc_name, cc_version_num, cc_build_string, cc_extra = m.groups() |
There was a problem hiding this comment.
Since you never care about the date group, how about just making the regexp a non-capturing group instead
m = re.match(r'(.*) version ([^ ]*) (?:[0-9]+ )?+(\([^(]*\))(.*)',
version_ln)
That way you don't need the change on line 82. But looks good with or without that change
There was a problem hiding this comment.
Thanks for the review! Yeah that's a good idea, will add
DavidSpickett
added a commit
that referenced
this pull request
Aug 13, 2025
This reverts commit 0e9a3ae.
DavidSpickett
added a commit
that referenced
this pull request
Aug 13, 2025
This reverts commit 0e9a3ae. Reverts #34 due to an error running the test suite on our Linux bots. https://lab.llvm.org/buildbot/#/builders/17/builds/10167 for example. ``` Traceback (most recent call last): File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/bin/lnt", line 33, in <module> sys.exit(load_entry_point('LNT', 'console_scripts', 'lnt')()) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/lib/python3.10/site-packages/click-6.7-py3.10.egg/click/core.py", line 722, in __call__ return self.main(*args, **kwargs) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/lib/python3.10/site-packages/click-6.7-py3.10.egg/click/core.py", line 697, in main rv = self.invoke(ctx) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/lib/python3.10/site-packages/click-6.7-py3.10.egg/click/core.py", line 1066, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/lib/python3.10/site-packages/click-6.7-py3.10.egg/click/core.py", line 1066, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/lib/python3.10/site-packages/click-6.7-py3.10.egg/click/core.py", line 895, in invoke return ctx.invoke(self.callback, **ctx.params) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/sandbox/lib/python3.10/site-packages/click-6.7-py3.10.egg/click/core.py", line 535, in invoke return callback(*args, **kwargs) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/lnt/lnt/tests/test_suite.py", line 1189, in cli_action results = test_suite.run_test(test_suite.opts) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/lnt/lnt/tests/test_suite.py", line 310, in run_test cc_info = self._get_cc_info(cmake_vars) File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/lnt/lnt/tests/test_suite.py", line 714, in _get_cc_info return lnt.testing.util.compilers.get_cc_info( File "/home/tcwg-buildbot/worker/clang-aarch64-sve-vla/test/lnt/lnt/testing/util/compilers.py", line 79, in get_cc_info m = re.match(r'(.*) version ([^ ]*) (?:[0-9]+ )?+(\([^(]*\))(.*)', File "/usr/lib/python3.10/re.py", line 190, in match return _compile(pattern, flags).match(string) File "/usr/lib/python3.10/re.py", line 303, in _compile p = sre_compile.compile(pattern, flags) File "/usr/lib/python3.10/sre_compile.py", line 788, in compile p = sre_parse.parse(p, flags) File "/usr/lib/python3.10/sre_parse.py", line 955, in parse p = _parse_sub(source, state, flags & SRE_FLAG_VERBOSE, 0) File "/usr/lib/python3.10/sre_parse.py", line 444, in _parse_sub itemsappend(_parse(source, state, verbose, nested + 1, File "/usr/lib/python3.10/sre_parse.py", line 672, in _parse raise source.error("multiple repeat", re.error: multiple repeat at position 33 ```
|
Had to revert this - #36 I don't immediately know what the error refers to but anything you need to know I can find out from the bot. Compiler names, python versions, whatever it is. |
Thanks, looks like python 3.10 doesn't like the optional quantifier on the non-capturing group. Are you happy if I go ahead and reland it with a fixed version of the regex? |
|
That's fine, we'll find out pretty quickly if there are more problems. |
lukel97
added a commit
that referenced
this pull request
Aug 13, 2025
This reverts commit 0d5b979, with a fix included for the regex. We had the + quantifier for the space between cc_version_num and cc_build_string in the wrong place which Python 3.10 complained about (but later versions mysteriously silently accepted?)
|
Reapplied in 37a5ba2. My regex actually just had a mistake in it to begin with, python 3.10 was doing the right thing and complained about it. Not sure why the later versions started accepting it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently if you use a version of GCC packaged from a Linux distro or built from trunk, you'll get an inferred machine name of something like "foo__gcc_None__riscv64".
The None is supposed to be the cc_build part of the inferred version info, so ideally you would get something like "foo__gcc_DEV__riscv64". But currently we only handle this for Apple packaged GCC versions.
This teaches the version handling to handle these other types of GCC, which will have a version string like
if built from trunk (note the extra date appended before cc_build_string, and
(GCC)in cc_extra), or