update dockutil to be python3 compatible #87
Conversation
|
Thank you for this! I have questions because I am not up to speed on python3 and best practices. Is this meant to work seamlessly with either python version? Can it be made to? I had started attempting that at PSU hack night but didn't get far struggling with unicode. In order to work with either python2 or python3 with same code, are additional modules required? I am kind of a purist about wanting tools to work seamlessly without dependencies. At least that is a goal anyway. Thanks again. I will probably have more questions. |
| @@ -1,5 +1,3 @@ | |||
| #!/usr/bin/python | |||
There was a problem hiding this comment.
What should shebang be? #!/usr/bin/env python (if it also works in python2) or #!/usr/bin/env python3 ? or something else?
There was a problem hiding this comment.
Great question. I didn't have an answer to that question so I axed it from the PR.
Ideally it would be interesting to test this with official python3 package and then use #!/usr/bin/env python3 but you could run into an issue where someone has a different python installed in their user path. dockutil is typically ran in the user space so that shebang could be a bad assumption.
My gut feeling is people triggering dockutil should use it without a shebang and then invoke it against the python3 of their choice. So for myself I do /Library/InstallApplications/Python.framework/blahblahblah/bin/python3 /path/to/shebangless/dockutil
Everyone writing python3 is going to run into this. If you want to be authoritative, it's highly likely you will have to include your own python framework for others to use and it will increase your support burden.
Wish I had a better answer.
| @@ -403,7 +401,9 @@ def writePlist(pl, plist_path): | |||
| # get a tempfile path for writing our plist | |||
| plist_import_path = tempfile.mktemp(dir='/tmp') | |||
| # Write the plist to our temporary plist for importing because defaults can't import from a pipe (yet) | |||
| plistlib.writePlist(pl, plist_import_path) | |||
There was a problem hiding this comment.
I might want to check if method exists if plistlib has changed and we want to support either.
There was a problem hiding this comment.
It doesn't. Python 3.4 and higher only :/
| @@ -543,10 +543,6 @@ def addItem(pl, add_path, replace_label=None, position=None, before_item=None, a | |||
| if showas == None: showas = 0 | |||
| if arrangement == None: arrangement = 2 | |||
|
|
|||
| #fix problems with unicode file names | |||
| enc = (sys.stdin.encoding if sys.stdin.encoding else 'UTF-8') | |||
There was a problem hiding this comment.
I will need to test with emojis etc.
There was a problem hiding this comment.
Python 3 handles Unicode much better than 2.7, but definitely did not test items with emojis
| @@ -584,18 +580,14 @@ def addItem(pl, add_path, replace_label=None, position=None, before_item=None, a | |||
| if tile_type == 'file-tile': | |||
| new_item = {'GUID': new_guid, 'tile-data': {'file-data': {'_CFURLString': add_path, '_CFURLStringType': 0},'file-label': label_name, 'file-type': 32}, 'tile-type': tile_type} | |||
| elif tile_type == 'directory-tile': | |||
| if subprocess.Popen(['/usr/bin/sw_vers', '-productVersion'], | |||
There was a problem hiding this comment.
Oh no we've dropped support for Mac OS X Tiger. 🤣
There was a problem hiding this comment.
Heh yep. I figure it was worth dropping.
I only tested python3 because python2 is EOL. Should you decide to be dual compliant, you can certainly try, but python2 and 3 handle Unicode very differently as well as strings and the methods used in the PR would have to be adapted for python2 as they don't exist until python 3.4 I personally don't have time to implement it in that fashion but if that's something you want to do, maybe just import this PR into a branch and use this as a starting point. It certainly wouldn't hurt my feelings :) |
|
Just a note, in Big Sur apple is even marking python2 EOL. The following displayed when launching "python" in terminal: WARNING: Python 2.7 is not recommended. and python3 is installed in /usr/bin, just like it was in Catalina. Catalina might even have had same warning, I can't remember. So I'd vote for making current version last supporting python2, and use python3 going forward. Prob not worth the effort of making dual compliant, and can always point to current version for anyone needing to use python2. |
|
Bumping this! In macOS Monterey, anything that calls Python 2 displays an alert to the user about its deprecation. For a tool like docktutil, we'd want it to run silently, so this is an issue for us. Should I switch to @erikng's fork, or hold out for an official release that utilizes Python 3? Thanks for this tool, @kcrawford! |
|
This will be addressed. Note the GUI prompts only present if running python from an app. |
|
I see python2 is removed in macOS 12.3 |
|
Is there a reason the version is not being bumped to |
|
I expect a beta version that works on macOS 12.3 will be available within a few days. |
|
dockutil 3.0.0 beta |
figured I would help others, but I made it python 3 compatible.
I tested this against Greg's relocatable python, but using my own python 3 requirements https://github.com/erikng/installapplications/blob/embeddedpython3/py3_requirements.txt from my installapplications tool.
I tested everything except manipulating the user template and the
--allhomesfunctionality. If someone needs that, they can test this PR and comment if it works or not.