chore: configure and use LOCAL_RPM_REPO for RPM artifacts#35980
chore: configure and use LOCAL_RPM_REPO for RPM artifacts#35980
Conversation
Define LOCAL_RPM_REPO environment variable in the Makefile to standardize RPM output directory.
esolitos
force-pushed
the
marlon/chore/configure-local-rpm-repo
branch
from
6a204a0 to
c4b0863
Compare
There was a problem hiding this comment.
Pull request overview
This PR standardizes where Buildkite jobs write and read RPM build artifacts by introducing a LOCAL_RPM_REPO environment variable (analogous to LOCAL_MVN_REPO) and updating CI scripts to use it consistently.
Changes:
- Add
LOCAL_RPM_REPO(defaulting to$WORKDIR/artifacts/$ARCH/rpms) to the Buildkite Makefile exports. - Update Buildkite steps to create, populate, consume, and publish RPM repo artifacts using
LOCAL_RPM_REPO. - Adjust artifact publishing to tar the Maven and RPM repositories based on the configured repo paths.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| .buildkite/Makefile | Exports LOCAL_RPM_REPO with a default RPM artifacts directory. |
| .buildkite/prepare.sh | Creates artifact directories using LOCAL_RPM_REPO / LOCAL_MVN_REPO. |
| .buildkite/build-rpms.sh | Moves built RPMs into LOCAL_RPM_REPO and runs createrepo there; adds a missing-var check. |
| .buildkite/build-container.sh | Copies RPM repo from LOCAL_RPM_REPO into the container build context. |
| .buildkite/basic-search-test.sh | Points the yum repo baseurl at LOCAL_RPM_REPO. |
| .buildkite/publish-artifacts.sh | Archives RPM and Maven repos based on configured local repo paths and copies a specific RPM from LOCAL_RPM_REPO. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Preparing RPMs for container build..." | ||
| rm -rf "${WORKDIR}/docker-image/rpms" | ||
| cp -a "${WORKDIR}/artifacts/$ARCH/rpms" "${WORKDIR}/docker-image/" | ||
| # Ensure clean state for rpms directory | ||
| rm -rf "${WORKDIR}/docker-image/rpms" && mkdir -p "${WORKDIR}/docker-image/rpms" | ||
| # Note: Appending "./" ensures that the directory's contents are copied, rather than the directory itself. | ||
| cp -a "${LOCAL_RPM_REPO}/." "${WORKDIR}/docker-image/rpms/" |
There was a problem hiding this comment.
This script now relies on LOCAL_RPM_REPO (line 46) but doesn’t validate it like the other Buildkite step scripts. With set -o nounset, a missing variable will fail with a generic “unbound variable” error at the cp line. Add an explicit : "${LOCAL_RPM_REPO:?…}" precondition near the top for a clearer failure mode (and to match the pattern used elsewhere in this PR).
gjoranv
left a comment
gjoranv
left a comment
There was a problem hiding this comment.
Looks good! One minor consistency issue noted below.
| # Ensure clean state for rpms directory | ||
| rm -rf "${WORKDIR}/docker-image/rpms" && mkdir -p "${WORKDIR}/docker-image/rpms" | ||
| # Note: Appending "./" ensures that the directory's contents are copied, rather than the directory itself. | ||
| cp -a "${LOCAL_RPM_REPO}/." "${WORKDIR}/docker-image/rpms/" |
There was a problem hiding this comment.
All other scripts in this PR add a : "${LOCAL_RPM_REPO:?...}" validation guard at the top, but this script is missing it. Since the script uses set -o nounset, a missing variable would produce an unhelpful "unbound variable" error at the cp line instead of a clear message. Consider adding it for consistency:
: "${LOCAL_RPM_REPO:?Environment variable LOCAL_RPM_REPO must be set (path to local RPM repo)}"There was a problem hiding this comment.
Good point. I have another PR coming with a general : "${VAR:?...} cleanup, so I'll apply this suggestion in there.
Define LOCAL_RPM_REPO environment variable in the Makefile to standardize RPM output directory.