[PATCH for-4.21] tools/tests: don't pass -E to sed

Jan Beulich posted 1 patch 3 weeks, 1 day ago
Failed in applying to current master (apply log)
[PATCH for-4.21] tools/tests: don't pass -E to sed
Posted by Jan Beulich 3 weeks, 1 day ago
Even the 2018 edition of The Open Group Base Specifications Issue 7 [1]
doesn't name -E as a standard option; only Issue 8 [2] does. As there's
nothing "extended" about the expression used, simply drop the -E.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/
[2] https://pubs.opengroup.org/onlinepubs/9799919799/

Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
In principle the -e could be dropped too, for being redundant.

Hitting the problem with an older sed pointed out another problem here as
well: The failed invocation left a 0-byte pdx.h, which upon re-invocation
of make was (obviously) deemed up-to-date, thus causing the build to fail
again (until the bad file was actually removed).

--- a/tools/tests/pdx/Makefile
+++ b/tools/tests/pdx/Makefile
@@ -35,7 +35,7 @@ uninstall:
 	$(RM) -- $(patsubst %,$(DESTDIR)$(LIBEXEC)/tests/%,$(TARGETS))
 
 pdx.h: $(XEN_ROOT)/xen/include/xen/pdx.h
-	sed -E -e '/^#[[:space:]]*include/d' <$< >$@
+	sed -e '/^#[[:space:]]*include/d' <$< >$@
 
 CFLAGS += -D__XEN_TOOLS__
 CFLAGS += $(APPEND_CFLAGS)
Re: [PATCH for-4.21] tools/tests: don't pass -E to sed
Posted by Roger Pau Monné 3 weeks, 1 day ago
On Wed, Oct 08, 2025 at 11:47:05AM +0200, Jan Beulich wrote:
> Even the 2018 edition of The Open Group Base Specifications Issue 7 [1]
> doesn't name -E as a standard option; only Issue 8 [2] does. As there's
> nothing "extended" about the expression used, simply drop the -E.
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/
> [2] https://pubs.opengroup.org/onlinepubs/9799919799/
> 
> Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

I think the -E was carried over from a previous expression, that was
indeed extended.

> ---
> In principle the -e could be dropped too, for being redundant.
> 
> Hitting the problem with an older sed pointed out another problem here as
> well: The failed invocation left a 0-byte pdx.h, which upon re-invocation
> of make was (obviously) deemed up-to-date, thus causing the build to fail
> again (until the bad file was actually removed).

Hm, we could do something like:

sed -e '/^#[[:space:]]*include/d' <$< >$@ || $(RM) $@

Thanks, Roger.

Re: [PATCH for-4.21] tools/tests: don't pass -E to sed
Posted by Jan Beulich 3 weeks, 1 day ago
On 08.10.2025 16:30, Roger Pau Monné wrote:
> On Wed, Oct 08, 2025 at 11:47:05AM +0200, Jan Beulich wrote:
>> Even the 2018 edition of The Open Group Base Specifications Issue 7 [1]
>> doesn't name -E as a standard option; only Issue 8 [2] does. As there's
>> nothing "extended" about the expression used, simply drop the -E.
>>
>> [1] https://pubs.opengroup.org/onlinepubs/9699919799/
>> [2] https://pubs.opengroup.org/onlinepubs/9799919799/
>>
>> Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>> ---
>> In principle the -e could be dropped too, for being redundant.
>>
>> Hitting the problem with an older sed pointed out another problem here as
>> well: The failed invocation left a 0-byte pdx.h, which upon re-invocation
>> of make was (obviously) deemed up-to-date, thus causing the build to fail
>> again (until the bad file was actually removed).
> 
> Hm, we could do something like:
> 
> sed -e '/^#[[:space:]]*include/d' <$< >$@ || $(RM) $@

As is that would hide failure of the sed invocation from make. I was first
thinking to sed into a temporary file, to then rename that file. But this
won't cover the more general case of the issue either. Meanwhile I think
that the Makefile itself should become a dependency of the of the target
header. That way, if the sed expression changes, the file will be rebuilt.
(Of course this still builds on an assumption, specifically that any
failure here would be dealt with by an adjustment to the rule. So possibly
we need a combination of both.)

Jan

Re: [PATCH for-4.21] tools/tests: don't pass -E to sed
Posted by Roger Pau Monné 3 weeks, 1 day ago
On Wed, Oct 08, 2025 at 04:42:13PM +0200, Jan Beulich wrote:
> On 08.10.2025 16:30, Roger Pau Monné wrote:
> > On Wed, Oct 08, 2025 at 11:47:05AM +0200, Jan Beulich wrote:
> >> Even the 2018 edition of The Open Group Base Specifications Issue 7 [1]
> >> doesn't name -E as a standard option; only Issue 8 [2] does. As there's
> >> nothing "extended" about the expression used, simply drop the -E.
> >>
> >> [1] https://pubs.opengroup.org/onlinepubs/9699919799/
> >> [2] https://pubs.opengroup.org/onlinepubs/9799919799/
> >>
> >> Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Thanks.
> 
> >> ---
> >> In principle the -e could be dropped too, for being redundant.
> >>
> >> Hitting the problem with an older sed pointed out another problem here as
> >> well: The failed invocation left a 0-byte pdx.h, which upon re-invocation
> >> of make was (obviously) deemed up-to-date, thus causing the build to fail
> >> again (until the bad file was actually removed).
> > 
> > Hm, we could do something like:
> > 
> > sed -e '/^#[[:space:]]*include/d' <$< >$@ || $(RM) $@
> 
> As is that would hide failure of the sed invocation from make. I was first
> thinking to sed into a temporary file, to then rename that file. But this
> won't cover the more general case of the issue either.

Well, it would work if the sed into temporary file is a FORCE target,
and then the move to the final file is only done if there are
differences?

> Meanwhile I think
> that the Makefile itself should become a dependency of the of the target
> header. That way, if the sed expression changes, the file will be rebuilt.
> (Of course this still builds on an assumption, specifically that any
> failure here would be dealt with by an adjustment to the rule. So possibly
> we need a combination of both.)

It feels weird to me that a Makefile depends on itself, but yes, it
might solve the issue you pointed out in a simpler way.  Doesn't
makefile consider all make generated targets as obsolete if the
makefile itself changes?  The pdx.h generation is a clear example
here, but the same could apply to runes used to build object files?

Thanks, Roger.

Re: [PATCH for-4.21] tools/tests: don't pass -E to sed
Posted by Jan Beulich 3 weeks ago
On 08.10.2025 17:07, Roger Pau Monné wrote:
> On Wed, Oct 08, 2025 at 04:42:13PM +0200, Jan Beulich wrote:
>> On 08.10.2025 16:30, Roger Pau Monné wrote:
>>> On Wed, Oct 08, 2025 at 11:47:05AM +0200, Jan Beulich wrote:
>>>> Even the 2018 edition of The Open Group Base Specifications Issue 7 [1]
>>>> doesn't name -E as a standard option; only Issue 8 [2] does. As there's
>>>> nothing "extended" about the expression used, simply drop the -E.
>>>>
>>>> [1] https://pubs.opengroup.org/onlinepubs/9699919799/
>>>> [2] https://pubs.opengroup.org/onlinepubs/9799919799/
>>>>
>>>> Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Thanks.
>>
>>>> ---
>>>> In principle the -e could be dropped too, for being redundant.
>>>>
>>>> Hitting the problem with an older sed pointed out another problem here as
>>>> well: The failed invocation left a 0-byte pdx.h, which upon re-invocation
>>>> of make was (obviously) deemed up-to-date, thus causing the build to fail
>>>> again (until the bad file was actually removed).
>>>
>>> Hm, we could do something like:
>>>
>>> sed -e '/^#[[:space:]]*include/d' <$< >$@ || $(RM) $@
>>
>> As is that would hide failure of the sed invocation from make. I was first
>> thinking to sed into a temporary file, to then rename that file. But this
>> won't cover the more general case of the issue either.
> 
> Well, it would work if the sed into temporary file is a FORCE target,
> and then the move to the final file is only done if there are
> differences?

Yes, splitting like this ought to do.

>> Meanwhile I think
>> that the Makefile itself should become a dependency of the of the target
>> header. That way, if the sed expression changes, the file will be rebuilt.
>> (Of course this still builds on an assumption, specifically that any
>> failure here would be dealt with by an adjustment to the rule. So possibly
>> we need a combination of both.)
> 
> It feels weird to me that a Makefile depends on itself, but yes, it
> might solve the issue you pointed out in a simpler way.  Doesn't
> makefile consider all make generated targets as obsolete if the
> makefile itself changes?

I don't think so, no. What it feels a little as if you may be thinking of
is that if a Makefile itself is a target, make will re-invoke itself once
it was updated.

>  The pdx.h generation is a clear example
> here, but the same could apply to runes used to build object files?

Well, yes. That's why we have the .*.cmd files in the hypervisor build
system. That's less coarse than using the full Makefile-s as dependencies.

Jan

Re: [PATCH for-4.21] tools/tests: don't pass -E to sed
Posted by Oleksii Kurochko 3 weeks, 1 day ago
On 10/8/25 11:47 AM, Jan Beulich wrote:
> Even the 2018 edition of The Open Group Base Specifications Issue 7 [1]
> doesn't name -E as a standard option; only Issue 8 [2] does. As there's
> nothing "extended" about the expression used, simply drop the -E.
>
> [1]https://pubs.opengroup.org/onlinepubs/9699919799/
> [2]https://pubs.opengroup.org/onlinepubs/9799919799/
>
> Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
> Signed-off-by: Jan Beulich<jbeulich@suse.com>

LGTM:
   Reviewed-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
   Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

> ---
> In principle the -e could be dropped too, for being redundant.
>
> Hitting the problem with an older sed pointed out another problem here as
> well: The failed invocation left a 0-byte pdx.h, which upon re-invocation
> of make was (obviously) deemed up-to-date, thus causing the build to fail
> again (until the bad file was actually removed).
>
> --- a/tools/tests/pdx/Makefile
> +++ b/tools/tests/pdx/Makefile
> @@ -35,7 +35,7 @@ uninstall:
>   	$(RM) -- $(patsubst %,$(DESTDIR)$(LIBEXEC)/tests/%,$(TARGETS))
>   
>   pdx.h: $(XEN_ROOT)/xen/include/xen/pdx.h
> -	sed -E -e '/^#[[:space:]]*include/d' <$< >$@
> +	sed -e '/^#[[:space:]]*include/d' <$< >$@
>   
>   CFLAGS += -D__XEN_TOOLS__
>   CFLAGS += $(APPEND_CFLAGS)