From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: Patches for elpa-admin Date: Fri, 15 Apr 2022 07:18:16 +0000 Message-ID: <87czhiddc7.fsf@posteo.net> References: <874k2x8jhb.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38800"; mail-complaints-to="usenet@ciao.gmane.io" Cc: ELPA Maintainers To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Apr 15 09:19:36 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nfGEy-0009va-E9 for ged-emacs-devel@m.gmane-mx.org; Fri, 15 Apr 2022 09:19:36 +0200 Original-Received: from localhost ([::1]:49900 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nfGEx-0004OW-3w for ged-emacs-devel@m.gmane-mx.org; Fri, 15 Apr 2022 03:19:35 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:59878) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nfGDr-0003h6-Fa for emacs-devel@gnu.org; Fri, 15 Apr 2022 03:18:28 -0400 Original-Received: from mout02.posteo.de ([185.67.36.66]:50649) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nfGDo-0006LD-P9 for emacs-devel@gnu.org; Fri, 15 Apr 2022 03:18:27 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id D0DBD240107 for ; Fri, 15 Apr 2022 09:18:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1650007102; bh=LiZfjp2iGi8LkeW8jbHadNPJvhafez2Ctgi83FkChRw=; h=From:To:Cc:Subject:Autocrypt:Date:From; b=r+0ePMzX0r1YvXsmdJAsZVoc2KbOZiZQf8eTJ+7+1nbgGsgisRBz1OHit+FBpNYHC ihpbcDCedXTw/dDDKVMKSpXgZGgcM+SNU8pcxr53IDXvKqOGlqgHKjPdMk90RE1uG/ 0VMmhFuGS3BDgFsIrWIo3HhH0nYHyNVz6jby+crbsNHrW0x+oOf7P2Puxn38lxZ6AW mD3RaVNRT31CZN2svn+7ZouZHfDNTHqGyzGu/MH+HMrm0AGMZ24nC7TmY1Qv8cEedl CcKb2+cbbTwWK+f1HbXOQDNuBj6204PGTKBEisFhKbVu1viTWbiX1xKmWL2sxaRf21 /l1sK71/c8bVg== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Kfnjn5D3bz9rxD; Fri, 15 Apr 2022 09:18:21 +0200 (CEST) Autocrypt: addr=philipk@posteo.net; prefer-encrypt=nopreference; keydata= mDMEYHHqUhYJKwYBBAHaRw8BAQdAp3GdmYJ6tm5McweY6dEvIYIiry+Oz9rU4MH6NHWK0Ee0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiQBBMWCAA4FiEEDM2H44ZoPt9Ms0eHtVrAHPRh1FwFAmBx6lICGwMFCwkIBwIGFQoJ CAsCBBYCAwECHgECF4AACgkQtVrAHPRh1FyTkgEAjlbGPxFchvMbxzAES3r8QLuZgCxeAXunM9gh io0ePtUBALVhh9G6wIoZhl0gUCbQpoN/UJHI08Gm1qDob5zDxnIHuDgEYHHqUhIKKwYBBAGXVQEF AQEHQNcRB+MUimTMqoxxMMUERpOR+Q4b1KgncDZkhrO2ql1tAwEIB4h4BBgWCAAgFiEEDM2H44Zo Pt9Ms0eHtVrAHPRh1FwFAmBx6lICGwwACgkQtVrAHPRh1Fw1JwD/Qo7kvtib8jy7puyWrSv0MeTS g8qIxgoRWJE/KKdkCLEA/jb9b9/g8nnX+UcwHf/4VfKsjExlnND3FrBviXUW6NcB In-Reply-To: (Stefan Monnier's message of "Fri, 15 Apr 2022 00:01:22 -0400") Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:288426 Archived-At: Stefan Monnier writes: >> The somewhat recent addition to render markdown caused an error when >> building a package locally on my system, as the executable "markdown" >> was not installed. So I gathered a number of implementations and >> had had `elpaa--section-to-html' use whatever is installed: > > I wouldn't work that hard at it: I think the only important part of the > behavior is either "reproduces faithfully what will happen on > elpa.gnu.org" or "doesn't fail stupidly", so rather than try out all the > possible alternatives the most important part is to fail gracefully. I guess to that end it should be enough to use cat if markdown isn't found. >> +(defvar elpaa--markdown-candidates >> + '("pandoc" "cmark" "marked" "discount" ;ideally >> + "lowdown" "hoedown" "sundown" "kramdown" ;backup >> + "markdown.pl" "markdown_py" "md2html.awk" ;fallback >> + "markdown" ;despair >> + )) > > I think this list is "a bit over the top", but feel free to keep it. > OTOH, I think "markdown" should come first since that's what's used on > `elpa.gnu.org`. On that topic, what program is markdown? On Debian there seem to be a number of alternatives: $ apt-file search /usr/bin/markdown | grep /markdown$ discount: /usr/bin/markdown libtext-markdown-perl: /usr/bin/markdown markdown: /usr/bin/markdown Considering that more often than not what we are actually rendering is "GitHub flavoured markdown" (https://github.github.com/gfm/), it might make sense to use an implementation that takes this into consideration. >> - (elpaa--call-sandboxed t "markdown" input-filename)) >> + (elpaa--call-sandboxed t (elpa--markdown-executable) input-filename)) > > I think this is more than 80 columns, please wrap. Will do. >> +(cl-defmethod elpaa--sandbox-args ((_mechaism (eql bwrap)) args) >> + (let ((dd (expand-file-name default-directory))) ;No `~' allowed! >> + (setq args (cl-list* "--bind" dd dd args))) >> + ;; Add read-only dirs in reverse order. >> + (dolist (b (append elpaa--sandbox-ro-binds elpaa--sandbox-extra-ro-dirs)) >> + (when (file-exists-p b) ;`brwap' burps on binds that don't exist! >> + (setq b (expand-file-name b)) >> + (setq args (cl-list* "--ro-bind" b b args)))) >> + (append (list "bwrap") elpaa--bwrap-args args)) >> + >> +(cl-defmethod elpaa--sandbox-args ((_mechaism (eql guix)) args) >> + ;; Indicate the remaining arguments are the command to be executed. >> + (append (list "guix") elpaa--guix-args (cons "--" args))) > > There's a typo "mechaism". > > More importantly, I understand that `elpaa--sandbox-ro-binds` doesn't > need to be passed to `guix shell`, but I don't understand why > `elpaa--sandbox-extra-ro-dirs` would not be needed either. I think that this is a mistake on my end. >> (defun elpa--markdown-executable () >> (catch 'exists >> + (when (eq elpaa--sandbox-mechanism 'guix) >> + ;; When using Guix, we can ensure what markdown implementation >> + ;; we want to use, so we just return a fixed one here. >> + (throw 'exists "cmark")) >> (dolist (cmd elpaa--markdown-candidates) >> (when (executable-find cmd) >> (throw 'exists cmd))) > > I think this optimization is not worth its cost. The reason I had to add this was that if using Guix, I cannot rely on `elpa--markdown-executable' to find what is installed, since that is evaluated on the host system. In the end, the entire markdown patch isn't really necessary, if one can make use of Guix. All that is needed is to change the markdown package from "cmark" to "markdown" (which would use Gruber's implementation). >> This approach could also be extended to Nix/nix-shell, but I have no >> experience with that tool. > > To be honest, I'm not thrilled about adding support for more container > systems to `elpa-admin.el` because it's not its focus. The containers > are important on `elpa.gnu.org` because I'm a bit paranoid about running > arbitrary software on a central server that could conceivably be > a target for an attack. But for most other uses it seems that setting > `elpaa--sandbox` to nil will do the job just fine if you don't want to > install `bwrap`. It is not only that, it is also the dependencies that someone might or might not have installed when wanting to contribute to ELPA. By using Guix, the right tools are automatically provided, which could also be extended for non-sandbox elpa--call invocations. The error messages generated if something goes wrong (e.g. missing makeinfo/install-info, imagemagick), can be hard to parse, and this approach just circumvents the issue entirely. > More interesting would be to add support for this kind of sandboxing in > Emacs itself (so ELisp's Flymake support can use it); and some years > from now `elpa-admin.el` will then be able to ditch its own sandboxing. I have actually been looking into implementing a TRAMP backend that would tunnel all communication through a Guix shell. I think in that case Flymake should be able to make use of that. >> (On a related note, is it necessary to sandbox markdown rendering? > > It's necessary if (like me) you don't want to try and convince yourself > that it's currently unnecessary nor want to depend on that fact > remaining true in the future. > >> I understand why org can be dangerous, but markdown shouldn't be able >> to have any side effects?) > > Yes, and 640kB should be enough for everyone. Ok, I see. >> If these changes are fine, I can push them myself. > > Feel free, I will wait a bit to implement the changes i mentioned. -- Philip Kaludercic