Skip to content

extend ${BUILD_PREFIX}/meson_cross_file.txt with pkgconfig & cmake setup#100

Merged
isuruf merged 2 commits into
conda-forge:mainfrom
h-vetinari:cross
Mar 26, 2023
Merged

extend ${BUILD_PREFIX}/meson_cross_file.txt with pkgconfig & cmake setup#100
isuruf merged 2 commits into
conda-forge:mainfrom
h-vetinari:cross

Conversation

@h-vetinari

Copy link
Copy Markdown
Member

meson will not auto-discover cmake/pkgconfig in the context of cross-compilation; it needs to be specified explicitly; same goes for the pkgconfig-metadata.

Add this to the default meson_cross_file.txt that's generated for our osx cross-compilation, so we don't have to add this manually (like in conda-forge/scipy-feedstock#205 currently).

I presume that the presence of these extra lines should not affect cross-compilation cases where either cmake or pkgconfig is not actually available. Perhaps @eli-schwartz can confirm/deny?

In any case, would appreciate your inputs @isuruf

@conda-forge-webservices

Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@eli-schwartz

eli-schwartz commented Mar 21, 2023

Copy link
Copy Markdown

I presume that the presence of these extra lines should not affect cross-compilation cases where either cmake or pkgconfig is not actually available. Perhaps @eli-schwartz can confirm/deny?

If you set it to a nonexistent program, and hit a codepath where you actually try to make use of a pkg-config / cmake dependency, you'll get the following meson log line:

WARNING: We thought we found pkg-config '/bin/nonexistent' but now it's not there. How odd!
Found Pkg-config: NO
WARNING: We thought we found CMake '/bin/nonexistent' but now it's not there. How odd!
Found CMake: NO

This is harmless unless you pass --fatal-meson-warnings to meson, which does exactly what it says on the lid.

After that, it will return "pkg-config is not found" and you are right back where you started -- meson considers that pkg-config cannot be found, which is not in and of itself fatal or even a problem except inasmuch as dependencies may not be found, and required dependencies that cannot be found are, naturally, fatal. :D

tl;dr

This is very safe.

@isuruf

isuruf commented Mar 21, 2023

Copy link
Copy Markdown
Member

Is this limited to just cmake and pkg-config? If it's needed for other binaries too, I don't want to pollute this package with lots of binaries being added here.

@h-vetinari

Copy link
Copy Markdown
Member Author

Is this limited to just cmake and pkg-config? If it's needed for other binaries too, I don't want to pollute this package with lots of binaries being added here.

For the moment (e.g. scipy), it's just cmake/pkg-config. But presumably it'll also be relevant for anything that needs to be found in the build env through meson (e.g. protoc). Not sure if such projects (e.g. python-based, using meson, needing protoc) exist already, but I guess we can solve this when we get to it.

Also, based on what Eli was saying, I don't think it would be that painful to include a couple common build tools in the default cross-file. If they're not actually present in the build env, nothing will happen.

@eli-schwartz

Copy link
Copy Markdown

cmake and pkg-config are extremely high-profile and valuable binaries to set, because they apply globally to the majority of dependency lookups. Other binaries used internally by meson are one-offs for a single dependency, and significantly less valuable... maybe llvm-config since it's a reasonably popular software project.

And after that, there's fully arbitrary binaries which are looked up by find_program('name') in a build recipe. Those are so recipe-specific that it's completely not worth it to try to centralize.

...

I would absolutely set up cmake / pkg-config by default, and absolutely not bother with anything else.

@h-vetinari

Copy link
Copy Markdown
Member Author

Sounds like we should be pretty much covered with cmake/pkgconfig. Does that alleviate your concerns @isuruf?

Comment thread recipe/activate-clang.sh Outdated
Comment on lines +156 to +157
# specify path to correct binaries/metadata the meson will not auto-discover out of caution
# binaries from build env

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I understand this comment. Can you make it clear?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So what I'm trying to say is that even though cmake/pkgconfig might be in build and/or host, meson will not auto-discover them and say "ah, this is the right one". It needs to be told explicitly via the cross-file.

Are we on the same page with that? If so, I can try to come up with a better formulation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, that's correct. However the comment in the code is confusing due to some grammatical errors maybe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I updated the comment & removed pkg_config_libdir. Could you PTAL?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Gentle ping @isuruf

Comment thread recipe/activate-clang.sh Outdated
Comment on lines +162 to +163
echo "[properties]" >> ${CONDA_PREFIX}/meson_cross_file.txt
echo "pkg_config_libdir = '${PREFIX}/lib'" >> ${CONDA_PREFIX}/meson_cross_file.txt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do this only if ${PREFIX} is defined.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Btw, is this necessary? pkg-config we have is a script that should fix this on its own.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I pushed a commit to conda-forge/scipy-feedstock#205 to try what happens without. I was following the upstream cross-compilation recommendations, but it's possible that our infra doesn't need this bit of info.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Commonly a cross-compile toolchain provides a custom pkg-config that actually just sets that variable and then executes the "real" pkg-config. The ultimate goal is to instruct pkg-config what configuration it can use, via several possible methods:

If you compiled pkg-config for native, you can use the same binary for cross as well, and all you need to do is set environment variables rather than rebuilding the whole thing. But ideally you can do something like <triplet>-pkg-config or /path/to/custom/toolchain/bin/pkg-config and that will set things up automatically.

@isuruf isuruf merged commit a9f5382 into conda-forge:main Mar 26, 2023
@h-vetinari h-vetinari deleted the cross branch March 27, 2023 01:34
h-vetinari added a commit to h-vetinari/ctng-compiler-activation-feedstock that referenced this pull request Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants