Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new windsurf-next package variant (experimental “Next” channel) alongside existing windsurf, including an auto-update script and pinned per-platform source metadata. It also includes a Linux-specific workaround for upstream renaming code.png → code-next.png, which otherwise breaks buildVscode’s icon-copy step.
Changes:
- Introduce
pkgs/by-name/wi/windsurf-nextwithpackage.nixand pinnedinfo.jsonfor supported platforms. - Add an update script (
update.mts) and minimal TS tooling config underwindsurf-next/update/to refreshinfo.json. - Override the Linux
installPhaseto adjust the icon filename expected bybuildVscode’s generic installer logic.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkgs/by-name/wi/windsurf-next/package.nix | Defines the new windsurf-next package using buildVscode and applies a Linux icon-name workaround via overrideAttrs. |
| pkgs/by-name/wi/windsurf-next/info.json | Pins per-platform version, VS Code base version, download URLs, and hashes for the “Next” channel. |
| pkgs/by-name/wi/windsurf-next/update/update.mts | Fetches latest metadata from the upstream update API and rewrites info.json when versions change. |
| pkgs/by-name/wi/windsurf-next/update/package.json | Dev-only tooling definition to run the update script locally. |
| pkgs/by-name/wi/windsurf-next/update/tsconfig.json | TypeScript configuration for validating the update script. |
| pkgs/by-name/wi/windsurf-next/update/.gitignore | Ignores local Node tooling artifacts for the update script directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }).overrideAttrs (oldAttrs: { | ||
| installPhase = if stdenv.hostPlatform.isLinux then | ||
| (builtins.replaceStrings | ||
| [ ''icon_file="$out/lib/windsurf-next/resources/app/resources/linux/code.png"'' ] | ||
| [ ''icon_file="$out/lib/windsurf-next/resources/app/resources/linux/code-next.png"'' ] | ||
| oldAttrs.installPhase) | ||
| else | ||
| oldAttrs.installPhase; | ||
| }) |
There was a problem hiding this comment.
The Linux installPhase override relies on builtins.replaceStrings against the full generated installPhase string from buildVscode/generic.nix. This is quite brittle (any upstream change to that line/whitespace/path will make the replacement fail silently and reintroduce the original build failure). It also hardcodes the windsurf-next library path in the match/replacement, so future refactors of libraryName would require updating two places. Consider overriding the icon-copying logic more directly (e.g., by replacing the icon line with a small shell conditional that selects code-next.png if present, otherwise code.png), and avoid hardcoding the library path so the override stays resilient.
| }).overrideAttrs (oldAttrs: { | |
| installPhase = if stdenv.hostPlatform.isLinux then | |
| (builtins.replaceStrings | |
| [ ''icon_file="$out/lib/windsurf-next/resources/app/resources/linux/code.png"'' ] | |
| [ ''icon_file="$out/lib/windsurf-next/resources/app/resources/linux/code-next.png"'' ] | |
| oldAttrs.installPhase) | |
| else | |
| oldAttrs.installPhase; | |
| }) | |
| }).overrideAttrs (oldAttrs: | |
| let | |
| libraryPath = "$out/lib/${oldAttrs.libraryName}"; | |
| in | |
| { | |
| installPhase = if stdenv.hostPlatform.isLinux then | |
| (builtins.replaceStrings | |
| [ ''icon_file="${libraryPath}/resources/app/resources/linux/code.png"'' ] | |
| [ | |
| '' | |
| icon_dir="${libraryPath}/resources/app/resources/linux" | |
| if [ -f "$icon_dir/code-next.png" ]; then | |
| icon_file="$icon_dir/code-next.png" | |
| else | |
| icon_file="$icon_dir/code.png" | |
| fi | |
| '' | |
| ] | |
| oldAttrs.installPhase) | |
| else | |
| oldAttrs.installPhase; | |
| }) |
| longName = "Windsurf Next"; | ||
| shortName = "windsurf-next"; | ||
| libraryName = "windsurf-next"; | ||
| iconName = "windsurf-next"; | ||
|
|
||
| sourceRoot = if stdenv.hostPlatform.isDarwin then "Windsurf - Next.app" else "Windsurf"; | ||
| sourceExecutableName = if stdenv.hostPlatform.isDarwin then "Windsurf - Next" else "windsurf-next"; |
There was a problem hiding this comment.
longName is set to "Windsurf Next", but the upstream bundle/executable naming you reference elsewhere in this derivation is "Windsurf - Next" (e.g., sourceRoot / sourceExecutableName, and the update script log messages). This will rename the macOS app bundle directory to "Windsurf Next.app" and the desktop entry to "Windsurf Next", which is inconsistent with the actual product name and may confuse users. Consider aligning longName (and, if needed, shortName) with the upstream "Windsurf - Next" naming to keep the app/desktop naming consistent across platforms.
|
|
||
| src = fetchurl { inherit (info) url sha256; }; | ||
|
|
||
| tests = nixosTests.vscodium; |
There was a problem hiding this comment.
buildVscode exposes the provided tests via passthru.tests and (in most call sites) expects an attribute set of tests. Here tests is set to a single derivation (nixosTests.vscodium), which is inconsistent with the common pattern (e.g. pkgs/applications/editors/vscode/vscode.nix uses tests = { inherit (nixosTests) vscode-remote-ssh; };). Consider changing this to an attrset (e.g. { inherit (nixosTests) vscodium; }) so downstream tooling can discover/compose tests consistently.
| tests = nixosTests.vscodium; | |
| tests = { inherit (nixosTests) vscodium; }; |
I've copied the existing Windsurf flow and added support for Windsurf-next (which has experimental features).
Also, for some reason, Windsurf changes the code.png to code-next.png on linux, which breaks buildVscode's generix.nix's build step.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.