Skip to content

build: use add_llvm_tool_symlink #6048

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
merged 1 commit into from
Dec 3, 2016
Merged

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Dec 3, 2016

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

Rather than invoke the create_symlink command manually which wont work on
Windows, use the add_llvm_tool_symlink.

Rather than invoke the create_symlink command manually which wont work on
Windows, use the add_llvm_tool_symlink.
@compnerd
Copy link
Member Author

compnerd commented Dec 3, 2016

@swift-ci please test

@compnerd
Copy link
Member Author

compnerd commented Dec 3, 2016

CC: @llvm-beanz

@hughbe
Copy link
Contributor

hughbe commented Dec 3, 2016

LGTM, will cause merge conflicts with my Windows port (I won't be available to contribute until Wednesday)... up to you in deciding which order to merge

@hughbe
Copy link
Contributor

hughbe commented Dec 3, 2016

Actually... just realised this is slightly broken - Windows files end in "exe" but this doesn't account for that?

@compnerd
Copy link
Member Author

compnerd commented Dec 3, 2016

That existed prior to this and the add_llvm_tool_symlink uses the ${CMAKE_EXECUTABLE_SUFFIX} which should add the .exe for you :-).

@compnerd compnerd merged commit 4ce4271 into swiftlang:master Dec 3, 2016
@compnerd compnerd deleted the tool-symlinks branch December 3, 2016 21:27
@hughbe
Copy link
Contributor

hughbe commented Dec 3, 2016

Awesome good to know - what about other symlinks for python files and the like? Does LLVM have an equivalent

@llvm-beanz
Copy link
Contributor

@compnerd I would make add_swift_tool_symlink basically the same as add_clang_symlink is in Clang. Since you're using it the same way every time.

@hughbe LLVM doesn't yet have a more-generic equivalent, but I need to make one for LLDB, so it will.

@compnerd
Copy link
Member Author

compnerd commented Dec 3, 2016

@llvm-beanz sure, will do a follow up commit soon.

@compnerd
Copy link
Member Author

compnerd commented Dec 3, 2016

@llvm-beanz created PR #6053 for the suggestion.

@hughbe we should be able to simplify the changes that you had for Windows support now as we dont need to do all the craziness for the symlinking ourselves.

@hughbe
Copy link
Contributor

hughbe commented Dec 3, 2016

@compnerd some of it, yes: symlinking clang header directories and swift-api-dump.py will need to be changed when @llvm-beanz adds the macro he suggested

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.

5 participants