Add Kconfig option for Supervisor channel #3618
Conversation
agners
changed the title
|
I was considering making this a package config, like so: --- a/buildroot-external/package/hassio/Config.in
+++ b/buildroot-external/package/hassio/Config.in
@@ -18,4 +18,10 @@ config BR2_PACKAGE_HASSIO_MACHINE
help
Machine to pull containers for (used for landing page).
+config BR2_PACKAGE_HASSIO_CHANNEL
+ string "Default Channel"
+ default "beta"
+ help
+ Channel to use by default.
+
endifBut then it can't be overriden from Makefile (at least it didn't work for me 🤔 ). I think we should rename the variable to |
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new configuration option Changes
Warning Rate limit exceeded@agners has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
buildroot-external/package/hassio/create-data-partition.sh (2)
6-6: LGTM! Consider adding parameter validation.The addition of the
channelparameter aligns with the PR objectives. However, to improve robustness, consider adding parameter validation and a default value.You could modify the script to include validation and a default value like this:
channel=$3 + +# Set default channel if not provided +channel=${channel:-stable} + +# Validate channel +if [[ ! "$channel" =~ ^(stable|beta|dev)$ ]]; then + echo "Error: Invalid channel. Must be 'stable', 'beta', or 'dev'." + exit 1 +fiThis ensures that the script always has a valid channel value, even if it's not explicitly provided.
28-28: LGTM! Consider adding a comment for clarity.The modification correctly passes the
channelparameter to thedind-import-containers.shscript, aligning with the PR objectives.For improved readability, consider adding a comment explaining the purpose of this line:
+# Execute container import script with specified channel docker exec "${container}" sh /build/dind-import-containers.sh "${channel}"This addition would make the script's intention clearer to future maintainers.
buildroot-external/package/hassio/dind-import-containers.sh (3)
4-4: LGTM! Consider adding input validation.The addition of the
channelvariable aligns well with the PR objectives, allowing users to set the release channel. This change enhances the script's flexibility.Consider adding input validation or a default value for the
channelvariable. This would prevent potential issues if the script is called without an argument. For example:channel=${1:-stable}This sets 'stable' as the default if no argument is provided.
33-33: LGTM! Consider adding a directory check.The creation of the JSON file with the channel information is a good addition. It provides a structured way to store and access the channel information, which aligns with the PR objectives.
Consider adding a check to ensure the
/data/supervisordirectory exists before creating the file. This would prevent potential errors if the directory structure is not as expected. For example:mkdir -p /data/supervisor echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.jsonThis ensures the directory exists before attempting to create the file.
Line range hint
1-33: Overall, the changes look good and align with the PR objectives.The addition of the
channelvariable and the creation of the JSON file with channel information successfully implement the feature to set and store the release channel. The changes are well-integrated into the existing script structure.To further improve the script's robustness and maintainability, consider:
- Implementing error handling for the channel parameter.
- Adding comments to explain the purpose of the new additions, especially the JSON file creation.
- If this channel information is used elsewhere in the system, ensure consistent naming and structure across different components.
Makefile (1)
34-35: LGTM: Effective integration of channel variable into build process.The modification to include
BR2_PACKAGE_HASSIO_CHANNEL=$(CHANNEL)in the make command effectively integrates the new channel variable into the build process. This change ensures that the specified channel is used during the build, aligning with the PR objectives.For improved readability, consider adding a comment explaining the purpose of the new parameter:
$(MAKE) -C $(BUILDROOT) O=$(O) BR2_EXTERNAL=$(BUILDROOT_EXTERNAL) \ + # Pass the channel information to Buildroot BR2_PACKAGE_HASSIO_CHANNEL=$(CHANNEL)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- Makefile (2 hunks)
- buildroot-external/package/hassio/create-data-partition.sh (2 hunks)
- buildroot-external/package/hassio/dind-import-containers.sh (2 hunks)
- buildroot-external/package/hassio/hassio.mk (2 hunks)
🧰 Additional context used
🔇 Additional comments (5)
buildroot-external/package/hassio/create-data-partition.sh (1)
Line range hint
1-32: Overall, the changes effectively implement the channel selection feature.The modifications to this script successfully incorporate the new
channelparameter, allowing for flexible release channel selection as per the PR objectives. The changes are minimal yet impactful, maintaining the overall structure and readability of the script.To further enhance the script:
- Consider adding parameter validation and a default value for the
channelvariable.- A brief comment explaining the purpose of the
dind-import-containers.shcall would improve clarity.These suggestions would make the script more robust and easier to maintain in the future.
buildroot-external/package/hassio/hassio.mk (2)
Line range hint
1-33: Overall assessment: Changes align well with PR objectivesThe modifications to this Makefile effectively implement the feature allowing users to set the release channel for pre-installed Home Assistant components. The changes are consistent and well-integrated into the existing structure.
Key points:
- The HASSIO_VERSION_URL now dynamically uses the specified channel.
- The channel information is passed to the create-data-partition.sh script during installation.
These changes provide the desired flexibility in component sourcing, both during installation and at runtime. The verification tasks will help ensure the completeness of the implementation across the codebase.
33-33: LGTM: Channel information passed to create-data-partition.shThe update to pass
$(BR2_PACKAGE_HASSIO_CHANNEL)as an argument to the create-data-partition.sh script is consistent with the PR objective and the previous change. This allows the script to use the specified channel during the installation process.Let's verify if the create-data-partition.sh script has been updated to handle this new argument:
#!/bin/bash # Description: Verify the create-data-partition.sh script for the new channel argument # Find the create-data-partition.sh script script_path=$(fd -t f "create-data-partition.sh") if [ -n "$script_path" ]; then echo "Found create-data-partition.sh at: $script_path" echo "\nChecking for channel argument handling:" rg -n "channel" "$script_path" else echo "create-data-partition.sh not found. Please verify the script location." fiMakefile (2)
6-6: LGTM: Well-implemented channel variable.The addition of the
CHANNELvariable with a default value of "stable" is a good implementation. Using the?=operator allows for easy overriding during the build process, providing flexibility for users to specify different channels as needed. This change aligns well with the PR objective of allowing users to set the release channel for pre-installed Home Assistant components.
Line range hint
1-53: Overall: Well-implemented feature for channel specification.The changes to the Makefile effectively implement the desired feature of allowing channel specification for pre-installed Home Assistant components. The addition of the
CHANNELvariable and its integration into the build process are minimal, focused, and maintain the existing structure and style of the Makefile.These modifications align well with the PR objectives and provide the flexibility for users to specify different channels during the build process. The implementation is clean and doesn't introduce any apparent issues or conflicts with the existing code.
| @@ -9,7 +9,7 @@ HASSIO_LICENSE = Apache License 2.0 | |||
| # HASSIO_LICENSE_FILES = $(BR2_EXTERNAL_HASSOS_PATH)/../LICENSE | |||
| HASSIO_SITE = $(BR2_EXTERNAL_HASSOS_PATH)/package/hassio | |||
| HASSIO_SITE_METHOD = local | |||
| HASSIO_VERSION_URL = "https://version.home-assistant.io/stable.json" | |||
| HASSIO_VERSION_URL = "https://version.home-assistant.io/$(BR2_PACKAGE_HASSIO_CHANNEL).json" | |||
There was a problem hiding this comment.
💡 Codebase verification
Issue Found: BR2_PACKAGE_HASSIO_CHANNEL is used but not defined.
- The variable
BR2_PACKAGE_HASSIO_CHANNELis utilized inbuildroot-external/package/hassio/create-data-partition.shbut its definition is missing in the Makefiles. - Ensure that
BR2_PACKAGE_HASSIO_CHANNELis properly defined and set within the appropriate configuration files to avoid potential build or runtime issues.
🔗 Analysis chain
LGTM: Dynamic channel selection implemented.
The change to use $(BR2_PACKAGE_HASSIO_CHANNEL) in the HASSIO_VERSION_URL allows for dynamic selection of the version URL based on the specified channel. This aligns well with the PR objective of allowing users to set the release channel for pre-installed Home Assistant components.
Let's verify the existence and setting of the BR2_PACKAGE_HASSIO_CHANNEL variable:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and setting of BR2_PACKAGE_HASSIO_CHANNEL
# Search for the definition of BR2_PACKAGE_HASSIO_CHANNEL
echo "Searching for BR2_PACKAGE_HASSIO_CHANNEL definition:"
rg -n "BR2_PACKAGE_HASSIO_CHANNEL" --type make
# Check if it's used in other places
echo "\nChecking usage of BR2_PACKAGE_HASSIO_CHANNEL in other files:"
rg -n "BR2_PACKAGE_HASSIO_CHANNEL" --type-not make
Length of output: 637
There was a problem hiding this comment.
Since this only affects the hassio package, I'll prefer having it as a (choice) config option there, so instead of passing a variable when running make, changing it would either mean to set it in the menuconfig or replace it in the .config (or just simply append it to the end). It's a little bit more complicated but IMO more appropriate.
Allow to set the release channel pre-installed Home Assistant components like Supervisor and add-on are fetched from. This channel is then also used at runtime.
agners
force-pushed
the
allow-to-select-hassio-source-channel
branch
from
503886f
to
e1f757c
Compare
agners
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
buildroot-external/package/hassio/Config.in (1)
24-25: Enhance the help text with available channel optionsThe current help text is minimal. Consider expanding it to list the available channel options (e.g., stable, beta) and their implications. This would help users make informed decisions.
help - Channel to use by default. + Channel to use by default for pre-installed Home Assistant components + (Supervisor and add-ons). Available options: + - stable: Production-ready releases + - beta: Pre-release versions for testingbuildroot-external/package/hassio/create-data-partition.sh (1)
28-28: Add error handling for the Docker commandThe script should handle potential errors when executing the Docker command.
Add error handling:
-docker exec "${container}" sh /build/dind-import-containers.sh "${channel}" +if ! docker exec "${container}" sh /build/dind-import-containers.sh "${channel}"; then + echo "Error: Failed to import containers for channel '${channel}'" + docker stop "${container}" || true + sudo umount "${build_dir}/data/" || true + exit 1 +fibuildroot-external/package/hassio/dind-import-containers.sh (1)
Line range hint
1-33: Consider architectural implications of channel configuration.The addition of channel configuration at the container import stage has important implications:
- This setting affects the entire system's update behavior
- Consider documenting the supported channel values and their implications
- Consider adding logging for the selected channel for debugging purposes
- Think about upgrade path and backward compatibility
Consider adding these features:
- Logging of the selected channel
- Documentation of supported channels
- Version compatibility checks
- Recovery mechanism if an invalid channel is specified
buildroot-external/package/hassio/hassio.mk (1)
18-18: Add timeout and retry logic to curl commandThe curl command should include timeout and retry logic to handle network issues gracefully. Also consider validating the JSON response.
Consider updating the command:
- curl -s $(HASSIO_VERSION_URL) | jq '.core = "landingpage"' > $(@D)/version.json + curl -s --retry 3 --retry-delay 2 --max-time 30 $(HASSIO_VERSION_URL) | jq 'if type=="object" then .core = "landingpage" else error("Invalid JSON") end' > $(@D)/version.json || (echo "Failed to fetch version info" >&2; exit 1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
buildroot-external/package/hassio/Config.in(1 hunks)buildroot-external/package/hassio/create-data-partition.sh(2 hunks)buildroot-external/package/hassio/dind-import-containers.sh(2 hunks)buildroot-external/package/hassio/hassio.mk(1 hunks)
🔇 Additional comments (5)
buildroot-external/package/hassio/Config.in (1)
21-26: Verify the intended default channel value
The implementation uses "stable" as the default channel, but the PR comments mentioned "beta" as the intended default. Please confirm which channel should be the default.
buildroot-external/package/hassio/create-data-partition.sh (1)
28-28: Verify the existence and permissions of the import script
Let's verify that the dind-import-containers.sh script exists and has the correct permissions.
✅ Verification successful
Let me check the actual content of the script to verify how it handles the channel parameter.
Script exists and is properly configured
The verification shows that:
- The script exists at the correct location with executable permissions (755)
- It properly accepts and uses the channel parameter
- The script's functionality is well-defined:
- Waits for Docker daemon
- Loads container images
- Tags supervisor image
- Sets up AppArmor
- Creates updater configuration with the provided channel
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the import script exists and has correct permissions
# Find the script
echo "Searching for dind-import-containers.sh..."
fd "dind-import-containers.sh"
# Check script permissions if found
if [ $? -eq 0 ]; then
echo "Checking script permissions..."
ls -l $(fd "dind-import-containers.sh")
fi
# Check if the script handles the channel parameter
echo "Checking script content for channel parameter handling..."
if [ -f $(fd "dind-import-containers.sh") ]; then
grep -A 5 "channel" $(fd "dind-import-containers.sh")
fi
Length of output: 1055
Script:
#!/bin/bash
# Check the full content of the script
cat $(fd "dind-import-containers.sh")
Length of output: 1150
buildroot-external/package/hassio/hassio.mk (3)
26-26: LGTM: Proper handling of container image fetching
The implementation correctly handles variable expansion, quoting, and path variables while fetching container images.
33-33: LGTM: Channel parameter properly passed to create-data-partition.sh
The implementation correctly passes the channel parameter to the script.
Let's verify how the script handles the new parameter:
#!/bin/bash
# Description: Check create-data-partition.sh for channel parameter handling
# Search for the script and show its channel parameter handling
echo "Checking create-data-partition.sh for channel parameter usage:"
fd "create-data-partition.sh$" --exec grep -A 5 -B 5 "channel"12-12: Verify BR2_PACKAGE_HASSIO_CHANNEL configuration
The variable needs to be properly defined in the Buildroot configuration system. Consider adding documentation about the available channel options.
Let's verify the variable definition and documentation:
✅ Verification successful
BR2_PACKAGE_HASSIO_CHANNEL is properly configured
The variable is correctly defined in buildroot-external/package/hassio/Config.in with a default value of "stable" and appropriate help text. It's properly used in the makefile and build scripts:
- Defined as a config option in
Config.in - Used in
hassio.mkfor version URL and data partition creation - Passed to build scripts that configure the supervisor
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for BR2_PACKAGE_HASSIO_CHANNEL definition and documentation
# Search for the variable definition in Config.in files
echo "Searching for BR2_PACKAGE_HASSIO_CHANNEL in Config.in files:"
fd "Config.in$" --exec grep -l "BR2_PACKAGE_HASSIO_CHANNEL"
# Check if the variable is documented in README or similar files
echo "\nChecking for documentation:"
rg -l "hassio.*channel|supervisor.*channel" -g "README*" -g "*.md"
Length of output: 735
Script:
#!/bin/bash
# Let's search more thoroughly for the channel configuration
# Search for BR2_PACKAGE_HASSIO_CHANNEL in Config.in files
echo "Searching for BR2_PACKAGE_HASSIO_CHANNEL in Config.in:"
rg "BR2_PACKAGE_HASSIO_CHANNEL" -A 5 -B 5
# Search for channel-related configuration in package/hassio directory
echo -e "\nChecking hassio package configuration:"
fd . "buildroot-external/package/hassio" -t f --exec cat {}
# Search for default channel value in any build scripts
echo -e "\nChecking for channel usage in build scripts:"
rg "channel.*=|CHANNEL.*=" -g "*.sh" -g "*.mk"
Length of output: 8410
| @@ -3,6 +3,7 @@ set -e | |||
|
|
|||
| build_dir=$1 | |||
| dst_dir=$2 | |||
| channel=$3 | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation for the channel argument
The script should validate that the required channel parameter is provided and contains a valid value.
Add this validation after the parameter declaration:
channel=$3
+
+# Validate channel parameter
+if [ -z "${channel}" ]; then
+ echo "Error: Channel parameter is required"
+ echo "Usage: $0 <build_dir> <dst_dir> <channel>"
+ exit 1
+fi
+
+# Validate channel value
+case "${channel}" in
+ stable|beta|dev) ;;
+ *)
+ echo "Error: Invalid channel '${channel}'. Must be one of: stable, beta, dev"
+ exit 1
+ ;;
+esac📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| channel=$3 | |
| channel=$3 | |
| # Validate channel parameter | |
| if [ -z "${channel}" ]; then | |
| echo "Error: Channel parameter is required" | |
| echo "Usage: $0 <build_dir> <dst_dir> <channel>" | |
| exit 1 | |
| fi | |
| # Validate channel value | |
| case "${channel}" in | |
| stable|beta|dev) ;; | |
| *) | |
| echo "Error: Invalid channel '${channel}'. Must be one of: stable, beta, dev" | |
| exit 1 | |
| ;; | |
| esac |
| channel=$1 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation and documentation.
The script now accepts a channel parameter but lacks validation and documentation. Consider these improvements:
#!/bin/sh
set -e
+# Usage: dind-import-containers.sh <channel>
+# <channel>: The release channel to use (e.g., "stable", "beta", "dev")
+
+if [ -z "$1" ]; then
+ echo "Error: Channel parameter is required"
+ echo "Usage: $0 <channel>"
+ exit 1
+fi
+
channel=$1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| channel=$1 | |
| #!/bin/sh | |
| set -e | |
| # Usage: dind-import-containers.sh <channel> | |
| # <channel>: The release channel to use (e.g., "stable", "beta", "dev") | |
| if [ -z "$1" ]; then | |
| echo "Error: Channel parameter is required" | |
| echo "Usage: $0 <channel>" | |
| exit 1 | |
| fi | |
| channel=$1 |
| @@ -27,3 +29,5 @@ docker tag "${supervisor}" "ghcr.io/home-assistant/${arch}-hassio-supervisor:lat | |||
| # Setup AppArmor | |||
| mkdir -p "/data/supervisor/apparmor" | |||
| wget -O "/data/supervisor/apparmor/hassio-supervisor" "${APPARMOR_URL}" | |||
|
|
|||
| echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json | |||
There was a problem hiding this comment.
Improve JSON file creation robustness and security.
The current implementation has several potential issues:
- No directory existence check
- No error handling
- Potential JSON injection vulnerability
- Hard-coded path
Consider this more robust implementation:
-echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json
+UPDATER_DIR="/data/supervisor"
+UPDATER_FILE="${UPDATER_DIR}/updater.json"
+
+# Ensure directory exists
+mkdir -p "${UPDATER_DIR}" || {
+ echo "Error: Failed to create directory ${UPDATER_DIR}"
+ exit 1
+}
+
+# Create JSON with proper escaping
+printf '{"channel":"%s"}\n' "$(echo "${channel}" | sed 's/[\"]/\\&/g')" > "${UPDATER_FILE}" || {
+ echo "Error: Failed to write to ${UPDATER_FILE}"
+ exit 1
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "{ \"channel\": \"${channel}\" }" > /data/supervisor/updater.json | |
| UPDATER_DIR="/data/supervisor" | |
| UPDATER_FILE="${UPDATER_DIR}/updater.json" | |
| # Ensure directory exists | |
| mkdir -p "${UPDATER_DIR}" || { | |
| echo "Error: Failed to create directory ${UPDATER_DIR}" | |
| exit 1 | |
| } | |
| # Create JSON with proper escaping | |
| printf '{"channel":"%s"}\n' "$(echo "${channel}" | sed 's/[\"]/\\&/g')" > "${UPDATER_FILE}" || { | |
| echo "Error: Failed to write to ${UPDATER_FILE}" | |
| exit 1 | |
| } |
There was a problem hiding this comment.
The title/commit message should be updated, as it's not a Makefile variable anymore. Otherwise LGTM.
I'll leave it up to you if you want to go with the suggestion below. It's quite a bit of boilerplate code but IMHO the experience of having the options in menuconfig is better and adds some error-proofing.
agners
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
buildroot-external/package/hassio/Config.in(1 hunks)buildroot-external/package/hassio/hassio.mk(1 hunks)
🔇 Additional comments (4)
buildroot-external/package/hassio/Config.in (1)
21-38: Well-structured choice configuration!
The implementation of the channel selection using Kconfig's choice mechanism is clean and follows best practices. The default is appropriately set to stable, and each option has clear help text.
buildroot-external/package/hassio/hassio.mk (3)
33-33: LGTM: Container image fetching looks good.
The script correctly uses the new version.json file and maintains proper parameter passing.
40-40: Verify create-data-partition.sh script updates.
The script now receives the channel parameter. Let's verify that the corresponding changes exist in the script.
✅ Verification successful
Script is properly updated to handle the channel parameter
The verification shows that create-data-partition.sh has been correctly updated to:
- Accept the channel parameter as $3
- Pass the channel parameter to dind-import-containers.sh during Docker execution
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if create-data-partition.sh is updated to handle the channel parameter
# Search for channel parameter handling in create-data-partition.sh
echo "Checking create-data-partition.sh for channel parameter handling:"
rg -A 5 "channel" "buildroot-external/package/hassio/create-data-partition.sh"
# Check if the script's usage/help text mentions the channel parameter
echo "\nChecking script usage text:"
head -n 20 "buildroot-external/package/hassio/create-data-partition.sh"
Length of output: 1078
13-18: Verify channel configuration definitions.
Let's ensure that the channel configuration variables are properly defined in Config.in.
✅ Verification successful
Channel configuration is properly defined and structured
The Config.in file correctly defines all the channel variables used in hassio.mk as a choice configuration:
BR2_PACKAGE_HASSIO_CHANNEL_STABLE(default)BR2_PACKAGE_HASSIO_CHANNEL_BETABR2_PACKAGE_HASSIO_CHANNEL_DEV
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify channel configuration definitions
# Search for channel configuration in Config.in
echo "Checking Config.in for channel definitions:"
rg "BR2_PACKAGE_HASSIO_CHANNEL" "buildroot-external/package/hassio/Config.in"
# Check if it's a choice configuration
echo "\nChecking for choice configuration:"
rg -A 10 "choice" "buildroot-external/package/hassio/Config.in"
Length of output: 743
| endchoice | ||
|
|
There was a problem hiding this comment.
Missing string config for channel value mapping
The choice configuration needs to be followed by a string config that maps the selected choice to actual channel values. This string config will be used by other scripts to determine the selected channel.
Add this configuration after the choice block:
endchoice
+
+config BR2_PACKAGE_HASSIO_CHANNEL
+ string
+ default "stable" if BR2_PACKAGE_HASSIO_CHANNEL_STABLE
+ default "beta" if BR2_PACKAGE_HASSIO_CHANNEL_BETA
+ default "dev" if BR2_PACKAGE_HASSIO_CHANNEL_DEV📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| endchoice | |
| endchoice | |
| config BR2_PACKAGE_HASSIO_CHANNEL | |
| string | |
| default "stable" if BR2_PACKAGE_HASSIO_CHANNEL_STABLE | |
| default "beta" if BR2_PACKAGE_HASSIO_CHANNEL_BETA | |
| default "dev" if BR2_PACKAGE_HASSIO_CHANNEL_DEV |
|
|
||
| HASSIO_CONTAINER_IMAGES_ARCH = supervisor dns audio cli multicast observer core | ||
|
|
||
| define HASSIO_CONFIGURE_CMDS | ||
| # Deploy only landing page for "core" by setting version to "landingpage" | ||
| curl -s $(HASSIO_VERSION_URL) | jq '.core = "landingpage"' > $(@D)/stable.json | ||
| curl -s $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE) | jq '.core = "landingpage"' > $(@D)/version.json |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for curl command.
The curl command should handle potential network failures or invalid responses to prevent silent build failures.
Enhance the command with error handling:
-curl -s $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE) | jq '.core = "landingpage"' > $(@D)/version.json
+curl -f -s $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE) | jq '.core = "landingpage"' > $(@D)/version.json || \
+ (echo "Error: Failed to fetch version file from $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE)" >&2; exit 1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -s $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE) | jq '.core = "landingpage"' > $(@D)/version.json | |
| curl -f -s $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE) | jq '.core = "landingpage"' > $(@D)/version.json || \ | |
| (echo "Error: Failed to fetch version file from $(HASSIO_VERSION_URL)$(HASSIO_VERSION_FILE)" >&2; exit 1) |
Allow to set the release channel pre-installed Home Assistant components like Supervisor and add-on are fetched from. This channel is then also used at runtime.
Summary by CodeRabbit
New Features
Improvements