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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

commented May 14, 2019

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

猬嗭笍 @atom/notify@1.3.3
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

This comment has been minimized.

Copy link
Member

commented May 14, 2019

Can confirm this fixes the issue 馃憤

@nathansobo

This comment has been minimized.

Copy link
Contributor Author

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

1 of 2 checks passed

Atom Pull Requests #20190514.4 failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nathansobo nathansobo deleted the ns-as/notify-snapshot-exclude branch May 14, 2019

nathansobo added a commit that referenced this pull request May 17, 2019

Revert "Merge pull request #19331 from atom/ns-as/notify-snapshot-exc鈥
鈥ude"

This reverts commit 1e08ad8, reversing
changes made to 0994d8a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.