Silence dependency analysis warning #7698
Conversation
|
@frederoni, thanks for your PR! By analyzing this pull request, we identified @1ec5, @boundsj and @friedbunny to be potential reviewers. |
frederoni
force-pushed
the
fred-silence-analysis-warning
branch
from
b9526f8
to
dbb29d9
Compare
frederoni
force-pushed
the
fred-silence-analysis-warning
branch
from
dbb29d9
to
4f3ab0c
Compare
frederoni
added
the
⚠️ DO NOT MERGE
frederoni
removed
the
⚠️ DO NOT MERGE
| @@ -2349,7 +2349,7 @@ | |||
| DA25D5BC1CCD9EDE00607828 /* Debug */ = { | |||
| isa = XCBuildConfiguration; | |||
| buildSettings = { | |||
| INFOPLIST_FILE = framework/Settings.bundle/Root.plist; | |||
| INFOPLIST_FILE = "framework/Info-static.plist"; | |||
There was a problem hiding this comment.
This build setting is part of the settings target, which produces Settings.bundle, not Mapbox.framework. Doesn’t this mean Settings.bundle will contain an Info.plist that describes the entire framework?
There was a problem hiding this comment.
It would. Same goes for the Bundle.bundle which the static target also depends on. (I copied this from the bundle target)
Is the Settings.bundle only meant to be used in iosapp?
There was a problem hiding this comment.
Settings.bundle is an example meant to be used in any iOS application. A Settings bundle isn’t supposed to have an Info.plist, though.
|
Moving to v3.4.1 because this issue doesn’t impact shipping code and may well be a regression in Xcode. (A settings bundle isn’t supposed to have an Info.plist to begin with.) |
|
Since this PR is scheduled for iOS SDK v3.5.0, it needs to be rebased and retargeted at the release-ios-v3.5.0-android-v5.0.0 branch. |
|
What’s the status of this, now that Xcode 8.3 is available? |
|
This whole saga started with a code-signing error: #6947. Do we even need to code-sign the Settings.bundle if it’s going to be plopped into an application bundle at the end of the day? |
frederoni
force-pushed
the
fred-silence-analysis-warning
branch
from
4f3ab0c
to
caf6903
Compare
frederoni
added
the
⚠️ DO NOT MERGE
frederoni
force-pushed
the
fred-silence-analysis-warning
branch
2 times, most recently
from
debc049
to
2031c2f
Compare
frederoni
removed
the
⚠️ DO NOT MERGE
|
Migrating the project to Xcode 9 also removes the plist from build phases which confirms that this is the right fix. |
| @@ -2110,7 +2109,6 @@ | |||
| buildActionMask = 2147483647; | |||
| files = ( | |||
| DA25D5C61CCDA06800607828 /* Root.strings in Resources */, | |||
| DA25D5C01CCD9F8400607828 /* Root.plist in Resources */, | |||
There was a problem hiding this comment.
I’m confused by this change. We’re removing Root.plist from the Copy Files build phase, but it remains the bundle’s Info plist?
There was a problem hiding this comment.
Which seems to be correct, no? It's what you get when you create a new framework from scratch. The plist is bundled in the framework but omitted from the Copy Files build phase.
(https://developer.apple.com/library/content/documentation/MacOSX/Conceptual/BPFrameworks/Concepts/FrameworkAnatomy.html)
There was a problem hiding this comment.
But the file in the resulting bundle is called Info.plist, not Root.plist, right? Does the Settings screen show the correct content after installing this bundle?
There was a problem hiding this comment.
🙈 Back to square one. We don't want to use an Info.plist but we are forced to use one.
frederoni
force-pushed
the
fred-silence-analysis-warning
branch
from
2031c2f
to
5fe3e0d
Compare
|
There appears to be no good way to get Xcode to not warn about our non-standard, example-only Settings.bundle. |
#6948 fixed a compile-time error that started with Xcode 8.1 but introduced a new warning in 8.2
This PR fixes that by specifying the correct Info.plist instead of the Root.plist that's bundled in settings.
https://developer.apple.com/library/content/qa/qa1649/_index.html
Verified that the Settings.bundle is still working:
