Skip to content

Introduce and use SWIFT_HAVE_LIBEDIT #6459

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 22, 2016
Merged

Introduce and use SWIFT_HAVE_LIBEDIT #6459

merged 1 commit into from
Dec 22, 2016

Conversation

hughbe
Copy link
Contributor

@hughbe hughbe commented Dec 22, 2016

This is an optional dependency (i.e. we can build the compiler without it).

I went for the CheckIncludeFiles CMake option, as this is simpler than having to adding a find_package implementation.

Extracted from #5904

@compnerd
Copy link
Member

Does the repl work without it?

@compnerd
Copy link
Member

@swift-ci please test

@hughbe
Copy link
Contributor Author

hughbe commented Dec 22, 2016

The repl is automatically disabled for platforms without histedit.h

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 877900bc042b3ac85abbed3b33df093fce62a5cd
Test requested by - @compnerd

@compnerd
Copy link
Member

CC @jrose-apple (in case you have a strong opinion on this, I think this is reasonable).

@jrose-apple
Copy link
Contributor

@mxswd just did something like this…or maybe it was limited to xpc.h. Max?

@mxswd
Copy link
Contributor

mxswd commented Dec 22, 2016

Yeah I just added 81ae0fd
Which worked for me. Without this what error do you get on Windows?

@hughbe
Copy link
Contributor Author

hughbe commented Dec 22, 2016

Oh, this may have been superseeded! I extracted this from the main PR - will let you know if this still doesn't work.. Will close if not

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 877900bc042b3ac85abbed3b33df093fce62a5cd
Test requested by - @compnerd

@hughbe
Copy link
Contributor Author

hughbe commented Dec 22, 2016

Actually!! I remember now - this is still necessary.

This is because we add target_link_libraries(edit). edit.lib is NOT available by default on Windows - this PR fixes that! This caused linker errors.

@compnerd could you retrigger the tests - I fixed a typo causing the builds to fail

@jrose-apple
Copy link
Contributor

Still seems like we should be able to use the same variable in both cases.

@hughbe
Copy link
Contributor Author

hughbe commented Dec 22, 2016

Still seems like we should be able to use the same variable in both cases.

True, good idea. I've moved the variable to CMakeLists.txt. I've found that the order of CMakeLists.txt resolving is unpredictable - its best to move this kind of check (for an optional package) to the root file, where it will always be run before target_link_libraries(edit)

@jrose-apple
Copy link
Contributor

@swift-ci Please smoke test

@jrose-apple jrose-apple merged commit 53e0353 into swiftlang:master Dec 22, 2016
@hughbe hughbe deleted the libedit-windows branch December 23, 2016 10:26
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