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

add option to disable warning about asar being off #982

Closed
wojtkowiak opened this issue Dec 7, 2016 · 11 comments · May be fixed by qcif/data-curator#563
Closed

add option to disable warning about asar being off #982

wojtkowiak opened this issue Dec 7, 2016 · 11 comments · May be fixed by qcif/data-curator#563
Labels

Comments

@wojtkowiak
Copy link
Contributor

Packaging using asar archive is disabled — it is strongly not recommended.

Can I add a PR with some env var that would disable this?
In meteor-desktop I am packing the app into three asars because I needed to split the app into three parts because of internal hot code push mechanism.
So the app is actually asared, and this warning is adding a little confusion in my case.

@develar
Copy link
Member

develar commented Dec 7, 2016

Is there a way to detect such packaging automatically?

@wojtkowiak
Copy link
Contributor Author

structure

Well theoretically, a simple detection could be just a simple lookout for asar (or asars files) in app directory. Or just a check for app.asar.


Since I have you on the line, I have something I would like to ask. I would like to benefit from your npmRebuild - but the problem is that I have the node_modules already packed into app.asar.
I do not like the idea that I am asking about modification that would only benefit my package but I guess I have no choice 😉

I would need an option to hook a callback before a build for a platform/arch is made, so that I could rebuild the node_modules myself and update them into app.asar. I am already calling your installOrRebuild here https://github.com/wojtkowiak/meteor-desktop/blob/master/lib/electronApp.js#L458 so I could just use it the same way. I would just need a callback like beforeBuild(platform, arch) that would be invoked before each build. I would then switch npmRebuild off and handle it myself. What do you think?

@develar
Copy link
Member

develar commented Dec 7, 2016

I would just need a callback like beforeBuild(platform, arch) that would be invoked before each build

Callback will be added.

@develar develar added the feature label Dec 9, 2016
@wojtkowiak
Copy link
Contributor Author

Great, I can not express how I appreciate that 👍 Really, creators and maintainers of various open source packages are often very reservedly when it comes to helping others integrate things with their packages and here you are always helpful and positive.

Can you estimate a timeline for this two changes? Can I help or do the PR? In the first case I think I know what to add and where, and in the second I am not so sure.

@develar
Copy link
Member

develar commented Dec 9, 2016

Can I help or do the PR?

Yes, it will be helpful.

I think I know what to add and where

Consider to use statOrNull and check that file is really file. https://github.com/electron-userland/electron-builder/blob/master/src/util/fs.ts

in the second I am not so sure.

I suppose, callback should return Promise and we should wait promise completion. Promise result is boolean — false to not perform build, and true to build.

@wojtkowiak
Copy link
Contributor Author

Thanks for the tips.
I have just started with this.

use added callback option in the https://github.com/electron-userland/electron-builder/blob/master/src/yarn.ts somehow. options: BuildMetadata is already available in the method installOrRebuild.

So instead of beforeBuild you would rather see sth like beforeYarnInstallOrRebuild? That would be more tailored for the situation I guess.

false to not perform build, and true to build.

So this would not condition whether to build but rather whether to run installOrRebuild, right?

@wojtkowiak
Copy link
Contributor Author

wojtkowiak commented Dec 14, 2016

Can I assume that there is an app directory always?
wojtkowiak@182d786#diff-f3f55770caf14ea02d69896e0b071d88R320

@develar
Copy link
Member

develar commented Dec 15, 2016

@wojtkowiak No, on computeAsarOptions files is not yet copied to resourcesPath. You should check you source project directory, not dist.

@develar
Copy link
Member

develar commented Dec 15, 2016

So instead of beforeBuild you would rather see sth like beforeYarnInstallOrRebuild? That would be more tailored for the situation I guess.

beforeYarnInstallOrRebuild is too long name :) beforeBuild is ok here, no?

@wojtkowiak
Copy link
Contributor Author

Finally got time to get back to this.
Please take a look at the PR. I already have a meteor-desktop prepared to use the beforeBuild callback and it works like a charm :)

@develar develar closed this as completed in 35a8cdf Jan 5, 2017
@wojtkowiak
Copy link
Contributor Author

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants