Skip to content

Conversation

@Sarthak-Shaha
Copy link
Contributor

@Sarthak-Shaha Sarthak-Shaha commented Nov 14, 2025

Note

Introduce a new tools Docker image and refactor build workflow/scripts to toggle between SDKs and tools builds with new inputs.

  • CI/Workflow:
    • Add workflow_dispatch inputs: push_images (default true) and tools (default true).
    • Only compute VERSION when not building tools; upgrade checkout to actions/checkout@v5.
    • Build and push tools image via docker/tools/build.sh tagged 25Q4-Tools.
  • Docker build scripts:
    • Replace docker/build.sh to delegate to docker/sdks/build.sh or docker/tools/build.sh based on tools input.
    • Add docker/sdks/build.sh to build/push ghcr.io/siliconlabssoftware/matter_extension_dependencies using parsed SDK tags.
    • Add tools image:
      • New docker/tools/Dockerfile to install SLT, conan engine, SLC CLI, Commander, and ARM GCC.
      • New docker/tools/build.sh and docker/tools/requirements.txt.

Written by Cursor Bugbot for commit a2446dc. This will update automatically on new commits. Configure here.

@Sarthak-Shaha Sarthak-Shaha requested a review from a team as a code owner November 14, 2025 14:15
docker build \
--build-arg SISDK_Tag="$SISDK_Tag" \
--build-arg WiFI_SDK_Tag="$WiFI_SDK_Tag" \
-f docker/Dockerfile \
Copy link

Choose a reason for hiding this comment

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

Bug: Dockerfile Path Mismatch

The Dockerfile path -f docker/Dockerfile is incorrect. Based on the file structure, it should be -f docker/sdks/Dockerfile to reference the SDK-specific Dockerfile in the docker/sdks/ subdirectory.

Fix in Cursor Fix in Web

ln -s "$CONAN_PATH" "$INSTALL_PATH/conan" \
else \
echo "Error: conan not found at $CONAN_PATH" \
fi \
Copy link

Choose a reason for hiding this comment

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

Bug: Missing dependency doesn't halt build.

When conan is not found at the expected path, the error message is printed but the script continues without exiting (missing exit 1), unlike the conan_engine check above. This inconsistency means the build continues even when conan installation fails.

Fix in Cursor Fix in Web

if: ${{ github.event.inputs.tools == false }}
run: |
chmod +x docker/sdks/build.sh
./docker/sdks/build.sh --tag "$VERSION" --push
Copy link

Choose a reason for hiding this comment

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

Bug: Unintended Image Pushing

The build step always passes --push flag regardless of the push_images input value. The workflow defines a push_images input but never uses it, so images are always pushed even when push_images is false.

Fix in Cursor Fix in Web

./docker/build.sh --tag "$VERSION"
fi
chmod +x docker/tools/build.sh
./docker/tools/build.sh --tag "25Q4-Tools" --push
Copy link

Choose a reason for hiding this comment

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

Bug: Push control ignored

The build step always passes --push flag regardless of the push_images input value. The workflow defines a push_images input but never uses it, so images are always pushed even when push_images is false.

Fix in Cursor Fix in Web

./docker/build.sh --tag "$VERSION"
fi
chmod +x docker/tools/build.sh
./docker/tools/build.sh --tag "25Q4-Tools" --push
Copy link

Choose a reason for hiding this comment

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

Bug: Build Workflow Ignores Config, Incomplete Images.

The workflow unconditionally builds only the tools image with a hardcoded tag, ignoring both the push_images input parameter and the VERSION environment variable set in the previous step. This prevents building SDK images and makes the conditional version parsing logic unused.

Fix in Cursor Fix in Web

if [[ "${{ github.event.inputs.tools }}" == "false" ]]; then
./docker/sdks/build.sh --tag "$VERSION"
else
./docker/tools/build.sh --tag "$VERSION"
Copy link

Choose a reason for hiding this comment

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

Bug: Fragile External Variable Dependency

The $VERSION variable is referenced but never defined in the script. The script expects this variable to be set in the environment, but there's no guarantee it will be available when the script runs.

Fix in Cursor Fix in Web

VERSION="SiSDK${SISDK_Tag}_WiFi_SDK${WiFI_SDK_Tag}"
echo "VERSION=$VERSION" >> $GITHUB_ENV
echo "Parsed VERSION: $VERSION"
fi
Copy link

Choose a reason for hiding this comment

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

Bug: Version Mismatch: Hardcoded Tag Overrides Computation

The VERSION environment variable is set conditionally but never used. The subsequent build step at line 55 uses a hardcoded tag "25Q4-Tools" instead of the computed VERSION, making this entire step pointless and wasting CI resources parsing the version file.

Fix in Cursor Fix in Web

if [[ "${{ github.event.inputs.tools }}" == "false" ]]; then
./docker/sdks/build.sh --tag "$VERSION"
else
./docker/tools/build.sh --tag "$VERSION"
Copy link

Choose a reason for hiding this comment

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

Bug: Incorrect Syntax and Undefined Variable Break Script

The script uses GitHub Actions template syntax ${{ github.event.inputs.tools }} which only works within GitHub Actions workflow files. When executed as a standalone shell script, this becomes a literal string that never equals "false", causing the else branch to always execute. Additionally, the VERSION variable is referenced but never defined, causing the script to pass an empty tag argument.

Fix in Cursor Fix in Web

@Sarthak-Shaha Sarthak-Shaha marked this pull request as draft November 17, 2025 19:40
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.

2 participants