Fix CI compile error and unbreak Nix builds from Windows clones - #3
Merged
PhysShell merged 3 commits intoSep 7, 2026
Merged
Conversation
Two defects that keep this branch from working outside the cloud sandbox:
1. `.github/workflows/ci.yml` never compiled. A step's own `shell:` accepts
no contexts at all, so `shell: ${{ matrix.shell }}` made GitHub reject the
whole workflow: run 34096684564 failed in 0s with no jobs, and the workflow
is still registered under its path instead of its `name:`. Job-level
`defaults.run` does accept the matrix context, so the value moves there.
Verified with actionlint, which flags the original and passes the fix.
2. There was no `.gitattributes`. Git for Windows defaults to
core.autocrlf=true, so a Windows clone gets CRLF in every `.nix` file. Nix
embeds those contents verbatim into derivation build scripts, bash then
reads the CR as a command, and the build dies:
qubix-openbox-rc.xml.drv: builder failed with exit code 127
> /build/.attr-...: line 1: $'\r': command not found
That made `nix build .#spotibox-release` impossible from a Windows clone,
and made the "manifest.json matches Nix" check report a false mismatch on
every line. Normalising to LF in the working tree fixes both; the CMD
entry points stay CRLF because cmd.exe needs it for multi-line blocks.
After this, on a Windows host: flake check passes, the release bundle builds,
manifest.json matches Nix, 42/42 controller checks pass under both PowerShell
5.1 and pwsh 7, and PSScriptAnalyzer is clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Audio redirection was silent on Windows for a reason that hides well: nixpkgs
builds xrdp with `--enable-mp3lame` and `--enable-opus`, mstsc then negotiates
WAVE_FORMAT_MPEGLAYER3, and drops the stream. Everything inside the guest looks
healthy while this happens, which is why it reads as a configuration problem
and is not one:
- pulseaudio xrdp-sink loaded
- pulseaudio xrdp-sink set as default
Connected ok fd 17
sound_process_training: round trip time 0
Socket 19: connection accepted from AF_UNIX
The sink is default, unmuted, at 100%, and cycles IDLE -> RUNNING in step with
the track. Only the host is silent, and the Windows mixer shows an mstsc slider
that never moves. chansrv names the culprit outright:
sound_process_output_format:
wFormatNo 2
wFormatTag WAVE_FORMAT_MPEGLAYER3
Dropping both encoders leaves PCM, which every RDP client decodes; ~176 kB/s is
nothing beside the video channel this appliance already pushes.
The override is an overlay rather than `services.xrdp.package`, because that
option does not work: the NixOS module declares it, then hardcodes `pkgs.xrdp`
in the ExecStart of xrdp.service and xrdp-sesman.service. Setting the option
rebuilds confDir only, and the daemons keep running the unmodified build --
confirmed on the host, where the override built as expected and the guest still
executed /nix/store/1fsy7...-xrdp. Worth reporting upstream separately.
Verified end to end on a Windows 11 host with Hyper-V: `nix flake check` passes,
the rebuilt xrdp links neither libmp3lame nor libopus, xrdp-sesman resolves to
the overridden package, chansrv offers WAVE_FORMAT_PCM only with zero
MPEGLAYER3 lines in its log, and audio is audible in the mstsc window.
README gains a Continuous Integration section (triggers, jobs, gh commands,
and the actionlint tip for workflows GitHub refuses to compile) plus a
"Why PCM-only audio" design note.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PhysShell
force-pushed
the
claude/fix-windows-clone-and-ci
branch
from
September 7, 2026 12:35
ee5d9f1 to
9d80ce0
Compare
The overlay here works around a nixpkgs bug, and that bug already has an open fix: NixOS/nixpkgs#452303, "nixos/xrdp: actually use cfg.package parameter", two approvals, unmerged since October 2025. It covers one more call site than this repo had found -- xrdp-keygen in ExecStartPre -- besides the two ExecStart lines. Record the link in both the profile and the design note so that whoever revisits this can drop the overlay for a plain services.xrdp.package assignment once the fix lands, instead of rediscovering why it is an overlay at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PhysShell
merged commit Sep 7, 2026
f803b7f
into
claude/gpu-vm-config-simplify-2iq62m
4 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes found by running this branch on a real Windows 11 host with Hyper-V and WSL, plus the findings from that run.
Fixed here
1. CI never compiled. Run 34096684564 failed in 0 s with no jobs, and the workflow is still registered under its path instead of its
name:— the signature of a workflow GitHub could not parse. Cause: a step's ownshell:accepts no contexts at all, soshell: ${{ matrix.shell }}was rejected. Job-leveldefaults.rundoes acceptmatrix, so the value moves there.actionlintflags the original and passes the fix.2. No
.gitattributes, so Nix could not build from a Windows clone at all. Git for Windows defaults tocore.autocrlf=true; every.nixfile then has CRLF, Nix embeds the contents verbatim into derivation build scripts, and bash chokes:The same CRLF also made the "manifest.json matches Nix" check report a false mismatch on every line. Normalising to LF fixes both;
.cmdstays CRLF becausecmd.exeneeds it for multi-line blocks.3. Audio redirection was silent on Windows. nixpkgs builds xrdp with
--enable-mp3lame/--enable-opus;mstscthen negotiatesWAVE_FORMAT_MPEGLAYER3and drops the stream. The failure hides well, because every check inside the guest is green —xrdp-sinkis default, unmuted, at 100 %, cycles IDLE↔RUNNING in step with the track, and chansrv logsConnected ok fd 17/round trip time 0. Only the host is silent. chansrv names it outright:Dropping both encoders leaves PCM, which every client decodes (~176 kB/s, nothing next to the video channel).
This also uncovered a nixpkgs bug worth reporting upstream:
services.xrdp.packagedoes not work. The module declares the option but hardcodespkgs.xrdpin theExecStartof bothxrdp.serviceandxrdp-sesman.service, so setting it rebuildsconfDironly while the daemons keep running the unmodified build. Measured on the host: the override built as17wn7wzvx4…, and the guest kept executing1fsy7gm3nn…. Hence thenixpkgs.overlaysentry instead.Verified live after the fix:
no mp3/opus linked, chansrv offersWAVE_FORMAT_PCMonly, zeroMPEGLAYER3lines, and audio is audible in the mstsc window.Verified on the host after these fixes
tests/qubixctl.Tests.ps1, Windows PowerShell 5.1tests/qubixctl.Tests.ps1, pwsh 7.6.5Invoke-ScriptAnalyzerontools/,tests/actionlinton both workflowsnix flake checkmanifest.jsonvs.#qubix-manifest-jsonnix build .#spotibox-releasequbixctl -Command buildfrom\wsl.localhost\...recreate+ PCM-only xrdpupon Hyper-Vrecreatekeeps/home/homesurvives; system disk replacedPersistence was checked by mounting the home VHDX in WSL, writing a marker file, running
recreate, and reading the marker back — it survived, byte for byte, and the home VHDX kept its originalCreationTimewhile the system VHDX was replaced.Not fixed here — needs a decision
-ImageSource wslcannot work withuporrecreate. Both callAssert-Administrator, and an elevated process cannot see\wsl.localhost— measured on the host:So the developer loop README documents around line 181 (
& "$Qubix\tools\qubixctl.cmd" -Command recreate -ImageSource wslwith$Qubixunder\wsl.localhost\...) fails immediately. It also breaks the repo-on-C: variant, becauseBuild-QubixImagesInWslhands back\wsl.localhost\NixOS\nix\store\...paths that the elevatedCopy-Itemthen cannot read. A fix probably means staging the images onto a local path from the unelevated side before elevating.qubix-up.cmdsilently drops its arguments when it elevates.Start-Process -FilePath '%~f0' -Verb RunAsis called with no-ArgumentList, soqubix-up.cmd -ImageSource wslfrom a normal console re-launches as a bareupand goes down the release path instead.No release or tag exists yet, so the default
auto→releasesource cannot resolve anything; the live run above had to use-ImageSource file.spotibox.vhdx.gzis 1 953 208 406 bytes against the workflow's own 2 000 000 000 limit — 2.3 % of headroom. The next nixpkgs or Spotify bump is likely to breakrelease.🤖 Generated with Claude Code