Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/log.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# ==============================================================================

# Unless $LOG_FD is already set to a valid fd
if ! [[ "$LOG_FD" =~ ^[0-9]+$ ]] || ! { true >&"$LOG_FD" ; } 2>/dev/null ; then
if ! [[ "${LOG_FD-}" =~ ^[0-9]+$ ]] || ! { : >&"${LOG_FD-2}"; } 2>/dev/null; then
Copy link

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the fix works correctly under set -u.

Please confirm that this change resolves the unbound variable error when running with set -u and that the logging functionality works as expected in both scenarios (when LOG_FD is set and when it's unbound).


🏁 Script executed:

#!/bin/bash
# Description: Test the LOG_FD handling under set -u to ensure the fix works correctly

# Create a test script to simulate the scenario
cat > test_log_fd.sh << 'EOF'
#!/bin/bash
set -u  # Enable nounset to catch unbound variables

# Source the log.sh file to test the fix
source lib/log.sh

echo "LOG_FD is now: $LOG_FD"
echo "Testing logging functionality..."

# Test basic logging
bashio::log "Test message"
bashio::log.info "Info message"
EOF

chmod +x test_log_fd.sh

# Test 1: Run without LOG_FD set (should not fail with set -u)
echo "=== Test 1: Running with set -u and no LOG_FD preset ==="
unset LOG_FD 2>/dev/null || true
./test_log_fd.sh

echo ""

# Test 2: Run with LOG_FD preset
echo "=== Test 2: Running with LOG_FD preset ==="
export LOG_FD=1
./test_log_fd.sh

# Cleanup
rm -f test_log_fd.sh

Length of output: 339


Initialize or guard __BASHIO_EXIT_OK to avoid unbound variable under set -u.
The tests still fail with “unbound variable” at lib/log.sh:27 for __BASHIO_EXIT_OK. Define it before use (e.g. : "${__BASHIO_EXIT_OK:=0}") or use ${__BASHIO_EXIT_OK:-} wherever it’s referenced.

🤖 Prompt for AI Agents
In lib/log.sh around line 11 (and affecting usage at line 27), the variable
__BASHIO_EXIT_OK may be referenced when running under set -u causing an "unbound
variable" error; initialize or guard it before use by setting a safe default
(e.g. ensure __BASHIO_EXIT_OK is defined to 0 if unset) or change references to
use parameter expansion that provides a default (use the :- form) so the script
never reads an undefined variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai bashio always sources lib/const.sh first, which defines __BASHIO_EXIT_OK. I think with that in mind things should be fine.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if lib/const.sh exists and contains __BASHIO_EXIT_OK definition
echo "=== Searching for lib/const.sh ==="
fd const.sh lib/

echo ""
echo "=== Checking if __BASHIO_EXIT_OK is defined in lib/const.sh ==="
if [ -f lib/const.sh ]; then
    echo "Found lib/const.sh, checking for __BASHIO_EXIT_OK definition:"
    rg -n "__BASHIO_EXIT_OK" lib/const.sh
else
    echo "lib/const.sh not found, searching for __BASHIO_EXIT_OK definitions in the entire codebase:"
    rg -n "__BASHIO_EXIT_OK.*=" --type=bash
fi

echo ""
echo "=== Checking how bashio is typically initialized ==="
# Look for files that source lib/const.sh
rg -n "source.*const\.sh|\..*const\.sh" --type=bash

Length of output: 316


🏁 Script executed:

#!/bin/bash
# Let's look at the const.sh file content and check how bashio is initialized
echo "=== Content of lib/const.sh showing __BASHIO_EXIT_OK definition ==="
cat lib/const.sh | head -30

echo ""
echo "=== Searching for sourcing patterns without file type restriction ==="
rg -n "source.*const\.sh|\..*const\.sh"

echo ""
echo "=== Looking for main bashio initialization files ==="
fd -e sh . | head -10

echo ""
echo "=== Checking if there's a main bashio.sh or similar entry point ==="
fd bashio

Length of output: 1774


@agners, you're absolutely right! Thank you for the clarification. I can confirm that:

  1. lib/const.sh properly defines __BASHIO_EXIT_OK=0 on line 21
  2. lib/bashio.sh sources lib/const.sh first (line 26), ensuring all constants are available before any other bashio functionality

My concern about __BASHIO_EXIT_OK being unbound was based on analyzing lib/log.sh in isolation rather than in the proper bashio usage context. In normal bashio usage, the constants are always properly initialized.

The LOG_FD fix using parameter expansion (${LOG_FD-} and ${LOG_FD-2}) is still the right approach for set -u compatibility, and the logging functionality should work correctly in both scenarios.


✏️ Learnings added
Learnt from: agners
PR: hassio-addons/bashio#173
File: lib/log.sh:11-11
Timestamp: 2025-09-26T15:27:24.191Z
Learning: In the bashio library, lib/const.sh defines all constants including __BASHIO_EXIT_OK, and lib/bashio.sh always sources lib/const.sh first, ensuring proper initialization before any other bashio functionality is used.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

# Preserve the original STDOUT on a free fd (stored in $LOG_FD) so that we can
# log to it without interfering with the STDOUT of subshells whose output we
# want to capture for other purposes.
Expand Down
Loading