Skip to content
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

Merged
merged 11 commits into from Sep 24, 2017
Merged

XCConfig files #64

merged 11 commits into from Sep 24, 2017

Conversation

yonaskolb
Copy link
Owner

@yonaskolb yonaskolb commented Sep 23, 2017

Fixes #59

  • fix target configFiles
  • add project configFiles
  • read xcconfig files while doing build setting lookups
  • add fileGroups to project for including non build files (like configs that don't exist in any target sources)
  • documentation

@yonaskolb
Copy link
Owner Author

@toshi0383 this should fix your xcconfig issues. Note that this is still using the old names

@yonaskolb
Copy link
Owner Author

project.groups and project.settingGroups might clash. They might be renamed as well as configs. Any suggestions? settingGroups used to be called settingPresets actually

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()
Copy link
Collaborator

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?

Copy link
Collaborator

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?

Copy link
Owner Author

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 👍

Copy link
Collaborator

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.👌

Copy link
Collaborator

@toshi0383 toshi0383 Sep 24, 2017

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. 🙄

Copy link
Owner Author

@yonaskolb yonaskolb Sep 24, 2017

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)

Copy link
Owner Author

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 */ = {
Copy link
Collaborator

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).

Copy link
Owner Author

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

Copy link
Owner Author

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

@toshi0383
Copy link
Collaborator

project.groups and project.settingGroups might clash. They might be renamed as well as configs. Any suggestions? settingGroups used to be called settingPresets actually

I've read the documentation.
Looks like it's useful feature, too. I think it should be buildSettings and buildSettingsGroup to be explicit.

- ⚪️ **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.

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

@yonaskolb yonaskolb merged commit 8343593 into master Sep 24, 2017
@yonaskolb yonaskolb deleted the xcconfig branch September 25, 2017 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support xcconfig
3 participants