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 plugin handling algorithm consistency improvements #10193

Merged
merged 2 commits into from Sep 5, 2018

Conversation

Projects
None yet
3 participants
@wojtkowiak
Contributor

wojtkowiak commented Sep 4, 2018

This strives to fix several issues with cordova integration plugins handling:

Fixes several cases causing cordova plugins reinstall on every build:

  • proper handling of scoped npm cordova plugins (adding one would cause reinstall on every build)
  • proper detection of plugin removal (previously a cordova plugin containing a dependency would make the algorithm think a package was removed from cordova-plugins)
  • proper handling of plugins which have plugin.xml id different than the npm package name

Additionally:

  • rechecks the build integrity verifying if packages were really installed and perform a retry if needed.
  • allows to override a meteor package cordova dependency with scoped package i.e. @scope/cordova-dummy-plugin will now override a cordova-dummy-plugin dependency. (now in this case the same plugin will be installed twice, cordova will leave the last one installed)
  • fixes <dependency id="es6-promise-plugin" url="https://github.com/vstirbu/PromisesPlugin.git"/> being translated by listFetchedPluginVersions as https://github.com/vstirbu/PromisesPlugin.git#undefined

The main benefit is that the full plugins uninstall and reinstall will only happen on plugin being updated, removed or added. Now for most projects it happens every build even though cordova-plugins file did not change. Additionally because cordova_lib is not handling properly the promises from plugin remove a situation is happening when plugins are missing - this adds a re-check of plugins state and retries to add missing plugins.

Fixes: #9548 #9973

darqs and others added some commits May 25, 2018

Improvements to cordova plugin set change detection algorithm
Fixes several cases causing cordova plugins reinstall on every build:
- proper handling of scoped npm cordova plugins
- proper detection of plugin removal (previously a cordova plugin containing a dependency would make the algorithm think a package was removed from cordova-plugins)
- proper handling of plugins which have plugin.xml id different than the npm package name
Additionally rechecks the build integrity verifying if packages were really installed and perform a retry if needed.
Allows to override a meteor package cordova dependency with scoped package i.e. @scope/cordova-dummy-plugin will now override a cordova-dummy-plugin dependency.

@benjamn benjamn added this to the Release 1.7.1 milestone Sep 5, 2018

@benjamn benjamn changed the base branch from devel to release-1.7.1 Sep 5, 2018

@benjamn benjamn merged commit 05dd609 into meteor:release-1.7.1 Sep 5, 2018

18 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci: Clean Up Your tests passed on CircleCI!
Details
ci/circleci: Docs Your tests passed on CircleCI!
Details
ci/circleci: Get Ready Your tests passed on CircleCI!
Details
ci/circleci: Isolated Tests Your tests passed on CircleCI!
Details
ci/circleci: Test Group 0 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 1 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 10 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 2 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 3 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 4 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 5 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 6 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 7 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 8 Your tests passed on CircleCI!
Details
ci/circleci: Test Group 9 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wojtkowiak

This comment has been minimized.

Show comment
Hide comment
@wojtkowiak

wojtkowiak Sep 7, 2018

Contributor

@benjamn thanks a lot for merging 🙂
Next week I will add another PR with more/better comments to better explain the difference between the plugin name from plugin.xml and npm package name and how that is handled right now as it might be confusing for someone who tries to get the reasoning behind this code.

Contributor

wojtkowiak commented Sep 7, 2018

@benjamn thanks a lot for merging 🙂
Next week I will add another PR with more/better comments to better explain the difference between the plugin name from plugin.xml and npm package name and how that is handled right now as it might be confusing for someone who tries to get the reasoning behind this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment