Merge branch 'master' into fix/multi-space-string-scraping

Author: mmaietta

.changeset/famous-berries-clap.md

@@ -0,0 +1,5 @@
+---
+"app-builder-lib": patch
+---
+
+fix(nsis): implement custom function to handle /D parameter with spaces

.changeset/flat-trees-return.md

@@ -0,0 +1,5 @@
+---
+"app-builder-lib": patch
+---
+
+feat(nsis): terminate only processes running in installation folder

.changeset/funny-bottles-occur.md

@@ -0,0 +1,6 @@
+---
+"builder-util-runtime": patch
+"electron-updater": patch
+---
+
+feat(electron-updater): add GitLab provider support

.changeset/heavy-hats-wonder.md

@@ -0,0 +1,9 @@
+---
+"app-builder-lib": patch
+"builder-util": patch
+"builder-util-runtime": patch
+"dmg-builder": patch
+"electron-updater": patch
+---
+
+fix: remove `shell: true` from node_modules collector so as to prevent shell console logging from malforming the json output

packages/app-builder-lib/src/codeSign/macCodeSign.ts

@@ -214,7 +214,11 @@ async function importCerts(keychainFile: string, paths: Array<string>, keyPasswo
 }
 
 export async function sign(opts: SignOptions): Promise<void> {
-  return retry(() => signAsync(opts), 3, 5000, 5000)
+  return retry(() => signAsync(opts), {
+    retries: 3,
+    interval: 5000,
+    backoff: 5000,
+  })
 }
 
 export let findIdentityRawResult: Promise<Array<string>> | null = null

packages/app-builder-lib/src/codeSign/windowsCodeSign.ts

@@ -21,17 +21,22 @@ export async function signWindows(options: WindowsSignOptions, packager: WinPack
 }
 
 function signWithRetry(signer: () => Promise<boolean>): Promise<boolean> {
-  return retry(signer, 3, 1000, 1000, 0, (e: any) => {
-    const message = e.message
-    if (
-      // https://github.com/electron-userland/electron-builder/issues/1414
-      message?.includes("Couldn't resolve host name") ||
-      // https://github.com/electron-userland/electron-builder/issues/8615
-      message?.includes("being used by another process.")
-    ) {
-      log.warn({ error: message }, "attempt to sign failed, another attempt will be made")
-      return true
-    }
-    return false
+  return retry(signer, {
+    retries: 3,
+    interval: 1000,
+    backoff: 1000,
+    shouldRetry: (e: any) => {
+      const message = e.message
+      if (
+        // https://github.com/electron-userland/electron-builder/issues/1414
+        message?.includes("Couldn't resolve host name") ||
+        // https://github.com/electron-userland/electron-builder/issues/8615
+        message?.includes("being used by another process.")
+      ) {
+        log.warn({ error: message }, "attempt to sign failed, another attempt will be made")
+        return true
+      }
+      return false
+    },
   })
 }

packages/app-builder-lib/src/codeSign/windowsSignToolManager.ts

@@ -449,13 +449,11 @@ export class WindowsSignToolManager implements SignManager {
       }
     }
 
-    await retry(
-      () => vm.exec(tool, args, { timeout, env }),
-      2,
-      15000,
-      10000,
-      0,
-      (e: any) => {
+    await retry(() => vm.exec(tool, args, { timeout, env }), {
+      retries: 2,
+      interval: 15000,
+      backoff: 10000,
+      shouldRetry: (e: any) => {
         if (
           e.message.includes("The file is being used by another process") ||
           e.message.includes("The specified timestamp server either could not be reached") ||
@@ -465,8 +463,8 @@ export class WindowsSignToolManager implements SignManager {
           return true
         }
         return false
-      }
-    )
+      },
+    })
   }
 }
 

packages/app-builder-lib/src/node-module-collector/index.ts

@@ -3,34 +3,27 @@ import { PnpmNodeModulesCollector } from "./pnpmNodeModulesCollector"
 import { YarnNodeModulesCollector } from "./yarnNodeModulesCollector"
 import { detectPackageManager, PM, getPackageManagerCommand } from "./packageManager"
 import { NodeModuleInfo } from "./types"
-import { exec } from "builder-util"
+import { TmpDir } from "temp-file"
 
-async function isPnpmProjectHoisted(rootDir: string) {
-  const command = getPackageManagerCommand(PM.PNPM)
-  const config = await exec(command, ["config", "list"], { cwd: rootDir, shell: true })
-  const lines = Object.fromEntries(config.split("\n").map(line => line.split("=").map(s => s.trim())))
-  return lines["node-linker"] === "hoisted"
-}
-
-export async function getCollectorByPackageManager(rootDir: string) {
+export async function getCollectorByPackageManager(rootDir: string, tempDirManager: TmpDir) {
   const manager: PM = detectPackageManager(rootDir)
   switch (manager) {
     case PM.PNPM:
-      if (await isPnpmProjectHoisted(rootDir)) {
-        return new NpmNodeModulesCollector(rootDir)
+      if (await PnpmNodeModulesCollector.isPnpmProjectHoisted(rootDir)) {
+        return new NpmNodeModulesCollector(rootDir, tempDirManager)
       }
-      return new PnpmNodeModulesCollector(rootDir)
+      return new PnpmNodeModulesCollector(rootDir, tempDirManager)
     case PM.NPM:
-      return new NpmNodeModulesCollector(rootDir)
+      return new NpmNodeModulesCollector(rootDir, tempDirManager)
     case PM.YARN:
-      return new YarnNodeModulesCollector(rootDir)
+      return new YarnNodeModulesCollector(rootDir, tempDirManager)
     default:
-      return new NpmNodeModulesCollector(rootDir)
+      return new NpmNodeModulesCollector(rootDir, tempDirManager)
   }
 }
 
-export async function getNodeModules(rootDir: string): Promise<NodeModuleInfo[]> {
-  const collector = await getCollectorByPackageManager(rootDir)
+export async function getNodeModules(rootDir: string, tempDirManager: TmpDir): Promise<NodeModuleInfo[]> {
+  const collector = await getCollectorByPackageManager(rootDir, tempDirManager)
   return collector.getNodeModules()
 }
 

packages/app-builder-lib/src/node-module-collector/nodeModulesCollector.ts

@@ -1,16 +1,24 @@
 import { hoist, type HoisterTree, type HoisterResult } from "./hoist"
 import * as path from "path"
-import * as fs from "fs"
+import * as fs from "fs-extra"
 import type { NodeModuleInfo, DependencyGraph, Dependency } from "./types"
-import { exec, log } from "builder-util"
+import { exists, log, retry, TmpDir } from "builder-util"
 import { getPackageManagerCommand, PM } from "./packageManager"
+import { exec, spawn } from "child_process"
+import { promisify } from "util"
+import { createWriteStream } from "fs"
+
+const execAsync = promisify(exec)
 
 export abstract class NodeModulesCollector<T extends Dependency<T, OptionalsType>, OptionalsType> {
   private nodeModules: NodeModuleInfo[] = []
   protected allDependencies: Map<string, T> = new Map()
   protected productionGraph: DependencyGraph = {}
 
-  constructor(private readonly rootDir: string) {}
+  constructor(
+    private readonly rootDir: string,
+    private readonly tempDirManager: TmpDir
+  ) {}
 
   public async getNodeModules(): Promise<NodeModuleInfo[]> {
     const tree: T = await this.getDependenciesTree()
@@ -37,11 +45,43 @@ export abstract class NodeModulesCollector<T extends Dependency<T, OptionalsType
   protected async getDependenciesTree(): Promise<T> {
     const command = getPackageManagerCommand(this.installOptions.manager)
     const args = this.getArgs()
-    const dependencies = await exec(command, args, {
-      cwd: this.rootDir,
-      shell: true,
+
+    const tempOutputFile = await this.tempDirManager.getTempFile({
+      prefix: path.basename(command, path.extname(command)),
+      suffix: "output.json",
     })
-    return this.parseDependenciesTree(dependencies)
+
+    return retry(
+      async () => {
+        await this.streamCollectorCommandToJsonFile(command, args, this.rootDir, tempOutputFile)
+        const dependencies = await fs.readFile(tempOutputFile, { encoding: "utf8" })
+        try {
+          return this.parseDependenciesTree(dependencies)
+        } catch (error: any) {
+          log.debug({ message: error.message || error.stack, shellOutput: dependencies }, "error parsing dependencies tree")
+          throw new Error(`Failed to parse dependencies tree: ${error.message || error.stack}. Use DEBUG=electron-builder env var to see the dependency query output.`)
+        }
+      },
+      {
+        retries: 2,
+        interval: 2000,
+        backoff: 2000,
+        shouldRetry: async (error: any) => {
+          if (!(await exists(tempOutputFile))) {
+            log.error({ error: error.message || error.stack, tempOutputFile }, "error getting dependencies tree, unable to find output; retrying")
+            return true
+          }
+          const dependencies = await fs.readFile(tempOutputFile, { encoding: "utf8" })
+          if (dependencies.trim().length === 0 || error.message?.includes("Unexpected end of JSON input")) {
+            // If the output file is empty or contains invalid JSON, we retry
+            // This can happen if the command fails or if the output is not as expected
+            log.error({ error: error.message || error.stack, tempOutputFile }, "dependency tree output file is empty, retrying")
+            return true
+          }
+          return false
+        },
+      }
+    )
   }
 
   protected resolvePath(filePath: string): string {
@@ -117,4 +157,60 @@ export abstract class NodeModulesCollector<T extends Dependency<T, OptionalsType
     }
     result.sort((a, b) => a.name.localeCompare(b.name))
   }
+
+  static async safeExec(command: string, args: string[], cwd: string): Promise<string> {
+    const payload = await execAsync([`"${command}"`, ...args].join(" "), { cwd, maxBuffer: 100 * 1024 * 1024 }) // 100MB buffer LOL, some projects can have extremely large dependency trees
+    return payload.stdout.trim()
+  }
+
+  async streamCollectorCommandToJsonFile(command: string, args: string[], cwd: string, tempOutputFile: string) {
+    const execName = path.basename(command, path.extname(command))
+    const isWindowsScriptFile = process.platform === "win32" && path.extname(command).toLowerCase() === ".cmd"
+    if (isWindowsScriptFile) {
+      // If the command is a Windows script file (.cmd), we need to wrap it in a .bat file to ensure it runs correctly with cmd.exe
+      // This is necessary because .cmd files are not directly executable in the same way as .bat files.
+      // We create a temporary .bat file that calls the .cmd file with the provided arguments. The .bat file will be executed by cmd.exe.
+      const tempBatFile = await this.tempDirManager.getTempFile({
+        prefix: execName,
+        suffix: ".bat",
+      })
+      const batScript = `@echo off\r\n"${command}" %*\r\n` // <-- CRLF required for .bat
+      await fs.writeFile(tempBatFile, batScript, { encoding: "utf8" })
+      command = "cmd.exe"
+      args = ["/c", tempBatFile, ...args]
+    }
+
+    await new Promise<void>((resolve, reject) => {
+      const outStream = createWriteStream(tempOutputFile)
+
+      const child = spawn(command, args, {
+        cwd,
+        shell: false, // required to prevent console logs polution from shell profile loading when `true`
+      })
+
+      let stderr = ""
+      child.stdout.pipe(outStream)
+      child.stderr.on("data", chunk => {
+        stderr += chunk.toString()
+      })
+      child.on("error", err => {
+        reject(new Error(`Spawn failed: ${err.message}`))
+      })
+
+      child.on("close", code => {
+        outStream.close()
+        // https://github.com/npm/npm/issues/17624
+        if (code === 1 && execName.toLowerCase() === "npm" && args.includes("list")) {
+          log.debug({ code, stderr }, "`npm list` returned non-zero exit code, but it MIGHT be expected (https://github.com/npm/npm/issues/17624). Check stderr for details.")
+          // This is a known issue with npm list command, it can return code 1 even when the command is "technically" successful
+          resolve()
+          return
+        }
+        if (code !== 0) {
+          return reject(new Error(`Process exited with code ${code}:\n${stderr}`))
+        }
+        resolve()
+      })
+    })
+  }
 }

packages/app-builder-lib/src/node-module-collector/npmNodeModulesCollector.ts

@@ -3,10 +3,6 @@ import { PM } from "./packageManager"
 import { NpmDependency } from "./types"
 
 export class NpmNodeModulesCollector extends NodeModulesCollector<NpmDependency, string> {
-  constructor(rootDir: string) {
-    super(rootDir)
-  }
-
   public readonly installOptions = { manager: PM.NPM, lockfile: "package-lock.json" }
 
   protected getArgs(): string[] {