Strip leading/trailing spaces? (Versioner Process) about recipes

[jessepeterson]

Thinking about this one level deeper I imagine the representation of CFBundleShortVersionString needs to be in a form that the Munki process will correctly match it. So maybe it needs to stay what it is? I hope not, though. However, it is annoying that the MunkiImporter uses the two spaces in both the plist and pkg names when importing.

[gregneagle]

I think it needs to stay as-is because the CFBundleShortVersionString in the Infor.plist on disk for Flip4Mac will be in this format.

Generally speaking, I don't think Munki (or any recipes) should modify the value in the CFBundleShortVersionString, except possibly for display-only purposes.

[jessepeterson]

Okay, I'm convinced on the CFBundleShortVersionString staying issue.

Can we add a filter for pkg and plist names to the MunkiImport process? It just feels wrong with the spaces in there.

[timsutton]

We actually used to strip the version string, exactly when this started happening with Flip4Mac, and went back on it because the version metadata would no longer reflect what Munki is actually using to compare versions, as you suspected.

I don't personally find the spaces in the filenames to be annoying (and on some level they serve as a reminder/warning that the package's metadata is sloppy), but I don't see any reason to not strip() surrounding whitespace before staging the files in the repo.

[jessepeterson]

Ok, I'll try to tinker and submit a pull. Hopefully I can find time for that soon.

[jessepeterson]

It seems as simple as this change, yes?

diff --git a/Code/autopkglib/MunkiImporter.py b/Code/autopkglib/MunkiImporter.py
index 6703e74..ee4c009 100644
--- a/Code/autopkglib/MunkiImporter.py
+++ b/Code/autopkglib/MunkiImporter.py
@@ -339,7 +339,7 @@ class MunkiImporter(Processor):
                 raise ProcessorError("Could not create %s: %s"
                                       % (destination_path, err.strerror))

-        pkginfo_name = "%s-%s.plist" % (pkginfo["name"], pkginfo["version"])
+        pkginfo_name = "%s-%s.plist" % (pkginfo["name"], pkginfo["version"].strip())
         pkginfo_path = os.path.join(destination_path, pkginfo_name)
         index = 0
         while os.path.exists(pkginfo_path):

[timsutton]

Yup!

[jessepeterson]

Want a PR for that little change? ;)

[timsutton]

No PR is too small.

🍰

[jessepeterson]

Submitted as autopkg/autopkg#39.