Skip to content

[x264] enable cross-compile settings propagation for arm64-linux#23532

Closed
sylvain-prevost wants to merge 13 commits intomicrosoft:masterfrom
sylvain-prevost:x264_arm64_linux_cross_compile
Closed

[x264] enable cross-compile settings propagation for arm64-linux#23532
sylvain-prevost wants to merge 13 commits intomicrosoft:masterfrom
sylvain-prevost:x264_arm64_linux_cross_compile

Conversation

@sylvain-prevost
Copy link
Contributor

@sylvain-prevost sylvain-prevost commented Mar 13, 2022

Describe the pull request

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

@JackBoosY JackBoosY added the category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. label Mar 14, 2022
@JackBoosY
Copy link
Contributor

Depends on #23525.

@JackBoosY JackBoosY added depends:different-pr This PR or Issue depends on a PR which has been filed and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Mar 14, 2022
@JackBoosY
Copy link
Contributor

/azp run

@JackBoosY
Copy link
Contributor

Close and reopen this PR to re-trigger the ci test.

@JackBoosY JackBoosY closed this Mar 14, 2022
@JackBoosY JackBoosY reopened this Mar 14, 2022
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sylvain-prevost sylvain-prevost marked this pull request as ready for review March 14, 2022 19:15
JackBoosY
JackBoosY previously approved these changes Mar 15, 2022
@JackBoosY
Copy link
Contributor

cc @dg0yt for review this.

Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

Disclaimer: I don't use this port.

@sylvain-prevost
Copy link
Contributor Author

Currently fails on PR formatting due to 'version-string'.
If 'version' is used instead of 'version-string', it passes the checks but then it does not match the dot-version definition.
Any clue/guidance ?

@dg0yt
Copy link
Contributor

dg0yt commented Mar 16, 2022

Currently fails on PR formatting due to 'version-string'.
If 'version' is used instead of 'version-string', it passes the checks but then it does not match the dot-version definition.
Any clue/guidance ?

Created #23585.

@JackBoosY
Copy link
Contributor

JackBoosY commented Mar 17, 2022

Can you please rerun the command ./vcpkg format-manifest ports/x264/vcpkg.json then check the changes?

@sylvain-prevost
Copy link
Contributor Author

Hi @JackBoosY , sorry for the delay, I was away for a while.
Running the mentioned command does not generate any local change.

λ vcpkg format-manifest ports/x264/vcpkg.json
Succeeded in formatting the manifest files.

@sylvain-prevost
Copy link
Contributor Author

issue #23766 is not anymore present, however the version-string issue is still there :(

When I created the PR, I used the command

vcpkg x-add-version x264

and the output was:

Use the version scheme "version" instead of "version-string" in port "x264".
Use --skip-version-format-check to disable this check.

Thereafter I used the suggested flag to be able to generate the version & baseline files.
Since then there is this PR Suggestion CI build error that is there.

If I replace 'version-string' by 'version' then it passes the check, but that would be invalid since the value of the version is: ''164-5db6aa6cab1b146" and this is not a dot-version.

@JackBoosY
Copy link
Contributor

Oh I totally lost track with this PR, sorry.

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 12, 2022
@strega-nil-ms
Copy link
Contributor

I don't know enough about this to say - I'm marking this as requires:discussion so we can talk with the team on thursday. Thanks for the PR!

@strega-nil-ms
Copy link
Contributor

This seems wrong; can you please post the CC= line from buildtrees/x264/arm64-linux-dbg/config.mak and the VCPKG_DETECTED_CMAKE_C_COMPILER line from buildtrees/x264/cmake-vars-arm64-linux-dbg.cmake.log, before the change that you've made? That should let us isolate where the failure is.

@LilyWangLL LilyWangLL removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 18, 2022
@sylvain-prevost
Copy link
Contributor Author

Hi @LilyWangLL ,

  1. To be able to generate the config.mak file, the first thing is to enable option --disable-asm for linux & arm64.

  2. Once done, here are the requested lines:

from buildtrees/x264/[target]-rel/config.mak:
CC=gcc

from buildtrees/x264/cmake-vars-[target]-rel.cmake.log:
set(VCPKG_DETECTED_CMAKE_C_COMPILER "/root/builds/jetson-nano-cross-compile-assets/toolchains/toolchain-gcc-cross-linaro-7.5.0/bin/aarch64-linux-gnu-gcc")

Just in case, to reduce ping-pong, I'm adding in attachment the corresponding files.

config.mak.log

cmake-vars-jetson-rel.cmake.log

@sylvain-prevost
Copy link
Contributor Author

And the triplet file I'm using for my target (jetson nano), cross-compiled on WSL2 Ubuntu 20.04:

set (VCPKG_TARGET_ARCHITECTURE arm64)
set (VCPKG_CRT_LINKAGE dynamic)
set (VCPKG_LIBRARY_LINKAGE static)
set (VCPKG_CMAKE_SYSTEM_NAME Linux)
set (VCPKG_CMAKE_SYSTEM_VERSION Tegra)

set (VCPKG_MAKE_BUILD_TRIPLET --host=aarch64-linux-gnu)

if (DEFINED ENV{GSTREAMER_FOR_DEEPSTREAM})
# gstreamer is required to be built as shared library (for nVidia deepstream), along with few other libraries.
if (${PORT} MATCHES "gstreamer|pango|cairo|glib|pcre|libffi")
set (VCPKG_LIBRARY_LINKAGE dynamic)
endif()
endif()

set (VCPKG_CHAINLOAD_TOOLCHAIN_FILE ${CMAKE_CURRENT_LIST_DIR}/../toolchains/aarch64_linux_jetson.cmake)

@sylvain-prevost
Copy link
Contributor Author

@strega-nil-ms , requested info is available.

@JackBoosY JackBoosY added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels May 5, 2022
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

As in #22331 , ideally we would be setting all the right tools to the right bits (via CC, AR, etc.) rather than trying to coax x264's build scripts into detecting the right bits themselves.

Based on https://github.com/mirror/x264/blob/master/configure#L557 it looks like you need:

  • CC
  • STRINGS
  • AR
  • RANLIB
  • PKGCONFIG (??? it seems like this runs on the host so we don't understand why it is cross-prefix'd)
  • STRIP (but we don't enable stripping so this one may not matter)


list(APPEND OPTIONS --enable-pic)

if (VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64")
Copy link
Member

Choose a reason for hiding this comment

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

This condition is probably wrong: Why is cross compiling for arm64 different than any other cross compiling?

@BillyONeal BillyONeal added requires:author-response and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels May 5, 2022
@BillyONeal
Copy link
Member

Based on https://github.com/mirror/x264/blob/master/configure#L557 it looks like you need:

To clarify, you need to get these values from vcpkg_cmake_get_vars and set those environment variables as consumed by that configure script. Start with CC, AR, and RANLIB since with those it will probably just work.

@JackBoosY
Copy link
Contributor

Convert this PR to draft since there is no progress.

@JackBoosY JackBoosY marked this pull request as draft July 15, 2022 05:49
@JackBoosY
Copy link
Contributor

Closing this PR since it seems that no progress is being made. Please reopen if work is still being done.

@JackBoosY JackBoosY closed this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[x264] arm64 cross-compile settings are ignored.

6 participants