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

Targets built with coverage data in Xcode 9 #2056

Closed
frankschlegel opened this issue Jul 11, 2017 · 34 comments · Fixed by #2158
Closed

Targets built with coverage data in Xcode 9 #2056

frankschlegel opened this issue Jul 11, 2017 · 34 comments · Fixed by #2158
Assignees

Comments

@frankschlegel
Copy link

One of my test builds I just submitted to iTunes Connect (built with Xcode 9) failed processing with the following message:

Invalid Bundle - Disallowed LLVM instrumentation. Do not submit apps with LLVM profiling instrumentation or coverage collection enabled. Turn off LLVM profiling or code coverage, rebuild your app and resubmit the app.

This is caused by this issue. The new Xcode seems to build coverage data into all builds, not only for testing. From the changelog:

Schemes with Code Coverage enabled (in the Test action) will now always build with Code Coverage, including for the Run and Launch actions. This reduces the need to build multiple versions of the application. Users who require code coverage to be off for their non-test actions can explicitly use a separate scheme with coverage disabled. (31848014)

When building frameworks during the "Archive" action Xcode seems to handle that. But since Carthage dependencies are built externally and copied into the bundle later, they still contain coverage data.

Somehow the Carthage build process needs to make sure that gathering coverage data is disabled during build, even though it's enabled in the project.

@ikesyo
Copy link
Member

ikesyo commented Jul 11, 2017

Thanks for filing the issue! I've submitted #2057 to address this.

@frankschlegel
Copy link
Author

Wow, that was fast, thanks!

@ikesyo
Copy link
Member

ikesyo commented Jul 11, 2017

#2057 (comment)

Looks like we will not be able to make xcodebuild to disable code coverage when building using command line options unfortunately.

@ikesyo ikesyo added the wontfix label Jul 11, 2017
@ikesyo ikesyo closed this as completed Jul 11, 2017
@aabosh
Copy link

aabosh commented Aug 13, 2017

Running into the same issue.

@dbmrq
Copy link

dbmrq commented Sep 11, 2017

I forked these two repositories, disabled coverage data and edited my Cartfile to use the forked versions:

https://github.com/dbmrq/RSBarcodes_Swift/commit/4306df0f32b251929117a366d67883fd3b6a25d8

https://github.com/dbmrq/SWXMLHash/commit/2f58a1981d09fee9908feac5b092c89312e8e7ed

I'm still getting the "Disallowed LLVM instrumentation" error though. Is there anything else I have to do? :/

@insidegui
Copy link

I'm having the same issue. Tried the same as @dbmrq, still getting the error from iTC.

@dbmrq
Copy link

dbmrq commented Sep 11, 2017

I don't know much about how Carthage works, so I'm sorry if this is a silly question, but wouldn't it be possible for Carthage to use the Archive action when building the frameworks?

With the 2 frameworks I mentioned above I just disabled the Skip Install setting, archived, exported and copied the resulting frameworks myself.

I looked around here and found this: #833 (comment)

So it seems this was an option for @mdiep. Why was it dismissed?

@jdhealy
Copy link
Member

jdhealy commented Sep 11, 2017

I looked around here and found this: #833 (comment)

So it seems this was an option for @mdiep. Why was it dismissed?

From #169 (comment) — referred to by @ikesyo in #2057 (comment):

Apparently xcodebuild archive does not work for the simulator SDK

@dbmrq
Copy link

dbmrq commented Sep 11, 2017

Ah, I don't know how I missed that.

It seems that the -exportFormat option mentioned in #169 (comment) doesn't exist anymore though, and I did manage to successfully archive the frameworks I mentioned with xcodebuild archive.

I just don't understand the simulator SDK part… that doesn't even come up for me:

iOS/RSBarcodes_Swift [master] > xcodebuild archive -scheme RSBarcodes
=== BUILD TARGET RSBarcodes OF PROJECT RSBarcodes WITH CONFIGURATION Release ===

Check dependencies

Write auxiliary files

#

** ARCHIVE SUCCEEDED **

@jdhealy
Copy link
Member

jdhealy commented Sep 11, 2017

Preventing the Carthage behavior to merge device and simulator builds into “one fat binary”, xcodebuild archive for simulator is still prohibited in Xcode 9.0.X (betas, as is the current extent of our knowledge).

$ xcodebuild -destination 'generic/platform=iOS Simulator' archive -scheme RSBarcodes
2017-09-11 00:26:09.026 xcodebuild[34407:8122792] [MT] DVTAssertions: Warning in /Library/Caches/com.apple.xbs/Sources/IDEFrameworks/IDEFrameworks-13238.4/IDEFoundation/Execution/RunDestinations/IDERunDestinationManager.m:789
Details:  There are multiple run destinations that support deployment for the current scheme; selecting anyObject: {(
    <IDERunDestination:0x7fb1682bd340:'Generic iOS Simulator Device'>,
    <IDERunDestination:0x7fb1682c5c20:'Generic iOS Device'>
)}
Object:   <IDERunDestinationManager: 0x7fb168112390>
Method:   -genericRunDestinationForRunDestination:scheme:schemeCommands:executionEnvironment:requiresSupportsArchiving:requiresDeploymentPlatformMatches:allowMultipleRunDestinationMatches:allowNoRunDestinationMatches:error:
Thread:   <NSThread: 0x7fb163416180>{number = 1, name = main}
Please file a bug at http://bugreport.apple.com with this warning message and any useful information you can provide.
2017-09-11 00:26:09.038 xcodebuild[34407:8122792] [MT] DVTAssertions: Warning in /Library/Caches/com.apple.xbs/Sources/IDEFrameworks/IDEFrameworks-13238.4/IDEFoundation/Execution/Schemes/IDEScheme.m:1431
Details:  Scheme <IDEScheme:0x7fb16834af00:'RSBarcodes'> was asked to build and archive, but the run destination <IDERunDestination:0x7fb1682bd340:'Generic iOS Simulator Device'> is not a deployment platform and this action shouldn't have been allowed.
Object:   <IDEScheme: 0x7fb16834af00>
Method:   -schemeOperationForSchemeOperationParameters:buildLog:overridingProperties:overridingBuildConfiguration:dontActuallyRunCommands:restorePersistedBuildResults:deviceAvailableChecker:error:completionBlock:
Thread:   <NSThread: 0x7fb163416180>{number = 1, name = main}
Please file a bug at http://bugreport.apple.com with this warning message and any useful information you can provide.
xcodebuild: error: Failed to build project RSBarcodes with scheme RSBarcodes.
	Reason: You cannot archive for the iOS Simulator platform.

$ xcodebuild -version
Xcode 9.0
Build version 9M214v

@dbmrq
Copy link

dbmrq commented Sep 11, 2017

Ah, I see. I didn't realize Carthage required the targer to be built for the simulator.

We're back to the first question then, why it still doesn't work after disabling coverage data.

@frankschlegel
Copy link
Author

@dbmrq I went to the Carthage/Checkout of the affected projects, opened their project files in Xcode and unchecked the build setting. Then I did a carthage build and the problem was gone. Mind that the setting gets reset when doing a update or bootstrap.

As for the archive action: I also think this is the way to go. You could use build for the simulator part and archive for the ARM binary. The simulator binary gets stripped in the build phase anyways.
I tried to implement that in Carthage myself, but unfortunately the paths for the archived builds are different and I didn't have to time to dig deeper into Carthage.

@dbmrq
Copy link

dbmrq commented Sep 11, 2017

@frankschlegel Aha! I checked the Checkout directory and the coverage data setting was still there, even if I changed it in my fork… so I tried specifying my commit in the Cartfile and Carthage complained about it not being in Caches/org.carthage.CarthageKit. I deleted that file and now it's all working fine. Thanks! @insidegui You might want to look into that.

About xcodebuild archive, I was just thinking about that... when you add a new Run Script Phase on Xcode there's the option to "run script only when installing". So instead of building a fat binary for all architectures it could be possible to use xcodebuild archive to build a different framework just for App Store submission. Then there would be something like the current copy-frameworks script, but it would use that "run script only when installing" option to replace the simulator frameworks with the archived ones. I guess this would add a little more complexity, but I think doing things the way Apple intends them to be done makes it worth it and would save other headaches on the long run. It would be even better if this could all be done with a single script, but I still think it's worth it.

unfortunately the paths for the archived builds are different

xcodebuild allows you to specify the archive path with -archivePath, so that might help.

-archivePath xcarchivepath
Specifies the path for the archive produced by the archive action, or specifies the archive that should be exported when -exportArchive is passed.

@insidegui
Copy link

@dbmrq I manually set the "Enable Code Coverage Support" to NO on all dependencies and it worked :)

cglarsen added a commit to nodes-ios/Cashier that referenced this issue Oct 4, 2017
ivanschuetz added a commit to ivanschuetz/ReactiveReSwift that referenced this issue Oct 5, 2017
Fixes app submission with Xcode 9
See Carthage/Carthage#2056
ivanschuetz added a commit to ivanschuetz/ReactiveReSwift that referenced this issue Oct 5, 2017
Fixes app submission with Xcode 9
See Carthage/Carthage#2056
defagos added a commit to SRGSSR/srglogger-apple that referenced this issue Oct 9, 2017
defagos added a commit to SRGSSR/srgappearance-apple that referenced this issue Oct 9, 2017
defagos added a commit to SRGSSR/srgmediaplayer-apple that referenced this issue Oct 9, 2017
defagos added a commit to SRGSSR/MAKVONotificationCenter that referenced this issue Oct 9, 2017
defagos added a commit to SRGSSR/libextobjc that referenced this issue Oct 9, 2017
@ikesyo
Copy link
Member

ikesyo commented Oct 11, 2017

0.26 has been released.

https://github.com/Carthage/Carthage/releases/tag/0.26.0

@nasht
Copy link

nasht commented Nov 14, 2017

Hey @ikesyo -Thanks so much for the update. Am I right in my understanding that the archive directive is not going to solve the issue we face when we have specific versions of a third party framework with CodeCoverage set to YES? In this case, we still need to rely on scripts that modify the downloaded source before we build? Is this correct?

@eneko
Copy link

eneko commented Nov 25, 2017

After running some tests with Carthage 0.26.2, seems like the issue is still there. With Code Coverage enabled in the schemes, the otool command finds the symbols in the framework.

$ carthage version
0.26.2
$ carthage build --no-skip-current --configuration Release
...
$ otool -l -arch all Carthage/Build/iOS/MyFramework.framework/MyFramework | grep __llvm_prf
  sectname __llvm_prf_cnts
  sectname __llvm_prf_data
  sectname __llvm_prf_names
  sectname __llvm_prf_vnds
  sectname __llvm_prf_cnts
  sectname __llvm_prf_data
  sectname __llvm_prf_names
  sectname __llvm_prf_vnds
$

Removing Code Coverage on the schemes seems to be the only way for the symbols to disappear:

$ carthage build --no-skip-current --configuration Release
...
$ otool -l -arch all Carthage/Build/iOS/MyFramework.framework/MyFramework | grep __llvm_prf
$

@ikesyo
Copy link
Member

ikesyo commented Nov 25, 2017

otool -l -arch all

@eneko I think those are from simulator slices and they will be stripped by carthage copy-frameworks. So that should not be an issue on App Store submission.

@eneko
Copy link

eneko commented Nov 25, 2017

Thanks for the reply. We'll try again on Monday, hopefully it will work :)

@sarah-j-smith
Copy link

sarah-j-smith commented Jan 16, 2018

This issue is still there for me with v 0.27.0 for:

UPDATE:

OK - I see #2057

That does fix the problem but I need to remember to specify the release configuration:

carthage build --configuration Release

@ikesyo
Copy link
Member

ikesyo commented Jan 16, 2018

Carthage uses Release configuration by default so you don't need to specify it explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment