Re: [PATCH] mksh.1: Clarify the effect of exit and return in a subshell

2017-04-13 Thread Jean Delvare
Hi Thorsten,

On mar., 2017-04-11 at 17:42 +, Thorsten Glaser wrote:
> Jean Delvare dixit:
> >@@ -2927,6 +2927,10 @@ function.
> > A function can be made to finish immediately using the
>  ↑
>   that’s a U+00A0 there, your MUA most likely garbled the patch.

Sorry about that. I thought that Evolution could be convinced to behave
correctly if the mail body was set to "Preformatted" but apparently
that's not always sufficient. What a pity. I'll remember to only use
Claws to send patches from now on.

> No problem though, I hand-applied it (I shortened it by a word
> to make a line fit in the last chunk, and formatted “while...read”,
> though).

Looks good, thank you for the editing work.

-- 
Jean Delvare
SUSE L3 Support


Re: pipes and sub-shells

2017-03-23 Thread Jean Delvare
Hi Martijn,

On mer., 2017-03-22 at 10:26 +0100, Martijn Dekker wrote:
> Op 22-03-17 om 10:12 schreef Jean Delvare:
> > 
> > Concretely, the customer's code looks like this:
> > 
> > command | while read line
> > do
> > if 
> > then
> > exit
> > fi
> > process $line
> > done
> [...]
> > 
> > I was wondering if there is any other trick you can suggest that would work 
> > in mksh?
> 
> A here-string with a command substitution:
> 
> while read line
> do
>   if 
>   then
>   exit
>   fi
>   process $line
> done <<<$(command)

Apparently it requires a more recent version of mksh than we are
shipping:

$ echo $KSH_VERSION
@(#)MIRBSD KSH R50 2014/06/29 openSUSE
$ ./test_return.sh
./test_return.sh[10]: syntax error: '(' unexpected

But indeed works find with the latest upstream version, thanks for the
pointer.

Thanks,
-- 
Jean Delvare
SUSE L3 Support


Re: several messages

2017-03-15 Thread Jean Delvare
On mar., 2017-03-14 at 11:43 +, Thorsten Glaser wrote:
> Jean Delvare dixit:
> >> if [[ $KSH_VERSION = *@(MIRBSD|LEGACY)\ KSH* ]]; then
> >>  function unset {
> >>  local __foo
> >>
> >>  for __foo in "$@"; do
> >>  eval "unset $__foo[*]"
> >>  done
> >>  }
> >> fi
> >
> >Does that actually work? It doesn't seem like special shell builtins
> >can be overloaded with functions, and unset is a special shell builtin.
> 
> Ouch. I admit not having tested it. But you can name the function x_unset
> and then define an alias. Aliases are parse-time and thus will be used
> in favour of builtins of any kind.

Tried it. My first attempt resulted in a delay followed by:

Segmentation fault (core dumped)

so I was hesitant to recommend it to my customer ;-) Then I realized
the code was infinitely recursive, as the unset in eval is caught by
the alias. Escaping it with \\ solved the problem, and then yes, it
works. Thanks for the suggestion, which I have forwarded to my
customer.

> (...)
> True, but this is Unix, we give users the rope to hang themselves.

Sure. And the stool. And a fine manual explaining how to make a noose
right.

> (...)
> Thanks, applied… I didn’t catch this because BSD mdoc, in contrast
> to GNU’s, did this right already with the No.

The language used for man pages seems to be the least portable thing on
earth, right after shells and shell scripts ;-)

-- 
Jean Delvare
SUSE L3 Support


[PATCH] Mention -e in print command synopsis

2017-02-27 Thread Jean Delvare
Now that the print command supports option -e and it is documented,
it should also appear in the command synopsis.
---
 mksh.1 |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/mksh.1
+++ b/mksh.1
@@ -3759,7 +3759,7 @@ however, distributors may have added thi
 .Pp
 .It Xo
 .Ic print
-.Oo Fl AclNnprsu Ns Oo Ar n Oc \*(Ba
+.Oo Fl AcelNnprsu Ns Oo Ar n Oc \*(Ba
 .Fl R Op Fl n Oc
 .Op Ar argument ...
 .Xc
-- 
Jean Delvare
SUSE L3 Support


Re: print -R compatibility

2017-02-27 Thread Jean Delvare
Hi Thorsten,

Sorry for the late reply, I have been away from home due to unexpected
family issues.

On ven., 2017-02-17 at 22:37 +, Thorsten Glaser wrote:
> Jean Delvare dixit:
> >Our previous products included ksh93.
> 
> OK. (Found out as well that ksh93 has print -e, which we had code
> for but no member in the getopt call…)

Good catch. As ‘\’ sequences are processed by default, the use cases
for -e must be fairly limited. As I understand it, the only purpose of
the option is to cancel a previous -r, which is kind of a corner case.
That must be why nobody noticed so far. But for compatibility with
other ksh implementations, I agree it is better to support it, so
thanks for having fixed it.

> (...)
> I looked at ksh93 and investigated several approaches and ended
> up codifying “as soon as -R is encountered, we jump into the BSD
> echo (formerly called Debian echo) codepath”.
> 
> This matches ksh93 in that things like a previous -n, -uN, -s, …
> are *not* reset:
> 
> tg@blau:/usr/src/bin/mksh $ obj/mksh -c 'print -u2 -R -42' >/dev/null
> -42
> 
> This allows your use cases:
> 
> tg@blau:/usr/src/bin/mksh $ obj/mksh -c 'print -R -n -4; print -R 2'
> -42

Looks good.

> Parsing after -R like ksh93 does is not supported:
> 
> tg@blau:/usr/src/bin/mksh $ ksh93 -c 'print -Rn foo; print bar'
> foobar
> tg@blau:/usr/src/bin/mksh $ obj/mksh -c 'print -Rn foo; print bar'
> foo
> bar
> 
> But:
> 
> tg@blau:/usr/src/bin/mksh $ obj/mksh -c 'print -R -n foo; print bar'
> foobar

Fair enough. I hope this minor incompatibility won't be a problem for
out customer.

> This will be in the next mksh release.

Thank you very much. I have backported your work to mksh R50 as this is
what our product includes, and will send to our customer shortly for
testing.

-- 
Jean Delvare
SUSE L3 Support


Re: print -R compatibility

2017-02-10 Thread Jean Delvare
On Fri, 10 Feb 2017 13:10:20 + (UTC), Thorsten Glaser wrote:
> That’s ksh88 where you(r customer) come(s) from?

Our previous products included ksh93.

> >> We have the echo you mean in POSIX mode though…
> >
> >Not sure what you mean here. I see that the echo command behavior is
> >changed if Flag(FPOSIX), but I can't seem to be able to set that flag
> >(I tried "export POSIX=1" but that doesn't seem to change anything?)
> 
> “set -o posix” ;-)

Got it, thanks.

> >(...)
> >For reference, here is the patch I came up with. The idea is to jump to
> >the "echo" command handling code a soon as we see -R. The rest of the
> >changes is to remove po.pminusminus as it is no longer needed. Known
> 
> That’s about what I’d have done as well (unless the BSD echo
> behaves different from classical echo in which 'print -R -ex'
> would be the same as 'print -r -- -ex'; the alternative would
> be to parse the -e from -ex and only then output it, turning
> it into 'print -- -ex'). This is why some investigation is
> likely still needed.

I don't think it makes sense to consider one half of a parameter as an
option and the other half as a non-option. And definitely not to handle
a given letter as an option and then still printing it... "-e" is not
necessarily relevant as ksh93's print command doesn't support it in the
first place, but I tested "-n":

$ print -R -nx
-nx
$

So it only treats -n as an option if not mixed with unsupported option
letters. That is exactly what mksh does for the echo command, and I
think we should stick to that for consistency.

> If I take your patch, do you wish to have your name added to
> the list of copyright holders at the top of the file?

No need, this is only a minor contribution.

> >caveat: -E would be handled as a valid option, while it was not
> 
> True, but that can be circumvented.

Sure it can. My patch was more of a proof-of-concept (one of many, I
tried different approaches before) and details can be discussed.

Thanks for your time,
-- 
Jean Delvare
SUSE L3 Support


Re: print -R compatibility

2017-02-10 Thread Jean Delvare
Hi Thorsten,

Thanks for your quick answer.

On jeu., 2017-02-09 at 18:46 +, Thorsten Glaser wrote:
> Jean Delvare dixit:
> >In mksh, "print -R" is not as close to "echo" as it was in original
> >ksh. Specifically, after "-R", mksh's print command keeps parsing
> >the command line in search of options, and stops as soon as it
> >finds an
> 
> Indeed, this is as documented:
> 
>     The -R option is used to emulate, to some degree, the BSD echo(1)
>     command which does not process ‘\’ sequences unless the -e option
>     is given.  As above, the -n option suppresses the trailing new‐
>     line.
> 
> The -R option does _not_ emulate the echo you mean but another.

I din't think -R currently emulates either properly (but I suppose I
can't complain, as "to some degree" could be read as implying just
that.) BSD echo will print arguments starting with a dash, other than a
leading -n. So if "print -R -42" is supposed to be like BSD "echo -42",
it should print "-42", as is the case with legacy ksh.

> We have the echo you mean in POSIX mode though…

Not sure what you mean here. I see that the echo command behavior is
changed if Flag(FPOSIX), but I can't seem to be able to set that flag
(I tried "export POSIX=1" but that doesn't seem to change anything?)

Also that would only change the behavior of echo anyway, not print.

> >Example with the original ksh:
> >
> >$ print -R -42
> >-42
> 
> Quick workaround (this will lose you support for -n though):
> 
> function print {
> if [[ $1 = -R ]]; then
> shift
> builtin print -r -- "$@"
> else
> builtin print "$@"
> fi
> }

Actually I already suggested to the customer that they could replace
all the instances of "print -R" by "print -r --" in their scripts. They
did not answer yet.

I believe "print -r --" is a better choice in general regardless of the
outcome of our discussion, as -R will treat -n and -e (and combinations
thereof) as options, and the user may not expect that.

> >I looked at the mksh code and was able to modify the print_c function
> >to get mksh's print command to behave the same with -R as echo and the
> >original ksh print with -R. I have a patch ready. Are you interested in
> >it, or is the different behavior on purpose?
> 
> While I think you’re the first user of “print -R”, this looks as if it
> was intended — I’d address this on the customer side, maybe in a
> two-step process:
> 
> 1. Check all uses of 'print.*-[^ ]R' for which syntax is used, e.g.
>    if we need to handle “print -R -n”, “print -Rn”, “print -nR” or
>    somesuch; adjust the abovementioned print function, use it
> 
> 2. Convert all uses of print’s -R option to 'print -r --' or
>    'print -nr --' in the scripts (yes, more effort, but worth it).

I agree, and I would go that way if it was my code (straight to step 2,
even.) But it's not, and my experience is that customers are often
reluctant to change scripts that have been running for a long time and
are still deployed on many, sometimes heterogeneous, systems. I can
make suggestions but they get to make decisions.

> I’d advocate against trying to do something with echo that even
> pretends portability; we have no less than three different echo
> implementations in mksh alone (and which is chosen depends on
> several factors).

I did suggest to them to just use "echo" at one point and they turned
it down exactly because of portability concerns. Their script is
running on different OS flavors where echo behaves differently. They
are using "print -R" instead precisely because that appeared to be
portable across all their systems. Until they started migrating to
mksh, that is.

> Nevertheless, thank you for mailing about this!
> 
> 
> Hrm. Funnily enough:
> 
> tglase@tglase-nb:~ $ print -R -- -42
> -- -42
> tglase@tglase-nb:~ $ print -R -x -42
> /bin/mksh: print: -x: unknown option

I did notice this inconsistency too as part of my investigation, and
that's one of the reasons why I'm not sure the current behavior is
really by design.

> Perhaps I’ll have to investigate “the BSD echo(1) command”
> further (no problem thanks to TUHS) and maybe the implementation
> is indeed wrong… but the result will likely end up in a formal
> mksh release too late for your customer anyway. I’ll make a note.

That would not be a problem. If I know for sure that such a change will
make it into a future release of mksh, I can backport the change
immediately to whatever version our customers are running.

For reference, here is the patch I came up with. The idea is to jump to
the "echo" com

[PATCH] funcs: Fix confusing indentation in print_c()

2017-02-08 Thread Jean Delvare
The incorrect indentation can easily cause the reader to interpret
the code wrongly.
---
 funcs.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/funcs.c
+++ b/funcs.c
@@ -445,8 +445,8 @@ c_print(const char **wp)
    if (wp[builtin_opt.optind] &&
    ksh_isdash(wp[builtin_opt.optind]))
    builtin_opt.optind++;
-   } else if (po.pminusminus)
-   builtin_opt.optind--;
+   } else if (po.pminusminus)
+   builtin_opt.optind--;
    wp += builtin_opt.optind;
    }
 
-- 
Jean Delvare
SUSE L3 Support


Re: Environment and pipes

2015-09-14 Thread Jean Delvare
Le Friday 11 September 2015 à 20:23 +, Thorsten Glaser a écrit :
> Dixi quod…
> 
> >I think coprocesses are pretty usable for this in almost all cases
> >(I did have one where they weren’t, but you can still often background
> >a part, then play with fd redirection).
> 
> Another thing that may help you, if it’s absolutely needed:
> 
> Before we had ${PIPESTATUS[*]} I wrote things like this:
> 
> foo | (bar; echo $? >bar.rv) | baz
> 
> Similarily, you can do tricks with high file descriptors.
> 
> x=$( (echo foo | (tr a-z A-Z >&4) | (echo bla >&5)) 4>&1) 5>&1
> echo x=$x
> 
> This gives:
> 
> bla
> x=FOO

"Nice" :]

> Both not as efficient as direct variable assignment, but…
> 
> And let's not forget using…
>   while …; do …; done  … instead of…
>   cat foo | while …; do …; done
> … (which is cat abuse anyway).

Amen. I've been working with a lot of bash scripts and I yell each time
I see the "cat foo |" construct.

> Often, some not-so-basic or more modern scripting approaches
> can eliminate the need for a construct like you were asking for.

I will check with the customer. The example they provided was clearly
made up to illustrate the problem (and I do appreciate when customers
make the problem easy to understand) so I don't know exactly what their
actually script is doing.

-- 
Jean Delvare
SUSE L3 Support