Conan: Conan 1.19.0 can break static library compilation when using visual_studio generator

[santhonisz]

Hi there,

Conan 1.19.0 has introduced changes to the visual_studio generator via this PR: https://github.com/conan-io/conan/pull/5564

However the decision to add the following code to the generated props file can break compilation of static libraries that reference other static libraries, but are not expecting to be linked against them (in our case this is done later when being consumed by a shared library or executable).

<Lib>
  <AdditionalLibraryDirectories>$(ConanLibraryDirectories)%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories>
  <AdditionalDependencies>{libs}%(AdditionalDependencies)</AdditionalDependencies>
</Lib>

Our organisation has been using Conan since the 0.28.0 release and now have over 400 private Conan packages, several of which are static libraries that reference other internal static libraries. We do not wish for our static libraries to be combined together by default. As it stands, the visual_studio generator is broken for us (multiple projects fail to build) and would require us to explicitly update all our affected projects to reset the the <Lib> configuration at the project level - not exactly what I would call a backwards-compatible change.

I do understand the core requirement of the PR in question. Looking into it, wouldn't an alternative (backwards-compatible) solution have been to, instead of adding the <Lib> section to the generated props file, add another custom property that end users could then use in their project files _if_ they desire that static libraries be combined?

<PropertyGroup Label="ConanVariables">
  ...
  <ConanLibraries>{libs}</ConanLibraries>
</PropertyGroup>

I think this is further backed up by the comment in the PR that questioned whether it was safe to add the <AdditionalLibraryDirectories> section in the first place.

https://github.com/conan-io/conan/pull/5564#issuecomment-521548338

Thanks,

Sam

[memsharded]

Hi Sam,

Yes, you are right, this would probably have been better the way you suggest, we probably misunderstood the <Lib> task.

Let's fix this asap and target this fix for Conan 1.19.1 (cc @kobalicek, do you agree?). Sorry for the inconvenience and thanks for the report!

[kobalicek]

Yeah, I agree. The original pull request btw didn't add <AdditionalDependencies> in <Lib>, it was added based on additional suggestion.

[kobalicek]

If I think of this retrospectively I think it would be nice to just support a template that users can override to support their own use cases.

Because I think that having <AdditionalIncludeDirectories> in <Lib> is really okay, and having there dependencies is okay too. It's use-cases that are different - some people don't want that, some do, so it would be best to either provide a setting to customize the behavior or ability to use a custom template, which should cover 100% use-cases by design.

However, I'm fine with just removing AdditionalDependencies if it broke existing packages.

[memsharded]

Yeah, priority here is to fix the regressions. Then for future improvements we can discuss in new issues.
The thing is that generators cannot be parameterized now, and templates that require further user action will mean extra friction for some users. It is likely that then new toolchains functionality we are starting to work on might be able to improve over these cases, but it is a bit early to know.

[santhonisz]

Thanks very much for the rapid response on this guys, it is much appreciated!

[danimtb]

Fix #5846 was released in Conan 1.19.1. Thanks!