Skip to content

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