conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.17k stars 974 forks source link

copy files should preserve symbolic links #204

Closed marache closed 7 years ago

marache commented 8 years ago

When packaging some libs that use semantic versioning it's not rare to find a library using symbolic names between different semantics, i.e. on mac for both static and dynamic :

libpng16.a
libpng.a -> libpng16.a

libpng16.16.dylib
libpng16.dylib -> libpng16.16.dylib
libpng.dylib -> libpng16.dylib

But when packaging these libs with conan I get hardcopies of all libs :

libpng.dylib
libpng16.16.21.0.dylib
libpng16.16.dylib
libpng16.dylib

Since the packaging format (tgz) is able to store symbolic link, wouldn't it be better to keep them ? Less waste of disk also :-)

lasote commented 8 years ago

I'm not sure at all about the convenience of keep the symbolic links. If there are absolute paths will be broken soon or later. Keep all files also allows to download the linux/osx packages in windows and even uploaded again without changing the package itself. Yes, it's a border case, but keeping generic tgz's without permissions nor symbolic links guarantees no bad surprises.

mathieu commented 8 years ago

Hi @lasote, I've looked around a bit but don't have a Windows box at hand to test but according to msdn, symbolic links are available since Windows Vista on NTFS : https://msdn.microsoft.com/en-us/library/windows/desktop/aa365680(v=vs.85).aspx . I'll try tomorrow at work :)

shinichy commented 8 years ago

I prefer keeping symbolic links as is because it's easier to create a OSX framework package.

SSE4 commented 7 years ago

is it possible to add some option to self.copy to keep symlinks (just like keep_path option)?

lasote commented 7 years ago

@SSE4 I think it could be done. What is your need about symlinks? Not duplicated libraries? linked folders? Any other? Any information will be nice.

SSE4 commented 7 years ago

@lasote exactly, avoid duplication of libraries (.dylib). for example in case of boost 1.62 universal libraries (x86 + x64) take ~100 Mb, and without symlinks it would be ~300 Mb.

lasote commented 7 years ago

OK, To have an optional "keep_symlinks" parameter (defaulted to False) sound good. The package creator will be responsible of activate it when he want to. We will need to do some checks/tests to confirm it's suitable but sounds good.

piponazo commented 7 years ago

I would also love to the an option for keeping the symbolic links.

I am trying to create a recipe in which I am using the CMake generator and providing my own FindXXX.cmake file. If in the package function I add this:

self.copy("*.so",   dst="lib",     src="openmesh/installFolder/lib")

Then the test_package that depends on my recipe fails in the linking phase:

[100%] Linking CXX executable bin/test
[100%] Built target test
./bin/test: error while loading shared libraries: libOpenMeshCore.so.4.1: cannot open shared object file: No such file or directory
ERROR: Error 32512 while executing ./bin/test

If I add this:

self.copy("*.so.*",   dst="lib",     src="openmesh/installFolder/lib")

Then only copy the file libOpenMeshCore.so.4.1 is copied. And later CMake fails to find to library in the normal way:

#FindXXX.cmake file
...
find_library(OPENMESH_CORE_LIB NAMES OpenMeshCore OpenMeshCored PATHS ${CONAN_LIB_DIRS_OPENMESH})
...

The only way I have to make the test_package work is to use this:

self.copy("*.so*",   dst="lib",     src="openmesh/installFolder/lib")

But then I would have 2/3 copies of the file depending on the library.

memsharded commented 7 years ago

I think it is doable adding something like this to FileCopier:

if os.path.islink(abs_src_name):
       linkto = os.readlink(abs_src_name)
       os.symlink(linkto, abs_dst_name)

I have to add some tests for it, if they pass I will submit a PR for this feature.

memsharded commented 7 years ago

Added tests, locally it works, but the upload with tgz is failing, still have to figure out how to fix it

piponazo commented 7 years ago

Hi @memsharded. Have you considered to use this?

https://docs.python.org/3/library/shutil.html#shutil.copytree

It could be interesting for copying the complete lib folder and not only individual files.

memsharded commented 7 years ago

Sure, but the problems is that we are not copying full directories, but only those files that match a pattern. You can check the current implementation here: https://github.com/conan-io/conan/blob/develop/conans/client/file_copier.py#L65

Is not big issue, I have already fixed the problems with the upload, and seems everything fine: #658, most likely it will be in the next release.

memsharded commented 7 years ago

Merged to develop, will be available in conan 0.16

memsharded commented 7 years ago

Released in 0.16

piponazo commented 7 years ago

I just tried the new feature in 0.16.0 and It did not work for me. In my recipe I have something like that:

    def package(self):
        self.copy("FindOpenMesh.cmake", ".", ".")
        self.copy("*",       dst="include", src="openmesh/installFolder/include")
        self.copy("*",       dst="lib",     src="openmesh/installFolder/lib")
        self.copy("*.so*",   dst="lib",     src="openmesh/installFolder/lib")
        self.copy("*.dylib", dst="lib",     src="openmesh/installFolder/lib")

The library generates these files in the lib folder:

luis@p4dDesktop:~/.conan/data/OpenMesh/4.1.1-0/pix4d/testing$ ls build/b1c1bba64e5b990636f4bcca97c3f5313c24bc3c/openmesh/installFolder/lib/ -lh
total 2.0M
-rw-r--r-- 1 luis luis 1.2M Nov 29 13:51 libOpenMeshCore.a
lrwxrwxrwx 1 luis luis   22 Nov 29 13:51 libOpenMeshCore.so -> libOpenMeshCore.so.4.1
-rw-r--r-- 1 luis luis 617K Nov 29 13:51 libOpenMeshCore.so.4.1
-rw-r--r-- 1 luis luis 132K Nov 29 13:51 libOpenMeshTools.a
lrwxrwxrwx 1 luis luis   23 Nov 29 13:51 libOpenMeshTools.so -> libOpenMeshTools.so.4.1
-rw-r--r-- 1 luis luis  89K Nov 29 13:51 libOpenMeshTools.so.4.1

And the resulting files in the package folder are:

luis@p4dDesktop:~/.conan/data$ ls OpenMesh/4.1.1-0/pix4d/testing/package/b1c1bba64e5b990636f4bcca97c3f5313c24bc3c/lib/ -lh
total 2.7M
-rw-r--r-- 1 luis luis 1.2M Nov 29 14:02 libOpenMeshCore.a
-rw-r--r-- 1 luis luis 617K Nov 29 14:02 libOpenMeshCore.so
-rw-r--r-- 1 luis luis 617K Nov 29 14:02 libOpenMeshCore.so.4.1
-rw-r--r-- 1 luis luis 132K Nov 29 14:02 libOpenMeshTools.a
-rw-r--r-- 1 luis luis  89K Nov 29 14:02 libOpenMeshTools.so
-rw-r--r-- 1 luis luis  89K Nov 29 14:02 libOpenMeshTools.so.4.1

So, basically nothing changed for me. Is somebody experiencing the same?

SSE4 commented 7 years ago

@piponazo I didn't have a chance to test 0.16 yet, but as I can see there are additional parameters in self.copy functions, named "links" and "symlinks". they are not docummented, but seems like valid values for them are None, False and True. so probably it makes sense to try something like:

self.copy("*.so*",   dst="lib",     src="openmesh/installFolder/lib", links=True)
self.copy("*.so*",   dst="lib",     src="openmesh/installFolder/lib", symlinks=True)
memsharded commented 7 years ago

symlinks only in develop, please try:

self.copy("*.so*",   dst="lib",     src="openmesh/installFolder/lib", links=True)

We had to make it opt-in to not break existing packages that wouldn't be happy with the symlinks.

memsharded commented 7 years ago

The docs have been updated accordingly (this time we have made an extra effort trying to keep the docs synced :) http://docs.conan.io/en/latest/reference/conanfile.html#package

piponazo commented 7 years ago

Ups, sorry for the false alarm. It worked like a charm after specifying the links=True :smile: .

memsharded commented 7 years ago

No worries, glad it worked :) :)

vertexmachina commented 7 years ago

Is it possible to implement this for exports as well? If I have a package that contains libraries, and those libraries have symlinked versions associated with them, they lose their link nature when copied over during export.

memsharded commented 7 years ago

Yes, it might be possible, could you please better open a new issue for it? Also, what is your use case, are you creating packages for already compiled libs? Usually it is recommended to implement the build() method that automates the building from source.