chore(build): move common functions to longhorn/dep-versions #456
Conversation
Longhorn 10208 Signed-off-by: Derek Su <[email protected]>
WalkthroughThis pull request introduces two new build arguments, Changes
Sequence Diagram(s)sequenceDiagram
participant M as Makefile
participant S as Shell
participant D as Docker Build
M->>S: Execute command to fetch current branch and tag
S-->>M: Return current branch and latest Git tag
M->>D: Initiate docker build with SRC_BRANCH and SRC_TAG as build args
D->>D: Set ENV variables (SRC_BRANCH, SRC_TAG) from build args
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
scripts/package (1)
32-39: 💡 Verification agent🧩 Analysis chain
Verify SRC_BRANCH and SRC_TAG are properly set.
The script uses
SRC_BRANCHandSRC_TAGin the build command, but these variables are not defined within the script. Ensure these variables are properly set before use to avoid empty values in the build process.Run this script to check if the variables are properly set:
🏁 Script executed:
#!/bin/bash # Description: Check if SRC_BRANCH and SRC_TAG are properly set in the build environment # Test: Check if variables are set in the environment echo "Checking environment variables..." env | grep -E "^(SRC_BRANCH|SRC_TAG)=" # Test: Check if variables are set in the Makefile echo -e "\nChecking Makefile exports..." make -n | grep -E "export (SRC_BRANCH|SRC_TAG)"Length of output: 336
Action Required: Define Environment Variables SRC_BRANCH and SRC_TAG
- The environment check didn’t return any values for SRC_BRANCH and SRC_TAG, meaning they are not set.
- Additionally, the attempt to inspect Makefile exports was inconclusive due to the "make" command not being available.
Please ensure that these variables are explicitly defined (or have default values) before invoking the build command to avoid potential issues during the build process.
🧹 Nitpick comments (1)
Dockerfile.dapper (1)
4-5: Consider adding SRC_BRANCH and SRC_TAG to DAPPER_ENV.The new environment variables
SRC_BRANCHandSRC_TAGshould be added toDAPPER_ENVto ensure they are properly passed through to the dapper environment.Apply this diff to update
DAPPER_ENV:-ENV DAPPER_ENV TAG REPO DRONE_REPO DRONE_PULL_REQUEST DRONE_COMMIT_REF +ENV DAPPER_ENV TAG REPO DRONE_REPO DRONE_PULL_REQUEST DRONE_COMMIT_REF SRC_BRANCH SRC_TAGAlso applies to: 16-17
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Dockerfile.dapper(2 hunks)Makefile(1 hunks)scripts/package(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build binaries
- GitHub Check: Summary
|
@mergify backport v1.8.x |
✅ Backports have been created
|
Which issue(s) this PR fixes:
Issue longhorn/longhorn#10208
What this PR does / why we need it:
Special notes for your reviewer:
Additional documentation or context