Add support for Cirrus CI

real-or-random commented at 10:37 pm on January 6, 2021: contributor

On top of #862.

This PR adds Cirrus CI integration. For now this is intended to replicate the functionality that we have in Travis currently. These are a lot of builds, we should also rethink the build matrix. Moreover, the CI shell script could have some refactoring and cleaning but these issues tasks are for another PR (see also #707).

Cirrus CI is relies on Docker images for Linux builds. We make use of this extensively by building our own Docker images (with necessary packages) on which CI then runs (https://cirrus-ci.org/guide/docker-builder-vm/#dockerfile-as-a-ci-environment), see the dockerfile setting in .cirrus.yml. The image is recreated for every build and then all “tasks” run on this image.

For the Linux builds, NixOs is used. This is a rather uncommon choice. My intention of this is that it gives you an easy way of opening a shell on your local machine that has the build tools exactly as in the CI environment. (Just install the nix package manager on top of your distro and run the command from the .cirrus.yml. Now after playing around with Nix, I’m not so convinced anymore:

Not many people are familiar with Nix (these is my first usage of Nix), only @jonasnick here knows more.
Support for cross-compilers is somewhat limited.
As we anyway have docker, there’s already a nice way to replicate the CI environment locally.

Still, I kept Nix for now since it’s working now.

Debian is already used for a s390x big-endian qemu test. I choose s390x over the more common PowerPC and others because s390x is the only officially supported big-endian port of Debian (https://www.debian.org/ports/).

On the MacOS builds, we use a forked valgrind version which support newer macOS versions (https://github.com/LouisBrunner/valgrind-macos). It’s not great but it works and this was change was overdue anyway.

Todo (in this PR):

Add build badge
Enable caching of brew binaries for macOS (note that the macOS builds on Cirrus don’t use Docker)

Todo (other PRs)

Reduce/reorganize jobs
Tidy cirrus.sh script
Add windows/MSVC
Maybe add “bleeding edge” jobs that are allowed to fail and test against latest compilers.

real-or-random commented at 9:41 am on January 7, 2021: contributor

Ok I’ve added a proper description. It’s still a draft PR but please have a look.

sipa commented at 6:05 pm on January 7, 2021: contributor

@theuni Care to have a look at the buildsystem stuff here?

real-or-random cross-referenced this on Jan 8, 2021 from issue Try to use qemu for big endian test on travis by real-or-random

real-or-random commented at 8:36 pm on January 8, 2021: contributor

@theuni Care to have a look at the buildsystem stuff here?

If you want to look at this, please look at #862 . All the buildsystem stuff is there, and this PR is not yet rebased on #862.

real-or-random cross-referenced this on Jan 11, 2021 from issue Autoconf improvements by real-or-random

real-or-random force-pushed on Jan 12, 2021

in ci/cirrus.sh:57 in 2d04692ef2 outdated

52+    then
53+       EXEC="$EXEC $QEMU_CMD"
54+    fi
55+    if [ "$RUN_VALGRIND" = "yes" ]
56+    then
57+        # Using the local `libtool` because on macOS the system's libtool has nothing to do with GNU libtool

jonasnick commented at 5:39 pm on January 13, 2021:

should that comment be above the definition of EXEC='./libtool...?

real-or-random commented at 3:10 pm on January 14, 2021:

fixed

real-or-random force-pushed on Jan 13, 2021

jonasnick commented at 9:27 pm on January 13, 2021: contributor

Thanks! Looks good overall. In particular, should have the same set of tests as before.

For the Linux builds, NixOs is used

It’s not NixOS but Alpine with the Nix package manager. That should also be reflected in the new files (e.g. NixOS shell -> Nix shell).

This sounds great but one drawback is that there is currently no way to invalidate the cache except for pushing a change to .cirrus.yml or the Dockerfile

Yeah would be nice to have this and I’m seeing you’re experimenting with that in this PR.

Now after playing around with Nix, I’m not so convinced anymore:

There’s an advantage of using Nix, namely you can reproduce the exact same dependencies locally by pinning the nixpkgs. When rebuilding the debian docker image you may get newer versions than in CI. Historically we didn’t have problems with reproducing test failures. Either way is fine for me, but if we keep nix we should output the nixpkgs commit to actually be able to reproduce it.

I’ll probably change the 32-bit i686 builds back to Debian, I had some issues with them.

i686 is only a “limitedSupportedSystem” in Nix which means that the nix cache only contains only a “minimal” package set. Best would be to either just use clang instead of clang_11 or switch to debian for i686.

Since we’re on a rolling-release channel, it’s rather easy to get the latest compilers for example and see miscompilations early for example.

Not sure about this, but we can try it out.

theuni commented at 2:26 am on January 14, 2021: contributor

If you want to look at this, please look at #862 . All the buildsystem stuff is there, and this PR is not yet rebased on #862.

Roger, will have a look this week.

jonasnick commented at 2:31 pm on January 14, 2021: contributor

Since I’m just noticing that Travis is still pending, I’m wondering if this PR wouldn’t also be the right time to remove it.

real-or-random force-pushed on Jan 14, 2021

sipa commented at 9:10 pm on January 27, 2021: contributor

Concept ACK. The NixOS approach doesn’t seem to add much overhead, and not too hard to replace if it’s an issue.

It seems Travis is acting up again, is there something left to get this to an minimal viable mergable state?

real-or-random marked this as ready for review on Jan 28, 2021

real-or-random force-pushed on Jan 28, 2021

real-or-random commented at 4:37 pm on January 28, 2021: contributor

Yes, this slipped out of my focus…

I talked to a few people including @MarcoFalke and I’m considering a different approach for “bleeding edge” builds, namely regular (e.g., nightly) builds (of master) that won’t get in the way of contributors working on their PRs. AFAIC this is supported by Cirrus (https://cirrus-ci.org/guide/writing-tasks/#cron-builds).

Also I think that Debian experimental would be a nice platform for this, and this would then probably obsolete the last reason to NixOs, so I’m also considering switching back to Debian for the stable builds.

But all of this can be done in a separate PR. Since this is currently working, let’s get this merged. I’ve added a commit that removes Travis as suggested above.

in ci/cirrus.sh:36 in eb16650ee0 outdated

28@@ -25,23 +29,35 @@ if [ -n "$BUILD" ]
29 then
30     make -j2 "$BUILD"
31 fi
32+
33 if [ "$RUN_VALGRIND" = "yes" ]
34 then
35     make -j2
36-    # the `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (https://www.valgrind.org/docs/manual/manual-core.html)
37+    # the `--error-exitcode` is required to make the test fail if valgrind found errors, otherwise it'll return 0 (http://valgrind.org/docs/manual/manual-core.html)

jonasnick commented at 8:38 pm on January 28, 2021:

why back to http? the existing link seems to work

real-or-random commented at 10:24 pm on January 28, 2021:

whoops, i think this is a result of not having rebased after the recent PR, I’ll fix this.

in ci/shell.nix:5 in eb16650ee0 outdated

0@@ -0,0 +1,6 @@
1+with (import <nixpkgs> {});
2+mkShell {
3+   buildInputs = [
4+       bash file pkgconfig autoconf automake libtool gmp valgrind clang gcc
5+   ];

jonasnick commented at 8:56 pm on January 28, 2021:

Could you add

0  shellHook = ''
1    echo Running nix-shell with nixpkgs version: $(nix eval --raw nixpkgs.lib.version)
2  '';

to this set? That way we can make use of nix by being able to exactly reproduce the dependencies of this nix-shell.

real-or-random commented at 3:32 pm on January 29, 2021:

Nice thanks. I’ll also switch back to the stable channel for now, I forgot to include this change (see my recent comment above).

in ci/cirrus.sh:15 in eb16650ee0 outdated

18+$CC -v || true
19+valgrind --version || true
20+
21+./autogen.sh
22+
23+# NixOS doesn't store GNU file in /usr/bin, see https://lists.gnu.org/archive/html/bug-libtool/2015-09/msg00000.html .

jonasnick commented at 8:58 pm on January 28, 2021:

s/NixOS/Nix

Perhaps also worth replacing all mentions of NixOS shell with Nix shell to avoid confusion.

real-or-random commented at 4:27 pm on January 29, 2021:

I changed NixOS shell to NixOS, Nix Shell to make clarify it’s a Nix shell running on NixOS docker image.

in .cirrus.yml:37 in 11157e54e4 outdated

32+      - cat test_env.log || true
33+      - env
34+
35+docker_arg_force_build_snippet: &DOCKER_ARGS_FORCE_REBUILD
36+  docker_arguments:
37+  # Add build id to the key of the Docker cache in order to force rebuild of Docker image for every build.

sipa commented at 8:59 pm on January 28, 2021:

Why is this needed/desirable?

real-or-random commented at 3:54 pm on January 29, 2021:

Yes, this deserves some explanation.

Cirrus CI is relies on Docker images for Linux builds. We make use of this extensively by building our own Docker images (with necessary packages) on which CI then runs (https://cirrus-ci.org/guide/docker-builder-vm/#dockerfile-as-a-ci-environment), see the dockerfile setting in .cirrus.yml. The image is recreated for every build and then all “tasks” run on this image.

Without this (somewhat hackish) line, the Docker image will be recreated only when the Dockerfile changes.

I initially added this with the idea of building with a bleeding edge toolchains in mind. Now that the plan is to do these builds differently, clearing the cache is not as important. I still think it’s a good idea because it adds only a few minutes to the build and there’s no button to clear the caches (https://github.com/cirruslabs/cirrus-ci-docs/issues/544).

BUT I just noticed that this line doesn’t work at all and the reason is that Cirrus simply reverted the change which made it work (https://github.com/cirruslabs/cirrus-cli/pull/221#issuecomment-756814062). So I’ll remove it…

We’ll need to change the Dockerfile then if we really want to force a rebuild, at least until the aforementioned Cirrus issue has been solved. Not elegant but acceptable.

jonasnick commented at 9:01 pm on January 28, 2021: contributor

ACK eb16650ee08526b96bc80f8efdab5b90bf2ae01e mod nits

real-or-random force-pushed on Jan 29, 2021

ci: Add support for Cirrus CI 8c02e465c5

ci: Enable simple cache for brewing valgrind on macOS 2b359f1c1d

ci: Remove support for Travis CI

So long, and thanks for all fish!

2480e55c8f

real-or-random force-pushed on Jan 29, 2021

ci: Refactor Nix shell files cc2a5451dc

real-or-random force-pushed on Jan 29, 2021

real-or-random commented at 9:27 pm on January 29, 2021: contributor

Ready for review again (when the checks pass).

sipa commented at 0:30 am on January 30, 2021: contributor

ACK cc2a5451dc8ac8a3a9368e1a5b3a1488b15a8bc3. Tested by introducing bugs: #883, #884, #885, #886, #887.

sipa cross-referenced this on Jan 30, 2021 from issue Safegcd inverses, drop Jacobi symbols, remove libgmp by sipa

sipa cross-referenced this on Jan 30, 2021 from issue Signed-digit multi-comb for ecmult_gen (by peterdettman) by sipa

jonasnick approved

jonasnick commented at 10:06 am on January 30, 2021: contributor

ACK cc2a5451dc8ac8a3a9368e1a5b3a1488b15a8bc3

jonasnick merged this on Jan 30, 2021

jonasnick closed this on Jan 30, 2021

real-or-random commented at 9:50 am on February 4, 2021: contributor

Argh, the “clang” builds use GCC in fact [0] because nix-shell redefines $CC. [1] There’s a lot of “wrapper” stuff in general. https://nixos.wiki/wiki/C I feel all of this may be great for users but not what we want in CI, where we want a pure environment.

Until now, my position was that nix doesn’t really give us the benefits I hoped for but it was working, so I kept it. But now that’s a real disadvantage. I think it’s better to get of rid, which is somewhat sad given that I spent some time on it.

[0] https://cirrus-ci.com/task/5368525575421952?command=test#L164 [1] https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/cc-wrapper/setup-hook.sh#L110

real-or-random commented at 5:15 pm on February 4, 2021: contributor

We should also consider implementing the hack from Core (https://github.com/cirruslabs/cirrus-ci-docs/issues/662#issuecomment-653901809) in order to run CI on the merge commit instead of the source branch (in the case of PRs).

Fabcien referenced this in commit 90f54a443f on Apr 13, 2021

deadalnix referenced this in commit 1744fb22be on Apr 14, 2021

real-or-random referenced this in commit 4a496a36fb on Apr 1, 2023

real-or-random cross-referenced this on Apr 1, 2023 from issue ct: Use volatile "trick" in all fe/scalar cmov implementations by real-or-random

real-or-random referenced this in commit 2d51a454fc on Apr 6, 2023

real-or-random referenced this in commit 96f4853850 on Apr 11, 2023

dderjoel referenced this in commit e9da2fe043 on May 23, 2023

Add support for Cirrus CI #864