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

[SR-2811] Generated Xcode projects need to allow .xcconfig files to override project settings #5202

Closed
abertelrud opened this issue Oct 1, 2016 · 12 comments
Assignees
Labels

Comments

@abertelrud
Copy link
Contributor

Previous ID SR-2811
Radar rdar://problem/28998196
Original Reporter @abertelrud
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Package Manager
Labels Bug, GeneratedXcodeProject
Assignee @abertelrud
Priority Medium

md5: 53602536c149152895c441deab64c5a3

Issue Description:

In the rewritten Xcode project generator, all build settings are generated into the project file, with the goal of keeping the generated project as simple as possible. The previous implementation always generated three intermediate .xcconfig files, in addition to the optional one passed in by the user: Project.xcconfig, Debug.xcconfig, Release.xcconfig.

The Project.xcconfig file contained definitions of these settings: `PRODUCT_NAME`, `SUPPORTED_PLATFORMS`, `MACOSX_DEPLOYMENT_TARGET`, `DYLIB_INSTALL_NAME_BASE`, `OTHER_SWIFT_FLAGS`, `COMBINE_HIDPI_IMAGES`, and `USE_HEADERMAP`

In the reworked generator, all settings are emitted to the project file, getting rid of the extra config file. However, this turns out to be too simplistic. Because Xcode considers .xcconfig files to be at a lower precedence than the project settings, this means settings in the user-provided .xcconfig file can not override those at the project level.

Even though only a small set of build settings could be overridden (those listed above), this is a semantic change from the previous behavior, and needs to be fixed, even though it means going back to a more complex generated project. Moreover, this should be fixed so that the other, currently hardcoded settings, can also be overridden (e.g. `SWIFT_VERSION`).

@abertelrud
Copy link
Contributor Author

I am starting to work on this right away. I should note that the `--xcconfig-overrides` option isn't actually documented, but some people rely on it nonetheless. Therefore this should be fixed. And it should be done a bit better than the original so that any of the settings can be overridden.

@abertelrud
Copy link
Contributor Author

The rewrite of the Xcode project generator kept the automatically emitted build settings at the same levels as the original version, in order to minimize semantic differences.

This means that `SWIFT_VERSION` (for example) is set at the target level, which still makes it unoverridable by the custom `.xcconfig` file. So part of this is to move some of these settings down to the project level.

@abertelrud
Copy link
Contributor Author

So by "any of the build settings be overridden": there are some, such as the target name, that we would need something more than an `.xcconfig` file to override.

@abertelrud
Copy link
Contributor Author

The simplest way in which to fix this is to associate the .xcconfig contents with each target, letting them override the project build settings (but still leaving them overridable by the target settings).

@abertelrud
Copy link
Contributor Author

We can bring back the four separate .xcconfig files in a separate change if we want, though it should be noted that that in and of itself won't affect the layering and thus overridability of anything set at the target level.

@abertelrud
Copy link
Contributor Author

This was fixed in #719

@keith
Copy link
Collaborator

keith commented Oct 10, 2016

It looks like this is still an issue on master. On the swift-3.0 branch of this repo Reflejo/LambdaKit@cc78c3a you can run `make project` and then see that the generated project's SDKROOT is overridden by the project, and not by my custom xcconfig.

@abertelrud
Copy link
Contributor Author

Ok, thanks. I'll take a look.

@ddunbar
Copy link
Member

ddunbar commented Oct 10, 2016

I suspect this is because some of the settings which LambdaKit is trying to override are set explicitly at the target level.

This is one benefit of the old model which tried to put everything into an xcconfig, it prevents us from making this mistake because we guaranteed the override xcconfig will win.

@abertelrud
Copy link
Contributor Author

Yes, the direct reason why this fix doesn't help is that the just-added `SDKROOT` setting was added at the target level, not the project level, and thus not overridable either in the old or the current project generator.

Regarding the `.xcconfig` files: unfortunately we did not in fact guarantee that the `.xcconfig` would win. In practice many of the build settings were set at the target level (`SWIFT_VERSION` for example), and thus not overridable. I pulled some of them down to the project level, but others depend on the kind of target, and are therefore tricky to avoid putting at the target level.

Regarding the old vs current way of using `.xcconfig` files: the goal was to reduce the noise by having just the user's provided `.xcconfig` file and not a total of four in the case of a custom .xcconfig or three in the absence of one. It's very possible that it would be better to go back to generating `.xcconfig` files, but it's important to note that that alone wouldn't actually have affected this unless we can find a way to get the targets to not set build settings. So we may very well do that, but still need to move `SDKROOT` down to the project level.

@abertelrud
Copy link
Contributor Author

I've filed https://bugs.swift.org/browse/SR-2904 since it's a separate issue (`SDKROOT` needs to be at the project level in order to be overridable).

@abertelrud
Copy link
Contributor Author

Since https://bugs.swift.org/browse/SR-2904 was both a separate issue from this one, and since it has already been resolved, I'm moving this back to resolved since I don't believe there is anything more to fix here. If I am wrong, please set back to Open. Thanks.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 4, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants