[Bash-completion-devel] [bash-completion-Bugs][312285] nfs mount completion broken

Tue Jan 26 21:38:54 UTC 2010

Bugs item #312285, was changed at 2010-01-22 21:03 by Ville Skyttä 
You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=312285&group_id=100114

Status: Closed
Priority: 3
Submitted By: Leonard Crestez (cdleonard-guest)
Assigned to: Nobody (None)
Summary: nfs mount completion broken 
Distribution: None
Originally reported in: None
Milestone: 2.0
Status: Fix Committed
Original bug number: 


Initial Comment:
Original report by "Ildar Mulyukov <ildar at users.sourceforge.net>" to the bash-completion-devel list.

"""
hi! I checked current GIT. This completes wrong:
        $ mount nfsserver:/<TAB>
        bin/        etc/        lost+found/ opt/        sbin/       tmp/
        boot/       home/       media/      proc/       srv/        usr/
        dev/        lib/        mnt/        root/       sys/        var/

I couldn't debug it myself, sorry :-(
Best regards,
"""

It's very likely that this is because COMP_WORDBREAKS now contains a colon. I don't use NFS; could somebody else test the following tiny patch?

diff --git a/contrib/mount b/contrib/mount
index 0474df0..99e77b4 100644
--- a/contrib/mount
+++ b/contrib/mount
@@ -12,9 +12,9 @@ _mount()
     local cur i sm host prev
 
     COMPREPLY=()
-    cur=`_get_cword`
+    cur=`_get_cword ':'`
+    prev=`_get_pword ':'`
     [[ "$cur" == \\ ]] && cur="/"
-    prev=${COMP_WORDS[COMP_CWORD-1]}
 
     for i in {,/usr}/{,s}bin/showmount; do [ -x $i ] && sm=$i && break; done
 


----------------------------------------------------------------------

>Comment By: Ville Skyttä  (scop-guest)
Date: 2010-01-26 23:38

Message:
> In theory $i can now contain spaces

Not sure if I followed your explanation, but if the path to showmount contains spaces, I'm pretty sure that its invocation would have failed too.  Hopefully fixed in git now.

> My original version used #! /bin/bash; isn't that 
> portable?

No, bash is not always installed there.  For example on FreeBSD boxes I have access to it's in /usr/local/bin/bash.  (We have this problem in a few places in bash-completion, I have an entry in my persontal TODO list for it.)

> it seems that alioth doesn't always allow 
> attachments

Let me try it out, I'm attaching a test.txt in the Attachments tab here along with this followup. I probably have special permissions to the Alioth project though, but there's nothing in the tracker settings that I gather would affect attachments :P

> It's very likely that a patch posted to a closed bug
> will be forgotten.

That's right, but this one just got rescued and is applied in git, thanks.  Ildar, don't get used to it :)

----------------------------------------------------------------------

Comment By: Leonard Crestez (cdleonard-guest)
Date: 2010-01-26 21:18

Message:
> Date: 2010-01-24 19:38
> Sender: Ville Skyttä 
> 
> Thanks! Applied in git with some changes:
> 
> * This hunk seems unrelated and a no-op so skipped:
> -        [ -x $i ] && sm=$i && break
> +        [[ -x $i ]] && sm="$i" &&
> break;
Good point. In theory $i can now contain spaces so the [[ is required. I tried running tests in a "bash completion" dir but other things broke first; so you were right to drop this.

> * Portability fixes to the mock showmount
My original version used #! /bin/bash; isn't that portable?

> * Added CHANGES entry
Duly noted.

> ...and consider submitting the patches as attachments instead of inlining them.
I'm sorry but it seems that alioth doesn't always allow attachments. As far as I can tell only the user who started an issue can modify attachments.

I was the original poster for this issue so I was allowed to upload, but I didn't notice this particular case. Alioth is strange, perhaps sending patches to the mailing list would be preferable?

----------

> Date: 2010-01-25 06:47
> Sender: Ildar Mulyukov
> 
> BTW showmount -e completed before. Now it's lost. We have
> a regression.
> Here's a quick patch:

It's very likely that a patch posted to a closed bug will be forgotten.

----------------------------------------------------------------------

Comment By: Ildar Mulyukov (ildar-guest)
Date: 2010-01-25 06:47

Message:
BTW showmount -e completed before. Now it's lost. We have a regression.
Here's a quick patch:

--- bash-completion-1.99/bash_completion.ORIG	2010-01-25 03:58:46 +0600
+++ bash-completion-1.99/bash_completion	2010-01-25 10:46:14 +0600
@@ -1402,7 +1402,7 @@
 } # _known_hosts_real()
 complete -F _known_hosts traceroute traceroute6 tracepath tracepath6 ping \
     ping6 fping fping6 telnet host nslookup rsh rlogin ftp dig mtr \
-    ssh-installkeys
+    ssh-installkeys showmount
 
 # This meta-cd function observes the CDPATH variable, so that cd additionally
 # completes on directories under those specified in CDPATH.


----------------------------------------------------------------------

Comment By: Ildar Mulyukov (ildar-guest)
Date: 2010-01-25 06:43

Message:
Hi, Leonard!
Thanks a lot, it's working!
Regards, Ildar

----------------------------------------------------------------------

Comment By: Ville Skyttä  (scop-guest)
Date: 2010-01-24 19:38

Message:
Thanks! Applied in git with some changes:

* This hunk seems unrelated and a no-op so skipped:
-        [ -x $i ] && sm=$i && break
+        [[ -x $i ]] && sm="$i" && break;
* Portability fixes to the mock showmount
* Added CHANGES entry
http://git.debian.org/?p=bash-completion/bash-completion.git;a=commitdiff;h=c5951118e906c8937312ff4605a9f47a9cfd108f

To make committers' job easier in the future, see also:
http://git.debian.org/?p=bash-completion/bash-completion.git;a=commitdiff;h=5c5d804696ad715c4d8516b061564d0ab5115ab6
...and consider submitting the patches as attachments instead of inlining them.

----------------------------------------------------------------------

Comment By: Leonard Crestez (cdleonard-guest)
Date: 2010-01-24 18:00

Message:
Here is a new patch against latest HEAD. It should work this time.

The patch contains an automated test. There's a fake showmount in fixtures/mount/bin.

diff --git a/contrib/mount b/contrib/mount
index 503c5a7..302ad98 100644
--- a/contrib/mount
+++ b/contrib/mount
@@ -12,17 +12,17 @@ _mount()
     local cur i sm host prev
 
     COMPREPLY=()
-    cur=`_get_cword`
+    cur=`_get_cword ':'`
+    prev=`_get_pword ':'`
     [[ "$cur" == \\ ]] && cur="/"
-    prev=${COMP_WORDS[COMP_CWORD-1]}
 
     for i in $(type -P showmount) {,/usr}/{,s}bin/showmount; do
-        [ -x $i ] && sm=$i && break
+        [[ -x $i ]] && sm="$i" && break;
     done
 
     if [[ -n "$sm" && "$cur" == *:* ]]; then
         COMPREPLY=( $( compgen -W "$( $sm -e ${cur%%:*} | \
-            awk 'NR>1 {print $1}' )" -- "$cur" ) )
+            awk 'NR>1 {print $1}' )" -- "${cur#*:}" ) )
     elif [[ "$cur" == //* ]]; then
         host=${cur#//}
         host=${host%%/*}
diff --git a/test/completion/mount.exp b/test/completion/mount.exp
new file mode 100644
index 0000000..c1f89ce
--- /dev/null
+++ b/test/completion/mount.exp
@@ -0,0 +1,3 @@
+if {[assert_bash_type mount]} {
+    source "lib/completions/mount.exp"
+}; # if
diff --git a/test/fixtures/mount/bin/showmount b/test/fixtures/mount/bin/showmount
new file mode 100755
index 0000000..dfc54f1
--- /dev/null
+++ b/test/fixtures/mount/bin/showmount
@@ -0,0 +1,12 @@
+#! /bin/bash
+
+if [[ $1 == -e && $2 == mocksrv ]]; then
+    echo "Header line"
+    echo "/test/path"
+    echo "/test/path2"
+    echo "/second/path"
+    exit 0
+fi
+
+echo "Usage: 'showmount -e mocksrv'; nothing else works."
+exit 1
diff --git a/test/lib/completions/mount.exp b/test/lib/completions/mount.exp
new file mode 100644
index 0000000..9e40803
--- /dev/null
+++ b/test/lib/completions/mount.exp
@@ -0,0 +1,33 @@
+proc setup {} {
+    save_env
+};
+
+
+proc teardown {} {
+    assert_env_unmodified
+};
+
+
+setup
+
+
+assert_complete_any "mount "
+
+
+sync_after_int
+
+
+set test "Check completing nfs mounts"
+set expected [list /test/path /test/path2 /second/path]
+set cmd "mount mocksrv:/"
+assert_bash_exec {OLDPATH="$PATH"; PATH="$TESTDIR/fixtures/mount/bin:$PATH";}
+# This needs an explicit cword param or will output "unresolved".
+assert_complete $expected $cmd $test "/@" 20 "/"
+sync_after_int
+assert_bash_exec {PATH="$OLDPATH"; unset -v OLDPATH}
+
+
+sync_after_int
+
+
+teardown

----------------------------------------------------------------------

Comment By: Ildar Mulyukov (ildar-guest)
Date: 2010-01-23 06:56

Message:
Hi, Leonard!
Thanks for opening this bug.
Unfortunately your patch doesn't fix the problem.

While you don't use NFS, you can fake it with a simple script. Name it showmount and put in PATHs before /usr/sbin:
#!/bin/sh
echo /1
echo /1/2
echo /3

----------------------------------------------------------------------

You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=312285&group_id=100114

