Main icon indicating copy to clipboard operation
Main copied to clipboard

libxml2: Set environment variables

Open brian6932 opened this issue 5 months ago • 7 comments

https://cmake.org/cmake/help/latest/module/FindLibXml2.html

  • [x] Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • [x] I have read the Contributing Guide

brian6932 avatar Aug 02 '25 06:08 brian6932

All changes look good.

Wait for review from human collaborators.

libxml2

  • [x] Description
  • [x] License
  • [x] Hashes

github-actions[bot] avatar Aug 02 '25 06:08 github-actions[bot]

I’d avoid setting LIBXML2_INCLUDE_DIR/LIBXML2_LIBRARY via env_set. Those are CMake cache variables, not broadly respected environment variables. A safer approach is to add notes that show users how to configure their own package manager like CMake, Meson, MSBuild etc.

Lutra-Fs avatar Aug 13 '25 01:08 Lutra-Fs

Those are CMake cache variables, not broadly respected environment variables.

What's the point of installing the package if you don't want to set the environment variables? This is a library package used for building with CMake and other generators. If someone wants to change the value, they're free to do that, but it should definitely be in the package.

brian6932 avatar Aug 13 '25 02:08 brian6932

Those are CMake cache variables, not broadly respected environment variables.

What's the point of installing the package if you don't want to set the environment variables? This is a library package used for building with CMake and other generators. If someone wants to change the value, they're free to do that, but it should definitely be in the package.

What I mean is that you cannot assume user is using CMake generator, and we should not set all possible env var for user.

Lutra-Fs avatar Aug 13 '25 02:08 Lutra-Fs

we should not set all possible env var for user

I don't agree, it's only a benefit to set it, we're packaging a library. Why package it at all if you don't set the environment variables? All library packages set the relevant environment variables. We're not modifying the CMAKE_PREFIX_PATH here, just setting library-specific environment variables, this is standard practice.

Do you have any idea how many environment variables a developer would have to set if every single packaged library lacked necessary environment variables on large codebases? This is literally one of the largest reasons why people use vcpkg, conan, and linux distro package managers instead of manually downloading dependencies.

brian6932 avatar Aug 13 '25 03:08 brian6932

Thanks for opening this and for thinking about downstream dev experience—totally with you on making library packages “just work.” As a developer myself, I appreciate when dependencies are easy to wire up.

That said, exporting LIBXML2_INCLUDE_DIR / LIBXML2_LIBRARY via env_set isn’t the right lever here. Those names are CMake cache variables used by FindLibXml2 at configure time, not environment variables that CMake consumes automatically. So setting them in the OS environment won’t reliably help CMake—and it won’t help Meson or MSBuild users either.

The portable discovery interfaces are:

  • CMake: use LibXml2_ROOT (env or CMake variable) and/or CMAKE_PREFIX_PATH, then find_package(LibXml2) → LibXml2::LibXml2.
  • Meson/pkg-config: add ...\lib\pkgconfig to PKG_CONFIG_PATH (semicolon-separated on Windows) and use dependency('libxml-2.0').

This approach matches the official docs and avoids global env pollution and version pinning issues across multiple installs/versions.

Suggestion: instead of env_set, add a short notes section showing the commands, so like for a user who only have one build system (e.g. MSBuild) installed (and not CMake) can also benefit with your change. It would also benefit if you update the other lib package in main bucket as well for c/cpp:

# CMake (either of these works)
$prefix = "$env:SCOOP\apps\libxml2\current"
cmake -S . -B build -DLibXml2_ROOT="$prefix"
# or
$env:CMAKE_PREFIX_PATH = "$prefix;$env:CMAKE_PREFIX_PATH"
cmake -S . -B build
# Meson / pkg-config
$prefix = "$env:SCOOP\apps\libxml2\current"
$env:PKG_CONFIG_PATH = "$prefix\lib\pkgconfig;$env:PKG_CONFIG_PATH"
# meson setup build   # then dependency('libxml-2.0')

References:

Lutra-Fs avatar Aug 17 '25 06:08 Lutra-Fs

I don't think we should be modifying package config path, or CMake prefix path, you're generally not supposed to use package config on Windows. Sure, I wouldn't be opposed to adding LibXml2_ROOT, but you should be setting cache variables, they literally get checked: https://github.com/search?q=repo%3AKitware%2FCMake+LibXml2_&type=code It doesn't prevent CMake from re-setting them again, or the user doing so, I set these as someone using the library, so I know it works. The cache article tells you not to edit the CMakeLists.txt file, nothing against setting the environment variables.

brian6932 avatar Aug 17 '25 12:08 brian6932