Skip to content

ci: add buildbot support#1985

Merged
danth merged 2 commits intonix-community:masterfrom
0xda157:buildbot
Nov 22, 2025
Merged

ci: add buildbot support#1985
danth merged 2 commits intonix-community:masterfrom
0xda157:buildbot

Conversation

@0xda157
Copy link
Copy Markdown
Contributor

@0xda157 0xda157 commented Nov 18, 2025

cc @trueNAHO @danth @zowoq

related: nix-community/infra#2027

inspired by nix-community/nixvim#3410


@stylix-automation stylix-automation bot added the topic: flake /flake.nix, /flake.lock, and /flake/ subsystems label Nov 18, 2025
@zowoq
Copy link
Copy Markdown

zowoq commented Nov 18, 2025

Triggered a couple of runs. The method I'm using for testing currently only works on the repos default branch so I've pushed this commit to master in my fork of this repo.

https://buildbot.nix-community.org/#/builders/6570/builds/7, https://buildbot.nix-community.org/#/builders/6570/builds/9

Copy link
Copy Markdown
Member

@danth danth left a comment

Choose a reason for hiding this comment

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

A few minor suggestions, but otherwise looks good to go :))

Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
@danth danth marked this pull request as ready for review November 19, 2025 15:42
Copy link
Copy Markdown
Member

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

SGTM. Should this PR also be responsible for removing the relevant github workflows?

Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Copy link
Copy Markdown
Member

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

Should this PR also be responsible for removing the relevant github workflows?

Ideally not to prevent temporary CI outage during the manual buildbot migration. This should be done in a follow-up PR when buildbot is fully ready. I would also like #1978 to be merged before removal for future reference and in case we ever move back to GitHub.

Comment thread flake/ci.nix Outdated
Comment thread flake/dev/packages.nix Outdated
@0xda157 0xda157 force-pushed the buildbot branch 3 times, most recently from 6f217b9 to 36b8be1 Compare November 20, 2025 22:29
Comment thread flake/options/testbeds.nix Outdated
Comment thread flake/options/testbeds.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/options/testbeds.nix Outdated
@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Nov 20, 2025

do we want to maintain the checks = config.packages? I would think it would make more sense to replace that with checks = { inherit (config.packages) docs serve-docs palette-generator; } so it's practical to run locally and we can use github ci to run our checks (e.g. pre-commit) without building all testbeds. we could also remove building any packages with checks entirely and build all of them with buildbot:

      ci.buildbot = lib.pipe config.packages [
        (lib.mapAttrs' (
          name: drv:
          if lib.hasPrefix "testbed:" name then
            let
              # name is formatted as `testbed:target:variant` e.g. `testbed:alacritty:dark`
              splitName = lib.splitString ":" name;
              target = builtins.elemAt splitName 1;
              variant = builtins.elemAt splitName 2;
            in
            lib.nameValuePair "testbeds-${target}" { ${variant} = drv; }
          else
            lib.nameValuePair "stylix-packages" { ${name} = drv; }
        ))
        (lib.mapAttrs pkgs.linkFarm)
      ];

@0xda157 0xda157 force-pushed the buildbot branch 2 times, most recently from 8f051bd to 017c193 Compare November 20, 2025 23:42
@MattSturgeon
Copy link
Copy Markdown
Member

do we want to maintain the checks = config.packages?

Good question, although slightly orthogonal to this PR.

I think if testbeds are already easily runnable, and aren't too relevant to local development (or at least, building all of them isn't too relevant), then dropping them from the checks output makes sense.

@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Nov 20, 2025

do we want to maintain the checks = config.packages?

Good question, although slightly orthogonal to this PR.

Speaking of which I think it makes most sense to add buildbot support in this pr and remove building testbeds with gh actions in a follow up once we've confirmed everything is working. That still leaves the question of building packages aside from testbeds with buildbot, which I'm in favor of.

Comment thread flake/dev/packages.nix Outdated
@0xda157 0xda157 requested a review from MattSturgeon November 20, 2025 23:48
@0xda157 0xda157 force-pushed the buildbot branch 3 times, most recently from a564a56 to 923e93b Compare November 21, 2025 01:06
Comment thread flake/dev/packages.nix
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/options/ci.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Comment thread flake/dev/packages.nix Outdated
Copy link
Copy Markdown
Member

@danth danth left a comment

Choose a reason for hiding this comment

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

LGTM - We should continue to add more packages to BuildBot and eventually remove the GitHub Actions workflows after this has been merged.

Copy link
Copy Markdown
Member

@trueNAHO trueNAHO left a comment

Choose a reason for hiding this comment

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

LGTM after resolving the #1985 (comment) nitpick.

Co-authored-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
@danth danth enabled auto-merge (squash) November 22, 2025 20:32
@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Nov 22, 2025

eventually remove the GitHub Actions workflows after this has been merged.

I disagree, running pre-commit and our other checks on github will yield much quicker results and be a better user experience and we don't really gain anything from putting them on buildbot.

@trueNAHO
Copy link
Copy Markdown
Member

trueNAHO commented Nov 22, 2025

eventually remove the GitHub Actions workflows after this has been merged.

I disagree, running pre-commit and our other checks on github will yield much quicker results and be a better user experience and we don't really gain anything from putting them on buildbot.

Does buildbot take much longer to start, or why would GitHub be faster? If buildbot shows the same error logs as GitHub, I would prefer moving off GitHub because its UI downloads 10--20 MB and renders extremely slowly. Unsure whether buildbot has the same problem, but I doubt it gets much worse than GitHub UI.

@danth
Copy link
Copy Markdown
Member

danth commented Nov 22, 2025

To me, it makes sense to put everything on one platform just to simplify the configuration.

@danth danth merged commit adc6506 into nix-community:master Nov 22, 2025
5 checks passed
@0xda157 0xda157 deleted the buildbot branch November 22, 2025 21:07
@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Nov 22, 2025

eventually remove the GitHub Actions workflows after this has been merged.

I disagree, running pre-commit and our other checks on github will yield much quicker results and be a better user experience and we don't really gain anything from putting them on buildbot.

Does buildbot take much longer to start, or why would GitHub be faster? If buildbot shows the same error logs as GitHub, I would prefer moving off GitHub because its UI downloads 10--20 MB and renders extremely slowly. Unsure whether buildbot has the same problem, but I doubt it gets much worse than GitHub UI.

github actions would be much faster to show status iiuc, nixvim moved their formatting back from builtbot to gh actions for this reason.

@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Nov 22, 2025

github actions would be much faster to show status iiuc

currently our check workflow takes a long time to show status, but this only due to building testbeds. it should report the status of the checks in a few minutes once the testbeds are removed.

@0xda157
Copy link
Copy Markdown
Contributor Author

0xda157 commented Nov 22, 2025

github actions would be much faster to show status iiuc

currently our check workflow takes a long time to show status, but this only due to building testbeds. it should report the status of the checks in a few minutes once the testbeds are removed.

so when testing locally even after removing checks = config.packages in still evals every single testbed, which is still going for me after 1h21m (on a i5 1340p w/ 32Gb ram). @MattSturgeon do you have any suggestions on how to make nix flake check usable?

@MattSturgeon
Copy link
Copy Markdown
Member

MattSturgeon commented Nov 23, 2025

github actions would be much faster to show status iiuc

currently our check workflow takes a long time to show status, but this only due to building testbeds. it should report the status of the checks in a few minutes once the testbeds are removed.

so when testing locally even after removing checks = config.packages in still evals every single testbed, which is still going for me after 1h21m (on a i5 1340p w/ 32Gb ram). @MattSturgeon do you have any suggestions on how to make nix flake check usable?

nix flake check will evaluate all outputs it "knows about", such as packages, and validate that they are the correct type. It also builds everything in the checks output.

So the only way to skip evaluating testbeds in nix flake check is to not include testbeds in any outputs that nix flake check will validate. legacyPackages may work, IIUC.

Mikilio pushed a commit to Mikilio/stylix that referenced this pull request Nov 27, 2025
Co-authored-by: Matt Sturgeon <matt@sturgeon.me.uk>
Co-authored-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Reviewed-by: Matt Sturgeon <matt@sturgeon.me.uk>
Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Approved-by: Matt Sturgeon <matt@sturgeon.me.uk>
Approved-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Approved-by: Daniel Thwaites <danth@danth.me>
@zowoq
Copy link
Copy Markdown

zowoq commented Dec 1, 2025

Looks like this needs to be backported to the stable branch as the checks there aren't grouped and buildbot kind of chokes on it.

@trueNAHO trueNAHO added backport: release-25.05 backport: release-25.11 To be backported to the release-25.11 stable branch labels Dec 2, 2025
stylix-automation bot pushed a commit that referenced this pull request Dec 2, 2025
Co-authored-by: Matt Sturgeon <matt@sturgeon.me.uk>
Co-authored-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Reviewed-by: Matt Sturgeon <matt@sturgeon.me.uk>
Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Approved-by: Matt Sturgeon <matt@sturgeon.me.uk>
Approved-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Approved-by: Daniel Thwaites <danth@danth.me>
(cherry picked from commit adc6506)
@stylix-automation
Copy link
Copy Markdown
Contributor

Successfully created backport PR for release-25.05:

@stylix-automation
Copy link
Copy Markdown
Contributor

Backport failed for release-25.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.11
git worktree add -d .worktree/backport-1985-to-release-25.11 origin/release-25.11
cd .worktree/backport-1985-to-release-25.11
git switch --create backport-1985-to-release-25.11
git cherry-pick -x adc650610085adbe130b9860d5bdb869f96050af

1 similar comment
@stylix-automation
Copy link
Copy Markdown
Contributor

Backport failed for release-25.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-25.11
git worktree add -d .worktree/backport-1985-to-release-25.11 origin/release-25.11
cd .worktree/backport-1985-to-release-25.11
git switch --create backport-1985-to-release-25.11
git cherry-pick -x adc650610085adbe130b9860d5bdb869f96050af

@github-actions github-actions bot added the has: port to stable This has been backported to the latest stable branch label Dec 2, 2025
@trueNAHO
Copy link
Copy Markdown
Member

trueNAHO commented Dec 2, 2025

Looks like this needs to be backported to the stable branch as the checks there aren't grouped and buildbot kind of chokes on it.

Would buildbot even work without this being backported? The release-25.11 already has this patch, and release-25.05 will after #2050.

trueNAHO added a commit that referenced this pull request Dec 2, 2025
Co-authored-by: Matt Sturgeon <matt@sturgeon.me.uk>
Co-authored-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Reviewed-by: Matt Sturgeon <matt@sturgeon.me.uk>
Reviewed-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Approved-by: Matt Sturgeon <matt@sturgeon.me.uk>
Approved-by: NAHO <90870942+trueNAHO@users.noreply.github.com>
Approved-by: Daniel Thwaites <danth@danth.me>
(cherry picked from commit adc6506)
@MattSturgeon
Copy link
Copy Markdown
Member

Would buildbot even work without this being backported

Not well...

Buildbot-nix will run on all your non-fork branches, Merge Queue groups, and PR branches.

This PR allows it to run efficiently, without queuing up a rediculous number of build jobs per-push.

Without the buildbot-nix.toml file, it'll default to using your checks flake output. This contains way too many top-level derivations for buildbot-nix to handle well; using checks it'd spawn 760 builds, each in its own worker, vs only 8 with ci.buildbot because it groups up top-level derivations as linkFarms.

$ nix repl
Nix 2.31.2
Type :? for help.

nix-repl> :lf .
Added 26 variables.

nix-repl> :a inputs.nixpkgs.lib
Added 464 variables.

nix-repl> countAttrs = foldlAttrs (count: _system: attrs: count + length (attrNames attrs)) 0

nix-repl> countAttrs checks
760

nix-repl> countAttrs ci.buildbot
8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport: release-25.11 To be backported to the release-25.11 stable branch has: port to stable This has been backported to the latest stable branch topic: flake /flake.nix, /flake.lock, and /flake/ subsystems

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants