Skip to content

[openvr] Added Linux support and updated to v1.10.30#10629

Merged
dan-shaw merged 4 commits intomicrosoft:masterfrom
makinori:master
Apr 3, 2020
Merged

[openvr] Added Linux support and updated to v1.10.30#10629
dan-shaw merged 4 commits intomicrosoft:masterfrom
makinori:master

Conversation

@makinori
Copy link
Copy Markdown
Contributor

ValveSoftware/openvr contains linux64 and linux32 but wasn't being used in the Vcpkg port file so I added them.

osx32 (no 64 bit and latest version Catalina is 64 bit only) exists too but hasn't been updated in 4 years. I think VR support on Mac is basically non existent but I know Valve is focusing on Linux a lot.

I updated the version as well.

@msftclas
Copy link
Copy Markdown

msftclas commented Mar 31, 2020

CLA assistant check
All CLA requirements met.

Comment thread ports/openvr/portfile.cmake
Comment thread ports/openvr/portfile.cmake Outdated
Comment thread ports/openvr/portfile.cmake Outdated
Comment thread ports/openvr/portfile.cmake
Comment thread ports/openvr/CONTROL
@NancyLi1013
Copy link
Copy Markdown
Contributor

Hi @MAKITSUNE
Thanks for this PR.
I noticed this port has passed on Linux platform. Could you please remove openvr:x64-linux=fail from ci.baseline.txt?

@makinori
Copy link
Copy Markdown
Contributor Author

makinori commented Apr 1, 2020

Thank you so much @NancyLi1013 for replying so quickly.
I made the changes you suggested.

I didn't replace the file(COPY ... calls for /lib and /bin with file(INSTALL ... because the file permissions get reset. Specifically they're not executable (+x) anymore.

Also, VCPKG_CMAKE_SYSTEM_NAME STREQUAL "" for detecting Windows doesn't work anymore. Though it's stated in the documentation here:
https://github.com/microsoft/vcpkg/blob/master/docs/users/triplets.md#vcpkg_cmake_system_name
Probably best to add that it's deprecated. I could open a pr?

@NancyLi1013
Copy link
Copy Markdown
Contributor

/azp run

@NancyLi1013
Copy link
Copy Markdown
Contributor

@MAKITSUNE
thanks for your update and reminder.
Currently, we suggest to use VCPKG_TARGET_IS_WINDOWS, VCPKG_TARGET_IS_LINUX , VCPKG_TARGET_IS_OSX, VCPKG_TARGET_IS_UWP etc to judge the platforms instead of VCPKG_CMAKE_SYSTEM_NAME ,
As for the usage of `VCPKG_CMAKE_SYSTEM_NAME for detecting Windows, it is the correct way to use like this:

NOT VCPKG_CMAKE_SYSTEM_NAME

Of course, we need to update the deprecated functions to document.
You don't need to open a new PR to update this.

@NancyLi1013 NancyLi1013 added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed waiting for response labels Apr 2, 2020
@dan-shaw dan-shaw merged commit b065d5f into microsoft:master Apr 3, 2020
@dan-shaw
Copy link
Copy Markdown
Contributor

dan-shaw commented Apr 3, 2020

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants