CB-6711: Use parseProjectFile when working with XCode projects. #15
Conversation
Otherwise the changes get overwritten by the cached version of the project file. Add a unit test to guard against similar bugs in the future.
|
Ping. Anyone willing to review? 😄 |
| @@ -510,8 +509,7 @@ function ConfigFile_load() { | |||
| self.data = xml_helpers.parseElementtreeSync(filepath); | |||
| } else if (ext == '.pbxproj') { | |||
| self.type = 'pbxproj'; | |||
| self.data = xcode.project(filepath); | |||
| self.data.parseSync(); | |||
| self.data = platforms.ios.parseProjectFile(self.project_dir).xcode; | |||
There was a problem hiding this comment.
I switched to directly using xcode here instead of parseProjectFile() here several months ago. The reason was speed. parseProjectFile() has a pretty slow blob call as one of the first lines (order of 50-100ms).
Is there any functional reason you want to use parseProjectFile here other than elegance?
There was a problem hiding this comment.
Yes, the reason is that parseProjectFile() caches the parsed project and changes made without it get overwritten. This is observed when the --plugin option is specified more than once.
There was a problem hiding this comment.
parseProjectFile is called when processing the action-stack, so even if we skip the call here, it will eventually happen. What was the purpose of this optimization? I think that it may only have effect on the unit tests execution time.
There was a problem hiding this comment.
It was part of a larger optimization to speed up "cordova prepare" during which lots of files were re-parsed several times (once per plugin). Looks like we introduced both changes at about the same time (Jan/Feb) this resulted in double caching and overwriting. Before the two of our changes the extra glob was executed per plugin during prepare and was one of the largest time wasters.
Anyway, right now the glob will only run once so it's not a big deal.
LGTM. Merging this PR.
Otherwise the changes get overwritten by the cached version of the project file.
Add a unit test to guard against similar bugs in the future.