Ticket #10 (closed defect: fixed)

Opened 8 years ago

Last modified 3 years ago

savannah: copy from fish vfs stalls on symlinks

Reported by: slavazanko Owned by: winnie
Priority: major Milestone: 4.6.2
Component: mc-core Version:
Keywords: Cc:
Blocked By: #149, #157 Blocking:
Branch state: merged Votes for changeset:

Description (last modified by ossi) (diff)

Original: http://savannah.gnu.org/bugs/?12972

Submitted by:NoneSubmitted on:Wed 04 May 2005 11:43:48 PM UTC
Category:VFSSeverity:|3 - Normal
Status:NonePrivacy:Public
Assigned to:Pavel Tsekov <ptsekov>Open/Closed:Open
Release:current (CVS or snapshot)Operating System:GNU/Linux

Original submission:

Copying symlinks from Fish VFS freezes the UI.

This looks to be because the Fish VFS runs ls with -L (dereference 
symlinks) on the remote side and therefore only receives information
on the dereferenced files.

This includes the length of the file so MC stalls waiting for data 
when the remote host is reading the much smaller contents of the 
symlink.

Comment 1 by Pavel Tsekov <ptsekov> at Thu 05 May 2005 08:23:21 AM UTC:

Symlinks to what ? A Directory, a file ? Have you checked that you 
have read permissions for the target of the symlink ?

Comment 2 by Pavel Tsekov <ptsekov> at Thu 05 May 2005 01:05:11 PM UTC:

I ask because I had experience with FISH similiar to the one 
described by the original poster. See this post:

http://mail.gnome.org/archives/mc-devel/2002-November/msg00067.html

Comment 3 by Anonymous at Fri 06 May 2005 05:33:00 PM UTC:

Symlinks to files. I don't think I had any problems with 
directories, now you come to mention it.

I did have sufficient permissions for the files: everything was 
world-readable.

Attachments

fix-stalls-on-symlinks-rev1.patch (1.9 KB) - added by slavazanko 8 years ago.
fix-stalls-on-symlinks-rev2.patch (1.9 KB) - added by Patrick Winnertz 8 years ago.
Added by email2trac
fix-stalls-on-symlinks-rev3.patch (2.1 KB) - added by Patrick Winnertz 8 years ago.
Added by email2trac
test-fish-ls.sh (511 bytes) - added by slavazanko 8 years ago.
fish-fix.patch (16.3 KB) - added by Patrick Winnertz 8 years ago.
Added by email2trac
fish_screen (182.4 KB) - added by styx 8 years ago.
connecting to just installed sshd on localhost

Change History

Changed 8 years ago by slavazanko

comment:1 Changed 8 years ago by slavazanko

Please, someone review and test patch fix-stalls-on-symlinks-rev1.patch

comment:2 Changed 8 years ago by winnie

  • Status changed from new to accepted
  • Owner set to winnie
  • Milestone set to 4.6.2

NACK from me..

S_ISLINK(ST.st_mode) exists... why don't use it.

BTW, this is a bugfix which is suitable for 4.6.2.. therefore Milestone 4.6.2

Changed 8 years ago by Patrick Winnertz

Added by email2trac

comment:3 Changed 8 years ago by Patrick Winnertz

Updated patch now in order to use S_ISLNK and to do not fail on files or links
which are called: "foo -> bar".
Attached a rev2 of the patch (this is also viewable in
d65198e9f286acae8a9d9a9a7b499fe3328c3bcd ).

Please comment on this fix.

comment:4 Changed 8 years ago by Patrick Winnertz

I've made a small update to this patch, as someone tries this in the past
without completing it.
There was a

case 'L': ent->ino->linkname = g_strdup(buffer+1);

but the ls command executed on the server never set a L<someinfo>, so this
code will never be executed.

Instead of setting a L<someinfo> line on the remote site via shell hacking, i
think it is more relyable to do this locally in c as we can't know which
commands are avialable on the remote site.

Please comment on this (rev3).

Changed 8 years ago by Patrick Winnertz

Added by email2trac

Changed 8 years ago by slavazanko

comment:5 Changed 8 years ago by slavazanko

see test-fish-ls.sh

Script like this attach used for retrieve listing if remote directory. I will change script for run independ from mc and add some thinks... eg. check if ls support option '-Q'

comment:6 Changed 8 years ago by winnie

  • Blocked By 147, 149 added

There is a fix in the 10_fish_stalls_on_symlink branch.. however this fix is uncomplete as it will fail in this scenario:

echo "ALL OK" >"this -> file"
ln -s "this -> file" "this -> link"

The link will point to -> file instead of "this -> file".

To fix this issue we need quoted output. (This means ls -Q). As this is no standard option but only used in gnu ls we have to check prior using it if this option is available.
However this is related to ticket:149.

comment:7 Changed 8 years ago by metux

  • Keywords review,vote-metux added

comment:8 Changed 8 years ago by slavazanko

  • Keywords rework added; review,vote-metux removed

comment:9 Changed 8 years ago by ossi

fwiw, kde's kio_fish can optionally use a perl-based stub which is faster and more reliable. given the availability of perl that might be more viable than special-casing shell code for more reliability.

comment:10 Changed 8 years ago by slavazanko

I think, wee need to keep compability with really pure OSes (even don't have 'test' command).
In this case we may connect via fish to a lot of OSes... to my iPhone (without perl), fo example ;)

comment:11 Changed 8 years ago by Enrico Weigelt

  • MC Ticket System <tickets@…> schrieb:

fwiw, kde's kio_fish can optionally use a perl-based stub which is faster
and more reliable. given the availability of perl that might be more
viable than special-casing shell code for more reliability.

hmm. would of course require an test whether perl works and we
have some temporary working place on the server.

cu

comment:12 Changed 8 years ago by metux

  • Blocked By 157 added

comment:13 Changed 8 years ago by winnie

  • Blocked By 147 removed

removed #147 as blocking bug. the escape functions are now provided inside the mhl lib which was addressed in #157

comment:14 Changed 8 years ago by slyfox

  • Keywords review added; rework removed

vfs/fish.c: Added perl backend, symlinks are properly showed as real symlinks (and copied the same way)

Current patchset is at branch:10_fish_whitespace_symlink_issue

Please review and test!

comment:15 Changed 8 years ago by Patrick Winnertz

Hey,

Please review, test and votee on the patch in the branch (I've added the patch
also here in the email so that you can have a look on it

Changed 8 years ago by Patrick Winnertz

Added by email2trac

Changed 8 years ago by styx

connecting to just installed sshd on localhost

comment:16 Changed 8 years ago by styx

  • Keywords vote-styx added
  • Description modified (diff)

comment:17 Changed 8 years ago by styx

  • Description modified (diff)

comment:18 Changed 8 years ago by slavazanko

  • Keywords vote-slavazanko approved added; review removed

Well, all my tests passed. Therefore my vote here

comment:19 Changed 8 years ago by winnie

  • Status changed from accepted to testing
  • Keywords committed-master committed-mc-4.6 added
  • Resolution set to fixed

Committed

comment:20 Changed 8 years ago by winnie

  • Status changed from testing to closed

comment:21 Changed 3 years ago by ossi

  • Description modified (diff)
  • Branch state set to no branch

comment:22 Changed 3 years ago by andrew_b

  • Keywords vote-styx vote-slavazanko approved committed-master committed-mc-4.6 removed
  • Branch state changed from no branch to merged
Note: See TracTickets for help on using tickets.