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

AutoToolsBuildEnv needs to be more flexible with "host" #3354

Closed
3 tasks
DavidZemon opened this issue Aug 15, 2018 · 2 comments · Fixed by #3603
Closed
3 tasks

AutoToolsBuildEnv needs to be more flexible with "host" #3354

DavidZemon opened this issue Aug 15, 2018 · 2 comments · Fixed by #3603
Assignees
Milestone

Comments

@DavidZemon
Copy link

To help us debug your issue please explain:

  • I've read the CONTRIBUTING guide.
  • I've specified the Conan version, operating system version and any tool that can be relevant.
  • I've explained the steps to reproduce the error or the motivation/use case of the question/suggestion.

I found a couple problems with the get_gnu_triplet() method of oss.py:

# Calculate the arch
machine = {"x86": "i686" if os != "Linux" else "x86",
           "x86_64": "x86_64",
           "armv6": "arm",
           "armv7": "arm",
           "armv7s": "arm",
           "armv7k": "arm",
           "armv7hf": "arm",
           "armv8": "aarch64"}.get(arch, None)
if machine is None:
    raise ConanException("Unknown '%s' machine, Conan doesn't know how to "
                         "translate it to the GNU triplet, please report at "
                         " https://github.com/conan-io/conan/issues" % arch)

First, the exception being raised is swallowed by `AutoBuildToolsBuildEnv._get_host_build_target_flags():

try:
    build = get_gnu_triplet(os_detected, arch_detected, self._compiler)
except ConanException:
    build = None
try:
    host = get_gnu_triplet(self._os, self._arch, self._compiler)
except ConanException:
    host = None

A warning would be great, instead of just silently building apps very badly.

Second, any way to make that a little more flexible instead of hardcoding architectures in there? I ran into this after adding armv5te to my settings.yml file and now I have two options: change all of my recipes (and fork open source recipes) to check for arch==armv5te and provide the host parameter explicitly or wait for this bug to get fixed. Neither solution is good at all.

Maybe a replacement could look something more like this:

# Calculate the arch
if 'x86' == arch:
    machine = "i686" if os != "Linux" else "x86"
elif arch == 'armv8':
    machine = 'aarch64'
elif arch.startswith('arm'):
    machine = 'arm'
else:
    machine = arch

This should work reasonably well so long as you stay on top of adding new ARM architectures in the future.

@lasote lasote added this to the 1.8 milestone Aug 16, 2018
@DavidZemon
Copy link
Author

DavidZemon commented Aug 16, 2018

Glad you guys approved it - that's great news. I wonder if you might be able to squeeze something simpler in to an earlier release though. I actually jumped online just now to modify my original suggestion, but there's no reason an environment variable couldn't be added to the current implementation, like so:

# Calculate the arch
best_guess_at_machine = {"x86": "i686" if os != "Linux" else "x86",
           "x86_64": "x86_64",
           "armv6": "arm",
           "armv7": "arm",
           "armv7s": "arm",
           "armv7k": "arm",
           "armv7hf": "arm",
           "armv8": "aarch64"}.get(arch, None)
machine = os.environ.get('CONAN_AUTOTOOLS_HOST_MACHINE', best_guess_at_machine)
if machine is None:
    raise ConanException("Unknown '%s' machine, Conan doesn't know how to "
                         "translate it to the GNU triplet, please report at "
                         " https://github.com/conan-io/conan/issues" % arch)

This should be a non-breaking change that won't affect a soul, unless they know about the new, special environment variable. It will allow me to continue the very exciting work I was doing yesterday, which was us starting to use Conan for our current platform (ARM9/armv5te), instead of only our future platform (armv7hf).

@lasote lasote self-assigned this Sep 24, 2018
@lasote
Copy link
Contributor

lasote commented Sep 24, 2018

Your suggestion of the env var doesn't work. This get_gnu_triplet is used both for the host(targeting machine) and the build (current machine) so it can change if we are cross building, and the env var won't be valid anymore.
I'm opening a PR in some minutes adding more flexible detections as you suggested first.

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 a pull request may close this issue.

2 participants