Skip to content

build.sh: fix error-handling for make#2898

Merged
cgwalters merged 1 commit intocoreos:mainfrom
jlebon:pr/fix-make
Jun 2, 2022
Merged

build.sh: fix error-handling for make#2898
cgwalters merged 1 commit intocoreos:mainfrom
jlebon:pr/fix-make

Conversation

@jlebon
Copy link
Copy Markdown
Member

@jlebon jlebon commented Jun 2, 2022

We have errexit in build.sh, but that has no effect on make here
because it's part of a command list.

Closes: #2897

We have `errexit` in `build.sh`, but that has no effect on `make` here
because it's part of a command list.

Closes: coreos#2897
@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Jun 2, 2022

Bash intricacies strike again!

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Jun 2, 2022

Here's how I tested this:

diff --git a/build.sh b/build.sh
index 992ead9cd..166f42707 100755
--- a/build.sh
+++ b/build.sh
@@ -101,11 +101,8 @@ install_ocp_tools() {
 }

 make_and_makeinstall() {
-    make
-    make install
-    # Remove go build cache
-    # https://github.com/coreos/coreos-assembler/issues/2872
-    rm -rf /root/.cache/go-build
+    false && make install
+    true
 }

 configure_user(){
$ ./build.sh make_and_makeinstall
++ pwd
+ srcdir=/home/jlebon/Code/gh/c/ca
+ '[' 1 -ne 0 ']'
+ make_and_makeinstall
+ false
+ true
$ echo $?
0

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Jun 2, 2022

Hmm OK right, I think the issue is actually that we're not using set -E which makes sure that function calls inherit errexit. Edit: no, -E affects trap inheritance, so I do think it's because of &&. bash(1) says:

-e      Exit immediately if a pipeline (which may consist of a single simple command), a list, or a  compound
        command (see SHELL GRAMMAR above), exits with a non-zero status.  The shell does not exit if the com‐
        mand that fails is part of the command list immediately following a while or until keyword,  part  of
        the test following the if or elif reserved words, part of any command executed in a && or || list ex‐
        cept the command following the final && or ||, any command in a pipeline but the last, or if the com‐
        mand's  return value is being inverted with !.

@dustymabe
Copy link
Copy Markdown
Member

Thanks for looking at this @jlebon!

@dustymabe
Copy link
Copy Markdown
Member

This should also fix #2884

@dustymabe dustymabe enabled auto-merge (rebase) June 2, 2022 18:35
@bgilbert bgilbert linked an issue Jun 2, 2022 that may be closed by this pull request
@cgwalters
Copy link
Copy Markdown
Member

Eh, let's just force this in

@cgwalters cgwalters disabled auto-merge June 2, 2022 18:43
@cgwalters cgwalters enabled auto-merge (rebase) June 2, 2022 18:44
@cgwalters cgwalters disabled auto-merge June 2, 2022 18:44
@cgwalters cgwalters merged commit f1e71a7 into coreos:main Jun 2, 2022
@jlebon jlebon deleted the pr/fix-make branch April 22, 2023 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Building cosa from Dockerfile sometimes passes even though compilation failed COSA container build not catching error

3 participants