Re: Contributors and maintainers

[Taylan Ulrich Bayırlı/Kammer]

David Kastrup <address@hidden> writes:

> Werner LEMBERG <address@hidden> writes:
>
>>>> No submitter can brow-beat us into accepting a patch because they
>>>> think it is "clear" or "right" or "obvious".  This isn't how
>>>> collaboration works in the free software world.  We decide who has
>>>> commit rights, and we reserve the right to reject and revert
>>>> commits.
>>> 
>>> I provided clarification several times.  It was ignored.
>>
>> Certainly not.  You got *a lot* of replies.
>>
>>> Let me list some different mails in which I repeated more or less
>>> the same explanation with different wording:
>>
>> This is exactly the `agree to disagree' situation I've mentioned in a
>> previous e-mail.
>
> Well, these days generally "discussion" is understood as everybody
> repeating his opinion until most drop out, maybe a trickling down from
> the culture of political debate, with a focus on scoring points rather
> than extending one's views.
>
> This mode of discussion tends to work rather bad in a closed round of
> experts.  Repeating your point on the assumption that your opponent was
> just too dumb to get it the first time gets old rather fast.  Instead of
> making the same point over and over again and riling everybody including
> yourself up in the process, you better try bringing up new facts or
> considerations.  Everything else is only likely to affect the emotional
> but not the factual result of the discussion.
>
> While "everybody's glad that this is over and one will not meet ever
> again" may be a somewhat emotionally conclusive resolution in substitute
> for a convergence to factual agreement, it's not much of a basis for
> ongoing work.

That sounds like saying a discussion where people stick to a clear,
thoroughly and carefully explained point, and don't move on until it's
addressed are silly political discussions, and a discussion between
experts should rather look like one where everyone just jumps in and
brings up another unique perspective which fails to address what was
shortly brought up.

I have to disagree, and offer an alternative analysis by someone who's
not me and is quite a bit better at social issues than me.

    The usual approach on emacs-devel when dealing with something they
    don't like is to come up with random arguments, ignore
    counter-arguments, and move goalposts around because getting
    convinced by arguments is not something you do on the internet.

    From the glimpse I took, that's roughly what's happening there:
    They don't like the idea because gut feeling, so they nitpick
    irrelevancies and go off on tangents to support their gut feeling.

    ...

    [Them] Taylan, could you summarize your core issue here? Not what
    e-d is discussing, but what would you want to happen as the ideal
    outcome?

    [Me] Shell-quote-argument should cleanly document its semantics in
    a way that gives a security-aware programmer confidence that they
    can use the function without worrying about injection.

    [Them] That was the purpose of your [PATCH] thread?

    [Me] That's the summary of the shell-quote-argument bug report.
    The original [PATCH] thread just wants to get shqq into ELPA.

    [Them] Is shqq getting into ELPA?

    [Me] They could have taken it as-is and worked on the "duplication
    of code" (one line of code) later.  They want it neither with the
    one line of duplicated effort, nor do they want to address the
    shell-quote-argument bug report, and I don't want it in ELPA with
    potentially broken semantics.

    [Them] Now that does sound like a more appropriate summary of the
    problem there.  "The thread is about getting shqq into GNU ELPA,
    but it is being rejected on spurious grounds of a single line of
    code supposedly duplicating the intent of some existing piece of
    code."  The latter is the derail.

Indeed, the whole absurdity of the thread is perhaps best summarized by
the fact that it boils down to one line of code (and alternatively, a
single documentation string).


How about, *first* of all, the latest version of my ELPA patch gets
applied, so there is an *immediate* benefit to Emacs users.  Claiming
that a single line of duplicated code outweighs that would be absurd.

After that, emacs-devel can make whatever change they want to the
package.  My opinion is that it's unethical to whitewash potential
security issues, so I beg you to think hard about them and do what's
necessary to eliminate their possibility in shell-quote-argument.  The
best suggestion I've heard was improving unit tests for that, although a
stricter documentation would also be helpful in my opinion.  You're free
to ignore these suggestions an opinions, in which case I'm not
responsible even if you change shqq to use shell-quote-argument.

In any case, please accept the ELPA patch.  Holding it off any longer
would be absurd unless there's a *serious* issue with it.

Here it is so you don't need to dig it out from the older mails.

Thanks.

>From 276e3adc61b2f083b0348fd231a97feaa7017e36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Taylan=20Ulrich=20Bay=C4=B1rl=C4=B1/Kammer?=
 <address@hidden>
Date: Sat, 17 Oct 2015 18:32:22 +0200
Subject: [PATCH 1/3] Add shell-quasiquote.

---
 packages/shell-quasiquote/shell-quasiquote.el | 151 ++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 packages/shell-quasiquote/shell-quasiquote.el

diff --git a/packages/shell-quasiquote/shell-quasiquote.el 
b/packages/shell-quasiquote/shell-quasiquote.el
new file mode 100644
index 0000000..1f18862
--- /dev/null
+++ b/packages/shell-quasiquote/shell-quasiquote.el
@@ -0,0 +1,151 @@
+;;; shell-quasiquote.el --- Turn s-expressions into shell command strings.
+
+;; Copyright (C) 2015  Free Software Foundation, Inc.
+
+;; Author: Taylan Ulrich Bayırlı/Kammer <address@hidden>
+;; Keywords: extensions, unix
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; "Shell quasiquote" -- turn s-expressions into POSIX shell command strings.
+;;
+;; Shells other than POSIX sh are not supported.
+;;
+;; Quoting is automatic and safe against injection.
+;;
+;;   (let ((file1 "file one")
+;;         (file2 "file two"))
+;;     (shqq (cp -r ,file1 ,file2 "My Files")))
+;;       => "cp -r 'file one' 'file two' 'My Files'"
+;;
+;; You can splice many arguments into place with ,@foo.
+;;
+;;   (let ((files (list "file one" "file two")))
+;;     (shqq (cp -r ,@files "My Files")))
+;;       => "cp -r 'file one' 'file two' 'My Files'"
+;;
+;; Note that the quoting disables a variety of shell expansions like ~/foo,
+;; $ENV_VAR, and e.g. {x..y} in GNU Bash.
+;;
+;; You can use ,,foo to escape the quoting.
+;;
+;;   (let ((files "file1 file2"))
+;;     (shqq (cp -r ,,files "My Files")))
+;;       => "cp -r file1 file2 'My Files'"
+;;
+;; And ,,@foo to splice and escape quoting.
+;;
+;;   (let* ((arglist '("-x 'foo bar' -y baz"))
+;;          (arglist (append arglist '("-z 'qux fux'"))))
+;;     (shqq (command ,,@arglist)))
+;;       => "command -x 'foo bar' -y baz -z 'qux fux'"
+;;
+;; Neat, eh?
+
+
+;;; Code:
+
+;;; We don't use `shell-quote-argument' because it doesn't provide any safety
+;;; guarantees, and this quotes shell keywords as well.
+(defun shqq--quote-string (string)
+  (concat "'" (replace-regexp-in-string "'" "'\\\\''" string) "'"))
+
+(defun shqq--atom-to-string (atom)
+  (cond
+   ((symbolp atom) (symbol-name atom))
+   ((stringp atom) atom)
+   ((numberp atom) (number-to-string atom))
+   (t (error "Bad shqq atom: %S" atom))))
+
+(defun shqq--quote-atom (atom)
+  (shqq--quote-string (shqq--atom-to-string atom)))
+
+(defun shqq--match-comma (form)
+  "Matches FORM against ,foo i.e. (\, foo) and returns foo.
+Returns nil if FORM didn't match.  You can't disambiguate between
+FORM matching ,nil and not matching."
+  (if (and (consp form)
+           (eq '\, (car form))
+           (consp (cdr form))
+           (null (cddr form)))
+      (cadr form)))
+
+(defun shqq--match-comma2 (form)
+  "Matches FORM against ,,foo i.e. (\, (\, foo)) and returns foo.
+Returns nil if FORM didn't match.  You can't disambiguate between
+FORM matching ,,nil and not matching."
+  (if (and (consp form)
+           (eq '\, (car form))
+           (consp (cdr form))
+           (null (cddr form)))
+      (shqq--match-comma (cadr form))))
+
+
+(defmacro shqq (parts)
+  "First, PARTS is turned into a list of strings.  For this,
+every element of PARTS must be one of:
+
+- a symbol, evaluating to its name,
+
+- a string, evaluating to itself,
+
+- a number, evaluating to its decimal representation,
+
+- \",expr\", where EXPR must evaluate to an atom that will be
+  interpreted according to the previous rules,
+
+- \",@list-expr\", where LIST-EXPR must evaluate to a list whose
+  elements will each be interpreted like the EXPR in an \",EXPR\"
+  form, and spliced into the list of strings,
+
+- \",,expr\", where EXPR is interpreted like in \",expr\",
+
+- or \",,@expr\", where EXPR is interpreted like in \",@expr\".
+
+In the resulting list of strings, all elements except the ones
+resulting from \",,expr\" and \",,@expr\" forms are quoted for
+shell grammar.
+
+Finally, the resulting list of strings is concatenated with
+separating spaces."
+  (let ((parts
+         (mapcar
+          (lambda (part)
+            (cond
+             ((atom part) (shqq--quote-atom part))
+             ;; We use the match-comma helpers because pcase can't match ,foo.
+             (t (pcase part
+                  ;; ,,foo i.e. (, (, foo))
+                  ((pred shqq--match-comma2)
+                   (shqq--match-comma2 part))
+                  ;; ,,@foo i.e. (, (,@ foo))
+                  ((and (pred shqq--match-comma)
+                        (let `,@,form (shqq--match-comma part)))
+                   `(mapconcat #'identity ,form " "))
+                  ;; ,foo
+                  ;; Insert redundant 'and x' to work around debbugs#18554.
+                  ((and x (pred shqq--match-comma))
+                   `(shqq--quote-atom ,(shqq--match-comma part)))
+                  ;; ,@foo
+                  (`,@,form
+                   `(mapconcat #'shqq--quote-atom ,form " "))
+                  (_
+                   (error "Bad shqq part: %S" part))))))
+          parts)))
+    `(mapconcat #'identity (list ,@parts) " ")))
+
+(provide 'shell-quasiquote)
+;;; shell-quasiquote.el ends here
-- 
2.5.0