search

munki / munki-pkg

Repo for the munkipkg tool and example projects
Other
344 stars 72 forks source link

python 3 modernization needed #62

Closed arubdesu closed 1 year ago

arubdesu commented 2 years ago

Coworker noticed an issue when importing an existing pkg where the version was slurped in as data/base64 instead of type string and actually wrote out the pkg with version b'1.2.3', there are probably other open and plist reading functions expecting older behavior that would require compatibility checks/handling supported in current python.

I'm happy to go over the code base and do so soon if nobody else gets to it, but logging issue for now

gregneagle commented 2 years ago

Same or similar issue described here (requires access to MacAdmins Slack): https://macadmins.slack.com/archives/C04QVPFGU/p1650659151859969

gregneagle commented 2 years ago

Would be nice if the fix doesn't break things when run under Python 2. IOW, no need to break people who aren't yet running macOS 12.3+ and are still using Apple's Python 2.7.

arubdesu commented 2 years ago

This happened to another coworker, depending on new regressions introduced by subsequent Ventura beta's, I should be able to put this on my shortterm todo's

arubdesu commented 2 years ago

I originally thought this was error'ing/causing undesired behavior with a specific version of python3, but the bytes instead of a 'plain' string issue is present with at least as old as Apple's 3.8.9, probably with minidom.parse and subprocess as the culprits.

As of 12.3+ just pointing at python with /usr/bin/env results in env: python: No such file or directory so I'd think we could consider making a clean break in the version and drop python2 support altogether...

arubdesu commented 1 year ago

Ok revisiting this on the other side of Ventura with xcode CLI tools python and a simple unsigned 'filedrop' pkg as my test case, although I acknowledge the readme currently allows a bunch of py3 options (covering the spread like outset). I'm not seeing the b'version' issue at the moment when doing a --create /tmp/test and then a build

image

Imports are troublesome, though:

% ./munkipkg --import /Users/allister/Downloads/snagitLicenseKey-2022.1.pkg /tmp/snagit
ERROR: /tmp/snagit/build-info.plist key 'postinstall_action' has illegal value: b'none'
ERROR: Legal values are: ['none', 'logout', 'restart']
munkipkg: Sync successful: no changes needed.
munkipkg: Created new package project at /tmp/snagit

Investigating.

gregneagle commented 1 year ago

I think this is no longer relevant and can be closed.