conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.25k stars 980 forks source link

[feature] Aggregate duplicate directories for components when using `MSBuildDeps` generator #14624

Open santhonisz opened 1 year ago

santhonisz commented 1 year ago

What is your suggestion?

Hi there,

One slight annoyance I've noticed since updating our private recipes to use the new MSBuildDeps generator is that when a requirement that provides multiple components such as boost is used, it will lead to MSBuild using duplicated directories for the includes, libdirs etc of each component in the package (assuming that you're including the generated conandeps.props in the project).

While this doesn't cause any functional issues, it is quite spammy in build logs and other places within Visual Studio that display such AdditionalIncludeDirectories etc e.g.

C:\BuildTools\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\CL.exe /c /IC:\work\src\ /I"C:/.conan2/p/g-tes30877b48c0cd3/p/include" /I"C:/.conan2/p/g-tes30877b48c0cd3/p/include/googlemock/include" /I"C:/.conan2/p/g-tes30877b48c0cd3/p/include/googletest/include" /IC:/.conan2/p/webso4cd9b92a77bce/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/booste13af8dfe6c0b/p/include /IC:/.conan2/p/zlib0c4591880ed8f/p/include /IC:/.conan2/p/opens3e5919e9ca0df/p/include /IC:/.conan2/p/opens3e5919e9ca0df/p/include /I"C:/.conan2/p/ssl-w90b4489a3701b/p/include" /IC:/.conan2/p/rapidcba706a2b9d1d/p/include/include /IC:/.conan2/p/micro24db803c0d533/p/include

Evidently this occurs because an individual props file is generated for each component of boost and they are all declaring the same additive values for <AdditionalIncludeDirectories/> etc.

I don't know if there's any possibility to prevent this e.g. via aggregation using the Distinct item functions or maybe adding a Condition which checks if the path already exists in the target property?

It's a minor niggle, but would be nice if there was a cleaner implementation.

Thanks!

Have you read the CONTRIBUTING guide?

memsharded commented 1 year ago

Hi @santhonisz

Thanks for your suggestion. I can see how this is not great, and it would be nice to reduce it. However, I don't know enough about MSBuild to see how it could be fixed, we are not aggregating things ourselves, but doing a simple <AdditionalIncludeDirectories>$(Conan{{name}}IncludeDirectories)%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> in every generated file.

I also see things like https://learn.microsoft.com/en-us/visualstudio/msbuild/removeduplicates-task?view=vs-2022, but the documentation and examples are scarce, no idea how this could be implemented. We would need some MSBuild expertise to help with this.

santhonisz commented 1 year ago

I've spent a reasonable amount of time looking further into this and have well and truly tumbled down the MSBuild rabbit hole.

It appears that as the various declarations of the AdditionalIncludeDirectories etc. are all within ItemDefinitionGroup (which is the standard approach w/ Visual C++ projects), they cannot be dynamically updated once defined. They ultimately provide the defaults that are assigned to matching Items when Targets are executed. As such, it is only possible modify their content by using them as the basis for an ItemGroup declaration within a Target.

Using this knowledge I was able to define the following props file which ensures that, prior to each relevant MSBuild target for C++ projects, the appropriate ItemGroup elements are defined that aggregate the values from the corresponding ItemDefinitionGroup elements, removing any duplicates. With this, I now only see a single reference to the boost folder in the Conan cache across the ClCompile, Midl, Link and ResourceCompile stages of the build. I suppose the same approach could also be applied to other elements such as AdditionalOptions, PreprocessorDefinitions etc.

<?xml version="1.0" ?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="4.0">
  <Target Name="DeDupClCompileProps" BeforeTargets="ClCompile">
    <ItemGroup>
      <ClCompile>
        <AdditionalIncludeDirectories>@(ClCompile->MetaData("AdditionalIncludeDirectories")->Distinct())</AdditionalIncludeDirectories>
      </ClCompile>
    </ItemGroup>
  </Target>
  <Target Name="DeDupLinkProps" BeforeTargets="Link">
    <ItemGroup>
      <Link>
        <AdditionalLibraryDirectories>@(Link->MetaData("AdditionalLibraryDirectories")->Distinct())</AdditionalLibraryDirectories>
      </Link>
    </ItemGroup>
  </Target>
  <Target Name="DeDupMidlProps" BeforeTargets="Midl">
    <ItemGroup>
      <Midl>
        <AdditionalIncludeDirectories>@(Midl->MetaData("AdditionalIncludeDirectories")->Distinct())</AdditionalIncludeDirectories>
      </Midl>
    </ItemGroup>
  </Target>
  <Target Name="DeDupResourceCompileProps" BeforeTargets="ResourceCompile">
    <ItemGroup>
      <ResourceCompile>
        <AdditionalIncludeDirectories>@(ResourceCompile->MetaData("AdditionalIncludeDirectories")->Distinct())</AdditionalIncludeDirectories>
      </ResourceCompile>
    </ItemGroup>
  </Target>
</Project>

I'm not sure as to whether you consider this a viable solution to this issue (or foundation for one) and whether it could/should be something that is generated by the MSBuildDeps generator. I suppose it could either be included automatically e.g. by conandeps.props or left as a stand-alone file for users to opt-in to adding the their projects?

Of course I'm also not saying this is the only way to address the issue at hand, I'd still be keen to see any alternative solutions.