[vcpkg] Don't override pkg-config prefix #17205

dg0yt · 2021-04-10T19:51:30Z

Describe the pull request

What does your PR fix?
This PR fixes breaking builds when pkg-config picks up .pc files from system directories: Overriding the prefix will invalidate include and lib paths.
Example case: gdal, picking up system OpenEXR in Azure Pipelines' Ubuntu image.
Which triplets are supported/not supported? Have you updated the CI baseline?
-/-
Does your PR follow the maintainer guide?
-/-

.pc file from vcpkg ports should be safe when properly using $(pcfiledir) and vcpkg_fixup_pkgconfig.
Related discussion: #10402 (comment)

dg0yt · 2021-04-11T05:25:42Z

First round of breaking ports:

x64-windows-static, x86-windows, x64-windows

fontconfig: error in config step:

configure: error: Package requirements (freetype2 >= 21.0.15) were not met:

No package 'freetype2' found

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

Alternatively, you may set the environment variables FREETYPE_CFLAGS
and FREETYPE_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.

x64-linux

pangolin: error in build step

/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_monitor_unref'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_enumerate_scan_devices'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_new'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_list_entry_get_name'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_enumerate_get_list_entry'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_list_entry_get_next'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_device_new_from_syspath'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_monitor_receive_device'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_device_get_sysname'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_device_get_action'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_unref'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_monitor_enable_receiving'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_monitor_filter_add_match_subsystem_devtype'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_monitor_get_fd'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_device_get_devnode'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_enumerate_unref'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_enumerate_new'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_enumerate_add_match_property'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_monitor_new_from_netlink'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_device_unref'
/mnt/vcpkg-ci/installed/x64-linux/debug/lib/libuvc.so: undefined reference to `udev_enumerate_add_match_subsystem'
collect2: error: ld returned 1 exit status

x64-osx

libidn2: error in autoconf step

Can't exec "gtkdocize": No such file or directory at /usr/local/Cellar/autoconf/2.71/share/autoconf/Autom4te/FileUtils.pm line 293.

libtasn1: error in autoconf step

Can't exec "gtkdocize": No such file or directory at /usr/local/Cellar/autoconf/2.71/share/autoconf/Autom4te/FileUtils.pm line 293.

x264: error in install step

install: mkdir /Users/vagrant/Data/packages/x264_x64-osx/Users/vagrant/Data/installed: File exists
make: *** [install-cli] Error 71

I will investigae step by step in my personal pipeline.

Neumann-A · 2021-04-11T07:55:43Z

pangolin

looks like a hidden dependency somewhere

libidn2, libtasn1

requires additional brew package. See #17073 (comment)

fontconfig: error in config step:

Strange since freetype installs pc files.

dg0yt · 2021-04-11T08:00:51Z

Strange since freetype installs pc files.

Indeed. A local vcpkg install fontconfig on Ubuntu 18.04 worked as expected, picking the vcpkg freetype via pkgconfig.

dg0yt · 2021-04-11T08:10:12Z

"brew install gtk-doc"

Maybe vcpkg_configure_make needs to do a vcpkg_find_acquire_program for autoconfig which in turn proposes

brew install autoconf automake gtk-doc

(On Azure pipelines Ubuntu 18.04, vcpkg proposed to apt-get install gperf on Linux. And there is an gperf port in vcpkg.)

dg0yt · 2021-04-13T18:33:31Z

Comparing PKG_CONFIG_PATH in fontconfig config.log and in plain MSYS2 MinGW64 shell, I came to the conclusion that even on Windows, this should use : as separator, which in turn implies unixish directory paths. I use cygpath here, as is done in bootstrap and in some ports.

dg0yt · 2021-04-14T05:13:07Z

linux: pangolin, unrelated but I could help to solve it ([vcpkg baseline][pangolin] Set CMAKE_DISABLE_FIND_PACKAGE_XX to ON #17250).
x64-windows-static: Download errors, unrelated.
x86-windows: Download errors, unrelated, plus:
- paraview:
  - first build stops without at step 5678/8375, shortly after
    LINK : fatal error LNK1201: error writing to program database 'D:\buildtrees\paraview\x86-windows-dbg\bin\vtkPVVTKExtensionsIOImage-pv5.8d.pdb'; check for insufficient disk space, invalid path, or insufficient privilege
  - build restarts in non-parallel mode due to "out-of-memory", but fails quickly again, at 32/2698:
    LINK : fatal error LNK1201: error writing to program database 'D:\buildtrees\paraview\x86-windows-dbg\bin\vtkPVVTKExtensionsIOImage-pv5.8d.pdb'; check for insufficient disk space, invalid path, or insufficient privilege ninja: build stopped: subcommand failed.
  Well, paraview has large fan-in (Qt, gdal) so it might be a side effect of some other package's interplay with pkg-config. Unfortunately I don't have a config log from a previuous build of paraview so I cannot see if there are new features enabled now.
  And it builds with dynamic linkage. I have no way to deal with this locally. I can only ask questions:
- Does the static build create significantly larger files (in particular debugging information)?
- Is it somewhat known to be flaky in CI, i.e. might it go away in another run?
- Is there a way to prevent the pointless seond run?
- Is there a way to reduce disk space requirements, or improve capacity?

dg0yt · 2021-04-14T06:07:42Z

On the download errors:
In my personal CI setup on Azure Pipelines, there is a --download-only stage before the actual build stage, passing the artifacts via cache. This helps with consistency, and it is more polite to the providers of the downloads. (Of course this might be too much for all vcpkg.)

dg0yt · 2021-04-22T06:34:37Z

Comparing PKG_CONFIG_PATH in fontconfig config.log and in plain MSYS2 MinGW64 shell, I came to the conclusion that even on Windows, this should use : as separator, which in turn implies unixish directory paths. I use cygpath here, as is done in bootstrap and in some ports.

I have to take this back. Pkg-config documentation and source code say that ; is the separator for Windows, and MSYS2 doesn't change this by patches. However, in MSYS2 bash, PKG_CONFIG_PATH uses :. I assume this works because of the implicit transformations which are applied when calling native windows programs such as /mingw62/bin/pkg-config.exe from MSYS2 programs such as bash. Note the difference:

$ /usr/bin/echo PKG_CONFIG_PATH=$PKG_CONFIG_PATH
PKG_CONFIG_PATH=/mingw64/lib/pkgconfig:/mingw64/share/pkgconfig

$ /mingw64/bin/cmake -E environment | grep PKG_CONFIG_PATH
PKG_CONFIG_PATH=D:\msys32\mingw64\lib\pkgconfig;D:\msys32\mingw64\share\pkgconfig

For the record, pkg-config --help will indicate if it is a native windows program: Only in this case it lists --msvc-syntax.

So I need to go one step back and find the actual cause for the fontconfig issue.

dg0yt · 2021-04-22T08:45:32Z

scripts/cmake/vcpkg_configure_make.cmake

@@ -721,7 +741,6 @@ function(vcpkg_configure_make)
            set(_LINK_CONFIG_BACKUP "$ENV{_LINK_}")
            set(ENV{_LINK_} "${LINK_ENV_${_VAR_SUFFIX}}")
        endif()
-        set(ENV{PKG_CONFIG} "${PKGCONFIG} --define-variable=prefix=${_VCPKG_INSTALLED}${PATH_SUFFIX_${_buildtype}}")


Oh, this was setting an environment variable. With this removed, pkg-config is taken from PATH, and this may lead to the MSYS version which doesn't understand the Windows path separator :.

dg0yt · 2021-04-23T05:17:06Z

Build errors unrelated to PR.

gtk:x64-windows: Download error
libidn2:x64-osx: [libidn2] build failure on osx pipeline #17057
libtasn1:x64-osx: [libtasn1] build failure in osx pipeline #17098

dg0yt · 2021-04-25T07:25:46Z

Please re-run. No longer blocked by libidn2/libtasn1.

dg0yt · 2021-05-03T05:18:18Z

New build, new mix of build failures....

triplet	package	comment
x64-windows-static	skia	failed in git fetch step
x64-osx	libmicrohttpd	failed in build step
x64-linux	ncurses	failed in download_distfile step
x64-linux	ncurses	cascaded due to missing dependencies
x64-linux	v8	unrelated: failed in git fetch step
x64-windows	?	still building
x86-windows	?	still building

dg0yt · 2021-05-03T06:45:34Z

x64-osx:libmicrohttpd
Build errors during linking:

Undefined symbols for architecture x86_64:
  "_SecCopyErrorMessageString", referenced from:
      _osstatus_error in libgnutls.a(certs.o)
  "_SecTrustSettingsCopyCertificates", referenced from:
      _add_system_trust in libgnutls.a(certs.o)
  "_SecItemExport", referenced from:
      _add_system_trust in libgnutls.a(certs.o)

and similar.
SecCopyErrorMessageString is part of the Security framework on macOS. How did it become needed now?

The configuration of libmicrohttpd tests for dependencies, but the port doesn't declare any dependencies.

curl
gettext
libgnutls

On macOS, it could pick up files provided either from brew or from a port if it is installed. The mixing of brew and overridden prefix creates volatile configurations.

Changed debug build configuration:

Failed build (debug):

pkg_cv_GNUTLS_CFLAGS=-I/Users/vagrant/Data/installed/x64-osx/debug/lib/pkgconfig/../../../include
pkg_cv_GNUTLS_LIBS='-L/Users/vagrant/Data/installed/x64-osx/debug/lib/pkgconfig/../../lib -lgnutls -lintl -liconv -Wl,-framework -Wl,CoreFoundation -lgmp -lhogweed -lgmp -lnettle -ltasn1 -lidn2 -liconv'

Original build (debug):

pkg_cv_GNUTLS_CFLAGS='-I/Users/vagrant/Data/installed/x64-osx/debug/include -I/Users/vagrant/Data/installed/x64-osx/debug/include/p11-kit-1'
pkg_cv_GNUTLS_LIBS='-L/Users/vagrant/Data/installed/x64-osx/debug/lib -lgnutls -lintl -Wl,-framework -Wl,CoreFoundation -lgmp -lunistring -lhogweed -lgmp -lnettle -ltasn1 -lidn2 -liconv -lunistring -lp11-kit'

Changed release build configuration:

Failed build (release):

pkg_cv_GNUTLS_CFLAGS=-I/Users/vagrant/Data/installed/x64-osx/lib/pkgconfig/../../include
pkg_cv_GNUTLS_LIBS='-L/Users/vagrant/Data/installed/x64-osx/lib/pkgconfig/../../lib -lgnutls -lintl -liconv -Wl,-framework -Wl,CoreFoundation -lgmp -lhogweed -lgmp -lnettle -ltasn1 -lidn2 -liconv'

Build (release) before this change:

pkg_cv_GNUTLS_CFLAGS=-I/Users/vagrant/Data/installed/x64-osx/include/p11-kit-1
pkg_cv_GNUTLS_LIBS='-L/Users/vagrant/Data/installed/x64-osx/lib -lgnutls -lintl -Wl,-framework -Wl,CoreFoundation -lgmp -lunistring -lhogweed -lgmp -lnettle -ltasn1 -lidn2 -liconv -lunistring -lp11-kit'

The transition from /Users/vagrant/Data/installed/x64-osx[/debug] to /Users/vagrant/Data/installed/x64-osx/lib/pkgconfig[/debug/..]/../.. is expected: With this PR, the prefix is really taken from the .pc file, not forced by an external parameter.

dg0yt · 2021-05-08T10:24:01Z

Collection of ports that are known to have pc files which contain an invalid prefix (not yet passed through vcpkg_fixup_pkgconfig):

gdal (broken anyway on Linux, but the root of my current efforts)
geos (will be added to [geos] Relocatable geos-config; dynamic builds on Unix #17616)
hdf5 (included in [szip, hdf5] Fix mingw import lib names, control linkage #17941)
netcdf-c (tbd, depends on hdf5)

PhoebeHui

LGTM

strega-nil-ms · 2021-05-28T21:46:31Z

@dg0yt I would like to understand what this difference means; or more accurately, what the original thing was trying to fix. Is there a reason it was done this way, and why does it no longer have to be done that way?

Neumann-A · 2021-05-28T21:59:23Z

@strega-nil-ms: Just merge it. the define-prefix was necessary because there was a time when vcpkg_fixup_pkgconfig was removing the prefix variable which was before it was found how to make it relocatable via pcfiledir. Once upon a time vcpkg_configure_meson also had the same line of code but this was already removed.

dg0yt · 2021-05-29T08:17:55Z

@strega-nil-ms You cannot really ask me what the original thing was trying to fix because I didn't add it. This PR is my way of asking that question to the vcpkg maintainers, in the light of the side effects it causes. So I can give a summary of what I took out of the feedback and from additional digging in the repository history:

The original thing was an early attempt to fix non-relocatable pc files.
This is no longer needed for proper ports because frequent fixes are automated in vcpkg_fixup_pkgconfig, and maintainers need to take care of additional fixes anyway.
There might have been an expectation that builds would not silently pick system packages.
But IMO the gdal situation in linux ([GDAL] On Linux completly broken #9068) can be taken as an indication that maintainers should not rely on this side effect. Apart from that, some developers might actually wish to use system packages at least temporarily.

strega-nil-ms · 2021-06-03T21:55:47Z

Ah, okay, thanks y'all!

Neumann-A approved these changes Apr 10, 2021

View reviewed changes

JonLiu1993 assigned PhoebeHui Apr 12, 2021

PhoebeHui changed the title ~~Don't override pkg-config prefix~~ [vcpkg] Don't override pkg-config prefix Apr 12, 2021

This was referenced Apr 12, 2021

[vcpkg] Add missing autoconf dependencies to osx provisioning #17227

Closed

[pangolin] build failure on linux #17005

Closed

dg0yt mentioned this pull request Apr 14, 2021

Add missing autoconf dependencies to Linux provisioning #17278

Closed

dg0yt commented Apr 22, 2021

View reviewed changes

Don't override pkg-config prefix

677d584

dg0yt force-pushed the pkg-config branch from 70fa23c to 677d584 Compare April 22, 2021 08:49

dg0yt marked this pull request as ready for review April 23, 2021 05:17

This was referenced May 3, 2021

[libmicrohttpd] Port doesn't declare its dependencies #17636

Closed

[GDAL] On Linux completly broken #9068

Closed

gdal build failure #16271

Closed

dg0yt mentioned this pull request May 9, 2021

[vcpkg] Many ports: Add vcpkg_fixup_pkgconfig() #11087

Closed

Rebuild

da949f9

dg0yt force-pushed the pkg-config branch from 82fff64 to da949f9 Compare May 24, 2021 19:29

PhoebeHui approved these changes May 27, 2021

View reviewed changes

PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label May 27, 2021

PhoebeHui requested a review from strega-nil-ms May 27, 2021 10:23

strega-nil-ms added the requires:author-response label May 28, 2021

PhoebeHui removed the requires:author-response label May 31, 2021

strega-nil-ms merged commit 4386dda into microsoft:master Jun 3, 2021

dg0yt deleted the pkg-config branch June 4, 2021 04:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vcpkg] Don't override pkg-config prefix #17205

[vcpkg] Don't override pkg-config prefix #17205

dg0yt commented Apr 10, 2021

dg0yt commented Apr 11, 2021

Neumann-A commented Apr 11, 2021

dg0yt commented Apr 11, 2021

dg0yt commented Apr 11, 2021

dg0yt commented Apr 13, 2021

dg0yt commented Apr 14, 2021

dg0yt commented Apr 14, 2021

dg0yt commented Apr 22, 2021

dg0yt Apr 22, 2021

dg0yt commented Apr 23, 2021

dg0yt commented Apr 25, 2021

dg0yt commented May 3, 2021

dg0yt commented May 3, 2021

dg0yt commented May 8, 2021 •

edited

Loading

PhoebeHui left a comment

strega-nil-ms commented May 28, 2021

Neumann-A commented May 28, 2021

dg0yt commented May 29, 2021

strega-nil-ms commented Jun 3, 2021

[vcpkg] Don't override pkg-config prefix #17205

[vcpkg] Don't override pkg-config prefix #17205

Conversation

dg0yt commented Apr 10, 2021

dg0yt commented Apr 11, 2021

x64-windows-static, x86-windows, x64-windows

x64-linux

x64-osx

Neumann-A commented Apr 11, 2021

dg0yt commented Apr 11, 2021

dg0yt commented Apr 11, 2021

dg0yt commented Apr 13, 2021

dg0yt commented Apr 14, 2021

dg0yt commented Apr 14, 2021

dg0yt commented Apr 22, 2021

dg0yt Apr 22, 2021

Choose a reason for hiding this comment

dg0yt commented Apr 23, 2021

dg0yt commented Apr 25, 2021

dg0yt commented May 3, 2021

dg0yt commented May 3, 2021

dg0yt commented May 8, 2021 • edited Loading

PhoebeHui left a comment

Choose a reason for hiding this comment

strega-nil-ms commented May 28, 2021

Neumann-A commented May 28, 2021

dg0yt commented May 29, 2021

strega-nil-ms commented Jun 3, 2021

dg0yt commented May 8, 2021 •

edited

Loading