MobileOrg / mobileorg

MobileOrg iPhone App
http://mobileorg.github.io
GNU General Public License v2.0
557 stars 70 forks source link

Switch to Carthage #255

Closed jdek closed 4 years ago

jdek commented 4 years ago

I originally started doing this when I couldn't build MobileOrg from the repository. I'm looking at merging the different profiles at the moment.

This will fix #247 when it is completed.

webframp commented 4 years ago

Wonderful to see someone working on this. I’ve wanted to modernize the build and project config for a while but it’s always ended up on the back burner for me

jdek commented 4 years ago

I integrated it into the config (also pinged you on IRC, but I guess you didn't see it), is there anything obvious I'm missing (other than a README update). If you think it's alright, then I will update the README, squash, and update the PR.

dive commented 4 years ago

@jdek, looks good, but you embed these frameworks into the application bundle directly via Build Phases -> Embed Frameworks without postprocessing. The problem is, that frameworks produced by Carthage are fat-binaries and include all architectures. Like:

$ file Carthage/Build/iOS/SwiftyDropbox.framework/SwiftyDropbox
Carthage/Build/iOS/SwiftyDropbox.framework/SwiftyDropbox: Mach-O universal binary with 4 architectures: [i386:Mach-O dynamically linked shared library i386] [x86_64] [arm_v7] [arm64]
Carthage/Build/iOS/SwiftyDropbox.framework/SwiftyDropbox (for architecture i386):   Mach-O dynamically linked shared library i386
Carthage/Build/iOS/SwiftyDropbox.framework/SwiftyDropbox (for architecture x86_64): Mach-O 64-bit dynamically linked shared library x86_64
Carthage/Build/iOS/SwiftyDropbox.framework/SwiftyDropbox (for architecture armv7):  Mach-O dynamically linked shared library arm_v7
Carthage/Build/iOS/SwiftyDropbox.framework/SwiftyDropbox (for architecture arm64):  Mach-O 64-bit dynamically linked shared library arm64

It is convenient for debugging and development but we have to strip all non-device architectures to submit the application to the AppStore.

To do so, just follow the instruction from the Carthage page (starting from point number 4).

jdek commented 4 years ago

@dive thanks for the comment, I wasn't aware of this, I have fixed it now. I will update the README later today.

webframp commented 4 years ago

Going to defer to @dive on this review since Carthage is also new to me, but I like the approach and agree it will simplify work for contributors in the future.

Once merged I will handle getting a new build pushed out to TestFlight.

dive commented 4 years ago

@dive thanks for the comment, I wasn't aware of this, I have fixed it now. I will update the README later today.

How is it going @jdek? Need help? The recent changes look good and fix the problem with fat-binaries. We are waiting for updates in README.md and ready to merge the PR.

jdek commented 4 years ago

Sorry, I honestly completely forgot (been super busy)!

@dive @webframp, PR updated with README, and squashed commits. Thank your for your patience.

dive commented 4 years ago

@webframp, ping.

webframp commented 4 years ago

Hey @dive I'm going to get to it this week, been a little backed up with work the last week

webframp commented 4 years ago

Found that the current config doesn't auto increment the build number, very minor but might need to adjust before submitting the next test flight release

webframp commented 4 years ago

Code signing on the frameworks also fails right now, so I'll be looking into that before merging.

image
dive commented 4 years ago

@webframp, you can post the distribution log here if you need help with the investigation.

webframp commented 4 years ago

@dive Here's what I found that seems of interest in the IDEDistributionPipeline.log:

2020-01-24 23:51:09 +0000  Running /usr/bin/codesign '-vvv' '--force' '--sign' '285394214F2EC2680082FE7AD1CD929CF41D67BC' '--entitlements' '/var/folders/66/180ynjt55z74l338gh0stq0c0000gn/T/XcodeDistPipeline.~~~UNzizR/entitlements~~~75UGvx' '--preserve-metadata=identifier,flags,runtime' '/var/folders/66/180ynjt55z74l338gh0stq0c0000gn/T/XcodeDistPipeline.~~~UNzizR/Root/Payload/MobileOrg.app/Frameworks/Alamofire.framework'
2020-01-24 23:51:09 +0000  /var/folders/66/180ynjt55z74l338gh0stq0c0000gn/T/XcodeDistPipeline.~~~UNzizR/Root/Payload/MobileOrg.app/Frameworks/Alamofire.framework: replacing existing signature
2020-01-24 23:51:09 +0000  /var/folders/66/180ynjt55z74l338gh0stq0c0000gn/T/XcodeDistPipeline.~~~UNzizR/Root/Payload/MobileOrg.app/Frameworks/Alamofire.framework: code object is not signed at all
2020-01-24 23:51:09 +0000  /usr/bin/codesign exited with 1

I'm definitely missing something related to codesigning on 3rd party deps with carthage

webframp commented 4 years ago

@dive sent you a message on gitter with d/l link for the zip

dive commented 4 years ago

@webframp, @jdek, I checked the logs and the branch. The problem is that we embed Alamofire.framework and SwiftyDropbox.framework frameworks twice:

  1. The Carthage script embed these frameworks first;
  2. And then we have "Embed Frameworks" build phase in the Xcode project settings.

In the end, the second step overrides properly signed and embedded by Carthage Alamofire.framework and SwiftyDropbox.framework with unsigned binaries.

@jdek, to fix it, we need to remove "Embed Frameworks" from the build phases (just press the cross at the top right corner).

Screenshot 2020-01-31 at 12 24 35
jdek commented 4 years ago

I'll do it tomorrow if I can. I'll be at FOSDEM

jdek commented 4 years ago

@dive see if that fixes it.

webframp commented 4 years ago

@jdek fixes the issue for me, one last thing failing validation now related to UILaunchImages type mismatch, which I can look at

webframp commented 4 years ago

I'll fixup the UILaunchImages issue with the plist since they're deprecated anyway

thanks @jdek !