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

[vcpkg_find_acquire_program] Enable find_acquire(PKGCONFIG) #12626

Merged
merged 10 commits into from
Aug 18, 2020

Conversation

ras0219
Copy link
Contributor

@ras0219 ras0219 commented Jul 29, 2020

This PR enables vcpkg_find_acquire_program(PKGCONFIG) so we do not need to rely on msys2 for this common component.

This uses a Win32-native build of pkg-config, which handles Win32 paths without cygwin/msys conversion and uses semicolon-delimited lists in path variables.

Reduce default verbosity of vcpkg_fixup_pkgconfig
@Neumann-A
Copy link
Contributor

Can MS not open a repo called vcpkg-buildtools or similiar which hosts all the tools on github? Pkg-config can easily be built by vcpkg. See #11523

…program.cmake

Co-authored-by: Alexander Neumann <30894796+Neumann-A@users.noreply.github.com>
@JackBoosY JackBoosY added category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:internal This PR or Issue was filed by the vcpkg team. labels Jul 30, 2020
@MVoz
Copy link
Contributor

MVoz commented Jul 30, 2020

pkg-config https://packages.msys2.org/base/mingw-w64-pkg-config


2020-07-30_222108

2020-07-30_222146


https://repo.msys2.org/mingw/x86_64/mingw-w64-x86_64-pkg-config-0.29.2-1-any.pkg.tar.xz

pkg-config.exe --help
Usage:
  pkg-config.exe [OPTION...]

Help Options:
  -h, --help                              Show help options

Application Options:
  --version                               output version of pkg-config
  --modversion                            output version for package
  --atleast-pkgconfig-version=VERSION     require given version of pkg-config
  --libs                                  output all linker flags
  --static                                output linker flags for static linking
  --short-errors                          print short errors
  --libs-only-l                           output -l flags
  --libs-only-other                       output other libs (e.g. -pthread)
  --libs-only-L                           output -L flags
  --cflags                                output all pre-processor and compiler flags
  --cflags-only-I                         output -I flags
  --cflags-only-other                     output cflags not covered by the cflags-only-I option
  --variable=NAME                         get the value of variable named NAME
  --define-variable=NAME=VALUE            set variable NAME to VALUE
  --exists                                return 0 if the module(s) exist
  --print-variables                       output list of variables defined by the module
  --uninstalled                           return 0 if the uninstalled version of one or more module(s) or their dependencies will be used
  --atleast-version=VERSION               return 0 if the module is at least version VERSION
  --exact-version=VERSION                 return 0 if the module is at exactly version VERSION
  --max-version=VERSION                   return 0 if the module is at no newer than version VERSION
  --list-all                              list all known packages
  --debug                                 show verbose debug information
  --print-errors                          show verbose information about missing or conflicting packages (default unless --exists or --atleast/exact/max-version given on the command line)
  --silence-errors                        be silent about errors (default when --exists or --atleast/exact/max-version given on the command line)
  --errors-to-stdout                      print errors from --print-errors to stdout not stderr
  --print-provides                        print which packages the package provides
  --print-requires                        print which packages the package requires
  --print-requires-private                print which packages the package requires for static linking
  --validate                              validate a package's .pc file
  --define-prefix                         try to override the value of prefix for each .pc file found with a guesstimated value based on the location of the .pc file
  --dont-define-prefix                    don't try to override the value of prefix for each .pc file found with a guesstimated value based on the location of the .pc file
  --prefix-variable=PREFIX                set the name of the variable that pkg-config automatically sets
  --msvc-syntax                           output -l and -L flags for the Microsoft compiler (cl)

@MVoz
Copy link
Contributor

MVoz commented Aug 4, 2020

@Neumann-A
add to vcpkg_fixup_pkgconfig

vcpkg_acquire_msys(MSYS_ROOT PACKAGES pkg-config)
find_program(PKGCONFIG_EXECUTABLE NAMES pkg-config HINTS ${MSYS_ROOT} PATH_SUFFIXES "usr/bin" NO_DEFAULT_PATH)
mark_as_advanced(FORCE PKGCONFIG_EXECUTABLE)
set(PKGCONFIG ${PKGCONFIG_EXECUTABLE})

@ras0219
Copy link
Contributor Author

ras0219 commented Aug 10, 2020

Thanks for the link @voskrese!

Based on my testing, the mingw32 copy of pkg-config only requires one additional dep (libwinpthread). This should give us the best of both worlds: up-to-date pkg-config as well as freedom from pacman instabilities.

If this works, I believe this is a technique we can apply across the ecosystem to remove dependence on msys pacman.

@ras0219-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ras0219-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JackBoosY JackBoosY added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly and removed category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed labels Aug 13, 2020
@ras0219
Copy link
Contributor Author

ras0219 commented Aug 13, 2020

The lapack errors are unrelated; in CI:

-- The Fortran compiler identification is unknown
-- The C compiler identification is unknown
CMake Error at CMakeLists.txt:3 (project):
  The CMAKE_Fortran_COMPILER:

    D:/downloads/tools/msys2/msys64/mingw32/bin/gfortran.exe

  is not a full path to an existing compiler tool.

  Tell CMake where to find the compiler by setting either the environment
  variable "FC" or the CMake cache entry CMAKE_Fortran_COMPILER to the full
  path to the compiler, or to the compiler name if it is in the PATH.


CMake Error at CMakeLists.txt:3 (project):
  The CMAKE_C_COMPILER:

    D:/downloads/tools/msys2/msys64/mingw32/bin/gcc.exe

  is not a full path to an existing compiler tool.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.


-- Configuring incomplete, errors occurred!

when trying to repro locally, vcpkg selected ninja from my VS instance:

.... "-DCMAKE_MAKE_PROGRAM=C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/Ninja/ninja.exe" ...
CMake Error at CMakeLists.txt:3 (project):
  The Ninja generator does not support Fortran using Ninja version

    1.8.2

  due to lack of required features.  Ninja 1.10 or higher is required.


-- Configuring incomplete, errors occurred!

lapack-reference failures are the causes of x64-windows, x64-windows-static, x86-windows, and x64-uwp failing.

@@ -2,7 +2,7 @@ set(GEOGRAM_VERSION 1.7.5)

vcpkg_download_distfile(ARCHIVE
URLS "https://gforge.inria.fr/frs/download.php/file/38314/geogram_${GEOGRAM_VERSION}.tar.gz"
FILENAME "geogram_${GEOGRAM_VERSION}.tar.gz"
FILENAME "geogram_${GEOGRAM_VERSION}_47dcbb8.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

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

Why did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On one of the CI machines, there was a extant geogram_${GEOGRAM_VERSION}.tar.gz file with the previous version's hash.

Renaming the file resolved the conflict.

Copy link
Member

Choose a reason for hiding this comment

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

I see; it just looks odd to have that variable GEOGRAM_VERSION but this be a hardcoded constant. Not necessarily worth resetting CI over but it's confusing nonetheless

scripts/cmake/vcpkg_configure_meson.cmake Show resolved Hide resolved
set(PKGCONFIG $ENV{PKG_CONFIG} PARENT_SCOPE)
return()
elseif(CMAKE_HOST_WIN32)
set(PROG_PATH_SUBDIR "${DOWNLOADS}/tools/${PROGNAME}/0.29.2-1")
Copy link
Member

Choose a reason for hiding this comment

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

It seems very strange to have ${PROGNAME} but then a specific hardcoded version string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. I can change it, but that would invoke a total rebuild of the tree.

@BillyONeal BillyONeal merged commit bc88079 into microsoft:master Aug 18, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

remz1337 pushed a commit to remz1337/vcpkg that referenced this pull request Aug 23, 2020
…ft#12626)

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: Alexander Neumann <30894796+Neumann-A@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants