-
Notifications
You must be signed in to change notification settings - Fork 990
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
[question] Why is full_deploy skipping binaries #14541
Comments
Hi @tgurriet Conan, when installing dependencies, knows that some binaries are not necessary. For example, if some package needs a The idea is that the |
Good to know, thanks! In our case it's an issue because Wouldn't it make more sense for the default behavior to keep the complete dependency tree during deployment, unless explicitly allowed via requires traits (visible=False, transitive_header=False, etc...), or are we missing something in how we deploy our package. Maybe it's the |
If this is the case, it is important to mark it with
This shouldn't be the case if
This seems not to be the preferred approach, and according to SW engineering, the transitive dependencies shouldn't be able to be explicitly
Not sure what you mean. Header-only libraries often have dependencies, from other header-onlys, but also depend on other compiler libraries. |
You mean that the public headers of my library should not include any header of any of its dependencies? Seems a bit restrictive!? Could you point to where in the talk you address this point? Indeed I'm not sure this completely solves my problem though. Let's say I remove The doc states for So in this case it would feel legitimate to have The doc also states (likely with a typo!?) for It although not 100% obvious, it would sound reasonable having to set Or would it be at the level of the |
Libs are automatically transitive when a static library depends on another static library. It is only not propagated when it is a shared library linking a static library, because the shared library will embed that static library, which by default will be an internal implementation detail of the shared library, unless we explicitly mark it as
header-only libraries already always define
The Please let me know if this clarifies a bit more. |
It does clarifies things a bit, but something is still off in the case I'm talking about. I've created a little test for you to see what I'm talking about: https://github.com/tgurriet/conan_deploy_test You can just execute Thanks for your help! |
@memsharded Should I maybe create an issue for this given that it's doesn't seem to be about skipping binaries but deployers |
Thanks for the project with the code to reproduce, it really helps. I think the main issue is that you are installing the default dependencies, which are |
@memsharded Sorry about the typo. I updated the example, but it still doesn't work 😞 |
Why harcoding the def package_info(self):
self.cpp_info.libs = ["my_lib"] No need to define the properties either, those are the default already. Changing it I managed to move forward, but not completely, but got a final linkage error of SSL things, need to check that |
Good to know, thanks for the info. I've updated the test repo. It seems to me that the issue is with the full_deployer assuming that header only dependencies don't need to be propagated to the consumer by default. |
Lets get rid of the
|
My apologies for previous deleted message, spoke too fast. |
@memsharded I do confirm that it works in both cases when using self.requires("cpp-httplib/0.12.2", transitive_headers=True). But as you can see, Maybe there should be another qualifier like |
@memsharded It seems like it's not a problem with the deployer but a fundamental issue with how header only libraries are considered in the dependency transitivity model of conan. Should I create a specific feature request or bug issue about it? |
Hi @tgurriet I think we can follow in this same thread, not sure if if is fundamentally different. |
@memsharded Sounds good.
Even if I've updated the minimal example repo : https://github.com/tgurriet/conan_deploy_test In that repo, there are 2 scripts to build with deployers See https://github.com/tgurriet/conan_deploy_test/tree/82c2c28b15c310d9d7bba6db959059301bcd5c9c for a commit without |
I am trying to reproduce with a minimal unit-test, and so far it seems to be working fine, adding it in a PR in #14673 This test is validating in # node, headers, lib, build, run
_check_transitive(app, [(libc, True, True, False, False),
(libb, False, False, False, False),
(liba, False, True, False, False)]) The tratis that The code you are providing is a bit more complicated because it depends on third parties packages, but I'll try to have a look. |
Ok, I think I found something:
Then, the solution would be for this case to add |
My bad! I did add the Here is the output:
I'll try to recreate the issue in the PR you created. |
For some reason you still have
But this definitely shouldn't happen, I cannot see it happening in your code either. |
Are you saying that when you run |
I am running in Windows, I get with
|
Interesting... |
Now I managed to reproduce, not sure why I couldn't see that earlier. |
Thanks for all the feedback above, very useful! I have changed the tag to |
Thanks for all this hard work, you guys rock! |
I think the PR in #14673 is ready and should fix the issue. |
It does work on the PR branch, both with and without deployer! |
Amazing! :) :) :) Thanks for the feedback! |
@memsharded , your branch also fixes one more bug I just discovered. I have All in all, I've tried using conan from your PR, and it works correctly. |
What is your question?
Hello,
I have a quick question about the full_deploy deployer in conan2.
In particular, it seems to skip generating CMake config files for some of my dependencies:
What controls which packages get exported during a full_deploy?
Thanks in advance,
Tom
Have you read the CONTRIBUTING guide?
The text was updated successfully, but these errors were encountered: