-
-
Notifications
You must be signed in to change notification settings - Fork 197
clean-up after release build in android to prevent runtime errors #2452
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
Conversation
@@ -657,6 +657,10 @@ We will now place an empty obsolete compatability white screen LauncScreen.xib f | |||
return null; | |||
} | |||
|
|||
public cleanProject(projectRoot: string, options: string[]): IFuture<void> { | |||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to use:
return Future.fromResult();
This way the code will not fail in case the method is called (currently in case you call cleanProject(..).wait()
it will fail that it cannot read property wait fo null.
@@ -217,6 +217,16 @@ export class PlatformService implements IPlatformService { | |||
this.ensurePlatformInstalled(platform).wait(); | |||
let changesInfo = this.$projectChangesService.checkForChanges(platform); | |||
if (changesInfo.hasChanges) { | |||
// android build artifacts need to be cleaned up when switching from release to debug builds | |||
if (platform.toLowerCase() === "android") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the whole logic is Android specific, instead of using this if, you can move the whole code inside AndroidProjectService's cleanProject method. Here you can just call:
platformData.platformProjectService.cleanProject(platformData.projectRoot, []).wait();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about that earlier, but I don't think cleanProject
should depend on conditions specific to the prepare phase, and the android test prevents unnecessary prepareInfo checks when preparing an ios project.
if (platform.toLowerCase() === "android") { | ||
let previousPrepareInfo = this.$projectChangesService.getPrepareInfo(platform); | ||
// clean up prepared plugins when not building for release | ||
if (previousPrepareInfo && previousPrepareInfo.release && !this.$options.release) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it good idea to call the clean method in case the previous build was for debug config and the current one is for release? For example:
if (previousPrepareInfo && previousPrepareInfo.release !== this.$options.release) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Build artifacts should be cleaned when changing the build type. Don't know why I didn't leave it that way.
413e271
to
9495948
Compare
👍 After green build, of course ;) |
9495948
to
2302507
Compare
If app's built with the
--release
option, the settings are recorded in.nsprepareinfo
in theplatforms/android
directory. If the file is present, and follow up builds are not built in release, then the project directory is cleaned manually to ensure that no build artifacts specific to just release (like the android snapshot binaries) are packaged into the debug application.File changes:
IPlatformProjectService
and created a stub on the ios side.android
Addresses #2443