Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Fix @atom/notify subprocess path resolution when V8 startup snapshot is in effect #19331

Merged
merged 1 commit into from
May 14, 2019

Conversation

nathansobo
Copy link
Contributor

Fixes #19311 (for real this time)

In #19325, I excluded @atom/notify's subprocess binary from the ASAR bundle, but didn't realize there was another problem prohibiting spawning, which was the fact that __dirname is invalid inside of the V8 startup snapshot.

In this PR, we move resolution of the subprocess binary path to a separate file and exclude it from the snapshot to prevent this issue.

🍐'd with @as-cii

You can't rely on `__dirname` inside a snapshotted file. Now path 
resolution takes place inside a specific file that is excluded from the 
startup snapshot.
@50Wliu
Copy link
Contributor

50Wliu commented May 14, 2019

Can confirm this fixes the issue 👍

@nathansobo
Copy link
Contributor Author

nathansobo commented May 14, 2019

Another spurious fuzzy finder failure, which is being tracked in atom/fuzzy-finder#386.

@nathansobo nathansobo merged commit 1e08ad8 into master May 14, 2019
@nathansobo nathansobo deleted the ns-as/notify-snapshot-exclude branch May 14, 2019 20:42
nathansobo pushed a commit that referenced this pull request May 17, 2019
…lude"

This reverts commit 1e08ad8, reversing
changes made to 0994d8a.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to spawn notify subprocess on Windows
2 participants