Skip to content

[libssh] Update to v0.11.2#46734

Closed
TimSC wants to merge 9 commits intomicrosoft:masterfrom
TimSC:updatelibssh
Closed

[libssh] Update to v0.11.2#46734
TimSC wants to merge 9 commits intomicrosoft:masterfrom
TimSC:updatelibssh

Conversation

@TimSC
Copy link

@TimSC TimSC commented Aug 3, 2025

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

I notice this didn't get merged (update to 0.11.1) #40896

@TimSC
Copy link
Author

TimSC commented Aug 3, 2025

@microsoft-github-policy-service agree

@TimSC TimSC changed the title Update libssh to v0.11.2 [libssh] Update to v0.11.2 Aug 3, 2025
@TimSC TimSC mentioned this pull request Aug 4, 2025
@BillyONeal
Copy link
Member

It looks like this changed libssh to link with the OpenSSL::Crypto target rather than copy-pasta-ing the individual libs, but it misses a find_dependency call necessary to make that work; the downstream new failures appear legitimate as a result.

image

@BillyONeal BillyONeal marked this pull request as draft August 4, 2025 22:27
@BillyONeal BillyONeal added the category:port-update The issue is with a library, which is requesting update new revision label Aug 4, 2025
@TimSC
Copy link
Author

TimSC commented Aug 5, 2025

During libssh:x64-windows-static build ffmpeg:x64-windows-static:

BUILD_ARCH=x86_64
OPTION_VARIABLE=OPTIONS_x86_64
=== CONFIGURING ===
ERROR: libssh >= 0.6.0 not found using pkg-config

Any ideas? I'm not that familiar with cmake.

@dg0yt
Copy link
Contributor

dg0yt commented Aug 5, 2025

During libssh:x64-windows-static build:

BUILD_ARCH=x86_64
OPTION_VARIABLE=OPTIONS_x86_64
=== CONFIGURING ===
ERROR: libssh >= 0.6.0 not found using pkg-config

I don't know why the port would expect to find libssh before the port installs libssh.
But it won't have the pkg-config program unless the portfile provides it and feeds into the build, e.g.

vcpkg_find_acquire_program(PKGCONFIG)
set(ENV{PKG_CONFIG} "${PKGCONFIG}")

@dg0yt
Copy link
Contributor

dg0yt commented Aug 5, 2025

https://gitlab.com/libssh/libssh-mirror/-/blob/master/CMakeLists.txt#L102

This should be changed to if(1) to install the pc file for all platforms.

@dg0yt
Copy link
Contributor

dg0yt commented Aug 5, 2025

All changes:
https://gitlab.com/libssh/libssh-mirror/-/compare/libssh-0.11.2...libssh-0.10.6?from_project_id=6055600

The pc file thing didn't even change 🤔

Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

Please keep the port concise.

@TimSC
Copy link
Author

TimSC commented Aug 5, 2025

@dg0yt I've attempted to address all your comments.

Do all the CI pipeline tests have to pass? I'm a little confused as to what might be causing errors.

@dg0yt
Copy link
Contributor

dg0yt commented Aug 6, 2025

CI has to pass.

The pc file stuff is still not complete. Don't add another pc file installation. Just make the existing one work everywhere.

@TimSC
Copy link
Author

TimSC commented Aug 6, 2025

That makes sense. I see the last update passes CI on all platforms and was authored by you @dg0yt #39734 In the avcpp build error (x64-windows-static) log:

-- Configuring done (10.0s)
CMake Error at D:/installed/x64-windows-static/share/libssh/libssh-config.cmake:64 (set_target_properties):
  The link interface of target "ssh" contains:

    OpenSSL::Crypto

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

I'm not sure how to diagnose the problem beyond that though. I'm not that familiar with cmake. Someone else might need to take on this PR to finish it.

@BillyONeal
Copy link
Member

I believe this change was caused by https://git.libssh.org/projects/libssh.git/commit/src/CMakeLists.txt?id=6ad455a8acfe6032c2a87cf83f2d20463c30f8af

/cc @gjasny @Jakuje

vcpkg always chooses the OpenSSL TLS provider so it's pretty easy to author a patch that just adds the missing find_dependency(OpenSSL CONFIG), but I imagine libssh's owners may want a solution that appropriately responds to which TLS provider is used.

I filed https://gitlab.com/libssh/libssh-mirror/-/issues/318

@dg0yt
Copy link
Contributor

dg0yt commented Aug 6, 2025

I'm not sure how to diagnose the problem beyond that though. I'm not that familiar with cmake. Someone else might need to take on this PR to finish it.

I guess I could finish the PR.

@BillyONeal
Copy link
Member

I missed that we were already patching in find_dependency(Threads, I just added the one for OpenSSL.

I don't really follow / understand the discussion above about the .pc file. As far as I can tell the installed .pc contents are not changed:

image

@BillyONeal
Copy link
Member

(I suppose it's also a problem that we are missing other find_dependency calls, for example, zlib is missing if libssh[zlib] is installed, but the only one detected in PR build seemed to be the OpenSSL one)

@dg0yt dg0yt mentioned this pull request Aug 6, 2025
@dg0yt
Copy link
Contributor

dg0yt commented Aug 6, 2025

about the .pc file.

It is not "the pc file". It is two of them when we need only one.

#46794

@dg0yt
Copy link
Contributor

dg0yt commented Aug 6, 2025

And they use pthread mostly without linking them ... how far can we get with this?

@TimSC
Copy link
Author

TimSC commented Aug 7, 2025

Thanks for looking into this guys!

@BillyONeal
Copy link
Member

Already included in #46794 . Thank you both!

@BillyONeal BillyONeal closed this Aug 7, 2025
@Jakuje
Copy link

Jakuje commented Aug 7, 2025

@BillyONeal thanks for the report! I commented in the gitlab issue. I see that there is also half a dozen of other patches here. If some of them can be upstreamed, I think it would be useful for everyone. For example the pkg-config is obviously also a windows thing (or at least for some configuration) so adjusting this upstream would be great:

 # pkg-config file
-if (UNIX OR MINGW)
+if (1)

I also see some android patches brought in by various people. Is there reason why they are not upstreamed?

@dg0yt
Copy link
Contributor

dg0yt commented Aug 7, 2025

I also see some android patches brought in by various people. Is there reason why they are not upstreamed?

This happens when they come as tiny drive-by fixes in a different context. Or/and when the port is already too outdated for immediate upstreaming. Or/and when the presence of the issue indicates the upstream doesn't "support" that platform or that build type (static library linkage). Or/and when the contributor gets tired ;-) Sometimes it is not an easy walk.

vcpkg:

  • hardly runs actual test-code,
  • has (only) static builds for non-Windows,
  • has multi-config installations for all platforms,
  • Tests downstream integration when updating a port,
  • has collected a fairly consistent set of pkgconfig modules and CMake packages.
  • use Github.

upstreams often

  • run in-build test code,
  • don't test static builds for non-Windows,
  • don't test multi-config installations,
  • don't test downstream integration,
  • face an uncontrolled variety of environment with absent or broken pkgconfig and CMake packages.
  • use different development platforms with different account requirements.

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.

4 participants