[WORG] Document in more detail about what maintainers do? (was: [PATCH] ob-clojure.el: Add support for babashka and nbb backend)
Bastien writes: > Replying to emails when we CC you should be enough, but more help is > always welcome, of course. > > Also, please create an account on https://savannah.gnu.org/git/ and > ask to join the Emacs group: https://savannah.gnu.org/git/?group=emacs > > An Emacs maintainer will give you write access so that you can push > commits for ob-clojure.el directly. > > When in doubt, just ask the mailing list :) I think the part about CC is missing in https://orgmode.org/worg/org-maintenance.html#orga0c76fb Should we give more details there? Also, I find it important to take note about worg documentation for built-in babel backends. I did not even know it exist for a long time. WDYT? -- Ihor Radchenko, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Hi Daniel, thanks for volunteering! I added you as the ob-clojure.el maintainer on the main branch (commit 1c7acb427). Replying to emails when we CC you should be enough, but more help is always welcome, of course. Also, please create an account on https://savannah.gnu.org/git/ and ask to join the Emacs group: https://savannah.gnu.org/git/?group=emacs An Emacs maintainer will give you write access so that you can push commits for ob-clojure.el directly. When in doubt, just ask the mailing list :) Cheers, -- Bastien
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Daniel Kraus writes: >> Would you consider taking over the maintainance of ob-clojure.el? > > I think I would. What does this exactly entail? > Should I subscribe to https://updates.orgmode.org/ or something? We have some info in https://orgmode.org/worg/org-maintenance.html#org37c7d5d Ideally, you can subscribe to Org mailing list and participate in the relevant discussions. Most importantly, when people propose new patches and report bugs related to ob-clojure.el. If you time does not permit following the mailing list closely, do note that we may CC you in emails when we need your attention wrt specific discussions. Since you are interested in maintaining a specific programming language backend, it would be best if you can help with confirming bugs. It is hard for the core maintainers to test all the possible supported programming languages. (I don't even have clojure installed on my machine). Finally, and optionally, you can also contribute new features to the maintained file, contribute to the ob-clojure documentation at https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-clojure.html (see WORG repo at https://sr.ht/~bzg/org/), and possibly write tests in tests/lisp/test-ob-clojure.el > Is there a backlog if issues to go through and try to resolve? I recall a couple of proposed patches and bug reports. Let me quickly pull them out of my todo list... https://orgmode.org/list/paxpr08mb6640fa07d8236d848005bb84a3...@paxpr08mb6640.eurprd08.prod.outlook.com https://list.orgmode.org/orgmode/m2lewez8zh.fsf@numbch...@gmail.com/ https://orgmode.org/list/cagtweadodwkm8s217ufxzxw0zpycvo45khgq13lwgyvz6v9...@mail.gmail.com https://orgmode.org/list/m2v9jp40ry@gmail.com -- Ihor Radchenko, Org mode contributor, Learn more about Org mode at https://orgmode.org/. Support Org development at https://liberapay.com/org-mode, or support my work at https://liberapay.com/yantar92
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Hi! Bastien writes: > Applied in main as 764642f5, thanks a lot and sorry for the delay. > > I also added you to https://orgmode.org/worg/contributors.html. Thank you very much :) > Would you consider taking over the maintainance of ob-clojure.el? I think I would. What does this exactly entail? Should I subscribe to https://updates.orgmode.org/ or something? Is there a backlog if issues to go through and try to resolve? Cheers, Daniel
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Hi Christopher, "Christopher M. Miles" writes: > Thanks, Bastien. (I know you sent a patch and I'll review it, no worry!) >> Would you consider taking over the maintainance of ob-clojure.el? > > Thanks for invitation, I already maintained many org-mode related > libraries. Even though clojure is my favourite programming language, But > I'm not familiar with CIDER and other Clojure tools interaction. So > currently I'm not confident to maintain ob-clojure.el. But I think I > might be able to do this in future someday. (If someone already > maintained, I still can contribute) Actually the invitation was for Daniel as I recalled you declined it already (correct me if I'm wrong!). Of course, we can have several maintainers. > BTW, I have a question, If someone maintain the ob-clojure.el, does this > library need to be separated out org-mode repository? No: we have many lisp/*el files with a specific maintainer. These maintainers of Org's cor handle bug fixes against the files they are maintaining and can be cc'ed on the list when discussing features. The burden is general quite low, it's just a guarantee that someone is in charge and can alleviate the workload of core maintainers. All best, -- Bastien
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Bastien writes: > Hi Daniel, > > Daniel Kraus writes: > >> Attached is the patch changed the logic to use a temp file with >> org-babel-eval. > > Applied in main as 764642f5, thanks a lot and sorry for the delay. > > I also added you to https://orgmode.org/worg/contributors.html. > > Would you consider taking over the maintainance of ob-clojure.el? Sorry, replied to wrong thread. Haha -- [ stardiviner ] I try to make every word tell the meaning that I want to express without misunderstanding. Blog: https://stardiviner.github.io/ IRC(libera.chat, freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 signature.asc Description: PGP signature
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Bastien writes: > Hi Daniel, > > Daniel Kraus writes: > >> Attached is the patch changed the logic to use a temp file with >> org-babel-eval. > > Applied in main as 764642f5, thanks a lot and sorry for the delay. > > I also added you to https://orgmode.org/worg/contributors.html. > Thanks, Bastien. > Would you consider taking over the maintainance of ob-clojure.el? Thanks for invitation, I already maintained many org-mode related libraries. Even though clojure is my favourite programming language, But I'm not familiar with CIDER and other Clojure tools interaction. So currently I'm not confident to maintain ob-clojure.el. But I think I might be able to do this in future someday. (If someone already maintained, I still can contribute) BTW, I have a question, If someone maintain the ob-clojure.el, does this library need to be separated out org-mode repository? -- [ stardiviner ] I try to make every word tell the meaning that I want to express without misunderstanding. Blog: https://stardiviner.github.io/ IRC(libera.chat, freenode): stardiviner, Matrix: stardiviner GPG: F09F650D7D674819892591401B5DF1C95AE89AC3 signature.asc Description: PGP signature
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Hi Daniel, Daniel Kraus writes: > Attached is the patch changed the logic to use a temp file with > org-babel-eval. Applied in main as 764642f5, thanks a lot and sorry for the delay. I also added you to https://orgmode.org/worg/contributors.html. Would you consider taking over the maintainance of ob-clojure.el? -- Bastien
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Daniel Kraus writes: > just wanted to bump this thread and ask if I can do anything > to move this forward? > I'm using it since a few month and works for me. Sorry for the late reply. I am not familiar at all with clojure, so it is hard for me to test the patch. It would help if you also provide the tests in testing/lisp/test-ob-clojure.el to check the added functionality. Best, Ihor
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
On 31/01/2022 14:58, Daniel Kraus wrote: just wanted to bump this thread and ask if I can do anything to move this forward? Your patch is tracked at https://updates.orgmode.org/ , so do not worry too much (look at the list of pending patches). Bunch of commits may happen before next major release.
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Hi, just wanted to bump this thread and ask if I can do anything to move this forward? I'm using it since a few month and works for me. Thanks, Daniel Daniel Kraus writes: > Thanks, now I got it :) > > Attached is the patch changed the logic to use a temp file with > org-babel-eval. > Somehow it doesn't feel too great to create unnecessary temp files > but I looked how other babel backends do it and e.g. ob-js and ob-haskell > use the same logic. >> > From cc9a24fcc42756cc76d59697bddc94a4ee2c475d Mon Sep 17 00:00:00 2001 > From: Daniel Kraus > Date: Sat, 13 Nov 2021 22:51:56 +0100 > Subject: [PATCH] ob-clojure.el: Add support for babashka and nbb backend > > * lisp/ob-clojure.el: Add support for babashka and nbb backend. > --- > [...]
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Daniel Kraus writes: > I filled out the copyright assignment and waiting for them. > I'll mail again when it's done. Just want to mention that I finally received my signed of the copyright agreement. so there is no blocker from this site in case it doesn't count as tinychange. Thanks, Daniel
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Max Nikulin writes: > Thank you for contribution. I do not have strong objection any more. I am not > familiar with babel internals, so I leave further discussion to maintainers. > > If you have not signed copyright assignment yet, likely you should do it to > proceed (I am unsure concerning precise rules concerning line counting). See > https://orgmode.org/contribute.html and > https://orgmode.org/worg/org-contribute.html for details. Thank you very much for your insights. I filled out the copyright assignment and waiting for them. I'll mail again when it's done. (Maybe this is even small enough to count as tinychange if the assignment should take a long time). Cheers, Daniel
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
On 15/11/2021 23:05, Daniel Kraus wrote: Max Nikulin writes: Attached is the patch changed the logic to use a temp file with org-babel-eval. Thank you for contribution. I do not have strong objection any more. I am not familiar with babel internals, so I leave further discussion to maintainers. If you have not signed copyright assignment yet, likely you should do it to proceed (I am unsure concerning precise rules concerning line counting). See https://orgmode.org/contribute.html and https://orgmode.org/worg/org-contribute.html for details. Somehow it doesn't feel too great to create unnecessary temp files but I looked how other babel backends do it and e.g. ob-js and ob-haskell use the same logic. Code fragment might be huge enough to exceed limit on arguments length, so I think that file is safer. Some interpreters and compilers generates more meaningful errors and stacktraces when act on a file. Another option is to feed content to process standard input. With `call-process' or an related command it should be possible to implement any variant, including raw argument without intermediate shell pass. See info "(elisp) Synchronous Processes" or https://www.gnu.org/software/emacs/manual/html_node/elisp/Synchronous-Processes.html +(with-temp-file script-file + (insert expanded)) +(org-babel-eval + (format "%s %s" bb (org-babel-process-file-name script-file)) + ""))) Since other babel packages use the same approach, I will not argue. By the way, isn't second argument of `org-babel-eval' intended for code that may be executed without a temporary file? Some babel languages support sessions. I have no idea if it is applicable to babashka.
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Max Nikulin writes: > The following source block must not execute echo and touch > > #+begin_src clojure >(str "`echo $HOME`" "`touch /tmp/pwned`") > #+end_src Thanks, now I got it :) Attached is the patch changed the logic to use a temp file with org-babel-eval. Somehow it doesn't feel too great to create unnecessary temp files but I looked how other babel backends do it and e.g. ob-js and ob-haskell use the same logic. Thanks, Daniel >From cc9a24fcc42756cc76d59697bddc94a4ee2c475d Mon Sep 17 00:00:00 2001 From: Daniel Kraus Date: Sat, 13 Nov 2021 22:51:56 +0100 Subject: [PATCH] ob-clojure.el: Add support for babashka and nbb backend * lisp/ob-clojure.el: Add support for babashka and nbb backend. --- lisp/ob-clojure.el | 27 +++ 1 file changed, 27 insertions(+) diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el index 3b995d94c..8548dc86d 100644 --- a/lisp/ob-clojure.el +++ b/lisp/ob-clojure.el @@ -36,6 +36,8 @@ ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode ;; For cider, see https://github.com/clojure-emacs/cider ;; For inf-clojure, see https://github.com/clojure-emacs/cider +;; For babashka, see https://github.com/babashka/babashka +;; For nbb, see https://github.com/babashka/nbb ;; For SLIME, the best way to install these components is by following ;; the directions as set out by Phil Hagelberg (Technomancy) on the @@ -73,6 +75,8 @@ (const :tag "inf-clojure" inf-clojure) (const :tag "cider" cider) (const :tag "slime" slime) + (const :tag "babashka" babashka) + (const :tag "nbb" nbb) (const :tag "Not configured yet" nil))) (defcustom org-babel-clojure-default-ns "user" @@ -80,6 +84,16 @@ :type 'string :group 'org-babel) +(defcustom ob-clojure-babashka-command (executable-find "bb") + "Path to the babashka executable." + :type 'file + :group 'org-babel) + +(defcustom ob-clojure-nbb-command (executable-find "nbb") + "Path to the nbb executable." + :type 'file + :group 'org-babel) + (defun org-babel-expand-body:clojure (body params) "Expand BODY according to PARAMS, return the expanded body." (let* ((vars (org-babel--get-vars params)) @@ -225,6 +239,15 @@ ,(buffer-substring-no-properties (point-min) (point-max))) (cdr (assq :package params) +(defun ob-clojure-eval-with-babashka (bb expanded) + "Evaluate EXPANDED code block using BB (babashka or nbb)." + (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj"))) +(with-temp-file script-file + (insert expanded)) +(org-babel-eval + (format "%s %s" bb (org-babel-process-file-name script-file)) + ""))) + (defun org-babel-execute:clojure (body params) "Execute a block of Clojure code with Babel." (unless org-babel-clojure-backend @@ -236,6 +259,10 @@ (cond ((eq org-babel-clojure-backend 'inf-clojure) (ob-clojure-eval-with-inf-clojure expanded params)) + ((eq org-babel-clojure-backend 'babashka) + (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded)) + ((eq org-babel-clojure-backend 'nbb) + (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded)) ((eq org-babel-clojure-backend 'cider) (ob-clojure-eval-with-cider expanded params)) ((eq org-babel-clojure-backend 'slime) -- 2.33.1
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
On 14/11/2021 23:30, Daniel Kraus wrote: Max Nikulin writes: On 14/11/2021 22:28, Daniel Kraus wrote: +(defun ob-clojure-escape-quotes (str-val) + "Escape quotes for STR-VAL." + (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL)) + +(defun ob-clojure-eval-with-babashka (bb expanded) + "Evaluate EXPANDED code block using BB (babashka or nbb)." + (let ((escaped (ob-clojure-escape-quotes expanded))) +(shell-command-to-string + (concat bb " -e \"" escaped "\"" Does not it an open door for security vulnerabilities? Consider a string somewhere in the code: "`echo arbitrary code execution`". Only outer quotes are escaped. The escaping is not done for security reasons. When I have a babel block like #+BEGIN_SRC clojure (str "foo" "bar") #+END_SRC babashka has to be called with bb -e "(str \"foo\" \"bar\")" Enough shell constructs may be interpreted by shell inside double quotes before result is passed to bb. I mentioned execution of code inside backticks, variable substitutions are mostly undesired as well. I do not think, users should escape "$" inside source blocks just because you chose incomplete escaping of shell specials. The following source block must not execute echo and touch #+begin_src clojure (str "`echo $HOME`" "`touch /tmp/pwned`") #+end_src Shell should not be used to launch any command unless it is really necessary. Arguments should be passed directly to execve(2) system call as an array. Combining them into string to pass through shell interpreter to parse into argument array again is error prone. Unfortunately Emacs API related to execution of external processes is awkward. In this particular case it encourages usage of the unsafe function since there is no convenient helper that accepts binary and *list* of arguments and returns output as a string. So more verbose code is required to invoke bb without intermediate interpretation of content of argument string. In my opinion it is better than using of more reliable and tested function to escape shell specials.
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
Hi! Max Nikulin writes: > On 14/11/2021 22:28, Daniel Kraus wrote: >> +(defun ob-clojure-escape-quotes (str-val) >> + "Escape quotes for STR-VAL." >> + (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL)) >> + >> +(defun ob-clojure-eval-with-babashka (bb expanded) >> + "Evaluate EXPANDED code block using BB (babashka or nbb)." >> + (let ((escaped (ob-clojure-escape-quotes expanded))) >> +(shell-command-to-string >> + (concat bb " -e \"" escaped "\"" > > Does not it an open door for security vulnerabilities? Consider a string > somewhere in the code: "`echo arbitrary code execution`". Only outer quotes > are > escaped. The escaping is not done for security reasons. When I have a babel block like #+BEGIN_SRC clojure (str "foo" "bar") #+END_SRC babashka has to be called with bb -e "(str \"foo\" \"bar\")" etc. Security wise someone should always be careful what he evaluates in an org-babel block. Nobody prevents you from evaluating #+BEGIN_SRC shell sudo rm -rf / #+END_SRC Cheers, Daniel
Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
On 14/11/2021 22:28, Daniel Kraus wrote: * lisp/ob-clojure.el: Add support for babashka and nbb backend. --- +(defun ob-clojure-escape-quotes (str-val) + "Escape quotes for STR-VAL." + (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL)) + +(defun ob-clojure-eval-with-babashka (bb expanded) + "Evaluate EXPANDED code block using BB (babashka or nbb)." + (let ((escaped (ob-clojure-escape-quotes expanded))) +(shell-command-to-string + (concat bb " -e \"" escaped "\"" Does not it an open door for security vulnerabilities? Consider a string somewhere in the code: "`echo arbitrary code execution`". Only outer quotes are escaped.
[PATCH] ob-clojure.el: Add support for babashka and nbb backend
* lisp/ob-clojure.el: Add support for babashka and nbb backend. --- This adds support to ob-clojure for babashka (https://github.com/babashka/babashka) and nbb (node version of babashka). It doesn't use `params` as I'm not really sure what they're used for and if they're important for evaluation. I'm also new to org development and the git email workflow so any feedback to the code or the email patch etc is appreciated. lisp/ob-clojure.el | 28 1 file changed, 28 insertions(+) diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el index 3b995d94c..dc42daa5c 100644 --- a/lisp/ob-clojure.el +++ b/lisp/ob-clojure.el @@ -36,6 +36,8 @@ ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode ;; For cider, see https://github.com/clojure-emacs/cider ;; For inf-clojure, see https://github.com/clojure-emacs/cider +;; For babashka, see https://github.com/babashka/babashka +;; For nbb, see https://github.com/babashka/nbb ;; For SLIME, the best way to install these components is by following ;; the directions as set out by Phil Hagelberg (Technomancy) on the @@ -73,6 +75,8 @@ (const :tag "inf-clojure" inf-clojure) (const :tag "cider" cider) (const :tag "slime" slime) + (const :tag "babashka" babashka) + (const :tag "nbb" nbb) (const :tag "Not configured yet" nil))) (defcustom org-babel-clojure-default-ns "user" @@ -80,6 +84,16 @@ :type 'string :group 'org-babel) +(defcustom ob-clojure-babashka-command (executable-find "bb") + "Path to the babashka executable." + :type 'file + :group 'org-babel) + +(defcustom ob-clojure-nbb-command (executable-find "nbb") + "Path to the nbb executable." + :type 'file + :group 'org-babel) + (defun org-babel-expand-body:clojure (body params) "Expand BODY according to PARAMS, return the expanded body." (let* ((vars (org-babel--get-vars params)) @@ -225,6 +239,16 @@ ,(buffer-substring-no-properties (point-min) (point-max))) (cdr (assq :package params) +(defun ob-clojure-escape-quotes (str-val) + "Escape quotes for STR-VAL." + (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL)) + +(defun ob-clojure-eval-with-babashka (bb expanded) + "Evaluate EXPANDED code block using BB (babashka or nbb)." + (let ((escaped (ob-clojure-escape-quotes expanded))) +(shell-command-to-string + (concat bb " -e \"" escaped "\"" + (defun org-babel-execute:clojure (body params) "Execute a block of Clojure code with Babel." (unless org-babel-clojure-backend @@ -236,6 +260,10 @@ (cond ((eq org-babel-clojure-backend 'inf-clojure) (ob-clojure-eval-with-inf-clojure expanded params)) + ((eq org-babel-clojure-backend 'babashka) + (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded)) + ((eq org-babel-clojure-backend 'nbb) + (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded)) ((eq org-babel-clojure-backend 'cider) (ob-clojure-eval-with-cider expanded params)) ((eq org-babel-clojure-backend 'slime) -- 2.33.1
