GNU bug report logs - #30594
[PATCH] Add coreutils 'ls' support for tramp adb

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 30594 in the body.
You can then email your comments to 30594 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: Mathieu Othacehe <m.othacehe <at> gmail.com>
Subject: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sat, 24 Feb 2018 19:06:40 +0100

Support some Android derived systems where 'ls' binary is provided by
GNU Coreutils.

* lisp/net/tramp-adb.el (tramp-adb-ls-toolbox-regexp): Allow '.'
character in file permissions. It indicates an SELinux security
context.
(tramp-adb-get-ls-command): Detect Coreutils version of 'ls'. Force
output on one column and long-iso time style to match the behaviour of
toybox and busybox 'ls' commands.
---
 lisp/net/tramp-adb.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index f5c45f68e9..be269aca51 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -71,7 +71,7 @@ tramp-adb-ls-date-regexp
 
 (defconst tramp-adb-ls-toolbox-regexp
   (concat
-   "^[[:space:]]*\\([-[:alpha:]]+\\)" 	; \1 permissions
+   "^[[:space:]]*\\([-\\.[:alpha:]]+\\)" ; \1 permissions
    "\\(?:[[:space:]]+[[:digit:]]+\\)?"	; links (Android 7/toybox)
    "[[:space:]]*\\([^[:space:]]+\\)"	; \2 username
    "[[:space:]]+\\([^[:space:]]+\\)"	; \3 group
@@ -462,9 +462,15 @@ tramp-adb-get-ls-command
   (with-tramp-connection-property vec "ls"
     (tramp-message vec 5 "Finding a suitable `ls' command")
     (cond
+     ;; Support Android derived systems where "ls" command is provided
+     ;; by GNU Coreutils. Force "ls" to print one column and set
+     ;; time-style to imitate other "ls" flavours.
+     ((tramp-adb-send-command-and-check vec "ls --help | grep coreutils")
+      "env COLUMNS=1 ls --time-style=long-iso")
      ;; Can't disable coloring explicitly for toybox ls command.  We
-     ;; must force "ls" to print just one column.
-     ((tramp-adb-send-command-and-check vec "toybox") "env COLUMNS=1 ls")
+     ;; also must force "ls" to print just one column.
+     ((tramp-adb-send-command-and-check vec "toybox")
+      "env COLUMNS=1 ls")
      ;; On CyanogenMod based system BusyBox is used and "ls" output
      ;; coloring is enabled by default.  So we try to disable it when
      ;; possible.
-- 
2.16.1

Message #8 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 30594 <at> debbugs.gnu.org
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sat, 24 Feb 2018 21:56:51 +0100

Mathieu Othacehe <m.othacehe <at> gmail.com> writes:

Hi Mathieu,

> Support some Android derived systems where 'ls' binary is provided by
> GNU Coreutils.

Thanks for this! I cannot test myself,  so I'll trust that it works
proper. However,

> +     ;; Support Android derived systems where "ls" command is provided
> +     ;; by GNU Coreutils. Force "ls" to print one column and set
> +     ;; time-style to imitate other "ls" flavours.
> +     ((tramp-adb-send-command-and-check vec "ls --help | grep coreutils")
> +      "env COLUMNS=1 ls --time-style=long-iso")

I believe it is not future-safe to depend on any string in --help
output. The better approach is to test, whether the ls command accepts
the --time-style=long-iso argument. Something like

((tramp-adb-send-command-and-check vec "ls --time-style=long-iso /dev/null")

Could you pls check, whether it works like this?

Best regards, Michael.

Message #11 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 30594 <at> debbugs.gnu.org
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sat, 24 Feb 2018 22:04:23 +0100

Michael Albinus <michael.albinus <at> gmx.de> writes:

Hi Mathieu,

> Could you pls check, whether it works like this?

Ahh, and btw: "." is not special in "[]" in regexps. So you might do

+   "^[[:space:]]*\\([-.[:alpha:]]+\\)" ; \1 permissions

Best regards, Michael.

Message #14 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 30594 <at> debbugs.gnu.org
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sun, 25 Feb 2018 10:42:18 +0100

[Message part 1 (text/plain, inline)]

Hi Michael,

> Ahh, and btw: "." is not special in "[]" in regexps. So you might do
>
> +   "^[[:space:]]*\\([-.[:alpha:]]+\\)" ; \1 permissions

Thanks for reviewing! I fixed your remarks, it still works fine. You'll
find v2 attached.

Mathieu

[0001-Add-coreutils-ls-support-for-tramp-adb.patch (text/x-patch, inline)]

From 553412dad14be2f8c12a61c7d96cbd2ce7e7693e Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe <at> gmail.com>
Date: Sat, 24 Feb 2018 18:56:54 +0100
Subject: [PATCH v2] Add coreutils 'ls' support for tramp adb

Support some Android derived systems where 'ls' binary is provided by
GNU Coreutils.

* lisp/net/tramp-adb.el (tramp-adb-ls-toolbox-regexp): Allow '.'
character in file permissions. It indicates an SELinux security
context.
(tramp-adb-get-ls-command): Detect Coreutils version of 'ls'. Force
output on one column and long-iso time style to match the behaviour of
toybox and busybox 'ls' commands.
---
 lisp/net/tramp-adb.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index f5c45f68e9..443d45d831 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -71,7 +71,7 @@ tramp-adb-ls-date-regexp
 
 (defconst tramp-adb-ls-toolbox-regexp
   (concat
-   "^[[:space:]]*\\([-[:alpha:]]+\\)" 	; \1 permissions
+   "^[[:space:]]*\\([-.[:alpha:]]+\\)" ; \1 permissions
    "\\(?:[[:space:]]+[[:digit:]]+\\)?"	; links (Android 7/toybox)
    "[[:space:]]*\\([^[:space:]]+\\)"	; \2 username
    "[[:space:]]+\\([^[:space:]]+\\)"	; \3 group
@@ -462,9 +462,15 @@ tramp-adb-get-ls-command
   (with-tramp-connection-property vec "ls"
     (tramp-message vec 5 "Finding a suitable `ls' command")
     (cond
+     ;; Support Android derived systems where "ls" command is provided
+     ;; by GNU Coreutils. Force "ls" to print one column and set
+     ;; time-style to imitate other "ls" flavours.
+     ((tramp-adb-send-command-and-check vec "ls --time-style=long-iso /dev/null")
+      "env COLUMNS=1 ls --time-style=long-iso")
      ;; Can't disable coloring explicitly for toybox ls command.  We
-     ;; must force "ls" to print just one column.
-     ((tramp-adb-send-command-and-check vec "toybox") "env COLUMNS=1 ls")
+     ;; also must force "ls" to print just one column.
+     ((tramp-adb-send-command-and-check vec "toybox")
+      "env COLUMNS=1 ls")
      ;; On CyanogenMod based system BusyBox is used and "ls" output
      ;; coloring is enabled by default.  So we try to disable it when
      ;; possible.
-- 
2.16.1

Message #17 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 30594 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sun, 25 Feb 2018 11:13:37 +0100

On Feb 25 2018, Mathieu Othacehe <m.othacehe <at> gmail.com> wrote:

> @@ -462,9 +462,15 @@ tramp-adb-get-ls-command
>    (with-tramp-connection-property vec "ls"
>      (tramp-message vec 5 "Finding a suitable `ls' command")
>      (cond
> +     ;; Support Android derived systems where "ls" command is provided
> +     ;; by GNU Coreutils. Force "ls" to print one column and set

Thats `ls -1'.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #20 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 30594 <at> debbugs.gnu.org, Michael Albinus <michael.albinus <at> gmx.de>
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sun, 25 Feb 2018 11:48:04 +0100

[Message part 1 (text/plain, inline)]

Hi Andreas,

> Thats `ls -1'.

You're right, I updated my patch. As toybox 'ls' seems to also support
this option, here's an other patch to use 'ls -1' for toybox too.

Thanks,

Mathieu

[0001-Add-coreutils-ls-support-for-tramp-adb.patch (text/x-patch, inline)]

From 20a0de9436cdac80ac22c1b1efbbaf9654d5bf7f Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <mathieu.othacehe <at> parrot.com>
Date: Sat, 24 Feb 2018 18:12:36 +0100
Subject: [PATCH] Add coreutils 'ls' support for tramp adb

Support some Android derived systems where 'ls' binary is provided by
GNU Coreutils.

* lisp/net/tramp-adb.el (tramp-adb-ls-toolbox-regexp): Allow '.'
character in file permissions. It indicates an SELinux security
context.
(tramp-adb-get-ls-command): Detect Coreutils version of 'ls'. Force
output on one column and long-iso time style to match the behaviour of
toybox and busybox 'ls' commands.
---
 lisp/net/tramp-adb.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index f5c45f68e9..5489dbe5a5 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -71,7 +71,7 @@ tramp-adb-ls-date-regexp
 
 (defconst tramp-adb-ls-toolbox-regexp
   (concat
-   "^[[:space:]]*\\([-[:alpha:]]+\\)" 	; \1 permissions
+   "^[[:space:]]*\\([-\\.[:alpha:]]+\\)" ; \1 permissions
    "\\(?:[[:space:]]+[[:digit:]]+\\)?"	; links (Android 7/toybox)
    "[[:space:]]*\\([^[:space:]]+\\)"	; \2 username
    "[[:space:]]+\\([^[:space:]]+\\)"	; \3 group
@@ -462,9 +462,15 @@ tramp-adb-get-ls-command
   (with-tramp-connection-property vec "ls"
     (tramp-message vec 5 "Finding a suitable `ls' command")
     (cond
+     ;; Support Android derived systems where "ls" command is provided
+     ;; by GNU Coreutils. Force "ls" to print one column and set
+     ;; time-style to imitate other "ls" flavours.
+     ((tramp-adb-send-command-and-check vec "ls --help | grep coreutils")
+      "ls -1 --time-style=long-iso")
      ;; Can't disable coloring explicitly for toybox ls command.  We
-     ;; must force "ls" to print just one column.
-     ((tramp-adb-send-command-and-check vec "toybox") "env COLUMNS=1 ls")
+     ;; also must force "ls" to print just one column.
+     ((tramp-adb-send-command-and-check vec "toybox")
+      "env COLUMNS=1 ls")
      ;; On CyanogenMod based system BusyBox is used and "ls" output
      ;; coloring is enabled by default.  So we try to disable it when
      ;; possible.
-- 
2.16.1

[0001-Use-ls-1-to-list-files-with-toybox-ls.patch (text/x-patch, inline)]

From c41d53965288843285faf1cf9344d6b751eaf1fc Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe <at> gmail.com>
Date: Sun, 25 Feb 2018 11:32:30 +0100
Subject: [PATCH] Use 'ls -1' to list files with toybox ls

* lisp/net/tramp-adb.el (tramp-adb-get-ls-command): Use 'ls -1'
instead of passing COLUMNS=1 env variable. This is isofunctional.
---
 lisp/net/tramp-adb.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index 5489dbe5a5..6fe38fb2e5 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -470,7 +470,7 @@ tramp-adb-get-ls-command
      ;; Can't disable coloring explicitly for toybox ls command.  We
      ;; also must force "ls" to print just one column.
      ((tramp-adb-send-command-and-check vec "toybox")
-      "env COLUMNS=1 ls")
+      "ls -1")
      ;; On CyanogenMod based system BusyBox is used and "ls" output
      ;; coloring is enabled by default.  So we try to disable it when
      ;; possible.
-- 
2.16.1

Message #23 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 30594 <at> debbugs.gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sun, 25 Feb 2018 11:57:54 +0100

Mathieu Othacehe <m.othacehe <at> gmail.com> writes:

> Hi Andreas,

Hi Mathieu,

> You're right, I updated my patch. As toybox 'ls' seems to also support
> this option, here's an other patch to use 'ls -1' for toybox too.

> +     ((tramp-adb-send-command-and-check vec "ls --help | grep coreutils")

This is not what we have agreed. Could you, pls, rework?

> Thanks,
>
> Mathieu

Best regards, Michael

Message #26 received at 30594 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 30594 <at> debbugs.gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Sun, 25 Feb 2018 12:18:52 +0100

[Message part 1 (text/plain, inline)]

> This is not what we have agreed. Could you, pls, rework?

Oops wrong rebase, sorry about that. Here's an update version.

Mathieu

[0001-Add-coreutils-ls-support-for-tramp-adb.patch (text/x-patch, inline)]

From c904d5da9d65b95a6176c86bb6ba143b29e90f1d Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <mathieu.othacehe <at> parrot.com>
Date: Sat, 24 Feb 2018 18:12:36 +0100
Subject: [PATCH] Add coreutils 'ls' support for tramp adb

Support some Android derived systems where 'ls' binary is provided by
GNU Coreutils.

* lisp/net/tramp-adb.el (tramp-adb-ls-toolbox-regexp): Allow '.'
character in file permissions. It indicates an SELinux security
context.
(tramp-adb-get-ls-command): Detect Coreutils version of 'ls'. Force
output on one column and long-iso time style to match the behaviour of
toybox and busybox 'ls' commands.
---
 lisp/net/tramp-adb.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index f5c45f68e9..6257f84d6f 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -71,7 +71,7 @@ tramp-adb-ls-date-regexp
 
 (defconst tramp-adb-ls-toolbox-regexp
   (concat
-   "^[[:space:]]*\\([-[:alpha:]]+\\)" 	; \1 permissions
+   "^[[:space:]]*\\([-.[:alpha:]]+\\)" ; \1 permissions
    "\\(?:[[:space:]]+[[:digit:]]+\\)?"	; links (Android 7/toybox)
    "[[:space:]]*\\([^[:space:]]+\\)"	; \2 username
    "[[:space:]]+\\([^[:space:]]+\\)"	; \3 group
@@ -462,9 +462,15 @@ tramp-adb-get-ls-command
   (with-tramp-connection-property vec "ls"
     (tramp-message vec 5 "Finding a suitable `ls' command")
     (cond
+     ;; Support Android derived systems where "ls" command is provided
+     ;; by GNU Coreutils. Force "ls" to print one column and set
+     ;; time-style to imitate other "ls" flavours.
+     ((tramp-adb-send-command-and-check vec "ls --time-style=long-iso /dev/null")
+      "ls -1 --time-style=long-iso")
      ;; Can't disable coloring explicitly for toybox ls command.  We
-     ;; must force "ls" to print just one column.
-     ((tramp-adb-send-command-and-check vec "toybox") "env COLUMNS=1 ls")
+     ;; also must force "ls" to print just one column.
+     ((tramp-adb-send-command-and-check vec "toybox")
+      "env COLUMNS=1 ls")
      ;; On CyanogenMod based system BusyBox is used and "ls" output
      ;; coloring is enabled by default.  So we try to disable it when
      ;; possible.
-- 
2.16.1

[0001-Use-ls-1-to-list-files-with-toybox-ls.patch (text/x-patch, inline)]

From 6d216933f307bdb946d569510853003fafea3c45 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe <at> gmail.com>
Date: Sun, 25 Feb 2018 11:32:30 +0100
Subject: [PATCH] Use 'ls -1' to list files with toybox ls

* lisp/net/tramp-adb.el (tramp-adb-get-ls-command): Use 'ls -1'
instead of passing COLUMNS=1 env variable. This is isofunctional.
---
 lisp/net/tramp-adb.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index 6257f84d6f..3f4de93bc5 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -470,7 +470,7 @@ tramp-adb-get-ls-command
      ;; Can't disable coloring explicitly for toybox ls command.  We
      ;; also must force "ls" to print just one column.
      ((tramp-adb-send-command-and-check vec "toybox")
-      "env COLUMNS=1 ls")
+      "ls -1")
      ;; On CyanogenMod based system BusyBox is used and "ls" output
      ;; coloring is enabled by default.  So we try to disable it when
      ;; possible.
-- 
2.16.1

Message #31 received at 30594-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 30594-done <at> debbugs.gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: Re: bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb
Date: Mon, 26 Feb 2018 16:52:24 +0100

Version: 27.1

Mathieu Othacehe <m.othacehe <at> gmail.com> writes:

> Oops wrong rebase, sorry about that. Here's an update version.

Thanks. I have pushed your patch to both Tramp and Emacs
repositories. Closing the bug.

> Mathieu

Best regards, Michael.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.