Re: [BUG] ob-shell may use /bin/sh instead of the specified shell when :cmdline is provided (was: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline)

2024-06-27 Thread Ihor Radchenko
Ihor Radchenko  writes:

> Reproducer:
> ...

Fixed, on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b64dbd838

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



[BUG] ob-shell may use /bin/sh instead of the specified shell when :cmdline is provided (was: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline)

2024-06-27 Thread Ihor Radchenko
Ihor Radchenko  writes:

> As for the problem with   
> when  does not contain shebang, we can trivially at that
> shebang like
>
> (with-temp-file script-file
>   (if shebang (insert shebang "\n")
>   (insert "#!" shell-file-name "\n"))
>   (when padline (insert "\n"))
>   (insert body))

I am branching off this bug into a new thread.

Reproducer:

#+begin_src dash
  echo This must be empty in dash: "$RANDOM"
#+end_src

#+RESULTS:
: This must be empty in dash:



#+begin_src dash :cmdline 1 2 3
  echo This must be empty in dash: "$RANDOM"
#+end_src

#+RESULTS:
: This must be empty in dash: 25561

The second code block uses shell as evidenced by $RANDOM variable being
initialized. This only happens when :cmdline header argument is
provided.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-06-27 Thread Ihor Radchenko
Ihor Radchenko  writes:

> I am looking at this again, and notice that the problem with
> `shell-command-switch' is tangent to the original bug report we are
> discussing.
> ...
> I conclude that your concern, while being valid, is a _different_ bug.
> Thus, I do not see it as a blocker for my patch - my patch will fix the
> *original bug reported on top of this thread*.

Applied.
Fixed, on main.
This is the original bug report and reproducer that is fixed.
Other bugs discussed in this thread have to be handled separately.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1c7b3ed26

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-28 Thread Ihor Radchenko
Max Nikulin  writes:

>> I conclude that your concern, while being valid, is a _different_ bug.
>> Thus, I do not see it as a blocker for my patch - my patch will fix the
>> *original bug reported on top of this thread*.
>
> My concern is that your patch trying to fix one bug (I am not convinced 
> it is an improvement despite it is a step toward consistency) introduces 
> another one that is not currently present in the code.

May you explain which bug you are referring to?

The bug with bash -c ./script-with-no-shebang described in
https://superuser.com/questions/502984/writing-shell-scripts-that-will-run-on-any-shell-using-multiple-shebang-lines
is already present, and my patch does not change anything about it.

>> (with-temp-file script-file
>>  (if shebang (insert shebang "\n")
>>(insert "#!" shell-file-name "\n"))
>>  (when padline (insert "\n"))
>>  (insert body))
>
> This code has an issue. Interpretation of relative file names in 
> shebangs varies across shells.

May you elaborate? Are you concerned that `shell-file-name' may not be
an absolute path?

> If you insist on parsing :cmdline by shell (I do not like it) then you 
> may try
>
>   ' 
>  '

May you explain how it is better?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-27 Thread Max Nikulin

On 27/04/2024 17:31, Max Nikulin wrote:

On 26/04/2024 18:49, Ihor Radchenko wrote:



+    shell-file-name

...

+    (list shell-command-switch
+  (concat (file-local-name 
script-file)  " " cmdline



Max Nikulin writes:

Using `shell-command-switch' unconditionally may lead to executing
/bin/sh instead of shell specified by `shell-file-name' for script 
files

having no shebang, see

https://superuser.com/questions/502984/writing-shell-scripts-that-will-run-on-any-shell-using-multiple-shebang-lines


I conclude that your concern, while being valid, is a _different_ bug.
Thus, I do not see it as a blocker for my patch - my patch will fix the
*original bug reported on top of this thread*.


My concern is that your patch trying to fix one bug (I am not convinced 
it is an improvement despite it is a step toward consistency) introduces 
another one that is not currently present in the code.


I read current variant of code once more. -c is unconditionally added, 
so your variant just makes the bug apparent. I still do not like behavior of


:cmdline 1 ; touch /tmp/not-an-arg

as I stated in

Re: [DISCUSSION] The meaning of :cmdline header argument across babel 
backends. Sat, 27 Apr 2024 17:53:25 +0700.

https://list.orgmode.org/v0ilf6$34l$1...@ciao.gmane.io




Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-27 Thread Max Nikulin

On 26/04/2024 18:49, Ihor Radchenko wrote:



+shell-file-name

...

+(list shell-command-switch
+  (concat (file-local-name script-file)  " " 
cmdline



Max Nikulin writes:

Using `shell-command-switch' unconditionally may lead to executing
/bin/sh instead of shell specified by `shell-file-name' for script files
having no shebang, see

https://superuser.com/questions/502984/writing-shell-scripts-that-will-run-on-any-shell-using-multiple-shebang-lines


I conclude that your concern, while being valid, is a _different_ bug.
Thus, I do not see it as a blocker for my patch - my patch will fix the
*original bug reported on top of this thread*.


My concern is that your patch trying to fix one bug (I am not convinced 
it is an improvement despite it is a step toward consistency) introduces 
another one that is not currently present in the code.



(with-temp-file script-file
(if shebang (insert shebang "\n")
   (insert "#!" shell-file-name "\n"))
(when padline (insert "\n"))
(insert body))


This code has an issue. Interpretation of relative file names in 
shebangs varies across shells.


If you insist on parsing :cmdline by shell (I do not like it) then you 
may try


  ' 
 '






Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-27 Thread Ihor Radchenko
Matt  writes:

> There's a lot to process in this bug.  We've also uncovered, at least, 
> related four bugs.  I think it would help to submit separate bug reports for 
> each of the related issues so that we may discuss them separately, if 
> possible.  Is Woof! the way to do that?  I recall there being some issues 
> with Woof!.  Is there something not in https://tracker.orgmode.org/howto that 
> I should know?

You can just send emails with [BUG] in them, as usual. Or change email
subject to branch out a new thread, as I did for :cmdline argument discussion.

What you need to know about Woof! now is that it is not working. We
need to wait for Bastien to investigate. (Bastien does not have much
time though)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-27 Thread Matt
Thank you both for your thoughtful replies.  

There's a lot to process in this bug.  We've also uncovered, at least, related 
four bugs.  I think it would help to submit separate bug reports for each of 
the related issues so that we may discuss them separately, if possible.  Is 
Woof! the way to do that?  I recall there being some issues with Woof!.  Is 
there something not in https://tracker.orgmode.org/howto that I should know?

--
Matt Trzcinski
Emacs Org contributor (ob-shell)
Learn more about Org mode at https://orgmode.org
Support Org development at https://liberapay.com/org-mode





Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-26 Thread Ihor Radchenko
Max Nikulin  writes:

> Actually script arguments (and :stdin) might be applied to python and at 
> least some other languages, so support of this feature should be moved 
> from ob-shell to common org-babel code.
>
> My point:
> - header arguments should have as close as possible meaning across 
> various languages.
> - withing ob-shell variation of ways to execute a script should be 
> minimized either some parameters (:cmdline, :shebang, :stdin) are 
> specified or not.

+1
ob-eval is a good candidate to implement such unified handling.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



[DISCUSSION] The meaning of :cmdline header argument across babel backends (was: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline)

2024-04-26 Thread Ihor Radchenko
Max Nikulin  writes:

> However looking wider, I do not like that :cmdline for ob-shell has 
> different meaning than for other languages, see e.g. ob-sql. Only for 
> shell this parameter is treated as arguments of a *script*. In other 
> cases :cmdline is used to specify arguments of *interpreter* and I think 
> ob-shell should follow this convention.

Alas, we already have the current state of affairs documented in
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html#orge70bc7b

So, no breaking changes.

And shell scripts are not like SQL queries - they often do need to check
arguments. So, the current behaviour is justified, IMHO.

For some languages, only switches to the interpreter make sense (ditaa,
lilypond, plantuml). For others, (bash, python, C++, etc), we may want
to pass switches to the script itself more often.

What might be done is introducing _two_ different header arguments - one
for interpreter switches, and another for script/program switches.

Say, :interpreter-cmdline and :script-cmdline.
Then, we can call the current :cmdline behaviour "dwim" and allow users
to be more explicit if necessary.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-26 Thread Ihor Radchenko
Ihor Radchenko  writes:

> Max Nikulin  writes:
>
>>> +shell-file-name
>> ...
>>> +(list shell-command-switch
>>> +  (concat (file-local-name script-file)  " " 
>>> cmdline
>>
>> Using `shell-command-switch' unconditionally may lead to executing 
>> /bin/sh instead of shell specified by `shell-file-name' for script files 
>> having no shebang, see
>>
>> https://superuser.com/questions/502984/writing-shell-scripts-that-will-run-on-any-shell-using-multiple-shebang-lines
>
> Good point.

I am looking at this again, and notice that the problem with
`shell-command-switch' is tangent to the original bug report we are
discussing.

Currently, ob-shell uses two ways to execute the source block:

1. When :shebang header argument is provided,
''

2. When :shebang is not provided
 ' '

The problem you linked to only manifests for calling script files
_without_ shebang, while the original bug report is about the case when
:shebang is given.

I conclude that your concern, while being valid, is a _different_ bug.
Thus, I do not see it as a blocker for my patch - my patch will fix the
*original bug reported on top of this thread*.

As for the problem with   
when  does not contain shebang, we can trivially at that
shebang like

(with-temp-file script-file
(if shebang (insert shebang "\n")
  (insert "#!" shell-file-name "\n"))
(when padline (insert "\n"))
(insert body))

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-25 Thread Max Nikulin

On 24/04/2024 19:52, Ihor Radchenko wrote:

Max Nikulin writes:

I believe, multiple arguments should be specified as '(1 a "b c").


Yes, but we do not, in general, know how to split them.


Something should be changed anyway since current behavior is 
inconsistent and so is buggy.


The only difference of script arguments from :var is that just a string 
should be converted to a list having single value. It should be possible 
to specify list of script argument as a reference to a named element 
similar to


#+name: shvar
#+header: :var a='(1 abc "def ghi") :results verbatim
#+begin_src bash
  printf '%s\n' "${a[@]}"
#+end_src

#+name: varval
- 1
- bcd
- list items

#+call: shvar(a=varval)


With shebang (as header arg or as part of the body) command should be
  /path/to/script [ARGUMENT]...
when there is no shebang
  /shell/executable /path/to/script [ARGUMENT]...


Maybe instead of `process-file' we can simply use `shell-command'?


Doesn't `shell-command` call `process-file` with `shell-file-name` and 
`shell-command-switch' under the hood like in your patch?






Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-24 Thread Ihor Radchenko
Max Nikulin  writes:

>> +shell-file-name
> ...
>> +(list shell-command-switch
>> +  (concat (file-local-name script-file)  " " 
>> cmdline
>
> Using `shell-command-switch' unconditionally may lead to executing 
> /bin/sh instead of shell specified by `shell-file-name' for script files 
> having no shebang, see
>
> https://superuser.com/questions/502984/writing-shell-scripts-that-will-run-on-any-shell-using-multiple-shebang-lines

Good point.

> I believe, multiple arguments should be specified as '(1 a "b c").

Yes, but we do not, in general, know how to split them.

> With shebang (as header arg or as part of the body) command should be
>  /path/to/script [ARGUMENT]...
> when there is no shebang
>  /shell/executable /path/to/script [ARGUMENT]...

Maybe instead of `process-file' we can simply use `shell-command'?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-24 Thread Max Nikulin

On 23/04/2024 17:28, Ihor Radchenko wrote:


I propose the attached patch.



+++ b/lisp/ob-shell.el
@@ -322,14 +322,12 @@ (defun org-babel-sh-evaluate (session body  
params stdin cmdline)
  (with-temp-buffer
 (with-connection-local-variables
  (apply #'process-file
-(if shebang (file-local-name script-file)
-  shell-file-name)
+shell-file-name
stdin-file
 (current-buffer)
 nil
-(if shebang (when cmdline (list cmdline))
-  (list shell-command-switch
-(concat (file-local-name script-file)  " " 
cmdline)
+(list shell-command-switch
+  (concat (file-local-name script-file)  " " 
cmdline


Using `shell-command-switch' unconditionally may lead to executing 
/bin/sh instead of shell specified by `shell-file-name' for script files 
having no shebang, see


https://superuser.com/questions/502984/writing-shell-scripts-that-will-run-on-any-shell-using-multiple-shebang-lines

The kernel refuses to execute such scripts and returns ENOEXEC, so the
exact behavior depends on the program you run such a script /from/.

- bash 4.2.39 -- uses itself
- busybox-ash 1.20.2 -- uses itself
- dash 0.5.7 -- runs /bin/sh
- fish 1.23.1 -- complains about ENOEXEC, then blames the wrong file
- AT ksh 93u+2012.08.01 -- uses itself
- mksh R40f -- runs /bin/sh
- pdksh 5.2.14 -- runs /bin/sh
- sh-heirloom 050706 -- uses itself
- tcsh 6.18.01 -- runs /bin/sh
- zsh 5.0.0 -- runs /bin/sh
- cmd.exe 5.1.2600 -- looks at you funny

I am not going to spend time testing current versions.

I believe, multiple arguments should be specified as '(1 a "b c").

With shebang (as header arg or as part of the body) command should be
/path/to/script [ARGUMENT]...
when there is no shebang
/shell/executable /path/to/script [ARGUMENT]...




Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-23 Thread Max Nikulin

On 23/04/2024 17:51, Max Nikulin wrote:
I am in favor of dropping `shell-command-switch' in the latter case to 
pass arguments literally in both cases.


Dropping "-c" may have side effects. Instead of :shebang, a source block 
may have shebang in the body


#+begin_src bash
#!/bin/bash -e
echo first; false; echo second
#+end_src

This shebang is ignored if the script is executed as
bash /tmp/script
and respected in the case of
bash -c /tmp/script
Shebang in the script body may be detected to run it as
/tmp/script
or
/bin/bash -e /tmp/script

To avoid interpretation of shell specials in script arguments when "-c" 
is used, it is possible to use a trick

bash -c /tmp/script ob-shell arg1 arg2 arg3
The -c option adds extra execve() call in comparison to
/tmp/script arg1 arg2 arg3
Perhaps it may be neglected.

It may be more tricky on Windows where shebangs are likely ignored even 
by bash. However I do not thing ob-shell is working on windows since 
`shell-command-switch' should be /c there instead of -c since default 
shell is cmd.exe.





Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-23 Thread Max Nikulin

On 21/04/2024 22:09, Matt wrote:

The proposed solution assumes we intend to parse the characters
following :cmdline as space delimited and grouped by quotes.  However,
AFAICT, the parsing issue makes this solution ambiguous.


Matt, I am sorry, but I do not agree with your proposal. I do not think 
that `split-string-and-unquote' will solve all issues.


Certainly issues with formatting of output should be treated separately.

I figured out there is at least one more issue. Consider

#+property: header-args:bash :results verbatim
#+begin_src bash :cmdline $LANG :shebang #!/bin/bash
  printf '"%s" ' "$0" "$@"
  printf '\n'
  tr '\000' '\n' First line is argv as it is represented for a script, next lines are 
exec arguments at lower level (actual executable may be obtained from

"readlink /proc/$$/exe")

Notice that in the former case "$LANG" is passed literally, but in the 
latter it is expanded. I am in favor of dropping `shell-command-switch' 
in the latter case to pass arguments literally in both cases.


I think, it would be more consistent with :var to specify multiple 
arguments using elisp lists

#+header: :cmdline '("first 1" "second 2")

However looking wider, I do not like that :cmdline for ob-shell has 
different meaning than for other languages, see e.g. ob-sql. Only for 
shell this parameter is treated as arguments of a *script*. In other 
cases :cmdline is used to specify arguments of *interpreter* and I think 
ob-shell should follow this convention.


Actually script arguments (and :stdin) might be applied to python and at 
least some other languages, so support of this feature should be moved 
from ob-shell to common org-babel code.


My point:
- header arguments should have as close as possible meaning across 
various languages.
- withing ob-shell variation of ways to execute a script should be 
minimized either some parameters (:cmdline, :shebang, :stdin) are 
specified or not.


Finally a note on tests

+(ert-deftest test-cmdline-alone-and-with-shebang-have-same-result ()
+  "Pass arguments to a block.  Don't use shebang.  Then use
+shebang set to the same language as the block.  The result should
+be the same."
+  (should (equal
+   (org-test-with-temp-text
+   "#+begin_src bash :cmdline 1 2 3
+echo \"$1\"
+
+#+end_src"
+ (org-babel-execute-src-block))
+   (org-test-with-temp-text
+   "#+begin_src bash :cmdline 1 2 3 :shebang #!/usr/bin/env bash
+echo \"$1\"
+
+#+end_src"
+ (org-babel-execute-src-block)
I believe, that starting point of the discussion is that the results 
should be same and decision what is more correct is the result of the 
discussion. Unit tests should check both cases independently and should 
fix particular treatment of arguments.





Re: [PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-23 Thread Ihor Radchenko
Matt  writes:

> Whether this is a solution, in part, depends on the perennial problem of 
> shell blocks: knowing what's wrong means knowing what's right.
>
> The proposed solution assumes we intend to parse the characters following 
> :cmdline as space delimited and grouped by quotes.  However, AFAICT, the 
> parsing issue makes this solution ambiguous.
>
> Thoughts?

Manually parsing the shell arguments is calling for trouble.
Especially when the arguments involve shell-specific escapes like
:cmdline 1\ 2\ 3

Since escape characters may vary from shell to shell, it is not a good
idea to parse the arguments on Elisp side. We should better leave this
job to the shell.

I propose the attached patch.

>From e0cf4161b4af05c513ba402ee9625851853c9465 Mon Sep 17 00:00:00 2001
Message-ID: 
From: Ihor Radchenko 
Date: Tue, 23 Apr 2024 13:22:22 +0300
Subject: [PATCH] ob-shell: Pass :cmdline arguments consistently regardless of
 :shebang

* lisp/ob-shell.el (org-babel-sh-evaluate): When invoking script file
generated from the code block, consistently use
 -c   command line, even when
:shebang is header argument is provided.  The previous approach with
  call caused differences in how shell
parsed the provided command line arguments.

Reported-by: Max Nikulin 
Link: https://orgmode.org/list/18f01342a2f.124ad27612732529.8693431365849276...@excalamus.com
---
 lisp/ob-shell.el | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
index 35d9e9376..30b3ea322 100644
--- a/lisp/ob-shell.el
+++ b/lisp/ob-shell.el
@@ -322,14 +322,12 @@ (defun org-babel-sh-evaluate (session body  params stdin cmdline)
 	  (with-temp-buffer
 (with-connection-local-variables
  (apply #'process-file
-(if shebang (file-local-name script-file)
-  shell-file-name)
+shell-file-name
 		stdin-file
 (current-buffer)
 nil
-(if shebang (when cmdline (list cmdline))
-  (list shell-command-switch
-(concat (file-local-name script-file)  " " cmdline)
+(list shell-command-switch
+  (concat (file-local-name script-file)  " " cmdline
 		(buffer-string
 	   (session			; session evaluation
 (if async
-- 
2.44.0


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 


[PATCH] Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2024-04-21 Thread Matt

  On Sat, 18 Nov 2023 16:54:39 +0100  Max Nikulin  wrote --- 
 > Hi,
 > 
 > Trying to figure out the origin of the confusion with
 > "bash -c bash /path/to/file-containing-the-source-code.sh"
 > I have faced an inconsistency with :cmdline treatment in ob-shell.el. I 
 > expect same results in the following cases:
 > 
 > #+begin_src bash :cmdline 1 2 3
 >printf "%s\n" "$1"
 > #+end_src
 > 
 > #+RESULTS:
 > : 1
 > 
 > #+begin_src bash :cmdline 1 2 3 :shebang #!/bin/bash
 >printf "%s\n" "$1"
 > #+end_src
 > 
 > #+RESULTS:
 > : 1 2 3
 > 
 > Emacs-28, Org is the current git HEAD.

AFAIU, the inconsistency is due to how the characters following :cmdline are 
interpreted when the subprocess call is made.

Consider the following, when only :cmdline is used:

# Evaluates like:
#
# bash -c "./sh-script-8GJzdG 1 2 3"
#
#+begin_src bash :cmdline 1 2 3
echo \"$1\"
#+end_src

#+RESULTS:
: 1

# Evaluates like:
#
# bash -c "./sh-script-8GJzdG \"1 2\" 3"
#
#+begin_src bash :cmdline "1 2" 3
echo \"$1\"
#+end_src

#+RESULTS:
: 1 2

For :cmdline alone, the characters following :cmdline are passed as though each 
is quoted.  That is, separate arguments are delimited by one or more spaces.  
The first example is equivalent to the following:

# Evaluates like:
#
# bash -c "./sh-script-8GJzdG \"1\" \"2\" \"3\""
#
#+begin_src bash :cmdline 1 2 3
echo \"$1\"
#+end_src

#+RESULTS:
: 1

How would you expect :cmdline "1 2 3" to be evaluated?

#+begin_src bash :cmdline "1 2 3"
echo \"$1\"
#+end_src

My expectation would be that it evaluates like:

  bash -c "./sh-script-8GJzdG \"1 2 3\""

It turns out, however, that it's evaluated exactly like :cmdline 1 2 3, or 
:cmdline "1" "2" "3".  The result is "1".

To make the block evaluate as expected requires an extra set of parentheses:

# Evaluates like:
#
# bash -c "./sh-script-8GJzdG \"1 2 3\""
#
#+begin_src bash :cmdline "\"1 2 3\""
echo \"$1\"
#+end_src

#+RESULTS:
: 1 2 3

This, however, appears to be separate from the reported issue[fn:1].

Now, consider :cmdline paired with :shebang, called with the same values as 
above.

# Evaluates like:
#
# /tmp/babel-Xd6rGS/sh-script-61jvMa "1 2 3"
#
#+begin_src bash :cmdline 1 2 3 :shebang #!/usr/bin/env bash
echo \"$1\"
#+end_src

#+RESULTS:
: 1 2 3

# Evaluates like:
#
# /tmp/babel-Xd6rGS/sh-script-61jvMa "\"1 2\" 3"
#
#+begin_src bash :cmdline "1 2" 3 :shebang #!/usr/bin/env bash
echo \"$1\"
#+end_src

#+RESULTS:
: 1 2" 3"

# Evaluates like:
#
# /tmp/babel-Xd6rGS/sh-script-61jvMa "1 2 3"
#
#+begin_src bash :cmdline "1 2 3" :shebang #!/usr/bin/env bash
echo \"$1\"
#+end_src

#+RESULTS:
: 1 2 3

# Evaluates like:
#
# /tmp/babel-Xd6rGS/sh-script-61jvMa "\"1 2 3\""
#
#+begin_src bash :cmdline "\"1 2 3\"" :shebang #!/usr/bin/env bash
echo \"$1\"
#+end_src

#+RESULTS:
: 1 2 3""

# Evaluates like:
#
# /tmp/babel-Xd6rGS/sh-script-61jvMa "\"1\" \"2\" \"3\""
#
#+begin_src bash :cmdline "1" "2" "3" :shebang #!/usr/bin/env bash
echo \"$1\"
#+end_src

#+RESULTS:
: 1" "2" "3""

An immediate observation is that the output results don't format correctly.  If 
you change the results type to "raw", however, you'll see that the Org results 
match those from a terminal, like xfce4-terminal.  The fact that raw output 
matches output from the terminal means that the formatting issue is (also) 
separate from the bug we're trying to fix.  That is, the bug we're trying to 
fix occurs in how the subprocess call is made, not in how the result is 
formatted.

In ob-shell, the subprocess call is made with 'process-file'.  Arguments are 
determined casewise:

1. shebang+cmdline
2. cmdline

The characters following :cmdline are received by the 'cmdline' argument to 
'org-babel-sh-evaluate' as a string.  Both cases put this string into a list 
for the ARGS of 'process-file':

| header   | 'org-babel-sh-evaluate' | process-file ARGS |
|  | cmdline variable value  | shebang+cmdline   |
|--+-+---|
| :cmdline 1 2 3   | "1 2 3" | ("1 2 3") |
| :cmdline "1 2" 3"| "\"1 2\" 3" | ("\"1 2\" 3") |
| :cmdline "1" "2" "3" | "\"1\" \"2\" \"3\"" | ("\"1\" \"2\" \"3\"") |

| header   | 'org-babel-sh-evaluate' | process-file ARGS |
|  | cmdline variable value  | cmdline   |
|--+-+---|
| :cmdline 1 2 3   | "1 2 3" | ("1 2 3") |
| :cmdline "1 2" 3"| "\"1 2\" 3" | ("\"1 2\" 3") |
| :cmdline "1" "2" "3" | "\"1\" \"2\" \"3\"" | ("\"1\" \"2\" \"3\"") |

Notice that the ARGS passed to 'process-file' are the same for both cases.  The 
problem is that the "block equivalent shell calls" are *not* the same.  If we 
arrange the equivalent shell calls from the blocks given above into a table, we 
see that the forms are 

Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2023-12-04 Thread Matt


  On Mon, 04 Dec 2023 14:55:58 +0100  Ihor Radchenko  wrote --- 
 > Matt m...@excalamus.com> writes:
 > 
 > >  > #+begin_src bash :cmdline 1 2 3
 > >  >printf "%s\n" "$1"
 > >  > #+end_src
 > >  > 
 > >  > #+RESULTS:
 > >  > : 1
 > >  > 
 > >  > #+begin_src bash :cmdline 1 2 3 :shebang #!/bin/bash
 > >  >printf "%s\n" "$1"
 > >  > #+end_src
 > >  > 
 > >  > #+RESULTS:
 > >  > : 1 2 3
 > >
 > > Thank you!  This makes a good test case.
 > 
 > I am confused. Isn't it a bug?

Yes, it's a bug.  And the test was wrong.

Here is a corrected version.  When the bug is fixed, the following should pass. 
 It currently fails because the first block returns 1 and the second block 
returns 1 2 3 (as seen in the quote).  Both blocks should return 1.

(ert-deftest test-cmdline-alone-and-with-shebang-have-same-result ()
  "Pass arguments to a block.  Don't use shebang.  Then use
shebang set to the same language as the block.  The result should
be the same."
  (should (equal
   (org-test-with-temp-text 
   "#+begin_src bash :cmdline 1 2 3   
echo \"$1\" 
 
#+end_src"  
 (org-babel-execute-src-block)) 
   (org-test-with-temp-text 
   "#+begin_src bash :cmdline 1 2 3 :shebang #!/bin/bash  
echo \"$1\" 
 
#+end_src"  
 (org-babel-execute-src-block)  



Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2023-12-04 Thread Ihor Radchenko
Matt  writes:

>  > #+begin_src bash :cmdline 1 2 3
>  >printf "%s\n" "$1"
>  > #+end_src
>  > 
>  > #+RESULTS:
>  > : 1
>  > 
>  > #+begin_src bash :cmdline 1 2 3 :shebang #!/bin/bash
>  >printf "%s\n" "$1"
>  > #+end_src
>  > 
>  > #+RESULTS:
>  > : 1 2 3
>
> Thank you!  This makes a good test case.

I am confused. Isn't it a bug?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [BUG] ob-shell: :cmdline fails with single argument (was Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline)

2023-11-18 Thread Matt


  On Sun, 19 Nov 2023 07:57:26 +0100  Max Nikulin  wrote --- 

 > I would say that :cmdline is treated in a different way in comparison to 
 > :var:

It most definitely is.  It's a completely separate process from :var.



Re: [BUG] ob-shell: :cmdline fails with single argument (was Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline)

2023-11-18 Thread Max Nikulin

On 19/11/2023 01:20, Matt wrote:

#+begin_src bash :cmdline 1
echo "$1"
#+end_src

Then it fails with

list: Wrong type argument: sequencep, 1


I would say that :cmdline is treated in a different way in comparison to 
:var:


#+header: :results verbatim
#+begin_src bash :var arr='(1 2 3) :cmdline '(97 98 99)
  printf '$1:%s\n' "$1"
  declare -p arr
#+end_src

#+RESULTS:
: $1:abc
: declare -a arr=([0]="1" [1]="2" [2]="3")

I would expect more consistent results since script arguments is an 
array (positional arguments). $1=97, $2=98, $3=99





[BUG] ob-shell: :cmdline fails with single argument (was Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline)

2023-11-18 Thread Matt

  On Sat, 18 Nov 2023 16:54:39 +0100  Max Nikulin  wrote --- 

 > I have faced an inconsistency with :cmdline treatment in ob-shell.el. 

These are sadly easy to find.  

If you run:

#+begin_src bash :cmdline 1 
echo "$1"
#+end_src

Then it fails with 

list: Wrong type argument: sequencep, 1

However, running this works:

#+begin_src bash :cmdline "1" 
echo "$1"
#+end_src

I didn't dig too much into it, but it looks like :cmdline expects a sequence.  
When multiple arguments are passed, such as :cmdline 1 2 3, then "1 2 3" is 
passed into process-file.  (sequencep "1 2 3") is t.  (sequencep 1) is nil.  
So, to work around this a single :cmdline argument must be surrounded by quotes 
to make it a sequence.  (sequencep "1") is t.  Obviously, this should be fixed.

Attached is a patch to test for this whenever we're ready to tackle making 
execution mutually consistent.  I'm still reviewing the library and am not 
quite ready for that yet.

test-cmdline-with-single-argument-shouldnt-require-quotes.patch
Description: Binary data


Re: [BUG] ob-shell: :shebang changes interpretation of :cmdline

2023-11-18 Thread Matt

  On Sat, 18 Nov 2023 16:54:39 +0100  Max Nikulin  wrote --- 

 > I have faced an inconsistency with :cmdline treatment in ob-shell.el. I 
 > expect same results in the following cases:
 > 
 > #+begin_src bash :cmdline 1 2 3
 >printf "%s\n" "$1"
 > #+end_src
 > 
 > #+RESULTS:
 > : 1
 > 
 > #+begin_src bash :cmdline 1 2 3 :shebang #!/bin/bash
 >printf "%s\n" "$1"
 > #+end_src
 > 
 > #+RESULTS:
 > : 1 2 3

Thank you!  This makes a good test case.

Ihor or Bastion, Craig and my employer reached agreement on the disclaimer 
language.  I've sent a signed copy to Craig and haven't heard back yet with the 
go-ahead to continue contributing.  I sent the paperwork only a week or so ago, 
so I'm sure he'll get back to me in time.  Because of this, I'm including 
patches rather than pushing (which assumes I still have access (I haven't 
checked)).


test-cmdline-alone-and-with-shebang-have-same-result.patch
Description: Binary data


[BUG] ob-shell: :shebang changes interpretation of :cmdline

2023-11-18 Thread Max Nikulin

Hi,

Trying to figure out the origin of the confusion with
"bash -c bash /path/to/file-containing-the-source-code.sh"
I have faced an inconsistency with :cmdline treatment in ob-shell.el. I 
expect same results in the following cases:


#+begin_src bash :cmdline 1 2 3
  printf "%s\n" "$1"
#+end_src

#+RESULTS:
: 1

#+begin_src bash :cmdline 1 2 3 :shebang #!/bin/bash
  printf "%s\n" "$1"
#+end_src

#+RESULTS:
: 1 2 3

Emacs-28, Org is the current git HEAD.