Git Product home page Git Product logo

Comments (10)

memsharded avatar memsharded commented on July 18, 2024

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.

from conan.

paulharris avatar paulharris commented on July 18, 2024

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.

from conan.

paulharris avatar paulharris commented on July 18, 2024

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.

from conan.

paulharris avatar paulharris commented on July 18, 2024

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.

from conan.

memsharded avatar memsharded commented on July 18, 2024

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.

from conan.

paulharris avatar paulharris commented on July 18, 2024

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.

from conan.

memsharded avatar memsharded commented on July 18, 2024

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.

from conan.

paulharris avatar paulharris commented on July 18, 2024

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?

from conan.

memsharded avatar memsharded commented on July 18, 2024

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.

from conan.

paulharris avatar paulharris commented on July 18, 2024

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

from conan.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.