Skip to content

[vcpkg/meson] fix some details#15756

Merged
vicroms merged 14 commits intomicrosoft:masterfrom
Neumann-A:some_meson_details
Mar 31, 2021
Merged

[vcpkg/meson] fix some details#15756
vicroms merged 14 commits intomicrosoft:masterfrom
Neumann-A:some_meson_details

Conversation

@Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Jan 19, 2021

  • Add ADD_BIN_TO_PATH to vcpkg_install_meson
  • Make x86 on x64_x86 a native build (unless uwp)
  • Make native compiler unuseable in cross builds.
  • Bonus: Fix single configuration builds.

changes extracted from #13100

deactivate native compiler in cross builds
make x86 on x86_x64 a native instead of a cross build
(as long as we are not building for UWP)
@Neumann-A Neumann-A marked this pull request as draft January 19, 2021 20:09
@JackBoosY JackBoosY self-assigned this Jan 20, 2021
@JackBoosY JackBoosY added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Jan 20, 2021
remove unnecessary comments
@Neumann-A Neumann-A marked this pull request as ready for review January 20, 2021 16:35
@JackBoosY JackBoosY added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. requires:discussion labels Jan 21, 2021
@JackBoosY
Copy link
Contributor

@ras0219 @ras0219-msft Ping for reivew again.

@venabled
Copy link
Contributor

@Neumann-A playing around with this PR as I'm trying to get the glib port working on Android.

It looks like some of the options in the internal meson cross file are getting generated with some spaces inbetween some of the dashes, e.g.:

c_args = ['-g', '-DANDROID', '-fdata-sections', '-ffunction-sections', '-funwind-tables', '-fstack-protector-strong', '-no-canonical-prefixes', '-D_FORTIFY_SOURCE=2', '-march=armv7-a', '-mthumb', '-Wformat', '-Werror=format-security', '-fPIC', '-', '-target=armv7-none-linux-androideabi21', '-', '-gcc-toolchain=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64', '-', '-sysroot=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/sysroot', '-O0', '-fno-limit-debug-info', '-I"/data/vcpkg/installed/arm-android/include"']

Leading to this error during configure:

../src/glib-2-4069d62e51.clean/meson.build:1:0: ERROR: Unable to detect linker for compiler "/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -Wl,--version -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -march=armv7-a -mthumb -Wformat -Werror=format-security -fPIC - -target=armv7-none-linux-androideabi21 - -gcc-toolchain=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64 - -sysroot=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/sysroot -O0 -fno-limit-debug-info -I"/data/vcpkg/installed/arm-android/include""
stdout: 
stderr: clang: error: unknown argument '-target=armv7-none-linux-androideabi21'; did you mean '--target=armv7-none-linux-androideabi21'?
clang: error: unknown argument '-gcc-toolchain=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64'; did you mean '--gcc-toolchain=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64'?
clang: error: unknown argument '-sysroot=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/sysroot'; did you mean '--sysroot=/home/venabled/android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/sysroot'?
clang: error: -E or -x required when input is from standard input
clang: error: -E or -x required when input is from standard input
clang: error: -E or -x required when input is from standard input

@JackBoosY
Copy link
Contributor

Ping @Neumann-A @ras0219 for review again.

@JackBoosY JackBoosY removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 26, 2021
# Conflicts:
#	scripts/cmake/vcpkg_install_meson.cmake
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Looks good so far; still needs merge from master due to @post-due's fixes

# Conflicts:
#	scripts/cmake/vcpkg_configure_meson.cmake
@Neumann-A
Copy link
Contributor Author

@ras0219 Sry, I think you have to re-review. I changed quite a bit in the merge since I was sick of the amount of duplication in the flags code. There is still quite a bit of duplication and I think that maybe the native and cross generators could be merged a bit more in a future PR.
The main problem is that the meson docs are very insufficient in what belongs where and most of the stuff is just me try&erroring it.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Feb 18, 2021
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Feb 19, 2021
@JackBoosY
Copy link
Contributor

Due to my lack of meson knowledge, I prefer @ras0219 @ras0219-msft to review this PR again.

# Conflicts:
#	scripts/cmake/vcpkg_configure_meson.cmake
@JackBoosY
Copy link
Contributor

Ping @ras0219 @ras0219-msft for review this PR again.

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

This generally LGTM. I'll wait to merge until after you've replied to my comments.

list(TRANSFORM FLAG_LIST APPEND "'")
list(TRANSFORM FLAG_LIST PREPEND "'")
list(JOIN FLAG_LIST ", " ${_out_var})
string(REPLACE "'', " "" ${_out_var} "${${_out_var}}") # remove empty elements if any
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't remove a single empty element.

It might be easier to construct this list using string(APPEND) and a foreach loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This won't remove a single empty element.

It will remove empty strings "". It should be noted here that the elements are already whitespace striped because otherwise meson will pass them wrongly to the tool invocation.

It might be easier to construct this list using string(APPEND) and a foreach loop.

Yeah I also thought about that but did not want to deal with strange corner cases if there are elements containing ;. So I thought list(TRANSFORM) is a saver approach

string(APPEND CROSS "cpu = '${BUILD_CPU}'\n")

if(NOT BUILD_CPU_FAM STREQUAL HOST_CPU_FAM OR VCPKG_TARGET_IS_ANDROID OR VCPKG_TARGET_IS_IOS)
if(NOT BUILD_CPU_FAM MATCHES "${HOST_CPU_FAM}" OR VCPKG_TARGET_IS_ANDROID OR VCPKG_TARGET_IS_IOS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the change to MATCHES? It seems like STREQUAL would be better (don't want arm matching arm64)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the other way around.
BUILD = arm64 | x64_x86
HOST = arm | x86
arm64 matches arm but not the other way around
the same for x64_x86 which matches x86 but not the other way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also if VCPKG_CROSSCOMPILING is a thing now I could use it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case we should just use TARGET_TRIPLET STREQUAL HOST_TRIPLET

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Copy link
Member

@vicroms vicroms left a comment

Choose a reason for hiding this comment

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

LGTM

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Mar 31, 2021
@vicroms vicroms merged commit 8646c65 into microsoft:master Mar 31, 2021
@Neumann-A Neumann-A deleted the some_meson_details branch March 31, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants