-
Notifications
You must be signed in to change notification settings - Fork 991
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
MSBuild: add argument to build() to opt-in autoconsumption of props files generated by MSBuildToolchain & MSBuildDeps #12817
base: develop
Are you sure you want to change the base?
Changes from all commits
7a60623
5326ef5
2976776
f6da520
d0fc429
86762f7
17d39c2
9ea12f8
8730bae
d3e48ba
24d75e5
a42e46f
f42f32c
3d178a6
68a2200
04e69cd
ab5295c
88cf4c1
0941ed3
b35577a
7b8e82a
b4e0801
97cc0fe
501e227
c4f5a86
c450449
7753e04
730cede
008c568
ce846a5
def7edc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,9 @@ | ||
import os | ||
import re | ||
import xml.etree.ElementTree as ET | ||
|
||
from conan.tools.microsoft.msbuilddeps import MSBuildDeps | ||
from conan.tools.microsoft.toolchain import MSBuildToolchain | ||
from conans.errors import ConanException | ||
|
||
|
||
|
@@ -16,10 +22,21 @@ def msbuild_arch(arch): | |
'armv8': 'ARM64'}.get(str(arch)) | ||
|
||
|
||
def msbuild_arch_to_conf_arch(arch): | ||
return { | ||
"Win32": "Win32", | ||
"x86": "Win32", | ||
"x64": "x64", | ||
"ARM": "ARM", | ||
"ARM64": "ARM64", | ||
}.get(str(arch)) | ||
|
||
|
||
class MSBuild(object): | ||
def __init__(self, conanfile): | ||
self._conanfile = conanfile | ||
self.build_type = conanfile.settings.get_safe("build_type") | ||
self.configuration = conanfile.settings.get_safe("build_type") | ||
# if platforms: | ||
# msvc_arch.update(platforms) | ||
arch = conanfile.settings.get_safe("arch") | ||
|
@@ -28,10 +45,13 @@ def __init__(self, conanfile): | |
msvc_arch = conanfile.settings.get_safe("os.platform") | ||
self.platform = msvc_arch | ||
|
||
def command(self, sln, targets=None): | ||
def command(self, sln, targets=None, force_import_generated_files=False): | ||
cmd = ('msbuild "%s" /p:Configuration=%s /p:Platform=%s' | ||
% (sln, self.build_type, self.platform)) | ||
|
||
if force_import_generated_files: | ||
cmd += f" {self._force_import_generated_files_cmd_line_arg()}" | ||
|
||
verbosity = msbuild_verbosity_cmd_line_arg(self._conanfile) | ||
if verbosity: | ||
cmd += " {}".format(verbosity) | ||
|
@@ -48,11 +68,83 @@ def command(self, sln, targets=None): | |
|
||
return cmd | ||
|
||
def build(self, sln, targets=None): | ||
cmd = self.command(sln, targets=targets) | ||
def build(self, sln, targets=None, force_import_generated_files=False): | ||
cmd = self.command(sln, targets=targets, force_import_generated_files=force_import_generated_files) | ||
self._conanfile.run(cmd) | ||
|
||
@staticmethod | ||
def get_version(_): | ||
return NotImplementedError("get_version() method is not supported in MSBuild " | ||
"toolchain helper") | ||
|
||
def _get_concrete_props_file(self, root_props_file): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure why the specific file needs to be passed, if the general toolchain file already contains conditionals to pick the right configuration one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because it doesn't work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (obviously, I would be more than happy if a smart solution not involving to parse xml files could be found) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still see a lot of complexity in this functionality. |
||
concrete_props_file = "" | ||
|
||
root = ET.parse(root_props_file).getroot() | ||
namespace = re.match('\{.*\}', root.tag) | ||
namespace = namespace.group(0) if namespace else "" | ||
importgroup_element = root.find(f"{namespace}ImportGroup") | ||
if importgroup_element: | ||
import_elements = importgroup_element.findall(f"{namespace}Import") | ||
if len(import_elements) == 1: | ||
concrete_props_file = import_elements[0].attrib.get("Project") | ||
else: | ||
expected_condition = (f"'$(Configuration)' == '{self.configuration}' And " | ||
f"'$(Platform)' == '{msbuild_arch_to_conf_arch(self.platform)}'") | ||
for import_element in import_elements: | ||
if expected_condition == import_element.attrib.get("Condition"): | ||
concrete_props_file = import_element.attrib.get("Project") | ||
break | ||
|
||
if concrete_props_file: | ||
concrete_props_file = os.path.join(self._conanfile.generators_folder, concrete_props_file) | ||
|
||
if not concrete_props_file or not os.path.exists(concrete_props_file): | ||
raise ConanException( | ||
f"MSBuildToolchain props file is missing for configuration={self.configuration} and " | ||
f"platform={msbuild_arch_to_conf_arch(self.platform)}." | ||
) | ||
|
||
return concrete_props_file | ||
|
||
def _get_msbuildtoolchain_properties(self, root_props_file): | ||
properties = {} | ||
|
||
# Get properties from props file of configuration and platform | ||
concrete_props_file = self._get_concrete_props_file(root_props_file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this can be avoided with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes maybe, I can try There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nop, it doesn't help :( |
||
root = ET.parse(concrete_props_file).getroot() | ||
namespace = re.match('\{.*\}', root.tag) | ||
namespace = namespace.group(0) if namespace else "" | ||
for propertygroup in root.iter(f"{namespace}PropertyGroup"): | ||
if propertygroup.attrib.get("Label") == "Configuration": | ||
for child in propertygroup: | ||
propert_name = child.tag.replace(namespace, "") | ||
properties[propert_name] = child.text | ||
return properties | ||
|
||
def _force_import_generated_files_cmd_line_arg(self): | ||
cmd_args = [] | ||
props_paths = [] | ||
|
||
# MSBuildToolchan must be in generators for this MSBuild mode | ||
msbuildtoolchain_file = os.path.join(self._conanfile.generators_folder, MSBuildToolchain.filename) | ||
if not os.path.exists(msbuildtoolchain_file): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is possible that some users want to inject only dependencies information, but they don't want to use the toolchain at all? Sounds very possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Il would make little sense. It would be like allowing |
||
raise ConanException("Missing MSBuildToolchain, it should be added to generators") | ||
props_paths.append(msbuildtoolchain_file) | ||
|
||
# Properties of MSBuildToolchain must be extracted and passed manually through command line | ||
# because they don't have precedence when props file is injected with /p:ForceImportBeforeCppTargets | ||
properties = self._get_msbuildtoolchain_properties(msbuildtoolchain_file) | ||
for k, v in properties.items(): | ||
cmd_args.append(f"/p:{k}=\"{v}\"") | ||
|
||
# MSBuildDeps generator is optional | ||
msbuilddeps_file = os.path.join(self._conanfile.generators_folder, MSBuildDeps.filename) | ||
if os.path.exists(msbuilddeps_file): | ||
props_paths.append(msbuilddeps_file) | ||
|
||
# Inject root props generated by MSBuildToolchain & MSBuildDeps | ||
if props_paths: | ||
cmd_args.append(f"/p:ForceImportBeforeCppTargets=\"{';'.join(props_paths)}\"") | ||
|
||
return " ".join(cmd_args) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,15 @@ | |
from conans.util.files import save, load | ||
|
||
|
||
def arch_to_vcproj_platform(arch): | ||
return { | ||
"x86": "Win32", | ||
"x86_64": "x64", | ||
"armv7": "ARM", | ||
"armv8": "ARM64", | ||
}.get(arch) | ||
|
||
|
||
class MSBuildToolchain(object): | ||
|
||
filename = "conantoolchain.props" | ||
|
@@ -52,27 +61,23 @@ def __init__(self, conanfile): | |
self.cflags = [] | ||
self.ldflags = [] | ||
self.configuration = conanfile.settings.build_type | ||
self.platform = arch_to_vcproj_platform(str(conanfile.settings.arch)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've exposed platform attribute in MSBuildToolchain because it was exposed in MSBuildDeps and they share the same underlying logic, but I'm not sure it makes sense. Is it really customizable in MSBuild files? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am ok with it, I think it is customizable, but in the worst case, it is not critical that it is exposed, just don't change it if you don't need to change it. |
||
self.runtime_library = self._runtime_library(conanfile.settings) | ||
self.cppstd = conanfile.settings.get_safe("compiler.cppstd") | ||
self.toolset = self._msvs_toolset(conanfile) | ||
self.properties = {} | ||
check_using_build_profile(self._conanfile) | ||
|
||
def _name_condition(self, settings): | ||
def _name_condition(self): | ||
props = [("Configuration", self.configuration), | ||
# TODO: refactor, put in common with MSBuildDeps. Beware this is != msbuild_arch | ||
# because of Win32 | ||
("Platform", {'x86': 'Win32', | ||
'x86_64': 'x64', | ||
'armv7': 'ARM', | ||
'armv8': 'ARM64'}.get(settings.get_safe("arch")))] | ||
("Platform", self.platform)] | ||
|
||
name = "".join("_%s" % v for _, v in props if v is not None) | ||
condition = " And ".join("'$(%s)' == '%s'" % (k, v) for k, v in props if v is not None) | ||
return name.lower(), condition | ||
|
||
def generate(self): | ||
name, condition = self._name_condition(self._conanfile.settings) | ||
name, condition = self._name_condition() | ||
config_filename = "conantoolchain{}.props".format(name) | ||
# Writing the props files | ||
self._write_config_toolchain(config_filename) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If used only once, then it is better to keep it local initially, until repetition pattern clearly arises (ideally at least 3 occurrences)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not exposed in
__init__.py
ofmicrosoft
folder, so it's not public according to conan documentation. What should I do? There are other non-public functions in this file actually.