Skip to content

fix windows native uvx#1775

Merged
Kvadratni merged 1 commit intomainfrom
mnovich/fix-windows-uvx
Mar 24, 2025
Merged

fix windows native uvx#1775
Kvadratni merged 1 commit intomainfrom
mnovich/fix-windows-uvx

Conversation

@Kvadratni
Copy link
Contributor

a bit of build time shenanigans to fix uvx for windows.

@Kvadratni
Copy link
Contributor Author

Kvadratni commented Mar 21, 2025

complete line by line analysis
Now that I've examined all the changes, let me explain each modification line by line:

Commit: "fix windows native uvx"

1. ui/desktop/forge.config.ts

+const { resolve } = require('path');
  • Added an import for the resolve function from Node.js path module, which will be used to resolve file paths correctly.
-            target: 'main',
-            target: 'preload',
  • Removed the explicit target properties from the Vite build configuration for both main and preload processes. This change allows the custom build process to handle these targets differently.

2. ui/desktop/package.json

-    "bundle:windows": "npm run make -- --platform=win32 --arch=x64 && node scripts/copy-windows-dlls.js",
+    "bundle:windows": "node scripts/build-main.js && npm run make -- --platform=win32 --arch=x64 && node scripts/copy-windows-dlls.js",
  • Modified the Windows bundling script to run a new build-main.js script before the standard build process. This script specifically handles building the main process for Windows.

3. ui/desktop/scripts/build-main.js (New File)

This is a completely new file that handles building the main process specifically:

  • Creates a custom build script using Vite's API directly
  • Sets up the output directory at .vite/build
  • Configures the build to:
    • Use CommonJS format (cjs) which is more compatible with Electron
    • Preserve the entry filename as main.js
    • Explicitly marks Node.js and Electron modules as external dependencies
    • Uses server-side rendering mode (ssr: true) which is appropriate for Electron's main process

4. ui/desktop/src/main.ts

+// Handle binary path requests
+ipcMain.handle('get-binary-path', (event, binaryName) => {
+  return getBinaryPath(app, binaryName);
+});
  • Added a new IPC handler that allows the renderer process to request the path to binary executables (like uvx) through a safe channel. This enables the renderer to know where to find binaries without having to implement the path resolution logic again.

5. ui/desktop/src/utils/binaryPath.ts

-  // On Windows, use .cmd for npx and .exe for uvx
  • Removed a comment that was no longer needed or accurate.
-    // In development, check multiple possible locations
-    // In production, check resources paths
-  // Log all paths we're checking
-  log.info('Checking binary paths:', possiblePaths);
-        log.info(`Found binary at: ${binPath}`);
  • Removed several logging and comment lines to clean up the code.
+    if (isWindows) {
+      possiblePaths.push(
+        path.join(process.resourcesPath, executableName),
+        path.join(app.getAppPath(), 'resources', executableName),
+        path.join(app.getPath('exe'), '..', 'bin', executableName)
+      );
+    }
  • Added Windows-specific paths to check for binaries. This is crucial for the fix, as it adds three new possible locations where the uvx.exe binary might be found on Windows:
    1. Directly in the resources folder
    2. In the app's resources folder
    3. In a 'bin' folder next to the executable
-  const error = `Could not find ${binaryName} binary in any of the expected locations: ${possiblePaths.join(', ')}`;
-  log.error(error);
-  throw new Error(error);
+  throw new Error(
+    `Could not find ${binaryName} binary in any of the expected locations: ${possiblePaths.join(', ')}`
+  );
  • Simplified the error handling code by directly throwing the error without first logging it separately.

6. ui/desktop/vite.preload.config.ts

-export default defineConfig({});
+export default defineConfig({
+  build: {
+    ssr: true,
+    outDir: '.vite/build',
+    rollupOptions: {
+      input: 'src/preload.ts',
+      output: {
+        format: 'cjs',
+        entryFileNames: 'preload.js'
+      },
+      external: ['electron']
+    }
+  }
+});
  • Completely rewrote the Vite configuration for the preload script to:
    • Use server-side rendering mode (ssr: true)
    • Set the output directory to .vite/build
    • Configure Rollup to use CommonJS format
    • Specify the entry point and output filename
    • Mark Electron as an external dependency

Summary of Changes

This commit fixes issues with the Windows native uvx binary by:

  1. Adding additional search paths for Windows-specific binary locations
  2. Creating a custom build script for the main process that ensures proper compatibility with Windows
  3. Adding an IPC handler to allow the renderer process to safely request binary paths
  4. Updating the Vite configurations to better support the Windows build process
  5. Cleaning up unnecessary logging and comments
  6. Ensuring consistent file output formats and locations

The primary goal was to fix the Windows native uvx executable discovery, which is critical for extension functionality on Windows platforms.

@Kvadratni
Copy link
Contributor Author

How These Changes Affect Other Platforms

Great question! Let's analyze how these changes to fix the Windows native UVX functionality might impact other platforms like macOS and Linux.

Cross-Platform Impact Analysis

1. Build Process Changes

// In package.json
-    "bundle:windows": "npm run make -- --platform=win32 --arch=x64 && node scripts/copy-windows-dlls.js",
+    "bundle:windows": "node scripts/build-main.js && npm run make -- --platform=win32 --arch=x64 && node scripts/copy-windows-dlls.js",

Impact on other platforms:

  • Minimal impact - This change only affects the Windows-specific build script (bundle:windows). The macOS and Linux build processes remain untouched as they use different npm scripts (bundle:default and bundle:intel for macOS).
  • The new build-main.js script is only executed during Windows builds, not during builds for other platforms.

2. Binary Path Resolution

+    if (isWindows) {
+      possiblePaths.push(
+        path.join(process.resourcesPath, executableName),
+        path.join(app.getAppPath(), 'resources', executableName),
+        path.join(app.getPath('exe'), '..', 'bin', executableName)
+      );
+    }

Impact on other platforms:

  • No impact - The additional search paths are wrapped in an isWindows conditional check, so these paths are only added when running on Windows.
  • macOS and Linux will continue to use their existing search paths without any changes.

3. IPC Handler Addition

+// Handle binary path requests
+ipcMain.handle('get-binary-path', (event, binaryName) => {
+  return getBinaryPath(app, binaryName);
+});

Impact on other platforms:

  • Positive impact - This is a platform-agnostic improvement that benefits all operating systems.
  • The new IPC handler provides a consistent way for the renderer process to request binary paths regardless of platform.
  • This actually improves cross-platform compatibility by centralizing the binary path resolution logic.

4. Vite Configuration Changes

// In vite.preload.config.ts
-export default defineConfig({});
+export default defineConfig({
+  build: {
+    ssr: true,
+    outDir: '.vite/build',
+    rollupOptions: {
+      input: 'src/preload.ts',
+      output: {
+        format: 'cjs',
+        entryFileNames: 'preload.js'
+      },
+      external: ['electron']
+    }
+  }
+});

Impact on other platforms:

  • Moderate impact - These changes to the Vite configuration will apply to all platforms during development.
  • The explicit configuration for the preload script ensures consistent output across all platforms.
  • Using CommonJS format (cjs) is appropriate for Electron across all platforms.
  • The removal of target properties from forge.config.ts affects all platforms but is compensated by the more explicit configurations.

5. Code Cleanup

-  // Log all paths we're checking
-  log.info('Checking binary paths:', possiblePaths);
-        log.info(`Found binary at: ${binPath}`);

Impact on other platforms:

  • Minimal impact - Removing logging statements and comments affects all platforms equally.
  • This is primarily a code maintenance improvement rather than a functional change.

Overall Cross-Platform Impact

  1. Functionality: The core functionality should remain unchanged on macOS and Linux. The changes are primarily additive for Windows support.

  2. Build Process:

    • The main build process for non-Windows platforms is unaffected.
    • The new build-main.js script is only used during Windows builds.
  3. Binary Resolution:

    • The Windows-specific paths are isolated in a conditional block.
    • The core binary resolution logic remains the same for other platforms.
  4. Development Experience:

    • The Vite configuration changes may slightly alter the development experience across all platforms, but should not cause issues.
    • The explicit configurations might actually improve consistency across platforms.
  5. Maintenance:

    • The code is now more maintainable with platform-specific logic properly isolated.
    • The IPC handler provides a more consistent approach to binary path resolution across all platforms.

Potential Concerns

  1. Testing Gap: If these changes weren't thoroughly tested on macOS and Linux after implementation, there's a small risk of unintended side effects.

  2. Build Configuration: The changes to the Vite configuration files affect all platforms, so there's a slight risk that something specific to macOS or Linux builds might be affected.

  3. Path Resolution: The binary path resolution is critical functionality, and changes to this area could potentially affect extension loading on all platforms if not carefully implemented.

However, based on the nature of the changes, these risks appear to be minimal. The commit seems well-structured to isolate Windows-specific changes while maintaining or improving cross-platform compatibility.

@Kvadratni
Copy link
Contributor Author

.bundle

@github-actions
Copy link
Contributor

macOS ARM64 Desktop App (Apple Silicon)

📱 Download macOS Desktop App (arm64, signed)

Instructions:
After downloading, unzip the file and drag the Goose.app to your Applications folder. The app is signed and notarized for macOS.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

@Kvadratni Kvadratni merged commit 41ce428 into main Mar 24, 2025
6 checks passed
@Kvadratni Kvadratni deleted the mnovich/fix-windows-uvx branch March 24, 2025 16:50
tcollier added a commit to tcollier/goose that referenced this pull request Mar 24, 2025
* origin/main:
  feat: Adjust UX of extension installs in V2 settings (block#1836)
  fix: goose modes styling (block#1833)
  New toasts (block#1777)
  feat: bring back install-link-generator which was lost in the extensions-site revamp (block#1832)
  feat: settings v2 extension add refactor (block#1815)
  fix: Update link color in chat view for user messages (block#1717) (block#1754)
  fix windows native uvx (block#1775)
  fix: correct deep link install link format (block#1830)
  fix(cli): multiselect visibility for light themes (block#1716)
  docs: Update styling (block#1831)
  Refactor top bar (block#1829)
  Docs: Revamp extensions site (block#1260)
  fix: extension site not rendering servers (block#1824)
  feat: add pdf reader (block#1818)
  fix: fix allowing multiple selectors in goosebench (block#1814)
  Fix chat width issues (block#1813)
ahau-square pushed a commit that referenced this pull request May 2, 2025
cbruyndoncx pushed a commit to cbruyndoncx/goose that referenced this pull request Jul 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants