Hello Frank,

Frank Terbeck schrieb/wrote:
> 
> Frank Terbeck <ft@grml.org> added the comment:
> 
> Looks good! I've just pushed this to the repository.
> 
> Thanks again!

Thank you! ;)

nice day.


kind regards,

     Thilo

Looks good! I've just pushed this to the repository.

Thanks again!

Frank Terbeck schrieb/wrote:

-- <snip> --
> The #f1# line needs to be located directly above the "sll() {" line,

check.

> I'd like this to be
> 
>     if ...; then

check.

-- <snip> --
> Also for consistency, "local -i RTN ..." and "local -a SEENINODES" here.

check.

-- <snip> --
> Also for consistency
> 
>     for i in ...; do

check.

Updated patch attached. It also fixes a bug in symlink loop detection.
I ran a complete test suite to make sure no regression is introduced.


> Other than that, this looks good to me.
> 
> Thanks for your work!

Thank you for your review!



kind regards,

     Thilo

Hi,

Mika highlighted this to me for review. So here goes:

-#f1# List symlinks in detail (more detailed version of 'readlink -f' and
'whence -s')
+#f1# List symlinks in detail (more detailed version of 'readlink -f', 'whence
-s' and 'namei -l')
+#
+# Usage:
+#
[...]

The #f1# line needs to be located directly above the "sll() {" line, because
the reference-chart generation script takes a look at the line below to
determine the name of the function that is documented.

 sll() {
-    [[ -z "$1" ]] && printf 'Usage: %s <file(s)>\n' "$0" && return 1
-    local file
-    for file in "$@" ; do
+    if [[ -z ${1} ]]
+    then

I'd like this to be

    if ...; then

for consistency with most of the existing code.

[...]
+    local curdir="${PWD}"
+    declare -i RTN LINODE i
+    declare -a SEENINODES

Also for consistency, "local -i RTN ..." and "local -a SEENINODES" here.

[...]
+                LINODE=$(zstat -L +inode "${file}")
+                for i in ${SEENINODES}
+                do

Also for consistency

    for i in ...; do


Other than that, this looks good to me.

Thanks for your work!


Regards, Frank

Thilo Six schrieb/wrote:
-- <snip> --
> This whole thing is quite more complex then i ever could imagine, before.
> But then what i really like is, i have learned quite a bit through all that.

Hello

now i have a version that suits me. The decision if it also suit grml is left to
you.

A few notes:
The usage of cd seem hackish probably at first. Yet i have come to conclude cd
is a rather powerfull builtin. Especially after reading the aforementioned
article. I now think using cd is no more insane then storing a value in a
variable. It simply just changes the env. Also this way we drop the hard work of
resolving absolute vs. relative symlinks to the shell which is rather informed
about how to do that.
Yes i am aware of modifiers :a and :A. Still these break in certain use cases.

anyway. Final version attached.


kind regards,

     Thilo

Hello

-- <snip a whole lot of crap> --

the more i look into this the more i get puzzeld.


First of i have read this:
http://unix.stackexchange.com/a/79621

Which seems to me like a comprehensive summary of the whole complexity surouding
handling of symlinks.

Linux Kernel source says:
This limits recursive symlink follows to 8, while limiting consecutive symlinks
to 40.

% namei -l
says "exceeded limit of symlinks" after an arbitrary limit of 20.

% ls
starts to mark symlinks as broken after 42 linked symlinks (which i guess is
what meant as consecutive symlinks).

I guess with my arbitrary limit of 8 i am arbitrary in the middle of everyone
else.  ;)

This whole thing is quite more complex then i ever could imagine, before.
But then what i really like is, i have learned quite a bit through all that.

i stop for tonight.

I look more into this later this week.



kind regards,

     Thilo

Thilo Six schrieb/wrote:
> -- <snip> --
>>
>>> I look into this. And also in updating its comment and usage hint.
>>
>> Attached is what i could come up with. It does not use zsh magic.
>> Could be zsh'ified though. ;) Personally i happy with the result.
>> Also attached is a shell script that creates a test env for sll().
>>
>> I testet it as good as i could think of. Tell me what you think.
> 
> Here is a slightly improved version. Improved regarding return status.

Fixed a bug with multiple arguments. Updated!



kind regards,

     Thilo

-- <snip> --
> 
>> I look into this. And also in updating its comment and usage hint.
> 
> Attached is what i could come up with. It does not use zsh magic.
> Could be zsh'ified though. ;) Personally i happy with the result.
> Also attached is a shell script that creates a test env for sll().
> 
> I testet it as good as i could think of. Tell me what you think.

Here is a slightly improved version. Improved regarding return status.

> 
>>
>> Thanks!
>>
>> kind regards,
>>
>>      Thilo
>>
> 
> _____________________________________
> GRML issue tracker <bts@bts.grml.org>
> <http://bts.grml.org/grml/issue1351>
> _____________________________________
>

-- <snip> --

> I look into this. And also in updating its comment and usage hint.

Attached is what i could come up with. It does not use zsh magic.
Could be zsh'ified though. ;) Personally i happy with the result.
Also attached is a shell script that creates a test env for sll().

I testet it as good as i could think of. Tell me what you think.

> 
> Thanks!
> 
> kind regards,
> 
>      Thilo
>

Michael Prokop schrieb/wrote:

-- <snip> --
>> Attached patch fixes this.
> 
> I think the present logic is fine:
> 
> | % sll =java
> | lrwxrwxrwx 1 root root 22 Nov 28  2013 /usr/bin/java -> /etc/alternatives/java
> | lrwxrwxrwx 1 root root 46 Nov 28  2013 /etc/alternatives/java -> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java

ahh ok. I have not been aware of this = thing.
I tried like this:
% l
total 0
-rw------- 1 user user 0  19:23 18.05.2015  file
lrwxrwxrwx 1 user user 4  19:24 18.05.2015  lnk1 -> file
% sll lnk1
lrwxrwxrwx 1 user user 4 Mai 18 19:24 lnk1 -> file

Which gave me nothing more usefull over ls -l. But as i understand its purpose
is to do a $path lookup first, which in turn is neat.

> So /usr/bin/java is my java binary in $PATH, pointing to
> /etc/alternatives/java which itself points to
> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java.
> 
> With your patch we would get:
> 
> | % sll =java
> | lrwxrwxrwx 1 root root 46 Nov 28  2013 /etc/alternatives/java -> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
> | -rwxr-xr-x 1 root root 6368 Apr 26 15:48 /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
> 
> ... which dereferences the files before listing them, so the
> output is incomplete because of the missing /usr/bin/java ->
> /etc/alternatives/java connection.
> 
> What we might want do though is actually adding a full listing of
> the resulting target file, so being
> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java in the above example
> - maybe that's the point you're trying to address? So what might be
> interesting to get is:
> 
> | % sll =java
> | lrwxrwxrwx 1 root root 22 Nov 28  2013 /usr/bin/java -> /etc/alternatives/java
> | lrwxrwxrwx 1 root root 46 Nov 28  2013 /etc/alternatives/java -> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
> | -rwxr-xr-x 1 root root 6368 Apr 26 15:48 /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
> 
> I'd definitely appreciate and welcome a patch which implements that.

I look into this. And also in updating its comment and usage hint.

Thanks!

kind regards,

     Thilo

* Thilo Six wrote in grml's BTS on 20150517 / 17:01:

> looking at sll() in etc/zsh/zshrc i think two lines should be swapped to
> actually do s.th. useful in that function.
> Excuse me if i miss s.th. obvious.

> Attached patch fixes this.

I think the present logic is fine:

| % sll =java
| lrwxrwxrwx 1 root root 22 Nov 28  2013 /usr/bin/java -> /etc/alternatives/java
| lrwxrwxrwx 1 root root 46 Nov 28  2013 /etc/alternatives/java -> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java

So /usr/bin/java is my java binary in $PATH, pointing to
/etc/alternatives/java which itself points to
/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java.

With your patch we would get:

| % sll =java
| lrwxrwxrwx 1 root root 46 Nov 28  2013 /etc/alternatives/java -> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
| -rwxr-xr-x 1 root root 6368 Apr 26 15:48 /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java

... which dereferences the files before listing them, so the
output is incomplete because of the missing /usr/bin/java ->
/etc/alternatives/java connection.

What we might want do though is actually adding a full listing of
the resulting target file, so being
/usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java in the above example
- maybe that's the point you're trying to address? So what might be
interesting to get is:

| % sll =java
| lrwxrwxrwx 1 root root 22 Nov 28  2013 /usr/bin/java -> /etc/alternatives/java
| lrwxrwxrwx 1 root root 46 Nov 28  2013 /etc/alternatives/java -> /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
| -rwxr-xr-x 1 root root 6368 Apr 26 15:48 /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java

I'd definitely appreciate and welcome a patch which implements that.

regards,
-mika-

> Hello
> 
> looking at sll() in etc/zsh/zshrc i think two lines should be swapped to
> actually do s.th. useful in that function.
> Excuse me if i miss s.th. obvious.

And since we are at this, make it whitespace save.
Patch updated accordingly.

> Attached patch fixes this.
> 
> 
> kind regards,
> 
>      Thilo
> 
> ----------
> files: 0001-etc_zsh_zshrc-fix-func-sll.patch
> messages: 4955
> nosy: Xk2c
> status: unread
> title: etc/zsh/zshrc: make sll() do actually s.th. usefull
> 
> _____________________________________
> GRML issue tracker <bts@bts.grml.org>
> <http://bts.grml.org/grml/issue1351>
> _____________________________________
>

Hello

looking at sll() in etc/zsh/zshrc i think two lines should be swapped to
actually do s.th. useful in that function.
Excuse me if i miss s.th. obvious.

Attached patch fixes this.


kind regards,

     Thilo