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

[Daxa] new port #29800

Merged
merged 8 commits into from
Mar 10, 2023
Merged

[Daxa] new port #29800

merged 8 commits into from
Mar 10, 2023

Conversation

GabeRundlett
Copy link
Contributor

@GabeRundlett GabeRundlett commented Feb 23, 2023

  • Changes comply with the maintainer guide
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@MonicaLiu0311 MonicaLiu0311 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Feb 23, 2023
ports/daxa/vcpkg.json Outdated Show resolved Hide resolved
ports/daxa/vcpkg.json Show resolved Hide resolved
@MonicaLiu0311
Copy link
Contributor

  • Install daxa error.
PS E:\daxa> ./vcpkg install daxa[*]:x86-windows
Computing installation plan...
...
Building vulkan[core]:x86-windows...
-- Querying VULKAN_SDK Enviroment variable
CMake Error at ports/vulkan/portfile.cmake:10 (message):
  Could not find Vulkan SDK.  Before continuing, please download and install
  Vulkan from:

      https://vulkan.lunarg.com/sdk/home

  If you have already downloaded it, make sure the VULKAN_SDK environment
  variable is set to vulkan's installation root.
Call Stack (most recent call first):
  scripts/ports.cmake:147 (include)
PS E:\daxa> ./vcpkg install daxa[*]:x86-windows
Computing installation plan...
...
-- Configuring x86-windows
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:112 (message):
    Command failed: "C:/Program Files/Microsoft Visual Studio/2022/Enterprise/Common7/IDE/CommonExtensions/Microsoft/CMake/Ninja/ninja.exe" -v
    Working Directory: E:/daxa/buildtrees/daxa/x86-windows-rel/vcpkg-parallel-configure
    Error code: 1
    See logs for more information:
      E:\daxa\buildtrees\daxa\config-x86-windows-dbg-CMakeCache.txt.log
      E:\daxa\buildtrees\daxa\config-x86-windows-rel-CMakeCache.txt.log
      E:\daxa\buildtrees\daxa\config-x86-windows-out.log

Call Stack (most recent call first):
  scripts/cmake/vcpkg_configure_cmake.cmake:310 (vcpkg_execute_required_process)
  ports/daxa/portfile.cmake:29 (vcpkg_configure_cmake)
  scripts/ports.cmake:147 (include)

E:\daxa\buildtrees\daxa\config-x86-windows-out.log:

-- Detecting CXX compile features - done
CMake Error at E:/daxa/downloads/tools/cmake-3.25.1-windows/cmake-3.25.1-windows-i386/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Vulkan (missing: Vulkan_LIBRARY) (found version "1.3.239")
Call Stack (most recent call first):
  E:/daxa/downloads/tools/cmake-3.25.1-windows/cmake-3.25.1-windows-i386/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  E:/daxa/downloads/tools/cmake-3.25.1-windows/cmake-3.25.1-windows-i386/share/cmake-3.25/Modules/FindVulkan.cmake:597 (find_package_handle_standard_args)
  E:/daxa/scripts/buildsystems/vcpkg.cmake:846 (_find_package)
  CMakeLists.txt:81 (find_package)

config-x86-windows-out.log
config-x86-windows-rel-CMakeCache.txt.log
config-x86-windows-dbg-CMakeCache.txt.log

@GabeRundlett
Copy link
Contributor Author

GabeRundlett commented Feb 23, 2023

I've reproduced the issue with it not building on x86-windows, the issue you're having is actually because you didn't install the 32bit Vulkan SDK lib
image

Though, I will say, we don't plan on supporting anything other than x64-windows[-static][-md], and x64-linux at the moment. x86-windows will fail to build for other reasons. I could fix those, but I'd have to bump the version number

@GabeRundlett
Copy link
Contributor Author

If we don't want to support x86, osx, or arm, is there something I need to do?

@MonicaLiu0311
Copy link
Contributor

If we don't want to support x86, osx, or arm, is there something I need to do?

Please add "supports" in vcpkg.json.
For example: faiss/vcpkg.json
Documentation: vcpkg-json.md

@MonicaLiu0311
Copy link
Contributor

##[error]Found the following errors:
    Error: While reading versions for port daxa from file: C:\a\1\s\versions\d-\daxa.json
       File declares version `1.0.0` with SHA: 74eeef09c9de60dccd6a73b145f953b18c6fa874
       But local port with the same version has a different SHA: 4b5f2e8019fab8391f6e46a58e4cb21b58f228ac
       Please update the port's version fields and then run:

           vcpkg x-add-version daxa
           git add versions
           git commit -m "Update version database"

       to add a new version.
To attempt to resolve all errors at once, run:
vcpkg x-add-version --all
##[error]PowerShell exited with code '1'.
Finishing: Validate version files

There is a CI error, please run this command to update git-tree: vcpkg x-add-version daxa ----overwrite-version

--overwrite-version
After the first modification, please run ./vcpkg x-add-version portName to generate the corresponding json; for subsequent modifications of the same version, please use ./vcpkg x-add-version portName --overwrite-version to update git-tree.

@GabeRundlett
Copy link
Contributor Author

thank you for the assistance, I think I got those things in now!

@GabeRundlett
Copy link
Contributor Author

Do I need to do more?

@MonicaLiu0311
Copy link
Contributor

All features are tested successfully in the following triplet:
x64-windows
x64-windows-static
x64-linux

@MonicaLiu0311
Copy link
Contributor

The usage test passed (header files found).

daxa provides CMake targets:

    # this is heuristically generated, and may not be correct
    find_package(daxa CONFIG REQUIRED)
    target_link_libraries(main PRIVATE daxa::daxa)

@MonicaLiu0311
Copy link
Contributor

Could you please fix some deprecated functions? grateful.

See portfile.cmake for details.

MonicaLiu0311
MonicaLiu0311 previously approved these changes Feb 27, 2023
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Feb 27, 2023
@JavierMatosD JavierMatosD added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Feb 27, 2023
JavierMatosD
JavierMatosD previously approved these changes Mar 2, 2023
"name": "vcpkg-cmake-config",
"host": true
},
"vulkan",
Copy link
Contributor

Choose a reason for hiding this comment

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

This port seems to have a dependency on x11 and Wayland guarded by CMAKE_SYSTEM_NAME https://github.com/Ipotrick/Daxa/blob/master/CMakeLists.txt#L163

x11 requires special consideration because, currently, it is assumed to be retrieved from the system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JavierMatosD I'm not exactly sure what needs to be done about this on my end. Could you clarify?

Copy link
Contributor

Choose a reason for hiding this comment

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

ports/daxa/portfile.cmake Outdated Show resolved Hide resolved
ports/daxa/vcpkg.json Show resolved Hide resolved
@JavierMatosD JavierMatosD added requires:author-response and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Mar 2, 2023
Co-authored-by: Javier Matos Denizac <[email protected]>
@GabeRundlett GabeRundlett dismissed stale reviews from JavierMatosD and MonicaLiu0311 via d746f50 March 4, 2023 01:38
@GabeRundlett
Copy link
Contributor Author

Am I meant to do something? I responded to a review and never got a response

@JavierMatosD JavierMatosD merged commit 35a6d4e into microsoft:master Mar 10, 2023
@MonicaLiu0311 MonicaLiu0311 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants