fix: smart unpack for local module with dll (electron-userland#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))
      },
    }
  )()
})
```

Author: beyondkmp

.changeset/brave-snakes-unite.md

@@ -0,0 +1,6 @@
+---
+"app-builder-lib": patch
+"builder-util": patch
+---
+
+fix: smart unpack for local module with dll

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

packages/app-builder-lib/src/asar/unpackDetector.ts

@@ -1,83 +1,32 @@
-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<string, Array<string>>, 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
   return file.endsWith(".dll") || file.endsWith(".exe") || file.endsWith(".dylib") || file.endsWith(".so") || file.endsWith(".node")
 }
 
 /** @internal */
-export function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDirs: Set<string>, defaultDestination: string) {
-  const dirToCreate = new Map<string, Array<string>>()
+export function detectUnpackedDirs(fileSet: ResolvedFileSet, autoUnpackDirs: Set<string>) {
   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)
   }
 }

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<string>): Promise<Array<string>> {
+  async collectNodeModules(moduleInfo: NodeModuleInfo, nodeModuleExcludedExts: Array<string>, destination: string): Promise<Array<string>> {
     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
             }

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) {

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

packages/builder-util/src/fs.ts

@@ -20,9 +20,17 @@ export class CopyFileTransformer {
 
 export type FileTransformer = (file: string) => Promise<null | string | Buffer | CopyFileTransformer> | 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