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

[dav1d] should not depend on nasm on arm64 #39920

Closed
BertalanD opened this issue Jul 15, 2024 · 2 comments · Fixed by #39923
Closed

[dav1d] should not depend on nasm on arm64 #39920

BertalanD opened this issue Jul 15, 2024 · 2 comments · Fixed by #39923
Assignees
Labels
category:port-bug The issue is with a library, which is something the port should already support

Comments

@BertalanD
Copy link
Contributor

Is your feature request related to a problem? Please describe.

When building the dav1d port on arm64 (both AArch64 Linux and macOS), the build fails with the following if the nasm package is not installed:

CMake Error at scripts/cmake/vcpkg_find_acquire_program.cmake:166 (message):
  Could not find nasm.  Please install it via your package manager:

      sudo apt-get install nasm
Call Stack (most recent call first):
  buildtrees/versioning_/versions/dav1d/0349d1196602b2983b1171647b30b5a9379920c6/portfile.cmake:9 (vcpkg_find_acquire_program)
  scripts/ports.cmake:191 (include)

This dependency is used only in x86 builds of the port, otherwise it uses GAS or Clang's integrated assembler by invoking the C compiler binary.

Proposed solution

Similarly to ports like x264, the call to vcpkg_find_acquire_program(NASM) should be gated behind a check for x86:

vcpkg_list(SET EXTRA_ARGS)
set(nasm_archs x86 x64)
set(gaspp_archs arm arm64)
if(NOT "asm" IN_LIST FEATURES)
vcpkg_list(APPEND OPTIONS --disable-asm)
elseif(NOT "$ENV{AS}" STREQUAL "")
# Accept setting from triplet
elseif(VCPKG_TARGET_ARCHITECTURE IN_LIST nasm_archs)
vcpkg_find_acquire_program(NASM)
transform_path_no_space(NASM)
list(APPEND EXTRA_ARGS CONFIGURE_ENVIRONMENT_VARIABLES AS)
set(AS "${NASM}") # for CONFIGURE_ENVIRONMENT_VARIABLES
set(ENV{AS} "${NASM}") # for non-WIN32
elseif(VCPKG_TARGET_ARCHITECTURE IN_LIST gaspp_archs AND VCPKG_TARGET_IS_WINDOWS AND VCPKG_HOST_IS_WINDOWS)
vcpkg_find_acquire_program(GASPREPROCESSOR)
list(FILTER GASPREPROCESSOR INCLUDE REGEX gas-preprocessor)
file(INSTALL "${GASPREPROCESSOR}" DESTINATION "${SOURCE_PATH}/tools" RENAME "gas-preprocessor.pl")
endif()

dav1d similarly supports gaspp when targeting arm/arm64 Windows.

Describe alternatives you've considered

No response

Additional context

No response

@BertalanD BertalanD added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jul 15, 2024
@github-actions github-actions bot added category:question This issue is a question and removed category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist labels Jul 15, 2024
Copy link

From the log:

  Could not find nasm.  Please install it via your package manager:

      sudo apt-get install nasm

@BertalanD
Copy link
Contributor Author

I think this issue was auto-closed mistakenly.

Yes, I can read the error message, and have installed nasm to make it go away locally. But that doesn't change the fact that this dependency is unnecessary (and useless) on arm.

BertalanD added a commit to BertalanD/vcpkg that referenced this issue Jul 15, 2024
`nasm` is only required when targeting x86(_64).

Fixes microsoft#39920
@LilyWangLL LilyWangLL reopened this Jul 15, 2024
@LilyWangLL LilyWangLL self-assigned this Jul 15, 2024
@LilyWangLL LilyWangLL added category:port-bug The issue is with a library, which is something the port should already support and removed category:question This issue is a question labels Jul 15, 2024
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants