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

CB-11777: Restore plugins before preparing #487

Closed
wants to merge 1 commit into from

Conversation

dpogue
Copy link
Member

@dpogue dpogue commented Aug 26, 2016

When running cordova prepare to restore platforms and plugins, the platform has prepare called before the plugins are restored. This leads to the top-level config.xml data being copied into the platform at prepare time, and then plugin config being appended when they are later restored.

In my case, this was causing the Crosswalk version defined in my top-level config.xml to be overwritten by an undefined version when the plugin was installed. A workaround is to run cordova prepare a second time.

A better fix is probably to restore the platforms, restore the plugins, and then run prepare after everything has been restored.

All the existing tests pass, but this does change the order of operations around the prepare hooks, and I'm not certain if other projects/plugins have dependencies on that ordering.
/cc @vladimir-kotikov

@codecov-io
Copy link

codecov-io commented Sep 8, 2016

Current coverage is 80.38% (diff: 100%)

Merging #487 into master will increase coverage by <.01%

@@             master       #487   diff @@
==========================================
  Files            67         67          
  Lines          5183       5184     +1   
  Methods         835        835          
  Messages          0          0          
  Branches       1005       1005          
==========================================
+ Hits           4166       4167     +1   
  Misses         1017       1017          
  Partials          0          0          

Powered by Codecov. Last update f8b58c7...03c2833

@stevengill
Copy link
Contributor

Nice investigating @dpogue!

Good point about the hook order changing. My gut says it should be fine. Would that be a considered a major change is the question.

@stevengill
Copy link
Contributor

Guess this won't matter if we rip restore out of prepare!

apache/cordova-discuss#54

@stevengill
Copy link
Contributor

I'm thinking we move forward with this as I'm feeling hesitant removing restore from prepare.

@asfgit asfgit closed this in 0369045 Apr 19, 2017
asfgit pushed a commit that referenced this pull request May 2, 2017
@dpogue dpogue deleted the prepare-plugins branch December 20, 2017 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants