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

Fix copy_entry for symlinks #1348

Merged
merged 2 commits into from Jul 20, 2017

Conversation

Projects
None yet
2 participants
@ServiusHack
Contributor

ServiusHack commented Jun 11, 2017

While creating pacman packages I noticed symbolic links become actual files in the package. Running with --debug-workspace I verified that the problem is with the code for pacman. Only the temporary directory for the pathtype "build_path" contained regular files instead of the expected symlinks.

The files are being copied by FPM::Util:copy_entry which in turn uses FileUtils::copy_entry. The problem is with that call. While the signature is:

copy_entry(src, dest, preserve = false, dereference_root = false, remove_destination = false)

The function is called like this:

FileUtils.copy_entry(src, dst, preserve=preserve,
                         remove_destination=remove_destination)

I'm not very familiar with Ruby but what I could get from various sites is that this call is not what it appears to be. preserve=preserve doesn't pass the variable preserve as the function parameter preserve. Instead this is a self-assignment which returns the assigned value. For preserve this works as it's in the correct location. For remove_destination it's problematic since the value of remove_destination is passed to the dereference_root parameter.

I've added a test case showing this and included a fix.

ServiusHack added some commits Jun 11, 2017

Add test for copying symlinks
The test explicitly sets `preserve=true` and `remove_destination=true`
to show the currently broken behavior.
Fix copy_entry for symlinks
The fourth argument to FileUtils.copy_entry is dereference_root to
which the value of remove_destination was passed.

The fix now passes the parameters in the required position.
@jordansissel

This comment has been minimized.

Owner

jordansissel commented Jun 15, 2017

Thank you for this patch. I will review it as soon as I can.

@@ -298,8 +298,8 @@ def copy_entry(src, dst, preserve=false, remove_destination=false)
if known_entry
FileUtils.ln(known_entry, dst)
else
FileUtils.copy_entry(src, dst, preserve=preserve,
remove_destination=remove_destination)
FileUtils.copy_entry(src, dst, preserve, false,

This comment has been minimized.

@jordansissel

jordansissel Jul 20, 2017

Owner

Good catch!

I have looked around fpm and the ruby docs and I'm not really sure what some of these arguments to FileUtils::copy_entry really mean.

@jordansissel jordansissel merged commit a3ddf38 into jordansissel:master Jul 20, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment