hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.24k stars 223 forks source link

CI `run-tests` script now attempts to detect Apple Clang 15 and other unsupported regression-tests compiler versions #972

Closed bluetarpmedia closed 4 months ago

bluetarpmedia commented 5 months ago

After investigating some spurious macOS regression-test job failures, I found that the macos-13 GitHub runner has both Apple Clang 14 and 15 installed.

It appears that on some runs, running clang++ incorrectly selects Apple Clang 15, which is currently unsupported as a compiler version for the regression-tests. For example, see this log file for this failed job.

This PR makes 2 changes:

CC @jarzec

gregmarr commented 5 months ago

Is there a reason to not add support for Clang 15?

hsutter commented 5 months ago

incorrectly selects Apple Clang 15, which is currently unsupported as a compiler version for the regression-tests

Is there an issue with Apple Clang 15, or just that there's no subdirectory for Apple Clang 15 in the test results?

jarzec commented 5 months ago

@hsutter In fact I wanted to propose adding support for Clang 15. I think the run-test.sh script has some leftovers of my trials for that. This version is available on Ubuntu 2204 and is installed by default on the GitHub runner. This is also the case on the macos-13 runner. The added value of adding this version is that all regression tests can be compiled and run using it, so it would be the only compiler on POSIX systems allowing for that (at least that was the case last time I tried). Can quickly prepare a PR with that.

bluetarpmedia commented 5 months ago

Is there a reason to not add support for Clang 15?

Nope, that's definitely an option. (Note there's Clang 15 as well as Apple Clang 15.)

bluetarpmedia commented 5 months ago

Is there an issue with Apple Clang 15, or just that there's no subdirectory for Apple Clang 15 in the test results?

Yeah, the latter, because there are no expected test-results yet for Apple Clang 15.

Even if we add test-results for Clang 15 and Apple Clang 15, I think it's still worth adding this part of the PR:

printf "Unhandled compiler version:\n$compiler_version\n\n"

That way in the future we'll get more details when the job fails due to a new compiler on the GitHub runner that we don't yet have test-results for.

hsutter commented 5 months ago

@jarzec I see an "approved these changes" from you midway up above, just checking: Does this look ready to merge to you?

jarzec commented 5 months ago

@jarzec I see an "approved these changes" from you midway up above, just checking: Does this look ready to merge to you?

Yes, please do merge. I've started to look into including Clang 15 on top of the updates here. (Sorry, I previously responded from my work account).

hsutter commented 4 months ago

Thanks!