[libarchive,lz4] Fix pc file, modernize portfile by dg0yt · Pull Request #20146 · microsoft/vcpkg

dg0yt · 2021-09-14T06:44:54Z

What does your PR fix?
- Fixes the invalid pkgconfig file by switching to modules. (Cf. [libgxps] Add new port #20062 (comment).)
- Modernizes the portfile. Cleanup.
- Drops the unused openssl dependency for osx.
- Fix find_dependency in the wrapper ([libarchive] wrapper emits warnings and uses find_dependency #20182).
Which triplets are supported/not supported? Have you updated the CI baseline?

unchanged, no
Does your PR follow the maintainer guide?

yes
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

yes
Features

pc file after vcpkg install 'libarchive[*]' on x64-osx:

prefix=${pcfiledir}/../..

exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/../include

Name: libarchive
Description: library that can create and read several streaming archive formats
Version: 3.5.2
Cflags: -I"${includedir}"
Cflags.private: -DLIBARCHIVE_STATIC
Libs: -L"${libdir}" -larchive 
Requires: zlib bzip2 liblzma lzo2 liblz4 libzstd libxml-2.0

(Note that libssl and libcrypto are missing. That's how I found that libarchive doesn't use OpenSSL on 'Darwin'.)

ports/libarchive/portfile.cmake

NancyLi1013 · 2021-09-14T07:59:08Z

ports/libarchive/pkgconfig-modules.patch

+ Cflags.private: -DLIBARCHIVE_STATIC
+ Libs: -L${libdir} -larchive
+ Libs.private: @LIBS@
+Requires.private: @LIBARCHIVE_REQUIRES_PRIVATE@


Could you please help submit the changes to upstream if possible?

There is also a similar issue from upstream libarchive/libarchive#1446

I will try, as time permits.

I would appreciate if someone could help us verify libarchive/libarchive#2253 which claims to be a fix for libarchive/libarchive#1446

ports/libarchive/portfile.cmake

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout e201002b4f4827d7b7b672f0c5c672a77fc3b77d -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/l-/libarchive.json b/versions/l-/libarchive.json
index 030b766..43ae7f8 100644
--- a/versions/l-/libarchive.json
+++ b/versions/l-/libarchive.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "88d645389e5de66763a38b44824f027bd0afb28c",
+      "git-tree": "eb7c77caa0e229ab1135c93439e828fe817ead73",
       "version-semver": "3.5.2",
       "port-version": 1
     },

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 5d734efcef674dbf9b7b204484fc8f006101dcb5 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/l-/libarchive.json b/versions/l-/libarchive.json
index 030b766..43ae7f8 100644
--- a/versions/l-/libarchive.json
+++ b/versions/l-/libarchive.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "88d645389e5de66763a38b44824f027bd0afb28c",
+      "git-tree": "eb7c77caa0e229ab1135c93439e828fe817ead73",
       "version-semver": "3.5.2",
       "port-version": 1
     },

dg0yt · 2021-09-16T07:47:59Z

Hm, there are no pc files for the MSVC builds of port openssl.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 5d734efcef674dbf9b7b204484fc8f006101dcb5 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/l-/libarchive.json b/versions/l-/libarchive.json
index c2bd24a..db88d23 100644
--- a/versions/l-/libarchive.json
+++ b/versions/l-/libarchive.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "8969063703167d3f1f756d4e4150e73ba55399db",
+      "git-tree": "d2243554df7c2ea6e5ea0c0400c938906a5931aa",
       "version-semver": "3.5.2",
       "port-version": 1
     },

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 5d734efcef674dbf9b7b204484fc8f006101dcb5 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/l-/libarchive.json b/versions/l-/libarchive.json
index c2bd24a..44b6870 100644
--- a/versions/l-/libarchive.json
+++ b/versions/l-/libarchive.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "8969063703167d3f1f756d4e4150e73ba55399db",
+      "git-tree": "1b734af1229f28de971d85044a365fb7069b1975",
       "version-semver": "3.5.2",
       "port-version": 1
     },

github-actions

You have modified or added at least one portfile where deprecated functions are used.

Details

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/libarchive/portfile.cmake

dg0yt · 2021-09-16T20:22:46Z

This is complete. If possible this should be tested with #20062 on windows, static and dynamic.
CC @c72578 @JonLiu1993

Neumann-A · 2021-09-17T20:54:40Z

ports/libarchive/vcpkg-cmake-wrapper.cmake.in

Why Z_VCPKG ? Where is the conflict?

Well, it is a wrapper. It is run in user project scope, isn't it?. And select_library_configurations is know to set <PREFIX>_FOUND. So this just tries to go out of the way of user projects.

NancyLi1013 · 2021-09-18T06:29:07Z

The problem caused by CI pipeline line depends on PR #20236.

c72578 · 2021-09-18T07:37:59Z

Remark: This is how libarchive.pc looks now (at 141ec0d4ccf3a65272c284ef49d573c8e696fc48) in case of x64-windows:

prefix=${pcfiledir}/../..

exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: libarchive
Description: library that can create and read several streaming archive formats
Version: 3.5.2
Cflags: -I"${includedir}"
Cflags.private: -DLIBARCHIVE_STATIC
Libs: -L"${libdir}" -larchive
Libs.private:  -lcrypto -lssl -lssl -lssl -lssl -lssl -lssl
Requires.private:  zlib bzip2 liblzma liblz4 libzstd libxml-2.0

It contains -lssl 6 times under Libs.private

dg0yt · 2021-09-18T07:58:12Z

Thanks for the feedback @c72578. I assume it works now for #20062?

It contains -lssl 6 times under Libs.private

I don't think this is a problem for now. The library is added for each matching algorithm. This could be improved upstream, instead of adding complexity to the patch. Maybe when I upstream the patch...

c72578 · 2021-09-18T20:22:03Z

@dg0yt the situation concerning #20062 has improved, however it is still failing for x64-linux:
/usr/bin/ld: cannot find -llz4

This is how libarchive.pc looks under x64-linux now (tested at commit 74f3712):

prefix=${pcfiledir}/../..

exec_prefix=${prefix}
libdir=${exec_prefix}/lib
includedir=${prefix}/include

Name: libarchive
Description: library that can create and read several streaming archive formats
Version: 3.5.2
Cflags: -I"${includedir}"
Cflags.private: -DLIBARCHIVE_STATIC
Libs: -L"${libdir}" -larchive 
Requires: zlib bzip2 liblzma liblz4 libzstd libcrypto libxml-2.0 libssl libssl libssl libssl libssl libssl

See: liblz4 in Requires

c72578 · 2021-09-19T07:31:14Z

@dg0yt The remaining issue is related to the port lz4.
lz4_x64-linux/debug/lib/pkgconfig\liblz4.pc contains -llz4, but it should contain -llz4d

prefix=${pcfiledir}/../..
#   LZ4 - Fast LZ compression algorithm
#   Copyright (C) 2011-2014, Yann Collet.
#   BSD 2-Clause License (http://www.opensource.org/licenses/bsd-license.php)

libdir=${prefix}/lib
includedir=${prefix}/../include

Name: lz4
Description: extremely fast lossless compression algorithm library
URL: http://www.lz4.org/
Version: 1.9.3
Libs: -L"${prefix}/lib" -llz4
Cflags: -I"${prefix}/../include"

The issue is resulting from here:

vcpkg/ports/lz4/portfile.cmake

Line 34 in 4428702

if(EXISTS "${CURRENT_PACKAGES_DIR}/debug/lib/pkgconfig/lz4.pc")

The file is called liblz4.pc

dg0yt · 2021-09-19T08:12:11Z

lz4_x64-linux/debug/lib/pkgconfig\liblz4.pc contains -llz4, but it should contain -llz4d

Thanks for reporting @c72578. I added a fix to this PR now.

c72578

@dg0yt Thanks. I have completed the testing and the PR LGTM now.

c72578 · 2021-09-22T07:07:49Z

@NancyLi1013 PR #20236 has been merged in the meantime.

NancyLi1013 · 2021-09-23T02:36:13Z

/azp run

azure-pipelines · 2021-09-23T02:36:22Z

Azure Pipelines successfully started running 1 pipeline(s).

…rchive

BillyONeal · 2021-09-24T19:05:53Z

Thanks @dg0yt for the fixes, @c72578 @JonLiu1993 for testing, and @Neumann-A for the review!

dg0yt added 4 commits September 14, 2021 07:36

Fix pc file

d23579e

Modernize portfile

8e63b06

OpenSSL unused on 'Darwin'

80816a3

x-add-version

266c59e

dg0yt changed the title ~~[ibarchive] Fix pc file, modernize portfile~~ [libarchive] Fix pc file, modernize portfile Sep 14, 2021

github-actions bot approved these changes Sep 14, 2021

View reviewed changes

dg0yt mentioned this pull request Sep 14, 2021

[libgxps] Add new port #20062

Merged

1 task

PhoebeHui assigned NancyLi1013 Sep 14, 2021

NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label Sep 14, 2021

NancyLi1013 suggested changes Sep 14, 2021

View reviewed changes

NancyLi1013 added the requires:author-response label Sep 14, 2021

dg0yt mentioned this pull request Sep 15, 2021

[libarchive] wrapper emits warnings and uses find_dependency #20182

Closed

JackBoosY linked an issue Sep 16, 2021 that may be closed by this pull request

[libarchive] wrapper emits warnings and uses find_dependency #20182

Closed

More port cleanup (CR requests)

f38fbdc

github-actions bot requested changes Sep 16, 2021

View reviewed changes

dg0yt force-pushed the libarchive branch from f2e7f90 to 006aad3 Compare September 16, 2021 07:22

github-actions bot requested changes Sep 16, 2021

View reviewed changes

Revise wrapper, using find_package

0454ce9

dg0yt force-pushed the libarchive branch from 006aad3 to 0454ce9 Compare September 16, 2021 07:47

github-actions bot requested changes Sep 16, 2021

View reviewed changes

Don't use pkgconfig modules for windows openssl

1623229

github-actions bot requested changes Sep 16, 2021

View reviewed changes

Update versions

bedf8c5

github-actions bot approved these changes Sep 16, 2021

View reviewed changes

Use namespaced variables for custom lzo lookup

02c2d64

Neumann-A reviewed Sep 17, 2021

View reviewed changes

NancyLi1013 added the depends:different-pr This PR or Issue depends on a PR which has been filed label Sep 18, 2021

dg0yt added 3 commits September 18, 2021 10:52

Wrapper needs CMP0012

ff2c842

Skip dependencies if not LibArchive_FOUND

2909177

Update versions

74f3712

dg0yt force-pushed the libarchive branch from 54e8a9e to 74f3712 Compare September 18, 2021 08:53

dg0yt changed the title ~~[libarchive] Fix pc file, modernize portfile~~ [libarchive,lz4] Fix pc file, modernize portfile Sep 19, 2021

dg0yt added 2 commits September 19, 2021 10:13

Fix liblz4.pc debug lib fixup

d38f8f6

Update versions

37b8fdb

dg0yt force-pushed the libarchive branch from 8403b1d to 37b8fdb Compare September 19, 2021 08:13

dg0yt mentioned this pull request Sep 19, 2021

[lz4] Install pc file, cleanup #20108

Merged

3 tasks

c72578 approved these changes Sep 19, 2021

View reviewed changes

NancyLi1013 removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Sep 23, 2021

Merge branch 'master' of https://github.com/Microsoft/vcpkg into liba…

eb32ad6

…rchive

github-actions bot approved these changes Sep 23, 2021

View reviewed changes

NancyLi1013 approved these changes Sep 23, 2021

View reviewed changes

NancyLi1013 added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:author-response labels Sep 23, 2021

BillyONeal merged commit 845a5fd into microsoft:master Sep 24, 2021

dg0yt deleted the libarchive branch December 12, 2021 09:15

Conversation

dg0yt commented Sep 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Features

Uh oh!

Uh oh!

NancyLi1013 Sep 14, 2021

Choose a reason for hiding this comment

Uh oh!

dg0yt Sep 16, 2021

Choose a reason for hiding this comment

Uh oh!

kientzle Jul 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Sep 16, 2021

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

dg0yt Sep 17, 2021

Choose a reason for hiding this comment

Uh oh!

NancyLi1013 commented Sep 18, 2021

Uh oh!

c72578 commented Sep 18, 2021

Uh oh!

dg0yt commented Sep 18, 2021

Uh oh!

c72578 commented Sep 18, 2021

Uh oh!

c72578 commented Sep 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dg0yt commented Sep 19, 2021

Uh oh!

c72578 left a comment

Choose a reason for hiding this comment

Uh oh!

c72578 commented Sep 22, 2021

Uh oh!

NancyLi1013 commented Sep 23, 2021

Uh oh!

azure-pipelines bot commented Sep 23, 2021

Uh oh!

BillyONeal commented Sep 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dg0yt commented Sep 14, 2021 •

edited

Loading

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

dg0yt commented Sep 16, 2021 •

edited

Loading

c72578 commented Sep 19, 2021 •

edited

Loading