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

[2/n] [xbuild] Add CLI arguments to support new crossbuilding model #5594

Merged
merged 17 commits into from Mar 30, 2020

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Aug 8, 2019

Changelog: Feature: Add the needed command-line arguments to existing commands to provide information about host and build profiles.
Docs: conan-io/docs#1629

A quick getting started can be found here: conan-io/docs#1586

Close #5594
Close #5202
Close #5238
Close #5030
Close #5851
Close #6040

This PR adds the needed command-line arguments to existing commands to provide information about host and build profiles.

Command-line interface:

  • Previous arguments without explicit context are assigned to host
  • Host context is specified using :h or :host suffixes to arguments: --profile:host, -pr:h, --env:host,...
  • Build context is specified using :b or :build suffixes: --options:build, -s:b,...

This PR should be complemented with #5592 and a minor change in the get_graph_info function to pass both profiles to the GraphInfo ctor.


It also adds a helper class ProfileData (just a namedtuple with a function) to gather the information for each context and avoid having so many arguments through the ConanAPI interface. This is against the commitment of accepting only raw arguments, but I think it is raw enough:

  • with this tuple:

    conan_api.function(profile_host, profile_build,...)
  • without it:

    conan_api.function(profile_names_host, settings_host, env_host, options_host, 
                       profile_names_build, settings_build, env_build, options_build,...)

conans/client/command.py Outdated Show resolved Hide resolved
@jgsogo jgsogo changed the title [WIP] [xbuild] Add CLI arguments to support new crossbuilding model [2/n] [xbuild] Add CLI arguments to support new crossbuilding model Mar 5, 2020
@jgsogo jgsogo added this to the 1.24 milestone Mar 5, 2020
@jgsogo jgsogo self-assigned this Mar 10, 2020
@madebr
Copy link
Contributor

madebr commented Mar 10, 2020

I'm trying this pr out on the gmp recipe of CCI:
default profile:

[settings]
os=Linux
arch=x86_64
compiler=gcc
compiler.version=9
compiler.libcxx=libstdc++
build_type=Release
[options]
[build_requires]
[env]
AR=as
AS=as
CC=gcc
CPP=cpp
CXX=g++
LD=ld
NM=nm
OBJCOPY=objcopy
OBJDUMP=objdump
RANLIB=ranlib
STRIP=strip

mingw profile:

[settings]
os=Windows
arch=x86_64
compiler=gcc
compiler.version=8
compiler.libcxx=libstdc++11
compiler.threads=win32
compiler.exception=seh
build_type=Release
[options]
[build_requires]
[env]
AR=x86_64-w64-mingw32-ar
AS=x86_64-w64-mingw32-as
CC=x86_64-w64-mingw32-gcc
CXX=x86_64-w64-mingw32-g++
CPP=x86_64-w64-mingw32-cpp
LD=x86_64-w64-mingw32-ld
NM=x86_64-w64-mingw32-nm
OBJCOPY=x86_64-w64-mingw32-objcopy
OBJDUMP=x86_64-w64-mingw32-objdump
RANLIB=x86_64-w64-mingw32-ranlib
RC=x86_64-w64-mingw32-windres
STRIP=x86_64-w64-mingw32-strip

Running conan create . gmp/6.1.2@ -pr:h mingw -pr:b default fails because the configure script requires a CC_FOR_BUILD and CPP_FOR_BUILD.
The error is:

checking for build system preprocessor... x86_64-w64-mingw32-gcc -E
checking for build system executable suffix... configure: error: Cannot determine executable suffix

This can be fixed in the mingw profile by setting CC_FOR_BUILD and CPP_FOR_BUILD to gcc and cpp but that is not very future proof and violates the boundary between build and host profile.
So we need some handle to the build environment.

@madebr
Copy link
Contributor

madebr commented Mar 10, 2020

When configuring binutils, a gnu triplet must be passed for the build, host and target systems.
To be able to access these, I have added settings = "os_build", "arch_build", "os", "arch", "compiler", "os_target", "arch_target" to the recipe.

But when running conan create . binutils/2.34@ -pr:h mingw -pr:b default ,
os_build and arch_build are None.
The recipe fails with: ERROR: binutils/2.34: 'settings.arch_build' value not defined.

So some way to access arch_build and os_build would still be nice.

@madebr
Copy link
Contributor

madebr commented Mar 10, 2020

When I add a private require in requirements, it is not available in self.deps_cpp_info.

So let's say in requirements, we do self.requires("mpfr/4.0.2", private=True), then
in the build() method, self.deps_cpp_info["mpfr"].rootpath fails with KeyError: 'mpfr'.

With these first 3 remarks fixed, or worked around, I was able to build and package binutils.

@jgsogo
Copy link
Contributor Author

jgsogo commented Mar 11, 2020

Hi! Thanks a lot for the feedback. The plan is to start with simple use-cases and add complexity step by step. But we have some a priori during this journey:

  • settings os_build and arch_build will be completely deprecated with this feature. If needed, both profiles will be available for the recipes.
  • no cross-compilers in this iteration, we still don't know what is needed related to the os/arch_target settings, at the beginning I thought that we wouldn't need a full profile, that a single option would be enough, but after talking with @SSE4 I'm more convinced that a full profile will be needed. Again, target is out of scope for this iteration

I'll have a look to the environment variables from the profiles that are available during build()


no cross-compilers in this iteration

🤔 Maybe talk about this feature like cross-building is the worst naming we've ever done...

@jgsogo
Copy link
Contributor Author

jgsogo commented Mar 11, 2020

About the environment variables defined in the profiles: right now this is the behavior:

profile_host

            [env]
            PROFILE=PROFILE_HOST

profile_build

            [env]
            PROFILE=PROFILE_BUILD

app/conanfile.py (this recipe should be compiled using host_profile)

    def build(self):
        # Check build_requires (build) are available
        self.output.write(">>>> cmake | cmake_exe")
        self.run("echo %PROFILE%")
        self.run("cmake_exe.exe", run_environment=True)

The output contains:

----Running------
> echo %PROFILE%
-----------------
PROFILE_HOST

----Running------                     
> cmake_exe.exe                       
-----------------
.... and it runs the CMake app that has been compiled using the profile_build

And I think it is correct, because in the context of the app conanfile (it uses profile_host) this is the value of the environment variable defined in the profile it is using. If the same variable is defined, Conan will prioritize the ones in the profile that matches the conanfile.


For your use case, @madebr , you should be using env vars CPP_FOR_BUILD and CC_FOR_BUILD, but the actual gcc and cpp that are in the path because they have been injected by a build_require (environment from the dependencies is injected before calling the build() method)

@madebr
Copy link
Contributor

madebr commented Mar 12, 2020

settings os_build and arch_build will be completely deprecated with this feature. If needed, both profiles will be available for the recipes.

I don't want to set it, but get it 😄 . Is that still possible?
How can a recipe detect that it is cross compiling if nothing about the build system is visible?
Because I don't know now when to set CC_FOR_BUILD.

no cross-compilers in this iteration, we still don't know what is needed related to the os/arch_target settings, at the beginning I thought that we wouldn't need a full profile, that a single option would be enough, but after talking with @SSE4 I'm more convinced that a full profile will be needed. Again, target is out of scope for this iteration

Roger. I only used arch_build and arch_os since it is there.
For now, adding them to settings or options does not make a difference.

thinking Maybe talk about this feature like cross-building is the worst naming we've ever done...

Hehe, in true cross building we have build, host and target (2 steps). In this pr, you're only addressing build and host. So baby steps 😄

For your use case, @madebr , you should be using env vars CPP_FOR_BUILD and CC_FOR_BUILD, but the actual gcc and cpp that are in the path because they have been injected by a build_require (environment from the dependencies is injected before calling the build() method)

If I read correctly, only the environment of the host profile will be included.
What I am asking now if some access to the environment of the build profile is possible?
So I can do something like this: cc_for_build = self.profiles.build.env["CC"]

If I have to add CC_FOR_BUILD in the host profile, isn't this a violation between build and host profile?
And requiring users of this recipe to add CC_FOR_BUILD to the host profile for only one recipe feels wrong.
Also, adding CC_FOR_BUILD to the host profile will block sharing of host profiles between users because it contains build system options.

@jgsogo
Copy link
Contributor Author

jgsogo commented Mar 12, 2020

Hi, there was a typo in my previous message, it should read: "you should NOT be using env vars CPP_FOR_BUILD and CC_FOR_BUILD"

In this approach you would have a recipe for your cross-compiler:

class MyCustomCompiler(ConanFile):

    def package(self):
         self.copy("gcc", dst="bin")
         self.copy("cpp", dst="bin")

    def package_info(self):
         self.env_info.PATH.append(os.path.join(self.package_folder, "bin"))

And your library:

class MyLib(Conanfile):
    def build_requirements(self):
        self.build_requires("mycustomcompiler/....")

    def build(self):
        self.run("gcc <all-my-things>", run_environment=True)
        self.run("cpp <all-my-things>", run_environment=True)

and run conan create .... -pr:h=pr_host -pr:b=pr_build

Before entering the build() method of your Mylib package, Conan activates the environment related to that conanfile, which includes:

  • all the environment associated with host profile: [env] section in the host profile, and values from --env:host command line
  • all the environment coming from build_requires (in the build context). Have a look to this snippet:
    env_info.DYLD_LIBRARY_PATH.extend(n.conanfile.cpp_info.lib_paths)
    env_info.DYLD_LIBRARY_PATH.extend(n.conanfile.cpp_info.framework_paths)
    env_info.LD_LIBRARY_PATH.extend(n.conanfile.cpp_info.lib_paths)
    env_info.PATH.extend(n.conanfile.cpp_info.bin_paths)
    
    so, your gcc and cpp executables from the mycustomcompiler package will be available to run.

Right now, besides the environment that is being applied, recipes know nothing but the profile applied to them (which is the host_profile for themselves) so nothing realted to build-helpers or tools might need to be modified. Once we start thinking about cross-compiling, we need to provide the recipes with the target_machine profile in case they are cross-compilers, so build-helpers, tools take it into account to generated the triplets or populate the command line (but this is another episode)

@madebr
Copy link
Contributor

madebr commented Mar 12, 2020

Hi, there was a typo in my previous message, it should read: "you should NOT be using env vars CPP_FOR_BUILD and CC_FOR_BUILD"

The configure script of gmp requires these variables when cross building...
So I assume, in general, it is interesting to make the build environment available (for querying).

Right now, besides the environment that is being applied, recipes know nothing but the profile applied to them (which is the host_profile for themselves) so nothing realted to build-helpers or tools might need to be modified. Once we start thinking about cross-compiling, we need to provide the recipes with the target_machine profile in case they are cross-compilers, so build-helpers, tools take it into account to generated the triplets or populate the command line (but this is another episode)

While cross building binutils with this pr, the autotools build helper was indeed ok.
But I still needed to customize the gnu-triplets and access to arch_build and os_build.
I am sure there will always exist a project that require esoteric triplets that tools.gnu_triplet does not supply

@memsharded
Copy link
Member

memsharded commented Mar 19, 2020

I think this is working quite well, I'd say take it out of draft and consider to merge it for next release. Nevermind, my browser tab was outdated.

I have been trying this, works pretty well, behavior seems a big improvement.

@memsharded memsharded merged commit 65e780d into conan-io:develop Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants