Skip to content

[CMake] Fix the Xcode generator #5190

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

Merged

Conversation

llvm-beanz
Copy link
Contributor

In PR 5182 I broke the Xcode generator in cases where the output path had ${CMAKE_CFG_INTDIR} in it. This broke because $(CONFIGURATION) is invalid in the name of a target.

Since all targets will have the same value for CMAKE_BINARY_DIR and CMAKE_CFG_INTDIR we can remove those from the target names and still have unique values.

This has the impact of shortening the generated target names so that the filename components are unlikely to hit OS limits using IDE generators where CMake generates Makefiles or scripts for custom commands. It also retains the debug ability that PR 5182 was attempting to add.

In PR 5182 I broke the Xcode generator in cases where the output path had ${CMAKE_CFG_INTDIR} in it. This broke because $(CONFIGURATION) is invalid in the name of a target.

Since all targets will have the same value for CMAKE_BINARY_DIR and CMAKE_CFG_INTDIR we can remove those from the target names and still have unique values.

This has the impact of shortening the generated target names so that the filename components are unlikely to hit OS limits using IDE generators where CMake generates Makefiles or scripts for custom commands. It also retains the debug ability that PR 5182 was attempting to add.
@llvm-beanz
Copy link
Contributor Author

@swift-ci please smoke test

@llvm-beanz llvm-beanz merged commit c3036c8 into swiftlang:master Oct 8, 2016
@jrose-apple
Copy link
Contributor

I'm not convinced. You're still turning a path into a path component. But thanks for unbreaking Xcode. :-)

@gottesmm
Copy link
Contributor

gottesmm commented Oct 9, 2016

LGTM

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.

4 participants