[libdeflate] New port

[SunBlack]

• Changes comply with the maintainer guide
• The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
• Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
• The versioning scheme in vcpkg.json matches what upstream says.
• The license declaration in vcpkg.json matches what upstream says.
• The installed as the "copyright" file matches what upstream says.
• The source code of the component installed comes from an authoritative source.
• The generated "usage text" is accurate. See adding-usage for context.
• The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
• Only one version is in the new port's versions file.
• Only one version is added to each modified port's versions file.

Unfortunately, the library only creates specific targets for each library linkage type:

find_package(libdeflate CONFIG REQUIRED)
target_link_libraries(main PRIVATE libdeflate::libdeflate_static)

So there is no target libdeflate::libdeflate, i.e. you cannot simply switch from the triplet x64-windows to x64-windows-static. Should I add a patch to add an ALIAS target?

[FrankXie05]

Suggested change

	if (${VCPKG_LIBRARY_LINKAGE} STREQUAL "dynamic")
	if (VCPKG_LIBRARY_LINKAGE STREQUAL "static")

Correct usage.

[dg0yt]

I think we prefer

string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "static" STATIC_BUILD)
string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "dynamic" SHARED_BUILD)

over if/else.

[SunBlack]

@dg0yt Are you sure? When I search for COMPARE[^\n]*VCPKG_LIBRARY_LINKAGE in the ports directory I get hits in 161 files vs 445 files when searching for if[^\n]*VCPKG_LIBRARY_LINKAGE ;-)

[dg0yt]

Yes, I'm sure. It is frequently implemented in port modernization.

$ git grep -A1 'if[^\n]*VCPKG_LIBRARY_LINKAGE'  ports/ | grep -i ' set' | wc -l
112

And if you look at the context, even some of the remaining set commands have a different nature, such as selecting particular patches.

[FrankXie05]

We need a generic usage not only static.

find_package(libdeflate CONFIG REQUIRED)
target_link_libraries(main PRIVATE libdeflate::libdeflate_static)

[dg0yt]

So there is no target libdeflate::libdeflate, i.e. you cannot simply switch from the triplet x64-windows to x64-windows-static. Should I add a patch to add an ALIAS target?

See port zstd: Uniform usage via generator expression.

[dg0yt]

I think we prefer

string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "static" STATIC_BUILD)
string(COMPARE EQUAL "${VCPKG_LIBRARY_LINKAGE}" "dynamic" SHARED_BUILD)

over if/else.

[SunBlack]

So there is no target libdeflate::libdeflate, i.e. you cannot simply switch from the triplet x64-windows to x64-windows-static. Should I add a patch to add an ALIAS target?

See port zstd: Uniform usage via generator expression.

ok, so we make a mountain out of a molehill - I hope no one is using clang-format with a line limit 😅

find_package(libdeflate CONFIG REQUIRED)
target_link_libraries(main PRIVATE $<IF:$<TARGET_EXISTS:libdeflate::libdeflate_shared>,libdeflate::libdeflate_shared,libdeflate::libdeflate_static>)

[SunBlack]

Does anyone see why the CI is failing?

• uwp: No idea if this is an CI issue or the library isn't compatible to UWP (I have locally no UWP setup to test it)

  CMake Warning at scripts/cmake/vcpkg_execute_build_process.cmake:65 (message):
  Please ensure your system has sufficient memory.
  

• windows_static_md: warning: The following binaries should use the Dynamic Release (/MD) CRT. - I don't see that /MD or /MT will be explicit set in the CMake code of the lib and I don't see any specific code in other ports for it, so what is missing?

[dg0yt]

ok, so we make a mountain out of a molehill

😆
At least the mountain fits into the clipboard, and the user is not locked into the vcpkg molehill.
It would be necessary to convince upstream to do the right thing. But usually they already had this discussion.

[dg0yt]

Does anyone see why the CI is failing?

It is possibly to click through from GH ot Azure Pipelines to published artifacts to download logs of the failed build.
On the last page, the download action is hidden behind three dots on the right side of each asset line:
https://dev.azure.com/vcpkg/public/_build/results?buildId=86029&view=artifacts&pathAsName=false&type=publishedArtifacts

• windows_static_md: warning: The following binaries should use the Dynamic Release (/MD) CRT. - I don't see that /MD or /MT will be explicit set in the CMake code of the lib and I don't see any specific code in other ports for it, so what is missing?

The package overwrites CMAKE_C_FLAGS_RELEASE. It should append instead.
https://github.com/ebiggers/libdeflate/blob/master/CMakeLists.txt#L64

[dg0yt]

The package overwrites CMAKE_C_FLAGS_RELEASE. It should append instead.

Maybe this also the answer to the uwp problem: Only the release build fails.

[Neumann-A]

The package overwrites CMAKE_C_FLAGS_RELEASE. It should append instead.
https://github.com/ebiggers/libdeflate/blob/master/CMakeLists.txt#L64

That should be patched out for vcpkg.

[SunBlack]

Does anyone see why the CI is failing?

It is possibly to click through from GH ot Azure Pipelines to published artifacts to download logs of the failed build. On the last page, the download action is hidden behind three dots on the right side of each asset line: https://dev.azure.com/vcpkg/public/_build/results?buildId=86029&view=artifacts&pathAsName=false&type=publishedArtifacts

• windows_static_md: warning: The following binaries should use the Dynamic Release (/MD) CRT. - I don't see that /MD or /MT will be explicit set in the CMake code of the lib and I don't see any specific code in other ports for it, so what is missing?

The package overwrites CMAKE_C_FLAGS_RELEASE. It should append instead. https://github.com/ebiggers/libdeflate/blob/master/CMakeLists.txt#L64

Oh thx, just searched for /MD, /MT, ... and not for such basic issue 😄 Interesting that MSVC doesn't complain them.

[njakob]

Is it expected that baseline.json does not define a port-version?

I started to get warnings in my scripts on vcpkg.link today since this is the only case that does not include the field.

@FrankXie05

[njakob]

Okay, it seems this will be automatically fixed as all open PRs add the missing field

https://github.com/microsoft/vcpkg/pull/29993/files#r1124389057