spotify / XCRemoteCache

Other
825 stars 50 forks source link

Precompile prefix headers fails on Consumer because the `-Swift.h` file includes an absolute path from Producer #140

Open samuelsainz opened 2 years ago

samuelsainz commented 2 years ago

My integration setup

[ x ] CocoaPods cocoapods-xcremotecache plugin [ ] Automatic integration using xcprepare integrate ... [ ] Manual integration [ ] Carthage

Expected/desired behavior

With the last version in master branch, the consumer mode is failing when precompiling prefix headers with the next error:

▸ Precompiling /Users/jenkins/workspace/xcremotecache-consumer/AppName/AppName/AppName-Prefix.pch

❌  /Users/jenkins/workspace/xcremotecache-consumer/AppName/derivedData-Simulator/Build/Intermediates.noindex/AppName.build/Debug Enterprise-iphonesimulator/MainTarget.build/DerivedSources/app-Swift.h:223:9: '/Users/jenkins/workspace/xcremotecache-producer/AppName/AppName/App-Bridging-Header.h' file not found

#import "/Users/jenkins/workspace/xcremotecache-producer/AppName/AppName/app-Bridging-Header.h"

After looking into the -Swift.h file, I have confirmed that there is an import with an absolute path created by the producer.

The /AppName/AppName/ path is where all the sources are and also the Podfile, the project, the workspace, etc.

Minimal reproduction of the problem with instructions

I can reproduce it 100% of the time just trying to build on consumer mode.

Environment

Others

I am running this on CI so I don't have the producer/consumer logs—but let me know if they're relevant and I can get them.

~I wasn't getting this error with v0.3.10~

polac24 commented 2 years ago

Hello @samuelsainz, I tried to reproduce an error but it works all the time.

Posted a sample project here: https://github.com/polac24/XCRCBridgingPCH

samuelsainz commented 2 years ago

Hey @polac24, thank you for building the project to troubleshoot this, super useful!

The difference I am seeing with that project vs. ours is that If I check the -Swift.h file created in DerivedSources for that project, it doesn't include an #import of the Bridging header.

I am checking the next file: /Users/samusan/Library/Developer/Xcode/DerivedData/App-drteszlujcryibcfcdnuntqmpdxz/Build/Intermediates.noindex/App.build/Debug-iphonesimulator/App.build/DerivedSources.

I am trying to figure it out why is that happening in our project and not in that sample project 🤔 any ideas?

polac24 commented 2 years ago

Thanks, that ringed a bell - I exposed an enum from ObjC and that led to #import ...Bridging-Header.h in the -Swift.h. See the pushed commit

Sidenote: That is absolutely a bug, but I am curious how could that work on previous version of XCRemoteCache (mentioned v0.3.10). Such a scenario was considered and we has never modified -Swift.h content.

samuelsainz commented 2 years ago

Cool! So that was it—I tried adding some imports to the bridging header but I didn't realize I had to add code in the files for the bridging header to be actually imported.

Answering your last question: I don't know to be honest, but I was able to produce and consume with that version. I had Precompile prefix header enabled—I was actually testing this fix.

I can test again that version to see if it works, I will let you know

samuelsainz commented 2 years ago

@polac24 you're right. I tried with that version and didn't work either. I can't explain why it did work before and it is not working now, I didn't change anything in our project since I've been working on a branch that I haven't updated—but I will fix the issue description. Any idea of how we can solve this issue? I might be able to make a PR

polac24 commented 2 years ago

Good to hear it was not a regression. I have a draft PR, but still, need to polish it a bit. FYI: https://github.com/spotify/XCRemoteCache/pull/141

samuelsainz commented 2 years ago

Hey @polac24, I have tested the PR #149, thank you for quickly addressing it btw.

I see that the error is fixed when using that branch, but I am experiencing another issue probably related:

When I build just the app it finishes successfully. However, when I build for testing I get these errors in all the tests targets:

Screen Shot 2022-06-24 at 12 54 57

That path to the app target Bridging Header is the absolute path in the producer side. I'm not finding where is taking that path from and how could I make it relative instead of absolute.

The Bridging Header of the main app is set in the Build Settings relative to the Sources folder: $SRCROOT/App-Bridging-Header.h

Do you know what could be causing this? Thanks in advance!

polac24 commented 2 years ago

If you extend the red error message with a hamburger icon, there should be a chain of import. It could help you to troubleshoot from which file the producer's header is still imported.

samuelsainz commented 2 years ago

The issue seems to be with the Bridging Header:

Screen Shot 2022-06-27 at 11 48 29

For all the types included through the Bridging Header I get redefinition of 'TypeName' error, because it is finding both the producer Bridging Header and the local Bridging header.

polac24 commented 2 years ago

I tried to reproduce it (no success), so probably your case is quite unique and it would be hard to guess all details like:

Reproduce that in a sample project could help: https://github.com/polac24/XCRCBridgingPCH

samuelsainz commented 2 years ago

Hi @polac24, I've tried to reproduce it in this fork (https://github.com/samuelsainz/XCRCBridgingPCH) but I'm facing a problem:

When I run the consumer for the sample project it always fail to cache the App target with the next error:

2022-06-29 10:44:50.326 I  xcprebuild[59166:553b54] (App) Local fingerprint Fingerprint(raw: "0b11613c5128f744970964b9ebde6d6a", contextSpecific: "00c4bf5c12754fbc40f0aa2628ebf902") does not match with remote one dff0b1be1400ab5a9b8c3b57166a1570.
2022-06-29 10:44:50.326 I  xcprebuild[59166:553b54] (App) Remote cache cannot be used

Since I can't consume the cached App target I can't reproduce the issue, because it recompiles the App target and also the Test target.

Do you know why I am getting that error?

samuelsainz commented 2 years ago

@polac24 Hey, looking at the meta file for the App target I saw that one of the dependencies was to the file DependencySwift/DependencySwift-Swift.h located in DerivedData

I removed the import of that file in the App target so it doesn't depend on it anymore and I was able to consume the cached App artifact—the fingerprints are equal now. Is this an expected behavior?

Having said that, I wasn't able to reproduce the original error yet, I will let you know If I make any progress.

polac24 commented 2 years ago

If you use XCRemoteCache from the #149 (I assume so, otherwise you would get a build error), there should be a cache hit.

Investigating that is required prior to merging it (#149) to master. Vacations time begins, and I am not sure about my availability to work on that.

samuelsainz commented 2 years ago

Hi @polac24! Sure, enjoy your well deserved vacations.

For when you come back from vacations—I have reproduced the issue in this sample project https://github.com/samuelsainz/XCRCBridgingPCH.

You have to build for testing when producing and consuming.

polac24 commented 1 year ago

Hi @samuelsainz! I reproduced an issue with Xcode 13.3 and 14.0 (beta 2) and seems it that .swiftmodule file embeds type definitions from bridging headers along the absolute path to the bridging header file it was taken from. Even if on a consumer side we fix the generated -Swift.h file (added support in #149), consumers may face duplicated types: one that is embedded in the .swiftmodule and the second from the header file. If consumer and producer absolute paths match each other, the compiler is able to duplicate them (consider them as the same definition), but if these paths are different, it reports an error.

However, if you look closer, the compiler generates a warning, which most likely is causing this kind of an edge case:

Implicit import of bridging header 'xxxxx-Bridging-Header.h' via module 'xxxxx' is deprecated and will be removed in a later version of Swift

Unfortunately, that behaviour is still present in Xcode 14 so in order to get rid of that error, you could apply the fix suggested in Apple's forum (a change in AppTests.swift):

// Fix suggested in https://developer.apple.com/forums/thread/74956: do not import module, but only `#import "App-Bridging-Header.h"` in `AppTests-Bridging-Header`
// @testable import App

With that, the error and warning go away and in your tests you still can use ViewController(). Would that unblock your scenario?

samuelsainz commented 1 year ago

Hey @polac24, sorry for the delay.

I'm afraid that that doesn't work for us, because in our case the App target has both Swift and Objective-C code. If we remove the import of the App module we won't have access to the Swift code in that test target.

I saw that warning as well. If I am understanding this right, not having that Implicit import of the Bridging-Header would solve our issue, right? But I didn't find a way to disable that either

polac24 commented 1 year ago

Me neither, looks that this https://github.com/apple/swift/pull/5977 has never got a follow-up PR so the bridging header is always explicitly imported.

Until Swift fixes that, this scenario will not be covered in XCRemoteCache.