Cleanup packaging scripts#2782
Conversation
This file is created by the `bosh_bundle_local` function used to install gems during packaging, but because the function is not executed while in `$BOSH_INSTALL_TARGET` the `.bundle/` directory is not present in the package.
This appears to be a vestage from when Ruby shipped with a default json gem that was > 2.0. This is no longer the case.
WalkthroughThe director, health monitor, and NATS packaging scripts now discover gemspecs with glob-based loops, build gems from their package directories, and move generated gems into 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/director/packaging (1)
10-16: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPass the gemspec file to
gem buildin all three packaging scripts.RubyGems expects a gemspec argument, so these loops will exit on the first iteration with
set -e.
packages/director/packaging#L10-L16:gem build "$(basename "$gemspec")"packages/health_monitor/packaging#L7-L13:gem build "$(basename "$gemspec")"packages/nats/packaging#L11-L17:gem build "$(basename "$gemspec")"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/director/packaging` around lines 10 - 16, Update the gem build loops in packages/director/packaging lines 10-16, packages/health_monitor/packaging lines 7-13, and packages/nats/packaging lines 11-17 to pass the current gemspec filename to gem build using the existing gemspec variable and basename handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/director/packaging`:
- Around line 10-16: Update the gem build loops in packages/director/packaging
lines 10-16, packages/health_monitor/packaging lines 7-13, and
packages/nats/packaging lines 11-17 to pass the current gemspec filename to gem
build using the existing gemspec variable and basename handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3590585f-42e8-4bca-aeff-8b42a02de434
📒 Files selected for processing (3)
packages/director/packagingpackages/health_monitor/packagingpackages/nats/packaging
There was a problem hiding this comment.
Pull request overview
This PR cleans up BOSH release package packaging scripts for nats, health_monitor, and director, aiming to simplify directory creation, gem building, and what gets copied into ${BOSH_INSTALL_TARGET}.
Changes:
- Consolidate install-target directory creation and improve quoting.
- Refactor gem build loop to iterate over gemspecs and populate
vendor/cache. - Copy
Gemfile,Gemfile.lock, and.bundleinto${BOSH_INSTALL_TARGET}in one command.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/nats/packaging | Refactors gem build + caching flow; adjusts install target setup and file copying. |
| packages/health_monitor/packaging | Refactors gem build + caching flow; adjusts install target setup and file copying. |
| packages/director/packaging | Refactors gem build + caching flow; adjusts install target setup and file copying. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for gemspec in ./*/*.gemspec; do | ||
| gem_dir="$(dirname "${gemspec}")" | ||
|
|
||
| pushd "${gem_dir}" | ||
| gem build | ||
| popd | ||
| done | ||
| mv ./*/*.gem vendor/cache/ |
| pushd "${gem_dir}" | ||
| gem build | ||
| popd | ||
| done | ||
| mv ./*/*.gem vendor/cache/ |
| pushd "${gem_dir}" | ||
| gem build | ||
| popd | ||
| done | ||
| mv ./*/*.gem vendor/cache/ |
No description provided.