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.

Sign up for GitHub

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

fix: electron-builder fails when list of node_modules files is too big to pass in a glob #8725

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

fix: electron-builder fails when list of node_modules files is too big to pass in a glob #8725

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f1078b
use dir pattern
beyondkmp Jan 9, 2025
d0938ac
add glob
beyondkmp Jan 9, 2025
9d85eb3
Merge branch 'master' into asarUnpackPattern
beyondkmp Jan 9, 2025
04fad09
add ut
beyondkmp Jan 10, 2025
4ac369a
Merge branch 'asarUnpackPattern' of github.com:beyondkmp/electron-bui…
beyondkmp Jan 10, 2025
86a6aa2
format code
beyondkmp Jan 10, 2025
e9470a6
add changeset
beyondkmp Jan 10, 2025
492a2d9
use relative path glob pattern
beyondkmp Jan 11, 2025
7be5628
use relative path glob pattern for all files
beyondkmp Jan 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions packages/app-builder-lib/src/asar/asarUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export class AsarPackager {
const links: Array<Link> = []
const symlinkType = platform() === "win32" ? "junction" : "file"

const matchUnpacker = (file: string, dest: string, stat: fs.Stats) => {
const matchUnpacker = (file: string, dest: string, stat: fs.Stats, tmpUnpackedPaths: Set<string>) => {
if (this.config.unpackPattern?.(file, stat)) {
log.debug({ file }, "unpacking")
unpackedPaths.add(dest)
tmpUnpackedPaths.add(dest)
return
}
}
Expand Down Expand Up @@ -145,6 +145,7 @@ export class AsarPackager {
}

// Don't use BluebirdPromise, we need to retain order of execution/iteration through the ordered fileset
const tmpUnpackedPaths = new Set<string>()
for (let i = 0; i < fileSet.files.length; i++) {
const file = fileSet.files[i]
const transformedData = fileSet.transformedFiles?.get(i)
Expand All @@ -153,13 +154,24 @@ export class AsarPackager {
const relative = path.relative(this.config.defaultDestination, getDestinationPath(file, fileSet))
const destination = path.resolve(this.rootForAppFilesWithoutAsar, relative)

matchUnpacker(file, destination, stat)
matchUnpacker(file, destination, stat, tmpUnpackedPaths)
taskManager.addTask(writeFileOrProcessSymlink({ transformedData, file, destination, stat, fileSet }))

if (taskManager.tasks.length > MAX_FILE_REQUESTS) {
await taskManager.awaitTasks()
}
}

if (tmpUnpackedPaths.size === fileSet.files.length) {
const relative = path.relative(this.config.defaultDestination, fileSet.destination)
const destination = path.resolve(this.rootForAppFilesWithoutAsar, relative)
unpackedPaths.add(`${destination}${path.sep}**`)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify this area of code with a comment? It looks like we're determining whether all files in the fileSet are being unpacked, if so, then we glob them together with an umbrella glob destination/** to reduce the number of unpacked paths that are being passed to minimatch?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly as you mentioned.

electron/asar doesn't implement ! filtering. Having to do the conversion at the electron-builder layer is very complicated, and there's currently no good method for conversion.

Therefore, we can only solve this problem from a different angle. Exceeding the length limit is always caused by filtering one or more directories. If we're just filtering specific files, it won't exceed the length limit. So solving these directory filtering issues would solve this problem.

Currently, there might still be some edge cases, such as when a directory contains tens of thousands of files, and the requirement is to put just a few files from this directory into asar while putting all other files into asar.unpack.
In practical use, this situation probably won't occur - it's rare to have a directory with tens of thousands of files where only a few files need to be in asar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For unpackedPaths.add(${destination}${path.sep}**), is it possible to use ${relative}${path.sep}** to shorten the minimatch paths even further? I can't recall off the top of my head whether it requires absolute paths or if relative works.

Copy link
Collaborator Author

@beyondkmp beyondkmp Jan 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. However, we need to use **${path.sep}${relative}${path.sep}**.

I've changed files to use this format as well, and current testing shows no issues.

electron-builder/packages/app-builder-lib/src/asar/asarUtil.ts

Lines 90 to 95 in 7be5628

const matchUnpacker = (file: string, dest: string, stat: fs.Stats, tmpUnpackedPaths: Set<string>) => {
if (this.config.unpackPattern?.(file, stat)) {
log.debug({ file }, "unpacking")
tmpUnpackedPaths.add(`**${path.sep}${dest}`)
return
}

} else {
// add all tmpUnpackedPaths to unpackedPaths
for (const it of tmpUnpackedPaths) {
unpackedPaths.add(it)
}
}
}
// finish copy then set up all symlinks
await taskManager.awaitTasks()
Expand Down
Loading