-
-
Notifications
You must be signed in to change notification settings - Fork 227
feat: enable nixosTest on aarch64 darwin #1989
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: develop
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis PR migrates many Nix extension tests to the Linux-specific package namespace ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Tester as pkgs.testers.runNixOSTest
participant VM as NixOS VM
participant Systemd as systemd
participant Postgres as PostgreSQL (pkg with PostGIS)
participant PSQL as psql (test script)
Tester->>VM: spawn test VM with nodes.server config
VM->>Systemd: bring up services (sshd, postgresql)
Systemd->>Postgres: enable/start PostgreSQL with PostGIS package
Tester->>PSQL: invoke testScript (psql commands)
PSQL->>Postgres: execute SQL (install/check/upgrade extension)
Postgres-->>PSQL: return results (extension versions)
PSQL-->>Tester: assert expected versions and upgrade outcomes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6dba03c to
307aa73
Compare
acf3ba4 to
42b0c29
Compare
Running NixOS tests on macOS requires a Linux VM capable of building
NixOS VMs. This adds a nix-darwin configuration that provisions an
ephemeral QEMU linux-builder with support for aarch64-linux and
x86_64-linux builds.
Usage:
nix run github:supabase/postgres#setup-darwin-linux-builder
Enables running NixOS integration tests from macOS development machines
(see #1989).
Running NixOS tests on macOS requires a Linux VM capable of building
NixOS VMs. This adds a nix-darwin configuration that provisions an
ephemeral QEMU linux-builder with support for aarch64-linux and
x86_64-linux builds.
Usage:
nix run github:supabase/postgres#setup-darwin-linux-builder
Enables running NixOS integration tests from macOS development machines
(see #1989).
Running NixOS tests on macOS requires a Linux VM capable of building
NixOS VMs. This adds a nix-darwin configuration that provisions an
ephemeral QEMU linux-builder with support for aarch64-linux and
x86_64-linux builds.
Usage:
nix run github:supabase/postgres#setup-darwin-linux-builder
Enables running NixOS integration tests from macOS development machines
(see #1989).
Running NixOS tests on macOS requires a Linux VM capable of building
NixOS VMs. This adds a nix-darwin configuration that provisions an
ephemeral QEMU linux-builder with support for aarch64-linux and
x86_64-linux builds.
Usage:
nix run github:supabase/postgres#setup-darwin-linux-builder
Enables running NixOS integration tests from macOS development machines
(see #1989).
fa26b67 to
0131d17
Compare
Running NixOS tests on macOS requires a Linux VM capable of building
NixOS VMs. This adds a nix-darwin configuration that provisions an
ephemeral QEMU linux-builder with support for aarch64-linux and
x86_64-linux builds.
Usage:
nix run github:supabase/postgres#setup-darwin-linux-builder
Enables running NixOS integration tests from macOS development machines
(see #1989).
fdd545e to
6953768
Compare
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
nix/ext/tests/pg_plan_filter.nix (1)
62-66: Change installedExtension version to match PostgreSQL 17 in line 65.Line 65 uses
(installedExtension "15").defaultSettingsfor the PostgreSQL 17 configuration, but should use"17"to match the version being configured. This is inconsistent with correct patterns inpgjwt.nixline 87 anddefault.nixline 94, which both use(installedExtension "17").defaultSettingsfor their postgresql17 specialisations.nix/ext/tests/http.nix (2)
5-9: Inconsistent host platform reference will break Darwin builds.
installedExtensionusespkgs.stdenv.hostPlatform.systemwhile other parts of this file (lines 15, 30, 62, 79, 82) usepkgs.pkgsLinux.stdenv.hostPlatform.system. On Darwin, this resolves toaarch64-darwinvsaarch64-linux, causing a mismatch where the extension lookup targets Darwin packages but the build environment is Linux.Other test files in this PR (e.g.,
pg_repack.nix,plpgsql_check.nix,pgmq.nix) consistently usepkgs.pkgsLinuxfor this lookup.Proposed fix
installedExtension = postgresMajorVersion: - self.legacyPackages.${pkgs.stdenv.hostPlatform.system}."psql_${postgresMajorVersion}".exts."${ + self.legacyPackages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}."psql_${postgresMajorVersion}".exts."${ pname }";
54-57: Inconsistent host platform reference for PostgreSQL 15.Line 56 uses
pkgs.stdenv.hostPlatform.systemwhile the PostgreSQL 17 reference at line 62 usespkgs.pkgsLinux.stdenv.hostPlatform.system. This inconsistency will cause version mismatch issues on Darwin hosts.Proposed fix
services.postgresql = { enable = true; - package = postgresqlWithExtension self.packages.${pkgs.stdenv.hostPlatform.system}.postgresql_15; + package = postgresqlWithExtension self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}.postgresql_15; };nix/ext/tests/default.nix (1)
13-38: AligninstalledExtensionto the Linux package set to avoid platform mismatch.
psql_15andpsql_17resolve packages frompkgs.pkgsLinux.stdenv.hostPlatform.system, butinstalledExtensionusespkgs.stdenv.hostPlatform.system. On aarch64‑darwin, this resolves to the Darwin platform while the extensions are then mixed into a LinuxbuildEnv(line 23), causing cross-platform package incompatibilities. Change line 15 to usepkgs.pkgsLinux.stdenv.hostPlatform.systemto keep all lookups within the same package set.🧩 Proposed fix
- self.legacyPackages.${pkgs.stdenv.hostPlatform.system}."psql_${postgresMajorVersion}".exts."${ + self.legacyPackages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}."psql_${postgresMajorVersion}".exts."${ pname }";
🤖 Fix all issues with AI agents
In `@nix/ext/tests/pgroonga.nix`:
- Around line 80-85: MECAB_CONFIG is set to the host-platform package pkgs.mecab
which can inject a Darwin binary into the Linux systemd service; change the
assignment to use the Linux-target package like the other vars (MECAB_DICDIR and
GRN_PLUGINS_DIR) by replacing pkgs.mecab with pkgs.pkgsLinux.mecab so
MECAB_CONFIG references the Linux mecab binary; update the
environment.MECAB_CONFIG definition to use pkgs.pkgsLinux.mecab/bin/mecab-config
to match the pattern used for GRN_PLUGINS_DIR and the mecab dictionary path.
🧹 Nitpick comments (1)
nix/ext/tests/plv8.nix (1)
58-62: Consider using the predefinedpsql_15variable.The
psql_15variable is already defined at lines 45-47 with the same expression. Other test files (e.g.,pg_plan_filter.nixline 58) use the predefined variable directly:package = psql_15;This would reduce redundancy and align with the PR objective of removing redundant
postgresqlWithExtensionwrappers.♻️ Suggested change
services.postgresql = { enable = true; - package = - postgresqlWithExtension - self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}.postgresql_15; + package = psql_15; authentication = ''
Running NixOS tests on macOS requires a Linux VM capable of building
NixOS VMs. This adds a nix-darwin configuration that provisions an
ephemeral QEMU linux-builder with support for aarch64-linux and
x86_64-linux builds.
Usage:
nix run github:supabase/postgres#setup-darwin-linux-builder
Enables running NixOS integration tests from macOS development machines
(see #1989).
6953768 to
3e60368
Compare
Build nixosTest using a local aarch64 linux builder and run nixosTest qemu VM in aarch64 darwin. This keeps the tests fast as they don't require nested virtualisation. For example, you can run the nixos test for the http extension on your aarch64 macOS with: ```shell nix build .\#checks.aarch64-darwin.ext-http ```
Current system value has been moved into `pkgs.stdenv.hostPlatform` by NixOS/nixpkgs#456527
The tests were unnecessarily wrapping psql_15 with postgresqlWithExtension when psql_15 already includes the extensions.
This cause problem starting nixos tests on darwin hosts.
3e60368 to
3ff13fd
Compare
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
nix/ext/tests/default.nix (3)
35-36: Inconsistent platform reference for OrioleDB extension.Line 36 uses
pkgs.stdenv.hostPlatform.systemwhile the rest of the file consistently usespkgs.pkgsLinux.stdenv.hostPlatform.system. On Darwin hosts, this would resolve toaarch64-darwininstead ofaarch64-linux, likely causing the OrioleDB extension lookup to fail.Compare with
pgrouting.nixline 25 which correctly usespkgs.pkgsLinux.stdenv.hostPlatform.systemfor the same OrioleDB reference.🐛 Proposed fix
++ lib.optional (postgresql.isOrioleDB - ) self.legacyPackages.${pkgs.stdenv.hostPlatform.system}.psql_orioledb-17.exts.orioledb; + ) self.legacyPackages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}.psql_orioledb-17.exts.orioledb;
147-151: Inconsistent platform reference in orioledb17 specialisation.Same pattern issue: should use
pkgs.pkgsLinux.stdenv.hostPlatform.system.🐛 Proposed fix
specialisation.orioledb17.configuration = { services.postgresql = { package = lib.mkForce ( - postgresqlWithExtension self.packages.${pkgs.stdenv.hostPlatform.system}.postgresql_orioledb-17 + postgresqlWithExtension self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}.postgresql_orioledb-17 );
184-189: Inconsistent platform reference in orioledb17 migration script.Same pattern issue: should use
pkgs.pkgsLinux.stdenv.hostPlatform.system.🐛 Proposed fix
script = let newPostgresql = postgresqlWithExtension - self.packages.${pkgs.stdenv.hostPlatform.system}.postgresql_orioledb-17; + self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}.postgresql_orioledb-17; innix/ext/tests/http.nix (1)
23-25: Replace allpkgs.stdenv.hostPlatform.systemwithpkgs.pkgsLinux.stdenv.hostPlatform.systemfor consistency.The file mixes
pkgs.stdenv.hostPlatform.systemwithpkgs.pkgsLinux, which can resolve to Darwin packages when run on aarch64-darwin hosts. Since this is a Linux NixOS test, all package references must usepkgs.pkgsLinuxto ensure only Linux packages are selected. This pattern is already established elsewhere in the file (line 7, 16, 33, 66) and consistently used in other test files (vault.nix, timescaledb.nix, postgis.nix, plv8.nix).Fix locations
- Line 24:
self.legacyPackages.${pkgs.stdenv.hostPlatform.system}→self.legacyPackages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}- Lines 47–48:
self.packages.${pkgs.stdenv.hostPlatform.system}→self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}- Line 51:
self.packages.${pkgs.stdenv.hostPlatform.system}→self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}- Line 147:
self.packages.${pkgs.stdenv.hostPlatform.system}→self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}- Line 189:
self.packages.${pkgs.stdenv.hostPlatform.system}→self.packages.${pkgs.pkgsLinux.stdenv.hostPlatform.system}
🤖 Fix all issues with AI agents
In `@nix/ext/tests/default.nix`:
- Around line 65-67: The orioledb_17 package definition uses an inconsistent
platform reference: change the platform expression used in the call to
postgresqlWithExtension for the symbol orioledb_17 from
pkgs.stdenv.hostPlatform.system to match psql_15/psql_17 by using
pkgs.pkgsLinux.stdenv.hostPlatform.system; update the RHS of orioledb_17 so it
references the same pkgs.pkgsLinux path as the other psql_* entries to restore
consistency.
In `@nix/ext/tests/postgis.nix`:
- Around line 139-156: The tests currently only assert the major version by
computing majMinVersion and checking f"{pname},{majMinVersion}" in
installed_extensions; update both checks (the first block using versions["15"]
and the upgrade block using versions["17"]) to compare the full latestVersion
string instead of slicing it—use latestVersion directly in the assertion (e.g.,
assert f"{pname},{latestVersion}" in installed_extensions) and remove the
majMinVersion computation so the test verifies the exact full extension version
installed; refer to variables latestVersion, versions, pname, and
installed_extensions to locate the changes.
| with subtest("Check ${pname} latest extension version"): | ||
| server.succeed("sudo -u postgres psql -c 'DROP EXTENSION ${pname};'") | ||
| server.succeed("sudo -u postgres psql -c 'CREATE EXTENSION ${pname} CASCADE;'") | ||
| installed_extensions=run_sql(r"""SELECT extname, extversion FROM pg_extension where extname = '${pname}';""") | ||
| latestVersion = versions["15"][-1] | ||
| majMinVersion = ".".join(latestVersion.split('.')[:1]) | ||
| assert f"${pname},{majMinVersion}" in installed_extensions, f"Expected ${pname} version {latestVersion}, but found {installed_extensions}" | ||
|
|
||
| with subtest("switch to postgresql 17"): | ||
| server.succeed( | ||
| "${pg17-configuration}/bin/switch-to-configuration test >&2" | ||
| ) | ||
|
|
||
| with subtest("Check ${pname} latest extension version after upgrade"): | ||
| installed_extensions=run_sql(r"""SELECT extname, extversion FROM pg_extension;""") | ||
| latestVersion = versions["17"][-1] | ||
| majMinVersion = ".".join(latestVersion.split('.')[:1]) | ||
| assert f"${pname},{majMinVersion}" in installed_extensions |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PostGIS extension definitions and version specifications
echo "=== Searching for PostGIS-related nix files ==="
fd -t f -g '*postgis*' nix 2>/dev/null || echo "No postgis files found with pattern"
echo ""
echo "=== Searching for version definitions in nix/ext ==="
fd -t f '\.nix$' nix/ext | head -20
echo ""
echo "=== Looking for postgis in ext directory ==="
find nix/ext -type f -name '*.nix' | xargs grep -l "postgis" 2>/dev/null | head -10Repository: supabase/postgres
Length of output: 885
🏁 Script executed:
#!/bin/bash
# Look at the actual test file to see full context
if [ -f "nix/ext/tests/postgis.nix" ]; then
echo "=== Full postgis test file ==="
wc -l nix/ext/tests/postgis.nix
cat -n nix/ext/tests/postgis.nix | head -50
fiRepository: supabase/postgres
Length of output: 1912
🏁 Script executed:
#!/bin/bash
# Search for version definitions, especially around line 139-156
if [ -f "nix/ext/tests/postgis.nix" ]; then
echo "=== Context around lines 139-156 ==="
sed -n '120,170p' nix/ext/tests/postgis.nix | cat -n
fiRepository: supabase/postgres
Length of output: 2591
🏁 Script executed:
#!/bin/bash
# Search for how versions dict is populated
if [ -f "nix/ext/tests/postgis.nix" ]; then
echo "=== Search for 'versions' variable definition ==="
rg -n "versions\s*=" nix/ext/tests/postgis.nix -A 5
fiRepository: supabase/postgres
Length of output: 618
🏁 Script executed:
#!/bin/bash
# Look at postgis extension definition to see version format
if [ -f "nix/ext/postgis.nix" ]; then
echo "=== PostGIS extension definition ==="
cat nix/ext/postgis.nix | head -150
fiRepository: supabase/postgres
Length of output: 5043
🏁 Script executed:
#!/bin/bash
# Search for version information in postgis package
rg -n "version" nix/ext/postgis.nix -C 3Repository: supabase/postgres
Length of output: 5181
🏁 Script executed:
#!/bin/bash
# Check versions.json to see actual version formats
if [ -f "nix/ext/versions.json" ]; then
echo "=== Checking versions.json for PostGIS versions ==="
jq '.postgis | keys' nix/ext/versions.json 2>/dev/null | head -20
fiRepository: supabase/postgres
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Get a sample of PostGIS versions to see format
if [ -f "nix/ext/versions.json" ]; then
jq '.postgis | keys | .[0:10]' nix/ext/versions.json 2>/dev/null
fiRepository: supabase/postgres
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Check if there are multiple major versions in the versions.json
jq '.postgis | keys | map(split(".")[0]) | unique' nix/ext/versions.jsonRepository: supabase/postgres
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Get all PostGIS versions with their majors and minors
jq '.postgis | keys | map(. as $v | {version: $v, major: ($v | split(".")[0]), majorMinor: ($v | split(".") | .[0:2] | join("."))}) | sort_by(.version)' nix/ext/versions.jsonRepository: supabase/postgres
Length of output: 213
Tighten the version assertion to verify the full latest version, not just major.
The current check only validates the major version ("3"), which is insufficient. Even the suggested major.minor check ("3.3") wouldn't distinguish between versions "3.3.2" and "3.3.7". The variables latestVersion suggest intent to verify the latest version is installed, so compare the full version string instead:
Proposed fix
- latestVersion = versions["15"][-1]
- majMinVersion = ".".join(latestVersion.split('.')[:1])
- assert f"${pname},{majMinVersion}" in installed_extensions, f"Expected ${pname} version {latestVersion}, but found {installed_extensions}"
+ latestVersion = versions["15"][-1]
+ assert f"${pname},{latestVersion}" in installed_extensions, f"Expected ${pname} version {latestVersion}, but found {installed_extensions}"
...
- latestVersion = versions["17"][-1]
- majMinVersion = ".".join(latestVersion.split('.')[:1])
- assert f"${pname},{majMinVersion}" in installed_extensions
+ latestVersion = versions["17"][-1]
+ assert f"${pname},{latestVersion}" in installed_extensions🤖 Prompt for AI Agents
In `@nix/ext/tests/postgis.nix` around lines 139 - 156, The tests currently only
assert the major version by computing majMinVersion and checking
f"{pname},{majMinVersion}" in installed_extensions; update both checks (the
first block using versions["15"] and the upgrade block using versions["17"]) to
compare the full latestVersion string instead of slicing it—use latestVersion
directly in the assertion (e.g., assert f"{pname},{latestVersion}" in
installed_extensions) and remove the majMinVersion computation so the test
verifies the exact full extension version installed; refer to variables
latestVersion, versions, pname, and installed_extensions to locate the changes.
- self.legacyPackages.${pkgs.stdenv.hostPlatform.system} -> pkgs.pkgsLinux.stdenv.hostPlatform.system
- self.packages.${pkgs.stdenv.hostPlatform.system} -> pkgs.pkgsLinux.stdenv.hostPlatform.system
17a9668 to
d8039b8
Compare
Build nixosTest using a local aarch64 linux builder and run nixosTest qemu VM in
aarch64 darwin. This keeps the tests fast as they don't require nested virtualisation.
For example, you can run the nixos test for the http extension on your aarch64 macOS with:
nix build .\#checks.aarch64-darwin.ext-httpSummary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.