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

[pkg-config] new port #17487

Closed
wants to merge 4 commits into from
Closed

Conversation

Neumann-A
Copy link
Contributor

revived from an old PR since host tools are now a thing

@JackBoosY JackBoosY self-assigned this Apr 25, 2021
@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Apr 25, 2021
@dg0yt
Copy link
Contributor

dg0yt commented Apr 25, 2021

I wonder how if this port might interact with cmake_find_acquire_program(PKGCONFIG) when building a port with a complex set of dependencies. Will all dependencies use the same pkg-config binary? If not, will the different pkg-config binaries give the same results? (In particular, how will pkg-config from this port handle system directories?)

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Apr 25, 2021
@Neumann-A
Copy link
Contributor Author

vcpkg_find_aquire_program does not look into the installed tree by default and thus there is no interaction. Since pkg-config is just used to discover libs/flags via standarized pc files the output should always be the same unless there is a bug. For system deps pkg-config probably uses the default behavior or relies on env variables.

@dg0yt
Copy link
Contributor

dg0yt commented Apr 25, 2021

Well, ports shouldn't depend on discovered pc files files anyway, so differences in discovered .pc file should not occur when dependencies are controlled properly.

TBH I'm surprised that the port is so straigtforward. Does this apply to using this port, too?

  • Does it handle vcpkg's debug vs. release build types?
  • Does it work for cross-compiling (Android, iOS, Mingw)?
    (This is were pkg-config really helps.)

Was this tested in some real project?
(I really miss tests in some areas of vcpkg, also as a synopsis of how things are meant to be used.)

Maybe it only needs some README how to use it correctly, unless it is really just a plain port dependency.

@Neumann-A
Copy link
Contributor Author

@dg0yt The real challenge is to bootstrap glib without pkg-config after #13100 is merged. Meson does not make it that simple.

Does it handle vcpkg's debug vs. release build types?

You need to setup PKG_CONFIG_PATH for this like vcpkg_configure_make|meson|qmake does it. In CMake this is done automatically via CMAKE_PREFIX_PATH and the vcpkg toolchain (Unless CMake paths are explicitly disabled for FindPkgConfig).

Was this tested in some real project?

No but there is no magic here. It is just the pkg-config executable build using vcpkg. In the future vcpkg might want to fallback to this as a host-port after script stabilization. Then the real challenge lies in bootstrapping the port and its dependencies without using pkg-config.

@ras0219-msft @strega-nil: If you intend to merge this one please run a CI test run where this is installed first. Otherwise CMake might pick up a lot of optional dependencies in ports which can suddenly be found due to FindPkgConfig actually working on windows.

@talregev
Copy link
Contributor

vote up

@dg0yt
Copy link
Contributor

dg0yt commented Apr 28, 2021

No but there is no magic here. It is just the pkg-config executable build using vcpkg. In the future vcpkg might want to fallback to this as a host-port after script stabilization.

I think pkg-config must be built for the host because it needs to run on the host. And then you will also have to set PKG_CONFIG_LIBDIR to stop pkg-config from looking up .pc files in default (host) directories. This is often done in wrapper scripts which come as <autotools_triplet>-pkg-config.

@BillyONeal
Copy link
Member

Should we replace uses of vcpkg_find_acquire_program with this?

@Neumann-A
Copy link
Contributor Author

Should we replace uses of vcpkg_find_acquire_program with this?

requires building of glib without pkg-config. Can be done, but is a bit more complicated.

@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Apr 28, 2021
@BillyONeal
Copy link
Member

OK, then 99% sure this is OK but want to ask a few folks to confirm

@dg0yt
Copy link
Contributor

dg0yt commented Apr 29, 2021

requires building of glib without pkg-config. Can be done, but is a bit more complicated.

Would it be easier when building --with-internal-glib? pkg-config being a tool not a library, the embedded copy seems more acceptable for bootstrapping purposes.

@ras0219-msft
Copy link
Contributor

Would it be easier when building --with-internal-glib? pkg-config being a tool not a library, the embedded copy seems more acceptable for bootstrapping purposes.

Agreed, I think this is the appropriate approach here.

And then you will also have to set PKG_CONFIG_LIBDIR to stop pkg-config from looking up .pc files in default (host) directories.

Unfortunately, it's not quite as clean as that. Most users build targeting their host machines and there are some things that we need to get from there (C library, drivers, core OS components). Users that want to opt-in to 100% host isolation can set(ENV{PKG_CONFIG_LIBDIR} ...) in the triplet file.

@Neumann-A
Copy link
Contributor Author

I think we could bootstrap glib with pkgconf instead or generally use pkgconf instead of pkg-config. pkgconf should be a drop in replacement for pkg-config.
Otherwise bootstrapping glib could be done using override_dependency (mesonbuild/meson#8224 (comment)) together with declare_dependency

@strega-nil-ms
Copy link
Contributor

strega-nil-ms commented Apr 29, 2021

Since pkgconf seems like a strictly better pkg-config, is there a reason to merge both of these tools?

Noting that we would really like to switch to using host ports instead of msys for pkg-config.

@Neumann-A
Copy link
Contributor Author

Since pkgconf seems like a strictly better pkg-config

Any real proof for that instead of people just saying it?

@strega-nil
Copy link
Contributor

@Neumann-A I mean if you have opinions to the contrary, I'd love to hear them; I think we still need msys2 for autoconf on windows, so I'd like to avoid it, and I'd also like for us to standardize upon exactly one pkg-config solution.

@BillyONeal
Copy link
Member

I think we still need msys2 for autoconf on windows

There are lots of ports that need only pkgconfig though.

@talregev
Copy link
Contributor

There are lots of ports that need only pkgconfig though.

Why we can't have both and let the user decide what to install with vcpkg?
Our opensource project is now using pkg-config to compile curl on linux and android with vcpkg.
I can imagine that there will be a user that will want to compile that on a linux base os that there isn't pkg-config
It really help that user to install it right away from vcpkg.

@dg0yt
Copy link
Contributor

dg0yt commented Apr 30, 2021

MSYS2 switch to pkgconf in November 2020, msys2/MINGW-packages#7263

we still need msys2 for autoconf on windows

Somewhat OT for this PR, but how many ports do really need to run autoconf when using official tarballs?

@talregev
Copy link
Contributor

@Neumann-A @dg0yt Can you vote here?

@JackBoosY
Copy link
Contributor

I think it's good, but I should wait for the consent of other members.

@BillyONeal
Copy link
Member

OK, then 99% sure this is OK but want to ask a few folks to confirm

I understand now: pkg-config requires a bunch of msys things anyway so that wouldn't actually fix anything.

If we want a better non-msys provider for this, it seems like it would be better to use the existing port pkgconf.

If we don't intend to replace the vcpkg_find_acquire_program uses, do we have a specific library use case here (given that vcpkg does not intend to provide standalone tools)?

@BillyONeal BillyONeal added requires:author-response and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Sep 16, 2021
@dg0yt
Copy link
Contributor

dg0yt commented Sep 16, 2021

AFAIU pkgconf should work well enough. A few days ago I was trying to create a port to leverage pkgconf instead of msys pkgconfig, in order to test its usability and fix the lack of generalized pkgconfig setup which also includes vcpkg_cmake_configure. (That's how I found the meson mingw issue BTW.) It could work well for classic mode. However, I miss a way to safely detect the host triplet / host tool path in manifest mode. Any ideas? The point is that ENV{PKG_CONFIG} needs to point to the host tool path. Butvcpkg-cmake-config.cmake is not loaded in manifest mode...

@Neumann-A
Copy link
Contributor Author

all vcpkg(_*)_configure(_*) functions should have an option NO_PKG_CONFIG which disables PKG_CONFIG lookup via vcpkg_find_acquire_program and the setup of PKG_CONFIG_PATH. This way pkgconf could easily be bootstrapped. Doing that with pkg-config is a bit more complex but also possible (requires manual setting of the appropriate lib variables). After that vcpkg could add CURRENT_HOST_INSTALLED_DIR to find_program in vcpkg_find_acquire_program and use the internally build version of pkgconf.

@dg0yt
Copy link
Contributor

dg0yt commented Sep 17, 2021

The major issues at the moment are:

  • no pkgconfig support in cmake maintainer scripts
  • no pkgconfig support for user projects. (Actually no substitute at all for vcpkg_find_acquire_program.)

And the second point (or generally finding tools) is hard to fix without knowing host installation path.

@JackBoosY
Copy link
Contributor

I have tried to add pkgconfig to the vcpkg toolchain but failed: the path to find the pc file is not recognized.

@dg0yt
Copy link
Contributor

dg0yt commented Sep 17, 2021

I have tried to add pkgconfig to the vcpkg toolchain but failed: the path to find the pc file is not recognized.

Did you also set the search path variables, as done e.g. in vcpkg_configure_make? That's my point: It is solved in some build helpers, but not for cmake.

@JackBoosY
Copy link
Contributor

I have tried to add pkgconfig to the vcpkg toolchain but failed: the path to find the pc file is not recognized.

Did you also set the search path variables, as done e.g. in vcpkg_configure_make? That's my point: It is solved in some build helpers, but not for cmake.

I wish to automatically use the pkgconfig installed by vcpkg and add lib/pkgconfig / debug/lib/pkgconfig to the search path.
There are two ways to add pc file path in cmake:

  1. Set environment variables
  2. Set a variable of cmake

Both them are failed.

@dg0yt
Copy link
Contributor

dg0yt commented Sep 17, 2021

I wish to automatically use the pkgconfig installed by vcpkg and add lib/pkgconfig / debug/lib/pkgconfig to the search path.
There are two ways to add pc file path in cmake:

1. Set environment variables

2. Set a variable of cmake

Both them are failed.

Okay. I will try again. We will find a way to share the WIP and review.

In any case, we need to provide the host install path to cmake user projects, for the benefit of all types of host tool dependencies. I would like to hear opinions before jumping into implementations.

@JackBoosY
Copy link
Contributor

@dg0yt After that, I can make a new feature to vcpkg to automaticly print the pkgconfig usage like cmake.

@Neumann-A
Copy link
Contributor Author

cmake 3.22 will support pkgconf my PR for that is already merged. setup of PKG_CONFIG_PATH for cmake is not required. Important: FindPkgConfig only works for single config generators unless we fix it and make it more optimized for vcpkg. (meaning less calls to pkgconf.)

@Neumann-A
Copy link
Contributor Author

the following ports alreday use FindPkgConfig:

Suche "PKG_CONFIG_EXECUTABLE" (8 Treffer in 8 Dateien von 6408 gesucht)
  E:\vcpkg_folders\master\ports\cpprestsdk\portfile.cmake (1 Treffer)
	Zeile 36:         -DPKG_CONFIG_EXECUTABLE=FALSE
  E:\vcpkg_folders\master\ports\fluidsynth\portfile.cmake (1 Treffer)
	Zeile 22:         -DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}
  E:\vcpkg_folders\master\ports\gmime\portfile.cmake (1 Treffer)
	Zeile 29:         -DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}
  E:\vcpkg_folders\master\ports\gts\portfile.cmake (1 Treffer)
	Zeile 19:         -DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}
  E:\vcpkg_folders\master\ports\libcroco\portfile.cmake (1 Treffer)
	Zeile 22:         -DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}
  E:\vcpkg_folders\master\ports\libheif\gdk-pixbuf.patch (1 Treffer)
	Zeile 13: -    execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE} gdk-pixbuf-2.0 --variable gdk_pixbuf_moduledir --define-variable=prefix=${CMAKE_INSTALL_PREFIX} OUTPUT_VARIABLE GDKPIXBUF2_MODULE_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
  E:\vcpkg_folders\master\ports\libnice\portfile.cmake (1 Treffer)
	Zeile 19:         -DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}
  E:\vcpkg_folders\master\ports\librsvg\portfile.cmake (1 Treffer)
	Zeile 22:         -DPKG_CONFIG_EXECUTABLE=${PKGCONFIG}

@dg0yt
Copy link
Contributor

dg0yt commented Sep 20, 2021

Fun fact: The Perl installation added by vcpkg on Windows comes with a perl-only implementation of pkg-config and pkg-config.bat. So when the path of perl.exe is added to ENV{PATH}, even cmake on Windows will find pkg-config...

@JackBoosY JackBoosY added depends:different-pr This PR or Issue depends on a PR which has been filed and removed requires:author-response labels Nov 5, 2021
@JackBoosY
Copy link
Contributor

Will continue review this PR untill cmake update to 3.22 or later.

@BillyONeal
Copy link
Member

Will continue review this PR untill cmake update to 3.22 or later.

Note that there are some good reasons to avoid updating the cmake version; going forward, that breaks Alpine: #21218

@JackBoosY
Copy link
Contributor

Depends on #21456.

@SamuelMarks
Copy link
Contributor

[offtopic]
If we go crazy on the pkg-config side we can contribute FindPackageName.cmake files for most all that vcpkg proffers; back to CMake itself, which could enable successful configuration of C/C++ projects from dependencies installed on the system or via a package manager like vcpkg. E.g., use vcpkg for dev and OS for prod; or vcpkg for Windows and OS for macOS & Linux.
[/offtopic]

@JackBoosY
Copy link
Contributor

pkgconfig is gradually becoming a way to use packages provided by cmake, so we should provide these methods like cmake.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 21, 2022

we can contribute FindPackageName.cmake files for most all that vcpkg proffers

Please don't. There is no benefit over using FindPkgConfig.cmake, but there are all downsides of Find modules.

@Neumann-A Neumann-A marked this pull request as draft February 22, 2022 22:00
@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Feb 28, 2022
@Neumann-A
Copy link
Contributor Author

@JackBoosY This PR really never dependent on the cmake update to 3.22. the pkgconf PR did which is why the CMake update was an issues.

@Neumann-A Neumann-A closed this Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants