-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Bravely revert "CI: Disable some Cirrus CI jobs during RIIR transition" #11884
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
base: master
Are you sure you want to change the base?
Conversation
631fe74 to
bd0af5d
Compare
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: #11884 (comment)
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: #11884 (comment)
Else this runs when people push to their master's forks, see fish-shell#11884 (comment)
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: fish-shell#11884 (comment)
The root cause was that FISH_CHECK_LINT was not set. By default, check.sh should fail if any tool is not installed, see 59b4398 (build_tools/style.fish: fail if formatters are not available, 2025-07-24); like it does for rustfmt and clippy. This reverts commit fbfd29d. See fish-shell#11884
I think we do want to stop using CMake on Cirrus but this should first be tested in combination with all the other changes that made it to master concurrently (test by pushing a temporary branch to the fish-shell repo), to avoid confusion as to what exactly broke. This reverts commit d167ab9. See fish-shell#11884
Else this runs when people push to their master's forks, see fish-shell#11884 (comment)
bd0af5d to
f5cafe0
Compare
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: #11884 (comment)
|
TODO: need to either re-revert b1e8fdf (Revert "CI: use build_tools/check.sh in Cirrus CI", 2025-10-06), The fact that |
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: #11884 (comment)
f5cafe0 to
da0d6b8
Compare
You can test it in your own fork and it will write to your own package registry. A dispatch trigger would be a fine thing to add. I was going to poke at this some more but it looks like you want to take it over? |
da0d6b8 to
4646331
Compare
right, that should work (probably some of the ones in https://github.com/krobelus/fish-shell/actions/runs/18277577848 failed because I had never pushed those images before, so I get a permissions error).
Feel free to take it back (and drop the bad commit that's unrelated to Cirrus). |
This reverts what remains of commit 91be748. Closes fish-shell#11871 Closes fish-shell#11884
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: fish-shell#11884 (comment)
This reverts what remains of commit 91be748. Closes fish-shell#11871 Closes fish-shell#11884
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: fish-shell#11884 (comment)
These should work now that we have (automatically-updated) docker builds via .github/workflows/docker_builds.yml. Ref: #11884 (comment)
This fails with "exec format error" because our container is built on a 64 bit system on GitHub Actions. Not yet sure how to fix that.
.cirrus.yml
Outdated
| # container: | ||
| # <<: *step | ||
| # image: ghcr.io/krobelus/fish-ci/focal-32bit:latest | ||
| # image: ghcr.io/fish-shell/fish-ci/focal-32bit:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok it wasn't so bad; I only had to add write permissions on some of the docker images in my registry.
I'm more or less ready to push a version that has successfully passed tests on my fork.
Though I disabled jammy-armv7-32bit for now, due to exec format error.. maybe this container
- os: ubuntu-24.04-arm
target: jammy-armv7-32bit
needs to be built on a 32bit host?
Whenever we make a change to docker/ on master,
we need to first push those changes,
wait for the docker images to be pushed,
and then re-run CI.
If the previous push needs the new docker images, its CI run would fail.
It's much better than before, but ideally we can declare build-docker-images a prerequisite for .cirrus.yml,
so Cirrus waits for that. I wonder what's the best way to do that.
(One workaround is manual kickoff of build-docker-images on a non-master branch before pushing to master)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait for the docker images to be pushed
pushed an untested draft commit for that.
Will need to test it with Cirrus on my fork,
and when that works add fish-shell's CIRRUS_TOKEN to github secrets.
4646331 to
8a4a8bb
Compare
(TODO this is totally untested and mostly vibe coded.) Whenever we push changes do docker/**, our docker images for Cirrus CI will be rebuilt. However, the Cirrus CI jobs will kick off at the same time as the Docker builds, so they will likely use old images. This can cause surprising (albeit transient) failures. Fix this by having Cirrus wait for GitHub Actions. This addresses the second part of #11884 (comment)
See fish-shell#11961 (comment) I didn't find a good way to add ~/.cargo/bin to $PATH only for fishuser, so hardcode it for all users for now. The default rustup behavior (source it in ~/.bashrc) only works for interactive shells, so it doesn't work in CI. I updated the images manually for now, because I don't want to test on master (see fish-shell#11884 and fish-shell#11961 (comment)) # Upload new docker images. repo+owner=krobelus git push $repo_owner HEAD:tmp gh --repo=$repo_owner/fish-shell \ workflow run --ref=tmp \ .github/workflows/build_docker_images.yml # https://github.com/$repo_owner/fish-shell/actions/workflows/build_docker_images.yml sleep 3m # Enable Cirrus CI on fork. sed -i "s/OWNER == 'fish-shell'/OWNER == '$repo_owner'/g" .cirrus.yml git commit && git push $repo_owner HEAD:tmp Part of fish-shell#11961
See fish-shell#11961 (comment) I didn't find a good way to add ~/.cargo/bin to $PATH only for fishuser, so hardcode it for all users for now. The default rustup behavior (source it in ~/.bashrc) only works for interactive shells, so it doesn't work in CI. I updated the images manually for now, because I'm not sure I want to test on master (see fish-shell#11884 and fish-shell#11961 (comment)) sed -i '/if: github.repository_owner/d' .github/workflows/build_docker_images.yml git commit # Upload new docker images. repo_owner=krobelus git push $repo_owner HEAD:tmp gh --repo=$repo_owner/fish-shell \ workflow run --ref=tmp \ .github/workflows/build_docker_images.yml # https://github.com/$repo_owner/fish-shell/actions/workflows/build_docker_images.yml sleep 3m # Enable Cirrus CI on fork. sed -i "s/OWNER == 'fish-shell'/OWNER == '$repo_owner'/g" .cirrus.yml git commit git push $repo_owner HEAD:tmp Part of fish-shell#11961
See fish-shell#11961 (comment) I didn't find a good way to add ~/.cargo/bin to $PATH only for fishuser, so hardcode it for all users for now. The default rustup behavior (source it in ~/.bashrc) only works for interactive shells, so it doesn't work in CI. I updated the images manually for now, because I'm not sure I want to test on master (see fish-shell#11884 and fish-shell#11961 (comment)) sed -i '/if: github.repository_owner/d' .github/workflows/build_docker_images.yml git commit # Upload new docker images. repo_owner=krobelus git push $repo_owner HEAD:tmp gh --repo=$repo_owner/fish-shell \ workflow run --ref=tmp \ .github/workflows/build_docker_images.yml # https://github.com/$repo_owner/fish-shell/actions/workflows/build_docker_images.yml sleep 3m # Enable Cirrus CI on fork. sed -i "s/OWNER == 'fish-shell'/OWNER == '$repo_owner'/g" .cirrus.yml git commit git push $repo_owner HEAD:tmp Part of fish-shell#11961
Closes fish-shell#11884 Closes fish-shell#11871
Closes fish-shell#11884 Closes fish-shell#11871
cirrus.yml depends on .github/workflows/build_docker_images.yml but the CI doesn't know that. Simplify things by building docker images directly in Cirrus CI (following https://cirrus-ci.org/guide/docker-builder-vm/). No need to push them to a registry, Cirrus caches for us. While at it, add back ARM jobs that should mostly match the ones removed in c0b7167 (Remove unused docker images for frozen OS releases, 2025-10-31). The exact Ubuntu versions are different, but those are probably arbitrary. Closes fish-shell#11884 Closes fish-shell#11871
cirrus.yml depends on .github/workflows/build_docker_images.yml but the CI doesn't know that. Simplify things by building docker images directly in Cirrus CI (following https://cirrus-ci.org/guide/docker-builder-vm/). No need to push them to a registry, Cirrus caches for us. While at it, add back ARM jobs that should mostly match the ones removed in c0b7167 (Remove unused docker images for frozen OS releases, 2025-10-31). The exact Ubuntu versions are different, but those are probably arbitrary. Closes fish-shell#11884 Closes fish-shell#11871
cirrus.yml depends on .github/workflows/build_docker_images.yml but the CI doesn't know that. Simplify things by building docker images directly in Cirrus CI (following https://cirrus-ci.org/guide/docker-builder-vm/). No need to push them to a registry, Cirrus caches for us. While at it, add back ARM jobs that should mostly match the ones removed in c0b7167 (Remove unused docker images for frozen OS releases, 2025-10-31). The exact Ubuntu versions are different, but those are probably arbitrary. Closes #11884 Closes #11871
Test this actually works on Cirrus