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

[question] How to I model private (lib-only) component dependencies? #16190

Closed
1 task done
paulharris opened this issue May 1, 2024 · 10 comments
Closed
1 task done
Assignees

Comments

@paulharris
Copy link
Contributor

What is your question?

I have made an example project
https://github.com/paulharris/conan-test-private-components

Two scripts:
test_fails_include.sh -- adds the component-requirement, but consumer can see header
test_fails_link.sh -- does NOT add the requirement, and the consumer cannot successfully link

How do I add a "libs only" component-requirement on another component in package_info() ?
I have a recipe with two components. Component public_component links to component private_component.
My consumer depends on public_component only, and SHOULD NOT have access to the headers from the private component (this is the key point).

In package_info() of the recipe, I should normally have a line that says:
self.cpp_info.components["public_component"].requires = ["private_component"]

But if I add that,
because I noticed that if I do that, my consumer then depends on PRIVATE too, and has access to PRIVATE's header files.

Skipping the 'requires' line works fine for DLL/shared lib builds.
But for the static lib, PRIVATE is not added to the final link lib list.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded memsharded self-assigned this May 1, 2024
@memsharded
Copy link
Member

Hi @paulharris

I am not sure if I understand the issue.
There are no such a thing in C++ like "private headers". If you package the "private_components" headers in the package, and you define:

self.cpp_info.components["private_function"].includedirs = ["include/private_component"]

Then, those headers cannot be "private". This is not how headers inclusion works in C++. If the include/private_components are included from the public headers in include/public_component, then there is nothing that any tool can do to make them "invisible" to the consumers, they must be in the -I<path> of the compiler args, and as such, they will always be visible to consumer users.

The only way that headers won't be visible to downstream consumers is when they are an internal implementation detail of the package, that is, when they are included exclusively by .cpp files, that are built into the binary, and those headers are not packaged at all. The moment the headers are packaged and #include included by other headers, then they are visible to consumers, no way C++ build model can avoid this.

In your case of your code, I don't see any such #include in the public_function.h header, so the solution would be: do not package at all the include/private_components, and do:

self.cpp_info.components["private_function"].includedirs = []

Please try that and let me know.

@paulharris
Copy link
Contributor Author

I guess the confusion comes from me saying "public" and "private".
What you say is true IF consumers would never want to use the "private" headers, but they do.
Instead, I should say "component-a" and "component-b".

I will adjust my example project to suit.

Notes:

  • component-a (currently) requires component-b for linking.
  • consumer may want to use one of those components, or both.
  • component headers are in subfolders, and included without the subfolder mentioned, ie #include <file_from_a.h> ... ie each component used will add its own include folder to the compiler argument list.

Also note that component-a's public headers do NOT include component-b's headers, it is an internal dependency. So if component-a is required, component-b's header dir should not also be added to the argument list.

The library has many parts, with the headers in different subfolders.
Adding the component to cmake's target_link_libraries(consumer lib::component-a) will add the include path to the compiler command line, and allow the consumer to #include <file_from_a.h>.
This is how this particular library works.

The goal is to ensure that if a consumer did NOT do target_link_libraries(consumer lib::component-b) will not be able to #include <file_from_b.h>.

The reason is that you can get "hidden dependencies" that way.
If the consumer only links to component-b, they should not be able to #include component-a.
Linking is a separate topic.

For example, consumer does target_link_libraries for component-b only, but also includes component-a headers due to fault in the conan recipe.
Everything will work while component-b depends on component-a.

BUT, if a conan-library-option changes, or the code changes, and component-b no longer links to component-a, then the headers will still #include but the linker will fail.
And worse, it could be a deep dependency with everything statically linked, so it won't fail until the very final executable is linked. So the error is far removed from the cause.

This problem is solved at the package level, with the transitive headers and default 'private' dependencies. But this same problem also occurs at the component level, and I don't see a mechanism to model this.

@paulharris
Copy link
Contributor Author

There, hopefully the example project demonstrates this nicely.

I see this behaviour on a much bigger scale, in a much bigger library/recipe I'm packaging.

@paulharris
Copy link
Contributor Author

I was thinking about this a little more.
If conan wasn't involved, the cmake files for this library project would have 2 components,
with componentA having something like this;

target_link_libraries(componentA
   PRIVATE
      componentB
)

And cmake would handle adding the libraries to the linker.

This doesn't seem possible to model with conan?

As a workaround, I could add componentB's libs to the list of libs for componentA in the package_info() section.
But there is more than one component (there are dozens), so I assume I would have to be careful to add all the dependency libraries in the correct order in the components.

And the libraries would then be repeated in the linker command line quite a lot, as it is common to depend on half a dozen components, each one of those depending on a tree of components. There would be a lot of common components between them eg a "MathCore" component, for example.

@memsharded
Copy link
Member

This should be possible to model it with Conan:

  • First, do not package at all the headers of compb, they are not necessary, you need to remove the set_target_properties(compb_function PROPERTIES PUBLIC_HEADER "include/compb/compb_function.h") from the CMakeLists.txt to avoid packaging it
  • Do not define the self.cpp_info.components["compb_function"].includedirs = ["include/compb"] in the conanfile.py, as those headers will not be there.
  • The linkage to the library still need to be defined, so self.cpp_info.components["compb_function"].libs = ["compb_function"] must remain, at least if you are linking it as static library. No matter that CMake defines a target_link_libraries(... PRIVATE ...) it will still propagate the linkage requirement of the transitive static library if necessary. Conan works the same with the .libs= definition, it will only really propagate it when needed, when it is not, it will not be propagated and it will be fully private.

So, in short, unless I have understood it incorrectly, it is just a mistake in the CMakeLists.txt packaging private headers that shouldn't be packaged and the conanfile.py defining the includedirs for a component that shouldn't define includedirs.

Please, let me know if this clarifies the issue.

@paulharris
Copy link
Contributor Author

I don't think you are understanding correctly. I'll try again.

The consumer may want to use compa, OR, compb, OR both compa and compb.

CompB must also be packaged.

The goal is that if your consumer's cmake target asks for CompA only, you do not get accidental access to CompB's headers.
and, vice versa, if your
If your consumer's cmake target asks for CompB only, you do not get accidental access to CompB's headers.

And the consumer can ask for both.

So for example, my project is cmake.
It has one module, ModuleX.

target_link_libraries(ModuleX
   PUBLIC
      whatever::compa)

In this case, if ModuleX's source code has this: #include <compb_function.h> then it should fail to compile.


And, in the same project, I have another module, ModuleY.

target_link_libraries(ModuleY
   PUBLIC
      whatever::compb)

In this case, if ModuleY's source code has this: #include <compa_function.h> then it should fail to compile.


And, in the same project, I have another module, ModuleZ.

target_link_libraries(ModuleZ
   PUBLIC
      whatever::compb
      whatever::compa
)

In this case, if ModuleZ's source code can #include both compa_function.h and compb_function.h successfully.

@memsharded
Copy link
Member

I see now what you mean.

No, I am afraid it is not possible. Header visibility is not defined at the component level. If the headers of compb are in the package, and they are exposed in the cpp_info via includedirs, they are visible for all consumers that transitively require it. As long as it is possible to directly require that component and have access to its headers, then the headers will be visible when it is transitively required.

@paulharris
Copy link
Contributor Author

So then conan can't fully model what cmake can do with its target_link_libraries() ?
ie in pure cmake, I would do something like...

add_library(compb ...)
add_library(compa ...)
target_link_libraries(compa   PRIVATE compb)

add_executable(something thecode.cpp)
target_link_libraries(something
   PUBLIC (or whatever)
      compa)

and "thecode.cpp" wouldn't get access to compb's headers.

Perhaps put this on the radar for the future?
There might be an opportunity to add support for this in some future conan release?


As a workaround, all I need is to add compb's libs to compa.
But, my question is, are the repeated libraries going to cause a problem with the linker?

For example

    def package_info(self):
        self.cpp_info.components["compb_function"].libs = ["compb_function"]
        self.cpp_info.components["compb_function"].includedirs = ["include/compb"]

        self.cpp_info.components["compa_function"].libs = ["compa_function"]
        self.cpp_info.components["compa_function"].includedirs = ["include/compa"]

        if self.options.require_compb:
            self.cpp_info.components["compa_function"].libs.append("compb_function")

I would have to be careful to append all the libs to all the dependent modules, but I think it can be done in my case. There is a whole tree of component dependencies, so it isn't so simple as this example.

Is the 'libs' a simple python list? Perhaps I could do:

self.cpp_info.components["compa_function"].libs.append( self.cpp_info.components["compb_function"].libs )

So then if a consumer requires/links to both compa and compb, then the compb_function library will be repeated twice in the linker command... right?
Does conan/cmake/others automatically simplify those lists?

@memsharded
Copy link
Member

So then conan can't fully model what cmake can do with its target_link_libraries() ?

Conan can model the visibility with even more control than cmake at the package level. But it is true that components have other scope, in this case, when some visibility of the headers is done for one component, then that visibility is defined at the "package" level, it is not possible to constraint the visibility of those headers for components transitivity.

Perhaps put this on the radar for the future?
There might be an opportunity to add support for this in some future conan release?

So far, it doesn't seem possible. That would imply to extend the traits model to the components, and that sounds too excessive for the value, bringing a ton of complexity for some relatively small protection against user errors, that wouldn't even work for all build systems (other build systems doesn't have the same targets visibility feature). Maybe after the CMakeDeps pending revamp with new targets, we might want to reconsider the possibility, but at the moment this wouldn't be planned.

But, my question is, are the repeated libraries going to cause a problem with the linker?

I think that for most modern linkers it shouldn't be a big issue, but this is not guaranteed.

I would have to be careful to append all the libs to all the dependent modules, but I think it can be done in my case. There is a whole tree of component dependencies, so it isn't so simple as this example.

The question is: is it really worth it? Conan 1 has served well for many years without any protection/hiding of transitive headers at all. All headers of all packages were always visible to the consumers.
It is true that developers might from time to time do errors and include a transitive components that was not explicitly required. But in practice this wouldn't be a huge problem, yes, components might evolve in future versions, and removing a component that got some headers transitively included by consumers without being explicitly required might happen. But how often? Is it really worth the complexity added to the recipes and other possible issues that could derive from this complexity? In my experience, it is typically not worth it, and other processes, like code reviews might also further reduce the issue.

Does conan/cmake/others automatically simplify those lists?

No, Conan does not simplify/collapse or remove duplicates from those lists. Sometimes, when there are circular dependencies, some linkers might actually need the repetition of some libraries.

@paulharris
Copy link
Contributor Author

Ok thanks, will accept the limitations and hope for the future.

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

No branches or pull requests

2 participants