asarUnpack not honored with electron-builder 25.1.8, brings many unneeded files into app.asar.unpacked (source files, readmes, package.json, etc.)

[ivanggq]

• Electron-Builder Version: 25.1.8

• Node Version: 20.12.1

• Electron Version: 33.0.2
• Electron Type (current, beta, nightly): current latest

• Target: Windows

Recent changes in electron-builder (not sure when exactly this started) bring all kind of unnecessary files into the app.asar.unpacked folder (where native dependencies are). We have an asarUnpack rule node_modules/**/*.node.

This works fine with electron-builder 23.6.0, but not with recent electron-builder versions.

The unnecessary files in app.asar.unpacked could be (but not limited to):

• C++ source files (.h, .cpp), but I think not .cc for example (!)
• .js files
• package.json, readmes, jenkinsfiles, etc. whatever the native dependency has

Example:

Repro code: source-files-unpacked.zip
To repro:

• Open the repro code on Windows (I have not tested on Mac yet, suspect it might be the same result)
• npm install
• npm run package
• Inspect the dist\win-unpacked\resources\app.asar.unpacked\node_modules\native-reg folder - it has a bunch of unneeded files that are not part of asarUnpack
• Change electron-builder version in package.json to 23.6.0, npm install and repackage - only the .node files are packaged

Depending on the native dependency, this issue could bring C++ source files, Jenkinsfiles, etc.
I did notice in builder-debug.yml that .cc extensions are excluded by firstOrDefaultFilePatterns and this is likely why .cc files are not there (while .h and .cpp are for example), but to my understanding, the asarUnpack rule should be respected here. At least it seems to be respected more in electron-builder version 23.6.0.

[mmaietta]

Can you try testing with next version of electron-builder v26.0.0-alpha.4? I migrated electron-builder to use the official @electron/asar package and that migration modified the asar unpack feature as well.

[ivanggq]

Tried with v26.0.0-alpha.4 - the issue is still there. @mmaietta

[mmaietta]

I tested your project locally and it looks like that regressed in 24.0.0-alpha.1. Can you confirm from your side?

Looks like that came in this diff: v23.6.0...v24.0.0-alpha.1
Nothing particularly sticks out to me in that diff though as no "unpacking" logic was modified AFAICT

[ivanggq]

Actually, it works fine in 24.0.0-alpha.1 in my test. Let me try to do a binary search and find where it regresses.

[ivanggq]

In my tests, the issue is introduced in 25.0.4. It does not exist in 25.0.3.
Full Changelog: v25.0.3...v25.0.4

[ivanggq]

I suspect this change: https://github.com/electron-userland/electron-builder/pull/8398/files

[mmaietta]

Hmmmm, I think it looks like this change caused the test snapshots to expand coverage to include parent directories of .node modules, which is what you're experiencing. #8392
I don't see any issues linked in the description that it was specifically fixing though, so I'm debating on reverting it. @beyondkmp do you recall what that PR change was for?

Can you test with this patch-package on top of 26.0.0-alpha.4?
app-builder-lib+26.0.0-alpha.4.patch

diff --git a/node_modules/app-builder-lib/out/asar/unpackDetector.js b/node_modules/app-builder-lib/out/asar/unpackDetector.js
index cd05746..678fd23 100644
--- a/node_modules/app-builder-lib/out/asar/unpackDetector.js
+++ b/node_modules/app-builder-lib/out/asar/unpackDetector.js
@@ -18,8 +18,7 @@ function addValue(map, key, value) {
     }
 }
 function isLibOrExe(file) {
-    // 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");
+    return file.endsWith(".dll") || file.endsWith(".exe") || file.endsWith(".dylib") || file.endsWith(".so");
 }
 /** @internal */
 function detectUnpackedDirs(fileSet, autoUnpackDirs, defaultDestination) {
diff --git a/node_modules/app-builder-lib/out/util/appFileCopier.js b/node_modules/app-builder-lib/out/util/appFileCopier.js
index d577623..25cc04a 100644
--- a/node_modules/app-builder-lib/out/util/appFileCopier.js
+++ b/node_modules/app-builder-lib/out/util/appFileCopier.js
@@ -38,7 +38,8 @@ async function copyAppFiles(fileSet, packager, transformer) {
     const taskManager = new builder_util_1.AsyncTaskManager(packager.cancellationToken);
     const createdParentDirs = new Set();
     const fileCopier = new builder_util_1.FileCopier(file => {
-        return !(0, unpackDetector_1.isLibOrExe)(file);
+        // https://github.com/electron-userland/electron-builder/issues/3038
+        return !(0, unpackDetector_1.isLibOrExe)(file) || file.endsWith(".node");
     }, transformer);
     const links = [];
     for (let i = 0, n = fileSet.files.length; i < n; i++) {

[ivanggq]

The patch above on top of 26.0.0-alpha.4 fixes the issue.

[beyondkmp]

@mmaietta All npm C++ native modules generate a file ending in .node, which is an executable file. Currently, it is default practice to place these inside app.asar, the user currently needs to manually specify asarUnpack. This MR (#8392) aims to handle .node files automatically like exe and dll files, so users won't need to manually specify them in asarUnpack.

[mmaietta]

Hmmm, that's a fair point, but I guess the previous MR also added all the parent directories during the unpack step. We need only the .node files unpacked, not every parent folder that contains it. I think that's why it was set up to have manually specified in asarUnpack. We need an alternative approach implemented for automatically handling .node files while not reproducing this issue.

[beyondkmp]

Yes, you are right.

I have a question: Should we apply the same treatment to exe and dll files as well, only extracting these specific files without unpacking other files in their parent folders?

[mmaietta]

I don't think so in this case as .node files exist on every OS, whereas exe and dll files are only Windows-specific. I think that's why the setup was as it was prior

That being said, I do agree that the .node files should be unpacked by default to follow best practices. I'm just struggling to figure out where that needs to be implemented. The unpack detector makes sense based off the name, but it's designed to add all parent directories to the unpack pattern which isn't what we're looking for

[beyondkmp]

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")
}

From this function's perspective, it appears to include executable files for multiple operating systems. dylib are macOS-specific and so is linux-specific.

I mean that all these executable files should be handled in the same way - only the executable files themselves should be placed in asarUnpack, while other files in their parent directories should not be included unless specifically specified by the user.

[mmaietta]

I mean that all these executable files should be handled in the same way - only the executable files themselves should be placed in asarUnpack, while other files in their parent directories should not be included unless specifically specified by the user.

I agree with this, but I want to know why it was originally coded this way in the first place. If we can get .node files to be auto-detected without adding parent's files, that would be ideal. I'm not sure about changing the behavior of the other extensions though

[mmaietta]

@ivanggq , out of curiosity, what is the core issue here? The "unneeded" files were always being included in your app.asar, just never included in the app.asar.unpacked, so nothing is really changed from the production asset contents per see.

The caveat of a revert is that **/*.node will need to be manually specified in asarUnpack, which I'm adverse to doing as I think that should be automatic. I'm trying to find a middleground that auto-unpacks only .node addons, while still any file that is dll/so/dylib/exe are also auto-unpacked with all parent files included. Just wanted to understand the reason this issue was opened and if it's causing any problems in production.

[ivanggq]

I was frankly not aware that these files are part of the app.asar. I now extracted and inspected the app.asar and saw them there... (which leads me to more questions).

To answer your question first - my motivation for filing this ticket is that the source files of native dependencies we use are easily visible in the app.asar.unpacked folder. Our company uses some internally created native deps and naturally it is undesirable to distribute the source code for them. I now see they are part of the app.asar, but unpacked folder is more easy to inspect compared to the app.asar. But it is also true that the app.asar is not exactly protecting the sources files, anybody interested could extract it and see them.

Which leads me to the additional questions:

• Even if we make no reverts are leave things as is, is there a recommendation on how to filter out these source files, from both the app.asar and the app.asar.unpacked? (question of not leaking source files)
• I see that the native dependencies are actually fully duplicated in the app.asar, with all binary .node/.dll/etc files and other files (source, readmes, jenkins, etc.). Not too big of an issue, but duplication increases the app size. Is there a way this to be avoided via configuration flags, filters, etc. (question of reducing app size by avoiding duplication)

[mmaietta]

Even if we make no reverts are leave things as is, is there a recommendation on how to filter out these source files, from both the app.asar and the app.asar.unpacked? (question of not leaking source files)

You can add them to the files patterns with a minimatch negation (!) like !node_modules/native-reg/**/*.d.ts and similar patterns to exclude what you need. Typically I always exclude all map files via !**/*.map for instance

I see that the native dependencies are actually fully duplicated in the app.asar, with all binary .node/.dll/etc files and other files (source, readmes, jenkins, etc.). Not too big of an issue, but duplication increases the app size. Is there a way this to be avoided via configuration flags, filters, etc. (question of reducing app size by avoiding duplication)

From what I understand, they're not actually duplicated, they're symlinks within the asar. Upon extraction of the asar, electron/asar copies the actual files that are symlinked from app.asar.unpacked into the extracted directory. So in actuality, the app size isn't being duplicated.

[ivanggq]

OK, I will try that for my internal project and report back. (May need a day or two for this). Thanks for the quick replies and looking into this.

[ivanggq]

This works for our project, so I think we can close this ticket now. Thanks.