From f4d40f91f1511fc55cbef7c9e7edfddaf6ab67bc Mon Sep 17 00:00:00 2001 From: beyondkmp Date: Thu, 28 Nov 2024 07:05:28 +0800 Subject: [PATCH] fix: smart unpack for local module with dll (#8645) Fix the following case: the foo module contains dll files that need to be packaged into asapUnpack ```javascript test.ifDevOrWinCi("smart unpack local module with dll file", () => { return app( { targets: Platform.WINDOWS.createTarget(DIR_TARGET), }, { isInstallDepsBefore:true, projectDirCreated: async (projectDir, tmpDir) => { const tempDir = await tmpDir.getTempDir() let localPath = path.join(tempDir, "foo") await outputFile(path.join(localPath, "package.json"), `{"name":"foo","version":"9.0.0","main":"index.js","license":"MIT","dependencies":{"ms":"2.0.0"}}`) await outputFile(path.join(localPath, "test.dll"), `test`) await modifyPackageJson(projectDir, data => { data.dependencies = { debug: "3.1.0", "edge-cs": "1.2.1", foo: `file:${localPath}`, } }) }, packed: async context => { await verifySmartUnpack(context.getResources(Platform.WINDOWS)) }, } )() }) ``` --- .changeset/brave-snakes-unite.md | 6 + packages/app-builder-lib/src/asar/asarUtil.ts | 2 +- .../src/asar/unpackDetector.ts | 70 +---- .../src/util/NodeModuleCopyHelper.ts | 6 +- .../app-builder-lib/src/util/appFileCopier.ts | 2 +- packages/app-builder-lib/src/util/filter.ts | 2 +- packages/builder-util/src/fs.ts | 14 +- test/snapshots/BuildTest.js.snap | 249 ++++++++++++++++++ test/snapshots/globTest.js.snap | 62 ++++- test/src/BuildTest.ts | 30 ++- 10 files changed, 360 insertions(+), 83 deletions(-) create mode 100644 .changeset/brave-snakes-unite.md diff --git a/.changeset/brave-snakes-unite.md b/.changeset/brave-snakes-unite.md new file mode 100644 index 00000000000..38e77dc8318 --- /dev/null +++ b/.changeset/brave-snakes-unite.md @@ -0,0 +1,6 @@ +--- +"app-builder-lib": patch +"builder-util": patch +--- + +fix: smart unpack for local module with dll diff --git a/packages/app-builder-lib/src/asar/asarUtil.ts b/packages/app-builder-lib/src/asar/asarUtil.ts index 790a8179a30..e1754f17c10 100644 --- a/packages/app-builder-lib/src/asar/asarUtil.ts +++ b/packages/app-builder-lib/src/asar/asarUtil.ts @@ -141,7 +141,7 @@ export class AsarPackager { for await (const fileSet of fileSets) { if (this.config.options.smartUnpack !== false) { - detectUnpackedDirs(fileSet, unpackedPaths, this.config.defaultDestination) + detectUnpackedDirs(fileSet, unpackedPaths) } // Don't use BluebirdPromise, we need to retain order of execution/iteration through the ordered fileset diff --git a/packages/app-builder-lib/src/asar/unpackDetector.ts b/packages/app-builder-lib/src/asar/unpackDetector.ts index 177f6281bea..649b48a741f 100644 --- a/packages/app-builder-lib/src/asar/unpackDetector.ts +++ b/packages/app-builder-lib/src/asar/unpackDetector.ts @@ -1,18 +1,7 @@ -import { log } from "builder-util" +import { log, FilterStats } from "builder-util" import { isBinaryFileSync } from "isbinaryfile" import * as path from "path" -import { NODE_MODULES_PATTERN } from "../fileTransformer" -import { getDestinationPath, ResolvedFileSet } from "../util/appFileCopier" - -function addValue(map: Map>, key: string, value: string) { - let list = map.get(key) - if (list == null) { - list = [value] - map.set(key, list) - } else { - list.push(value) - } -} +import { ResolvedFileSet } from "../util/appFileCopier" export function isLibOrExe(file: string): boolean { // https://github.com/electron-userland/electron-builder/issues/3038 @@ -20,64 +9,24 @@ export function isLibOrExe(file: string): boolean { } /** @internal */ -export function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDirs: Set, defaultDestination: string) { - const dirToCreate = new Map>() +export function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDirs: Set) { const metadata = fileSet.metadata - function addParents(child: string, root: string) { - child = path.dirname(child) - if (autoUnpackDirs.has(child)) { - return - } - - do { - autoUnpackDirs.add(child) - const p = path.dirname(child) - // create parent dir to be able to copy file later without directory existence check - addValue(dirToCreate, p, path.basename(child)) - - if (child === root || p === root || autoUnpackDirs.has(p)) { - break - } - child = p - } while (true) - - autoUnpackDirs.add(root) - } - for (let i = 0, n = fileSet.files.length; i < n; i++) { const file = fileSet.files[i] - const index = file.lastIndexOf(NODE_MODULES_PATTERN) - if (index < 0) { + const stat: FilterStats = metadata.get(file)! + if (!stat.moduleRootPath || autoUnpackDirs.has(stat.moduleRootPath)) { continue } - let nextSlashIndex = file.indexOf(path.sep, index + NODE_MODULES_PATTERN.length + 1) - if (nextSlashIndex < 0) { - continue - } - - if (file[index + NODE_MODULES_PATTERN.length] === "@") { - nextSlashIndex = file.indexOf(path.sep, nextSlashIndex + 1) - } - - if (!metadata.get(file)!.isFile()) { - continue - } - - const packageDir = file.substring(0, nextSlashIndex) - const packageDirPathInArchive = path.relative(defaultDestination, getDestinationPath(packageDir, fileSet)) - const pathInArchive = path.relative(defaultDestination, getDestinationPath(file, fileSet)) - if (autoUnpackDirs.has(packageDirPathInArchive)) { - // if package dir is unpacked, any file also unpacked - addParents(pathInArchive, packageDirPathInArchive) + if (!stat.isFile()) { continue } // https://github.com/electron-userland/electron-builder/issues/2679 let shouldUnpack = false // ffprobe-static and ffmpeg-static are known packages to always unpack - const moduleName = path.basename(packageDir) + const moduleName = stat.moduleName const fileBaseName = path.basename(file) const hasExtension = path.extname(fileBaseName) if (moduleName === "ffprobe-static" || moduleName === "ffmpeg-static" || isLibOrExe(file)) { @@ -91,9 +40,8 @@ export function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDirs: Set } if (log.isDebugEnabled) { - log.debug({ file: pathInArchive, reason: "contains executable code" }, "not packed into asar archive") + log.debug({ file: stat.moduleFullFilePath, reason: "contains executable code" }, "not packed into asar archive") } - - addParents(pathInArchive, packageDirPathInArchive) + autoUnpackDirs.add(stat.moduleRootPath) } } diff --git a/packages/app-builder-lib/src/util/NodeModuleCopyHelper.ts b/packages/app-builder-lib/src/util/NodeModuleCopyHelper.ts index 827c0e8d097..5a71774742d 100644 --- a/packages/app-builder-lib/src/util/NodeModuleCopyHelper.ts +++ b/packages/app-builder-lib/src/util/NodeModuleCopyHelper.ts @@ -49,7 +49,7 @@ export class NodeModuleCopyHelper extends FileCopyHelper { super(matcher, matcher.isEmpty() ? null : matcher.createFilter(), packager) } - async collectNodeModules(moduleInfo: NodeModuleInfo, nodeModuleExcludedExts: Array): Promise> { + async collectNodeModules(moduleInfo: NodeModuleInfo, nodeModuleExcludedExts: Array, destination: string): Promise> { const filter = this.filter const metadata = this.metadata @@ -114,7 +114,9 @@ export class NodeModuleCopyHelper extends FileCopyHelper { } return lstat(filePath).then((stat: FilterStats) => { - stat.relativeNodeModulesPath = path.join("node_modules", moduleName, path.relative(depPath, filePath)) + stat.moduleName = moduleName + stat.moduleRootPath = destination + stat.moduleFullFilePath = path.join(destination, path.relative(depPath, filePath)) if (filter != null && !filter(filePath, stat)) { return null } diff --git a/packages/app-builder-lib/src/util/appFileCopier.ts b/packages/app-builder-lib/src/util/appFileCopier.ts index ec21b9faf71..c31adda3e91 100644 --- a/packages/app-builder-lib/src/util/appFileCopier.ts +++ b/packages/app-builder-lib/src/util/appFileCopier.ts @@ -192,7 +192,7 @@ export async function computeNodeModuleFileSets(platformPackager: PlatformPackag const source = dep.dir const matcher = new FileMatcher(source, destination, mainMatcher.macroExpander, mainMatcher.patterns) const copier = new NodeModuleCopyHelper(matcher, platformPackager.info) - const files = await copier.collectNodeModules(dep, nodeModuleExcludedExts) + const files = await copier.collectNodeModules(dep, nodeModuleExcludedExts, path.relative(mainMatcher.to, destination)) result[index++] = validateFileSet({ src: source, destination, files, metadata: copier.metadata }) if (dep.conflictDependency) { diff --git a/packages/app-builder-lib/src/util/filter.ts b/packages/app-builder-lib/src/util/filter.ts index c831d35a79b..fa8a1d6e2de 100644 --- a/packages/app-builder-lib/src/util/filter.ts +++ b/packages/app-builder-lib/src/util/filter.ts @@ -24,7 +24,7 @@ function ensureEndSlash(s: string) { } function getRelativePath(file: string, srcWithEndSlash: string, stat: FilterStats) { - let relative = stat.relativeNodeModulesPath || file.substring(srcWithEndSlash.length) + let relative = stat.moduleFullFilePath || file.substring(srcWithEndSlash.length) if (path.sep === "\\") { if (relative.startsWith("\\")) { // windows problem: double backslash, the above substring call removes root path with a single slash, so here can me some leftovers diff --git a/packages/builder-util/src/fs.ts b/packages/builder-util/src/fs.ts index d53bf812483..307bddced65 100644 --- a/packages/builder-util/src/fs.ts +++ b/packages/builder-util/src/fs.ts @@ -20,9 +20,17 @@ export class CopyFileTransformer { export type FileTransformer = (file: string) => Promise | null | string | Buffer | CopyFileTransformer export interface FilterStats extends Stats { - // relative path of the dependency(node_modules + moduleName + file) - // Mainly used for filter, such as files filtering and asarUnpack filtering - relativeNodeModulesPath?: string + // These module name and paths are mainly used for: + // 1. File filtering + // 2. Asar unpacking rules + // 3. Dependency resolution + + // The name of the node module (e.g. 'express') + moduleName?: string + // The root path of the node module (e.g. 'node_modules/express') + moduleRootPath?: string + // The full file path within the node module (e.g. 'node_modules/express/lib/application.js') + moduleFullFilePath?: string // deal with asar unpack sysmlink relativeLink?: string linkRelativeToFile?: string diff --git a/test/snapshots/BuildTest.js.snap b/test/snapshots/BuildTest.js.snap index 41f689cfd3b..d78fe2b4080 100644 --- a/test/snapshots/BuildTest.js.snap +++ b/test/snapshots/BuildTest.js.snap @@ -4920,6 +4920,255 @@ exports[`relative index 1`] = ` } `; +exports[`smart unpack local module with dll file 1`] = ` +{ + "win": [], +} +`; + +exports[`smart unpack local module with dll file 2`] = ` +{ + "files": { + "app": { + "files": { + "package.json": { + "files": { + "readme.md": { + "size": "", + }, + }, + }, + "readme.md": { + "size": "", + }, + }, + }, + "index.html": { + "size": "", + }, + "index.js": { + "size": "", + }, + "node_modules": { + "files": { + "debug": { + "files": { + "LICENSE": { + "size": "", + }, + "Makefile": { + "size": "", + }, + "node.js": { + "size": "", + }, + "package.json": { + "size": "", + }, + "src": { + "files": { + "browser.js": { + "size": "", + }, + "debug.js": { + "size": "", + }, + "index.js": { + "size": "", + }, + "node.js": { + "size": "", + }, + }, + }, + }, + }, + "edge-cs": { + "files": { + "LICENSE.txt": { + "size": "", + "unpacked": true, + }, + "lib": { + "files": { + "bootstrap": { + "files": { + "Dummy.cs": { + "size": "", + "unpacked": true, + }, + "project.json": { + "size": "", + "unpacked": true, + }, + }, + "unpacked": true, + }, + "edge-cs.dll": { + "size": "", + "unpacked": true, + }, + "edge-cs.js": { + "size": "", + "unpacked": true, + }, + }, + "unpacked": true, + }, + "package.json": { + "size": "", + "unpacked": true, + }, + "src": { + "files": { + "Edge.js.CSharp": { + "files": { + "EdgeCompiler.cs": { + "size": "", + "unpacked": true, + }, + "gulpfile.js": { + "size": "", + "unpacked": true, + }, + "package.json": { + "size": "", + "unpacked": true, + }, + "project.json": { + "size": "", + "unpacked": true, + }, + }, + "unpacked": true, + }, + "edge-cs": { + "files": { + "EdgeCompiler.cs": { + "size": "", + "unpacked": true, + }, + "Properties": { + "files": { + "AssemblyInfo.cs": { + "size": "", + "unpacked": true, + }, + }, + "unpacked": true, + }, + }, + "unpacked": true, + }, + }, + "unpacked": true, + }, + "tools": { + "files": { + "install.js": { + "size": "", + "unpacked": true, + }, + }, + "unpacked": true, + }, + }, + "unpacked": true, + }, + "foo": { + "files": { + "package.json": { + "size": "", + "unpacked": true, + }, + "test.dll": { + "size": "", + "unpacked": true, + }, + }, + "unpacked": true, + }, + "ms": { + "files": { + "index.js": { + "size": "", + }, + "license.md": { + "size": "", + }, + "package.json": { + "size": "", + }, + }, + }, + }, + }, + "package.json": { + "size": "", + }, + }, +} +`; + +exports[`smart unpack local module with dll file 3`] = ` +[ + "app.asar", + { + "content": "{"name":"foo","version":"9.0.0","main":"index.js","license":"MIT","dependencies":{"ms":"2.0.0"}}", + "name": "app.asar.unpacked/node_modules/foo/package.json", + }, + "app.asar.unpacked/node_modules/foo/test.dll", + "app.asar.unpacked/node_modules/edge-cs/LICENSE.txt", + { + "content": "{ + "name": "edge-cs", + "author": { + "name": "Tomasz Janczuk ", + "url": "http://tomasz.janczuk.org", + "twitter": "tjanczuk" + }, + "version": "1.2.1", + "description": "Edge-cs: run C# and node.js code in-process with edge.js", + "main": "./lib/edge-cs.js", + "engines": { + "node": ">= 0.8" + }, + "licenses": [ + { + "type": "Apache", + "url": "http://www.apache.org/licenses/LICENSE-2.0" + } + ], + "dependencies": {}, + "homepage": "http://tjanczuk.github.com/edge", + "repository": { + "type": "git", + "url": "git@github.com:tjanczuk/edge-cs.git" + } +}", + "name": "app.asar.unpacked/node_modules/edge-cs/package.json", + }, + "app.asar.unpacked/node_modules/edge-cs/tools/install.js", + "app.asar.unpacked/node_modules/edge-cs/src/edge-cs/EdgeCompiler.cs", + "app.asar.unpacked/node_modules/edge-cs/src/edge-cs/Properties/AssemblyInfo.cs", + "app.asar.unpacked/node_modules/edge-cs/src/Edge.js.CSharp/EdgeCompiler.cs", + "app.asar.unpacked/node_modules/edge-cs/src/Edge.js.CSharp/gulpfile.js", + { + "content": "{ + "devDependencies": { + "gulp": "3.9.0" + } +}", + "name": "app.asar.unpacked/node_modules/edge-cs/src/Edge.js.CSharp/package.json", + }, + "app.asar.unpacked/node_modules/edge-cs/src/Edge.js.CSharp/project.json", + "app.asar.unpacked/node_modules/edge-cs/lib/edge-cs.dll", + "app.asar.unpacked/node_modules/edge-cs/lib/edge-cs.js", + "app.asar.unpacked/node_modules/edge-cs/lib/bootstrap/Dummy.cs", + "app.asar.unpacked/node_modules/edge-cs/lib/bootstrap/project.json", +] +`; + exports[`win smart unpack 1`] = ` { "win": [], diff --git a/test/snapshots/globTest.js.snap b/test/snapshots/globTest.js.snap index 897d306549f..13afce25840 100644 --- a/test/snapshots/globTest.js.snap +++ b/test/snapshots/globTest.js.snap @@ -9896,6 +9896,7 @@ exports[`symlinks everywhere w/ static framework 3`] = ` "files": { "hello-world-framework.sh": { "size": "", + "unpacked": true, }, "lib": { "files": { @@ -9922,84 +9923,94 @@ exports[`symlinks everywhere w/ static framework 3`] = ` "Headers": { "files": { "HelloFramework.h": { - "executable": true, "size": "", + "unpacked": true, }, }, + "unpacked": true, }, "Hello": { - "executable": true, "size": "", + "unpacked": true, }, "Info.plist": { - "executable": true, "size": "", + "unpacked": true, }, "Modules": { "files": { "Hello.abi.json": { - "executable": true, "size": "", + "unpacked": true, }, "Hello.swiftdoc": { - "executable": true, "size": "", + "unpacked": true, }, "Hello.swiftmodule": { "files": { "arm64.private.swiftinterface": { - "executable": true, "size": "", + "unpacked": true, }, "arm64.swiftinterface": { - "executable": true, "size": "", + "unpacked": true, }, "arm64.swiftmodule": { - "executable": true, "size": "", + "unpacked": true, }, "x86_64.private.swiftinterface": { - "executable": true, "size": "", + "unpacked": true, }, "x86_64.swiftinterface": { - "executable": true, "size": "", + "unpacked": true, }, "x86_64.swiftmodule": { - "executable": true, "size": "", + "unpacked": true, }, }, + "unpacked": true, }, "Hello.swiftsourceinfo": { - "executable": true, "size": "", + "unpacked": true, }, "module.modulemap": { - "executable": true, "size": "", + "unpacked": true, }, }, + "unpacked": true, }, }, + "unpacked": true, }, "Current": { "link": "node_modules/hello-world/lib/Release/Hello.framework/Versions/A", }, }, + "unpacked": true, }, }, + "unpacked": true, }, }, + "unpacked": true, }, }, + "unpacked": true, }, "package.json": { "size": "", + "unpacked": true, }, }, + "unpacked": true, }, "ms": { "files": { @@ -10026,6 +10037,31 @@ exports[`symlinks everywhere w/ static framework 3`] = ` exports[`symlinks everywhere w/ static framework 4`] = ` [ "app.asar", + "app.asar.unpacked/node_modules/hello-world/hello-world-framework.sh", + { + "content": "{ + "name": "stripped-native-dep", + "version": "1.0.0", + "description": "", + "author": "", + "license": "ISC" + } + ", + "name": "app.asar.unpacked/node_modules/hello-world/package.json", + }, + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Hello", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Info.plist", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.abi.json", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftdoc", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftsourceinfo", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/module.modulemap", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftmodule/arm64.private.swiftinterface", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftmodule/arm64.swiftinterface", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftmodule/arm64.swiftmodule", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftmodule/x86_64.private.swiftinterface", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftmodule/x86_64.swiftinterface", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Modules/Hello.swiftmodule/x86_64.swiftmodule", + "app.asar.unpacked/node_modules/hello-world/lib/Release/Hello.framework/Versions/A/Headers/HelloFramework.h", ] `; diff --git a/test/src/BuildTest.ts b/test/src/BuildTest.ts index 6684bc96148..697f38b0df4 100644 --- a/test/src/BuildTest.ts +++ b/test/src/BuildTest.ts @@ -2,7 +2,7 @@ import { checkBuildRequestOptions } from "app-builder-lib" import { doMergeConfigs } from "app-builder-lib/out/util/config/config" import { Arch, createTargets, DIR_TARGET, Platform } from "electron-builder" import { promises as fs } from "fs" -import { outputJson } from "fs-extra" +import { outputJson ,outputFile} from "fs-extra" import * as path from "path" import { createYargs } from "electron-builder/out/builder" import { app, appTwo, appTwoThrows, assertPack, linuxDirTarget, modifyPackageJson, packageJson, toSystemIndependentPath } from "./helpers/packTester" @@ -324,6 +324,34 @@ test.ifDevOrLinuxCi("win smart unpack", () => { )() }) +test.ifDevOrWinCi("smart unpack local module with dll file", () => { + return app( + { + targets: Platform.WINDOWS.createTarget(DIR_TARGET), + }, + { + isInstallDepsBefore:true, + projectDirCreated: async (projectDir, tmpDir) => { + const tempDir = await tmpDir.getTempDir() + let localPath = path.join(tempDir, "foo") + await outputFile(path.join(localPath, "package.json"), `{"name":"foo","version":"9.0.0","main":"index.js","license":"MIT","dependencies":{"ms":"2.0.0"}}`) + await outputFile(path.join(localPath, "test.dll"), `test`) + await modifyPackageJson(projectDir, data => { + data.dependencies = { + debug: "3.1.0", + "edge-cs": "1.2.1", + foo: `file:${localPath}`, + } + }) + }, + packed: async context => { + await verifySmartUnpack(context.getResources(Platform.WINDOWS)) + }, + } + )() +}) + + // https://github.com/electron-userland/electron-builder/issues/1738 test.ifDevOrLinuxCi( "posix smart unpack",