[1/4] build: work around bash issue
diff mbox series

Message ID b427e8e6-b9ff-65d2-074b-19439a2e3d02@suse.com
State Superseded
Headers show
Series
  • build: corrections to .init.o generation logic
Related show

Commit Message

Jan Beulich Aug. 6, 2020, 9:04 a.m. UTC
Older bash fails to honor "set -e" for certain built-in commands
("while" here), despite the command's status correctly bein non-zero.
The subsequent objcopy invocation now being separated by a semicolon
results in no failure. Insert an explicit "exit" (replacing ; by &&
ought to be another possible workaround).

Fixes: e321576f4047 ("xen/build: start using if_changed")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I've done a pretty light-weight audit of possible further instances of
this issue, but didn't find any.

Comments

Jan Beulich Aug. 6, 2020, 9:14 a.m. UTC | #1
On 06.08.2020 11:07, Julien Grall wrote:
> On 06/08/2020 10:04, Jan Beulich wrote:
>> Older bash fails to honor "set -e" for certain built-in commands
> 
> "Older" is pretty vague. May I ask the exact version you run into the issue?

If I knew in what version the issue got fixed, I'd have specified
that version. I've observed it with 3.2.57(2).

Jan
Julien Grall Aug. 6, 2020, 9:44 a.m. UTC | #2
On 06/08/2020 10:25, Jan Beulich wrote:
> On 06.08.2020 11:20, Julien Grall wrote:
>> On 06/08/2020 10:14, Jan Beulich wrote:
>>> I've observed it with 3.2.57(2).
>>
>> Thank you. Please mention it in the commit message.
> 
> Well, added. If I had seen any use of the precise version, I would
> have done so right away. Would you mind educating me how knowing
> the precise version is useful, without knowing up to which version
> the issue exists?

Indeed, this wouldn't tell us whether other bash versions are affected.

However, as a reviewer, I need to be able to verify what you wrote is 
correct and that your patch is the correct way to solve it.

So providing the bash version where you observed the issue will at least 
fulfill this part.

Cheers,
Andrew Cooper Aug. 6, 2020, 2:57 p.m. UTC | #3
On 06/08/2020 10:14, Jan Beulich wrote:
> On 06.08.2020 11:07, Julien Grall wrote:
>> On 06/08/2020 10:04, Jan Beulich wrote:
>>> Older bash fails to honor "set -e" for certain built-in commands
>> "Older" is pretty vague. May I ask the exact version you run into the issue?
> If I knew in what version the issue got fixed, I'd have specified
> that version. I've observed it with 3.2.57(2).

Its still very useful information for the commit message.

I tend to phrase something like this as "Older versions of bash (at
least, 3.2.57(2))" which gives a clear indication that the problem was
observed with the specified version, without suggesting that this is a
boundary of when the issue was introduced/fixed.

With this adjusted, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
to avoid a second posting.

~Andrew

Patch
diff mbox series

--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -193,7 +193,7 @@  define cmd_obj_init_o
             echo "Error: size of $<:$$name is 0x$$sz" >&2; \
             exit $$(expr $$idx + 1);; \
         esac; \
-    done; \
+    done || exit $$?; \
     $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 endef