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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy symlinks (folders) in 'merge_directories' func #5237

Merged
merged 8 commits into from May 30, 2019

Conversation

@jgsogo
Copy link
Member

@jgsogo jgsogo commented May 28, 2019

Changelog: Fix: Copy symlinked folder when using merge_directories function
Docs: omit

closes #5080

This PR is just targeting the bug reported in #5080, which was already patched in lasote's branch 馃嵃 .

About replacing the code in merge_directories with FileCopier one, I'm agree, but I think we should leave it for another issue/pr, wdyt?

@jgsogo jgsogo added this to the 1.16 milestone May 28, 2019
@jgsogo jgsogo requested a review from lasote May 28, 2019
@SSE4
Copy link
Contributor

@SSE4 SSE4 commented May 28, 2019

this doesn't work for me:

ERROR: [HOOK - conan-center.py] pre_export(): [Errno 62] Too many levels of symbolic links: '/Users/sse4/bincrafters/test-symlinks/test'
Traceback (most recent call last):
  File "/Users/sse4/bincrafters/conan/conans/client/hook_manager.py", line 58, in execute
    method(output=output, **kwargs)
  File "/Users/sse4/.conan/hooks/conan-center.py", line 58, in wrapper
    ret = func(output, *args, **kwargs)
  File "/Users/sse4/.conan/hooks/conan-center.py", line 142, in pre_export
    @run_test("RECIPE FOLDER SIZE", output)
  File "/Users/sse4/.conan/hooks/conan-center.py", line 67, in tmp
    ret = func(out)
  File "/Users/sse4/.conan/hooks/conan-center.py", line 151, in test
    total_size += os.path.getsize(file_path)
  File "/usr/local/Cellar/python@2/2.7.16/Frameworks/Python.framework/Versions/2.7/lib/python2.7/genericpath.py", line 57, in getsize
    return os.stat(filename).st_size
OSError: [Errno 62] Too many levels of symbolic links: '/Users/sse4/bincrafters/test-symlinks/test'

repro:

mkdir test-symlinks
cd test-symlinks
conan new test/1.0@user/testing
ln -s test test
conan create . user/testing

Copy link
Contributor

@SSE4 SSE4 left a comment

still broken

@jgsogo
Copy link
Member Author

@jgsogo jgsogo commented May 28, 2019

@SSE4, that one is failing in the hook, not in Conan itself. Can you run your test without the hook? (...and maybe submit this issue to the hooks repo if it is related to an official one).

@SSE4
Copy link
Contributor

@SSE4 SSE4 commented May 28, 2019

ah, didn't notice that, sorry :(

@SSE4
Copy link
Contributor

@SSE4 SSE4 commented May 28, 2019

still fails:

ERROR: Traceback (most recent call last):
  File "/Users/sse4/bincrafters/conan/conans/errors.py", line 24, in conanfile_exception_formatter
    yield
  File "/Users/sse4/bincrafters/conan/conans/client/packager.py", line 68, in create_package
    conanfile.package()
  File "/Users/sse4/.conan/data/test/1.0/bincrafters/testing/export/conanfile.py", line 48, in package
    self.copy("*", dst="a", keep_path=False)
  File "/Users/sse4/bincrafters/conan/conans/client/file_copier.py", line 75, in __call__
    keep_path, excluded_folders=excluded)
  File "/Users/sse4/bincrafters/conan/conans/client/file_copier.py", line 93, in _copy
    copied_files = self._copy_files(files_to_copy, src, dst, keep_path, symlinks)
  File "/Users/sse4/bincrafters/conan/conans/client/file_copier.py", line 221, in _copy_files
    shutil.copy2(abs_src_name, abs_dst_name)
  File "/usr/local/Cellar/python@2/2.7.16/Frameworks/Python.framework/Versions/2.7/lib/python2.7/shutil.py", line 153, in copy2
    copyfile(src, dst)
  File "/usr/local/Cellar/python@2/2.7.16/Frameworks/Python.framework/Versions/2.7/lib/python2.7/shutil.py", line 96, in copyfile
    with open(src, 'rb') as fsrc:
IOError: [Errno 62] Too many levels of symbolic links: '/Users/sse4/.conan/data/test/1.0/bincrafters/testing/build/f8bda7f0751e4bc3beaa6c3b2eb02d455291c8a2/test1'

@jgsogo
Copy link
Member Author

@jgsogo jgsogo commented May 29, 2019

... but I think that it is an OS error, because of the line ln -s test test, you are linking a folder to itself and it creates some kind of recursion trying to iterate it ("Too many levels of symbolic links"):

鈬  mkdir tmp
鈬  ln -s tmp tmp
鈬  cd tmp
鈬  cd tmp
cd: too many levels of symbolic links: tmp

I'm not sure if this is something we can handle in Conan, nevertheless, I'll add a try/except to get rid of the ugly traceback and print just the error message.

@jgsogo
Copy link
Member Author

@jgsogo jgsogo commented May 29, 2019

btw, @SSE4, I'm not able to reproduce that error. If I create those kind of symlinks I get an error like this one: "ERROR: The file is a broken symlink, verify that you are packaging the needed destination files: '.../test/1.0/user/testing/export_source/src/src'.You can skip this check adjusting the 'general.skip_broken_symlinks_check' at the conan.conf file." which is not the best, but at least it points in the right direction to the file/folder originating the problem.

@SSE4
Copy link
Contributor

@SSE4 SSE4 commented May 29, 2019

@jgsogo I've added the following to ensure symlinks are copied:

def package(self):
   ....
   self.copy("*", dst="symlinks", keep_path=False)

@lasote lasote merged commit 083e389 into conan-io:develop May 30, 2019
2 checks passed
@jgsogo jgsogo deleted the issue/5080-scm-symlink branch May 30, 2019
jgsogo added a commit to jgsogo/conan that referenced this issue Jun 12, 2019
memsharded added a commit that referenced this issue Jun 13, 2019
鈥5342)

* reproduce in a test

* add single file (possibly regression in #5237

* symlink the folder and keep going (do not enter subdirectories)

* check layout in the cache

* py2 doesn't have named argument for symlink

* remove unused import

* do not run this tests in Windows

* add test with broken symlinks in the sources

* synlinks only not-windows

* symlink folders

* only if isabs

* same behavior for files and folders

* remove symlink argument from merge_directories function (always True)

* create the symlink also for files, handle abs/rel use cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants