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

An overhaul of Nim's CI #17424

Closed
wants to merge 16 commits into from
Closed

An overhaul of Nim's CI #17424

wants to merge 16 commits into from

Conversation

alaviss
Copy link
Collaborator

@alaviss alaviss commented Mar 20, 2021

Todo:

  • Consistent environment
    • Linux
      • Node.js v12 (except i386, Node.js no longer has official support for this platform since v10)
      • GCC 8.4.0 (as requested by @Araq)
    • Windows
      • Node.js v12
      • GCC 8.4.0
  • Split components into shell scripts, the ci/ folder can be re-used for this. Also consider copying some code from nightlies.

Fixes #17325

alaviss added 16 commits March 19, 2021 14:39
We are gonna add it back later.
As a zombie reaper and for the image to stay alive.
This image sets CMD to a run forever process, which will keep the image running.
This new version specify the location of the preinstalled node.js binary.
This update includes ca-certificates, needed for cloning.
Node.js has deprecated 32-bit builds for x86 Linux:
nodejs/build#885
This gives us greater control over the linux testing environment.
The nim-ci image already bundled everything neccessary.
Node.js no longer has official support for this platform.
Node.js no longer officially support this platform.
@Varriount
Copy link
Contributor

Depending on the complexity of the various tasks, you might want to consider using something like Python, Ruby, or Perl over Bash.

@timotheecour
Copy link
Member

timotheecour commented Mar 20, 2021

in #17359 (comment) I was able to make i386 work again using a much smaller patch (i could turn it into a cleaned up PR if needed) but 3 tests failed:

  • tests/async/tasyncssl.nim
  • tests/stdlib/tssl.nim
  • tests/niminaction/Chapter8/sfml/sfml_test.nim

still though, isn't disabling 3 tests better than disabling all nim js tests on 32bit linux?

that said, a docker based approach might be more flexible than the way we've been doing via cross compilation (although it does serve as an example of cross compilation tested in CI, which is useful)

can't we have a Dockerfile in the sources instead of relying on an external docker image (which is hard/impossible to change in PR's, say when a new dependency is needed). At least github actions supports using Dockerfile.

links

@dom96
Copy link
Contributor

dom96 commented Mar 20, 2021

Depending on the complexity of the various tasks, you might want to consider using something like Python, Ruby, or Perl over Bash.

Better yet, consider using Nim. You can easily grab a Nim release that is known to work for the CI code.

containerImage: ghcr.io/alaviss/nim-ci:sha-aa56233@sha256:43dbe085d041c95b5d39bcbc5e0c4e05d922d407ac8a8938d33064402a5a5f22
CPU: amd64
i386:
containerImage: ghcr.io/alaviss/nim-ci:sha-aa56233@sha256:7f6d204d43e213ff8621e9f0c9f2b788858b9f3788fed95ac62e1c821baa4476
Copy link
Collaborator

@juancarlospaco juancarlospaco Mar 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything to test the repo must live in the Nim organization IMHO.

Copy link

@ghost ghost Mar 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juancarlospaco then maybe just move nim-ci under @nim-lang organization, no need to have it in the same repo

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True

@alaviss
Copy link
Collaborator Author

alaviss commented Mar 20, 2021

@timotheecour

still though, isn't disabling 3 tests better than disabling all nim js tests on 32bit linux?

It's mostly out of practical concerns. Node.js no longer provides official builds on 32-bit x86 Linux, and support for it has been moved to "experimental". I can add a recent Node.js to the image by building it from source, but I don't consider that worth the hassle, considering that no one in their right mind would run Node.js on 32-bit x86 Linux.

If we want 32-bit Node testing, there is Windows and my ARM CI in #16396.

that said, a docker based approach might be more flexible than the way we've been doing via cross compilation (although it does serve as an example of cross compilation tested in CI, which is useful)

The way we "test" cross compilation is mostly a hack. ./koch boot doesn't understand cross compilers at all, and we don't test the usage of <arch>.<os>.<cfg> configuration variables. For now I'd say that nightlies fulfills this role, given how it cross compile to x86_32, armv7 and arm64 (though we still don't "cross" compile but we use "native" cross compilers).

can't we have a Dockerfile in the sources instead of relying on an external docker image (which is hard/impossible to change in PR's, say when a new dependency is needed). At least github actions supports using Dockerfile.

I can move the alaviss/nim-ci repo to nim-lang/nim-ci.

The main reason for keeping it in a separated repo is so that we can develop further images there, not just for CI. Also, if you look at the GitHub Actions used to build my images, it takes 7 minutes to complete, and I don't want to shoulder that cost on the main CI.

@dom96

Better yet, consider using Nim. You can easily grab a Nim release that is known to work for the CI code.

That would only work if you're testing Windows, Linux or macOS. We test the BSDs too, and there are no prebuilts for them.

And most of the code we're dealing with is just environment setup. 80% of CI work is done in koch already.

@stale
Copy link

stale bot commented Mar 20, 2022

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

@stale stale bot added the stale Staled PR/issues; remove the label after fixing them label Mar 20, 2022
@stale stale bot closed this Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Staled PR/issues; remove the label after fixing them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI fails on azure 32 bit linux Linux_i386: sudo apt-fast install fails
5 participants