Ensure resource bundles aren't marked as executable #9354
Conversation
This patch reverts a single line from swiftlang@a112ab8#diff-b1845cad8f64e37500b06818ff71629726c14cf5a8d34add6ef3a9d38236cfe5 The executable name should be deliberately omitted from the build settings to avoid having the bundle be marked as executable with the CFBundleExecutable in its Info.plist.
plemarquand
requested review from
bkhouri,
bripeticca,
cmcgee1024,
daveyc123,
dschaefer2,
jakepetroules,
owenv and
rconnell9
as code owners
|
@swift-ci please test |
|
@swift-ci please test |
|
@swift-ci test windows |
| ) | ||
| try #require(result.binContents.contains("TIF_TIF.bundle")) | ||
| let contentsDir = result.binPath.appending("TIF_TIF.bundle", "Contents") | ||
| let bundleResourceDir = contentsDir.appending("Resources") |
There was a problem hiding this comment.
Question: is there value is validating this path exists so we can have "better" message in the event it does not?
There was a problem hiding this comment.
Suggest: can we rename "TIF" with something more descriptive?
| #expect(bundleResources.contains("SomeAlert.nib")) | ||
|
|
||
| // Check that the Info.plist of the resource bundle looks reasonable. In particular, it shouldn't have a CFBundleExecutable key, since it's a codeless bundle. | ||
| let infoPlistPath = contentsDir.appending("Info.plist") |
There was a problem hiding this comment.
Suggestion: SwiftPM has no control over the contents of the info.plist file. As a result, changes to the SwiftBuild service can impact this test. Can we move the intent of this test to SwiftBuild?
There was a problem hiding this comment.
SwiftPM does have control depending on what build settings it passes down to swift-build.
This PR is not test only, it reverts a line of code that changed the structure of the Info.plist in resource bundles.
|
@swift-ci test windows |
| settings[.PRODUCT_MODULE_NAME] = bundleName | ||
| settings[.PRODUCT_BUNDLE_IDENTIFIER] = "\(self.package.identity).\(module.name).resources" | ||
| .spm_mangledToBundleIdentifier() | ||
| // Resource bundles are not executable. Setting the name to an empty string will |
There was a problem hiding this comment.
CFBundleExecutable doesn't mean the binary is executable per se. It's present if the bundle has an executable (which could be a dylib).
Was this found to cause an actual problem somewhere, or just speculative?
5 participants
This patch reverts a single line from
a112ab8#diff-b1845cad8f64e37500b06818ff71629726c14cf5a8d34add6ef3a9d38236cfe5
The executable name should be deliberately omitted from the build settings to avoid having the bundle be marked as executable with the CFBundleExecutable in its Info.plist.