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

Exclude @atom/notify binary from the ASAR bundle #19325

Merged
merged 3 commits into from May 13, 2019

Conversation

@nathansobo
Copy link
Contributor

commented May 13, 2019

馃崘'd with @jasonrudolph

Fixes #19311

In #19244, I included a dependency on @atom/notify to support watching the file system. It spawns an external binary, which worked fine in tests and in dev mode, but broke when the module was packaged in an ASAR archive. If you want to spawn a process from a node module that is inside an ASAR archive, that binary must be on the physical file system. Electron's file system hacks support require and other file system operations, but they don't support spawn.

This PR updates the @atom/notify dependency to support transforming the binary path to account for the module being packaged in a separate location (atom.asar/@atom/notify) than the binary it is trying to spawn (atom.asar.unpacked/@atom/notify/bin).

@rafeca

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

Nice! 馃帀

@jasonrudolph

This comment has been minimized.

Copy link
Member

commented May 13, 2019

We have one CI failure on macOS on Azure DevOps, but it's a flake that we're tracking in atom/fuzzy-finder#386, so I'm gonna go ahead and merge. 馃槆

@jasonrudolph jasonrudolph merged commit da8b1a1 into master May 13, 2019

1 of 2 checks passed

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

@jasonrudolph jasonrudolph deleted the ns/notify-asar-exclude branch May 13, 2019

@Arcanemagus Arcanemagus referenced this pull request May 13, 2019
1 of 1 task complete

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

Revert "Merge pull request #19325 from atom/ns/notify-asar-exclude"
This reverts commit da8b1a1, reversing
changes made to 1edf94a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.