spotify / XCRemoteCache

Other
825 stars 50 forks source link

Enabling cache can insert a nil build phase in certain circumstances #217

Closed Raznic closed 11 months ago

Raznic commented 11 months ago

Problem

While integrating XCRemoteCache into my project, I was getting vague error messages about nil objects:

NoMethodError - undefined method `uuid' for nil:NilClass

            Nanaimo::String.new(object.uuid, object.ascii_plist_annotation)

Some deeper investigation revealed that these errors were only happening for targets related to running tests. Excluding these targets allowed me to enable XCRemoteCache for my project, but it was unclear what about these targets was causing issues.

Cause

Eventually I found the root cause was the following line for inserting the postbuild script at the very end of the build phases:

https://github.com/spotify/XCRemoteCache/blob/afb1f9e53121e88895da40860761818740a7fdd0/cocoapods-plugin/lib/cocoapods-xcremotecache/command/hooks.rb#L209

In most cases this logic is correct, however I discovered that target.source_build_phase will either return a PBXSourcesBuildPhase if it exists, or create one if it doesn't. In the case of my test targets, I did not have a PBXSourcesBuildPhase by default, so it would create one for me. Prior to moving the postbuild script, my target.build_phases looked like this:

[
    <PBXFrameworksBuildPhase UUID=`3FD09B4D78E7D6E98FEE6B62`>,
    <PBXShellScriptBuildPhase name=`[XCRC] Postbuild UnitTests` UUID=`52A7EAFA724B9FB94DE7DE96`>,
    <PBXSourcesBuildPhase UUID=`8CF1A801DA0F6A4AF286BED0`>
]

And then after this line, my target.build_phases looks like this:

[
    <PBXFrameworksBuildPhase UUID=`3FD09B4D78E7D6E98FEE6B62`>,
    <PBXSourcesBuildPhase UUID=`8CF1A801DA0F6A4AF286BED0`>,
    nil,
    <PBXShellScriptBuildPhase name=`[XCRC] Postbuild UnitTests` UUID=`52A7EAFA724B9FB94DE7DE96`>
]
polac24 commented 11 months ago

If your target doesn't have any sources to compile, there is no reason to enable XCRemoteCache for it.

We could add that logic to the CocoaPods plugin, but generally, that would be equivalent to the target exclusion.

Raznic commented 11 months ago

If your target doesn't have any sources to compile, there is no reason to enable XCRemoteCache for it.

That makes total sense, definitely agree that we can just add the targets to the exclusion list rather than needing to implement special logic for it.

Would it make sense to add a warning for targets with no sources, or at least print which targets are causing issues in these cases? Part of the issue for me was the time it took to identify which targets were being problematic because they weren't captured anywhere in the stacktrace.

polac24 commented 11 months ago

Of course, once we identify the non-compilation target, we might optimistically disable XCRemoteCache for that target and print a message. Disabling should we really simple, as it would be the same logic as for the exclusion.

polac24 commented 11 months ago

The newly release CocoaPods plugin 0.0.18 should fix that.