Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

electron builder v26.0.0-alpha.2 fails to copy native deps with sym links #8655

Closed
ivanggq opened this issue Nov 1, 2024 · 18 comments · Fixed by #8663 · May be fixed by electron/asar#341
Closed

electron builder v26.0.0-alpha.2 fails to copy native deps with sym links #8655

ivanggq opened this issue Nov 1, 2024 · 18 comments · Fixed by #8663 · May be fixed by electron/asar#341

Comments

@ivanggq
Copy link

ivanggq commented Nov 1, 2024

  • Electron-Builder Version: 26.0.0-alpha.2
  • Node Version: 20.17.0
  • Electron Version: 31.5.0
  • Electron Type (current, beta, nightly):
  • Target: macOS

Version 26.0.0-alpha.2 introduces an issue when copying native deps that contain sym links, e.g.

  ⨯ Cannot cleanup: 

Error #1 --------------------------------------------------------------------------------
Error: EISDIR: illegal operation on a directory, copyfile '/Users/<project folder omitted>/node_modules/<private native dep name omitted>/lib/Release/QtCore.framework/Versions/Current' -> '/private/var/folders/d2/svj1s5gn6_v88skdzfjwdmbdmb7f47/T/t-SBUwbm/asar-app-0/node_modules/<private native dep name omitted>/lib/Release/QtCore.framework/Versions/A'

Error #2 --------------------------------------------------------------------------------
Error: EISDIR: illegal operation on a directory, copyfile '/Users/<project folder omitted>/node_modules/<private native dep name omitted>/lib/Release/QtCore.framework/Resources' -> '/private/var/folders/d2/svj1s5gn6_v88skdzfjwdmbdmb7f47/T/t-SBUwbm/asar-app-0/node_modules/<private native dep name omitted>/lib/Release/QtCore.framework/Versions/A/Resources'

Error #3 --------------------------------------------------------------------------------
Error: ENOTSUP: operation not supported on socket, copyfile '/Users/<project folder omitted>/node_modules/<private native dep name omitted>/lib/Release/QtNetwork.framework/Resources' -> '/private/var/folders/d2/svj1s5gn6_v88skdzfjwdmbdmb7f47/T/t-SBUwbm/asar-app-0/node_modules/<private native dep name omitted>/lib/Release/QtNetwork.framework/Versions/A/Resources'  failedTask=build stackTrace=Error: Cannot cleanup: 

(Apologies for not sharing the full paths, hiding private names.)

I tracked down the issue to https://github.com/electron-userland/electron-builder/pull/8570/files, specifically I think the change in copyFileOrData, and the replacement of fileCopier.copy with fs.copyFile.

As you can see from the log, the issue is when copying sym links (like we find in Mac frameworks).

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 1, 2024

Any chance you'd be willing to provide a minimum repro repo for this? I can definitely look into the fileCopier implementation, I just would also like a way to add a test case for this.

@ivanggq
Copy link
Author

ivanggq commented Nov 1, 2024

The native dependency in question is private company library. I tried hard to find a similar public dependency, but couldn't... I can look more, but also I am happy to provide more logs (sanitized), test implementations, etc.

Edit: Not that there is much more in the logs other than what I shared above, only the call stack of the failure.

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 2, 2024

I think I can just create one via https://jano.dev/apple/mach-o/2024/06/28/Hello-Static-Framework.html

Can you share your electron-builder config?

@beyondkmp
Copy link
Collaborator

I think we might be able to reproduce the issue by putting a framework file in a node module. If that doesn't work, it might be related to QtCore.framework file structure, which is open source. @ivanggq Could you provide it to us for testing?

@ivanggq
Copy link
Author

ivanggq commented Nov 2, 2024

Hi, I actually managed to create a minimal repo reproducing the issue: sym-link-demo.zip

  • (Note that in the past there were some unzip utilities that did not preserve the symbolic links when extracting, the built-in macOS Archive Utility app works fine).

The project contains a stripped down native dep with a stripped down Qt framework that reproduces the issue.

To repro:

  • npm install
  • npm run package

Thanks for being so responsive.

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 7, 2024

@ivanggq just released a patch version that resolves the bundling issue you were receiving when I tested locally. Since it was a slimmed down app you provided though, can you please try v26.0.0-alpha.6 and test your app configuration for both bundling+runtime?

@ivanggq
Copy link
Author

ivanggq commented Nov 7, 2024

Hi @mmaietta , I tested our internal project with v26.0.0-alpha.6. While it seems the original copying issue is solved, there is an issue later during code signing, seemingly caused by the Qt framework not copied correctly:

The error (sanitized):

  • signing         file=dist/mac/SanitizedAppName.app platform=darwin type=distribution identity=<SanitizedIdentity> provisioningProfile=none
  • Above command failed, retrying 3 more times
  • Above command failed, retrying 3 more times
  • Above command failed, retrying 3 more times
  • Above command failed, retrying 3 more times
  ⨯ Command failed: codesign --sign <SanitizedIdentity> --force --timestamp --options runtime --entitlements ./build/entitlements.mac.plist /Users/<project-folder>/dist/mac/SanitizedAppName.app/Contents/Resources/app.asar.unpacked/node_modules/<sanitized-dep-name>/lib/Release/QtCore.framework
/Users/<project-folder>/dist/mac/SanitizedAppName.app/Contents/Resources/app.asar.unpacked/node_modules/<sanitized-dep-name>/lib/Release/QtCore.framework: bundle format unrecognized, invalid, or unsuitable
  failedTask=build stackTrace=Error: Command failed: codesign --sign <SanitizedIdentity> --force --timestamp --options runtime --entitlements ./build/entitlements.mac.plist /Users/<project-folder>/dist/mac/SanitizedAppName.app/Contents/Resources/app.asar.unpacked/node_modules/<sanitized-dep-name>/lib/Release/QtCore.framework
/Users/<project-folder>/dist/mac/SanitizedAppName.app/Contents/Resources/app.asar.unpacked/node_modules/<sanitized-dep-name>/lib/Release/QtCore.framework: bundle format unrecognized, invalid, or unsuitable
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         at genericNodeError (node:internal/errors:984:15)
    at wrappedFn (node:internal/errors:538:14)
    at ChildProcess.exithandler (node:child_process:422:12)
    at ChildProcess.emit (node:events:519:28)
    at maybeClose (node:internal/child_process:1105:16)
    at Socket.<anonymous> (node:internal/child_process:457:11)
    at Socket.emit (node:events:519:28)
    at Pipe.<anonymous> (node:net:339:12)
From previous event:
    at processImmediate (node:internal/timers:483:21)
From previous event:
    at readDirectoryAndSign (/Users/<project-folder>/node_modules/app-builder-lib/src/macPackager.ts:496:29)
    at MacPackager.signApp (/Users/<project-folder>/node_modules/app-builder-lib/src/macPackager.ts:506:11)
    at MacPackager.doSignAfterPack (/Users/<project-folder>/node_modules/app-builder-lib/src/platformPackager.ts:414:21)
    ...

I inspected the folder in question and this is what I see:
image

As you can see some folders are missing compared to a previous version we used.

And after I wrote this, I tested with 26.0.0-alpha.1 - it is working fine in this version. So it seems the issue introduced in 26.0.0-alpha.2 is still not fully fixed in 26.0.0-alpha.6.

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 7, 2024

Thank you for the detailed report! I'll take a look at this asap

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 7, 2024

Hmmm, I'm unable to reproduce with the sym-link-demo.zip project for some reason. Can you confirm if this newer issue is in your private project or also in the demo project you provided?

  • executing @electron/rebuild  electronVersion=33.0.2 arch=arm64 buildFromSource=false appDir=./
  • installing native dependencies  arch=arm64
  • completed installing native dependencies
  • packaging       platform=darwin arch=arm64 electron=33.0.2 appOutDir=dist/mac-arm64
  • default Electron icon is used  reason=application icon is not set
  • signing         file=dist/mac-arm64/sym-link-demo.app platform=darwin type=distribution identity=<ID> provisioningProfile=none
  • skipped macOS notarization  reason=`notarize` options were unable to be generated

I'll keep hammering at at investigating as well

@ivanggq
Copy link
Author

ivanggq commented Nov 7, 2024

Right, I also did not repro the issue with the demo project but then I noticed that there is no app.asar.unpacked produced.
I added electron-builder.yml with contents asarUnpack: "node_modules/stripped-native-dep/**/*" and the issue reproduces for me now.

@beyondkmp
Copy link
Collaborator

It appears to be an issue with electron/asar, where symlink files are placed directly into app.asar without following the asarUnpack rules.

https://github.com/electron/asar/blob/4df48cd53e68299db9ebf8f3bf002cc1fc5111f7/src/asar.ts#L144-L166

@ivanggq
Copy link
Author

ivanggq commented Nov 8, 2024

If this is the case, what would be the path forward? Maybe revert the migration to electron/asar for now, or try to fix it?

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 8, 2024

The path forward will definitely not be a revert.

@beyondkmp, how is it a bug in electron/asar code? We'll need to fix it if so

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 8, 2024

Hmmmm, running asar extract on the generated asar file returns the correct folder tree, so that means those other symlinks must be getting embedded in the asar directly (instead of getting unpacked?)

~/D/sym-link-demo> asar extract "./dist/mac-arm64/Electron.app/Contents/Resources/app.asar" ./test-asar
~/D/sym-link-demo> tree ./test-asar
./test-asar
├── main.js
├── node_modules
│   └── stripped-native-dep
│       ├── lib
│       │   └── Release
│       │       ├── QtCore.framework
│       │       │   ├── QtCore -> Versions/Current/QtCore
│       │       │   ├── Resources -> Versions/Current/Resources
│       │       │   └── Versions
│       │       │       ├── A
│       │       │       │   ├── QtCore
│       │       │       │   └── Resources
│       │       │       │       ├── Info.plist
│       │       │       │       └── QtCore.prl
│       │       │       └── Current -> A
│       │       ├── QtNetwork.framework
│       │       │   ├── QtNetwork -> Versions/Current/QtNetwork
│       │       │   ├── Resources -> Versions/Current/Resources
│       │       │   └── Versions
│       │       │       ├── A
│       │       │       │   ├── QtNetwork
│       │       │       │   └── Resources
│       │       │       │       ├── Info.plist
│       │       │       │       └── QtNetwork.prl
│       │       │       └── Current -> A
│       │       └── qt-plugins
│       │           └── tls
│       │               └── libqsecuretransportbackend.dylib
│       └── package.json
├── package.json
└── stripped-native-dep
    ├── lib
    │   └── Release
    │       ├── QtCore.framework
    │       │   ├── QtCore -> Versions/Current/QtCore
    │       │   ├── Resources -> Versions/Current/Resources
    │       │   └── Versions
    │       │       ├── A
    │       │       │   ├── QtCore
    │       │       │   └── Resources
    │       │       │       ├── Info.plist
    │       │       │       └── QtCore.prl
    │       │       └── Current -> A
    │       ├── QtNetwork.framework
    │       │   ├── QtNetwork -> Versions/Current/QtNetwork
    │       │   ├── Resources -> Versions/Current/Resources
    │       │   └── Versions
    │       │       ├── A
    │       │       │   ├── QtNetwork
    │       │       │   └── Resources
    │       │       │       ├── Info.plist
    │       │       │       └── QtNetwork.prl
    │       │       └── Current -> A
    │       └── qt-plugins
    │           └── tls
    │               └── libqsecuretransportbackend.dylib
    └── package.json

36 directories, 22 files

@mmaietta
Copy link
Collaborator

mmaietta commented Nov 9, 2024

@ivanggq can you please try out this patch-package? If it works, I'll push the patch upstream for review by the project's maintainers. So far, it worked correctly in both my local test project, the sym-link-demo, and in the unit tests I've written for @electron/asar package
@electron+asar+3.2.17.patch

diff --git a/node_modules/@electron/asar/lib/asar.js b/node_modules/@electron/asar/lib/asar.js
index 12272d9..1598c99 100644
--- a/node_modules/@electron/asar/lib/asar.js
+++ b/node_modules/@electron/asar/lib/asar.js
@@ -87,6 +87,7 @@ async function createPackageFromFiles(src, dest, filenames, metadata = {}, optio
     });
     const filesystem = new filesystem_1.Filesystem(src);
     const files = [];
+    const links = [];
     const unpackDirs = [];
     let filenamesSorted = [];
     if (options.ordering) {
@@ -162,14 +163,23 @@ async function createPackageFromFiles(src, dest, filenames, metadata = {}, optio
                 files.push({ filename: filename, unpack: shouldUnpack });
                 return filesystem.insertFile(filename, shouldUnpack, file, options);
             case 'link':
-                filesystem.insertLink(filename);
+                shouldUnpack = false;
+                if (options.unpack) {
+                    shouldUnpack = (0, minimatch_1.default)(filename, options.unpack, { matchBase: true });
+                }
+                if (!shouldUnpack && options.unpackDir) {
+                    const dirName = path.relative(src, path.dirname(filename));
+                    shouldUnpack = isUnpackedDir(dirName, options.unpackDir, unpackDirs);
+                }
+                links.push({ filename, unpack: shouldUnpack });
+                filesystem.insertLink(filename, shouldUnpack);
                 break;
         }
         return Promise.resolve();
     };
     const insertsDone = async function () {
         await wrapped_fs_1.default.mkdirp(path.dirname(dest));
-        return disk.writeFilesystem(dest, filesystem, files, metadata);
+        return disk.writeFilesystem(dest, filesystem, files, links, metadata);
     };
     const names = filenamesSorted.slice();
     const next = async function (name) {
diff --git a/node_modules/@electron/asar/lib/disk.js b/node_modules/@electron/asar/lib/disk.js
index 5b89f89..ddbd768 100644
--- a/node_modules/@electron/asar/lib/disk.js
+++ b/node_modules/@electron/asar/lib/disk.js
@@ -55,7 +55,7 @@ async function streamTransformedFile(originalFilename, outStream, transformed) {
         stream.on('end', () => resolve());
     });
 }
-const writeFileListToStream = async function (dest, filesystem, out, fileList, metadata) {
+const writeFileListToStream = async function (dest, filesystem, out, fileList, linkList, metadata) {
     for (const file of fileList) {
         if (file.unpack) {
             // the file should not be packed into archive
@@ -66,9 +66,19 @@ const writeFileListToStream = async function (dest, filesystem, out, fileList, m
             await streamTransformedFile(file.filename, out, metadata[file.filename].transformed);
         }
     }
+    for (const file of linkList) {
+        if (file.unpack) {
+            // the symlink needs to be recreated outside in asar.unpacked
+            const filename = path.relative(filesystem.getRootPath(), file.filename);
+            const srcFile = path.join(filesystem.getRootPath(), filename);
+            const destFile = path.join(`${dest}.unpacked`, filename);
+            const link = wrapped_fs_1.default.readlinkSync(srcFile);
+            wrapped_fs_1.default.symlinkSync(link, destFile);
+        }
+    }
     return out.end();
 };
-async function writeFilesystem(dest, filesystem, fileList, metadata) {
+async function writeFilesystem(dest, filesystem, fileList, linkList, metadata) {
     const headerPickle = pickle_1.Pickle.createEmpty();
     headerPickle.writeString(JSON.stringify(filesystem.getHeader()));
     const headerBuf = headerPickle.toBuffer();
@@ -81,7 +91,7 @@ async function writeFilesystem(dest, filesystem, fileList, metadata) {
         out.write(sizeBuf);
         return out.write(headerBuf, () => resolve());
     });
-    return writeFileListToStream(dest, filesystem, out, fileList, metadata);
+    return writeFileListToStream(dest, filesystem, out, fileList, linkList, metadata);
 }
 function readArchiveHeaderSync(archivePath) {
     const fd = wrapped_fs_1.default.openSync(archivePath, 'r');
diff --git a/node_modules/@electron/asar/lib/filesystem.js b/node_modules/@electron/asar/lib/filesystem.js
index 015bd33..d228240 100644
--- a/node_modules/@electron/asar/lib/filesystem.js
+++ b/node_modules/@electron/asar/lib/filesystem.js
@@ -134,7 +134,7 @@ class Filesystem {
         }
         this.offset += BigInt(size);
     }
-    insertLink(p) {
+    insertLink(p, shouldUnpack) {
         const symlink = wrapped_fs_1.default.readlinkSync(p);
         // /var => /private/var
         const parentPath = wrapped_fs_1.default.realpathSync(path.dirname(p));
@@ -143,6 +143,10 @@ class Filesystem {
             throw new Error(`${p}: file "${link}" links out of the package`);
         }
         const node = this.searchNodeFromPath(p);
+        const dirNode = this.searchNodeFromPath(path.dirname(p));
+        if (shouldUnpack || dirNode.unpacked) {
+            node.unpacked = true;
+        }
         node.link = link;
         return link;
     }

@ivanggq
Copy link
Author

ivanggq commented Nov 9, 2024

@mmaietta , this patch fixes the issue for our project. It builds and signs successfully. I compared the produced asar.unpacked and other output files - the produced folders/files are the same as the older electron-builder version v23.6.0 that we use currently. So the fix seems good to me. Good job.

mmaietta added a commit to mmaietta/asar that referenced this issue Nov 11, 2024
… previously unpacked directories. This directly fixes unpacking static `.framework` modules on Mac, as otherwise codesigning will fail due to symlink files/directories not being reflected in the app.asar.unpacked directory.

Added unit test with Hello.framework, generated from tutorial https://jano.dev/apple/mach-o/2024/06/28/Hello-Static-Framework.html
Fixes: electron-userland/electron-builder#8655
@beyondkmp
Copy link
Collaborator

beyondkmp commented Nov 11, 2024

    for (const file of linkList) {
        if (file.unpack) {
            // the symlink needs to be recreated outside in asar.unpacked
            const filename = path.relative(filesystem.getRootPath(), file.filename);
             const srcFile = path.join(filesystem.getRootPath(), filename);
             const destFile = path.join(`${dest}.unpacked`, filename);
             const link = wrapped_fs_1.default.readlinkSync(srcFile);
             wrapped_fs_1.default.symlinkSync(link, destFile);
         }
     }

@mmaietta We should check if the link is an absolute path.

      let link = await readlink(file)
      if (path.isAbsolute(link)) {
        link = path.relative(path.dirname(file), link)
      }

@mmaietta
Copy link
Collaborator

AFAICT links within asars can't be absolute paths, otherwise it'll fail during extraction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants