jasonacox / Powerwall-Dashboard

Grafana Monitoring Dashboard for Tesla Solar and Powerwall Systems
MIT License
270 stars 57 forks source link

3.0.3 upgrade errors - MacOS #400

Closed jgleigh closed 6 months ago

jgleigh commented 6 months ago

compose.env: line 35: weather411,default: command not found

Looks like the setup.sh script is corrupting the compose.env file if there are already profiles set.

Original file: COMPOSE_PROFILES=default,weather411

Corrupted file after running upgrade: COMPOSE_PROFILES=default weather411,default

Running upgrade with no compose.env file works correctly.

jgleigh commented 6 months ago

MacOS 14.1.2 in case there are weird differences between OSes.

jasonacox commented 6 months ago

Thanks, @jgleigh - I'll investigate. I thought I had tested an upgrade eon MacOS but likely forgot the weather411 path. @mcbirse If you have a chance, take a look as well in case I'm missing something simple.

mcbirse commented 6 months ago

Hi @jasonacox - I'm not sure, but it seems it could be a compatibility issue with the separator (IFS) being set to a comma to process the bash array and update COMPOSE_PROFILES.

From what @jgleigh has shown, it looks like IFS does not get set to a comma so it remained as the default (space) when the compose.env file was updated. Must only affect some specific OS's or bash shells, as I tested many upgrade scenarios (but not on MacOS).

At a guess, the below might fix the issue. Have not tested yet but I'll try to do that tonight if I get time.

i.e. replace all occurrences of IFS=,; with IFS=',' in all scripts - example of updated helper function below.

add_profile() {
    # Create default docker compose env file if needed.
    if [ ! -f ${COMPOSE_ENV_FILE} ]; then
        cp "${COMPOSE_ENV_FILE}.sample" "${COMPOSE_ENV_FILE}"
    fi
    if ! get_profile "${1}"; then
        # Add profile to COMPOSE_PROFILES and save to env file
        PROFILES+=("${1}")
        if [ -z "${COMPOSE_PROFILES}" ]; then
            if grep -q "^#COMPOSE_PROFILES=" "${COMPOSE_ENV_FILE}"; then
                sed -i.bak "s@^#COMPOSE_PROFILES=.*@COMPOSE_PROFILES=$(IFS=',' echo "${PROFILES[*]}")@g" "${COMPOSE_ENV_FILE}"
            else
                echo -e "\nCOMPOSE_PROFILES=$(IFS=',' echo "${PROFILES[*]}")" >> "${COMPOSE_ENV_FILE}"
            fi
        else
            sed -i.bak "s@^COMPOSE_PROFILES=.*@COMPOSE_PROFILES=$(IFS=',' echo "${PROFILES[*]}")@g" "${COMPOSE_ENV_FILE}"
        fi
    fi
}
jasonacox commented 6 months ago

I can confirm that this impacts MacOS. Working on fix...

    # Check COMPOSE_PROFILES for profile
    IFS=',' read -a PROFILES <<< ${COMPOSE_PROFILES}
    echo "PROFILES: ${PROFILES[@]}"
    for p in "${PROFILES[@]}"; do
        echo "[${p}]"
        if [ "${p}" == "${1}" ]; then
            return 0
        fi
    done
    return 1
}

On Linux:

PROFILES: default weather411
[default]

On Mac:

PROFILES: default weather411
[default weather411]
PROFILES: default weather411
[default weather411]
jasonacox commented 6 months ago

I found the fix... simple, crazy... line 52:

from:

IFS=',' read -a PROFILES <<< ${COMPOSE_PROFILES}

to:

IFS=',' read -ra PROFILES <<< "${COMPOSE_PROFILES}"

I'll submit the PR for 3.0.4.

mcbirse commented 6 months ago

Oh wow, great find and yes, crazy!

jasonacox commented 6 months ago

It had me stumped...

I eventually just pasted it into ChatGPT and it suggested quoting ${COMPOSE_PROFILES} (see chat session).

😁

mcbirse commented 6 months ago

Amazing! ChatGPT and AI's in general have become seriously impressive, to the point of being a bit disturbing sometimes. 😄

jgleigh commented 6 months ago

Fascinating stuff. Thanks for the quick fix.

jasonacox commented 6 months ago

Thanks @mcbirse for fixing the rest! 🙏