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

[bug] MesonToolchain does not pick up CC/CXX from build requirements (in build profile) #8311

Closed
madebr opened this issue Jan 10, 2021 · 10 comments · Fixed by #8353
Closed

Comments

@madebr
Copy link
Contributor

madebr commented Jan 10, 2021

The compiler paths of a build requirement with a cross compiler that sets the CC/CXX environment variables
are not available in conan_meson_cross.ini.

I have a build requirement that sets some compiler variables:

def package_info(self):
    self.env_info.CC = os.path.join(self.package_folder, "bin", "aarch64-none-elf-gcc")
    self.env_info.CC_LD = os.path.join(self.package_folder, "bin", "aarch64-none-elf-gcc")
    self.env_info.CXX = os.path.join(self.package_folder, "bin", "aarch64-none-elf-g++")
    self.env_info.CXX_LD = os.path.join(self.package_folder, "bin", "aarch64-none-elf-g++")
    self.env_info.AR = os.path.join(self.package_folder, "bin", "aarch64-none-elf-ar")

But the [binaries] section of the cross file remains empty.

When I run conan with CC=abc conan create .... -pr:h ... -pr:b ..., then c = 'abc' is present in the binaries` section.

CFLAGS/CXXFLAGS from a build requirement are also not propagated.

Environment Details (include every applicable attribute)

  • Operating System+version: Fedora Linux 30
  • Compiler+version: native: gcc 9.3, cross: gcc 10.2
  • Conan version: 1.32.1
  • Python version: 3.7.7

Steps to reproduce (Include if Applicable)

  1. Create a conan recipe that packages a compiler (sets C/CXX/...). Let's name it crosscompiler/0.1.
  2. Create a host profile that adds the package created in step 1 as a build requirement, let's name the profile: cross.
  3. Create a build profile for your native compiler. Let's name it native.
  4. Cross build a meson project (using MesonToolchain like: conan create . meson-project/0.1@ -pr:h cross -pr:b native

The generated conan_meson_cross.ini won't contain the properties.c values, set by crosscompiler/0.1.
If you'd run CC=abc conan create . meson-project/0.1@ -pr:h cross -pr:b native, then it would.

CC'ing @SSE4 for being the author of MesonToolchain.

@madebr madebr changed the title [bug] MesonToolchain does not pick up CC/CXX from build requirements [bug] MesonToolchain does not pick up CC/CXX from build requirements (in build profile) Jan 10, 2021
@SSE4
Copy link
Contributor

SSE4 commented Jan 11, 2021

@jgsogo do you have an idea why could it happen? e.g. meson tests use CC from the conan profile, and it works flawlessly

jgsogo added a commit to jgsogo/conan that referenced this issue Jan 14, 2021
@jgsogo
Copy link
Contributor

jgsogo commented Jan 14, 2021

Hi!

The environment from the build requires is populated before entering the build() method. The MesonToolchain takes all these values from the environment, but they are not available in the generate() function.

I'm not sure how we want to pass these variables from now on (ping @memsharded ), we can populate the environment during generate() the same way we do during build() or we can leverage on #8266

Not a bug because it's all about experimental features, but something to define and implement for sure.

🤔 With the toolchains and generators, if everything is written down during the generate() call, then we no longer need the environment in the build() call.

@memsharded
Copy link
Member

I would say that if we want to be very transparent and have a better developer experience, the way to go might be to generate virtualenv files for everything in the environment. After a conan install a developer should be able to build and run, with their native tools (cmake, make, meson), without relying on conan build. The only way is that Conan translate whatever is in its environment, its build_requires, etc into a virtualenv file that can be used. I would say that the developer experience should be something similar to:

$ git clone .... & cd ...
$ conan install ...
$ (if necessary) source conantoolchainenv.sh
$ meson ... (using Conan generated files with MesonToolchain)
# or
$ cmake .... -DCMAKE_TOOLCHAIN_FILE=conantoolchain.cmake

And yes, if the environment becomes something explicit to the build, then the Conan build() method do not need to change the environment before launching, but instead it should be explicit in some way:

def build(self):
     with activate_env("conantoolchainenv.sh"):
           meson = Meson(self)
           meson.build()

This is a very preliminary vision, sure we need to work on it, but the main principles would be:

  • Developer should be able to have a native build experience without invoking conan build, achieving the same build as in with conan create without much pain
  • Making the process and the environment more explicit and decoupled, so it is kind of easier to debug things, and probably easy to have greater configurability.

@jgsogo
Copy link
Contributor

jgsogo commented Jan 14, 2021

Right now MesonToolchain writes a file with values taken from the environment. I don't know if that conan_meson_cross.ini file can read the environment in runtime (so it can leverage on the user/conan running source conantoolchainenv.sh to populate the environment), but it would be a completely different implementation of that toolchain class.

Before thinking about the local workflow, we need conan create to work. As it is implemented now, we need to pass configuration to generate() function, it is where MesonToolchain is used:

I'm not sure how we want to pass these variables from now on (ping @memsharded ), we can populate the environment during generate() the same way we do during build() or we can leverage on #8266

...or use a completely different approach for MesonToolchain.

@memsharded
Copy link
Member

Ok, now I understand better the issue. It is not about the transfer between the generate() and the build(), but how the generate() populate things. Which would be related to what @madebr was talking about the new conf, if it was a way of propagating information from the build_requires to the consumers of those build_requires.

I would like if we could try to explicit define the interfaces that will be available for Conan 2.0 about the dependencies, so the generate() method uses them, of course being the environment one of them. But maybe that doesn't mean to automatically activate the environment for the generate() method, so it can capture those env-vars and write them to files, because that makes also difficult to separate things defined by Conan, potentially making the generated environment files very noisy.

@memsharded memsharded added this to the 1.34 milestone Jan 14, 2021
@SSE4
Copy link
Contributor

SSE4 commented Jan 18, 2021

that seems like conanfile has an env attribute, which is populated from the build_requires (method _propagate_info).
this is used at least by the VirtualEnvGenerator right now.
however, the build method seems to be very special (see run_build_method which does env management).
moreover, it seems like by design, build_requires are only applied to the build method?
at least I see self._conanfile.env is empty, as well as self._conanfile.build_requires is also empty during the generate method.

@SSE4
Copy link
Contributor

SSE4 commented Jan 19, 2021

addition from #8311 - CC from the profile aren't captured by MesonToolchain, they are captured by meson executable itself.
it turns out it's problematic to implement preprocessor_defininitions because of this.

@SSE4
Copy link
Contributor

SSE4 commented Jan 19, 2021

okay, I think I was able to understand the flow on how conanfile env is propagated.
I've added tests ensuring both cases work, see #8362 and #8353

@memsharded memsharded modified the milestones: 1.34, 1.35 Feb 23, 2021
@memsharded memsharded modified the milestones: 1.35, 1.36 Mar 29, 2021
@memsharded memsharded modified the milestones: 1.36, 1.37 Apr 27, 2021
@memsharded memsharded modified the milestones: 1.37, 1.38 May 27, 2021
@lasote lasote self-assigned this Jun 22, 2021
@lasote
Copy link
Contributor

lasote commented Jun 22, 2021

@madebr I updated #8353 to allow receiving the information from a build require.
Please note that the new environment model requires to use self.buildenv_info at the package_info() method, you can check the added test in the PR. e.g: self.buildenv_info.define("LD", "LD_VALUE")

@memsharded
Copy link
Member

#8353 was merged, and will be using the new experimental buildenv_info and [buildenv] profile information to define the environment. This PR was an improvement, but there are still some questions regarding the environment definition. It will be released in 1.38, feedback very welcome!

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