gitbls / sdm

Raspberry Pi SD Card Image Manager
MIT License
469 stars 48 forks source link

non-zero exit on custom script #264

Closed mattie47 closed 2 weeks ago

mattie47 commented 2 weeks ago

Hi @gitbls,

I see from https://github.com/gitbls/sdm/issues/125 that functionality was implemented in V10.0 (diff here to runplugins "$theseplugins" $phase || exit on non-zero return codes.

Am I correct in that this won't work currently with custom scripts?

I see there's validation when trying to run a custom script:

[ "$cscript" != "" -a ! -f "$cscript" ] && errexit "? Custom Phase Script '$cscript' not found"
[ "$cscript" != "" -a ! -x "$cscript" ]  && errexit "? Custom Phase Script '$cscript' not executable"

However within the functions for running custom scripts with phase 0 and 1, I don't see any check on non-zero?

if [ "$cscript" != "" ]
then
    csfn="$sdmdir/$(basename $cscript)"
    logtoboth "> Run Custom Phase Script '$csfn' Phase 1" 
    $csfn 1
else
    csfn=""
fi

Is that intentional, or possibly an oversight?

We were trying to add some validation to our custom script so that if something fails, it should stop the entire SDM build.

Instead, right now if a failure /non-zero occurs:

As a very cut down example:

#!/bin/bash
#
# Custom Phase script
#

errexit() {
    echo -e $1
    exit 1
}

<code omitted>

elif [ "$phase" == "1" ]; then
    #
    # Phase 1 (in nspawn - use direct paths)
    #
    logtoboth "* $pfx Phase 1"
    logfreespace "at start of $pfx Phase 1"

    # Check we can actually install packages from APT.
    # Company firewall may block certain APT mirrors meaning desired apps don't actually
    # get installed. We quit building in this case since the image won't be complete. 
    logtoboth "* Checking Apt is able to install packages"
    apt update
    apt install --reinstall sl -y
    [ $? -ne 0 ] && errexit "? unable to install packages from APT. Quitting building SDM Image"

Thanks,

Matt

gitbls commented 2 weeks ago

If you want a failure in your custom phase script to stop sdm, just use

logtobothex "Failure description. Exiting"

logtobothex does a logtoboth and then exits with a failure status (1).

Does this do what you want?

gitbls commented 2 weeks ago

New day, closer look. For completeness, will update sdm-phase0 and sdm-phase1 to properly fail if custom phase script exits with fail status.

I've pushed an update for this. Please test and LMK if this resolves the issue for you. Thx

mattie47 commented 2 weeks ago

Amazing! Thank you for doing that @gitbls !

Have verified today and can confirm the added || exit is working as desired.

Will close the issue.