vcpkg [z_vcpkg_setup_pkgconfig_path] debug needs `share/pkgconfig`

Pass share/pkgconfig instead of debug/share/pkgconfig: The share location is for config-independent data. Integrate CURRENT_PACKAGES_DIR locations. Change the function interface to take a CONFIG parameter instead of prefix list (suggested below). Use z_vcpkg_setup_pkgconfig_path also for vcpkg_fixup_pkgconfig (which already had proper handling of CURRENT_PACKAGES_DIR and share). Remove redundant trailing slash. Extend unit test.

Fixes consuming wayland-protocols and similar.

Jan 12 '24 21:01 dg0yt

Fixes consuming wayland-protocols and similar.

Sorry for being thick headed on this, what do you mean by this?

Jan 17 '24 21:01 BillyONeal

Answering both questions at once:

While playing with mesa on linux, I forced the installation of wayland-protocols via the port instead of via the system.
wayland-protocols installs /share/pkgconfig/wayland-protocols.pc.
For the debug build of mesa, z_vcpkg_setup_pkgconfig_path is called with BASE_DIRS "${CURRENT_INSTALLED_DIR}/debug".
So the function used to add ${CURRENT_INSTALLED_DIR}/debug/share/pkgconfig. This location doesn't exist, and wayland-protocols.pc was not found.
With this PR, this particular invalid path is mapped to the known good path. The path must be added. There can't be continue, because this would still prevent wayland-protocols from being found.

Jan 18 '24 06:01 dg0yt

BTW, the function prepends, i.e. in the case of multiple base dirs, they would end up in reverse order. I didn't fix it because it didn't want to to discuss it...

Jan 18 '24 06:01 dg0yt

For the debug build of mesa, z_vcpkg_setup_pkgconfig_path is called with BASE_DIRS "${CURRENT_INSTALLED_DIR}/debug".

Why don't we fix that rather than trying to second guess what the caller said?

Jan 19 '24 21:01 BillyONeal

For the debug build of mesa, z_vcpkg_setup_pkgconfig_path is called with BASE_DIRS "${CURRENT_INSTALLED_DIR}/debug".

Why don't we fix that rather than trying to second guess what the caller said?

The caller does it right, it doesn't need to be fixed. However, with vcpkg's atypical installation layout mapping debug/share to share, ${CURRENT_INSTALLED_DIR}/share/pkgconfig is the actual implementation of ${CURRENT_INSTALLED_DIR}/debug/share/pkgconfig, and the substitution of this special value in the single called function is the appropriate solution, not changing the indeterminate number of callers.

Jan 19 '24 21:01 dg0yt

the substitution of this special value in the single called function is the appropriate solution, not changing the indeterminate number of callers.

I kind of disagree with that. If callers aren't passing a value, and we are supplying default values, then sure, party hard. But callers are not doing that, they asked for a specific value, and a function shouldn't be lying to the caller.

with vcpkg's atypical installation layout mapping debug/share to share,

If vcpkg is doing something funny with where the .pcs are located in a funny place, why aren't we changing the other place in vcpkg calling the x_'d function to do the right thing?

Jan 19 '24 22:01 BillyONeal

There are exactly 2 callers doing:

        if ("${current_buildtype}" STREQUAL "DEBUG")
            z_vcpkg_setup_pkgconfig_path(BASE_DIRS "${CURRENT_INSTALLED_DIR}/debug")
        else()
            z_vcpkg_setup_pkgconfig_path(BASE_DIRS "${CURRENT_INSTALLED_DIR}")
        endif()

one of which is vcpkg_configure_make and the other of which is vcpkg_configure_meson. I think the right fix is to change that block in both of those to just

z_vcpkg_setup_pkgconfig_path(BASE_DIRS "${CURRENT_INSTALLED_DIR}")

Jan 19 '24 22:01 BillyONeal

The caller does it right, it doesn't need to be fixed.

I'm confused by this statement. Isn't this change undoing what vcpkg_configure_make and vcpkg_configure_meson are trying to do?

Jan 20 '24 00:01 BillyONeal

I'm confused by this statement. Isn't this change undoing what vcpkg_configure_make and vcpkg_configure_meson are trying to do?

No the idea is that /share/pkgconfig/ is independent on configuration. As such it should always be in CURRENT_INSTALLED_DIR/share/pkgconfig. This also applies to CURRENT_PACKAGES_DIR which this PR does not fix.

Jan 20 '24 00:01 Neumann-A

And yes z_vcpkg_setup_pkgconfig_path(BASE_DIRS "${CURRENT_INSTALLED_DIR}") is terrible. It should have been z_vcpkg_setup_pkgconfig_path(CONFIG <config>) and abstracted away passing in any paths at all.

Jan 20 '24 01:01 Neumann-A

And yes z_vcpkg_setup_pkgconfig_path(BASE_DIRS "${CURRENT_INSTALLED_DIR}") is terrible. It should have been z_vcpkg_setup_pkgconfig_path(CONFIG <config>) and abstracted away passing in any paths at all.

This is a z_ thing so that change can be made.

Jan 20 '24 01:01 BillyONeal

Ping for review @BillyONeal.

Feb 16 '24 20:02 dg0yt

@Neumann-A Are you happy with this?

Feb 17 '24 03:02 BillyONeal