Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client/cordova/apple/ios): fix failing release build for the cordova ios target #1840

Merged
merged 4 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ $(BUILDDIR)/android/tun2socks.aar: $(GOMOBILE)

$(BUILDDIR)/ios/Tun2socks.xcframework: $(GOMOBILE)
# -iosversion should match what outline-client supports.
$(GOBIND) -iosversion=11.0 -target=ios,iossimulator -o $@ -ldflags '-w' -bundleid org.outline.tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks
$(GOBIND) -iosversion=12.0 -target=ios,iossimulator -o $@ -ldflags '-w' -bundleid org.outline.tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/tun2socks $(IMPORT_PATH)/$(ROOT_PKG)/outline/shadowsocks
jyyi1 marked this conversation as resolved.
Show resolved Hide resolved

$(BUILDDIR)/macos/Tun2socks.xcframework: $(GOMOBILE)
# MACOSX_DEPLOYMENT_TARGET and -iosversion should match what outline-client supports.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,7 @@
GCC_THUMB_SUPPORT = NO;
GCC_VERSION = "";
INFOPLIST_FILE = "Outline/Outline-Info.plist";
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
daniellacosse marked this conversation as resolved.
Show resolved Hide resolved
LD_RUNPATH_SEARCH_PATHS = "@executable_path/Frameworks";
ONLY_ACTIVE_ARCH = YES;
PRODUCT_BUNDLE_IDENTIFIER = org.outline.ios.client;
Expand Down Expand Up @@ -987,7 +987,7 @@
GCC_THUMB_SUPPORT = NO;
GCC_VERSION = "";
INFOPLIST_FILE = "Outline/Outline-Info.plist";
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
LD_RUNPATH_SEARCH_PATHS = "@executable_path/Frameworks";
ONLY_ACTIVE_ARCH = YES;
PRODUCT_BUNDLE_IDENTIFIER = org.outline.ios.client;
Expand Down Expand Up @@ -1391,7 +1391,7 @@
GCC_WARN_UNINITIALIZED_AUTOS = YES;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
MACOSX_DEPLOYMENT_TARGET = "$(RECOMMENDED_MACOSX_DEPLOYMENT_TARGET)";
ONLY_ACTIVE_ARCH = YES;
SDKROOT = iphoneos;
Expand Down Expand Up @@ -1435,7 +1435,7 @@
GCC_WARN_UNINITIALIZED_AUTOS = YES;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
IPHONEOS_DEPLOYMENT_TARGET = 11.0;
IPHONEOS_DEPLOYMENT_TARGET = 12.0;
MACOSX_DEPLOYMENT_TARGET = "$(RECOMMENDED_MACOSX_DEPLOYMENT_TARGET)";
SDKROOT = iphoneos;
SKIP_INSTALL = NO;
Expand Down
1 change: 0 additions & 1 deletion src/cordova/apple/xcode/ios/Outline/Outline.entitlements
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
<key>com.apple.security.application-groups</key>
<array>
<string>group.org.outline.ios.client</string>
<string>$(TeamIdentifierPrefix)org.outline.macos.client</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sbruens will this break mac Catalyst?

I don't understand why having this would break the build, since an app can be in multiple application groups. Perhaps we need to configure the application group to include the ios app.

Copy link
Contributor Author

@daniellacosse daniellacosse Feb 16, 2024

Choose a reason for hiding this comment

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

when it fails it says that the provision (org.outline.ios.client) doesn't match what's in this group (e.g. the macos group). perhaps we can use the interpolation to change this at compile time?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This will most likely break macOS, because they share this file.
Please wait for @sbruens to chime in.
Other than that, Makefile and pbxproj look good.

Copy link
Contributor

@sbruens sbruens Feb 16, 2024

Choose a reason for hiding this comment

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

It will break catalyst because this app group is used in several places to share data between the network extension, launcher and host app, like the logs directory for Sentry and the tunnel store (see https://github.com/search?q=repo%3AJigsaw-Code%2Foutline-client%20%22QT8Z3Q9V3A.org.outline.macos.client%22&type=code).

Those values need to match the app groups the app has access to. I think we may be able to move catalyst over to use the ios app group but it would mean that the catalyst app would no longer use the existing mac paths for logs and tunnel store preferences, but that might be ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw I added those entitlements very recently: #1811

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps the iOS app provisioning profile needs to be updated online to add those groups?

Copy link
Contributor

@sbruens sbruens Feb 20, 2024

Choose a reason for hiding this comment

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

I think the problem is that macOS groups are special and not managed in the Developer portal. So you can't even create an app group that doesn't start with group..

That app groups shouldn't be set on the iOS app anyway. But the XCode UI is weird where it automatically adds macOS app groups to the iOS section (but not the other way around?). Is there a way to set them in the release script?

I guess the best course of action would be to update those hard-coded values and move catalyst over to the iOS app group.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will the user lose their server list? That would be a very bad experience.
Perhaps we need separate projects? Can we have a per-platform plist?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so. They are not stored under the app group.

@daniellacosse you should be able to check that by running the catalyst app after you change these values and checking that your servers show up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, still seeing my servers:

Screenshot 2024-02-22 at 10 09 22 AM

</array>
<key>com.apple.security.network.client</key>
<true/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
<key>com.apple.security.application-groups</key>
<array>
<string>group.org.outline.ios.client</string>
<string>$(TeamIdentifierPrefix)org.outline.macos.client</string>
</array>
<key>com.apple.security.network.client</key>
<true/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@
</array>
<key>com.apple.security.app-sandbox</key>
<true/>
<key>com.apple.security.application-groups</key>
<array>
<string>$(TeamIdentifierPrefix)org.outline.macos.client</string>
</array>
<key>com.apple.security.files.user-selected.read-only</key>
<true/>
</dict>
Expand Down
Loading