Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

nested tools.vcvars() ignores arch change #3673

Closed
db4 opened this issue Oct 8, 2018 · 5 comments
Closed

nested tools.vcvars() ignores arch change #3673

db4 opened this issue Oct 8, 2018 · 5 comments

Comments

@db4
Copy link
Contributor

db4 commented Oct 8, 2018

The code below doesn't work as one would expect:

        with tools.vcvars(self.settings, arch="x86_64"):
            self.run("cl")
            with tools.vcvars(self.settings, arch="x86"):
                self.run("cl")

result:

Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26726 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26726 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

(note x64 printed by second cl). The reason I'm asking for this is following. Consider a test recipe:

from conans import ConanFile, tools


class TestConan(ConanFile):
    name = "test"
    version = "0.0.1"
    settings = "os_build", "compiler", "arch_build"

    def build(self):
        if self.settings.compiler == "Visual Studio":
            with tools.vcvars(self.settings, arch="x86"):
                self.run("cl")

Then I build it with conan create, 32-bit compiler is called. It's OK. But then I try to use conan package tools:

from conans import tools
from conan.packager import ConanMultiPackager


if __name__ == "__main__":
    builder = ConanMultiPackager()
    builder.add()
    builder.run()

I get output:

test/0.0.1@user/channel: Calling build()
Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26726 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

64-bit environment is set. That's because ConanMultiPackager() calls tools.vcvars() itself and the second call of tools.vcvars() from the recipe is just ignored.

@lasote lasote self-assigned this Oct 9, 2018
@lasote lasote added this to the 1.9 milestone Oct 9, 2018
@lasote
Copy link
Contributor

lasote commented Oct 9, 2018

But, why are you forcing the arch to x86 in the vcvars instead of building a configuration with the arch setting x86?

@db4
Copy link
Contributor Author

db4 commented Oct 9, 2018

But, why are you forcing the arch to x86 in the vcvars instead of building a configuration with the arch setting x86?

Because this way an installer package cannot be correctly build from recipe when it's binary is not available. Say, I'm building pkg1 on arch_build==arch1 for arch==arch2 that in turn requires pkg2_installer for arch1. pkg2_installer doesn't have arch1 binary available and will be build along with package1. If one use self.settings.arch in pkg2_installer recipe, arch==arch2 will be used during its build that is plain wrong (arch2 executables may not be able to run on arch1 host)

I'm afraid all recommendations from docs to use

    def package_id(self):
        self.info.include_build_settings()
        del self.info.settings.compiler
        del self.info.settings.arch

are flawed for the same reason. One's packages should behave exactly the same, whether their binaries are available or not.

@lasote
Copy link
Contributor

lasote commented Oct 10, 2018

So there are two issues the underlying cross-build model issue (#3329, #2301) and the multiple vcvars calls. I will take a look at the vcvars in case we can fix something but at first sight, probably is the ms command vcvars who is prioritizing the first set arch.

For the underlying issue, I would recommend to build first the package for the installer and then the regular packages. A new cross build is needed but it will take time.

@lasote lasote modified the milestones: 1.9, 1.10 Oct 26, 2018
@lasote lasote removed this from the 1.10 milestone Nov 19, 2018
@lasote lasote removed their assignment Nov 21, 2018
@SSE4
Copy link
Contributor

SSE4 commented Jan 17, 2019

vcvars sets the following environment variables:

  • VisualStudioVersion
  • Platform (x64/x86/ARM)
  • PreferredToolArchitecture (x64/None)
  • CommandPromptType (Native/Cross)

we may use them to detect if we need to change variables
currently, conan sets vcvars to be no-op if VisualStudioVersion matches one that was set earlier (https://github.com/conan-io/conan/blob/develop/conans/client/tools/win.py#L370). it raises otherwise (if VS version changed).

@memsharded
Copy link
Member

These tools.vcvars has been removed in 2.0, closing this as outdate, please report a new ticket if necessary for the new tools, thanks.

@memsharded memsharded closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

4 participants