bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Mathieu Othacehe]

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

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Michael Albinus]

Mathieu Othacehe <m.othacehe@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.

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Michael Albinus]

Michael Albinus <michael.albinus@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.

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-coreutils-ls-support-for-tramp-adb.patch --]
[-- Type: text/x-patch, Size: 2226 bytes --]

From 553412dad14be2f8c12a61c7d96cbd2ce7e7693e Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@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

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Andreas Schwab]

On Feb 25 2018, Mathieu Othacehe <m.othacehe@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@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 187 bytes --]


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-coreutils-ls-support-for-tramp-adb.patch --]
[-- Type: text/x-patch, Size: 2213 bytes --]

From 20a0de9436cdac80ac22c1b1efbbaf9654d5bf7f Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <mathieu.othacehe@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Use-ls-1-to-list-files-with-toybox-ls.patch --]
[-- Type: text/x-patch, Size: 1017 bytes --]

From c41d53965288843285faf1cf9344d6b751eaf1fc Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@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

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Michael Albinus]

Mathieu Othacehe <m.othacehe@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

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 133 bytes --]


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

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

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-coreutils-ls-support-for-tramp-adb.patch --]
[-- Type: text/x-patch, Size: 2219 bytes --]

From c904d5da9d65b95a6176c86bb6ba143b29e90f1d Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <mathieu.othacehe@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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Use-ls-1-to-list-files-with-toybox-ls.patch --]
[-- Type: text/x-patch, Size: 1017 bytes --]

From 6d216933f307bdb946d569510853003fafea3c45 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <m.othacehe@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

bug#30594: [PATCH] Add coreutils 'ls' support for tramp adb

[Michael Albinus]

Version: 27.1

Mathieu Othacehe <m.othacehe@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.