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

Cordova 7.1.0, Android 6.3.0 and iOS 4.5.3 updates #9213

Merged
merged 14 commits into from Nov 8, 2017
Merged

Cordova 7.1.0, Android 6.3.0 and iOS 4.5.3 updates #9213

merged 14 commits into from Nov 8, 2017

Conversation

@skirunman
Copy link
Contributor

@skirunman skirunman commented Oct 11, 2017

Closes meteor/meteor-feature-requests#196

This is WIP as I did not build and update the bundle version so hoping this can be done by someone else.

Also, was not sure how to deal with the deprecated Cordova plugins that are still in our build, but did mark them as deprecated and could be removed in the future.

@benjamn benjamn added this to the Release 1.5.3 milestone Oct 11, 2017
@abernix
Copy link
Member

@abernix abernix commented Oct 12, 2017

Thanks for submitting this!

Since your FR (meteor/meteor-feature-requests#196) mentioned getting it into Meteor 1.6, I'll just point out: Meteor 1.6 is already in release candidate status and meteor/meteor-feature-requests#196 isn't a bug so unless this fixes critical bugs present in Meteor 1.6 RCs, this will likely surface in other versions (e.g. 1.6.1 and 1.5.3).

@skirunman
Copy link
Contributor Author

@skirunman skirunman commented Oct 12, 2017

No worries, your call on Meteor 1.6.1. The Cordova Android 6.3.0 update does fix some issues that I've seen, but did not report as Meteor problems. Specifically This release now targets the latest Android API level of API 26 and has fixed issues related to the Android SDK Tools 26.0.2 release. Google changed how the Android emulator was executed, causing errors when deploying to the emulator. so this is something you might want to consider. Thanks.

@skirunman
Copy link
Contributor Author

@skirunman skirunman commented Oct 12, 2017

Also, if this does not make it into Meteor 1.6 then we should break out the second comment I made in the History.md file as this change is in 1.6.

iOS icons and launch screens have been updated to support iOS 11
  [Issue #9196](https://github.com/meteor/meteor/issues/9196)
  [PR #9198](https://github.com/meteor/meteor/pull/9198)

@skirunman
Copy link
Contributor Author

@skirunman skirunman commented Oct 12, 2017

One more potential thing is that we are already shipping cordova-ios@4.5.1 which probably should be bundled with cordova-plugin-console@1.1.0 for consistency reasons, but is technically not strictly required. Cheers!

@skirunman
Copy link
Contributor Author

@skirunman skirunman commented Oct 12, 2017

@abernix Spoke with rest of my team on this and we'd like to request getting this in 1.6 if possible. Reason is this PR is pretty much the same as #9137 in scope, which updates cordova-ios to support a new version of iOS and was also introduced during the Release Candidate phase of 1.6. cordova-android@6.3.0 solves the same type of issue, supporting a new version of Android. Anyway, I think the risk is low as this is not really introducing any core changes to Meteor. Thanks again for the consideration.

@abernix
Copy link
Member

@abernix abernix commented Oct 13, 2017

Perfectly fair request! Though, I think I'm going to stick to my thinking of 1.6.1 and 1.5.3. The fix in #9137 was pulled in (late in the 1.6 process already) primarily because of #9098, which created a problem where the simulator just could not be launched, even without any updates to an existing project and on already-supported devices, due to hyphens in iOS device names being removed/changed in XCode's Device List (12.4-inch vs 12.4 inch).

We're getting very close on 1.6 and if we put this in now, there's a chance it won't have time to fully vet and the cordova-android bump to the 26 SDK is hard to judge, especially if we're not fixing a show-stopper bug. We certainly understand that this is important and we will get it out as soon as possible. We do typically do Cordova updates in the first point release after a major release.

Has Cordova been working okay for you otherwise with Meteor 1.6?

@abernix
Copy link
Member

@abernix abernix commented Oct 13, 2017

On another, semi-related note, thank you very much for your diligence on helping us stay on top of Cordova items! You've stood out countless times now, in that you're willing to look into and stay on top of the changes in Cordova, and even more valuably, step up and fix them quickly when you (and others) encounter them. It's truly appreciated!

This change is now in #9227
benjamn added a commit that referenced this issue Oct 14, 2017
Removed this change from #9213 as it goes with #9198
@hwillson hwillson changed the title [WIP] Cordova 7.1.0 and Android 6.3.0 updates Cordova 7.1.0 and Android 6.3.0 updates Oct 18, 2017
@skirunman skirunman changed the title Cordova 7.1.0 and Android 6.3.0 updates Cordova 7.1.0, Android 6.3.0 and iOS 4.5.2 updates Oct 19, 2017
@skirunman
Copy link
Contributor Author

@skirunman skirunman commented Oct 19, 2017

I updated this PR to include the Cordova iOS 4.5.2 release
as I was running into this specific bug regarding icon mappings.

Also, there was a small merge conflict in the History.md file that might need to be sorted out on merge.

@skirunman
Copy link
Contributor Author

@skirunman skirunman commented Nov 2, 2017

@abernix I assume this PR will be released in 1.6.1 and not in 1.5.3 as #9198 is not in 1.5.3 branch either, thanks.

@skirunman skirunman changed the title Cordova 7.1.0, Android 6.3.0 and iOS 4.5.2 updates Cordova 7.1.0, Android 6.3.0 and iOS 4.5.3 updates Nov 2, 2017
skirunman added 2 commits Nov 2, 2017
Updating iOS launch screens to match what is supported in Cordova iOS 4.5.3 and remove latest iPad Pro sizes that are not supported in Xcode 9 and therefore not in Cordova iOS. See https://github.com/surajpindoria/cordova-ios/blob/c3d24a9f02aa096c2b8951cc6980eefbbd26fd72/bin/templates/scripts/cordova/lib/prepare.js
@abernix abernix removed this from the Release 1.5.3 milestone Nov 4, 2017
@abernix abernix added this to the Release 1.5.4 milestone Nov 4, 2017
@abernix abernix removed this from the Release 1.5.4 milestone Nov 7, 2017
@abernix abernix added this to the Release 1.6.1 milestone Nov 7, 2017
@adamgins
Copy link

@adamgins adamgins commented Nov 8, 2017

Hi, I am getting the original error when trying to meteor run android with 1.6
Just trying to follow the details here... does this mean I should rather go with Meteor 1.5.3, until 1.6.1 hits the streets?

abernix
abernix approved these changes Nov 8, 2017
Copy link
Member

@abernix abernix left a comment

LGTM. I'm all for this getting into 1.6.1.

@abernix abernix merged commit 073f241 into meteor:devel Nov 8, 2017
12 checks passed
@skirunman skirunman deleted the cordova-7.1.0 branch Nov 8, 2017
abernix added a commit that referenced this issue Nov 8, 2017
While 'cordova-lib' used to live in 'dev-bundle-tool-package.js' where
it was pre-bundled into the "dev bundle", that is no longer the case.

It is now automatically installed, on demand, when Cordova is used.

This follows up on #9213, which added it back to the
'dev-bundle-tool-package.js' and updates it in the new location.

Ref: #9213
Ref: 073f241

cc @skirunman.
abernix added a commit that referenced this issue Nov 8, 2017
I think I just typo-ed that patch version right before I committed it.

It certainly should have been 7.1.0. :(

Ref: #9213

cc @skirunman.
abernix added a commit that referenced this issue Nov 8, 2017
The changes made in #9213 were seemingly innocent, but when merging
I missed the fact that the 'cordova-lib' package bump was done to a new
entry on the 'dev bundle', rather than on the
`CORDOVA_DEV_BUNDLE_VERSIONS` in tools/cordova/index.js which, as of
#8976, is responsible for auto-installing Cordova when it
is used rather than including it in the pre-packaged 'dev bundle'.

That mistake was fixed with 958c44f and
d6adc1b, but not before it caused the
Cordova tests to falsely pass on the PR since it was functionally still
testing the previous version of Cordova, 7.0.0.

Unfortunately, one of the changes in the 7.1.0 was the deprecation of an
API we use within Meteor: the `raw` API on `cordova_lib`. Luckily, as
seen on the following commit, that change was merely a re-organizational
commit and still provides us access to that API by simply removing
the `raw.` segment and accessing the various methods directly:

apache/cordova-lib@90b6857

Never saw that deprecation message, but we certainly saw the failure in
CI: https://circleci.com/gh/meteor/meteor/10412.

With any luck, this commit/PR will fix the problem.
benjamn added a commit that referenced this issue Nov 9, 2017
The changes made in #9213 were seemingly innocent, but when merging
I missed the fact that the 'cordova-lib' package bump was done to a new
entry on the 'dev bundle', rather than on the
`CORDOVA_DEV_BUNDLE_VERSIONS` in tools/cordova/index.js which, as of
#8976, is responsible for auto-installing Cordova when it
is used rather than including it in the pre-packaged 'dev bundle'.

That mistake was fixed with 958c44f and
d6adc1b, but not before it caused the
Cordova tests to falsely pass on the PR since it was functionally still
testing the previous version of Cordova, 7.0.0.

Unfortunately, one of the changes in the 7.1.0 was the deprecation of an
API we use within Meteor: the `raw` API on `cordova_lib`. Luckily, as
seen on the following commit, that change was merely a re-organizational
commit and still provides us access to that API by simply removing
the `raw.` segment and accessing the various methods directly:

apache/cordova-lib@90b6857

Never saw that deprecation message, but we certainly saw the failure in
CI: https://circleci.com/gh/meteor/meteor/10412.

With any luck, this commit/PR will fix the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants