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

[openxr-loader] Fix package name and enable arm #23928

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

bwrsandman
Copy link
Contributor

@bwrsandman bwrsandman commented Apr 1, 2022

Describe the pull request

  • What does your PR fix?

Makes it easier to use openxr-loader and enabled arm

Previously it prints this with find_package(openxr-loader which is wrong and should have OpenXR.

[cmake] The package openxr-loader provides CMake targets:
[cmake] 
[cmake]     find_package(openxr-loader CONFIG REQUIRED)
[cmake]     target_link_libraries(main PRIVATE OpenXR::headers OpenXR::openxr_loader)

Now it prints:

[cmake] The package openxr-loader provides CMake targets:
[cmake] 
[cmake]     find_package(OpenXR CONFIG REQUIRED)
[cmake]     target_link_libraries(main PRIVATE OpenXR::headers OpenXR::openxr_loader)
  • Which triplets are supported/not supported? Have you updated the CI baseline?

I have removed the arm restriction after testing cross compiling to android and host compiling on linux arm

Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@bwrsandman bwrsandman force-pushed the openxr-loader-usage branch from 75b96a7 to 3452955 Compare April 1, 2022 22:36
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/openxr-loader/vcpkg.json

Valid values for the license field can be found in the documentation

@bwrsandman bwrsandman force-pushed the openxr-loader-usage branch from 3452955 to 8ed338d Compare April 1, 2022 22:41
@bwrsandman bwrsandman marked this pull request as ready for review April 1, 2022 22:41
@bwrsandman bwrsandman force-pushed the openxr-loader-usage branch from 8ed338d to 262beea Compare April 1, 2022 22:42
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 6fa767aca9474519b737f56fcf0a519023c8ee56 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/o-/openxr-loader.json b/versions/o-/openxr-loader.json
index f56d149..e576bd8 100644
--- a/versions/o-/openxr-loader.json
+++ b/versions/o-/openxr-loader.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "29282b8e08976099dea0e7b8aab03077c0047afd",
+      "git-tree": "4f55db29b61060220d599e61470a0db5e53a595c",
       "version": "1.0.22",
       "port-version": 2
     },

@bwrsandman bwrsandman force-pushed the openxr-loader-usage branch from 262beea to 2c28e98 Compare April 1, 2022 22:45
@JonLiu1993 JonLiu1993 self-assigned this Apr 2, 2022
@JonLiu1993 JonLiu1993 added the category:port-bug The issue is with a library, which is something the port should already support label Apr 2, 2022
@bwrsandman bwrsandman force-pushed the openxr-loader-usage branch from 2c28e98 to c13905a Compare April 2, 2022 03:32
@JonLiu1993
Copy link
Member

@bwrsandman ,Thanks for yur pr, I tested the usage on my local machine but failed, This is error log:

1> Command line: "C:\WINDOWS\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\ENTERPRISE\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Visual Studio 16 2019" -A Win32  -DCMAKE_CONFIGURATION_TYPES:STRING="Debug" -DCMAKE_INSTALL_PREFIX:PATH="C:\Users\vzhli17\source\repos\test-lib\out\install\x86-Debug" -DCMAKE_TOOLCHAIN_FILE=F:/Feature-test/openxr/vcpkg/scripts/buildsystems/vcpkg.cmake "C:\Users\vzhli17\source\repos\test-lib" 2>&1"
1> Working directory: C:\Users\vzhli17\source\repos\test-lib\out\build\x86-Debug
1> [CMake] -- Selecting Windows SDK version 10.0.19041.0 to target Windows 10.0.19042.
1> [CMake] -- The C compiler identification is MSVC 19.29.30137.0
1> [CMake] -- The CXX compiler identification is MSVC 19.29.30137.0
1> [CMake] -- Detecting C compiler ABI info
1> [CMake] -- Detecting C compiler ABI info - done
1> [CMake] -- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x86/cl.exe - skipped
1> [CMake] -- Detecting C compile features
1> [CMake] -- Detecting C compile features - done
1> [CMake] -- Detecting CXX compiler ABI info
1> [CMake] -- Detecting CXX compiler ABI info - done
1> [CMake] -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x86/cl.exe - skipped
1> [CMake] -- Detecting CXX compile features
1> [CMake] -- Detecting CXX compile features - done
1> [CMake] CMake Error at F:/Feature-test/openxr/vcpkg/scripts/buildsystems/vcpkg.cmake:816 (_find_package):
1> [CMake]   Could not find a package configuration file provided by "openxr_loader"
1> [CMake]   with any of the following names:
1> [CMake] 
1> [CMake]     openxr_loaderConfig.cmake
1> [CMake]     openxr_loader-config.cmake
1> [CMake] 
1> [CMake]   Add the installation prefix of "openxr_loader" to CMAKE_PREFIX_PATH or set
1> [CMake]   "openxr_loader_DIR" to a directory containing one of the above files.  If
1> [CMake]   "openxr_loader" provides a separate development package or SDK, be sure it
1> [CMake]   has been installed.

CMakeLists:

# CMakeList.txt : CMake project for test-lib, include source and define
# project specific logic here.
#
cmake_minimum_required (VERSION 3.8)

project(test)
# Add source to this project's executable.
add_executable (test-lib "test-lib.cpp" "test-lib.h")

# TODO: Add tests and install targets if needed.
find_package(openxr_loader CONFIG REQUIRED)

target_link_libraries(test-lib PRIVATE OpenXR::headers OpenXR::openxr_loader)

@bwrsandman
Copy link
Contributor Author

I updated it after testing. The find package should be OpenXR. The PR should be updated

@bwrsandman
Copy link
Contributor Author

Also, I don't see why arm is disallowed since Android ARM is one of the biggest usecases for OpenXR (oculus Quest)

@dg0yt
Copy link
Contributor

dg0yt commented Apr 4, 2022

The port installs share/openxr-loader/OpenXRConfig.cmake. The filename indicates that the proper package name is OpenXR (case-sensitive!) however the location is not right. It must match the package name (case-insensitive). The easiest fix is adding PACKAGE_NAME OpenXR to vcpkg_cmake_config_fixup.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/openxr-loader/vcpkg.json b/ports/openxr-loader/vcpkg.json
index 9b867d9..6c65bc3 100644
--- a/ports/openxr-loader/vcpkg.json
+++ b/ports/openxr-loader/vcpkg.json
@@ -5,7 +5,7 @@
   "description": "A royalty-free, open standard that provides high-performance access to Augmented Reality (AR) and Virtual Reality (VR)—collectively known as XR—platforms and devices",
   "homepage": "https://github.com/KhronosGroup/OpenXR-SDK",
   "license": "Apache-2.0",
-  "supports": "android|!(arm | uwp)",
+  "supports": "android | !(arm | uwp)",
   "dependencies": [
     "jsoncpp",
     {
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout e79aaaaf3c1b157d29388951b5aa2672defc78b8 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/o-/openxr-loader.json b/versions/o-/openxr-loader.json
index 87bca6b..8e59894 100644
--- a/versions/o-/openxr-loader.json
+++ b/versions/o-/openxr-loader.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "9492018923238cff2976b1399f171817f80968a3",
+      "git-tree": "8d0ca56cad6ab2877b6d43ffe30dda20519d9091",
       "version": "1.0.22",
       "port-version": 2
     },

@bwrsandman bwrsandman force-pushed the openxr-loader-usage branch from 4112250 to 4af53ef Compare April 5, 2022 22:11
@bwrsandman bwrsandman changed the title [openxr-loader] add usage that has correct spelling for find package [openxr-loader] Fix package name and enable arm Apr 5, 2022
@bwrsandman
Copy link
Contributor Author

@dg0yt I changed the package name with vcpkg_cmake_config_fixup as you recommended

I also went and enabled arm after testing cross compiling to android armeabi-v7a and testing a host build on ARM linux (raspberry pi).
Still haven't tested arm windows.

One thing of note is that on linux it requires host to have Xlib.h like sdl2[xlib]

@bwrsandman
Copy link
Contributor Author

I am also pretty sure OpenXR supports uwp for hololens. I don't have the means to test it though

@JonLiu1993
Copy link
Member

I tested the usage successfully:

1> CMake generation started for configuration: 'x64-Debug'.
1> Command line: "C:\WINDOWS\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && 
...
...
1> [CMake] -- Selecting Windows SDK version 10.0.19041.0 to target Windows 10.0.19042.
1> [CMake] -- The C compiler identification is MSVC 19.29.30137.0
1> [CMake] -- The CXX compiler identification is MSVC 19.29.30137.0
1> [CMake] -- Detecting C compiler ABI info
1> [CMake] -- Detecting C compiler ABI info - done
1> [CMake] -- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting C compile features
1> [CMake] -- Detecting C compile features - done
1> [CMake] -- Detecting CXX compiler ABI info
1> [CMake] -- Detecting CXX compiler ABI info - done
1> [CMake] -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting CXX compile features
1> [CMake] -- Detecting CXX compile features - done
1> [CMake] -- Looking for pthread.h
1> [CMake] -- Looking for pthread.h - not found
1> [CMake] -- Found Threads: TRUE  
1> [CMake] -- Configuring done
1> [CMake] -- Generating done
1> [CMake] -- Build files have been written to: C:/Users/test/source/repos/opex/out/build/x64-Debug
1> Extracted CMake variables.
1> Extracted source files and headers.
1> Extracted code model.
1> Extracted toolchain configurations.
1> Extracted includes paths.
1> CMake generation finished.

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Apr 7, 2022
@strega-nil-ms
Copy link
Contributor

Thanks!

@strega-nil-ms strega-nil-ms merged commit 2f45391 into microsoft:master Apr 7, 2022
@bwrsandman bwrsandman deleted the openxr-loader-usage branch April 7, 2022 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants