* [9front] [PATCH] rc: keep env vars and functions in sync
@ 2024-07-11 5:32 Romano
2024-07-12 19:36 ` Romano
0 siblings, 1 reply; 8+ messages in thread
From: Romano @ 2024-07-11 5:32 UTC (permalink / raw)
To: 9front
From env(3):
The env device serves a one-level directory containing files
with arbitrary names and contents. The intention is that
the file name is the name of an environment variable (see
rc(1)), and the content is the variable's current value.
And from rc(1):
Environment
The environment is a list of strings made available to exe-
cuting binaries by the env device (see env(3)).
But currently, the environment variables and functions are not kept
in sync with /env . For instance, '>/env/a echo -n b' will not
update $a to be 'b'. Also, even when the namespaces are shared
between processes, once the child process completes the namespaces
are out of sync.
This patch attempts to address that by updating the environment
variables and functions after children processes exit when the
children share the same namespace. Tests are added for the
functionality as well.
---
diff 8a64e5171aad8bb0201a61e7df3046892ee6617f c17447a0c0bc8211015c87261abe57ed46ffe0ca
--- a/sys/src/cmd/rc/fns.h
+++ b/sys/src/cmd/rc/fns.h
@@ -27,6 +27,7 @@
void Setstatus(char*);
void Trapinit(void);
void Updenv(void);
+void Updenvall(void);
void Vinit(void);
int Waitfor(int);
long Write(int, void*, long);
--- a/sys/src/cmd/rc/havefork.c
+++ b/sys/src/cmd/rc/havefork.c
@@ -64,6 +64,7 @@
setvar("apid", newword(npid, (word *)0));
break;
}
+ Updenvall();
}
void
@@ -99,6 +100,7 @@
p->pid = pid;
break;
}
+ Updenvall();
}
/*
@@ -128,7 +130,7 @@
Close(pfd[PRD]);
start(runq->code, runq->pc+1, runq->local, runq->redir);
pushredir(ROPEN, pfd[PWR], 1);
- return;
+ break;
default:
addwaitpid(pid);
Close(pfd[PWR]);
@@ -149,8 +151,9 @@
Waitfor(pid);
runq->pc = runq->code[runq->pc].i;
- return;
+ break;
}
+ Updenvall();
}
void
@@ -196,6 +199,7 @@
p->pc = p->code[pc+1].i;
break;
}
+ Updenvall();
}
void
@@ -219,6 +223,7 @@
runq->pc = runq->code[runq->pc].i;
break;
}
+ Updenvall();
}
int
--- a/sys/src/cmd/rc/plan9.c
+++ b/sys/src/cmd/rc/plan9.c
@@ -259,6 +259,17 @@
}
void
+Updenvall(void)
+{
+ Vinit();
+ char *cmds = estrdup("for(fn in /env/fn'#'*) { . -bq $fn }\n");
+ int line = runq->line;
+ execcmds(openiocore(cmds, strlen(cmds)), estrdup(srcfile(runq)), runq->local, runq->redir);
+ runq->lex->line = line;
+ runq->lex->qflag = 1;
+}
+
+void
Exec(char **argv)
{
exec(argv[0], argv+1);
--- a/sys/src/cmd/rc/simple.c
+++ b/sys/src/cmd/rc/simple.c
@@ -102,6 +102,7 @@
/* interrupts don't get us out */
while(Waitfor(pid) < 0)
;
+ Updenvall();
}
}
}
--- /dev/null
+++ b/sys/src/cmd/rc/test/mkfile
@@ -1,0 +1,6 @@
+</$objtype/mkfile
+
+TEST=\
+ updenvall\
+
+</sys/src/cmd/mktest
--- /dev/null
+++ b/sys/src/cmd/rc/test/updenvall.file
@@ -1,0 +1,80 @@
+>/env/var1 echo -n a
+ok `\x04{whatis var1} 'var1=a
+' '$var1 is /env/var1'
+ok $var1 a '$var1 is a'
+ok `\x04{cat /env/var1} a '/env/var1 is a'
+
+echo testing var with rc
+../$O.out -c 'var1=ab'
+ok `\x04{whatis var1} 'var1=ab
+' 'var1 is ab'
+ok $var1 ab '$var1 is ab'
+ok `\x04{cat /env/var1} ab '/env/var1 is ab'
+
+echo testing var with rc -c
+../$O.out -c 'var1=b'
+ok `\x04{whatis var1} 'var1=b
+' 'var1 is b'
+ok $var1 b '$var1 is b'
+ok `\x04{cat /env/var1} b '/env/var1 is b'
+
+echo testing var with subshell
+@{ var1=c }
+ok `\x04{whatis var1} 'var1=c
+' 'var1 is c'
+ok $var1 c '$var1 is c'
+ok `\x04{cat /env/var1} c '/env/var1 is c'
+
+echo testing var with backtick
+echo `\x04{ var1=d } >/dev/null
+ok `\x04{whatis var1} 'var1=d
+' 'var1 is d'
+ok $var1 d '$var1 is d'
+ok `\x04{cat /env/var1} d '/env/var1 is d'
+
+a=()
+> '/env/fn#a' echo -n 'fn a {
+ echo a
+}
+'
+ok `\x04{whatis a} 'fn a {
+ echo a
+}
+' 'fn a is echo a'
+ok `\x04{cat '/env/fn#a'} 'fn a {
+ echo a
+}
+' '/env/fn#a is echo a'
+
+echo testing fn with rc -c
+../$O.out -c 'fn a { echo b }'
+ok `\x04{whatis a} 'fn a {
+ echo b
+}
+' 'fn a is echo b'
+ok `\x04{cat '/env/fn#a'} 'fn a {
+ echo b
+}
+' '/env/fn#a is echo b'
+
+echo testing fn with subshell
+@{ fn a { echo c } }
+ok `\x04{whatis a} 'fn a {
+ echo c
+}
+' 'fn b is echo c'
+ok `\x04{cat '/env/fn#a'} 'fn a {
+ echo c
+}
+' '/env/fn#a is echo c'
+
+echo testing fn with backtick
+echo `\x04{fn a { echo d } } >/dev/null
+ok `\x04{whatis a} 'fn a {
+ echo d
+}
+' 'fn a is echo d'
+ok `\x04{cat '/env/fn#a'} 'fn a {
+ echo d
+}
+' '/env/fn#a is echo d'
--- /dev/null
+++ b/sys/src/cmd/rc/test/updenvall.rc
@@ -1,0 +1,16 @@
+#!/bin/rc
+# test that all environment variables and functions
+# are in sync.
+
+fn ok {
+ actual=$1
+ shift
+ expected=$1
+ shift
+ desc=$1
+ if(~ $"actual $"expected) echo ok $desc
+ if not echo 'not ok '^$"desc^', got '$"actual
+}
+
+# now let's run the test with our build
+../$O.out <updenvall.file
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-11 5:32 [9front] [PATCH] rc: keep env vars and functions in sync Romano
@ 2024-07-12 19:36 ` Romano
2024-07-12 19:58 ` ori
0 siblings, 1 reply; 8+ messages in thread
From: Romano @ 2024-07-12 19:36 UTC (permalink / raw)
To: 9front
After dogfooding this on my own system, I am liking how it works, save
for how it messed with $*, $pid, and $apid, which are all documented as
special variables and (at least for $*) affects the use of fn calls. So,
in other words, the previous patch had desiderata.
I also noticed the bug re: $status being overwritten when using ~ to
check its value. That is now fixed with the patch below.
In case anyone wants to try it out, after applying both the previous
patch and the patch below, you can run 'mk test' and then run 6.out
(which has a handy prompt, 'broken!', so you know what you're dealing).
For more widespread testing I have been then doing:
cp 6.out $home/bin/$objtype/rc.share
to see how it compares with rc with some scripts.
From: Romano <me+unobe@fallglow.com>
Date: Fri, 12 Jul 2024 19:24:48 +0000
Subject: [PATCH] rc: do not share certain special vars; fix $status bug for most cases
Certain special vars (namely $*, $pid, $apid, and $status)
are used internally and should not be set after a child
returns.
There has been a long-standing bug where checking the value
of $status with ~ overwrites the value of $status. $status
is now restored in most cases, other than running it
as a standalone built-in command.
Man page for rc is updated
---
diff 7637f8f5fd6490d07ba64bcf404321028e698ed9 675bcd62992e59002c834ef445d515b346f4374d
--- a/sys/src/cmd/rc/code.c
+++ b/sys/src/cmd/rc/code.c
@@ -275,6 +275,8 @@
codeswitch(t, eflag);
break;
case TWIDDLE:
+ if(c0 && c0->type=='$' && c0->child[0] && (c0->child[0]->type==WORD && strcmp(c0->child[0]->str, "status")==0))
+ emitf(Xstashstatus);
emitf(Xmark);
noglobs(c1, 1);
outcode(c1, eflag);
--- a/sys/src/cmd/rc/exec.c
+++ b/sys/src/cmd/rc/exec.c
@@ -282,6 +282,16 @@
dotrap();
}
}
+
+void
+unstashstatus(void)
+{
+ var *oldstatus = vlook("status-");
+ if(oldstatus->val && oldstatus->val->word){
+ setstatus(oldstatus->val->word);
+ }
+}
+
/*
* Opcode routines
* Arguments on stack (...)
@@ -415,6 +425,7 @@
void
Xifnot(void)
{
+ unstashstatus();
if(ifnot)
runq->pc++;
else
@@ -597,7 +608,9 @@
void
Xtrue(void)
{
- if(truestatus()) runq->pc++;
+ int stat = truestatus();
+ unstashstatus();
+ if(stat) runq->pc++;
else runq->pc = runq->code[runq->pc].i;
}
@@ -605,7 +618,9 @@
Xif(void)
{
ifnot = 1;
- if(truestatus()) runq->pc++;
+ int stat = truestatus();
+ unstashstatus();
+ if(stat) runq->pc++;
else runq->pc = runq->code[runq->pc].i;
}
@@ -661,6 +676,12 @@
}
poplist();
poplist();
+}
+
+void
+Xstashstatus(void)
+{
+ setvar("status-", Newword(estrdup(getstatus()), (word *)0));
}
void
--- a/sys/src/cmd/rc/exec.h
+++ b/sys/src/cmd/rc/exec.h
@@ -4,7 +4,7 @@
extern void Xappend(void), Xasync(void), Xbackq(void), Xbang(void), Xclose(void);
extern void Xconc(void), Xcount(void), Xdelfn(void), Xdol(void), Xqw(void), Xdup(void);
extern void Xexit(void), Xfalse(void), Xfn(void), Xfor(void), Xglob(void);
-extern void Xjump(void), Xmark(void), Xmatch(void), Xpipe(void), Xread(void), Xhere(void), Xhereq(void);
+extern void Xjump(void), Xmark(void), Xmatch(void), Xstashstatus(void), Xpipe(void), Xread(void), Xhere(void), Xhereq(void);
extern void Xrdwr(void), Xsrcline(void);
extern void Xunredir(void), Xstar(void), Xreturn(void), Xsubshell(void);
extern void Xtrue(void), Xword(void), Xwrite(void), Xpipefd(void), Xcase(void);
--- a/sys/src/cmd/rc/pfnc.c
+++ b/sys/src/cmd/rc/pfnc.c
@@ -30,6 +30,7 @@
Xword, "Xword",
Xwrite, "Xwrite",
Xmatch, "Xmatch",
+ Xstashstatus, "Xstashstatus",
Xcase, "Xcase",
Xconc, "Xconc",
Xassign, "Xassign",
--- a/sys/src/cmd/rc/plan9.c
+++ b/sys/src/cmd/rc/plan9.c
@@ -144,7 +144,12 @@
if(n <= 0)
break;
for(i = 0; i<n; i++){
- if(ent[i].length<=0 || strncmp(ent[i].name, "fn#", 3)==0)
+ if(ent[i].length<=0
+ || strncmp(ent[i].name, "fn#", 3)==0
+ || strcmp(ent[i].name,"*")==0
+ || strcmp(ent[i].name,"apid")==0
+ || strcmp(ent[i].name,"pid")==0
+ || strcmp(ent[i].name,"status")==0)
continue;
if((fd = Open(Env(ent[i].name, 0), 0))>=0){
io *f = openiofd(fd);
--- a/sys/src/cmd/rc/test/updenvall.file
+++ b/sys/src/cmd/rc/test/updenvall.file
@@ -78,3 +78,40 @@
echo d
}
' '/env/fn#a is echo d'
+
+echo testing special vars
+*=b
+echo -n a > /env/'*'
+ok $"* 'b' '* not set'
+echo -n $"pid >/tmp/pid.rc.test
+rc -c ''
+ok $"pid `{cat /tmp/pid.rc.test} 'pid not set'
+rm /tmp/pid.rc.test
+echo -n `{apid=foobar; echo $apid} >/dev/null
+ok $"apid '' 'apid not set'
+echo -n foo > /env/status
+ok $"status '' 'status not set'
+status=foo
+~ $status foo
+ok $"status '' 'naked ~ sets status to '''''
+status=`{echo foo}
+ok $"status foo 'backtick sets status'
+if(~ $status foo)
+ ok $"status foo '$status'
+if(! ~ $status a)
+ ok $"status foo '$status for ! ~'
+if(~ $status a)
+ ;
+if not
+ ok $status foo '$status for if not'
+if(~ $"status foo)
+ ok $"status foo '$"status'
+if(echo a | grep -s a || ls)
+ ok $"status foo '||'
+while(! ~ $status foo)
+ status=bar
+ok $status foo 'after while not match'
+while(~ $status foo)
+ status=bar;
+ok $status bar 'after while match'
+status=''
--- a/sys/src/cmd/rc/test/updenvall.rc
+++ b/sys/src/cmd/rc/test/updenvall.rc
@@ -9,7 +9,7 @@
shift
desc=$1
if(~ $"actual $"expected) echo ok $desc
- if not echo 'not ok '^$"desc^', got '$"actual
+ if not echo 'NOT OK '^$"desc^', got '$"actual
}
# now let's run the test with our build
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-12 19:36 ` Romano
@ 2024-07-12 19:58 ` ori
2024-07-12 20:15 ` Dave Woodman
0 siblings, 1 reply; 8+ messages in thread
From: ori @ 2024-07-12 19:58 UTC (permalink / raw)
To: 9front
this seems like it would make rc ridiculously slow.
Quoth Romano <me+unobe@fallglow.com>:
> After dogfooding this on my own system, I am liking how it works, save
> for how it messed with $*, $pid, and $apid, which are all documented as
> special variables and (at least for $*) affects the use of fn calls. So,
> in other words, the previous patch had desiderata.
>
> I also noticed the bug re: $status being overwritten when using ~ to
> check its value. That is now fixed with the patch below.
>
> In case anyone wants to try it out, after applying both the previous
> patch and the patch below, you can run 'mk test' and then run 6.out
> (which has a handy prompt, 'broken!', so you know what you're dealing).
> For more widespread testing I have been then doing:
> cp 6.out $home/bin/$objtype/rc.share
> to see how it compares with rc with some scripts.
>
> From: Romano <me+unobe@fallglow.com>
> Date: Fri, 12 Jul 2024 19:24:48 +0000
> Subject: [PATCH] rc: do not share certain special vars; fix $status bug for most cases
>
>
> Certain special vars (namely $*, $pid, $apid, and $status)
> are used internally and should not be set after a child
> returns.
>
> There has been a long-standing bug where checking the value
> of $status with ~ overwrites the value of $status. $status
> is now restored in most cases, other than running it
> as a standalone built-in command.
this is not a bug; ~ is a command.
>
> Man page for rc is updated
> ---
> diff 7637f8f5fd6490d07ba64bcf404321028e698ed9 675bcd62992e59002c834ef445d515b346f4374d
> --- a/sys/src/cmd/rc/code.c
> +++ b/sys/src/cmd/rc/code.c
> @@ -275,6 +275,8 @@
> codeswitch(t, eflag);
> break;
> case TWIDDLE:
> + if(c0 && c0->type=='$' && c0->child[0] && (c0->child[0]->type==WORD && strcmp(c0->child[0]->str, "status")==0))
> + emitf(Xstashstatus);
> emitf(Xmark);
> noglobs(c1, 1);
> outcode(c1, eflag);
> --- a/sys/src/cmd/rc/exec.c
> +++ b/sys/src/cmd/rc/exec.c
> @@ -282,6 +282,16 @@
> dotrap();
> }
> }
> +
> +void
> +unstashstatus(void)
> +{
> + var *oldstatus = vlook("status-");
> + if(oldstatus->val && oldstatus->val->word){
> + setstatus(oldstatus->val->word);
> + }
> +}
> +
> /*
> * Opcode routines
> * Arguments on stack (...)
> @@ -415,6 +425,7 @@
> void
> Xifnot(void)
> {
> + unstashstatus();
> if(ifnot)
> runq->pc++;
> else
> @@ -597,7 +608,9 @@
> void
> Xtrue(void)
> {
> - if(truestatus()) runq->pc++;
> + int stat = truestatus();
> + unstashstatus();
> + if(stat) runq->pc++;
> else runq->pc = runq->code[runq->pc].i;
> }
>
> @@ -605,7 +618,9 @@
> Xif(void)
> {
> ifnot = 1;
> - if(truestatus()) runq->pc++;
> + int stat = truestatus();
> + unstashstatus();
> + if(stat) runq->pc++;
> else runq->pc = runq->code[runq->pc].i;
> }
>
> @@ -661,6 +676,12 @@
> }
> poplist();
> poplist();
> +}
> +
> +void
> +Xstashstatus(void)
> +{
> + setvar("status-", Newword(estrdup(getstatus()), (word *)0));
> }
>
> void
> --- a/sys/src/cmd/rc/exec.h
> +++ b/sys/src/cmd/rc/exec.h
> @@ -4,7 +4,7 @@
> extern void Xappend(void), Xasync(void), Xbackq(void), Xbang(void), Xclose(void);
> extern void Xconc(void), Xcount(void), Xdelfn(void), Xdol(void), Xqw(void), Xdup(void);
> extern void Xexit(void), Xfalse(void), Xfn(void), Xfor(void), Xglob(void);
> -extern void Xjump(void), Xmark(void), Xmatch(void), Xpipe(void), Xread(void), Xhere(void), Xhereq(void);
> +extern void Xjump(void), Xmark(void), Xmatch(void), Xstashstatus(void), Xpipe(void), Xread(void), Xhere(void), Xhereq(void);
> extern void Xrdwr(void), Xsrcline(void);
> extern void Xunredir(void), Xstar(void), Xreturn(void), Xsubshell(void);
> extern void Xtrue(void), Xword(void), Xwrite(void), Xpipefd(void), Xcase(void);
> --- a/sys/src/cmd/rc/pfnc.c
> +++ b/sys/src/cmd/rc/pfnc.c
> @@ -30,6 +30,7 @@
> Xword, "Xword",
> Xwrite, "Xwrite",
> Xmatch, "Xmatch",
> + Xstashstatus, "Xstashstatus",
> Xcase, "Xcase",
> Xconc, "Xconc",
> Xassign, "Xassign",
> --- a/sys/src/cmd/rc/plan9.c
> +++ b/sys/src/cmd/rc/plan9.c
> @@ -144,7 +144,12 @@
> if(n <= 0)
> break;
> for(i = 0; i<n; i++){
> - if(ent[i].length<=0 || strncmp(ent[i].name, "fn#", 3)==0)
> + if(ent[i].length<=0
> + || strncmp(ent[i].name, "fn#", 3)==0
> + || strcmp(ent[i].name,"*")==0
> + || strcmp(ent[i].name,"apid")==0
> + || strcmp(ent[i].name,"pid")==0
> + || strcmp(ent[i].name,"status")==0)
> continue;
> if((fd = Open(Env(ent[i].name, 0), 0))>=0){
> io *f = openiofd(fd);
> --- a/sys/src/cmd/rc/test/updenvall.file
> +++ b/sys/src/cmd/rc/test/updenvall.file
> @@ -78,3 +78,40 @@
> echo d
> }
> ' '/env/fn#a is echo d'
> +
> +echo testing special vars
> +*=b
> +echo -n a > /env/'*'
> +ok $"* 'b' '* not set'
> +echo -n $"pid >/tmp/pid.rc.test
> +rc -c ''
> +ok $"pid `{cat /tmp/pid.rc.test} 'pid not set'
> +rm /tmp/pid.rc.test
> +echo -n `{apid=foobar; echo $apid} >/dev/null
> +ok $"apid '' 'apid not set'
> +echo -n foo > /env/status
> +ok $"status '' 'status not set'
> +status=foo
> +~ $status foo
> +ok $"status '' 'naked ~ sets status to '''''
> +status=`{echo foo}
> +ok $"status foo 'backtick sets status'
> +if(~ $status foo)
> + ok $"status foo '$status'
> +if(! ~ $status a)
> + ok $"status foo '$status for ! ~'
> +if(~ $status a)
> + ;
> +if not
> + ok $status foo '$status for if not'
> +if(~ $"status foo)
> + ok $"status foo '$"status'
> +if(echo a | grep -s a || ls)
> + ok $"status foo '||'
> +while(! ~ $status foo)
> + status=bar
> +ok $status foo 'after while not match'
> +while(~ $status foo)
> + status=bar;
> +ok $status bar 'after while match'
> +status=''
> --- a/sys/src/cmd/rc/test/updenvall.rc
> +++ b/sys/src/cmd/rc/test/updenvall.rc
> @@ -9,7 +9,7 @@
> shift
> desc=$1
> if(~ $"actual $"expected) echo ok $desc
> - if not echo 'not ok '^$"desc^', got '$"actual
> + if not echo 'NOT OK '^$"desc^', got '$"actual
> }
>
> # now let's run the test with our build
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-12 19:58 ` ori
@ 2024-07-12 20:15 ` Dave Woodman
2024-07-12 20:52 ` Romano
0 siblings, 1 reply; 8+ messages in thread
From: Dave Woodman @ 2024-07-12 20:15 UTC (permalink / raw)
To: 9front
To Ori's point regarding ~, the rc documentation would seem to *require*
that ~ update $status:
~ subject pattern ...
The subject is matched against each pattern in
sequence. If it matches any pattern, $status is set to
zero. Otherwise, $status is set to one.
So, as Ori points out, it is not a bug.
Just my tuppence ha'penny,
Dave.
On 12/07/2024 20:58, ori@eigenstate.org wrote:
> this seems like it would make rc ridiculously slow.
>
> Quoth Romano <me+unobe@fallglow.com>:
>> After dogfooding this on my own system, I am liking how it works, save
>> for how it messed with $*, $pid, and $apid, which are all documented as
>> special variables and (at least for $*) affects the use of fn calls. So,
>> in other words, the previous patch had desiderata.
>>
>> I also noticed the bug re: $status being overwritten when using ~ to
>> check its value. That is now fixed with the patch below.
>>
>> In case anyone wants to try it out, after applying both the previous
>> patch and the patch below, you can run 'mk test' and then run 6.out
>> (which has a handy prompt, 'broken!', so you know what you're dealing).
>> For more widespread testing I have been then doing:
>> cp 6.out $home/bin/$objtype/rc.share
>> to see how it compares with rc with some scripts.
>>
>> From: Romano <me+unobe@fallglow.com>
>> Date: Fri, 12 Jul 2024 19:24:48 +0000
>> Subject: [PATCH] rc: do not share certain special vars; fix $status bug for most cases
>>
>>
>> Certain special vars (namely $*, $pid, $apid, and $status)
>> are used internally and should not be set after a child
>> returns.
>>
>> There has been a long-standing bug where checking the value
>> of $status with ~ overwrites the value of $status. $status
>> is now restored in most cases, other than running it
>> as a standalone built-in command.
> this is not a bug; ~ is a command.
>
>> Man page for rc is updated
>> ---
>> diff 7637f8f5fd6490d07ba64bcf404321028e698ed9 675bcd62992e59002c834ef445d515b346f4374d
>> --- a/sys/src/cmd/rc/code.c
>> +++ b/sys/src/cmd/rc/code.c
>> @@ -275,6 +275,8 @@
>> codeswitch(t, eflag);
>> break;
>> case TWIDDLE:
>> + if(c0 && c0->type=='$' && c0->child[0] && (c0->child[0]->type==WORD && strcmp(c0->child[0]->str, "status")==0))
>> + emitf(Xstashstatus);
>> emitf(Xmark);
>> noglobs(c1, 1);
>> outcode(c1, eflag);
>> --- a/sys/src/cmd/rc/exec.c
>> +++ b/sys/src/cmd/rc/exec.c
>> @@ -282,6 +282,16 @@
>> dotrap();
>> }
>> }
>> +
>> +void
>> +unstashstatus(void)
>> +{
>> + var *oldstatus = vlook("status-");
>> + if(oldstatus->val && oldstatus->val->word){
>> + setstatus(oldstatus->val->word);
>> + }
>> +}
>> +
>> /*
>> * Opcode routines
>> * Arguments on stack (...)
>> @@ -415,6 +425,7 @@
>> void
>> Xifnot(void)
>> {
>> + unstashstatus();
>> if(ifnot)
>> runq->pc++;
>> else
>> @@ -597,7 +608,9 @@
>> void
>> Xtrue(void)
>> {
>> - if(truestatus()) runq->pc++;
>> + int stat = truestatus();
>> + unstashstatus();
>> + if(stat) runq->pc++;
>> else runq->pc = runq->code[runq->pc].i;
>> }
>>
>> @@ -605,7 +618,9 @@
>> Xif(void)
>> {
>> ifnot = 1;
>> - if(truestatus()) runq->pc++;
>> + int stat = truestatus();
>> + unstashstatus();
>> + if(stat) runq->pc++;
>> else runq->pc = runq->code[runq->pc].i;
>> }
>>
>> @@ -661,6 +676,12 @@
>> }
>> poplist();
>> poplist();
>> +}
>> +
>> +void
>> +Xstashstatus(void)
>> +{
>> + setvar("status-", Newword(estrdup(getstatus()), (word *)0));
>> }
>>
>> void
>> --- a/sys/src/cmd/rc/exec.h
>> +++ b/sys/src/cmd/rc/exec.h
>> @@ -4,7 +4,7 @@
>> extern void Xappend(void), Xasync(void), Xbackq(void), Xbang(void), Xclose(void);
>> extern void Xconc(void), Xcount(void), Xdelfn(void), Xdol(void), Xqw(void), Xdup(void);
>> extern void Xexit(void), Xfalse(void), Xfn(void), Xfor(void), Xglob(void);
>> -extern void Xjump(void), Xmark(void), Xmatch(void), Xpipe(void), Xread(void), Xhere(void), Xhereq(void);
>> +extern void Xjump(void), Xmark(void), Xmatch(void), Xstashstatus(void), Xpipe(void), Xread(void), Xhere(void), Xhereq(void);
>> extern void Xrdwr(void), Xsrcline(void);
>> extern void Xunredir(void), Xstar(void), Xreturn(void), Xsubshell(void);
>> extern void Xtrue(void), Xword(void), Xwrite(void), Xpipefd(void), Xcase(void);
>> --- a/sys/src/cmd/rc/pfnc.c
>> +++ b/sys/src/cmd/rc/pfnc.c
>> @@ -30,6 +30,7 @@
>> Xword, "Xword",
>> Xwrite, "Xwrite",
>> Xmatch, "Xmatch",
>> + Xstashstatus, "Xstashstatus",
>> Xcase, "Xcase",
>> Xconc, "Xconc",
>> Xassign, "Xassign",
>> --- a/sys/src/cmd/rc/plan9.c
>> +++ b/sys/src/cmd/rc/plan9.c
>> @@ -144,7 +144,12 @@
>> if(n <= 0)
>> break;
>> for(i = 0; i<n; i++){
>> - if(ent[i].length<=0 || strncmp(ent[i].name, "fn#", 3)==0)
>> + if(ent[i].length<=0
>> + || strncmp(ent[i].name, "fn#", 3)==0
>> + || strcmp(ent[i].name,"*")==0
>> + || strcmp(ent[i].name,"apid")==0
>> + || strcmp(ent[i].name,"pid")==0
>> + || strcmp(ent[i].name,"status")==0)
>> continue;
>> if((fd = Open(Env(ent[i].name, 0), 0))>=0){
>> io *f = openiofd(fd);
>> --- a/sys/src/cmd/rc/test/updenvall.file
>> +++ b/sys/src/cmd/rc/test/updenvall.file
>> @@ -78,3 +78,40 @@
>> echo d
>> }
>> ' '/env/fn#a is echo d'
>> +
>> +echo testing special vars
>> +*=b
>> +echo -n a > /env/'*'
>> +ok $"* 'b' '* not set'
>> +echo -n $"pid >/tmp/pid.rc.test
>> +rc -c ''
>> +ok $"pid `{cat /tmp/pid.rc.test} 'pid not set'
>> +rm /tmp/pid.rc.test
>> +echo -n `{apid=foobar; echo $apid} >/dev/null
>> +ok $"apid '' 'apid not set'
>> +echo -n foo > /env/status
>> +ok $"status '' 'status not set'
>> +status=foo
>> +~ $status foo
>> +ok $"status '' 'naked ~ sets status to '''''
>> +status=`{echo foo}
>> +ok $"status foo 'backtick sets status'
>> +if(~ $status foo)
>> + ok $"status foo '$status'
>> +if(! ~ $status a)
>> + ok $"status foo '$status for ! ~'
>> +if(~ $status a)
>> + ;
>> +if not
>> + ok $status foo '$status for if not'
>> +if(~ $"status foo)
>> + ok $"status foo '$"status'
>> +if(echo a | grep -s a || ls)
>> + ok $"status foo '||'
>> +while(! ~ $status foo)
>> + status=bar
>> +ok $status foo 'after while not match'
>> +while(~ $status foo)
>> + status=bar;
>> +ok $status bar 'after while match'
>> +status=''
>> --- a/sys/src/cmd/rc/test/updenvall.rc
>> +++ b/sys/src/cmd/rc/test/updenvall.rc
>> @@ -9,7 +9,7 @@
>> shift
>> desc=$1
>> if(~ $"actual $"expected) echo ok $desc
>> - if not echo 'not ok '^$"desc^', got '$"actual
>> + if not echo 'NOT OK '^$"desc^', got '$"actual
>> }
>>
>> # now let's run the test with our build
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-12 20:15 ` Dave Woodman
@ 2024-07-12 20:52 ` Romano
2024-07-12 21:25 ` Dave Woodman
2024-07-12 21:43 ` Romano
0 siblings, 2 replies; 8+ messages in thread
From: Romano @ 2024-07-12 20:52 UTC (permalink / raw)
To: 9front
Thanks, dave and ori. See my responses below.
On Fri Jul 12 13:19:53 -0700 2024, dave@naffnet.org.uk wrote:
> To Ori's point regarding ~, the rc documentation would seem to *require*
> that ~ update $status:
>
> ~ subject pattern ...
> The subject is matched against each pattern in
> sequence. If it matches any pattern, $status is set to
> zero. Otherwise, $status is set to one.
>
> So, as Ori points out, it is not a bug.
The BUGS section of the man page listed it as one, which is what
I was going off of. And I realize I didn't commit the man page
patch. That's now included below so you can see where in the
man page I'm referring to, and how I updated it which I hope
clarifies the scope.
> On 12/07/2024 20:58, ori@eigenstate.org wrote:
> > this seems like it would make rc ridiculously slow.
I haven't benchmarked it, but I haven't noticed a significant
slowdown in the scripts I've run thus far. Granted, they are
not enormously complex. The processing of very large /env/
files would add some time, surely; the max size of env
variables (under /env) is currently set to 1M.
Do you have an idea of how you would want a benchmark for
this to look like?
To be clear, I don't expect this to be signed off on any time
soon but did want to show it to the list for feedback.
I appreciate the feedback thus far.
From: Romano <me+unobe@fallglow.com>
Date: Fri, 12 Jul 2024 20:40:40 +0000
Subject: [PATCH] rc: updated man page
---
diff 675bcd62992e59002c834ef445d515b346f4374d c32dabd4853888f62f09a6d3f8e0deed4077b6a7
--- a/sys/man/1/rc
+++ b/sys/man/1/rc
@@ -825,7 +825,13 @@
When
.I rc
starts executing it reads variable and function definitions from its
-environment.
+environment, except for the special variables
+.BR $* ,
+.BR $pid ,
+and
+.BR $apid ,
+and
+.BR $status .
.SS Special Variables
The following variables are set or used by
.IR rc .
@@ -1019,7 +1025,16 @@
.B ~
to check the value of
.B $status
-changes
-.BR $status .
+as a changes
+.BR $status
+when used alone as a built-in command. E.g.,
+.EX
+cpu% status=b; ~ $status a; echo $status
+no match
+cpu% status=b; if(~ $status a; echo $status) echo $status
+no match
+b
+cpu%
+.EE
.PP
Free carets don't get inserted next to keywords.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-12 20:52 ` Romano
@ 2024-07-12 21:25 ` Dave Woodman
2024-07-12 21:36 ` Romano
2024-07-12 21:43 ` Romano
1 sibling, 1 reply; 8+ messages in thread
From: Dave Woodman @ 2024-07-12 21:25 UTC (permalink / raw)
To: 9front
I always understood (from so many years of UNIX and UNIX-like systems)
that BUGS did not necessarily mean defect.
From a "modern" source:
*"BUGS A list of limitations, known defects or inconveniences, and other
questionable activities"*
*I, personally, would characterise the ~ behaviour as a limitation**.
*
*Happy to bow to the majority, however.*
*Dave.
*
On 12/07/2024 21:52, Romano wrote:
> Thanks, dave and ori. See my responses below.
>
> On Fri Jul 12 13:19:53 -0700 2024, dave@naffnet.org.uk wrote:
>> To Ori's point regarding ~, the rc documentation would seem to *require*
>> that ~ update $status:
>>
>> ~ subject pattern ...
>> The subject is matched against each pattern in
>> sequence. If it matches any pattern, $status is set to
>> zero. Otherwise, $status is set to one.
>>
>> So, as Ori points out, it is not a bug.
> The BUGS section of the man page listed it as one, which is what
> I was going off of. And I realize I didn't commit the man page
> patch. That's now included below so you can see where in the
> man page I'm referring to, and how I updated it which I hope
> clarifies the scope.
>
>> On 12/07/2024 20:58, ori@eigenstate.org wrote:
>>> this seems like it would make rc ridiculously slow.
> I haven't benchmarked it, but I haven't noticed a significant
> slowdown in the scripts I've run thus far. Granted, they are
> not enormously complex. The processing of very large /env/
> files would add some time, surely; the max size of env
> variables (under /env) is currently set to 1M.
>
> Do you have an idea of how you would want a benchmark for
> this to look like?
>
> To be clear, I don't expect this to be signed off on any time
> soon but did want to show it to the list for feedback.
>
> I appreciate the feedback thus far.
>
> From: Romano <me+unobe@fallglow.com>
> Date: Fri, 12 Jul 2024 20:40:40 +0000
> Subject: [PATCH] rc: updated man page
>
> ---
> diff 675bcd62992e59002c834ef445d515b346f4374d c32dabd4853888f62f09a6d3f8e0deed4077b6a7
> --- a/sys/man/1/rc
> +++ b/sys/man/1/rc
> @@ -825,7 +825,13 @@
> When
> .I rc
> starts executing it reads variable and function definitions from its
> -environment.
> +environment, except for the special variables
> +.BR $* ,
> +.BR $pid ,
> +and
> +.BR $apid ,
> +and
> +.BR $status .
> .SS Special Variables
> The following variables are set or used by
> .IR rc .
> @@ -1019,7 +1025,16 @@
> .B ~
> to check the value of
> .B $status
> -changes
> -.BR $status .
> +as a changes
> +.BR $status
> +when used alone as a built-in command. E.g.,
> +.EX
> +cpu% status=b; ~ $status a; echo $status
> +no match
> +cpu% status=b; if(~ $status a; echo $status) echo $status
> +no match
> +b
> +cpu%
> +.EE
> .PP
> Free carets don't get inserted next to keywords.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-12 21:25 ` Dave Woodman
@ 2024-07-12 21:36 ` Romano
0 siblings, 0 replies; 8+ messages in thread
From: Romano @ 2024-07-12 21:36 UTC (permalink / raw)
To: 9front
On Fri Jul 12 14:29:26 -0700 2024, dave@naffnet.org.uk wrote:
> I always understood (from so many years of UNIX and UNIX-like systems)
> that BUGS did not necessarily mean defect.
> …
Ah, with that understanding, I see where you're coming from then.
I don't know how the the original author (Duffy) considered this
behavior. But like you, I'll bow to the majority.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [9front] [PATCH] rc: keep env vars and functions in sync
2024-07-12 20:52 ` Romano
2024-07-12 21:25 ` Dave Woodman
@ 2024-07-12 21:43 ` Romano
1 sibling, 0 replies; 8+ messages in thread
From: Romano @ 2024-07-12 21:43 UTC (permalink / raw)
To: 9front
Also, on the topic of if this syncing the env vars from /env
perhaps a refactor broke it? I haven't used vanilla plan9 in
more than a decade, but Tom Duffy's paper mentions /env
should work like this (quoted from /sys/doc/rc.ms:1036,1038 ):
A consequence of this organization is that
commands can change environment entries and
see the changes reflected in rc.
Without the $status changes, this set of patches would make
what is stated there true.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-07-12 21:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-11 5:32 [9front] [PATCH] rc: keep env vars and functions in sync Romano
2024-07-12 19:36 ` Romano
2024-07-12 19:58 ` ori
2024-07-12 20:15 ` Dave Woodman
2024-07-12 20:52 ` Romano
2024-07-12 21:25 ` Dave Woodman
2024-07-12 21:36 ` Romano
2024-07-12 21:43 ` Romano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).