[question] Whole archive linking

rddesmond commented 5 years ago

Good afternoon!

Conan is an awesome tool, and I'm trying to wrangle it to help build a machine learning project. One of the design goals is to use Caffe statically in the final binary. This software package has runtime registration of "layers", so the whole archive must be linked for it to work. In GCC, this translates to:

-Wl,--whole-archive -lcaffe-nv -lproto -Wl,--no-whole-archive

My first cut in conan was to translate put this into the cpp_info.libs, without the -ls so that the libraries could be resolved to absolute paths.

self.cpp_info.libs = [
    "-Wl,--whole-archive",
    "caffe-nv",
    "proto",
    "-Wl,--no-whole-archive",
]

The problem is that this causes the caffe and all it's dependencies to be placed between the whole-archive block.

This lead me to option 2:

self.cpp_info.libs = [
    "-Wl,--whole-archive",
    "-L" + os.path.join(self.package_folder, 'lib'),
    "-lcaffe-nv",
    "-lproto",
    "-Wl,--no-whole-archive",
    "caffe-nv",
    "proto",
]

To force the libraries to be used without dependency resolution in the whole-archive block and then again outside with dependency resolution.

Do you have any thoughts on this approach?

To help us debug your issue please explain:

[X] I've read the CONTRIBUTING guide.
[X] I've specified the Conan version, operating system version and any tool that can be relevant.

Ubuntu 16.04.5 LTS Conan version 1.12.0 cmake version 3.12.1 gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609 Caffe: nvidia 0.17.2

[X] I've explained the steps to reproduce the error or the motivation/use case of the question/suggestion.

memsharded commented 5 years ago

Hi @rddesmond

Are you aware of other cpp_info attributes? Please have a look to: https://docs.conan.io/en/latest/reference/conanfile/attributes.html?highlight=cpp_info#cpp-info

I'd say that you could put those flags in self.cpp_info.exelinkflags, if I understood correctly, please have a look and tell us. Thanks!

uilianries commented 5 years ago

I think this case is very similar to Abseil

The libs listed in cpp_info are appended at the end of compiler command, however he needs to "wrap" the libs in the command, instead of just adding new flags. The exelinkflags will not help in this case, it will append more flags only, not wrapping.

rddesmond commented 5 years ago

Both great comments, thank you!

Two follow up questions:

Is the ordering between consistent (at both a practical and documented stability level) between exelinkflags/sharedlinkflags and libs? One thing with this is that the order does matter -- it needs to be first the whole-archive libraries and then the dependency tree.
Is there a way to put in libraries that are resolved, but not with dependencies. (In this case, I used -l<library> so the gcc linker did the resolution, but is that the right way?)

szmyd commented 4 years ago

@rddesmond I solved this myself for DPDK and SPDK doing the following hack in their package_info methods:

        self.cpp_info.libs = [
                              "-Wl,--whole-archive -lrte_eal",
                              "rte_mempool",
                              # // snip
                              "rte_hash",
                              "-lrte_cryptodev -Wl,--no-whole-archive"
                              ]

This prevented conan from seeing them as duplicate "libraries" and filtering out only the lowest in the link list. Did you find an alternative solution?

rddesmond commented 4 years ago

@szmyd I had been using option 2 from my original post. That work-around started failing with 1.22.0, which orders system libs after conan libs. (Before, the entries to cpp_info.libs were kept in order so that the conan-resolved caffe-nv was after the system whole-archive one. This allowed the conan-resolved archive's symbols to get passed over. Now that it's reordered, the whole archive is second. The linker pulls in symbols a second time and linking fails.)

self.cpp_info.libs = [
    "-Wl,--whole-archive",
    "-L" + os.path.join(self.package_folder, 'lib'),
    "-lcaffe-nv",
    "-lproto",
    "-Wl,--no-whole-archive",
    "caffe-nv",
    "proto",
]

Mirko-von-Leipzig commented 4 years ago

I've run into the same issue with the UHD library from Ettus (USRP drivers).

The static UHD library requires whole-archive, so we need some way of wrapping just that library in linker commands without including its dependencies.

Our current work-around is by manually overriding it in the users CMake files which is obviously less than ideal.

keef-cognitiv commented 4 years ago

Same issue. This used to work but in our case the libs end up getting pushed up to the front of the list instead of the end so it produces /link/to/static-lib-1.a /link/to/static-lib-2.a -Wl,--whole-archive -Wl,--no-whole-archive which is not particularly useful. Desperately need a work-around.

rddesmond commented 4 years ago

@keef-cognitiv I have since written a patch to the upstream project to make an init function (and I just call in my code which means that there is no longer a need to whole archive link). But see my workaround on Feb 11, I think it still works. The key is is a hack where you make conan think that the library is a system library:

Conan separates the system libs (anything that it does not recognize.. e.g. -Wl,--whole-archive and -lproto, the latter because the compiler prefix obscures the name) from the libs (anything it does recognize.. e.g. proto). In later versions of conan, cppinfo.system_libs was added. I think this separation was put in place for backward compatibility to allow recipes from before this point to benefit from the separation. When conan generates the compiler instructions (i'm looking at the CMake generator, but I imagine others are similar), they are ordered: <libs> <system_libs> <frameworks> <dependencies>).

I also added the library a second time without the prefix to force dependency resolution. This may not be needed anymore.

keef-cognitiv commented 4 years ago

oh ok thanks. I wonder if that works with static libs too. I'll give it a try when the work week starts again.

rddesmond commented 4 years ago

Whole library linking is only applicable for static libraries.

On Sat, Jul 25, 2020, 4:38 AM keef-cognitiv notifications@github.com wrote:

oh ok thanks. I wonder if that works with static libs too. I'll give it a try when the work week starts again.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/conan-io/conan/issues/4480#issuecomment-663829251, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXSRDY6CNZWVFAEAKTNY5TR5KKYVANCNFSM4GUXJ22Q .

SpaceIm commented 3 years ago

Anybody has found a robust way (which doesn't break with new conan versions) to add whole archive for specific libs in cpp_info.libs? It's a big issue for libtorch static, since sereral libs MUST be linked with whole archive: torch, caffe2_observers, torch_cpu, caffe2_protos (wrapper to ensure that symbols of protobuf are not discoped when libtorch is static), torch_cuda, torch_hip, Caffe2_perfkernels_avx, Caffe2_perfkernels_avx2, Caffe2_perfkernels_avx512

Should we use exelinkflags and sharedlinkflags instead of libs? How to know that order of libs will be correct between whole archive libs in exelinkflags, sharedlinkflags and others libs in libs (maybe with components)?

SpaceIm commented 3 years ago

Answering myself:

I had to do something like this (successfully tested with Visual Studio & apple-clang for the moment):

    def package_info(self):
        self.cpp_info.names["cmake_find_package"] = "Torch"
        self.cpp_info.names["cmake_find_package_multi"] = "Torch"

        def _add_whole_archive_lib(component, libname, shared=False):
            if shared:
                self.cpp_info.components[component].libs.append(libname)
            else:
                lib_folder = os.path.join(self.package_folder, "lib")
                if self.settings.compiler == "Visual Studio":
                    lib_fullpath = os.path.join(lib_folder, "{}".format(libname))
                    whole_archive = "\"/WHOLEARCHIVE:{}\"".format(lib_fullpath)
                elif self.settings.compiler == "gcc":
                    lib_fullpath = os.path.join(lib_folder, "lib{}.a".format(libname))
                    whole_archive = "-Wl,--whole-archive,{},--no-whole-archive".format(lib_fullpath)
                elif self.settings.compiler in ["clang", "apple-clang"]:
                    lib_fullpath = os.path.join(lib_folder, "lib{}.a".format(libname))
                    whole_archive = "-Wl,-force_load,{}".format(lib_fullpath)
                else: # no idea of the syntax for others compilers
                    whole_archive = "-l{}".format(libname)
                self.cpp_info.components[component].exelinkflags.append(whole_archive)
                self.cpp_info.components[component].sharedlinkflags.append(whole_archive)

        # torch
        _add_whole_archive_lib("_libtorch", "torch", shared=self.options.shared)
        self.cpp_info.components["_libtorch"].requires.append("libtorch_cpu")

        # torch_cpu
        _add_whole_archive_lib("libtorch_cpu", "torch_cpu", shared=self.options.shared)
        self.cpp_info.components["libtorch_cpu"].requires.append("libtorch_c10")

        # c10
        self.cpp_info.components["libtorch_c10"].libs = ["c10"]

        # More libs (one component per lib for this trick), some with whole archive, some without
        ...

Chemrat commented 3 years ago

Putting whole-archive in exelinkflags for the library results in those flags being used twice, I suppose when linking the executable itself and when linking the library, which naturally results in multiple definitions error. My conanbuildinfo.txt is generated with those flags in two sections simultaneously: in exelinkflags_MYLIBRARY and exelinkflags.

hoxnox commented 2 years ago

There is another problem with exelinkflags workaround. Libraries can be dependent on the other libraries, which are in the regular cpp_info.libs. So when cmake discovers compiler capabilities it can't link sample programs - exelinkflags is set, but libs is not.

I'm stuck with the problem on dpdk. It requires whole achive.

hoxnox commented 2 years ago

In my case it was acceptable to treat all these dpdk stuff as one lib:

    def package_info(self):
        libs=(
            'kvargs', # eal depends on kvargs
            'telemetry', # basic info querying
            'eal', # everything depends on eal
            'ring',
            'rcu', # rcu depends on ring
            'mempool',
            'mempool_ring',
            'mempool_bucket',
            'mempool_stack',
            'mbuf',
            'net',
            'meter',
            'ethdev',
            'pci', # core
            'cmdline',
            'hash',    # efd depends on this
            'timer',   # eventdev depends on this
            'acl',
            'bbdev',
            'compressdev',
            'cryptodev',
            'distributor',
            'efd',
            'eventdev',
            'ip_frag',
            'lpm',
            'member',
            'pcapng',
            'rawdev',
            'regexdev',
            'dmadev',
            'rib',
            'reorder',
            'sched',
            'security',
            'stack',
            'ipsec', # ipsec lib depends on net, crypto and security
            'fib', #fib lib depends on rib
            'port', # pkt framework libs which use other libs from above
            'graph',

            # drivers

            'bus_pci',
            'bus_vdev',
            'bus_vmbus',

            'net_i40e',
            'net_e1000',
            'net_ixgbe',
            'net_virtio',
        )

        libs_s = "{0}/librte_" + ".a {0}/librte_".join(libs) + ".a"
        self.cpp_info.libs = ["-Wl,--whole-archive {} -Wl,--no-whole-archive -lbsd".format(libs_s.format(os.path.join(self.package_folder, "lib")))]

Chemrat commented 5 months ago

From what I understand situtaion is even more dire with CMakeDeps generator (I'm still on conan1 tho), because it expects everything in libs being a real library:

CMake Error at build/GCC 12.2.0 x86_64-unknown-linux-gnu/devel/cmakedeps_macros.cmake:39 (message):
[cmake]   Library '-Wl,--whole-archive' not found in package.  If
[cmake]   '-Wl,--whole-archive' is a system library, declare it with
[cmake]   'cpp_info.system_libs' property

instead, I had to put everything into system_libs. After this conan seemingly no longer adds linker search paths to -L (because the package has no libs?) so I had to fill out cpp_info.sharedlinkflags and cpp_info.exelinkflags 😿

CJCombrink commented 3 months ago

Yes this is dire with CMakeDeps. I am also now trying to migrate and end up with the same problem and no clear way how to proceed.

conan-io / conan

[question] Whole archive linking #4480