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

Unable to resolve conflicts between requires and build_requires graphs #4753

Closed
DoDoENT opened this issue Mar 15, 2019 · 22 comments
Closed

Unable to resolve conflicts between requires and build_requires graphs #4753

DoDoENT opened this issue Mar 15, 2019 · 22 comments
Assignees

Comments

@DoDoENT
Copy link
Contributor

DoDoENT commented Mar 15, 2019

Conan v1.13 introduced reporting conflicts between requires and build_requires graphs. This is very good, as it solves many problems, but sometimes you need to be able to resolve reported conflicts and this seems not being possible at the moment (using conan v1.13.1).

Here is a simple project that demonstrates the issue.

Basically, we have a package Aaa with three versions: v1.0.0, v1.1.0 and v1.2.0. Package Bbb requires Aaa/1.0.0, package Ccc requires both Aaa/1.1.0 and Bbb/1.1.0 (thus overriding Bbb's dependency on Aaa and package Ddd build_requires both Aaa/1.2.0, Bbb/1.0.0 and Ccc/1.0.0. However, instead of overriding both Bbb's and Ccc's requirements on Aaa with version 1.2.0, conan reports a conflict:

ERROR: Conflict in Bbb/1.0.0@user/testing
    Requirement Aaa/1.0.0@user/testing conflicts with already defined Aaa/1.2.0@user/testing
    To change it, override it in your base requirements

However, if I change Ddd's build_requires expression from

    build_requires = (
        'Aaa/1.2.0@user/testing',
        'Bbb/1.0.0@user/testing',
        'Ccc/1.0.0@user/testing',
    )

to

    build_requires = (
        'Bbb/1.0.0@user/testing',
        'Ccc/1.0.0@user/testing',
        'Aaa/1.2.0@user/testing',
    )

I get different conflict:

ERROR: Conflict in Ccc/1.0.0@user/testing
    Requirement Aaa/1.1.0@user/testing conflicts with already defined Aaa/1.0.0@user/testing
    To change it, override it in your base requirements

Also, other permutations of build_requires order also give different conflicts.

My questions are:

  • why does conan report conflict here? Why it doesn't override Bbb's and Ccc's dependency to Aaa/1.2.0?
  • how can I solve this conflict? Build_requires expression does not support override keyword (a solution I would use if Ddd required Aaa, Bbb and Ccc instead of only build_required it).
  • if I replace build_requires with requires in Ddd's conanfile, then no conflict is reported, but I don't want Ddd to carry Aaa, Bbb and Ccc as dependencies to downstream and I don't want to them to affect Ddd's package_id. (Image that Ddd is a package that contains only executable built against Aaa, Bbb and Ccc static libraries and consumers of the Ddd package do not need executable rebuilt if they themselves use different versions of those static libraries (you can think of them as utility libraries)).
  • why does the order of packages in build_requires matter? Why can't I simply sort all my dependencies alphabetically? (we noticed the similar problem also for requires clause, although in more complex examples).
@memsharded
Copy link
Member

Hi @DoDoENT

I think the main point is that build_requires are private, they cannot be overwritten by downstream consumer. You cannot tell one of your dependencies that they should use a certain build_requires. That can only be done from the profiles.

Maybe it would be good to understand your use case if the build_requires had a real name, like cmake_installer, gtest, etc., instead of AA, BB, CC.

The way to not propagate downstream 2 different versions of A is to use them privately, otherwise, as you realized, they are a conflict. Because the system will effectively try to find the headers and link of both of them, but only one of them will be found, because they will collide in the consumer self.deps_cpp_info. In your case, it seems that the key is that BB build-requires AA, instead of requiring AA. Or at least require it as a private dependency. But the AA1.0 requirement cannot leak the BB one without colliding. Please let me know if such conversion is possible. Many thanks!

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Mar 16, 2019

Hi @memsharded! Thank for the quick response!

I think the main point is that build_requires are private, they cannot be overwritten by downstream consumer. You cannot tell one of your dependencies that they should use a certain build_requires. That can only be done from the profiles.

I understand that and example I've attached does not even try that. The whole purpose of DD to have AA, BB and CC as build_requires instead of requires is because those are only required to build DD. However, DD build_requires newer version of AA than BB does, so I would expect that I should be able to somehow override BB's dependency (which is requires, not build_requires) to newer version of AA (maybe not automatically, but with something like 'overrides' keyword that is also available for the requires graph).

Maybe it would be good to understand your use case if the build_requires had a real name, like cmake_installer, gtest, etc., instead of AA, BB, CC.

Of course - I think this will also make things more clear why private dependencies do not work for me.

Let's say that Aaa is a kind of utility library - let's call it CoreUtils. It contains some utilities, such as string wrappers, smart pointer implementations, string processing utilities and file system utilities. We build it as a static library.

Also, let's say that Bbb is a different kind of utility library, specialized for software protection. Let's call it Protection. This static library contains algorithms for generating and checking license keys and for encrypting and decrypting various resources. For simplification, let's just focus on support for encryption and decryption of various resources.
Let's also say that Protection library has an option specifying whether or not it should contain encryption keys. The idea is that a production binaries must link to a version of Protection library that only contain decryption keys (option all_keys=False) and internal tools (such as resources packaging utility) must link to a version of Protection library with encryption keys (in order to be able to encrypt the resources).

After that, let Ccc be a static library that provides functions for performing a OCR (optical character recognition). Let's call it an OcrEngine. It uses some algorithms from CoreUtils and needs functions for decrypting resources required for OCR (such as dictionaries, trained ML models, etc.). Of course, it needs to link to version of Protection that contains only decryption keys, therefore it uses its configure method to enforce that Protection:all_keys = False:

def configure(self):
    self.options['Protection'].all_keys = False

This is to prevent accidentally leaking encryption keys into production binary.

Here we come to the Ddd package, which we can call OcrModelBuilder. This is an executable (an internal tool) which can pack resources that should be consumed by the OcrEngine package. It directly uses the Protection to encrypt the resources (therefore it needs the version with all_keys = True) and CoreUtils to use file system utilities. It is also tied to the OcrEngine because it needs to know the exact resource packaging format for specific version of OcrEngine.

After that, there are several models packages that are presented to Conan as header-only packages, but they don't contain any headers nor libraries. They only contain built resources to be used with OcrEngine using the OcrModelBuilder of the same version. For example let's have an EuropeanOcrModels package that contains dictionaries and ML models for european OCR tasks. To build a package, we need to use OcrModelBuilder, but we need to also have dependency on OcrEngine to ensure that if someone downstream overrides the version of the OcrEngine package that models will get rebuilt using the compatible OcrModelBuilder. We model that using the following trick:

class AaaConan(ConanFile):
    name = 'EuropeanOcrModels'
    requires = (
        'OcrEngine/1.0.0@user/testing'
    )

    def build_requirements(self):
        model_builder = str(self.requires['OcrEngine']).replace('OcrEngine', 'OcrModelBuilder')
        self.build_requires(model_builder)

From the example above it looks like that OcrModelBuilder and OcrEngine should be the same package, but the problem is that they have different requirements on all_keys option of the Protection package. Therefore, we configured our CI to always build those packages together (they are even in the same source repository) and then use the conan's mechanism of resolving the version of model builder via the version of OcrEngine when OCR models need to be rebuilt due to incompatible change in OcrEngine's packaging format.

Finally, there is a final product (an ios/android app or SDK) that uses one or more OCR models packages (let's say one app uses only european OCR models, another needs european and chineese, the third one only japaneese, etc.) so in its conanfile.txt it only specifies.

Here is a described sample that correctly works with conan v1.12.3, but does not work with conan v1.13.1.

With conan v1.13.1, when creating OcrModelBuilder package, I get the following error:

ERROR: Conflict in OcrEngine/1.0.0@user/testing
    Requirement CoreUtils/1.1.0@user/testing conflicts with already defined CoreUtils/1.2.0@user/testing
    To change it, override it in your base requirements

If I change OcrModelBuilder to use private requirements instead of build_requirements, I get the exact same error. However, with conan v1.12.3 both cases work, but the case with private requirements instead of build requirements generate a broken OcrModelBuilder package that contains both versions of Protection library in its conanbuildinfo.cmake and all three versions of CoreUtils, as can be seen from conaninfo.txt files in build folders:

This is the conaninfo.txt when using build_requires for OcrModelBuilder:

[settings]
    arch=x86_64
    build_type=Release
    compiler=apple-clang
    compiler.libcxx=libc++
    compiler.version=10.0
    os=Macos

[requires]


[options]


[full_settings]
    arch=x86_64
    build_type=Release
    compiler=apple-clang
    compiler.libcxx=libc++
    compiler.version=10.0
    os=Macos

[full_requires]


[full_options]


[recipe_hash]
    4e50eaacd7529047e0fd67d166944a3b

[env]

and this one when using private requirements:

[settings]
    arch=x86_64
    build_type=Release
    compiler=apple-clang
    compiler.libcxx=libc++
    compiler.version=10.0
    os=Macos

[requires]
    CoreUtils/1.Y.Z
    OcrEngine/1.Y.Z
    Protection/1.Y.Z

[options]


[full_settings]
    arch=x86_64
    build_type=Release
    compiler=apple-clang
    compiler.libcxx=libc++
    compiler.version=10.0
    os=Macos

[full_requires]
    CoreUtils/1.0.0@user/testing:f8bda7f0751e4bc3beaa6c3b2eb02d455291c8a2
    CoreUtils/1.1.0@user/testing:f8bda7f0751e4bc3beaa6c3b2eb02d455291c8a2
    CoreUtils/1.2.0@user/testing:f8bda7f0751e4bc3beaa6c3b2eb02d455291c8a2
    OcrEngine/1.0.0@user/testing:242ef0fed90392e13562b9be9a6b2f2c0e80f688
    Protection/1.0.0@user/testing:4f5e1f000e8b5d28dc395ce69b8221246c97cb38
    Protection/1.0.0@user/testing:ecd294d693b8730315c40a198abe661c38e25a28

[full_options]
    Protection:all_keys=True

[recipe_hash]
    e217915bfb250c974a9d8ac387375587

[env]

This problem is also reflected in conanbuildinfo.cmake - in case of using build_requires I can see that only Protection with hash 4f5e1f000e8b5d28dc395ce69b8221246c97cb38 is in mentioned (this hash contains all_keys=True), while in case of using private requirements I can see that both hashes (same as in conaninfo.txt) are mentioned in the generated cmake file.

This may or may not lead to compile errors, depending on whether the compiler compiling the source of the OcrModelBuilder will first see headers from Protection that do or do not contain encryption keys required to successfully build the executable.

Here is the modified example which uses private dependencies for OcrModelBuilder and also does not work with conan v1.13.1, but works incorrectly with conan v1.12.3.

I hope I explained the issue a bit better now. Please let me know if I can help more.

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Mar 28, 2019

The problem also happens with conan v1.14.0.

However, if in OcrModelBuilder package I change

    requires = (
        ('CoreUtils/1.2.0@user/testing', 'private'),
        ('OcrEngine/1.0.0@user/testing', 'private'),
        ('Protection/1.0.0@user/testing', 'private'),
    )

into

def requirements(self):
        self.requires('CoreUtils/1.2.0@user/testing', private=True, override=True)
        self.requires('OcrEngine/1.0.0@user/testing', private=True, override=True)
        self.requires('Protection/1.0.0@user/testing', private=True, override=True)

then conan does not complain, but the generated conanbuildinfo.txt and conanbuildinfo.cmake files in build folder are empty, i.e. do not contain header and library search paths for any of the dependencies (and cmake does not have any targets that I would expect, such as CONAN_PKG::CoreUtils, etc.). Same thing also happens with conan v1.13.3 and v1.12.3.

So, I am currently stuck with conan v1.12.3 for reasons explained in my earlier messages.

@thorntonryan
Copy link
Contributor

thorntonryan commented Mar 28, 2019

@memsharded , I think I might be hitting this same problem.

I first noticed it when upgrading to 1.13.2 from 1.12.3. But I also see the same problem on 1.14.0.

Expected Behavior ( I think )

I expect MyApp's semver compatible requirements to override additional requirements coming from its build requirements.

Current Behavior (1.13+)

  1. Unzip 4753.zip
  2. Run the following:
# Create SharedCOMLib 1.0.1
$ conan create shared_com_lib.py 1.0.1@user/testing
# Create SharedCOMLib 1.0.2
$ conan create shared_com_lib.py 1.0.2@user/testing
# Create SharedGuiLib 0.1
$ conan create shared_gui_lib.py user/testing
# Resolve requirements for MyApp 0.1
$ conan info myapp.py
  1. And receive the following error:
ERROR: Conflict in SharedGuiLib/0.1@user/testing
    Requirement SharedCOMLib/1.0.1@user/testing conflicts with already defined SharedCOMLib/1.0.2@user/testing
    To change it, override it in your base requirements

Old Behavior (1.12.3)

  1. Unzip 4753.zip
  2. Run the following:
# Create SharedCOMLib 1.0.1
$ conan create shared_com_lib.py 1.0.1@user/testing
# Create SharedCOMLib 1.0.2
$ conan create shared_com_lib.py 1.0.2@user/testing
# Create SharedGuiLib 0.1
$ conan create shared_gui_lib.py user/testing
# Resolve requirements for MyApp 0.1
$ conan info myapp.py
  1. The requirement provided by the build requirement is overridden as expected:
$ ./conan-1.12.3/Scripts/conan info myapp.py
SharedGuiLib/0.1@user/testing requirement SharedCOMLib/1.0.1@user/testing overridden by your conanfile to SharedCOMLib/1.0.2@user/testing

Context

I have three Conan packages in play:

  1. MyApp
    The primary Qt Desktop application that I'm trying to build. Not an actual package per se, but it uses Conan to describe its build dependencies.
  2. SharedCOMLib
    Multiple COM (Component Object Model) dlls implementing various things. Used at runtime by "MyApp"
  3. SharedGuiLib
    Static Qt GUI library offering convenient Widgets / wrappers / utilities, including some that take advantage of classes found in "SharedCOMLib". Used by "MyApp" to avoid re-inventing the wheel on common widgets/dialogs/etc. shared between a variety of tools.

With the following package versions:

SharedCOMLib

  • Version 1.0.1
  • Version 1.0.2

SharedGuiLib

  • Version 0.1
  • Requires SharedCOMLib/1.0.1

MyApp

  • Version 0.1
  • Requires SharedCOMLib/1.0.2
  • Build Requires SharedGuiLib/0.1

A build requirement of MyApp (i.e. "SharedCOMLib") has outdated requirements on another shared library (i.e. "SharedCOMLib"). MyApp also has a requirement on the same shared library (i.e. "SharedCOMLib"), albeit with a more up to date version.

Up until Conan 1.13, that has been okay, because Conan overrode the older version of "SharedCOMLib" coming from my MyApp's build requirements with the newer version coming from MyApp's requirements.

I can apply the fix outlined above in #4753 (comment), and do something like:

- requires = ("SharedCOMLib/1.0.2@user/testing",)
+def requirements(self):
+        self.requires('SharedCOMLib/1.0.2@user/testing', override=True)

But I'm not certain whether the above change is necessary in a 1.13+ world or whether my example illustrates a regression.

As always, thanks to you and all the other maintainers for all your hard work and support maintaining Conan!

Edit 3/29/2019
Updated example with suggestion from #4753 (comment). Thanks @puetzk!

Edit 4/10/2019
Fixed bug in example. See thorntonryan/conan-requirements-overriding-change-in-behavior@55da6d9 for more details

@puetzk
Copy link

puetzk commented Mar 29, 2019

@thorntonryan: I think you got one quirk wrong while simplifying the scenario from our real package for into your example:

The versions of SharedCOMLib should be more like 1.0.1 and 1.0.2; i.e. they are semver compatible, and the package_id of SharedGuiLib (which uses that default mode) does not change due to this override, and the existing binary package can/should be used.

By making your example 0.1 and 0.2, they would not be semver compatible and the package_id would have the full version in it.

I guess this doesn't change the initial error, since conan raises "Requirement ... conflicts with already defined ..." without even considering that the previous behavior of 1.12.x (treating the consuming package's chosen version as an override), but if it had continued your example would hit an unrelated (and valid) error that there was no binary in the cache for with that package_id

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Apr 3, 2019

With the official release of Visual Studio 2019 this now begins to pose a serious problem. Conan 1.12.3 does not work correctly with cmake generator for VS 2019 (it sets generator to "Visual Studio UnknownVersion 16 Win64") and latest conan version (which handles VS 2019 correctly) does not support resolving conflicts between build requirements.

We really need to get this fixed.

@puetzk puetzk mentioned this issue Apr 10, 2019
3 tasks
@memsharded
Copy link
Member

Hi there! @DoDoENT @puetzk @thorntonryan

I am processing this feedback now, but it is a lot to process and understand, in the meantime, it would be great if you could have a look or even try (from the source branch) the current proposal for a patch in Conan 1.14.2: #4937

It changes the way that build-requires of libraries already existing in the regular-requires graph is processed. They won't span a new node anymore, as that would be a conflict, but it reuses the upstream node. Still, if the build_requires uses a different version, it will conflict (you don't want to change the upstream library version because of a build-requires), but it can be overriden from profile build-requires.

@memsharded
Copy link
Member

Ok, there is too much feedback, lets start with the case of @DoDoENT

I have put the zip in a repo here: https://github.com/memsharded/tools_build_requires, with a build.py script to run everything. I have added a clear output of the all_keys option to make sure the correct value is used in the different contexts.

Lets start with this part of the graph:

image

Some pre-conditions:

  • The OCRModelBuilder EXE can only link with one version and configuration of all the other static libraries.
  • If at some point the dependency graph of OCRModelBuilder has two different versions or two different configurations (like all_keys=True and all_keys=False) of the same dependency, that is a conflict, and an error should be raised, and the build should not continue as the final result would be buggy, as it is not determined which package version and configuration will be linked

The expansion of the graph IF OcrModelBuilder uses build_requires as currently defined would be something like:

image

This graph had 2 important bugs in conan 1.12 and older versions:

  • There are several versions of CoreUtils that try to be linked in the Exe. Which version was finally being used was not deterministic and totally quiet, the different versions of CoreUtils were dumping their information in deps_cpp_info, and only the last version that did it was the one finally used.
  • There can be different configurations of the Protection package doing the same overwrite. If the consumer package OcrModelBuilder didn't explicitly change the all_keys, there would be 2 different configurations, and the one finally being used was also not deterministic.

With the new expansion model defined in Conan 1.13, those bugs have been fixed. Conflicts will be raised, and the final dependency graph when the conflicts are solved, will resemble the first figure, with diamonds. So far, this behavior is not a considered a regression and it won't be reverted, the conflicts totally make sense and are the evidence of something that in the past was failing silently. I am not saying that in some cases the final result couldn't be a good one, but that was mostly a lucky coincidence.

Now the problem is that build_requires do not behave like normal requires. They do not override the same way, because they are essentially private to the package that declares them. If you want to make them work correctly, you need to correctly align the versions used in the different packages requires and build requires. I think the underlying cause of this discussion is the abuse of build_requires trying to approach a different use case that the one they were designed for. Please let me summarize some critical points from the docs:

  • There are requirements that are only needed when you need to build a package from sources, but if the binary package already exists, you don’t want to install or retrieve them.
  • These could be dev tools, compilers, build systems, code analyzers, testing libraries, etc.
  • They can be very orthogonal to the creation of the package. It doesn’t matter whether you build zlib with CMake 3.4, 3.5 or 3.6. As long as the CMakeLists.txt is compatible, it will produce the same final package.
  • Build requirements do not affect the binary package ID. If using a different build requirement produces a different binary, you should consider adding an option or a setting to model that (if not already modeled).

That means, they are designed for tools like cmake or gtest. Not for libraries to be linked with. They shouldn't affect at all to your binary. It doesn't matter if you are using the CMake from your system, or from a Conan package, and it doesn't matter if you are building your tests linked with gtest or not, the final package (headers, library), should be the same.

This is definitely not the case for an executable linking with libraries. They are really required, they affect the binary, they are not tools that will be installed in the system. Another view, would be what happens if now the libraries are shared instead of static. Then, a build_require will not work, as the dependencies are necessary not only at build time, but also at consume time. That could be addressed with some conditional logic that uses build_requires if the some option is shared=False, and regular requires if the option is shared=True, but that construct would be a clear smell.

So, the first stopper, which is building OcrModelBuilder is solved if we use regular requires instead of build_requires in it. Now the problem is moved downstream when we want to use this OcrModelBuilder as a build_require for EuropeanOcrModels, which is trying to do:

image

The different versions of CoreUtils and the different configurations of Protection (with and without all_keys) will conflict, because EuropeanOcrModels need to build, it will build_require OcrModelBuilder, and that will transitively try to plug the incompatible dependencies.

So, what could be a possible approach right now, with the current provided tools? I think you already realized that 2 levels of build_requires is what it is really needed to fully isolate dependencies when they are joined at build time in the same package, coming both from the regular requires and from build_requires. So, as all problems in SW Engineering, I'd say this could be approached adding a level of indirection:

The graph would be:

image

This approach has some other nice properties:

  • The OcrModelBuilder would contain only the executable.
  • The OcrModelBuilderTool package will repackage the executable, but it might be able for example to repackage the shared libraries, if the option shared=True was used to build the OcrModelBuilder.
  • The result would be an independent OcrModelBuilderTool that can be used in the same way a CMake package would be use.

Please check the repo at https://github.com/memsharded/tools_build_requires, it implements exactly this later graph

So, in my opinion, the changes introduced in Conan 1.13 and maintained in Conan 1.14, (modulo the bug fix pending for Conan 1.14.2 in #4937) are valid, and actually protecting from very dangerous effects of abuse of build-requires, like linking the wrong dependency version, or what could be worse, to link with the wrong all_keys=True option in production without even noticing.

Now, it is true that a mechanism to be able to depend on tools with build_requires, when those tools also have transitive and incompatible dependencies with our main dependency graph is necessary.
I think the possible approach with current tools is the creation of a dedicated "Tool" re-package. Also, this should probably be the point where a new model is necessary, to specify that EuropeanOcrModels depends as build_require to OcrModelBuilder, AND that it doesn't need its transitive dependencies. Or maybe a characteristic in OcrModelBuilder, that marks the package as a "Tool", and it knows how to model that regarding the transitivity. Those are very preliminary ideas, and it would be a complicated thing, but I feel that the real solution goes in this direction.

Looking forward your feedback, if necessary we could even do a call/chat in slack or zoom.

Thanks very much for all your detailed feedback, your prepared code, etc., you rock! It is great to be able to work this way :) :) :)

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Apr 10, 2019

Wow!! That's really huge feedback! Thank you very very much! I will definitely try this approach with another level of indirection (i.e. the OcrModelBuilderTool package) and report back if I stumble upon new problems.

However, since I am currently in the middle of another project, it will probably take me a few days to find time to try this, so stay tuned!

Anyway, @memsharded, this comment of yours is so detailed and great that it could be reused as a case study of how Conan can be used in rather complex development environments (maybe a blog post? or special part of documentation with case studies?). Of course, I first need to check that it really works for my real use case.

@memsharded
Copy link
Member

Yes, definitely. I wanted to make sure first that we are addressing this issue in a way that make sense for most parties, and when we are all ok, it will be then converted to documentation.

So looking forward to your feedback when you manage to try it. We will keep working on it, also checking the specific use cases of @puetzk and @thorntonryan too, will report progress here.

Cheers!!

@thorntonryan
Copy link
Contributor

Okay, FWIW, I've uploaded my example here too:
https://github.com/thorntonryan/conan-requirements-overriding-change-in-behavior

Problem still exists on 1.14. Haven't tried 1.14.2 yet.

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Apr 17, 2019

Hi, sorry for reporting back late, but I've tested this approach on our codebase and so far it works flawlessly. Thank you again! I think this issue can be closed now and your comment added to the documentation.

@memsharded
Copy link
Member

Hi @DoDoENT

Thanks very much for your feedback and for your efforts trying this approach!

It would also be great if you could try the changes proposed for a patch Conan 1.14.4 here: #4987, they affect mostly privates, but it would be better if you could try everything is ok.

I will keep this issue open at the moment, as @thorntonryan might still have issues.

@thorntonryan
Copy link
Contributor

thorntonryan commented Apr 17, 2019

@memsharded , thanks for the update! @puetzk and I haven't had a chance to test out your fix yet, but it's definitely on our todo list. We'll report back once we have more information.

@memsharded
Copy link
Member

No problem, take your time, thanks for following up @thorntonryan 😄

Looking forward your feedback when possible. Cheers!

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Apr 18, 2019

Hi @memsharded, I've tried your patch for v1.14.4 and it works for us. However, we were using privates only as described in my example and replaced them with build requirements, which triggered this issue. Since we are not using shared libraries in our codebase (at the moment), but only statics and executables, we are OK with using your workaround with an additional level of indirection and build requirements as you described above.

Thank you for all the hard work and your continuous support!

@thorntonryan
Copy link
Contributor

thorntonryan commented Apr 18, 2019

Ok, I'm still trying to parse what we should be doing based on how we've been apparently mis-using build_requirements.

According to #4753 (comment) and the doc for Build Requirements, it looks like build_requirements are intended primarily for build tools (e.g. CMake, dev tools, compilers, testing libraries, etc.).

As as summarized above:

That means, they are designed for tools like cmake or gtest. Not for libraries to be linked with. They shouldn't affect at all to your binary. It doesn't matter if you are using the CMake from your system, or from a Conan package, and it doesn't matter if you are building your tests linked with gtest or not, the final package (headers, library), should be the same.

And so, it is correct usage for build_requirements to reference build tools like cmake and jom or test libraries like gtest. That's the intended usage.

This is definitely not the case for an executable linking with libraries. They are really required, they affect the binary, they are not tools that will be installed in the system. Another view, would be what happens if now the libraries are shared instead of static.

However, we are doing exactly this, which appears to be our problem.

In a variety of places, we have static libraries listed as build_requires because it seemed to align with:

There are requirements that are only needed when you need to build a package from sources, but if the binary package already exists, you don’t want to install or retrieve them.

Excerpt from Build Requirements

These pseudo "build_requirements" of ours are only needed when building our static library from source, and otherwise aren't part of our static libraries' public API. If the static library's package has already been built, then there's no need for a consumer to download these "build_requirements".

If I'm following, is the suggestion to make "build_requirements" such as these into Private Requirements ?

private: a dependency can be declared as private if it is going to be fully embedded and hidden from consumers of the package. Typical examples could be a header only library which is not exposed through the public interface of the package, or the linking of a static library inside a dynamic one, in which the functionality or the objects of the linked static library are not exposed through the public interface of the dynamic library.

Excerpt from Private Requirements

And so requirements on libraries that are required to build a static library from source, but are not otherwise part of its public API (e.g. boost, etc.) should be marked as "private" requires instead of build_requires ?

@DoDoENT
Copy link
Contributor Author

DoDoENT commented Apr 18, 2019

@thorntonryan, let me ask you about this part:

These pseudo "build_requirements" of ours are only needed when building our static library from source, and otherwise aren't part of our static libraries' public API. If the static library's package has already been built, then there's no need for a consumer to download these "build_requirements".

So if I understood correctly, you need package A (static library) when building package B (also a static library), but when package C (executable or shared library that is a user of package B) uses B, it does not need to see the headers of package A, as public API of B hides public API of A. However, when linking C, it needs to also link to A (because B uses it internally).

Is that the case?

If no, can you then please explain further why is A even needed for B?

If yes, then B must have a normal requirement on A (neither private nor build), as it must transfer its dependency downstream, so that C knows that it needs to link to it. However, this also transfers the visibility of A's public API to C, which is something I opened issue #4038 for (I don't know of any workarounds for that at the moment).

thorntonryan added a commit to thorntonryan/conan-requirements-overriding-change-in-behavior that referenced this issue Apr 24, 2019
Looks like we were misusing build_requirements

See conan-io/conan#4753 (comment)
and conan-io/conan#4753 (comment)

Leaving the COM libraries as requires also makes some logical sense,
because the DLLs are not header only or static, therefore they are not
embedded inside the mock static SharedGuiLib, and consuming applications
linking against SharedGuiLib should still need these DLL binaries in some way.

Arguably, SharedGuiLib *could* have a "private" requires on these
COM libraries.

However, that's a bit of an academic argument due to the nature of our COM
libraries, which maintain strict semver ABI guarantees and for many applications
are installed system wide and not shipped alongside the consuming tool.

In our case, these COM libraries are not part of the public API of SharedGuiLib,
so "private" seems appropriate, but private overrides don't seem to work
that way in Conan 1.14.4

So, committing our workaround, which we'll take in practice,
and will submit further examples to illustrate our remaining issues.
@kad-beekw
Copy link

I have this issue with the following Conan file:

[build_requires]
cmake/3.20.2
[requires]
qt/6.0.4

This does not install because of the following conflict:

ERROR: Conflict in cmake/3.20.0:
    'cmake/3.20.0' requires 'openssl/1.1.1j' while 'qt/6.0.4' requires 'openssl/1.1.1k'.
    To fix this conflict you need to override the package 'openssl' in your root package.

Adding openssl/1.1.1j or openssl/1.1.1k to [build_requires] and/or [requires] does not change the situation for me.

My current workaround is to not use build requirements, but this is not a long-term solution.

@memsharded
Copy link
Member

Hi @kad-beekw

When you do install, make sure to use both --profile:host myprofilehost --profile:build myprofilebuild.
That will allow to coexist 2 openssl, one in the build context, dependency of cmake, and the other in the host context, dependency of qt.

This will be the mode of operation in Conan 2.0.

@AndreyMlashkin
Copy link

This issue was reported in 2019 and it seems, it will be long time "known issue".
I think, it is time to document it.

https://docs.conan.io/en/latest/versioning/introduction.html
'Version and configuration conflicts' section should include information about this limitation

@memsharded
Copy link
Member

Closing this ticket as solved in Conan 2, please create new issues against Conan 2 if necessary, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

6 participants