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

[anari] Added ANARI port. #33213

Merged
merged 35 commits into from
Aug 18, 2023
Merged

[anari] Added ANARI port. #33213

merged 35 commits into from
Aug 18, 2023

Conversation

acdemiralp
Copy link
Contributor

@acdemiralp acdemiralp commented Aug 16, 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.

@Adela0814 Adela0814 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 16, 2023
@Adela0814 Adela0814 changed the title Added ANARI port. [anari] Added ANARI port. Aug 16, 2023
Comment on lines 1 to 2
vcpkg_minimum_required(VERSION 2022-10-12) # For ${PORT} and ${VERSION}.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary, please remove this,

vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO KhronosGroup/ANARI-SDK
REF v${VERSION}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
REF v${VERSION}
REF "v${VERSION}"


vcpkg_install_copyright(
FILE_LIST "${SOURCE_PATH}/LICENSE"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a new line at the end of the file.

"host": true
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a new line at the end of the file.

@Adela0814
Copy link
Contributor

You should run vcpkg x-add-version anari to add the versions file.

@Adela0814
Copy link
Contributor

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". That way, I can be aware that you've responded since you can't modify the tags.

@Adela0814 Adela0814 marked this pull request as draft August 16, 2023 10:10
@acdemiralp
Copy link
Contributor Author

acdemiralp commented Aug 16, 2023

I am looking into the CI failures. I locally tested x86-windows and x64-windows and they seem to work.

Edit: The CI seems to fail on line find_package(Python3 REQUIRED COMPONENTS Interpreter) on Windows.

Edit: The only remaining error is:

C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\include\immintrin.h(15): fatal error C1189: #error:  This header is specific to X86, X64, ARM64, and ARM64EC targets

This occurs in ARM64 debug mode, release works fine. I am looking for a solution.

Edit: The error is due to MSVC or Embree. Disabling port on ARM64 Windows until it is fixed.

Also, is it OK for the default implementation library (anari::helide) to depend on an internally downloaded version of Embree? Look at https://github.com/KhronosGroup/ANARI-SDK/blob/main/libs/helide/external/embree/CMakeLists.txt to see where this happens. I believe this is an intentional decision by Khronos; to have one "safe" device that works anywhere and does not depend on anything. If not, it is possible to disable it via -DBUILD_HELIDE_DEVICE=OFF in vcpkg_cmake_configure.

@acdemiralp acdemiralp marked this pull request as ready for review August 16, 2023 23:09
@acdemiralp
Copy link
Contributor Author

I think its good to go. Checked the installation directories as a last step, everything looks clean.

@Adela0814
Copy link
Contributor

Edit: The error is due to MSVC or Embree. Disabling port on ARM64 Windows until it is fixed.

Please add the failed triplet to the ci. baseline instead of the supports field.

@Adela0814
Copy link
Contributor

Usage tested pass on x64-windows.
No feature needs to test.

@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Aug 18, 2023
@dan-shaw dan-shaw merged commit 6dce771 into microsoft:master Aug 18, 2023
15 checks passed
@BillyONeal
Copy link
Member

populate-prefix/src/anari_helide_embree-populate-stamp/download-anari_helide_embree-populate.cmake:170 (message):
  Each download failed!

    error: downloading 'https://github.com/embree/embree/archive/refs/tags/v3.13.5.zip' failed
          status_code: 55
          status_string: "Failed sending data to the peer"
          log:

Looks like this port is being a bad kitty and trying to recursively download inside the build.

Also it's a bit scary that this isn't using the embree port

@acdemiralp
Copy link
Contributor Author

acdemiralp commented Sep 6, 2023

populate-prefix/src/anari_helide_embree-populate-stamp/download-anari_helide_embree-populate.cmake:170 (message):
  Each download failed!

    error: downloading 'https://github.com/embree/embree/archive/refs/tags/v3.13.5.zip' failed
          status_code: 55
          status_string: "Failed sending data to the peer"
          log:

Looks like this port is being a bad kitty and trying to recursively download inside the build.

Also it's a bit scary that this isn't using the embree port

I am looking into this now. The fastest way out is to disable the Helide implementation with -DBUILD_HELIDE_DEVICE=OFF. The better way is to patch the cmake to use the vcpkg Embree. I'll probably do the latter and open an MR.

Edit: Currently, the only real solution is -DBUILD_HELIDE_DEVICE=OFF. Helide depends on the https://github.com/embree/embree/tree/master/common directory of Embree which is not packaged with the Embree3 port. Please see MR #33616 .

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.

None yet

4 participants