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

[question] How to separate generator files by requirements and tool requirements #12342

Closed
prince-chrismc opened this issue Oct 20, 2022 · 6 comments · Fixed by #15813
Closed
Assignees
Milestone

Comments

@prince-chrismc
Copy link
Contributor

In conan-io/conan-center-index#13612

We need to do some hackery in the build script to avoid both the dependencies and build requirements (which overlap) PkgConfigDeps being in the same folder.

@jwillikers implemented two different solutions

  • one with PkgConfigDeps.build_context_suffix and patching the Meson build script to use the new name.
  • one with manually copying the files to a seperate folder

Ideally there would be a generator folder for each for the advance case where we need to seperate them.


I'll get an error if I don't set the build_context_suffix for the wayland package because this library requires the wayland-client library from the wayland package as a regular requires while simultaneously requiring the wayland-scanner tool from the wayland package as a tool_requires.

If we were able to place the build-context pkg-config files in a separate directory from the others, this would avoid naming conflicts and the necessary patching to find the tool_requires dependencies. Meson would be able to handle this gracefully just by setting the standard project option build.pkg_config_path to a value including the directory for the build-context pkg-config files.

Originally posted by @jwillikers in conan-io/conan-center-index#13612 (comment)

@memsharded
Copy link
Member

I'd like to understand one thing: is the wayland that is used via a tool_require some executable that must be used in the build, or is it a library to link with?

Because in that PR I see:

if self._has_build_profile and self.options.get_safe("with_wayland"):
            self.tool_requires("wayland/1.21.0")
            self.tool_requires("wayland-protocols/1.27")

But the with_wayland option is not removed from package_id(), so it means that it is relevant to create a different binary, not just a tool that is necessary? I don't know much about wayland...

@jwillikers
Copy link
Contributor

@memsharded It's used as both a regular requires and a tool_requires in this case. It provides both a library and a tool known as wayland-scanner that generates source code from XML files.

@jwillikers
Copy link
Contributor

It's worth noting that several projects locate the wayland-scanner utility through its pkg-config file, including the wayland package itself.

There's also an important problem with the current build_context_suffix. It doesn't automatically provide the necessary pkg-config files for dependencies in the build context, so it's actually mixing pkg-config files from the build and host contexts which is concerning.

To illustrate, the wayland-scanner component requires libxml2::libxml2 and expat::expat. The associated pkg-config names are listed in the pkg-config file generated for wayland-scanner. The build context version of wayland-scanner still contains the pkg-config names for expat and libxml2 in the host context.

When attempting to use a separate build context directory for the pkg-config files in the build context, I think it's necessary to include all the required pkg-config files there so that pkgconf doesn't fail to find the dependency when the dependency is required in the build context and set as required in Meson.

jwillikers added a commit to jwillikers/conan-center-index that referenced this issue Oct 21, 2022
I made an oops in my previous change.
This actually does end up breaking cross-compilation.
The requirements of `wayland-scanner` aren't available in the build context.
This causes pkg-config to fail to "find" `wayland-scanner`.
The `expat` and `libxml2` files would need to be in this same directory.
Omitting the separate build context directory for pkg-config files works around this issue for now.
Work will need to be done upstream to ensure dependencies are also made available in the build context for PkgConfigDeps.
See conan-io/conan#12342 for progress on the issue upstream.

It's important to note that pointing Meson to the generators directory for "native" pkg-config files isn't the safest thing to do.
It's possible for Meson to mistakenly think dependencies in the generators directory are meant for the build context when they are of course meant for the host context whenever they lack the `_BUILD` suffix.

I've also placed `wayland-protocols` in the build context.
While it provides XML files only, really, this maps to how it is used.
conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this issue Oct 24, 2022
…ontext

* xkbcommon: Revert using a separate directory for the build context

I made an oops in my previous change.
This actually does end up breaking cross-compilation.
The requirements of `wayland-scanner` aren't available in the build context.
This causes pkg-config to fail to "find" `wayland-scanner`.
The `expat` and `libxml2` files would need to be in this same directory.
Omitting the separate build context directory for pkg-config files works around this issue for now.
Work will need to be done upstream to ensure dependencies are also made available in the build context for PkgConfigDeps.
See conan-io/conan#12342 for progress on the issue upstream.

It's important to note that pointing Meson to the generators directory for "native" pkg-config files isn't the safest thing to do.
It's possible for Meson to mistakenly think dependencies in the generators directory are meant for the build context when they are of course meant for the host context whenever they lack the `_BUILD` suffix.

I've also placed `wayland-protocols` in the build context.
While it provides XML files only, really, this maps to how it is used.

* Remove lingering import

* Improve the set of topics
@franramirez688 franramirez688 self-assigned this Nov 24, 2022
@franramirez688 franramirez688 added this to the 1.56 milestone Nov 24, 2022
@jwillikers
Copy link
Contributor

Update: It looks like updates for the Flex recipe are facing a similar conundrum, but regarding supplied CMake modules: conan-io/conan-center-index#14013

@memsharded memsharded modified the milestones: 1.56, 1.57 Dec 20, 2022
@memsharded memsharded modified the milestones: 1.57, 1.58 Jan 10, 2023
@memsharded memsharded modified the milestones: 1.58, 1.59 Jan 30, 2023
@czoido czoido modified the milestones: 1.59, 1.60 Feb 16, 2023
@memsharded memsharded modified the milestones: 1.60, 1.61 May 8, 2023
@memsharded memsharded modified the milestones: 1.61, 1.62 Sep 11, 2023
@franramirez688 franramirez688 modified the milestones: 1.62, 1.63 Nov 7, 2023
@memsharded
Copy link
Member

This couldn't be prioritized into 1.X, but I am moving it to 2.X, because it will get more attention there.

I think this is doable by introducing a PkgConfigDeps.build_prefix, lets give it a try.

@memsharded memsharded removed this from the 1.63 milestone Feb 12, 2024
@memsharded
Copy link
Member

Implemented in #15813 for next 2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants