Skip to content

Remove https:// from Info.plist to address compilation fail - #24297

Merged
kean merged 1 commit into
task/remove-app-constantsfrom
mokagio/fix-build-settings-https
Mar 24, 2025
Merged

Remove https:// from Info.plist to address compilation fail#24297
kean merged 1 commit into
task/remove-app-constantsfrom
mokagio/fix-build-settings-https

Conversation

@mokagio

@mokagio mokagio commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

Builds on top of #24294 to address a build failure seen in CI and reproducible locally running Fastlane.

Apparently, Xcode/xcodebuild fails to parse straight "https://" in Info.plists, see how https://buildkite.com/automattic/wordpress-ios/builds/26578/steps?jid=0195bb59-1f1a-40ad-89c9-3964c36cff28#0195bb59-1f1a-40ad-89c9-3964c36cff28/973-5502 failed and notice the build for this commit passes.

@dangermattic

dangermattic commented Mar 24, 2025

Copy link
Copy Markdown
Collaborator
1 Error
🚫 PR requires at least one label.

Generated by 🚫 Danger

@mokagio
mokagio force-pushed the mokagio/fix-build-settings-https branch from 7c774b4 to 6054c9e Compare March 24, 2025 09:50
mokagio added a commit that referenced this pull request Mar 24, 2025
@wpmobilebot

wpmobilebot commented Mar 24, 2025

Copy link
Copy Markdown
Contributor
WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr24297-6054c9e
Version25.8
Bundle IDorg.wordpress.alpha
Commit6054c9e
App Center BuildWPiOS - One-Offs #11814
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot

Copy link
Copy Markdown
Contributor
Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr24297-6054c9e
Version25.8
Bundle IDcom.jetpack.alpha
Commit6054c9e
App Center Buildjetpack-installable-builds #10836
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@mokagio
mokagio requested review from crazytonyli and kean March 24, 2025 10:13
Comment on lines +43 to +46
func urlValue(forKey key: String) -> URL {
let urlWithoutScheme: String = infoValue(forKey: key)
return URL(string: "https://\(urlWithoutScheme)")!
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be improved with a fallback like if the strings already has "https://" at the start, don't prefix it with it.

Comment thread WordPress/Info.plist
Comment on lines -26 to +28
<string>https://twitter.com/WordPressiOS</string>
<string>twitter.com/WordPressiOS</string>
<key>WPProductBlogURL</key>
<string>https://wordpress.org/news/</string>
<string>wordpress.org/news/</string>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach fixes the issue but introduces the undocumented and unforced rule of having URLs specified without the schema.

@mokagio
mokagio marked this pull request as ready for review March 24, 2025 10:49
@mokagio mokagio mentioned this pull request Mar 24, 2025
14 tasks
@kean

kean commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

Ah, right. I don't know how I tested it – evidently I did not. I'm pretty sure you can put URLs in regular .plist files without preprocessing and it simply escapes them.

Thanks for addressing it! For a single key/value, I think it's good enough.

@kean
kean merged commit 8bbd61c into task/remove-app-constants Mar 24, 2025
@kean
kean deleted the mokagio/fix-build-settings-https branch March 24, 2025 15:13
github-merge-queue Bot pushed a commit that referenced this pull request Mar 24, 2025
* Move some content group and styles to FormattableContentKit

These are needed by the WordPressData files.

* Address SwiftLint violations

* Change `formatter` to `private`

* Add note to address #24297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants