cordova-electron
cordova-electron copied to clipboard
Invalid mas build?
I'm running a version of cordova-electron forked from 1.1.1, with the only change being that Electron is updated to 6.1.7 as you can see here. As far as I can tell based on the output below, this works as intended.
However, my package is being rejected by the Mac App Store: https://github.com/electron/electron/issues/20027#issuecomment-585545947. The replies from the Electron maintainers suggest that this is likely an issue with cordova-electron:
dirty build with dmg in it . . . I'm saying you didn't use the
mas
build. Whatever tooling you are using is not using themas
build of Electron as that symbol list is basically a history lesson in all the symbols we've removed in the past.
Any ideas?
[Cordova Electron] The built package size may be larger than necessary. Please run with --verbose for more details.
• electron-builder version=22.3.2 os=19.2.0
• writing effective config file=platforms/electron/build/builder-effective-config.yaml
• installing production dependencies platform=darwin arch=x64 appDir=/Users/buu700/cyph/cyph-phonegap-build/platforms/electron/www
• packaging platform=darwin arch=x64 electron=6.1.7 appOutDir=platforms/electron/build/mac
• packaging platform=mas arch=x64 electron=6.1.7 appOutDir=platforms/electron/build/mas
• building target=DMG arch=x64 file=platforms/electron/build/Cyph-1.0.42.dmg
• signing file=platforms/electron/build/mas/Cyph.app identityName=3rd Party Mac Developer Application: Cyph, Inc. (SXZZ8WLPV2) identityHash=C8AE49EA521129CB8F6E7EFC648A04CC9385C411 provisioningProfile=/Users/buu700/.cyph/nativereleasesigning/apple/macOS_Distribution.provisionprofile
• building block map blockMapFile=platforms/electron/build/Cyph-1.0.42.dmg.blockmap
The Electron and builder dependencies were already updated in the master branch. The master branch is in the process of being prepared for a major release.
There is currently no ETA on the release date.
Releasing a major takes time to prepare as there are many other repos (core tools) that need to be prepared and released before we can release platforms.
As for the commit on your forked branch where you are adding mas
as a default target package type, we would like to keep the default target to dmg. But, You can config the build to include mas
. Please take a look at our docs first as it talks about how to change/add other target package types.
https://cordova.apache.org/docs/en/latest/guide/platforms/electron/index.html
Sorry that the major release process is taking a long time, as there are a lot of moving parts.
Thanks for the quick reply @erisu. Yes, I was previously using the nightly version, but I had to do this in order to get Electron version 6.1.7, as only the most recent 6.x releases (and not 7.x or above) currently include the patch to work in the Mac App Store.
I should mention that I also tested forking master to use Electron 6.1.7, with the same result.
Sorry for the confusion with the commit you mentioned; I should have explained that. Yes, I'm already using the mas
build (https://github.com/cyph/app/blob/master/build.json). That was an unsuccessful test for @JCBsystem and @MarshallOfSound's suggestion that the error I'm seeing was due to cordova-electron somehow incorrectly configuring the mas
build, rather than an issue in Electron itself.
I was able to build a mas
target in the past when I tested.
That most likely would have been around the time we added the information to the docs. I wouldn't say we configured it wrong but I also can not remember if there was changes around the logic that did the translation from build.json
to electron-builder
's configuration format.
Most likely there might not have been changes.
There are also possibilities that it could have broken naturally over time. For example, Apple had began enforcing notarizing build. With this, some known issues appeared. The next major will also have the notarizing fix.
When I get some free time, I can try to build for the mas
target, with current master (nightly) and current release.
Just to clarify, if this part wasn't clear, building and signing the mas
target appears to work; the issue is that it's being rejected from the Mac App Store, which I'm told by the Electron team is because there's something wrong with the generated .pkg file.
Specifically @MarshallOfSound says that the .pkg file isn't actually a mas
build, although I'm still unclear on what else it would be. Does that sound plausible? Or maybe cordova-electron is just incorrectly generating a debug build even when --release
is specified?
The log output your showing says it is using the 3rd Party Mac Developer Application
for signing. That seems correct as it should be the Mac App Distribution..
• signing file=platforms/electron/build/mas/Cyph.app identityName=3rd Party Mac Developer Application: Cyph, Inc. (SXZZ8WLPV2) identityHash=C8AE49EA521129CB8F6E7EFC648A04CC9385C411 provisioningProfile=/Users/buu700/.cyph/nativereleasesigning/apple/macOS_Distribution.provisionprofile
When you asked on Electron/Electron-Builder GH, did you show them your platforms/electron/build/builder-effective-config.yaml
file? That would be their config file.
That's good to know, thanks! I didn't, but I'll find and post that in the other thread today.
Okay, we narrowed things down a bit over in the Electron issue thread. Turns out that the electron
npm package being included in app.asar.unpacked is the problem. Is cordova-electron doing anything that might cause that?
I would recommend ensuring that all dev related dependencies from package.json
are in the devDependencies
and not dependencies
.
Since Cordova is wrapped around the Electron project, the contents of the dependencies
are copied into the app's package.json
. This is so that dependencies are bundled with the app but also available for running the electron project with the cordova run electron
command.
This is probably why you are getting electron in your built app.
From looking at your package.json
you have
"dependencies": {
"@cyph/prettier": "*",
"cordova": "*",
"cordova-support-google-services": "https://github.com/LuisEGR/cordova-support-google-services.git",
"electron-push-receiver": "*"
},
I am not 100% sure if the electron-push-reciever
is used in app code or some build process, but the top three IMO should be devDependencies
.
This is my first recommendation.
Next would be, after you run npx cordova platform add ${platform}
and npx cordova plugin add
from your build.sh
script, have the script update the package.json
by moving them from the dependencies
field to devDependencies
.
Again platform and plugins are also dev. When they are installed and prepared during build, the prepare steps is what moves contents from these packages to the appropriate location for building. The entire package itself is not meant to be. Since the cordova-electron
platform was being bundled with your app, electron was being installed since it is a dependency of cordova-electron
.
Also when you make these changes, your final built app should even be smaller in size as all the dev related dependencies and Cordova build scripts are not packaged with the app.
In the current Cordova CLI, all platform and plugins are added as dependencies
which is not the correct pattern and in the next major release, this will be fixed.
I think this is what you are seeing.
Thanks for explaining all that! I'd been meaning to look into the [Cordova Electron] The built package size may be larger than necessary.
warning at some point. So basically anything that I don't explicitly need to require
at run-time should be in devDependencies
? If that's the case, yep, only electron-push-receiver
needs to be in there.
So I just went ahead and implemented your suggestion: https://github.com/cyph/app/commit/ca1170efc77c2cba69d6037a9124a4a685f3e86f
I've verified that this works, but it seems that cordova build electron --release
partially undoes it and moves cordova-android
, cordova-electron
, and cordova-ios
all right back into dependencies
. electron
is still in app.asar.unpacked/node_modules
.
I'm also curious about the other dependencies
like electron-push-receiver
not being in app.asar.unpacked/node_modules
; that makes me think that maybe all those dependencies would live somewhere else, and that the inclusion of electron
in app.asar.unpacked/node_modules
is possibly a separate unrelated issue?
I tried building your project, somewhat, to follow along with,
What I noticed is that the plugins and platforms are all defined in config.xml
. Cordova has been moving away from defining plugins and platforms in the config.xml
. The newer pattern is to define them directly in the package.json
.
When you run the build the first time, you will notice messages like:
Plugin 'cordova-clipboard' found in config.xml... Migrating it to package.json
So there is some changes that can be made to improve this area..
But the main issue right now would be that the platforms are being added back as dependencies
. As you pointed out it occurring on cordova build
. This is because during build, it triggers a prepare step and it sees in your config.xml
these lines:
<engine name="android" spec="^8.1.0" />
<engine name="electron" spec="git+https://github.com/buu700/cordova-electron-tmp" />
<engine name="ios" spec="^5.1.0" />
And it also sees that they are not in package.json
dependencies
field.
Since the build.sh
script uses npx cordova platform add
, it is not necessary to be in config.xml
. You can delete those lines.
I am not exactly sure on your entire build process and why everything is done in that certain way, for example build.sh
installs the platforms and plugins instead of committing the package.json
to repo that already pertains all that information. All Cordova related items in the package.json
will be checked out if they were missing when running cordova prepare
or cordova build
. Only an additional npm install
is needed if there were non-Cordova related dependencies
& devDependencies
that are required.
I quickly made repo to show how I would have handled having plugins
and platforms
in the package.json
and not have them defined in config.xml
.
But like I said earlier, I do not know the entire use case or goal with your build process. There could be something I am not seeing.
https://github.com/erisu/cdvSampleProject
Didn't mean to take up so much of your time, but thanks for looking into this!
As far as why the dependencies are organized as they are, it's less a case of "this is the best way to do things in a new project" than "if it ain't broke don't fix it". Specifically, since we've had issues with some plugins meant for one platform being enabled on other platforms, until Cordova adds a built-in solution for platform-specific plugins, we're just leaving the custom solution we already have in place that depends on config.xml.
Aside from that, sounds like all I need to do is remove those three lines from config.xml (or at least remove them before cordova build
) and all should work as expected?