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

[libzip] Update to version 1.8.0 #18552

Closed
wants to merge 8 commits into from
Closed

Conversation

ctabin
Copy link
Contributor

@ctabin ctabin commented Jun 20, 2021

No description provided.

@ctabin ctabin force-pushed the update-libzip branch 3 times, most recently from de8e4cd to 5c41a9a Compare June 20, 2021 13:57
@JonLiu1993 JonLiu1993 self-assigned this Jun 21, 2021
@JonLiu1993 JonLiu1993 added the category:port-update The issue is with a library, which is requesting update new revision label Jun 21, 2021
Copy link
Member

@JonLiu1993 JonLiu1993 left a comment

Choose a reason for hiding this comment

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

@ctabin ,Please add 'FEATURES' in the function"vcpkg_check_features" like:

vcpkg_check_features(
OUT_FEATURE_OPTIONS FEATURE_OPTIONS
FEATURES
    bzip2 ENABLE_BZIP2   
    liblzma ENABLE_LZMA    
    openssl ENABLE_OPENSSL   
    wincrypto ENABLE_WINDOWS_CRYPTO    
    commoncrypto ENABLE_COMMONCRYPTO   
    mbedtls ENABLE_MBEDTLS)

ports/libzip/vcpkg.json Outdated Show resolved Hide resolved
@JonLiu1993
Copy link
Member

@ctabin ,Have you test all features locally in the following triplet:

  • x64-windows
  • x64-windows-static
  • x64-linux

ports/libzip/portfile.cmake Outdated Show resolved Hide resolved
@JonLiu1993
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

ports/libzip/portfile.cmake Outdated Show resolved Hide resolved
@PhoebeHui
Copy link
Contributor

@ctabin, can you address the review suggestions and the CI test failures?

@ctabin
Copy link
Contributor Author

ctabin commented Jul 26, 2021

@PhoebeHui Sorry for the late reply. I'll make the changes asap !

@ctabin ctabin force-pushed the update-libzip branch 2 times, most recently from 6b8e0eb to d70b5b0 Compare July 27, 2021 17:15
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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 7dbc05515b44bf54d2a42b4da9d1e1f910868b86 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/libzip.json b/versions/l-/libzip.json
index dea575d..ab7326c 100644
--- a/versions/l-/libzip.json
+++ b/versions/l-/libzip.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "26ba5523db09213f532821875542dba7afa04b65",
+      "git-tree": "a3adaf767853bfd7f7f2d511f4e3ec9eb9fa0b46",
       "version-semver": "1.8.0",
       "port-version": 0
     },

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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 7dbc05515b44bf54d2a42b4da9d1e1f910868b86 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/libzip.json b/versions/l-/libzip.json
index dea575d..ab7326c 100644
--- a/versions/l-/libzip.json
+++ b/versions/l-/libzip.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "26ba5523db09213f532821875542dba7afa04b65",
+      "git-tree": "a3adaf767853bfd7f7f2d511f4e3ec9eb9fa0b46",
       "version-semver": "1.8.0",
       "port-version": 0
     },

@ctabin ctabin requested a review from JonLiu1993 July 27, 2021 17:52
@JonLiu1993
Copy link
Member

JonLiu1993 commented Jul 29, 2021

@ctabin ,Yesterday CI reported an error that the patch could not be applied. I re-made the patch and submitted it, but today it shows that there is still an error under Linux. Could you please take a look?

@ctabin
Copy link
Contributor Author

ctabin commented Aug 1, 2021

Hi @JonLiu1993,
Thanks for help on this. Unfortunately, since I'm not the maintainer/developer of libzip, it is quite hard for me to know exactly what's going on.
So far, in the CI logs, I see:

2021-07-28T13:34:07.4376012Z Error: Building package ignition-fuel-tools4:x64-linux failed with: BUILD_FAILED
2021-07-28T13:34:07.4376816Z Elapsed time for package ignition-fuel-tools4:x64-linux: 2.381 s
2021-07-28T13:34:07.4377495Z Starting package 152/152: libzippp:x64-linux
2021-07-28T13:34:07.4378143Z Building package libzippp[core]:x64-linux...
2021-07-28T13:34:10.0290862Z -- Downloading https://github.com/ctabin/libzippp/archive/8299422194ce3c5be0677550ce3d6d4e15d40dd8.tar.gz -> ctabin-libzippp-8299422194ce3c5be0677550ce3d6d4e15d40dd8.tar.gz...
2021-07-28T13:34:10.0292386Z -- Extracting source /mnt/vcpkg-ci/downloads/ctabin-libzippp-8299422194ce3c5be0677550ce3d6d4e15d40dd8.tar.gz
2021-07-28T13:34:10.0293151Z -- Applying patch fix-find-lzma.patch
2021-07-28T13:34:10.0293894Z -- Using source at /mnt/vcpkg-ci/buildtrees/libzippp/src/4e15d40dd8-75ef2fee11.clean
2021-07-28T13:34:10.0294532Z -- Configuring x64-linux-dbg
2021-07-28T13:34:10.0295095Z -- Configuring x64-linux-rel
2021-07-28T13:34:10.0295860Z -- Building x64-linux-dbg
2021-07-28T13:34:10.0296382Z -- Building x64-linux-rel
2021-07-28T13:34:10.0297067Z -- Installing: /mnt/vcpkg-ci/packages/libzippp_x64-linux/share/libzippp/copyright
2021-07-28T13:34:10.0297719Z -- Performing post-build validation
2021-07-28T13:34:10.0298312Z -- Performing post-build validation done

So it looks like it fails because with the current changes, libzippp (which is my library, that I updated in #18553) do not compile. But with the log output, I'm unable to tell why (maybe related to the cmake configuration ?).

@JonLiu1993
Copy link
Member

@ctabin ,Is this PR being worked on?

@JonLiu1993
Copy link
Member

@dg0yt ,Do you have any advice?

@ctabin
Copy link
Contributor Author

ctabin commented Sep 22, 2021

@JonLiu1993 Unfortunately not. As said, I'm unable to provide more help here because I'm unable to see/understand the error (and also I'm not the author of libzip).

@dg0yt
Copy link
Contributor

dg0yt commented Sep 23, 2021

It is too late to look at ignition-fuel-tools4 CI build logs, and too much to do locally now. But it is clear that it needs proper exported configuration from libzip. So I did a build of libzip now. Inspecting the pc file indicates an issue wit zstd:

Libs:  -L"${libdir}" -lzip  -lbz2 -lZstd::Zstd -lz

Now looking for zstd in the exported cmake config indicates a missing find_dependency:

% grep -i zstd packages/libzip_x64-osx/share/libzip/libzip-*
packages/libzip_x64-osx/share/libzip/libzip-targets.cmake:  INTERFACE_LINK_LIBRARIES "\$<LINK_ONLY:BZip2::BZip2>;\$<LINK_ONLY:Zstd::Zstd>;\$<LINK_ONLY:ZLIB::ZLIB>"

This is just for a default build. Similar verification needs to be done for the optional features.

@JonLiu1993
Copy link
Member

@dg0yt ,Thanks for your investigation and suggestions

@JonLiu1993
Copy link
Member

@ctabin ,Have you verified according to @dg0yt's investigation and suggestions?

@JonLiu1993
Copy link
Member

@ctabin ,Is there any progress on this pr?

@ctabin
Copy link
Contributor Author

ctabin commented Oct 26, 2021

Hi @JonLiu1993, Unfortunately, no.
Since I'm not the author of libzip (only libzippp) and it seems the build has slightly changed, I'm unaware of the adaptations needed for vcpkg...
Maybe someone else should take over this PR to finish it properly.

@JonLiu1993
Copy link
Member

JonLiu1993 commented Oct 26, 2021

Hi @JonLiu1993, Unfortunately, no. Since I'm not the author of libzip (only libzippp) and it seems the build has slightly changed, I'm unaware of the adaptations needed for vcpkg... Maybe someone else should take over this PR to finish it properly.

@ctabin ,Thanks for your contribution,I temporarily close your pr, I will resubmit pr to update libzip port

@JonLiu1993 JonLiu1993 closed this Oct 26, 2021
@ctabin
Copy link
Contributor Author

ctabin commented Oct 26, 2021

@JonLiu1993 Thanks for your support.

@ctabin ctabin deleted the update-libzip branch May 23, 2022 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants