-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Block broken lorax versions #152
Conversation
/run tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine!
Except it doesn't work: https://github.com/JasonN3/build-container-installer/actions/runs/12419915429/job/34677378879 |
@coderabbitai review |
📝 WalkthroughWalkthroughThe pull request introduces modifications to several GitHub Actions workflow files and an external Makefile. The changes primarily focus on enhancing job context and logging details in the workflows by including additional matrix parameters like Changes
Sequence DiagramsequenceDiagram
participant Workflow as GitHub Actions Workflow
participant Matrix as Matrix Configuration
participant Job as Job Execution
Matrix->>Workflow: Provide image_repo and image_name
Workflow->>Job: Incorporate matrix variables in job context
Job->>Workflow: Execute with enhanced logging details
The sequence diagram illustrates how the matrix configuration now provides additional variables that are incorporated into the workflow's job context, enabling more detailed logging and traceability. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
external/Makefile (1)
3-3
: Add a comment explaining the version block.Add a comment above the line explaining why lorax-40.5 is blocked to prevent future confusion and maintain institutional knowledge.
lorax/branch-$(VERSION): git config advice.detachedHead false + # Block lorax-40.5 as it can remove /usr/sbin/ldconfig which breaks pyudev functionality cd lorax && git reset --hard HEAD && git checkout $(if $(_RHEL),rhel$(word 1,$(subst ., ,$(VERSION)))-branch,tags/$(shell cd lorax && git tag -l lorax-$(VERSION).* --sort=creatordate | grep -v 'lorax-40.5' | tail -n 1))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build_iso.yml
(3 hunks).github/workflows/test_deployment.yml
(3 hunks).github/workflows/test_iso.yml
(3 hunks)external/Makefile
(1 hunks)
🔇 Additional comments (4)
.github/workflows/test_iso.yml (1)
55-55
: LGTM! Enhanced job context improves observability.The addition of
image_repo
andimage_name
to the job context strings provides better traceability without affecting the core functionality.Also applies to: 64-64, 101-101
.github/workflows/test_deployment.yml (1)
55-55
: LGTM! Consistent with test_iso.yml changes.The job context string updates match those in test_iso.yml, maintaining consistency across workflows.
Also applies to: 64-64, 110-110
.github/workflows/build_iso.yml (2)
56-56
: LGTM: Job context enrichment looks good.The changes consistently enhance job naming and status context by including
matrix.image_repo
andmatrix.image_name
parameters across all relevant steps.Also applies to: 65-65, 141-141
Line range hint
1-143
: Verify the implementation of lorax version blocking.While this workflow file's changes look correct, they don't address the PR's primary objective of blocking broken lorax versions that could remove
/usr/sbin/ldconfig
. The implementation for this critical fix should be verified.Let's search for the lorax-related changes:
✅ Verification successful
Lorax version 40.5 is correctly blocked in the implementation
The verification confirms that the problematic lorax version 40.5 is properly blocked in
external/Makefile
. The implementation usesgrep -v 'lorax-40.5'
to explicitly exclude version 40.5 when selecting the lorax version to use:cd lorax && git tag -l lorax-$(VERSION).* --sort=creatordate | grep -v 'lorax-40.5' | tail -n 1This ensures that even if lorax version 40.5 is available, it will be filtered out during the version selection process, effectively preventing the use of the broken version that could remove
/usr/sbin/ldconfig
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for lorax-related changes and version constraints # Expected: Find implementation that blocks problematic lorax versions # Search for lorax-related files and their content echo "=== Files containing lorax references ===" rg -l 'lorax' echo -e "\n=== Lorax version constraints or tag filtering ===" rg -i 'lorax.*(version|tag|4[0-9]\.5)' echo -e "\n=== Makefile changes related to lorax ===" rg 'lorax' -A 5 -B 5 '*.mk' 'Makefile*'Length of output: 1151
Script:
#!/bin/bash # Let's examine the external/Makefile more closely to understand the lorax version blocking implementation echo "=== Examining lorax version blocking in external/Makefile ===" cat external/Makefile echo -e "\n=== Checking git history for recent lorax-related changes ===" git log --oneline -p -- external/Makefile | grep -A 10 -B 10 'lorax'Length of output: 822
Some versions of lorax will remove all files installed by the package
glibc
into the directory/usr/sbin
. This results in/sbin/ldconfig
being removed because/sbin
is a link to/usr/sbin
. pyudev usesctypes.util.find_library
to find libudev and that function uses /sbin/ldconfig -p` to find the library.Fixes: #151
Summary by CodeRabbit
Workflow Updates
Build Configuration