| Message ID | 20210831030426.nwcoelcnshq4wrxx@xzhoux.usersys.redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | common/rc: fix MKSWAP_PROG quoting | expand |
On Tue, Aug 31, 2021 at 11:04:26AM +0800, Murphy Zhou wrote: > After commit > 0e4dd8b9 common/rc: fix ignoring of errors on > we are getting this error message when running swapfiles tests: > +./common/rc: line 2553: MKSWAP_PROG: command not found > > Signed-off-by: Murphy Zhou <xzhou@redhat.com> > --- > common/rc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index 46b6b220..0597de13 100644 > --- a/common/rc > +++ b/common/rc > @@ -2550,7 +2550,7 @@ _format_swapfile() { > $CHATTR_PROG +C "$fname" > /dev/null 2>&1 > _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full > # Ignore permission complaints on filesystems that don't support perms > - $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full > + ($MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full The change history is: Commit 0c95c8ac tried to "hide permision warning", so did: - $MKSWAP_PROG "$fname" >> $seqres.full + # Ignore permission complaints on filesystems that don't support perms + $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full Then commit 0e4dd8b9 said "it broke older versions of bash such as 4.4.23", so tried to use "a $(foo) to run the executable": - $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full + $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full Now this patch try to do ($FOO_PROG): ($MKSWAP_PROG "$fname" .....) I'm *not* a bash expert, if I'm wrong feel free to correct me:) If the original problem is trying to hide permision warning from stderr, and to avoid new syntax breaking old bash, Why we must struggle with this complex syntax which isn't compatible. How about: $MKSWAP_PROG "$fname" >> $seqres.full 2>&1 | grep -v "insecure permission" Or other better and compatible way ? Thanks, Zorro > } > > _swapon_file() { > -- > 2.20.1 >
On 8/31/21 7:26 AM, Zorro Lang wrote: > On Tue, Aug 31, 2021 at 11:04:26AM +0800, Murphy Zhou wrote: >> After commit >> 0e4dd8b9 common/rc: fix ignoring of errors on >> we are getting this error message when running swapfiles tests: >> +./common/rc: line 2553: MKSWAP_PROG: command not found >> >> Signed-off-by: Murphy Zhou <xzhou@redhat.com> >> --- >> common/rc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/common/rc b/common/rc >> index 46b6b220..0597de13 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -2550,7 +2550,7 @@ _format_swapfile() { >> $CHATTR_PROG +C "$fname" > /dev/null 2>&1 >> _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full >> # Ignore permission complaints on filesystems that don't support perms >> - $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full >> + ($MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full > The change history is: > > Commit 0c95c8ac tried to "hide permision warning", so did: > - $MKSWAP_PROG "$fname" >> $seqres.full > + # Ignore permission complaints on filesystems that don't support perms > + $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full > > Then commit 0e4dd8b9 said "it broke older versions of bash such as 4.4.23", so tried > to use "a $(foo) to run the executable": > - $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full > + $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full > > Now this patch try to do ($FOO_PROG): > ($MKSWAP_PROG "$fname" .....) > > I'm *not* a bash expert, if I'm wrong feel free to correct me:) If the original problem > is trying to hide permision warning from stderr, and to avoid new syntax breaking old > bash, Why we must struggle with this complex syntax which isn't compatible. How about: Hi, sorry I do suck at bash, but: > > $MKSWAP_PROG "$fname" >> $seqres.full 2>&1 | grep -v "insecure permission" >> redirects stdout to the seqres.full and the 2>&1 redirects stderr to wherever stdout goes...so $MKSWAP_PROG "$fname" >> $seqres.full 2>&1 sends both stdout and stderr to the file so nothing is piped to grep. Or do I miss something (which is entirely possible)? Thanks! > > Or other better and compatible way ? > > Thanks, > Zorro > >> } >> >> _swapon_file() { >> -- >> 2.20.1 >>
On Tue, Aug 31, 2021 at 09:38:25AM +0200, Pavel Reichl wrote: > > On 8/31/21 7:26 AM, Zorro Lang wrote: > > On Tue, Aug 31, 2021 at 11:04:26AM +0800, Murphy Zhou wrote: > > > After commit > > > 0e4dd8b9 common/rc: fix ignoring of errors on > > > we are getting this error message when running swapfiles tests: > > > +./common/rc: line 2553: MKSWAP_PROG: command not found > > > > > > Signed-off-by: Murphy Zhou <xzhou@redhat.com> > > > --- > > > common/rc | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/common/rc b/common/rc > > > index 46b6b220..0597de13 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -2550,7 +2550,7 @@ _format_swapfile() { > > > $CHATTR_PROG +C "$fname" > /dev/null 2>&1 > > > _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full > > > # Ignore permission complaints on filesystems that don't support perms > > > - $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full > > > + ($MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full > > The change history is: > > > > Commit 0c95c8ac tried to "hide permision warning", so did: > > - $MKSWAP_PROG "$fname" >> $seqres.full > > + # Ignore permission complaints on filesystems that don't support perms > > + $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full > > > > Then commit 0e4dd8b9 said "it broke older versions of bash such as 4.4.23", so tried > > to use "a $(foo) to run the executable": > > - $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full > > + $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full > > > > Now this patch try to do ($FOO_PROG): > > ($MKSWAP_PROG "$fname" .....) > > > > I'm *not* a bash expert, if I'm wrong feel free to correct me:) If the original problem > > is trying to hide permision warning from stderr, and to avoid new syntax breaking old > > bash, Why we must struggle with this complex syntax which isn't compatible. How about: > > Hi, sorry I do suck at bash, but: > > > > > $MKSWAP_PROG "$fname" >> $seqres.full 2>&1 | grep -v "insecure permission" > > >> redirects stdout to the seqres.full and the 2>&1 redirects stderr to > wherever stdout goes...so $MKSWAP_PROG "$fname" >> $seqres.full 2>&1 sends > both stdout and stderr to the file so nothing is piped to grep. Or do I miss > something (which is entirely possible)? Sorry, I reversed the order. I mean: $MKSWAP_PROG "$fname" 2>&1 >> $seqres.full | grep -v "insecure permission" Thanks, Zorro > > > Thanks! > > > > > > Or other better and compatible way ? > > > > Thanks, > > Zorro > > > > > } > > > _swapon_file() { > > > -- > > > 2.20.1 > > > >
On Tue, Aug 31, 2021 at 09:42:07PM +0800, Zorro Lang wrote: > > Sorry, I reversed the order. I mean: > > $MKSWAP_PROG "$fname" 2>&1 >> $seqres.full | grep -v "insecure permission" That looks right, although I might have done something like this: filter_mkswap_stderr() { grep -v "insecure permission" } ... $MKSWAP_PROG "$fname" 2>&1 >> $seqres.full | filter_mkswap_stderr More for the documentation effect rather than the abstraction. But I wouldn't insist on it; some folks might consider this to be excess levels abstraction. - Ted
diff --git a/common/rc b/common/rc index 46b6b220..0597de13 100644 --- a/common/rc +++ b/common/rc @@ -2550,7 +2550,7 @@ _format_swapfile() { $CHATTR_PROG +C "$fname" > /dev/null 2>&1 _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full # Ignore permission complaints on filesystems that don't support perms - $(MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full + ($MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2)) >> $seqres.full } _swapon_file() {
After commit 0e4dd8b9 common/rc: fix ignoring of errors on we are getting this error message when running swapfiles tests: +./common/rc: line 2553: MKSWAP_PROG: command not found Signed-off-by: Murphy Zhou <xzhou@redhat.com> --- common/rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)