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

[CMake] Improve Xcode project generation with folders #15738

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

jrose-apple
Copy link
Contributor

  • Re-enable the use of folders with the USE_FOLDER setting. This got lost a while ago when we stopped including LLVM's top-level CMakeLists.txt.

  • Put a bunch of new targets into folders.

Should not affect the built product and definitely shouldn't affect anyone not building with Xcode (or MSVC, I guess).

- Re-enable the use of folders with the USE_FOLDER setting. This got
  lost a while ago when we stopped including LLVM's top-level
  CMakeLists.txt.

- Put a bunch of new targets into folders.

Should not affect the built product and definitely shouldn't affect
anyone not building with Xcode (or MSVC, I guess).
@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test

@gottesmm
Copy link
Member

gottesmm commented Apr 4, 2018

@jrose-apple is there a way that we can test this?

@jrose-apple
Copy link
Contributor Author

I don't really see one, no. I don't want to write a test poking at the generated Xcode's project structure, and simply asserting that the CMake targets have such a property is more of a step than we want to take at the moment. (That is, there are targets without a FOLDER and I don't want to track them down right now; these were just the biggest offenders.)

@gottesmm
Copy link
Member

gottesmm commented Apr 4, 2018

@jrose-apple that isn't what I was suggesting. What I was suggesting was a build system unit test that uses:

xcodebuild -scheme "schemeName" -showBuildSettings

for one of these and maybe grep to see if it has that property. That way we can be sure that this does not break in the future.

@gottesmm
Copy link
Member

gottesmm commented Apr 4, 2018

that being said, given that we do not run a bot with Xcode, perhaps such a test is not interesting (since nothing will prevent it from breaking). Your thoughts?

@jrose-apple
Copy link
Contributor Author

I'd be open to doing such a thing but I don't know of any supported/stable way to do it. This isn't a build setting; it's "just" the structure in the navigator.

@gottesmm
Copy link
Member

gottesmm commented Apr 4, 2018

I understand that. I was asking b/c you said that this used to work b/c we were importing more of LLVM's cmake (I guess). Whenever something like this breaks, I ask this question. That being said, I think listing/dumping/grepping the settings would work. That being said, I am not going to argue strongly for this.

Copy link
Contributor

@Rostepher Rostepher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with these CMake changes. 👍

@jrose-apple jrose-apple merged commit 20ae19e into apple:master Apr 5, 2018
@jrose-apple jrose-apple deleted the folderol branch April 5, 2018 01:23
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.

None yet

3 participants