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

ci: Optimize build matrix #1048

Closed

Conversation

real-or-random
Copy link
Contributor

@real-or-random real-or-random commented Dec 21, 2021

We should try to merge #995 before this PR to avoid conflicts.

This tries to get more coverage of "useful" configurations with fewer tasks. I think what I suggest here is good but on the other hand there's no real need to reduce the number of tasks right now, so please suggest additional configs if you think they're worth testing. Maybe we could add a -UVERIFY build? (Even nicer would be to run with and without VERIFY on the same seed and compare the results -- but that needs more work.)

This now always runs make check and then make distcheck but ensures that the latter skips the tests already run during make check.

I refrained from reducing the number of tasks on macOS further. Testing clang on macOS is useful because it's "Apple clang" and testing gcc is useful because it's gcc 9 (as opposed to gcc 10 on Linux). Moreover, the changes in #1047 make macOS tasks quicker again. The annoying thing about these macOS tasks is not that they run too long, but just that there is more demand/scheduling pressure for macOS machines on Cirrus, so they often sit there for hours scheduled and waiting to be run. But there's nothing we can do about this.

@real-or-random
Copy link
Contributor Author

Hm, my plan was to always run make distcheck but I need to rethink that change. So make distcheck includes make check but does in a clean tree which is removed afterwards (including the test logs... -.-), so maybe it's not a good idea to always run it. On the other hand, this means we have this bug with test files being removed already now with the single distcheck task on CI: https://cirrus-ci.com/task/4941156000727040.

Maybe we should stick to the idea of always running distcheck and rescue the test logs. Or just introduce a separate distcheck build.

@real-or-random
Copy link
Contributor Author

real-or-random commented Mar 17, 2022

This now runs make check and then make distcheck but ensures that the latter skips the tests already run during make check. Rebased and ready for review.

edit: also edited the PR description to mention the distcheck change.

@real-or-random real-or-random marked this pull request as draft March 17, 2022 10:20
This removes a check for $ac_cv_prog_cc_c89 which is set by AC_PROG_CC
if defined(__STDC__) in the preprocessor. (Standard compliant compilers
are supposed to define __STDC__ to 1 but the value is actually not
checked here.)

Unfortunately, MSVC doesn't define it, so configure fails for MSVC.

This check is not very useful in practice. Over 30 years after C89 has
been released, there are no C compilers out there that are not
sufficiently compliant with C89 for the project. The only practically
relevant case was that the check rejected C++ compilers. A different
method to reject C++ compilers will be introduced in a later commit.
This adds MSVC builds built on Linux using wine. This requires some
settings of tools and flags because the autotools support for MSVC is
naturally somewhat limited.

The advantage of this approach is that it is compatible with our
existing CI scripts, so there's no need to write a Windows CI script
(in PowerShell or similar). If we want to test building and running on
Windows native (e.g., as supported by Cirrus CI) we could still do this
in the future.

Another advantage of this approach is that contributors can simply use
the docker image if they need a MSVC installation in a non-Windows
environment.

This commit also improves the Dockerfile by grouping RUN commands
according to Docker docs:
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run
This commit also raises the TEST_ITERS for wine tasks to the default.
The overhead of wine is negligible, so we can certainly afford the same
number of iterations as for native Linux tests.
@maflcko
Copy link
Contributor

maflcko commented Oct 1, 2023

What is the status here?

@real-or-random
Copy link
Contributor Author

Yeah, this is obsolete. It still makes sense to take a look at the build matrix and see if we're happy with it. Let me track this in #1392.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants