build: Improve build by cmake for Cygwin

[erw7]

• Fix issue that can't build on Cygwin.
• Add installation targets to Windows.
• Fix an issue that can't be compiled with MSVC in specific language environment.
• Fix file name of static library.
• Change to be able to select whether to build static library or shared library.
• Fix problem where building on Windows fails if BUILD_TESTING is set.

[justinmk]

cc @bradking

[erw7]

#1850 (comment) problem is not solved. So I changed test of shared library to not compile on Windows. The cause of the build failure is that the required function(make_program_args, quote_cmd_arg, make_program_env) has not been exported to dll. I think that it can be fixed by the following method.

• Add src/win/process.c masked with #undef to uv_test_sources.
• Export those functions to dll.

Should we fix it in either way?

[justinmk]

Anything blocking this from inclusion?

[blueyed]

@erw7
Can you rebase this to fix conflicts?

(I came across a fix for the static library name also just yet (which is included here already): #2374)

[justinmk]

Needs rebase (conflict).

[saghul]

Heads up: this one will conflict heavily with #2504

I believe that landing #2504 first is a good idea here, since it targets our build more broadly than just making it work on Cygwin. Then this one would need to be rebased.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[justinmk]

Waiting on #2504

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[justinmk]

@erw7 looks like #2504 was merged, is this PR still relevant?

[erw7]

looks like #2504 was merged, is this PR still relevant?

I fixed it to match the change in #2504.

[saghul]

@slurps-mad-rips Issabella, any chance you could take a look at this?

[bruxisma]

@saghul I left a few comments. I've got some personal choices I disagree with, but that's because I haven't had time to finish cleaning up the entire file itself. 😛
That said, we still haven't merged my unit test changes, which affects this PR to a degree as well. I'll ping on that commit when I have a moment (it's late here in my timezone so I'll have to wait until the day star is back in the sky 🙂)

That said, I know I can't have any authority on this, but I'm vehemently against renaming the "logical target" names for uv and uv_a until I have time to make sure that exporting targets works correctly. Right now if we change the CMake name for any of these targets, anyone including it directly within their build via add_subdirectory (which my changes in #2504 allow for), their builds will break. It's possible to fix this, but I'd need to sit down and outline a plan as it's more than just a simple name change with some property settings.

[vtjnash]

still needs a rebase

[erw7]

still needs a rebase

I rebased and squashed.

[vtjnash]

CI: https://ci.nodejs.org/job/libuv-test-commit/2201/

CI no longer fails, as per #2085 (comment)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[vtjnash]

@vtjnash Since the main focus of this PR is to rename the static library, your suggestion is not acceptable.

I don't understand why such a basic explanation is needed, but the following assertion in #2374 (comment) is wrong.

I have learned a lot more about CMake in the last couple years, starting from zero. I can now understand why that assertion was wrong. Furthermore:

The fact that there is already a user for uv_a is a problem, but we should stop installing static libraries with names that deviate from common practice. If you still can't accept the above explanation, you can close this.

Actually, it turns out that we already have numerous tests for this in CI, showing that luv should be perfectly happy with this change too. Had to rebase (merge conflicts with LIBUV_BUILD_SHARED) and then will plan to merge once CI finishes.

[sl1200mk2]

Hi,
these changes have broken my windows build:
17ea56e

especially the if(CYGWIN OR MSYS)

I'm using msys2 which is up to date and cmake to build libUV. my cmake line is :
cmake -G "MSYS Makefiles" .. -DCMAKE_C_COMPILER=gcc -DCMAKE_BUILD_TYPE=Release -DCMAKE_PREFIX_PATH=/opt/local -DCMAKE_INSTALL_PREFIX=/opt/local -DCMAKE_C_FLAGS="-O2" -DLIBUV_BUILD_TESTS:BOOL=OFF

[autoantwort]

Why is there a colon? It breaks the build for me:

[133/228] : && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -fPIC -g -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names  wpinet/CMakeFiles/wpinet_dsclient.dir/examples/dsclient/dsclient.cpp.o -o bin/wpinet_dsclient  lib/libwpinetd.a  lib/libwpiutild.a  /Users/leanderSchulten/git_projekte/vcpkg/vcpkg_installed/arm64-osx/debug/lib/libfmtd.a  -l:libuv.a  /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/lib/libpthread.tbd && :
FAILED: bin/wpinet_dsclient 
: && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -fPIC -g -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names  wpinet/CMakeFiles/wpinet_dsclient.dir/examples/dsclient/dsclient.cpp.o -o bin/wpinet_dsclient  lib/libwpinetd.a  lib/libwpiutild.a  /Users/leanderSchulten/git_projekte/vcpkg/vcpkg_installed/arm64-osx/debug/lib/libfmtd.a  -l:libuv.a  /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/lib/libpthread.tbd && :
ld: library not found for -l:libuv.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)

and removing it resolved this error.

[vtjnash]

-l: is a different flag from -l and specifies a full file name (instead of just the soname)

[autoantwort]

Is this a gcc thing? Because I have a libuv.a on my library search patch, but as written in the error message it is not found.

[autoantwort]

Seems that this is only supported by the gcc linker and not the llvm linker.

[vtjnash]

It sounds like an ld.lld emulation bug in LLVM in that case

[autoantwort]

But nearly all *.pc files I have seen are wrong in this respect.

[vtjnash]

True, I don't really know why *.pc files seem to use -L${libdir}, when the logical thing would be ${libdir}/libuv.a

[vtjnash]

Ah, apparently it is because those particular flags have special meaning to pkg-config beyond their use in ld (c.f. PKG_CONFIG_SYSROOT_DIR)

[autoantwort]

Hm ok. But this would mean that I use a pkg-config file from a lib of my host system to use a lib on my target system instead of using the pkg-config file of the lib for the target system directly (via PKG_CONFIG_PATH).

[autoantwort]

I probably patch the lib that uses libuv via pkgconfig to use cmake config files ... :D

[mizvekov]

Part of this MR is causing issues building on windows with Ninja generator.

See #4139

[bnoordhuis]

As commit 17ea56e is apparently causing regressions, should it be reverted? It reverts cleanly.

[vtjnash]

There is a part of that commit that is supposed to add the lib prefix when not gcc, which presumably fixes that already. But it only supports msvc

[mizvekov]

The only parts of the MR that needs to be reverted are the lines which change the OUTPUT_NAME.

• set_target_properties(uv_a PROPERTIES OUTPUT_NAME "uv")
• set_target_properties(uv_a PROPERTIES OUTPUT_NAME "uv")
• The changes to the package config template.

I don't think this part fixes anything, even in MSVC, as that repro I posted on the issue is testing exactly that configuration.

if(MSVC)
  set_target_properties(uv_a PROPERTIES PREFIX "lib")
endif()

That also imposing an unixism on windows. On windows, libs don't use a lib prefix and cmake takes well care that this is modelled correctly.