-
Notifications
You must be signed in to change notification settings - Fork 995
feat(desktop): add Windows build to CI/CD pipeline #1617
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
Closed
Sandman-Ren
wants to merge
14
commits into
superset-sh:main
from
Sandman-Ren:feat/windows-desktop-build
Closed
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b4ca5f2
feat(desktop): add Windows build to CI/CD pipeline
Sandman-Ren c842512
fix(desktop): skip postinstall on Windows build
Sandman-Ren f6e8d3d
fix(desktop): make postinstall cross-platform
Sandman-Ren 8cf7432
fix(desktop): handle Windows junctions in copy-native-modules
Sandman-Ren ceffac8
fix(desktop): use correct Windows manifest filename
Sandman-Ren d59d939
fix(desktop): disable GPU hardware acceleration on Windows to prevent…
Sandman-Ren 938a131
fix(desktop): strip crossorigin attr to fix black screen in ASAR builds
Sandman-Ren 5b1d2a4
chore(desktop): add diagnostic logging for black screen debugging
Sandman-Ren 1d0ac11
fix(desktop): use || instead of ?? in defineEnv for empty string fall…
Sandman-Ren 19bcfab
Merge branch 'superset-sh:main' into feat/windows-desktop-build
Sandman-Ren e0071d4
chore(desktop): address PR review feedback
Sandman-Ren 9669a90
Merge branch 'superset-sh:main' into feat/windows-desktop-build
Sandman-Ren bf257f4
chore(desktop): gate Windows-specific changes behind platform checks
Sandman-Ren 56bf5f6
fix(ci): enable long paths for Windows git checkout
Sandman-Ren File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| /** | ||
| * Cross-platform postinstall script. | ||
| * | ||
| * Replaces the bash-only postinstall.sh so that `bun install` works on | ||
| * Windows, macOS and Linux without special flags. | ||
| * | ||
| * Steps: | ||
| * 1. Guard against infinite recursion (electron-builder install-app-deps | ||
| * can trigger nested bun installs which would re-run this script). | ||
| * 2. Run sherif for workspace validation. | ||
| * 3. Install native dependencies for the desktop app. | ||
| */ | ||
|
|
||
| import { execSync } from "node:child_process"; | ||
|
|
||
| // Prevent infinite recursion during postinstall | ||
| if (process.env.SUPERSET_POSTINSTALL_RUNNING) { | ||
| process.exit(0); | ||
| } | ||
| process.env.SUPERSET_POSTINSTALL_RUNNING = "1"; | ||
|
|
||
| /** Run a command, inheriting stdio so output is visible. */ | ||
| function run(cmd) { | ||
| execSync(cmd, { stdio: "inherit" }); | ||
| } | ||
|
|
||
| // Run sherif for workspace validation | ||
| run("sherif"); | ||
|
|
||
| // Install native dependencies for desktop app | ||
| run("bun run --filter=@superset/desktop install:deps"); |
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.
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.
process.platformis evaluated at build time — cross-compilation silently skips the fix.process.platform !== "win32"runs in the Vite/Node.js process during the build, not inside the packaged Electron app. This works correctly when the CI runner iswindows-latest, but if a Windows artifact is ever produced from a macOS or Linux host (cross-compilation), thecrossoriginstripping will be silently skipped, causing the black-screen regression to reappear with no indication.Consider asserting the intent in a comment, or pass the target platform explicitly via an env var so the build graph doesn't couple correctness to the build host:
💡 Proposed hardening
export function stripCrossOriginPlugin(): Plugin { return { name: "strip-crossorigin", transformIndexHtml: { order: "post", handler(html) { - if (process.platform !== "win32") return html; + // NOTE: evaluated at Vite build time, not Electron runtime. + // For cross-compilation, set TARGET_PLATFORM=win32 explicitly. + const platform = process.env.TARGET_PLATFORM ?? process.platform; + if (platform !== "win32") return html; return html.replace(/ crossorigin(?:="[^"]*")?/g, ""); }, }, }; }The CI workflow would then pass
TARGET_PLATFORM=win32when invoking the Windows build step, making the condition explicit and immune to the build-host assumption.📝 Committable suggestion
🤖 Prompt for AI Agents