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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doesnt work in Electron apps that have used .asar to package themselves #94

Closed
pfrazee opened this issue Aug 10, 2016 · 10 comments
Closed
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@pfrazee
Copy link

pfrazee commented Aug 10, 2016

See electron/electron#2708

The startDaemon() call never returns error or success. I'm fairly sure the issue is as in 2708: the relative paths fail.

@pfrazee
Copy link
Author

pfrazee commented Aug 10, 2016

I solved this by not using .asar. I havent figured out why Electron folks bother with the .asar, so I'm not sure what the downside is for my change.

@jbenet
Copy link
Member

jbenet commented Aug 11, 2016

(My comment is Off topic)

bother with .asar

I think it's supposed to be a bit better than tar, less error prone (stream
cutoff errors) and packed, etc. similar to our desires for .car

On Wed, Aug 10, 2016 at 17:45 Paul Frazee notifications@github.com wrote:

I solved this by not using .asar. I havent figured out why Electron folks
bother with the .asar, so I'm not sure what the downside is for my change.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#94 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIcoUDfbVh6U8Qmrem3iiS97D21ytfYks5qekZ5gaJpZM4Jhm6p
.

@pfrazee
Copy link
Author

pfrazee commented Aug 11, 2016

Yeah but there's no compression in .asar, and osx is already bundled into a
dmg. Maybe it's for windows and linux? not sure

On Wed, Aug 10, 2016 at 8:47 PM, Juan Benet notifications@github.com
wrote:

(My comment is Off topic)

bother with .asar

I think it's supposed to be a bit better than tar, less error prone (stream
cutoff errors) and packed, etc. similar to our desires for .car

On Wed, Aug 10, 2016 at 17:45 Paul Frazee notifications@github.com
wrote:

I solved this by not using .asar. I havent figured out why Electron folks
bother with the .asar, so I'm not sure what the downside is for my
change.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#94 (comment),
or mute the thread
<https://github.com/notifications/unsubscribe-auth/
AAIcoUDfbVh6U8Qmrem3iiS97D21ytfYks5qekZ5gaJpZM4Jhm6p>
.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#94 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABNhU2A78kzo4pA_gWi8JuWB6TkZLmJpks5qen85gaJpZM4Jhm6p
.

@pfrazee
Copy link
Author

pfrazee commented Aug 12, 2016

Aha:

To mitigate issues around long path names on Windows, slightly speed up require and conceal your source code from cursory inspection, you can choose to package your app into an asar archive with little changes to your source code.

http://electron.atom.io/docs/tutorial/application-packaging/

@pfrazee
Copy link
Author

pfrazee commented Aug 12, 2016

It turns out, they'd even documented this specific issue: http://electron.atom.io/docs/tutorial/application-packaging/#executing-binaries-inside-asar-archive

@daviddias
Copy link
Member

@haadcode do you run into this issue with orbit? How did you solve it?

@haadcode
Copy link
Member

haadcode commented Dec 5, 2016

@diasdavid fixed it in the same way as @pfrazee did: not using asar.

@daviddias daviddias added kind/bug A bug in existing code (including security flaws) exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue status/deferred Conscious decision to pause or backlog and removed status/ready Ready to be worked labels Jan 29, 2017
@thisconnect
Copy link
Contributor

thisconnect commented Jun 22, 2017

It seems that the solution is to exclude go-ipfs-dep from node_modules during packaging. I got it working with electron-packager + the modification in PR #155 this PR is not a final solution yet. Only tested on MacOS

electron-packager ./ --overwrite --out ./dist --asar.unpackDir=node_modules/go-ipfs-dep/**

@daviddias daviddias added status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up and removed status/deferred Conscious decision to pause or backlog labels Oct 17, 2017
@daviddias
Copy link
Member

@thisconnect thanks for working on a interim solution and documenting it on the README 👍 https://github.com/ipfs/js-ipfsd-ctl#packaging

@thisconnect
Copy link
Contributor

Can this issue be closed?

@ghost ghost removed the status/ready Ready to be worked label Oct 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

5 participants