Fix generating project with custom configurations (other than Debug and Release) via SPM packages #4259
Conversation
mstfy
requested review from
danieleformichelli,
fortmarek,
luispadron and
adellibovi
| @@ -1026,7 +1027,21 @@ extension PackageInfo { | |||
| settingsDictionary["SWIFT_VERSION"] = .string(swiftLanguageVersion) | |||
| } | |||
|
|
|||
| return settingsDictionary.isEmpty ? nil : .settings(base: settingsDictionary) | |||
| let hasCustomConfig = buildConfigs.firstIndex(where: { !["Debug", "Release"].contains($0.name) }) != nil | |||
There was a problem hiding this comment.
It would probably be better to make this field optional. If user populates it then it has a value, otherwise it's nil and we use defaults.
There was a problem hiding this comment.
I think the logic should be that we always use what we got. If the user wants to use the defaults it' already handled by the init AFAIR
| @@ -1026,7 +1027,21 @@ extension PackageInfo { | |||
| settingsDictionary["SWIFT_VERSION"] = .string(swiftLanguageVersion) | |||
| } | |||
|
|
|||
| return settingsDictionary.isEmpty ? nil : .settings(base: settingsDictionary) | |||
| let hasCustomConfig = buildConfigs.firstIndex(where: { !["Debug", "Release"].contains($0.name) }) != nil | |||
There was a problem hiding this comment.
I think the logic should be that we always use what we got. If the user wants to use the defaults it' already handled by the init AFAIR
|
Also, remember to add a changelog entry 🙏 |
mstfy
force-pushed
the
fix/spm-dependencies-with-custom-config
branch
from
8886215
to
24735be
Compare
| @@ -1026,7 +1027,20 @@ extension PackageInfo { | |||
| settingsDictionary["SWIFT_VERSION"] = .string(swiftLanguageVersion) | |||
| } | |||
|
|
|||
| return settingsDictionary.isEmpty ? nil : .settings(base: settingsDictionary) | |||
| if let buildConfigs = buildConfigs, | |||
| buildConfigs.firstIndex(where: { !["Debug", "Release"].contains($0.name) }) != nil { | |||
There was a problem hiding this comment.
Why do we need this condition? If a custom config is passed we should take it, regardless of its name, isn't it?
There was a problem hiding this comment.
Debug and Release wil always be in the generated project by default. Adding them here also breaks almost all tests here. So I choose to add settings if custom config specified by the user.
There was a problem hiding this comment.
I wouldn't add this change if it's only for the tests. Will it be so hard to fix them without including this line?
There was a problem hiding this comment.
Hey @mstfy 👋
The changes look good, however, I think we need to bump the version here to 2.0.0 since the SPM dependencies now have additional configurations that they did not have previously.
That should fix this issue - I have tried it locally and was getting Framework ProjectDescription not found. @danyf90 since you were the one who introduced it here, does bumping the cache version make sense to you? Or is it a bug inside the caching functionality instead?
@mstfy, you will also need to fix the lint issues.
|
No, that should be bumped only if something cache related changes, if the change is before (e.g. in the mapping to TuistGraph), then it shouldn't be needed. |
adellibovi
changed the title
adellibovi
added
the
changelog:fixed
Short description 📝
This PR attempts to fix that generated project doesn't have correct configurations when custom config specified via Dependencies.swift
Long description
Xcode fails to compiling generated project with 'Framework not found' error when importing spm dependency via Dependencies.swift to a project that have configurations other than
DebugandRelease. This pr adds custom config to generated xcode project from spm dependency.