Closed homebysix closed 3 years ago
This seems a reasonable change. I'd like to see it tested against the default Python on macOS as well (2.7.x) and some thought there -- should it be coerced to Unicode? I think in Python 2 plist "strings" are more like native Python Unicode strings, but not 100% sure.
I don't think we need to coerce to Unicode for Python two given that the change works as expected in Python 2 already (see new tests below). In fact, I'm surprised that emoji doesn't break pkgbuild
here:
% cd SuppressSetupAssistant
% pkgbuild --identifier com.example.test --version 👻 --root payload --install-location / test-👻.pkg
pkgbuild: Inferring bundle components from contents of payload
pkgbuild: Wrote package to test-👻.pkg
Emoji seems to work in both the main
and version-strings
branches of MunkiPkg as well:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg plist_test
pkgbuild: Inferring bundle components from contents of plist_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpw8cxh3kj/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpw8cxh3kj/component.plist
pkgbuild: Wrote package to plist_test/build/plist_test-👻.🌮.🦑.pkg
% munkipkg json_test
pkgbuild: Inferring bundle components from contents of json_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp3188rh9l/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp3188rh9l/component.plist
pkgbuild: Wrote package to json_test/build/json_test-👻.🌮.🦑.pkg
% munkipkg yaml_test
pkgbuild: Inferring bundle components from contents of yaml_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpdw16ayu7/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpdw16ayu7/component.plist
pkgbuild: Wrote package to yaml_test/build/yaml_test-👻.🌮.🦑.pkg
% git checkout version-strings
Already on 'version-strings'
Your branch is up to date with 'origin/version-strings'.
% munkipkg plist_test
pkgbuild: Inferring bundle components from contents of plist_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpridfda4d/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpridfda4d/component.plist
pkgbuild: Wrote package to plist_test/build/plist_test-👻.🌮.🦑.pkg
% munkipkg json_test
pkgbuild: Inferring bundle components from contents of json_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp2p1h9fpr/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp2p1h9fpr/component.plist
pkgbuild: Wrote package to json_test/build/json_test-👻.🌮.🦑.pkg
% munkipkg yaml_test
pkgbuild: Inferring bundle components from contents of yaml_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmprgzfjhc0/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmprgzfjhc0/component.plist
pkgbuild: Wrote package to yaml_test/build/yaml_test-👻.🌮.🦑.pkg
In all tests below, the default macOS Python 2 has been used as the interpreter for MunkiPkg (per the MunkiPkg shebang).
Build-info version:
<key>version</key>
<string>1.1</string>
Test output:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg plist_test
pkgbuild: Inferring bundle components from contents of plist_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpymppgho0/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpymppgho0/component.plist
pkgbuild: Wrote package to plist_test/build/plist_test-1.1.pkg
Test output:
% git checkout version-strings
Switched to branch 'version-strings'
Your branch is up to date with 'origin/version-strings'.
% munkipkg plist_test
pkgbuild: Inferring bundle components from contents of plist_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpc8m_z8p1/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpc8m_z8p1/component.plist
pkgbuild: Wrote package to plist_test/build/plist_test-1.1.pkg
Build-info version:
<key>version</key>
<integer>2</integer>
Test output:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg plist_test
Traceback (most recent call last):
File "munkipkg", line 1056, in <module>
main()
File "munkipkg", line 1052, in main
result = build(arguments[0], options)
File "munkipkg", line 633, in build
build_info = get_build_info(project_dir, options)
File "munkipkg", line 260, in get_build_info
file_info = read_build_info(build_file + '.' + file_type)
File "munkipkg", line 156, in read_build_info
build_info['name'] = build_info['name'].replace(
TypeError: replace() argument 2 must be str, not int
Test output:
% git checkout version-strings
Switched to branch 'version-strings'
Your branch is up to date with 'origin/version-strings'.
% munkipkg plist_test
pkgbuild: Inferring bundle components from contents of plist_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp1hswcso2/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp1hswcso2/component.plist
pkgbuild: Wrote package to plist_test/build/plist_test-2.pkg
Build-info version:
"version": "1.1",
Test output:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg json_test
pkgbuild: Inferring bundle components from contents of json_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpxia7qhnr/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpxia7qhnr/component.plist
pkgbuild: Wrote package to json_test/build/json_test-1.1.pkg
Test output:
% git checkout version-strings
Switched to branch 'version-strings'
Your branch is up to date with 'origin/version-strings'.
% munkipkg json_test
pkgbuild: Inferring bundle components from contents of json_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp6c_1u045/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmp6c_1u045/component.plist
pkgbuild: Wrote package to json_test/build/json_test-1.1.pkg
Build-info version:
"version": 1.1,
Test output:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg json_test
Traceback (most recent call last):
File "munkipkg", line 1056, in <module>
main()
File "munkipkg", line 1052, in main
result = build(arguments[0], options)
File "munkipkg", line 633, in build
build_info = get_build_info(project_dir, options)
File "munkipkg", line 260, in get_build_info
file_info = read_build_info(build_file + '.' + file_type)
File "munkipkg", line 156, in read_build_info
build_info['name'] = build_info['name'].replace(
TypeError: replace() argument 2 must be str, not float
Test output:
% git checkout version-strings
Switched to branch 'version-strings'
Your branch is up to date with 'origin/version-strings'.
% munkipkg json_test
pkgbuild: Inferring bundle components from contents of json_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpibhxyz8i/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpibhxyz8i/component.plist
pkgbuild: Wrote package to json_test/build/json_test-1.1.pkg
Note: install PyYAML first if necessary: pip install -U PyYAML --user
Build-info version:
version: '1.1'
Test output:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg yaml_test
pkgbuild: Inferring bundle components from contents of yaml_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpbqs662ad/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpbqs662ad/component.plist
pkgbuild: Wrote package to yaml_test/build/yaml_test-1.1.pkg
Test output:
% git checkout version-strings
Switched to branch 'version-strings'
Your branch is up to date with 'origin/version-strings'.
% munkipkg yaml_test
pkgbuild: Inferring bundle components from contents of yaml_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpl629fgu4/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpl629fgu4/component.plist
pkgbuild: Wrote package to yaml_test/build/yaml_test-1.1.pkg
Build-info version:
version: 1.1
Test output:
% git checkout main
Switched to branch 'main'
Your branch is up to date with 'upstream/main'.
% munkipkg yaml_test
Traceback (most recent call last):
File "munkipkg", line 1056, in <module>
main()
File "munkipkg", line 1052, in main
result = build(arguments[0], options)
File "munkipkg", line 633, in build
build_info = get_build_info(project_dir, options)
File "munkipkg", line 260, in get_build_info
file_info = read_build_info(build_file + '.' + file_type)
File "munkipkg", line 156, in read_build_info
build_info['name'] = build_info['name'].replace(
TypeError: replace() argument 2 must be str, not float
Test output:
% munkipkg yaml_test
pkgbuild: Inferring bundle components from contents of yaml_test/payload
pkgbuild: Writing new component property list to /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpzq7_kwbe/component.plist
pkgbuild: Reading components from /var/folders/jj/4ykdc7714tq8jwb25_j8czxh0000gn/T/tmpzq7_kwbe/component.plist
pkgbuild: Wrote package to yaml_test/build/yaml_test-1.1.pkg
OK! I don't use the yaml or json style build-info files, so I'm less likely to notice any issues this introduces, but it looks fine.
This change makes MunkiPkg more resilient against improperly formatted build-info files where the
version
may not be interpreted as a string.Problem
Currently, if the
version
is not interpreted as a string, MunkiPkg fails to build the package and produces a traceback.This problem state is difficult to get into accidentally if one is using a plist build-info file, as is the default, because the plist specifies
<string>
as the version type in an obvious manner. However, with either YAML or JSON build-info files, entering this error state is as simple as omitting quotes:YAML:
JSON:
Solution
Casting the version with
str( )
each time it's referenced prior to packaging ensures that MunkiPkg gracefully handles an improper version type in the build-info file.Testing
In all tests below, an installation of MacAdmins managed Python 3 has been used as the interpreter for MunkiPkg.
Correctly formatted plist, before
Build-info version:
Test output:
Correctly formatted plist, after
Test output:
Incorrectly formatted plist, before
Build-info version:
Test output:
Incorrectly formatted plist, after
Test output:
Correctly formatted json, before
Build-info version:
Test output:
Correctly formatted json, after
Test output:
Incorrectly formatted json, before
Build-info version:
Test output:
Incorrectly formatted json, after
Test output:
Correctly formatted yaml, before
Build-info version:
Test output:
Correctly formatted yaml, after
Test output:
Incorrectly formatted yaml, before
Build-info version:
Test output:
Incorrectly formatted yaml, after
Test output: