Initial Darwin support (except agent)#140
Conversation
It's not needed and can increase build time depending on what's in `environment.systemPackages` (e.g. Python).
| let bavail = stat.blocks_available().saturating_mul(block_size); | ||
| let bfree = stat.blocks_free().saturating_mul(block_size); | ||
| let btotal = stat.blocks().saturating_mul(block_size); | ||
| let bavail = stat.blocks_available().saturating_mul(block_size.try_into().map_err(|e| CiError::Io(std::io::Error::other(e)))?); |
There was a problem hiding this comment.
I’m going to hope these get elided away on Linux (where block_size is the same type as stat.blocks_available()).
| vmTests | ||
| filteredNixosTests | ||
| // { | ||
| nixos-module-agent-package = nixosModuleAgentPackage; |
There was a problem hiding this comment.
Why is there both this and the completely different circus-agent definition above?
There was a problem hiding this comment.
This was introduced by @amaanq, iirc for the purpose of easing builds for exotic arches earlier.
| inherit (lib.modules) mkDefault mkForce; | ||
| circus-packages = self.packages.${pkgs.stdenv.hostPlatform.system}; | ||
| in { | ||
| documentation.enable = false; |
There was a problem hiding this comment.
I was under the impression that this is done in runNixOSTest? Haven't traced too carefully, but looks like nodes.nix in nixos/lib/testing already sets documentation.info.enable = lib.mkDefault false;
There was a problem hiding this comment.
I don’t think those two options are equivalent — I saw Python docs being built for the one test that had it in its environment.systemPackages.
I can look into this further later.
amaanq
left a comment
There was a problem hiding this comment.
left some comments, sorry for taking a while to get to this
| let bavail = stat.blocks_available().saturating_mul(block_size.try_into().map_err(|e| CiError::Io(std::io::Error::other(e)))?); | ||
| let bfree = stat.blocks_free().saturating_mul(block_size.try_into().map_err(|e| CiError::Io(std::io::Error::other(e)))?); | ||
| let btotal = stat.blocks().saturating_mul(block_size.try_into().map_err(|e| CiError::Io(std::io::Error::other(e)))?); |
There was a problem hiding this comment.
fsblkcnt_t is c_uint (aka u32) on Darwin, so we should probably widen this to u64 before multiplying because it will saturate at 4GB. maybe a helper widen fn like so:
fn widen(n: impl Into<u64>) -> u64 {
n.into()
}can be used to wrap the statvfs values before multiplying.
| postgresql_18 | ||
|
|
||
| # circus-evaluator builds evix's Nix C bindings. | ||
| # nixVersions.nix_2_34.dev |
There was a problem hiding this comment.
Can we uncomment this? nix_2_34.dev evaluates fine on darwin
There was a problem hiding this comment.
Nix 2.34.8 was tagged earlier, I'll be able to make a 0.2348.0 release for the Nix bindings, which'll include better Darwin support per NotAShelf/nix-bindings#73 is not yet updated, but we can handle it once I tag evix 2.0 and update in #133
| adminPasswordFile = pkgs.writeText "admin-password" "AdminPassword123!"; | ||
| demoPasswordFile = pkgs.writeText "demo-password" "DemoPassword123!"; |
There was a problem hiding this comment.
Can we update this to also use builtins.toFile?
| // lib.optionalAttrs (muslCrossAttr ? ${system}) { | ||
| circus-agent = callCratePackage ./nix/packages/circus-agent.nix "circus-agent" "--package circus-agent"; |
There was a problem hiding this comment.
with this, we lose the agent on systems like powerpc64le and s390x, gating on it being in the muslCrossAttr is not the right choice here. i think we can just gate circus-agent on stdenv.hostPlatform.isLinux no? and keep the muslCrossAttr check just for circus-agent-static.
| virtualisation = { | ||
| memorySize = mkForce 4096; | ||
| cores = mkForce 4; |
There was a problem hiding this comment.
we should set virtualisation.host.pkgs to the outer package set, because NixOS guest evaluation selects Linux packages on Darwin. you could declare an outer hostPkgs = pkgs and then declare host.pkgs = hostPkgs
Please review commit-by-commit, I think everything is split up decently well enough.
I haven't run the container tests as
nix flake checkis bad and doesn't let you skip incompatible derivations with--keep-going.