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

Define a target per library when multiple libraries in a package #1323

Closed
packadal opened this issue May 24, 2017 · 21 comments
Closed

Define a target per library when multiple libraries in a package #1323

packadal opened this issue May 24, 2017 · 21 comments

Comments

@packadal
Copy link

I am using a Boost conan package, and I'd like to specify against which libraries I link, while still benefiting from using CMake targets.

Ideally, I'd like to have a global target as it stands today (e.g. CONAN_PKG::Boost) and one additional target per library defined (e.g. CONAN_PKG::Boost_thread)

This would require the generated conanbuildinfo.cmake to create more targets, but would allow for much more flexibility when using the package.

Best

@memsharded
Copy link
Member

The thing to achieve this is that you force the package creator to define the full model of dependencies inside the package. In this case, the boost package should contain a complete dependency graph of the internal relations of boost libraries, to be able to define such targets properly. As you know, this is not a simple task, there are even tools like bcp to analyze such dependencies.

Besides that, the true issue is that having a package for the whole boost is not the proper approach. Implementing a dependency manager internal to packages is an unnecessary complexity. The good one would be to have one package per library, and let the package manager handle it, this is exactly the reason of a package manager. I think we should start considering modularizing Boost in this way, though it can take some time.
Thanks for your time!

@packadal
Copy link
Author

Hmm, you are right, I did not realize this would push the burden of maintaining dependencies to the package.

So if the proper way is to modularize boost there it would be extremely helpful to be able to create multiple packages from the same souce/build.

Because boost sources are distributed as monolithic, building one boost library would require downloading the sources multiple times, one for each dependency. My specific case requires using boost's bcp tool to rename the boost namespace, in order to avoid symbol conflicts. This causes the sources to be copied from the build folder to the sources folder, then to the bcp folder with the new namespace. Since boost is comprised of a lot of header files, this operation is quite costly on windows machines.
Separating the package in multiple packages would make the build time of the package skyrocket unless multiple packages were generated at the same time.
Does this goes against the philosophy of conan ?

Another way to do it would then be to create a single 'master' package that contains the whole set of boost libraries, and then create conan packages that contain each one single lib, with the dependencies graph. Is this better in your opinion ?

Thanks

@memsharded
Copy link
Member

You might be interested in: http://docs.conan.io/en/latest/packaging/package_info.html#build-once-package-many

This is not possible to create different packages (with different names, versions, user, channel), but to create different binaries.

Yes, my opinion is definitely the second one, basically create conan packages, one for each library, let the package and dependency management resolve them, and yes, using a "project" master package that require all of them to be able to fire complete builds, or to depend on it as a whole, seems the way to go. The only problem with it is that it requires a large investment, as Boost is large. But we are analyzing it, might try to make a proof of concept.

@packadal
Copy link
Author

Thanks a lot !

@csteuer
Copy link

csteuer commented Dec 1, 2017

I have a cmake library project, lets call it libA

The project contains two targets:

  • libA: the actual library
  • libA-testutils: contains mocks and matchers for the classes in libA

The test suite that is part of the project links both targets

Another cmake library project (libB) requires libA.
The library just links libA and the test suite links: libB, libA and libA-testutils.

Now we are switching to conan and i can't see a way to maintain this separation since a conan package only exports one cmake target.
I do not want libB to link against libA-testutils and include the corresponding headers.

I can't see a way to resolve this without the ability to export multiple cmake targets from a conan package.

@memsharded
Copy link
Member

Hi!
First, why are you packaging LibA-testutils at all? I think the consumers of LibA doesn't require it. Also, how does your package_info() method looks like? Because if you don't declare the dependency to it, consumers won't link to it.

Finally, conan now create targets:
CONAN_PKG::PkgName
CONAN_LIB:LibA
CONAN_LIB:LibA-testutils
With CONAN_PKG::PkgName => CONAN_LIB::LibA
CONAN_PKG::PkgName => CONAN_LIB::LibA-testutils

Also, it is important to know which version are you running, these later changes are in 0.29. Please tell me, thanks!

@petermbauer
Copy link

Great to hear, is this new feature somewhere jn the docs?

@memsharded
Copy link
Member

Nope, the truth is that it was supposed to be an internal implementation detail, mostly transpared to users. Added it as an issue to the docs: conan-io/docs#389

@petermbauer
Copy link

thank you! We plan to have rather big packages with lots ob libs, at least in the first place, so its nice to have that feature.

@csteuer
Copy link

csteuer commented Dec 3, 2017

@memsharded Thanks for the quick response.

I'm using version 0.29.

First, why are you packaging LibA-testutils at all?

The test suite of libB uses gtest-matchers and mocks defined in libA-testutils.

E.g. libA defines a class Vector and libA-testutils contains gtest-matchers for Vector. The matchers are reused in the test suite of libB. The dependencies should look like this:

  • libB requires libA
  • libB-tests requires libB and libA-testutils

how does your package_info() method looks like

def package_info(self):
	self.cpp_info.libs = ["libA", "libA-testutils"]
	self.cpp_info.includedirs = ["include", "testutils/include"]

I tried the feature that you mentioned above and it almost does what i need:

target_link_libraries(libB CONAN_LIB::LibA_libA)
target_link_libraries(libB-tests libB CONAN_LIB::LibA_libA-testutils)

However, since the INTERFACE_INCLUDE_DIRECTORIES property is not set on the CONAN_LIB targets i have to add:

include_directories(${CONAN_INCLUDE_DIRS_LIBA})

Thats better than before, but it would be nice to have the INTERFACE_INCLUDE_DIRECTORIES property per CONAN_LIB target.

@memsharded
Copy link
Member

The test suite of libB uses gtest-matchers and mocks defined in libA-testutils.

Ah, ok, that makes sense, I thought it was only used for testing LibA, because of its name

Thats better than before, but it would be nice to have the INTERFACE_INCLUDE_DIRECTORIES property per CONAN_LIB target.

But this is almost impossible, because you don't know which include directories belong to each target. Adding all include directories, like include dirs for LibA and Liba-testutils, to both CONAN_LIB::LibA and CONAN_LIB::LibA-testutils would be very confusing, because they will have unrelated include directories. There is no way conan can know which include directories belong to each library, that is why all include directories are associated to the CONAN_PKG target, not to the libs. Does this make sense?

Maybe another possibility would be to package LibA-testutils in its own package, it sounds good to me. So, for example, when you deploy the final application, there is no such testing library there.

@csteuer
Copy link

csteuer commented Dec 3, 2017

Does this make sense?

Yes it does

Would it be possible to extend/change the package_info method so that one can define includedirs per lib (without breaking compatibility with the current way of defining includedirs)?
E.g. instead of passing a list to cpp_info includedirs one can also pass a dictionary that maps from lib name to includedirs like this:

def package_info(self):
  self.cpp_info.libs = ["libA", "libA-testutils"]
  self.cpp_info.includedirs = {
    "libA" :  ["include"], 
    "libA-testutils" : ["testutils/include"]
  }

Maybe another possibility would be to package LibA-testutils in its own package, it sounds good to me. So, for example, when you deploy the final application, there is no such testing library there.

The problem is that the test suite of LibA uses the matchers/mocks as well. If i put LibA-testutils into its own package i would have to move the tests of LibA into that package as well, which i find a bit weird.

However; i'm not sure yet how to do the test+CI part with conan.
My first approach was to run the tests as part of the build but thats problematic when doing cross builds. Maybe i will put the tests into their own cmake project... then i would also make a conan package out of the test utils.

Any suggestions/best practices?

@memsharded
Copy link
Member

The "multi-config" system is not restricted to build-types only, you can do something like:

def package_info(self):
    self.cpp_info.includedirs = ["include"] # as they are common, no need to sefl.cpp_info.libA
    self.cpp_info.libAtestutils.includedirs = ["testutils/include"]

This will generate in conanbuildinfo.cmake, some extra variables, one of them being CONAN_INCLUDE_DIRS_PKGNAME_LIBATESTUTILS, which value will be "testutils/include". The only issue with this variable is that it is not automatically applied to the target, but at least you can use it in your cmake code

From your feedback I would say that a possible configuration would be:

  • Package "testutils" with those matchers, mocks. If they are used both in LibA and LibB, then it seems they have their own entity
  • I tend to favor tests of each package in the same package, at two different levels: the library unit-tests are to be run in the build() method. Read the build as building the package, which also runs the tests. Then there are the "test_package", which is done by conan, to ensure the package is properly created, but not intended to run full suite tests
  • Then, the tests are better inside the same package
  • A good approach could be to define a build_requires to the "testutils" package, so it is only retrieved/dependent when it is necessary to build from sources. Also, upgrading the test suite doesn't change the package binaries IDs
  • build_requires can be defined in the same recipe, or in a profile. Maybe use the later if you upgrade frequently the "testutils", also, you can use that to enable/disable running the tests (based on the existence or not of the build_requires)

@csteuer
Copy link

csteuer commented Dec 4, 2017

Thanks for the "multi-config" hint. It works perfectly.

If I understand you correctly you would put the testutils into their own conan-package but still have the tests suite as a part of the library-package. The test suite is only built when build_requires is set (e.g. build_requires=gtest).
But doesn't that create a cyclic dependency between the library-package and the testutils-package?

@memsharded
Copy link
Member

Oh, I wasn't aware that LibA-testutils depended on LibA, I thought they were mocks/matchers, but were actually a replacement, without a dependency to LibA. If they depend, then, you are right, that would be a cyclic dependency, which is impossible, and they should live in the same package.

@mtvec
Copy link

mtvec commented Jul 12, 2018

The only issue with this variable is that it is not automatically applied to the target, but at least you can use it in your cmake code

@memsharded: Are there plans to address this issue? It would be nice if we could just link against targets and not have to manually add include directories.

By the way, my use case is for the gtest package which creates multiple libraries (e.g., gmock, gmock_main, and gtest) but you only ever want to link against one of those. Creating three different packages for those libraries also seems overkill.

@memsharded
Copy link
Member

Hi @JobNoorman

Not sure what you mean.

The cmake generator already generates targets, for each package. So you can specify to link targets, no need to manually add include directories. Actually, it was never necessary to manually add include directories, as the conan_basic_setup() from the generated conanbuildinfo.cmake already does the job for you.

The package level targets are called CONAN_PKG::xxx and they are an imported target that holds all the libraries within the package. Every library will have its CONAN_LIB::${package_name}_${_LIBRARY_NAME}${build_type} target (the CONAN_PKG target depends on the CONAN_LIB targets). You can use them in your CMakeLists.txt as you want.

@mtvec
Copy link

mtvec commented Jul 12, 2018

I was referring to the CONAN_LIB targets which, as far as I can tell, don't have the INTERFACE_INCLUDE_DIRECTORIES property set, even if the multi-config approach you mention above is used. This means that I have to do the following to link against, in my use case, gmock:

target_link_libraries(some_exe PRIVATE CONAN_LIB::gtest_gmock)
target_include_directories(some_exe PRIVATE ${CONAN_INCLUDE_DIRS_GTEST_GMOCK})

By having Conan set the INTERFACE_INCLUDE_DIRECTORIES in CONAN_LIB::gtest_gmock, we could avoid the second line. My question was whether there are plans to implement this.

@memsharded
Copy link
Member

I see. But the problem is that there is no way to know what would be the include directories for each individual library. Conan could set the CONAN_LIB::xxx include directories to the CONAN_PKG::xxx package include directories, but it feels a bit dirty, isn't it?

@mtvec
Copy link

mtvec commented Jul 13, 2018

From the example you gave above, I gathered that a CMake variable called CONAN_INCLUDE_DIRS_GTEST_GMOCK would be created containing whatever I added to self.cpp_info.gmock.includedirs inside package_info. This variable could then also be used to populate the INTERFACE_INCLUDE_DIRECTORIES property on the CONAN_LIB::gtest_gmock. Am I missing something?

@memsharded
Copy link
Member

@JobNoorman That is an interesting idea. The "multi-configurations" nested in self.cpp_info generates some cmake variables, but they are not correlated to the libraries. Maybe matching the names... This has to be thought, because, similarly to the self.cpp_info.gmock.includedirs the other fields should be consider self.cpp_info.gmock.libdirs, self.cpp_info.gmock.lib, etc. Also, what if gmock depends on gtest internal libraries? That probably should be modeled too.

I think this is very related to the issue of #2387. Maybe the best would be to follow information there. Please check. Thanks!

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

No branches or pull requests

5 participants