Skip to content

Commit 45a1683

Browse files
mmaiettammaietta
authored
fix: change root path for electron/rebuild (#9376)
1 parent 82c07af commit 45a1683

File tree

10 files changed

+2470
-74
lines changed

10 files changed

+2470
-74
lines changed

‎.changeset/shaggy-clouds-pick.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"electron-builder": patch
3+
"app-builder-lib": patch
4+
---
5+
6+
fix: `install-app-deps` missing `workspaceRoot` for passing `projectRootPath` into electron/rebuild

‎.github/workflows/test.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ jobs:
6868
matrix:
6969
testFiles:
7070
- ArtifactPublisherTest,BuildTest,ExtraBuildTest,RepoSlugTest,binDownloadTest,configurationValidationTest,filenameUtilTest,filesTest,globTest,httpExecutorTest,ignoreTest,macroExpanderTest,mainEntryTest,urlUtilTest,extraMetadataTest,linuxArchiveTest,linuxPackagerTest,HoistedNodeModuleTest,MemoLazyTest,HoistTest,ExtraBuildResourcesTest,utilTest
71-
- snapTest,debTest,fpmTest,protonTest
71+
- snapTest,debTest,fpmTest,protonTest,rebuilderTest
7272
- winPackagerTest,winCodeSignTest,webInstallerTest
7373
- oneClickInstallerTest,assistedInstallerTest,packageManagerTest
7474
- concurrentBuildsTest
@@ -218,7 +218,7 @@ jobs:
218218
fail-fast: false
219219
matrix:
220220
testFiles:
221-
- oneClickInstallerTest,assistedInstallerTest,webInstallerTest,packageManagerTest,HoistedNodeModuleTest
221+
- oneClickInstallerTest,assistedInstallerTest,webInstallerTest,packageManagerTest,HoistedNodeModuleTest,rebuilderTest
222222
- winPackagerTest,winCodeSignTest,BuildTest,blackboxUpdateTest
223223
- masTest,dmgTest,filesTest,macPackagerTest,differentialUpdateTest,macArchiveTest
224224
- concurrentBuildsTest

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

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
1-
import { exists, log, spawn } from "builder-util"
2-
import { CancellationToken } from "builder-util-runtime"
3-
import * as fs from "fs-extra"
4-
import * as path from "path"
1+
import { CancellationToken, Nullish } from "builder-util-runtime"
52
import { TmpDir } from "temp-file"
63
import { NpmNodeModulesCollector } from "./npmNodeModulesCollector"
7-
import { detectYarnBerry as detectIfYarnBerry, detectPackageManagerByEnv, detectPackageManagerByFile, getPackageManagerCommand, PM } from "./packageManager"
4+
import { detectPackageManager, getPackageManagerCommand, PM } from "./packageManager"
85
import { PnpmNodeModulesCollector } from "./pnpmNodeModulesCollector"
96
import { NodeModuleInfo } from "./types"
107
import { YarnBerryNodeModulesCollector } from "./yarnBerryNodeModulesCollector"
118
import { YarnNodeModulesCollector } from "./yarnNodeModulesCollector"
129
import { BunNodeModulesCollector } from "./bunNodeModulesCollector"
10+
import { Lazy } from "lazy-val"
11+
import { spawn, log } from "builder-util"
12+
import * as fs from "fs-extra"
13+
import * as path from "path"
1314

1415
export { getPackageManagerCommand, PM }
1516

@@ -47,40 +48,26 @@ export function getNodeModules(
4748
return collector.getNodeModules({ cancellationToken, packageName })
4849
}
4950

50-
export async function detectPackageManager(searchPaths: string[]): Promise<{ pm: PM; corepackConfig: string | undefined; resolvedDirectory: string | undefined }> {
51-
let pm: PM | null = null
52-
const dedupedPaths = Array.from(new Set(searchPaths)) // reduce file operations, dedupe paths since primary use case has projectDir === appDir
53-
54-
const resolveIfYarn = (pm: PM, version: string, cwd: string) => (pm === PM.YARN ? detectIfYarnBerry(cwd, version) : pm)
55-
56-
for (const dir of dedupedPaths) {
57-
const packageJsonPath = path.join(dir, "package.json")
58-
const packageManager = (await exists(packageJsonPath)) ? (await fs.readJson(packageJsonPath, "utf8"))?.packageManager : undefined
59-
if (packageManager) {
60-
const [pm, version] = packageManager.split("@")
61-
if (Object.values(PM).includes(pm as PM)) {
62-
const resolvedPackageManager = await resolveIfYarn(pm as PM, version, dir)
63-
log.debug({ resolvedPackageManager, packageManager, cwd: dir }, "packageManager field detected in package.json")
64-
return { pm: resolvedPackageManager, corepackConfig: packageManager, resolvedDirectory: dir }
51+
export const determinePackageManagerEnv = ({ projectDir, appDir, workspaceRoot }: { projectDir: string; appDir: string; workspaceRoot: string | Nullish }) =>
52+
new Lazy(async () => {
53+
const availableDirs = [projectDir, appDir, workspaceRoot].filter((it): it is string => it != null)
54+
const pm = await detectPackageManager(availableDirs)
55+
const root = await findWorkspaceRoot(pm.pm, projectDir)
56+
if (root != null) {
57+
// re-detect package manager from workspace root, this seems particularly necessary for pnpm workspaces
58+
const actualPm = await detectPackageManager([root])
59+
return {
60+
pm: actualPm.pm,
61+
workspaceRoot: Promise.resolve(actualPm.resolvedDirectory),
6562
}
6663
}
67-
68-
pm = await detectPackageManagerByFile(dir)
69-
if (pm) {
70-
const resolvedPackageManager = await resolveIfYarn(pm, "", dir)
71-
log.debug({ resolvedPackageManager, cwd: dir }, "packageManager detected by file")
72-
return { pm: resolvedPackageManager, resolvedDirectory: dir, corepackConfig: undefined }
64+
return {
65+
pm: pm.pm,
66+
workspaceRoot: Promise.resolve(pm.resolvedDirectory),
7367
}
74-
}
75-
76-
pm = detectPackageManagerByEnv() || PM.NPM
77-
const cwd = process.env.npm_package_json ? path.dirname(process.env.npm_package_json) : (process.env.INIT_CWD ?? process.cwd())
78-
const resolvedPackageManager = await resolveIfYarn(pm, "", cwd)
79-
log.debug({ resolvedPackageManager, detected: cwd }, "packageManager not detected by file, falling back to environment detection")
80-
return { pm: resolvedPackageManager, resolvedDirectory: undefined, corepackConfig: undefined }
81-
}
68+
})
8269

83-
export async function findWorkspaceRoot(pm: PM, cwd: string): Promise<string | undefined> {
70+
async function findWorkspaceRoot(pm: PM, cwd: string): Promise<string | undefined> {
8471
let command: { command: string; args: string[] } | undefined
8572

8673
switch (pm) {

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

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,40 @@ export function getPackageManagerCommand(pm: PM) {
4545
return resolved
4646
}
4747

48-
export function detectPackageManagerByEnv(): PM | null {
48+
export async function detectPackageManager(searchPaths: string[]): Promise<{ pm: PM; corepackConfig: string | undefined; resolvedDirectory: string | undefined }> {
49+
let pm: PM | null = null
50+
const dedupedPaths = Array.from(new Set(searchPaths)) // reduce file operations, dedupe paths since primary use case has projectDir === appDir
51+
52+
const resolveIfYarn = (pm: PM, version: string, cwd: string) => (pm === PM.YARN ? detectYarnBerry(cwd, version) : pm)
53+
54+
for (const dir of dedupedPaths) {
55+
const packageJsonPath = path.join(dir, "package.json")
56+
const packageManager = (await exists(packageJsonPath)) ? (await fs.readJson(packageJsonPath, "utf8"))?.packageManager : undefined
57+
if (packageManager) {
58+
const [pm, version] = packageManager.split("@")
59+
if (Object.values(PM).includes(pm as PM)) {
60+
const resolvedPackageManager = await resolveIfYarn(pm as PM, version, dir)
61+
log.debug({ resolvedPackageManager, packageManager, cwd: dir }, "packageManager field detected in package.json")
62+
return { pm: resolvedPackageManager, corepackConfig: packageManager, resolvedDirectory: dir }
63+
}
64+
}
65+
66+
pm = await detectPackageManagerByFile(dir)
67+
if (pm) {
68+
const resolvedPackageManager = await resolveIfYarn(pm, "", dir)
69+
log.debug({ resolvedPackageManager, cwd: dir }, "packageManager detected by file")
70+
return { pm: resolvedPackageManager, resolvedDirectory: dir, corepackConfig: undefined }
71+
}
72+
}
73+
74+
pm = detectPackageManagerByEnv() || PM.NPM
75+
const cwd = process.env.npm_package_json ? path.dirname(process.env.npm_package_json) : (process.env.INIT_CWD ?? process.cwd())
76+
const resolvedPackageManager = await resolveIfYarn(pm, "", cwd)
77+
log.debug({ resolvedPackageManager, detected: cwd }, "packageManager not detected by file, falling back to environment detection")
78+
return { pm: resolvedPackageManager, resolvedDirectory: undefined, corepackConfig: undefined }
79+
}
80+
81+
function detectPackageManagerByEnv(): PM | null {
4982
const priorityChecklist = [(key: string) => process.env.npm_config_user_agent?.includes(key), (key: string) => process.env.npm_execpath?.includes(key)]
5083

5184
const pms = Object.values(PM).filter(pm => pm !== PM.YARN_BERRY)
@@ -59,7 +92,7 @@ export function detectPackageManagerByEnv(): PM | null {
5992
return null
6093
}
6194

62-
export async function detectPackageManagerByFile(dir: string): Promise<PM | null> {
95+
async function detectPackageManagerByFile(dir: string): Promise<PM | null> {
6396
const has = (file: string) => exists(path.join(dir, file))
6497

6598
const detected: PM[] = []
@@ -83,7 +116,7 @@ export async function detectPackageManagerByFile(dir: string): Promise<PM | null
83116
return null
84117
}
85118

86-
export async function detectYarnBerry(cwd: string, version: string): Promise<PM.YARN | PM.YARN_BERRY> {
119+
async function detectYarnBerry(cwd: string, version: string): Promise<PM.YARN | PM.YARN_BERRY> {
87120
const checkBerry = () => {
88121
try {
89122
if (parseInt(version.split(".")[0]) > 1) {

‎packages/app-builder-lib/src/packager.ts‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import { installOrRebuild, nodeGypRebuild } from "./util/yarn"
4343
import { PACKAGE_VERSION } from "./version"
4444
import { AsyncEventEmitter, HandlerType } from "./util/asyncEventEmitter"
4545
import asyncPool from "tiny-async-pool"
46-
import { detectPackageManager, findWorkspaceRoot, PM } from "./node-module-collector"
46+
import { determinePackageManagerEnv, PM } from "./node-module-collector"
4747

4848
async function createFrameworkInfo(configuration: Configuration, packager: Packager): Promise<Framework> {
4949
let framework = configuration.framework
@@ -96,23 +96,7 @@ export class Packager {
9696
return this._appDir
9797
}
9898

99-
private _packageManager = new Lazy(async () => {
100-
const availableDirs = [this.projectDir, this.appDir]
101-
const pm = await detectPackageManager(availableDirs)
102-
const workspaceRoot = await findWorkspaceRoot(pm.pm, this.projectDir)
103-
if (workspaceRoot != null) {
104-
// re-detect package manager from workspace root, this seems particularly necessary for pnpm workspaces
105-
const actualPm = await detectPackageManager([workspaceRoot])
106-
return {
107-
pm: actualPm.pm,
108-
workspaceRoot: Promise.resolve(actualPm.resolvedDirectory),
109-
}
110-
}
111-
return {
112-
pm: pm.pm,
113-
workspaceRoot: Promise.resolve(pm.resolvedDirectory),
114-
}
115-
})
99+
private readonly _packageManager: Lazy<{ pm: PM; workspaceRoot: Promise<string | undefined> }>
116100
async getPackageManager(): Promise<PM> {
117101
return (await this._packageManager.value).pm
118102
}
@@ -281,6 +265,7 @@ export class Packager {
281265

282266
this.projectDir = options.projectDir == null ? process.cwd() : path.resolve(options.projectDir)
283267
this._appDir = this.projectDir
268+
this._packageManager = determinePackageManagerEnv({ projectDir: this.projectDir, appDir: this.appDir, workspaceRoot: undefined })
284269

285270
this.options = {
286271
...options,

‎packages/app-builder-lib/src/util/yarn.ts‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import { homedir } from "os"
55
import * as path from "path"
66
import { Configuration } from "../configuration"
77
import { executeAppBuilderAndWriteJson } from "./appBuilder"
8-
import { PM, detectPackageManager, getPackageManagerCommand } from "../node-module-collector"
8+
import { PM, getPackageManagerCommand } from "../node-module-collector"
9+
import { detectPackageManager } from "../node-module-collector/packageManager"
910
import { NodeModuleDirInfo } from "./packageDependencies"
1011
import { rebuild as remoteRebuild } from "./rebuild"
1112
import * as which from "which"
1213
import { RebuildOptions as ElectronRebuildOptions } from "@electron/rebuild"
14+
import { Nullish } from "builder-util-runtime"
1315

1416
export async function installOrRebuild(
1517
config: Configuration,
@@ -160,34 +162,39 @@ export interface RebuildOptions {
160162
export interface DirectoryPaths {
161163
appDir: string
162164
projectDir: string
163-
workspaceRoot: string | null
165+
workspaceRoot: string | Nullish
164166
}
165167

166168
/** @internal */
167-
export async function rebuild(config: Configuration, { appDir, projectDir }: DirectoryPaths, options: RebuildOptions) {
168-
const configuration = {
169-
dependencies: await options.productionDeps.value,
170-
nodeExecPath: process.execPath,
171-
platform: options.platform || process.platform,
172-
arch: options.arch || process.arch,
173-
additionalArgs: options.additionalArgs,
174-
execPath: process.env.npm_execpath || process.env.NPM_CLI_JS,
175-
buildFromSource: options.buildFromSource === true,
176-
}
177-
const { arch, buildFromSource, platform } = configuration
169+
export async function rebuild(config: Configuration, { appDir, projectDir, workspaceRoot }: DirectoryPaths, options: RebuildOptions) {
170+
const buildFromSource = options.buildFromSource === true
171+
const platform = options.platform || process.platform
172+
const arch = options.arch || process.arch
178173

179174
if (config.nativeRebuilder === "legacy") {
175+
const configuration = {
176+
platform,
177+
arch,
178+
buildFromSource,
179+
dependencies: await options.productionDeps.value,
180+
nodeExecPath: process.execPath,
181+
additionalArgs: options.additionalArgs,
182+
execPath: process.env.npm_execpath || process.env.NPM_CLI_JS,
183+
}
180184
const env = getGypEnv(options.frameworkInfo, platform, arch, buildFromSource)
181185
return executeAppBuilderAndWriteJson(["rebuild-node-modules"], configuration, { env, cwd: appDir })
182186
}
183187

184188
const {
185189
frameworkInfo: { version: electronVersion },
186190
} = options
191+
const projectRootPath = workspaceRoot || projectDir || appDir
187192
const logInfo = {
188193
electronVersion,
189194
arch,
190195
buildFromSource,
196+
workspaceRoot,
197+
projectDir: log.filePath(projectDir) || "./",
191198
appDir: log.filePath(appDir) || "./",
192199
}
193200
log.info(logInfo, "executing @electron/rebuild")
@@ -198,7 +205,7 @@ export async function rebuild(config: Configuration, { appDir, projectDir }: Dir
198205
arch,
199206
platform,
200207
buildFromSource,
201-
projectRootPath: projectDir,
208+
projectRootPath,
202209
mode: config.nativeRebuilder || "sequential",
203210
disablePreGypCopy: true,
204211
}

‎packages/electron-builder/src/cli/install-app-deps.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { orNullIfFileNotExist } from "app-builder-lib/out/util/config/load"
66
import { createLazyProductionDeps } from "app-builder-lib/out/util/packageDependencies"
77
import { installOrRebuild } from "app-builder-lib/out/util/yarn"
88
import { PACKAGE_VERSION } from "app-builder-lib/out/version"
9+
import { determinePackageManagerEnv } from "app-builder-lib/out/node-module-collector"
910
import { getArchCliNames, log, printErrorAndExit } from "builder-util"
1011
import { readJson } from "fs-extra"
1112
import { Lazy } from "lazy-val"
@@ -48,13 +49,15 @@ export async function installAppDeps(args: any) {
4849
const config = await getConfig(projectDir, null, null, packageMetadata)
4950
const [appDir, version] = await Promise.all<string>([computeDefaultAppDirectory(projectDir, config.directories?.app), getElectronVersion(projectDir, config)])
5051

52+
const packageManagerEnv = determinePackageManagerEnv({ projectDir, appDir, workspaceRoot: undefined })
53+
5154
// if two package.json — force full install (user wants to install/update app deps in addition to dev)
5255
await installOrRebuild(
5356
config,
5457
{
5558
appDir,
5659
projectDir,
57-
workspaceRoot: null,
60+
workspaceRoot: await (await packageManagerEnv.value).workspaceRoot,
5861
},
5962
{
6063
frameworkInfo: { version, useCustomDist: true },

0 commit comments

Comments
 (0)