Re: Add chmod builtin

[Matthew Martin <phy1729@gmail.com>]

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

On Sun, Mar 17, 2019 at 6:27 AM dana <dana@dana.is> wrote:
> >+if zmodload -e zsh/files && [[ $words[1] != */* ]]; then
>
> I don't think this is reliable. It will detect any chmod as the built-in even
> if only zf_chown is loaded (with -F). And `zmodload -e` always returns 1 for
> modules loaded through selective auto-loading (-Fa), so it will *never*
> detect the built-in in that case. (Not sure if that -e behaviour is intended?)
>
> I know the $OSTYPE-$variant thing is dumb (should have passed $OSTYPE directly
> to _pick_variant), but otherwise i think the method i used for distinguishing
> the built-ins in _stat is fairly robust.

Reading the docs for _pick_variant, there's a -b option to detect
builtins. It has some issues (namely with precommands), but those can be
fixed in a separate commit. Requires some reindenting, so I've split the
patch in two.

> PS: Your patch seems to have corrupt white space

Gmail hasn't been sending mails sent via mutt for the past few months.
Not sure what's causing it, but I've had to resort to using the web
interface which seems to mangle whitespace. I've attached the patches
and they're on my github repo as well.

[-- Attachment #2: 0001-44135-_chmod-Reformat-to-minimize-next-diff.-No-func.patch --]
[-- Type: text/x-patch, Size: 4760 bytes --]

From 2f670be951b1b6e242691562a9daf06010bfce99 Mon Sep 17 00:00:00 2001
From: Matthew Martin <phy1729@gmail.com>
Date: Sun, 17 Mar 2019 19:24:56 -0500
Subject: [PATCH 1/2] 44135: _chmod: Reformat to minimize next diff. No
 functional change.

---
 Completion/Unix/Command/_chmod | 94 ++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 44 deletions(-)

diff --git a/Completion/Unix/Command/_chmod b/Completion/Unix/Command/_chmod
index af64b9eb9..6a44a4ef1 100644
--- a/Completion/Unix/Command/_chmod
+++ b/Completion/Unix/Command/_chmod
@@ -1,52 +1,58 @@
 #compdef chmod gchmod
 
-local curcontext="$curcontext" state line expl ret=1
+local curcontext="$curcontext" state line expl ret=1 variant
 local -a args privs
 
 args=( '*: :->files' '1: :_file_modes' )
 
-if _pick_variant gnu=Free\ Soft unix --version; then
-  args+=(
-    '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
-    '(-v --verbose -c --changes)'{-v,--verbose}'[output a diagnostic for every file processed]'
-    '(-f --silent --quiet)'{-f,--silent,--quiet}'[suppress most error messages]'
-    '(--no-preserve-root)--preserve-root[fail to operate recursively on /]'
-    "(--preserve-root)--no-preserve-root[don't treat / specially (default)]"
-    '(1)--reference=[copy permissions of specified file]:file:_files'
-    '(-R --recursive)'{-R,--recursive}'[change files and directories recursively]'
-    '(- : *)--help[display help information]'
-    '(- : *)--version[display version information]'
-  )
-else
-  args+=(
-    '-f[suppress most error messages]'
-    '-R[change files and directories recursively]'
-  )
-  case $OSTYPE in
-    freebsd*|dragonfly*|darwin*)
-      args+=( '-v[output a diagnostic for every file processed]')
+_pick_variant -r variant gnu=Free\ Soft $OSTYPE --version
+case "$variant" in
+  gnu)
+    args+=(
+      '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
+      '(-v --verbose -c --changes)'{-v,--verbose}'[output a diagnostic for every file processed]'
+      '(-f --silent --quiet)'{-f,--silent,--quiet}'[suppress most error messages]'
+      '(--no-preserve-root)--preserve-root[fail to operate recursively on /]'
+      "(--preserve-root)--no-preserve-root[don't treat / specially (default)]"
+      '(1)--reference=[copy permissions of specified file]:file:_files'
+      '(-R --recursive)'{-R,--recursive}'[change files and directories recursively]'
+      '(- : *)--help[display help information]'
+      '(- : *)--version[display version information]'
+    )
+    ;;
+  *)
+    args+=(
+      '-f[suppress most error messages]'
+      '-R[change files and directories recursively]'
+    )
+    ;|
+  freebsd*|dragonfly*|darwin*)
+    args+=(
+      '-v[output a diagnostic for every file processed]'
+    )
     ;|
-    freebsd*|netbsd*|darwin*|dragonfly*)
-      args+=( "-h[operate on symlinks them self]" )
+  freebsd*|netbsd*|darwin*|dragonfly*)
+    args+=(
+      '-h[operate on symlinks them self]'
+    )
     ;|
-    freebsd*|openbsd*|netbsd*|darwin*|dragonfly*)
-      args+=(
-	'(-H -L -P)-L[follow all symlinks]'
-	'(-H -L -P)-H[follow symlinks on the command line]'
-	'(-H -L -P)-P[do not follow symlinks (default)]'
-      )
+  freebsd*|openbsd*|netbsd*|darwin*|dragonfly*)
+    args+=(
+      '(-H -L -P)-L[follow all symlinks]'
+      '(-H -L -P)-H[follow symlinks on the command line]'
+      '(-H -L -P)-P[do not follow symlinks (default)]'
+    )
     ;|
-    darwin*)
-      args+=(
-        '(1)-C[returns false if any of the named files have ACLs]'
-	'(1)-N[remove ACLs from specified files]'
-	'(1)-E[read ACL info from stdin as a sequential list of ACEs]'
-	'(1)-i[removes inherited bit from all entries in named files ACLs]'
-        '(1)-I[removes all inherited entries from named files ACLs]'
-      )
+  darwin*)
+    args+=(
+      '(1)-C[returns false if any of the named files have ACLs]'
+      '(1)-N[remove ACLs from specified files]'
+      '(1)-E[read ACL info from stdin as a sequential list of ACEs]'
+      '(1)-i[removes inherited bit from all entries in named files ACLs]'
+      '(1)-I[removes all inherited entries from named files ACLs]'
+    )
     ;;
-  esac
-fi
+esac
 
 _arguments -C -s "$args[@]" && ret=0
 
@@ -62,12 +68,12 @@ case "$state" in
       local spec who op priv
       local -a specs
       for spec in ${(s:,:)line[1]}; do
-	if [[ ${spec#*[+-=]} != [rwxst]## ]]; then
-	  _files && ret=0
-	  return ret
-	fi
+        if [[ ${spec#*[+-=]} != [rwxst]## ]]; then
+          _files && ret=0
+          return ret
+        fi
 
-	specs+=( ${${(M)spec##[+-=]*}:+a}$spec )
+        specs+=( ${${(M)spec##[+-=]*}:+a}$spec )
       done
       _wanted files expl file _files -g "*(-.^f:${(j.,.)specs}:)" && ret=0
     fi
-- 
2.21.0


[-- Attachment #3: 0002-44135-Add-chmod-builtin.patch --]
[-- Type: text/x-patch, Size: 4874 bytes --]

From 8d526281e56cec6579892706c2c305246269b7eb Mon Sep 17 00:00:00 2001
From: Matthew Martin <phy1729@gmail.com>
Date: Sun, 17 Mar 2019 19:30:47 -0500
Subject: [PATCH 2/2] 44135: Add chmod builtin

---
 Completion/Unix/Command/_chmod | 13 +++++++++--
 Doc/Zsh/mod_files.yo           | 20 +++++++++++++++++
 Src/Modules/files.c            | 41 ++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/Completion/Unix/Command/_chmod b/Completion/Unix/Command/_chmod
index 6a44a4ef1..89a7fbd43 100644
--- a/Completion/Unix/Command/_chmod
+++ b/Completion/Unix/Command/_chmod
@@ -1,12 +1,21 @@
-#compdef chmod gchmod
+#compdef chmod gchmod zf_chmod
 
 local curcontext="$curcontext" state line expl ret=1 variant
 local -a args privs
 
 args=( '*: :->files' '1: :_file_modes' )
 
-_pick_variant -r variant gnu=Free\ Soft $OSTYPE --version
+_pick_variant -r variant -b zsh gnu=Free\ Soft $OSTYPE --version
 case "$variant" in
+  zsh)
+    # Assign, not append because zf_chmod only supports octal modes.
+    args=(
+      '-R[change files and directories recursively]' \
+      '-s[enable paranoid behavior]' \
+      '1:octal mode:' \
+      '*: :->files'
+    )
+    ;;
   gnu)
     args+=(
       '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
diff --git a/Doc/Zsh/mod_files.yo b/Doc/Zsh/mod_files.yo
index 90e988474..3cf7b61e3 100644
--- a/Doc/Zsh/mod_files.yo
+++ b/Doc/Zsh/mod_files.yo
@@ -23,6 +23,26 @@ item(tt(chgrp) [ tt(-hRs) ] var(group) var(filename) ...)(
 Changes group of files specified.  This is equivalent to tt(chown) with
 a var(user-spec) argument of `tt(:)var(group)'.
 )
+findex(chmod)
+item(tt(chmod) [ tt(-Rs) ] var(mode) var(filename) ...)(
+Changes mode of files specified.
+
+The specified var(mode) must be in octal.
+
+The tt(-R) option causes tt(chmod) to recursively descend into directories,
+changing the mode of all files in the directory after
+changing the mode of the directory itself.
+
+The tt(-s) option is a zsh extension to tt(chmod) functionality.  It enables
+paranoid behaviour, intended to avoid security problems involving
+a tt(chmod) being tricked into affecting files other than the ones
+intended.  It will refuse to follow symbolic links, so that (for example)
+``tt(chmod 600 /tmp/foo/passwd)'' can't accidentally chmod tt(/etc/passwd)
+if tt(/tmp/foo) happens to be a link to tt(/etc).  It will also check
+where it is after leaving directories, so that a recursive chmod of
+a deep directory tree can't end up recursively chmoding tt(/usr) as
+a result of directories being moved up the tree.
+)
 findex(chown)
 item(tt(chown) [ tt(-hRs) ] var(user-spec) var(filename) ...)(
 Changes ownership and group of files specified.
diff --git a/Src/Modules/files.c b/Src/Modules/files.c
index 6f816bac0..85764d55e 100644
--- a/Src/Modules/files.c
+++ b/Src/Modules/files.c
@@ -619,6 +619,45 @@ bin_rm(char *nam, char **args, Options ops, UNUSED(int func))
     return OPT_ISSET(ops,'f') ? 0 : err;
 }
 
+/* chmod builtin */
+
+struct chmodmagic {
+    char *nam;
+    mode_t mode;
+};
+
+/**/
+static int
+chmod_dochmod(char *arg, char *rp, UNUSED(struct stat const *sp), void *magic)
+{
+    struct chmodmagic *chm = magic;
+
+    if(chmod(rp, chm->mode)) {
+	zwarnnam(chm->nam, "%s: %e", arg, errno);
+	return 1;
+    }
+    return 0;
+}
+
+/**/
+static int
+bin_chmod(char *nam, char **args, Options ops, int func)
+{
+    struct chmodmagic chm;
+    char *str = args[0], *ptr;
+
+    chm.nam = nam;
+
+    chm.mode = zstrtol(str, &ptr, 8);
+    if(!*str || *ptr) {
+	zwarnnam(nam, "invalid mode `%s'", str);
+	return 1;
+    }
+
+    return recursivecmd(nam, 0, OPT_ISSET(ops,'R'), OPT_ISSET(ops,'s'),
+	args + 1, chmod_dochmod, recurse_donothing, chmod_dochmod, &chm);
+}
+
 /* chown builtin */
 
 struct chownmagic {
@@ -754,6 +793,7 @@ static struct builtin bintab[] = {
     /* The names which overlap commands without necessarily being
      * fully compatible. */
     BUILTIN("chgrp", 0, bin_chown, 2, -1, BIN_CHGRP, "hRs",    NULL),
+    BUILTIN("chmod", 0, bin_chmod, 2, -1, 0,         "Rs",    NULL),
     BUILTIN("chown", 0, bin_chown, 2, -1, BIN_CHOWN, "hRs",    NULL),
     BUILTIN("ln",    0, bin_ln,    1, -1, BIN_LN,    LN_OPTS, NULL),
     BUILTIN("mkdir", 0, bin_mkdir, 1, -1, 0,         "pm:",   NULL),
@@ -763,6 +803,7 @@ static struct builtin bintab[] = {
     BUILTIN("sync",  0, bin_sync,  0,  0, 0,         NULL,    NULL),
     /* The "safe" zsh-only names */
     BUILTIN("zf_chgrp", 0, bin_chown, 2, -1, BIN_CHGRP, "hRs",    NULL),
+    BUILTIN("zf_chmod", 0, bin_chmod, 2, -1, 0,         "Rs",    NULL),
     BUILTIN("zf_chown", 0, bin_chown, 2, -1, BIN_CHOWN, "hRs",    NULL),
     BUILTIN("zf_ln",    0, bin_ln,    1, -1, BIN_LN,    LN_OPTS, NULL),
     BUILTIN("zf_mkdir", 0, bin_mkdir, 1, -1, 0,         "pm:",   NULL),
-- 
2.21.0