feat(preprod): fail upload if app is missing Info.plist #2793
Conversation
src/utils/build/validation.rs
Outdated
| } | ||
|
|
||
| // All .app bundles in Products/Applications/*.app should have an Info.plist file | ||
| let applications_dir = path.join("Products").join("Applications"); |
There was a problem hiding this comment.
I don't think it's the case that the .app needs to be in Products/Applications That is the default but it there is a ApplicationPath key in the archives Info.plist that determines the folder (it always starts with Products but the Applications part can differ) It looks like right now this code only checks the Info.plist if the app is in Products/Applications and otherwise allows any app? Should we ensure the .app still has an Info.plist even if the xcarchive uses a custom ApplicaitonPath?
There was a problem hiding this comment.
Good point. I updated this to glob for all .app dirs within Products/**/*.app so we are no longer coupled to the Applications folder. I want to avoid having to parse the top-level plist file since that will further couple us to macOS tooling.
src/utils/build/validation.rs
Outdated
| error!("Invalid XCArchive: Missing required Info.plist file at XCArchive root"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
m: I would recommend changing the function signature for this function to a Result<()>, and also updating the method name accordingly.
Then, rather than logging the error and returning false, you can just use anyhow::bail! to return the errors directly.
In the case where you currently return true, you'd instead return Ok(()).
There was a problem hiding this comment.
ah nice I see what you mean, I was a bit confused by the anyhow lib at first. I updated it to propagate errors up so now it fails with the error as you suggested instead of a separate log:
DEBUG 2025-09-24 14:19:46.752468 -04:00 Validating build format for: <redacted>.app.xcarchive
error: Invalid XCArchive: Missing required Info.plist file in .app bundle: /Users/telkins/Downloads/<redacted>.app.xcarchive/Products/Applications/<redacted>.app
3 participants
Every
.appinside of theProduct/Applicationsdirectory needs a correspondingInfo.plistfile since we use this to gather important information about the app. Currently we are allowing these uploads to take place which then immediately fail during processing -- not the greatest UX. It would be better to fail immediately so the user knows there is an issue since this means something went wrong with packaging their app.Worth noting: technically there's a way to embed this file directly in the Mach-O binary but that is seldom used and we don't support parsing that in
launchpadeither.Now we will fail and produce an error log:
Resolves EME-311