-
Notifications
You must be signed in to change notification settings - Fork 991
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
[PkgConfigDeps] Added bindir
to generated .pc
files
#13623
Conversation
Hi @bruchar1 sorry about the delay getting this merged. |
Hello. I use conan with meson build system on Windows. meson uses pkgconfig generated files to find libraries provided by conan. To be able to run tests on Windows, meson must add paths to DLLs in the PATH environment variable. However, the pkgconfig file currently only provide paths to .lib link libraries, and there is no way to know for sure the path to the DLL. Once conan provide this path, I will be able to use it in meson. This is kind of chicken and egg problem, because meson will not try to use variables that do not exist in the pkgconfig file, but conan cannot test it with meson before meson actually uses the variable... |
Thanks for the insight, I'll get back to you with my progress in a few days :) |
Hi, @bruchar1 - Thanks for your comment, and sorry for the delay! 😅 Could you provide a little repository to see your use case and why Conan's current behavior is insufficient? For instance, if you run:
The |
Unfortunately, I cannot provide a repo for this. The detail you are missing is that it's not the DLL path that is in libdir, but the import lib path used for linking with the DLL. That's fine for compiling and linking. What I am missing is a way to run tests using that lib, without installing it. Suppose you have an executable target, depending on a shared library provided by conan. On Windows, the way the exe can find the DLL is to have the DLL in the PATH env var. However, to link to the DLL, only the .lib import library is needed. The generated .pc file generated from conan provides the path to the .lib (usually in the lib folder), but not to the .dll (usually in the dll folder). If the executable is a test program that I want to run using |
Hi @bruchar1 The responsible for the runtime execution location of shared libs is the Maybe this PR was not addressing the original issue, as it might not be possible at all via PkgConfig? |
That would probably work if I was invoking meson from conan. But the way I use it is to use conan to generate pkgconfig files and meson toolchain file, and then to invoke meson directly. This is needed to avoid the overhead of calling meson as a subprocess, and to allow the flexibility of using meson features directly. Therefore, in this context, meson know nothings about conan. Furthermore,
I don't think so. Exposing bindir via a variable is the way to go, imho. The only part that tricks me is the numbered suffix. Since in most cases, there should only be one bindir, having |
Even if not calling meson from conan, the environment can be activated before calling meson. So I understand from your explanation that Meson has some feature that is able to automatically set the PATH environment variable while executing tests based on the information gathered from
Not sure how it works in a Windows machine, not following GNU conventions. The dependencies packages will contain a Do you have some link to Meson docs or example that describes this it that is the case? I am afraid that I dont know enough about Meson to really understand the issue here. |
I see... Yes it could work. The only drawback I see is that it would put all conan dependencies path into PATH, and not only the ones I need for a specific test executable (because I use one conanfile for a huge monorepo with severals libs and tests).
On Linux, meson use rpaths for that. On Windows, it use a big hack to build the PATH variable. It works well for dependencies defined inside the project, but for external dependencies (like pkgconfig dependencies), it was using the libpath, which is wrong, as I explained before. The relevant code is here: https://github.com/mesonbuild/meson/pull/11649/files |
bindir
to generated .pc
file in PkgConfigDeps
bindir
to generated .pc
files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't approve my PR, but this now looks good, thanks a lot @franramirez688 for the work and @bruchar1 for your patience :)
Can this be backported to Conan 1.x? Some libraries use I've tried to backport it locally leaving alone all testing infrastructure - cherry-picked 69d3b27 seems to work. |
Hi @db4 |
@franramirez688 Why do you think that replacing As for |
It's breaking because some recipes use the |
I'm still using 1.x, and I can't build
(gio-2.0.pc file has And Meson 1.3.1 shows even stranger error:
Are there known workarounds I could try? |
@franramirez688 I still believe that fixing PkgConfigDeps in Conan 1.x is a way to go. PkgConfigDeps was always advertised as an experimental Conan 2 feature:
Many CCI recipes depend or will depend on Conan 2's PkgConfigDeps behavior, and that will generate troubles for users who still have to stick with Conan 1.x for various reasons. If you're strongly against direct fix, maybe conditional Conan 2-compatible behavior is possible? (switched on via some setting). |
@db4 Ok, I will discuss this issue with the team. I'll tell you when we address it. Thanks for the comment!
@Talkless I think you should open a new issue in the repo https://github.com/conan-io/conan-center-index/issues because your comment seems related to the recipe. |
Hi @db4 - could you provide a specific example of a recipe in The variable names at the top of the Some recipes in Conan Center used to make an assumption on the variable names atop the .pc files - which is wrong and should be fixed in the recipes themselves. It would be good for us to understand which other tools are making assumptions about the contents of the .pc files in a way that breaks encapsulation - we did find such an example with Meson, but we want to understand how generalised this is. |
|
are you able to paste a log with the specific failures that you are getting? Please open a new issue about it so that we can track this |
Qt build log also doesn't tell you very much: just that GStreamer is not found (Qt runs pkg-config with --silence-errors and doesn't explain what's wrong with .pc files). I've spent enough time to guess what's going on and prove it with locally-patched Conan. But OK, I'll open a new issue for that. Should it be conan-center-index issue for qt recipe? |
yes please, that would be great! We can try and replicate and trace the build logs of Qt. It's interesting that you mention that qt runs the |
Changelog: Feature: Added
bindir
to generated.pc
file inPkgConfigDeps
.Changelog: Bugfix: Removed
libdir1
andincludedir1
as the default index. Now,PkgConfigDeps
creates thelibdir
andincludedir
variables by default in.pc
files.Docs: conan-io/docs#3269
After asking @bruchar1, this seems to be the mvp for the asked feature :)
Closes #13532
UPDATED:
In a nutshell:
bindir
is harmless.libdir
,includedir
andbindir
as default variables created (if several ones have to be created, then, they'll belibdir
,libdir1
,libdir2
, and so on). After running a grep in CCI, these libraries could be affected by this change (they'd need a migration if they use this latest feature):So
gdk-pixbuf
,glib
andgstreamer
should be fixed a priori.