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

[liblsl] Fix installation #7945

Merged
merged 2 commits into from Aug 28, 2019
Merged

[liblsl] Fix installation #7945

merged 2 commits into from Aug 28, 2019

Conversation

ehsan-mohammadi
Copy link
Contributor

@ehsan-mohammadi ehsan-mohammadi commented Aug 28, 2019

In the previous PR (#7906), I changed add_excutable to add_library with a patch file to remove .exe files and avoid build failures. But this is a wrong way. So I fixed it in this PR.

@ehsan-mohammadi
Copy link
Contributor Author

@cbezault @cenit Can you please check this PR?

@ehsan-mohammadi ehsan-mohammadi marked this pull request as ready for review August 28, 2019 13:46
@cbezault
Copy link
Contributor

Yeah this looks right, though why only on WIN32? Is the port only buildable on a windows host? If it has to do with the target as opposed to the host (though right now I see that this port is only getting built on Windows) you should use VCPKG_TARGET_IS_WINDOWS.

@ehsan-mohammadi
Copy link
Contributor Author

ehsan-mohammadi commented Aug 28, 2019

@cbezault I thought liblsl buildable on Linux and OSx too. So, do you think it getting built only on Windows? And should I change if(WIN32) to if(VCPKG_TARGET_IS_WINDOWS)?

@Rastaban
Copy link
Contributor

Yes, change the line to if(VCPKG_TARGET_IS_WINDOWS)

liblsl may be build able on Linux and OSX, but it is currently marked as not supported in our CI system. I have attached the failure build log if you are curious. install-x64-linux-dbg-out.log

@ehsan-mohammadi
Copy link
Contributor Author

@Rastaban Thanks for helping me. I'll do it.

@Rastaban Rastaban merged commit 8a05dc0 into microsoft:master Aug 28, 2019
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