electron-builder icon indicating copy to clipboard operation
electron-builder copied to clipboard

"Cannot find module 'sharp'" on Windows only after upgrading to 26.0.11

Open Nantris opened this issue 9 months ago • 77 comments

After upgrading to 26.0.11 it seems Sharp isn't being properly bundled or run. None of our config changed except changing win.publisherName to win.signToolOptions.publisherName.

I'm not sure how to go further in investigating this. Any advice appreciated!

Nantris avatar Mar 17 '25 21:03 Nantris

Hiya! Can you share a minimum reproducible repo that I can test locally with and/or create a unit test around? Would like to assist but need more information first.

mmaietta avatar Mar 17 '25 22:03 mmaietta

Hey @mmaietta thanks for your reply.

I did find the problem - in files we had this rule, which worked fine in 25.x but not in 26.x (Sharp version didn't change):

!**/node_modules/sharp/install/*

Simply removing this is enough of a fix for us and probably only adds a tiny bit to the package size, but I do wonder why that would now be breaking in 26.x (and seemingly only on Windows.)

Thanks again!

Nantris avatar Mar 17 '25 23:03 Nantris

Actually, I must have overlooked something because that's definitely not resolving the issue now...

Nantris avatar Mar 18 '25 01:03 Nantris

Hm... I really thought I had this working but every attempt now results in this same issue. I'm not sure if I changed something else or if what I thought fixed it never really did. I'll update soon.

Nantris avatar Mar 18 '25 02:03 Nantris

@mmaietta I did notice that there's nothing under app.asar.unpacked from Sharp (or any other Node dependencies.

Nantris avatar Mar 18 '25 03:03 Nantris

Yeahhhh, definitely need a minimum repro repo for this unfortunately. The node collector logic was completely migrated to TS from Golang to support pnpm amongst other things, and a migration to electron/asar in order to support electron/fuses integration

mmaietta avatar Mar 18 '25 03:03 mmaietta

Thanks as always for the great work and for your help @mmaietta!


I put together a repro here: https://github.com/Nantris/electron-builder-repro-8970

It requires Yarn v4 (which I think should be used automatically as long as you have corepack enabled?)

Repro steps:

  1. yarn install
  2. yarn package
  3. Run executable in win-unpacked

This isn't an exact copy of our config and may not be 100% correct in every regard, but it should at least output the app.asar.unpacked folder I would think. Alas, it does not.

Nantris avatar Mar 18 '25 22:03 Nantris

Well this is odd indeed. Your .asar doesn't have any node modules dir present. What version of electron-builder are you upgrading from? I'm wondering if this is a yarn 4-specific bug

mmaietta avatar Mar 19 '25 17:03 mmaietta

Thanks as always for all you do @mmaietta!

I was wondering if it might be related to Yarn 4 myself.

We upgraded from 25.1.8 and I believe I verified this affects 26.0.0 specifically but I'm not certain now.

It's super strange to me that while Windows fails, Linux builds remain unaffected. I haven't had a chance to test macOS. Maybe there's some sort of path issue that makes this only work on POSIX?

Nantris avatar Mar 19 '25 19:03 Nantris

I confirmed macOS is unaffected. So only Windows has the problem.

I also tried setting supportedArchitectures in .yarnrc.yml even though it works in dev, and as expected, no difference.

Nantris avatar Mar 19 '25 19:03 Nantris

I am experiencing this issue too, but with a different dependency. this works with 26.0.2 (before the node_module collector change), but not with 26.0.11. I am using yarn 4.7.0 Reported by a windows user, but it looks like it affects all our builds.

If I look inside the app.asar, there is no node_modules folder at all, we have 10+ direct dependencies that should be included there.

I should add a disclaimer that we are using a lightly patched app-builder-lib, but that should only be including more node_modules than usual loose inside the resources directory.

The snippet of our config that I expect could affect this is:

files: ['**/*', 'assets/*'],
			extraResources: [
				{
					from: '../dist',
					to: '.',
					filter: ['**/*', '!.yarn'],
				},
			],

Julusian avatar Mar 20 '25 09:03 Julusian

@beyondkmp can you please take a look at this when you have a chance? Seems there's an issue specifically on Windows, and it may be specific to yarn 4.

I'm trying to write a unit test from my side, but once corepack is enabled in the docker container to support a yarn4-scoped test, subsequent runs of other unit tests break due to corepack now being enabled.

mmaietta avatar Mar 20 '25 16:03 mmaietta

Image

Confirming that the node module collection logic is not pulling any node modules when using yarn 4, even on macOS dir target. For some reason, npm list -a --include prod --include optional --omit dev --json --long --silent returns an empty dependency tree when running from the cwd, whereas running it from root seems to work correctly

mmaietta avatar Mar 20 '25 17:03 mmaietta

@mmaietta that's very strange! Because our macOS build worked unless I'm crazy.

Edit: Reconfirming macOS built properly with [email protected]

Nantris avatar Mar 20 '25 18:03 Nantris

Hmmm, I'm running off latest master

I was able to resolve the dependency collection by changing the rootDir to no longer be the app dir, but the project root dir, however, now JSON.parse fails on parsing the scripts response due to malformed JSON in the npm list CLI output

"concurrently \\"npm run build:main\\" \\"npm run build:renderer\\""

Trying to figure out how to get around this issue now, as at least the dependency tree is being returned correctly. Suggestions welcome as I'm still pretty sick and my mind is an absolute ball of fuzz right now

mmaietta avatar Mar 20 '25 19:03 mmaietta

I'm getting similar problem on 26.0.11 - multiworkspace project, unpacking asar shows there are no node_modules there at all.

stq avatar Mar 20 '25 19:03 stq

@stq, is that also a yarn 4 project?

mmaietta avatar Mar 20 '25 19:03 mmaietta

@stq, is that also a yarn 4 project?

No, npm9/node20. Downgrading to 26.0.2 fixed issue.

stq avatar Mar 20 '25 20:03 stq

@stq would you mind putting together a minimum reproducible repo since yours isn't yarn 4? I'd like to create a dedicated unit test for this multi-workspace project as we already have some two-package.json app fixtures, but it seems like we're missing a use case.

mmaietta avatar Mar 20 '25 21:03 mmaietta

@Nantris can you try this patch-package? It works all of a sudden for me on macos target, still need to setup a test env in a windows VM though patches/app-builder-lib+26.0.11.patch

diff --git a/node_modules/app-builder-lib/out/util/appFileCopier.js b/node_modules/app-builder-lib/out/util/appFileCopier.js
index 79dee3f..3308f61 100644
--- a/node_modules/app-builder-lib/out/util/appFileCopier.js
+++ b/node_modules/app-builder-lib/out/util/appFileCopier.js
@@ -144,7 +144,7 @@ function validateFileSet(fileSet) {
 }
 /** @internal */
 async function computeNodeModuleFileSets(platformPackager, mainMatcher) {
-    const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.info.appDir);
+    const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.projectDir);
     builder_util_1.log.debug({ nodeModules: deps }, "collected node modules");
     const nodeModuleExcludedExts = getNodeModuleExcludedExts(platformPackager);
     // serial execution because copyNodeModules is concurrent and so, no need to increase queue/pressure

mmaietta avatar Mar 20 '25 21:03 mmaietta

@mmaietta I'm sorry to report it didn't resolve the issue. But macOS always worked for us, so maybe there are two issues here? It's pretty confusing.

Nantris avatar Mar 20 '25 22:03 Nantris

Ok that is weird, as it looks like it packaged correctly for me with just that change. Launches just fine too. Image

mmaietta avatar Mar 21 '25 02:03 mmaietta

@mmaietta you're testing on the master branch, right? Unfortunately I can't get that working here, apparently due to this issue, so I guess we may be testing slightly different setups.

That image is from macOS, right?

Nantris avatar Mar 21 '25 03:03 Nantris

@Nantris can you try this patch-package? It works all of a sudden for me on macos target, still need to setup a test env in a windows VM though patches/app-builder-lib+26.0.11.patch

diff --git a/node_modules/app-builder-lib/out/util/appFileCopier.js b/node_modules/app-builder-lib/out/util/appFileCopier.js
index 79dee3f..3308f61 100644
--- a/node_modules/app-builder-lib/out/util/appFileCopier.js
+++ b/node_modules/app-builder-lib/out/util/appFileCopier.js
@@ -144,7 +144,7 @@ function validateFileSet(fileSet) {
 }
 /** @internal */
 async function computeNodeModuleFileSets(platformPackager, mainMatcher) {
-    const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.info.appDir);
+    const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.projectDir);
     builder_util_1.log.debug({ nodeModules: deps }, "collected node modules");
     const nodeModuleExcludedExts = getNodeModuleExcludedExts(platformPackager);
     // serial execution because copyNodeModules is concurrent and so, no need to increase queue/pressure

This will include all dependencies from projectDir, while the dependencies in appDir should only be a subset of projectDir's dependencies and don't need to be entirely included.

beyondkmp avatar Mar 21 '25 03:03 beyondkmp

@mmaietta you're testing on the master branch, right? Unfortunately I can't get that working here, https://github.com/yarnpkg/berry/issues/5715, so I guess we may be testing slightly different setups. That image is from macOS, right?

Should be the same setup. That was on a default yarn install from a fresh git clone, then just patched the appFileCopier.js directly within node_modules/app-builder-lib. Built within a Windows arm64 VM

mmaietta avatar Mar 21 '25 07:03 mmaietta

@mmaietta this is my bad, but actually even more confusing. Originally I tested against our project and not against the repro.

The patch works on the repro :)

Does not work on our project :(


Edit 1: Investigating further. If I come across anything I'll update. Edit 2: I updated our config to match the repro as closely as I can and I can't find any discrepancies, but our real project still fails to bundle node_modules. I guess I need to next test the repro on macOS to see if that fails for me as it did for you @mmaietta, and beyond that I'm at a total loss.

Nantris avatar Mar 21 '25 18:03 Nantris

Oh lordy, this is becoming a conundrum n' a half.

I was going to suggest this as an easy fix. Checks the appDir (current behavior) and if no dependencies found, it has a fallback up the tree to the projectDir

export async function getNodeModules(projectDir: string, appDir: string): Promise<NodeModuleInfo[]> {
  const collector = await getCollectorByPackageManager(appDir)
  let deps = await collector.getNodeModules()
  if (!deps.length && appDir !== projectDir) {
    deps = await (await getCollectorByPackageManager(projectDir)).getNodeModules()
  }
  return deps
}

But since it doesn't work for your original/source project, I'm not sure where to go from here either.

Might need @stq to get back to us on a minimum repro project so I can do a deeper dive since this doesn't seem isolated to yarn 4. Might be due to multi-workspace setups, which is my hunch, but I need a local setup to repro and write a unit test for. Otherwise I'm just stabbing in the dark to find a needle in a haystack 😅

I think it may be useful to add a log.warn when there's no deps found to prevent the issue going unnoticed by unsuspecting victims

mmaietta avatar Mar 21 '25 19:03 mmaietta

The repro I provided does indeed fail for me on macOS, whereas my project does not.


Might be due to multi-workspace setups, which is my hunch

Seems very possible, since our project is indeed a Yarn workspace, but all the native deps are at the project root so one wouldn't think it would be the cause, but it's the most promising theory I'd say.

The only other differences I can find are:

  1. Our build config file is nested in a directory in our real project whereas it's in the project root in the repro, but I don't think there's any plausible way that could make a difference?
  2. We have additional native dependencies.

I noticed some debug statements in app-builder-lib. I wonder if I can force those to output and try to find any discrepancies between the repro and our real project?

For now I guess we're going to stick with 26.0.2 and pin hopes on an independent repro hopefully pointing to whatever this other issue is.

Nantris avatar Mar 21 '25 19:03 Nantris

I noticed some debug statements in app-builder-lib. I wonder if I can force those to output and try to find any discrepancies between the repro and our real project?

Set env var DEBUG=electron-builder and extract the relevant logs maybe? Might be a bit long, so try using markdown to have it a collapsible section

mmaietta avatar Mar 21 '25 19:03 mmaietta

Related note: Once we iron this issue out, I think we'll be able to promote 26.0.12 to latest and see what issues come in. We already have a solid % of downloads on 26.0.2+ (post node-collector migration).

Post-rollout, I'll also begin working on creating a new electron/ repo to migrate our parsing logic there for broader availability to the electron ecosystem. 💪

mmaietta avatar Mar 21 '25 21:03 mmaietta