feat: Support app bundles nested in directories #73
Conversation
It is possible, using a heuristic, to move nested app bundles to the root of the working directory when installing an artifact, and prevent an installation error. The heuristic can be outlined as follows: - Search for the shallowest "Info.plist" in the archive - If one doesn't exist, or the "Info.plist" is already at the root, there is nothing more to do - If one does exist, then assume its parent directory is the app bundle root
|
I have signed the CLA! |
| if let appBundleRoot = try? zipArtifactApplicationBundleRoot(at: url) { | ||
| log.info("Nested application bundle detected") | ||
| // Move the original destination to "_tmp", then move the nested app bundle to the destination. | ||
| let workingDestinationURL = url.deletingLastPathComponent().appending(path: "_tmp") | ||
| try FileManager.default.moveItem(at: destinationURL, to: workingDestinationURL) | ||
| let nestedAppBundleRootURL = workingDestinationURL.appending(path: appBundleRoot) | ||
| try FileManager.default.moveItem(at: nestedAppBundleRootURL, to: destinationURL) | ||
| } |
There was a problem hiding this comment.
I wonder if this logic might be a bit too complex for what we need—is there any reason why we can't simply check for Info.plist at the root of the zip, then check for Info.plist inside the first nested folder? We could probably just throw an error if it doesn't exist in either of those locations.
I'm wondering how often the structure of the zip would actually be more complicated than that. For applications built with standard tooling you would probably only ever find the .app at 0 or 1 levels deep.
There was a problem hiding this comment.
Hi @lfroms, it's a good question, let me try answer it:
"First" (quoted because zip archives have no well-defined ordering) may not necessarily be the correct app folder. For example, if the archive contains a metadata folder (e.g. screenshots, or a dot-folder) then the first folder may unluckily be the wrong folder. Regardless, finding the first folder involves scanning the contents of the zip archive, at this stage the decision is whether you blindly accept the first folder or try to find the right folder (I decided that was one containing an Info.plist).
Once you're already iterating the zip archive looking for the right first folder, the logic isn't much less complicated than doing it for nested folders, and I thought it might be prudent to avoid the reoccurence of the "Tophat doesn't work with app bundles nested N+1 levels deep" issues.
Hopefully this gives you some insight into my journey to get to this point.
There was a problem hiding this comment.
I'm not super convinced that we would often encounter a structure other than at the 0th or 1st level. Right-clicking and compressing in Finder or Xcode's default output yields one level of nesting, or zipping the contents yields 0 levels of nesting. I think that should be enough to cover 99% of use cases.
Could we update the logic to match that?
What does this change accomplish?
Allows zip artifacts to contain app bundles that are zipped with a parent directory.
Resolves #32
How have you achieved it?
I decided to modify
ArtifactUnpacker, since it seemed like the least intrusive change. If it could get straightened out at this point, then the remainder of Tophat would continue working as expected.The change implements a heuristic to detect nested app bundles:
/in the path of each zip entryIf a nested app bundle is detected, then the unpacked bundle is moved out of the way (to
_tmp, which I'll admit is not the best), and the nested app bundle directory is moved into the original destination.There are multiple
Info.plistfiles inside an app bundle, and the one that matters for this purpose is the one nearest the root, since it will either be at the root (in which case there is nothing to do), or it'll be at the root of the app bundle itself.It's possible that I've overlooked something, and improvements would be greatly appreciated.
How can the change be tested?
MyApp.app)$ zip -r MyApp.zip ./MyApp.app # Verify that the zip contains the bundle directory, not just the contents $ unzip -t MyApp.zip Archive: MyApp.zip testing: MyApp.app/ OK …