[vcpkg_configure_make] Misc fixes #31228
Conversation
dg0yt
changed the title
|
vcpkg/scripts/cmake/vcpkg_configure_make.cmake Lines 681 to 684 in 6d69bba Also needs fixing. |
What kind of fix do you expect? e.g. |
Maybe it should be corrected to I would like to reduce the |
|
Which toolchain is expected to match if (CMAKE_HOST_WIN32 AND VCPKG_DETECTED_CMAKE_C_COMPILER_ID MATCHES [[cl\.exe$]]) What I see in CI for x64-windows is |
|
Probably |
MonicaLiu0311
added
the
category:tool-update
|
@Neumann-A To implement the lib path flags change it would be ideal to simply have
For now I decided to not change that script, but just leave comments. I don't have access to MSVC or clang-cl outside vcpkg CI, so I can't check any logs (or paths with spaces) for these triplets unless CI fails. |
MonicaLiu0311
added
the
info:reviewed
|
I do understand the concerns about breaking old versions. |
Drop all port changes from this branch.
dg0yt
force-pushed
the
configure-make-rollup
branch
from
7b5e07d
to
7d3f3c9
Compare
dg0yt
force-pushed
the
configure-make-rollup
branch
from
aff9dcb
to
37108e2
Compare
|
Requesting a new review @BillyONeal @JavierMatosD @dan-shaw @vicroms.
|
| # https://www.gnu.org/software/libtool/manual/html_node/Link-mode.html | ||
| # -avoid-version is handled specially by libtool link mode, this flag is not forwarded to linker, | ||
| # and libtool tries to avoid versioning for shared libraries and no symbolic links are created. | ||
| if(VCPKG_TARGET_IS_ANDROID) | ||
| set(ENV{LDFLAGS} "-avoid-version $ENV{LDFLAGS}") | ||
| set(ENV{LDFLAGS_FOR_BUILD} "-avoid-version $ENV{LDFLAGS_FOR_BUILD}") | ||
| endif() |
There was a problem hiding this comment.
Removing -avoid-version is a key change to make NDK r26 work. Libtool doesn't need it any longer explicitly. And shared libs are already broken for Android. I will add another change which modifies the generated libtool scripts in the build directories, but I will put this behind a condition.
| if("libtool-link-pass-target" IN_LIST VCPKG_BUILD_MAKE_FIXUP) | ||
| # Pass --target to the linker, e.g. for Android | ||
| file(GLOB_RECURSE libtool_files "${working_directory}/libtool") | ||
| foreach(file IN LISTS libtool_files) | ||
| vcpkg_replace_string("${file}" [[-xtarget=*|]] [[-xtarget=*|--target=*|]]) | ||
| endforeach() | ||
| endif() |
There was a problem hiding this comment.
--target belongs into CC/CXX. libtool doesn't invoke the linker it invokes the linker through the compiler.
There was a problem hiding this comment.
What is your proposal? Any references?
The compiler which invokes the linker is clang here. --target already is in the LDFLAGS, and it is used this way during configure when libtool is not used. This change makes libtool` pass it through (as passed in). And this is needed to make clang select the right runtime artifacts for Android and the desired API level.
There was a problem hiding this comment.
# Before:
/bin/bash ../libtool --tag=CC --mode=link /android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -g -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mthumb -Wformat -Werror=format-security -fPIC -fno-limit-debug-info -fopenmp -L/vcpkg/installed/arm-neon-android-dynamic/debug/lib --target=armv7-none-linux-androideabi28 -Wl,--build-id=sha1 -Wl,--no-rosegment -Wl,--no-undefined-version -Wl,--fatal-warnings -Wl,--no-undefined -Qunused-arguments --sysroot=/android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/sysroot -version-info 1:4:0 -o libb2.la -rpath /vcpkg/installed/arm-neon-android-dynamic/debug/lib libb2_la-blake2s-ref.lo libb2_la-blake2b-ref.lo libb2_la-blake2sp.lo libb2_la-blake2bp.lo -latomic -lm
libtool: link: /android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -shared -fPIC -DPIC .libs/libb2_la-blake2s-ref.o .libs/libb2_la-blake2b-ref.o .libs/libb2_la-blake2sp.o .libs/libb2_la-blake2bp.o -L/vcpkg/installed/arm-neon-android-dynamic/debug/lib -latomic -lm -g -fstack-protector-strong -mthumb -fopenmp -Wl,--build-id=sha1 -Wl,--no-rosegment -Wl,--no-undefined-version -Wl,--fatal-warnings -Wl,--no-undefined --sysroot=/android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fopenmp -Wl,-soname -Wl,libb2.so -o .libs/libb2.so
ld.lld: error: cannot open crti.o: No such file or directory
ld.lld: error: cannot open crtbeginS.o: No such file or directory
ld.lld: error: unable to find library -lm
ld.lld: error: unable to find library -lgcc
ld.lld: error: unable to find library -lgcc_s
ld.lld: error: unable to find library -lpthread
ld.lld: error: unable to find library -lc
ld.lld: error: unable to find library -lgcc
ld.lld: error: unable to find library -lgcc_s
ld.lld: error: cannot open crtendS.o: No such file or directory
ld.lld: error: cannot open crtn.o: No such file or directory
clang-17: error: linker command failed with exit code 1 (use -v to see invocation)
# After:
/bin/bash ../libtool --tag=CC --mode=link /android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -g -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mthumb -Wformat -Werror=format-security -fPIC -fno-limit-debug-info -fopenmp -L/vcpkg/installed/arm-neon-android-dynamic/debug/lib --target=armv7-none-linux-androideabi28 -Wl,--build-id=sha1 -Wl,--no-rosegment -Wl,--no-undefined-version -Wl,--fatal-warnings -Wl,--no-undefined -Qunused-arguments --sysroot=/android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/sysroot -version-info 1:4:0 -o libb2.la -rpath /vcpkg/installed/arm-neon-android-dynamic/debug/lib libb2_la-blake2s-ref.lo libb2_la-blake2b-ref.lo libb2_la-blake2sp.lo libb2_la-blake2bp.lo -latomic -lm
libtool: link: /android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -shared -fPIC -DPIC .libs/libb2_la-blake2s-ref.o .libs/libb2_la-blake2b-ref.o .libs/libb2_la-blake2sp.o .libs/libb2_la-blake2bp.o -L/vcpkg/installed/arm-neon-android-dynamic/debug/lib -latomic -lm -g -fstack-protector-strong -mthumb -fopenmp --target=armv7-none-linux-androideabi28 -Wl,--build-id=sha1 -Wl,--no-rosegment -Wl,--no-undefined-version -Wl,--fatal-warnings -Wl,--no-undefined --sysroot=/android-ndk-r26/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fopenmp -Wl,-soname -Wl,libb2.so -o .libs/libb2.so
There was a problem hiding this comment.
Hmm while searching for the issues i found: wolfSSL/wolfssl#4519
So you should probably use the wrapper script instead of invoking clang directly.
Is there an issue raised with libtool? Seems more like a generic problem with it.
There was a problem hiding this comment.
There is also https://debbugs.gnu.org/db/25/25944.html from 2017. So probably just missing somebody who is willing to submit a patch.
I don't think vcpkg should currently correct this.
There was a problem hiding this comment.
To put things straight, these are the official NDK instructions for autotools:
https://developer.android.com/ndk/guides/other_build_systems#autoconf
So there are prefixed tools. But they are not the tools used in vcpkg's CMake toolchains and forwarded to autotools as implemented in vcpkg_configure_make.
There was a problem hiding this comment.
So there are prefixed tools. But they are not the tools used in vcpkg's CMake toolchains
Then that should be changed
There was a problem hiding this comment.
Change vcpkg's cmake toolchain setup?
Or transform Android clang --target=x86_64-linux-android32 into x86_64-linux-android32-clang if it exists?
Or simply write a wrapper of that name ourself?
There was a problem hiding this comment.
transform Android clang --target=x86_64-linux-android32 into x86_64-linux-android32-clang
Change the toolchain to use the wrapper since the wrapper probably needs to be used in more places?
Alternatively, change it to the wrapper in cmake_get_vars?
I don't see a need to generate a wrapper ourself if upstream provides one (but I also already thought about generating wrappers for windows to make sure flags a correctly passed.).
There was a problem hiding this comment.
Change the toolchain to use the wrapper since the wrapper probably needs to be used in more places?
I'm afraid this is not acceptable because it is not the recommended use of the NDK with CMake. And it would need to deal with different versions of the NDK.
Alternatively, change it to the wrapper in cmake_get_vars?
I'm afraid this is not acceptable because this affects more places, including user ports.
JavierMatosD
added
the
requires:vcpkg-team-review
|
Ok, I like the changes here. I'm going to bring it up to the team during our next review meeting :) Thanks @dg0yt for your work and patience! |
JavierMatosD
added
the
info:reviewed
|
The team has reviewed the changes and agrees with the approach, as long as we can confirm that the command line to the compiler remains the same before and after, with the specific handling of the |
* Fix lib suffix matching * Preserve flags when transforming standard libs * Use separate_arguments * Fix transform * [vcpkg_configure_make] Fix SKIP_CONFIGURE * Update linker flags * Update linker flags * Fix regex substitution * Restore full CI * Reuse configure's linker flags setup * Handle ldflags separately from linker flags * Factor out common definitions * Control docdir and mandir for debug * Use vcpkg_list APPEND * Don't pass -avoid-version into configure * Restore lost escaping * Copy --target from LDFLAGS to CC/CXX * Consolidate macros into one function * Elaborate ABI flags * Elaborate flag processing * Update warnings for embedded space * Update windows tool path fixup * [libudns] No cross builds * [vcpkg_find_acquire_program] Add VSCLANG Find VS's clang without fallback to the giant LLVM download. * [gmp] Update CCAS setup * [nettle] Update CCAS setup * Fix VSCLANG * WIP * Restore processing of isysroot * [nettle] Fix assembler option * [liburing] Adjust * Variable name changes * Remove redundant separate arguments * Move -m32/-m64 to ABI flags * Unify ABIFLAGS spelling * CI * Revert VSCLANG * Use GNU Make to build autotools ports on FreeBSD (microsoft#32282) * Fix flag removal * Drop separation of ABI flags * Handle '-arch=...' * Revert "Control docdir and mandir for debug" This reverts commit d8293a1. * [icu] Pass uwp option to pkgdata * Restore old osx flags hook * Add triplet variable to make libtool pass --target to linker --------- Co-authored-by: Schaich, Alonso <[email protected]>
Changes numbered
C<n>for further reference. Don't renumber.C1: Includes #30381: Fix processing ofCMAKE_<LANG>_STANDARD_LIBRARIES.-l.C2: Fixes #14389SKIP_CONFIGUREoption.C3: Redirects man and doc dirs for the debug build.- Establishes consistency between release and debug.- Simplifying post-build cleanup for debug.- Behavioral change may affect ports bydebug/sharenow appearing. Fixed forliburing.(Too disruptive, may break user ports.)
C4: Fixes passing of linker options for windows, #31228 (comment).C5: Generally moves "ABI flags" from CFLAGS/CXXFLAGS to CC/CXX.Establishes consistency: This was already done for Apple toolchains.Actually for Apple toolchains, all CFLAGS used to be added to CC. This is now reduced to the set of known ABI flags.Allows to remove the-avoid-versionlibtool flag which breaksconfigure, cf. [vcpkg/scripts/make] avoid versioning for android #17089 (comment).Fixes [libb2] build failure with Android NDK r26 canary #31332: dynamic builds for Android. (Reverts [vcpkg/scripts/make] avoid versioning for android #17089.)C6: Improves warnings about unsupported whitespace. There are three different cases:C7: UpdatesCCASsetup forgmpandnettle.C8: On this occasion, adds aVSCLANGtool tovcpkg_find_acquire_programto improve behaviour when VS's clang isn't found. (No pointless LLVM download, no lengthy extraction, but clear messages. There were some issues were users struggled with that.) This addition is feasible/cheap since #30780.C9: Fixes #32282, use GNU make on FreeBSD (credits: @SchaichAlonso)C10: Fixes #30479, problems with special chars in paths due to use in regex....