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

[vcpkg/scripts/make] trying to iron out some issues #11836

Merged
merged 34 commits into from
Aug 13, 2020

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Jun 8, 2020

includes #11810
closes #11765

make all make dependent ports depend on vcpkg-make for easier testing and rebuilding.
Fix some script errors in vcpkg_configure_make
Removed a lot of unnecessary stuff in vcpkg_build_make

ports/vcpkg-make/CONTROL Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MVoz
Copy link
Contributor

MVoz commented Jun 9, 2020

2020-06-09T14:52:04.9166987Z Building package libwandio[core]:x86-windows...
2020-06-09T14:52:04.9167779Z Could not locate cached archive: W:\fe\fe2314dad8161f43cdcb0a6f0d6f2ac22fa2079e.zip
2020-06-09T14:53:42.4048442Z -- Using cached D:/downloads/wanduow-wandio-012b646e7ba7ab191a5a2206488adfac493fcdc6.tar.gz
2020-06-09T14:53:42.4049322Z -- Extracting source D:/downloads/wanduow-wandio-012b646e7ba7ab191a5a2206488adfac493fcdc6.tar.gz
2020-06-09T14:53:42.4049784Z -- Applying patch configure.lib.patch
2020-06-09T14:53:42.4050129Z -- Applying patch configure.patch
2020-06-09T14:53:42.4051153Z -- Using source at E:/buildtrees/libwandio/src/ac493fcdc6-43e4434cbc
2020-06-09T14:53:42.4051679Z -- Acquiring MSYS Packages...
2020-06-09T14:53:42.4052694Z -- Acquiring MSYS Packages... OK
2020-06-09T14:53:42.4053404Z -- Generating configure for x86-windows
2020-06-09T14:53:42.4054509Z -- Finished generating configure for x86-windows
2020-06-09T14:53:42.4055029Z -- Configuring x86-windows-dbg
2020-06-09T14:53:42.4055847Z -- Configuring x86-windows-rel
2020-06-09T14:53:42.4057610Z -- Building x86-windows-dbg
2020-06-09T14:53:42.4058696Z CMake Error at scripts/cmake/vcpkg_execute_build_process.cmake:136 (message):
2020-06-09T14:53:42.4059882Z     Command failed: D:/downloads/tools/msys2/msys64/usr/bin/make.exe -j 17 --trace -f Makefile all
2020-06-09T14:53:42.4060762Z     Working Directory: E:/buildtrees/libwandio/x86-windows-dbg
2020-06-09T14:53:42.4061794Z     See logs for more information:
2020-06-09T14:53:42.4062752Z       E:\buildtrees\libwandio\build-x86-windows-dbg-out.log
2020-06-09T14:53:42.4063481Z       E:\buildtrees\libwandio\build-x86-windows-dbg-err.log

@Neumann-A
Copy link
Contributor Author

@voskrese because it fails in the baseline?

libwandio:x86-windows=fail
libwandio:x64-windows=fail
libwandio:x64-windows-static=fail
libwandio:x64-uwp=fail
libwandio:arm64-windows=fail

@MVoz
Copy link
Contributor

MVoz commented Jun 9, 2020

libtool: link:  E:/tools/vcpkg/downloads/tools/msys2/msys64/usr/share/automake-1.16/compile cl.exe -nologo -Fe .libs\\b2-1.dll  .libs/libb2_la-blake2s-ref.obj .libs/libb2_la-blake2b-ref.obj .libs/libb2_la-blake2sp.obj .libs/libb2_la-blake2bp.obj   -O3 -openmp   -openmp  -LE:/tools/vcpkg/installed/x64-windows/debug/lib "@.libs\\b2-1.dll.exp" -Wl,-DLL,-IMPLIB:".libs\\b2.lib"
cl : Command line warning D9002 : ignoring unknown option '-O3'
cl : Command line warning D9024 : unrecognized source file type '.libs\b2-1.dll', object file assumed
LINK : fatal error LNK1181: cannot open input file '.libs\b2-1.dll'
�[1;34mmake[2]:�[0m *** [Makefile:923: libb2.la] Error 2

add port - vcpkg_check_linkage(ONLY_STATIC_LIBRARY)

-- Checking file: E:/tools/vcpkg/packages/libb2_x64-windows/lib/pkgconfig/libb2.pc
-- Fixing pkgconfig - debug
-- Checking file: E:/tools/vcpkg/packages/libb2_x64-windows/debug/lib/pkgconfig/libb2.pc
-- Fixing pkgconfig --- finished
-- Installing: E:/tools/vcpkg/packages/libb2_x64-windows/share/libb2/copyright
-- Performing post-build validation
Expected Debug,Dynamic crt linkage, but the following libs had invalid crt linkage:

    E:/tools/vcpkg/packages/libb2_x64-windows/debug/lib/b2.lib: Release,Static

To inspect the lib files, use:
    dumpbin.exe /directives mylibfile.lib
Expected Release,Dynamic crt linkage, but the following libs had invalid crt linkage:

    E:/tools/vcpkg/packages/libb2_x64-windows/lib/b2.lib: Release,Static

To inspect the lib files, use:
    dumpbin.exe /directives mylibfile.lib
Found 2 error(s). Please correct the portfile:

@JackBoosY JackBoosY added the category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) label Jun 10, 2020
@JackBoosY
Copy link
Contributor

Is this PR ready for review?

@MVoz
Copy link
Contributor

MVoz commented Jun 11, 2020

remove WINDOWS support from the ports, because it doesn't work

@Neumann-A
Copy link
Contributor Author

@voskrese: It probably works with a MinGW community triplet and as such failing on Windows would be too restrictive

@MVoz
Copy link
Contributor

MVoz commented Jun 11, 2020

on MINGW, 80% of the ports do not work, the main goal of MSVC under WINDOWS. I think we should stick to priorities or delineate

https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/control-files.md

a6257ac

wasm32 - VCPKG_TARGET_ARCHITECTURE == "wasm32"
emscripten - VCPKG_CMAKE_SYSTEM_NAME == "Emscripten"

add

VCPKG_CMAKE_SYSTEM_NAME = MSYS2 or CYGWIN
VCPKG_TARGET_ARCHITECTURE = MinGW32 and MinGW64

@JackBoosY JackBoosY added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly and removed category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) labels Jun 12, 2020
@MVoz
Copy link
Contributor

MVoz commented Jun 14, 2020

@Neumann-A
look
#11934
?

@JackBoosY
Copy link
Contributor

Any update?

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Jul 9, 2020

Waiting for the pkgconfig PR to get merged first. I mean the one rewriting fixup_pkgconfig

@PhoebeHui
Copy link
Contributor

Waiting for PR #11550 merged first, right?

@strega-nil strega-nil removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Aug 7, 2020
@strega-nil strega-nil requested a review from JackBoosY August 7, 2020 17:45
add quotes around restoring of ENV
add LIB and LIBPATH correctly
Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Since this PR lasts too long, please open a new PR to update the vcpkg_configure_make document after this PR merges.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 10, 2020
@JackBoosY
Copy link
Contributor

Thanks for your hard work!

- LIBWANDIO_LIBS="$LIBWANDIO_LIBS -lcurl"
+ if test "$ac_cv_search_curl_easy_pause" != "none required"; then
+ LIBWANDIO_LIBS="$LIBWANDIO_LIBS $ac_cv_search_curl_easy_pause -lssl -lcrypto $ac_cv_search_pthread_create -ldl"
+ LIBS="$LIBS -lssl -lcrypto $ac_cv_search_pthread_create -ldl"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be fixed instead by producing .pc files for curl and using

PKG_CHECK_MODULES([CURL], [curl])
LIBS="$LIBS $CURL_LIBS"

Based on my reading of https://autotools.io/pkgconfig/pkg_check_modules.html, the default action-if-not-found is to error out.

Otherwise, we should Build-Depends: openssl in libwandio because it's directly expressing a dependency on openssl (e.g. imagine a case of building curl without openssl and thus this would fail).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be a proper fix but I did not want to touch this port any more than necessary. Because every dependency in libwandio would need that fix. If you want I can back out the patches to libwandio.

ports/libwandio/portfile.cmake Show resolved Hide resolved
scripts/cmake/vcpkg_build_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_build_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_build_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
set(REQUIRES_AUTOGEN TRUE)
set(REQUIRES_AUTOCONFIG FALSE)
if(VCPKG_LIBRARY_LINKAGE STREQUAL "static" AND NOT PKGCONFIG STREQUAL "--static")
set(PKGCONFIG "${PKGCONFIG} --static")
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on my reading of https://people.freedesktop.org/~dbn/pkg-config-guide.html, I think we should never pass --static. Instead, the producer side should always generate .pc files that can be used without --static; if the producer build is being built statically, it should list its dependencies as direct Requires: instead of Requires.private:.

Having the consumer predict how the producer was built seems fundamentally broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having the consumer predict how the producer was built seems fundamentally broken.

I agree with that. Would probably be a job for vcpkg_fixup_pkgconfig to correct the fields. But that is probably something for a different PR.

We could also always pass --static without any problems/corrections since the only thing we would be doing is overlinking all the time which decreases performance but should not yield a build link error due to missing symbols.

endif()

if (CMAKE_HOST_WIN32)
set(TMP_CFLAGS "${C_FLAGS_GLOBAL} ${VCPKG_C_FLAGS_${CMAKE_BUILDTYPE}}")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's enabling all this regex machinery to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything is setup while configuring and not while building. Which means the flags get embedded into the makefile on configure.

E.g. from one Makefile:

CFLAGS =  -MT -O2 -Oi -Gy -DNDEBUG 
CONFIGDIR = ${BASECONFIGDIR}/conf.d
CPP = compile cl.exe -nologo -E
CPPFLAGS =  -IE:/qt/installed/x64-windows-static/include -D_WIN32_WINNT=0x0601 -DWIN32_LEAN_AND_MEAN -DWIN32 -D_WINDOWS

scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
scripts/cmake/vcpkg_configure_make.cmake Outdated Show resolved Hide resolved
Neumann-A and others added 3 commits August 10, 2020 23:02
- remove mingw make
- add doc for missing options
- introduce two new macros for backup/restore of env vars
@Neumann-A
Copy link
Contributor Author

@JackBoosY: I assume you are trying to fix the regression seen in this PR. It also seems like you qt retry fix does not work correctly.

@JackBoosY
Copy link
Contributor

JackBoosY commented Aug 12, 2020

@Neumann-A That's weird, I need to collect the failure logs.
Confirmed, I searched the wrong log file.

@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil strega-nil merged commit 4935f12 into microsoft:master Aug 13, 2020
@Neumann-A Neumann-A deleted the fix_make_details branch August 13, 2020 21:03
remz1337 pushed a commit to remz1337/vcpkg that referenced this pull request Aug 23, 2020
* take changes from fontconfig pr

* [farmhash] add autoconf

* [freexl] add autoconf

* [healpix] add autoconf

* [libb2] add autoconf

* [libwandio] add autoconf and patch

* more autoconf

* [x264] fix windows build issues

* minimal cleanup

* [libwandio] some fixes

* [vcpkg/scripts/make] add include to C/CXX flags correctly set machine flags for linker

* remove unnecessary comments part 1

* cleanup part 2

* cleanup

* remove unnecessary code

* [pbc] fix osx regressions

* [lzokay] format manifest

* try to copy sources to fix build issues

* add autoconfig to force updated configure scripts

* bump port versions of openmpi and ocilib

* added lib paths back into vcpkg_build_make because they are probably required

* Use CPP flags
add quotes around restoring of ENV
add LIB and LIBPATH correctly

* Apply suggestions from code review

Co-authored-by: ras0219 <[email protected]>

* Apply more changes forom CR
- remove mingw make
- add doc for missing options
- introduce two new macros for backup/restore of env vars

* fix wrong variables.

* use the list macro instead of the single var macro

* also use it at the top

Co-authored-by: Jack·Boos·Yu <[email protected]>
Co-authored-by: Nicole Mazzuca <[email protected]>
Co-authored-by: ras0219 <[email protected]>
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:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[x264:x86-windows] build failure x264 build failure - /usr/bin/bash -j invalid option
8 participants