shared links can only be files #1574

Closed
jaytaph opened this Issue Jan 25, 2016 · 13 comments

Comments

Projects
None yet
6 participants
@jaytaph

jaytaph commented Jan 25, 2016

When running deploy:check:linked_files, it will fail on shared files that are in fact symlinks. This is because it will explicitly check for files at https://github.com/capistrano/capistrano/blob/master/lib/capistrano/tasks/deploy.rake#L72.

In our case, these shared files are links themselves, but the same issue will occur when the shared files are sockets, pipes etc.

I don't know capistrano well enough to add a patch (i'd reckon changing the test to -e should be enough), but i'm probably overlooking some other cases here as well.

@leehambley

This comment has been minimized.

Show comment
Hide comment
@leehambley

leehambley Jan 25, 2016

Member

Thanks for the report. It's a valid point. It should be as easy to fix as you say.

Member

leehambley commented Jan 25, 2016

Thanks for the report. It's a valid point. It should be as easy to fix as you say.

@jaytaph

This comment has been minimized.

Show comment
Hide comment
@jaytaph

jaytaph Jan 26, 2016

I think it is,.. I'd recommend changing it to ! -d instead of -e, so it will match everything that is not a directory, since these should be linked with linked_dirs. A PR is easy, but I cannot setup the cucumber tests, as I don't have an environment that runs vagrant (as my development environments are already running inside a virtual machines).

jaytaph commented Jan 26, 2016

I think it is,.. I'd recommend changing it to ! -d instead of -e, so it will match everything that is not a directory, since these should be linked with linked_dirs. A PR is easy, but I cannot setup the cucumber tests, as I don't have an environment that runs vagrant (as my development environments are already running inside a virtual machines).

@mattbrictson

This comment has been minimized.

Show comment
Hide comment
@mattbrictson

mattbrictson Jan 26, 2016

Member

! -d would be true even if no file exists at all, which is not what we want.

Member

mattbrictson commented Jan 26, 2016

! -d would be true even if no file exists at all, which is not what we want.

@jaytaph

This comment has been minimized.

Show comment
Hide comment
@jaytaph

jaytaph Jan 26, 2016

ah yes.. maybe a ! -d and -e test?

jaytaph commented Jan 26, 2016

ah yes.. maybe a ! -d and -e test?

@mattbrictson

This comment has been minimized.

Show comment
Hide comment
@mattbrictson

mattbrictson Jan 26, 2016

Member

If you are willing to submit a PR, and you've tested your branch in an actual real-world deployment and it works as you expect, that's good enough for me. I can double-check that the Vagrant tests still pass before merging it. I believe we already have decent acceptance tests in place for deploy:check:linked_files, so I'm not too worried.

Member

mattbrictson commented Jan 26, 2016

If you are willing to submit a PR, and you've tested your branch in an actual real-world deployment and it works as you expect, that's good enough for me. I can double-check that the Vagrant tests still pass before merging it. I believe we already have decent acceptance tests in place for deploy:check:linked_files, so I'm not too worried.

@nielspetersen

This comment has been minimized.

Show comment
Hide comment
@nielspetersen

nielspetersen Feb 2, 2016

This issue is part of the current issue (14th) of the RubyIssues. https://rubyissues.ongoodbits.com/. 💎

This issue is part of the current issue (14th) of the RubyIssues. https://rubyissues.ongoodbits.com/. 💎

@mattbrictson

This comment has been minimized.

Show comment
Hide comment
@mattbrictson

mattbrictson Feb 2, 2016

Member

Thanks for spreading the word on this for us. It would be nice to get this fixed. Pull requests are welcome!

That said, I think this is overstating the problem:

When you'd like to check your Capistrano deployment setup with deploy:check:linked_files, it will probably fail.

To clarify: the problem only occurs if the file you are asking Capistrano to symlink to is in fact a symlink itself (i.e. two levels of indirection). I think it is pretty uncommon.

Member

mattbrictson commented Feb 2, 2016

Thanks for spreading the word on this for us. It would be nice to get this fixed. Pull requests are welcome!

That said, I think this is overstating the problem:

When you'd like to check your Capistrano deployment setup with deploy:check:linked_files, it will probably fail.

To clarify: the problem only occurs if the file you are asking Capistrano to symlink to is in fact a symlink itself (i.e. two levels of indirection). I think it is pretty uncommon.

@leehambley

This comment has been minimized.

Show comment
Hide comment
@leehambley

leehambley Feb 3, 2016

Member

I don't think @nielspetersen manages the mailing list, I think it's @spacetraveler_.

Member

leehambley commented Feb 3, 2016

I don't think @nielspetersen manages the mailing list, I think it's @spacetraveler_.

@irvingwashington

This comment has been minimized.

Show comment
Hide comment
@irvingwashington

irvingwashington May 14, 2016

Contributor

I was trying to fix this issue, but after I created a scenario with a symlink-
it passed with the current [ -f #{file} ] implementation (on the cucumber vagrant ubuntu).
-f only fails for broken symlinks (on that ubuntu or os x).
@jaytaph: what os did you see this issue happen on?

Contributor

irvingwashington commented May 14, 2016

I was trying to fix this issue, but after I created a scenario with a symlink-
it passed with the current [ -f #{file} ] implementation (on the cucumber vagrant ubuntu).
-f only fails for broken symlinks (on that ubuntu or os x).
@jaytaph: what os did you see this issue happen on?

@mattbrictson

This comment has been minimized.

Show comment
Hide comment
@mattbrictson

mattbrictson May 15, 2016

Member

@irvingwashington Thanks for investigating. You're right: on OS X for example -f works fine for a symlink. Let's wait for @jaytaph to clarify the issue before making any changes to the code.

Member

mattbrictson commented May 15, 2016

@irvingwashington Thanks for investigating. You're right: on OS X for example -f works fine for a symlink. Let's wait for @jaytaph to clarify the issue before making any changes to the code.

@jaytaph

This comment has been minimized.

Show comment
Hide comment
@jaytaph

jaytaph May 16, 2016

@mattbrictson @irvingwashington I'm not active on the given project anymore, but it was an ubuntu (12.04) system.

The issue only occurred when using two levels of symlinks, which indeed is not very common. However, imho it should not matter where a capistrano symlink points to: a file, socket, symlink or anything else (but not directories, as they are handled differently already). I've did a quick test on OSX and CentOS, and see that test -f on symlinks and even double symlinks indeed work fine.

So I cannot reproduce the issue quickly at the moment, and if nobody is able to reproduce it either, I don't mind have the issue closed.

jaytaph commented May 16, 2016

@mattbrictson @irvingwashington I'm not active on the given project anymore, but it was an ubuntu (12.04) system.

The issue only occurred when using two levels of symlinks, which indeed is not very common. However, imho it should not matter where a capistrano symlink points to: a file, socket, symlink or anything else (but not directories, as they are handled differently already). I've did a quick test on OSX and CentOS, and see that test -f on symlinks and even double symlinks indeed work fine.

So I cannot reproduce the issue quickly at the moment, and if nobody is able to reproduce it either, I don't mind have the issue closed.

@mattbrictson

This comment has been minimized.

Show comment
Hide comment
@mattbrictson

mattbrictson May 16, 2016

Member

OK, I will close this for now. If someone has a real-world use case of linking non-files (e.g. sockets) then we can revisit this.

Member

mattbrictson commented May 16, 2016

OK, I will close this for now. If someone has a real-world use case of linking non-files (e.g. sockets) then we can revisit this.

@scones

This comment has been minimized.

Show comment
Hide comment
@scones

scones May 8, 2018

As i ran into this issue:
wouldn't it be easier to just copy the symlink from the share dir to the release dir thus avoiding multilevel symlinks?

scones commented May 8, 2018

As i ran into this issue:
wouldn't it be easier to just copy the symlink from the share dir to the release dir thus avoiding multilevel symlinks?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment