Add Python 3 compatibility to bin/cordova_plist_to_config_xml

[cclauss]

These changes should be compatible with both Python 2 and Python 3.

Platforms affected

Motivation and Context

Description

Testing

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

[codecov-io]

Codecov Report

Merging #769 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #769   +/-   ##
=======================================
  Coverage   74.42%   74.42%           
=======================================
  Files          11       11           
  Lines        1830     1830           
=======================================
  Hits         1362     1362           
  Misses        468      468

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 266d339...da2511d. Read the comment docs.

[brodycj]

Thanks for the contribution, I just requested a review from some iOS experts. My apologies for the delay.

[DavidM42]

lgtm

[NiklasMerz]

lgtm. Asked if end='' really is needed but that's non breaking

If have no idea about python, that's why I got someone to review it who has :-)

[NiklasMerz]

After dicussion with @DavidM42 and clarification from @cclauss I think it's good to merge.

Thank you very much @DavidM42 for your review. 🥇

[brodycj]

After another look at the script, I now start to wonder if we could find a better way to solve the same problem with Cordova.plist and config.xml. A couple ideas I can think of:

• do this in a Node.js script, with help from our Cordova node-xcode utility
• update the Cordova xcode project to use the correct configuration from config.xml, with no dynamic fix needed

[erisu]

Just as a side-note, this file seems to be used only for the Platform-Centered workflow.

There are multiple ways of going about creating a Platform-Centered project. One is a fresh project from Xcode. This means the Cordova's Xcode based project is not used and therefore does not have the config.xml file.

IMO, I think we do not have a lot of resources to focus on the Platform-Center workflow. I think it may have been discussed to consolidate our focus only on the CLI workflow and potentially dropping Platform-Center workflow.

If the changes work, we could just push it through and not spend too much time on this.

Just My Opinion...

[brodycj]

Agreed @erisu, let's get this merged.

I just raised #791 to avoid losing track of improving the platform-centered workflow ([1]) and hopefully alleviate the need for Python (someday). Definitely very unfortunate that we cannot afford the resources to make this kind of improvement right now.

[1] https://cordova.apache.org/docs/en/latest/guide/platforms/ios/#platform-centered-workflow

[brodycj]

I think we want to drop this functionality in the near future, as I proposed in apache/cordova#198. I wonder if we can get this fix into a minor release somehow. What do you think @erisu?

@erisu let's please get this merged in the next 12-24 hours if there are no objections, I do think it would be good to get this solution committed even if we do not want to support it too much longer.

[brodycj]

Why add a space here?

[cclauss]

To put a space between each line.

[brodycj]

I don't really understand (did not work with Python for about 10 years), hope someone else can review it. We do appreciate your efforts to get this whole Python compatibility issue fixed.

[cclauss]

Which do we want?

>>> for char in "abcdefg":
...     print(char, end="")  # --> abcdefg
    -- or --
>>> for char in "abcdefg":
...     print(char, end=" ")  # --> a b c d e f g

[knight9999]

I'm wondering this script is now necessary or not.
Because Cordova.plist is removed at the commit https://github.com/apache/cordova-ios/tree/321420b9c3520b0cf32e5b27117701d3ced760b4/bin/templates/project/__TESTING__ . The last of Cordova.plist is in the previous commit https://github.com/apache/cordova-ios/tree/df186efa37406f4347218e33b264aec6e0110194/bin/templates/project/__TESTING__ . This date is 10 Nov 2012. I imagine the role of this script is to convert old style Cordova.plist to new style config.xml and not necessary now.

When we apply this script to usual Xcode project, the error

raise Exception('Could not find a file named "Cordova.plist" within ' + search_path)

happens because there is no Cordova.plist file.
Or if we rename some Info.plist to Cordova.plist the another error

    AppendDict(plist['Plugins'], plugins, 'plugin')
KeyError: 'Plugins'

happens because (usually) plist file does not contains the key Plugins. This key is old cordova style proejct specific feature. The error happens on about line 90,

  AppendDict(plist['Plugins'], plugins, 'plugin')   # <--- plist['Plugins'] exists only for old cordova style (before cordova-ios 2.3) project.

[knight9999]

I tried this PR with Python 3.6.4. But I got the following error.

TypeError: string argument expected, got 'bytes'

I feel it is better to use BytesIO with ElementTree.write in place of StringIO as follows.
(In my environment this can fix the above error)

diff --git a/bin/cordova_plist_to_config_xml b/bin/cordova_plist_to_config_xml
index f6c47505..94fe3371 100755
--- a/bin/cordova_plist_to_config_xml
+++ b/bin/cordova_plist_to_config_xml
@@ -38,6 +38,7 @@ try:  # Python 2
 except ImportError:  # Python 3
   from io import StringIO
 
+from io import BytesIO
 
 def Usage():
   sys.stderr.write(__doc__)
@@ -93,10 +94,11 @@ def ConvertPlist(src_path, dst_path):
     root.append(ElementTree.Element('access', attrib={'origin':value}))
 
   tree = ElementTree.ElementTree(root)
-  s = StringIO()
+  # s = StringIO()
+  s = BytesIO()
   tree.write(s, encoding='UTF-8')
   mini_dom = minidom.parseString(s.getvalue())
-  with open(dst_path, 'w') as out:
+  with open(dst_path, 'wb') as out:
     out.write(mini_dom.toprettyxml(encoding='UTF-8'))