-
Notifications
You must be signed in to change notification settings - Fork 822
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
XCConfig files #64
XCConfig files #64
Conversation
@toshi0383 this should fix your xcconfig issues. Note that this is still using the old names |
|
public func getCombinedBuildSettings(basePath: Path, target: Target, config: Config, includeProject: Bool) -> BuildSettings { | ||
var buildSettings: BuildSettings = [:] | ||
if includeProject { | ||
if let configFilePath = configFiles[config.name] { | ||
if let configFile = try? XCConfig(path: basePath + configFilePath) { | ||
buildSettings += configFile.flattenedBuildSettings() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe including xcconfig should just #include
the included xcconfig, not adding to buildSettings section.🤔
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and what if included xcconfig #include
other xcconfigs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean by the first point? Maybe these will answer your question.
- this function is just use for getting all the combined settings for the purposes of checking what build settings will be applied - similar to Xcode's
combined
settings view - xcconfigs aren't added using #include they are just set as a baseconfiguration for the target or project. The rest of the build settings are applied directly on the build setting or project and don't use an xcconfig
For the second point configFile. flattenedBuildSettings()
goes through all #include files and merges them 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I'm going to take a look at the details.👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the second point configFile. flattenedBuildSettings() goes through all #include files and merges them 👍
What I meant was XcodeGen:Include
.
I see XcodeGen:Include
just ignores XcodeGen:Included
spec's configFile
.
TestProject $ head spec.yml
include: included.yml
name: GeneratedProject
groups:
- Configs
targets:
TestProject:
type: application
platform: iOS
sources: TestProject
settings:
TestProject $ head included.yml
configFiles:
Debug: Included/Configs/included.xcconfig # This is completely ignored when `XcodeGen:included`.
This behavior is perfectly 🙆♂️ to me. Maybe it worth it to warn user when XcodeGen:included
spec has configFile attribute, because it's ignored.
Maybe we should really rename XcodeGen:include
to something else to avoid confusion. 🙄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because include is a list not just a string. I should change this though to allow a simple string for a single value, just like a target's sources:
include: [included.yml] #works, using json style
include:
- included.yml #works, using yaml style
include: included.yml #doesn't work (yet, will change)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include
can now be a single string 39c8af3
@@ -102,9 +104,20 @@ | |||
path = TestProjectTests; | |||
sourceTree = "<group>"; | |||
}; | |||
CC9392641F76EDE600C1934A /* Configs */ = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you create this TestProject.xcodeproj/project.pbxproj
?
On Xcode, Configs
group is missing reference (appears red).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TestProject.xcodeproj
was created and edited in Xcode. GeneratedProject.xcodeproj
is the one generated by XcodeGen.
I'm not sure what you mean by missing configs group. Both projects appear fine for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw what you mean by the red group, I've fixed this in master. This project isn't really used in any tests and is more just for a comparison between a project created in xcode versus created in XcodeGen
I've read the documentation. |
docs/ProjectSpec.md
Outdated
- ⚪️ **settings**: [Settings](#settings) - Project specific settings. Default base and config type settings will be applied first before any settings defined here | ||
- ⚪️ **settingGroups**: [Setting Groups](#setting-groups) - Setting groups mapped by name | ||
- ⚪️ **targets**: [Target](#target) - The list of targets in the project mapped by name | ||
- ⚪️ **fileGroups**: `[String]` - A list of paths to add to the top level groups. These are files that aren't build files but that you'd like in the project heirachy. For example a folder xcconfig files that aren't already added by any target sources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a typo heirachy
~> hierarchy
Fixes #59
configFiles
configFiles
fileGroups
to project for including non build files (like configs that don't exist in any target sources)