feat: add script to build from source and support binary path with install.sh #58
Conversation
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nice addition overall — BINARY_DIR support is a useful escape hatch.
A couple small robustness tweaks would make the scripts easier to debug and safer to run:
- Consider
set -euo pipefail+ making theOUTPUT_DIRdefaulting safe with-u. - Truncate
build.logonce per run so failures don’t get buried in previous output. - In
install.sh’sBINARY_DIRpath, validate expected files exist before copying to produce clearer errors.
scripts/build-from-source.sh
Outdated
| # OUTPUT_DIR - Full path of directory to place built binaries (optional, default: $(pwd)/bin) | ||
| # | ||
|
|
||
| set -e |
There was a problem hiding this comment.
Consider enabling pipefail here so failures in piped commands don’t get masked. If you do add -u, you’ll also want the OUTPUT_DIR check below to be safe with unset vars.
| set -e | |
| set -euo pipefail |
scripts/build-from-source.sh
Outdated
| if [ -z "$OUTPUT_DIR" ]; then | ||
| OUTPUT_DIR=${SOURCE_DIR}/bin | ||
| fi |
There was a problem hiding this comment.
If you enable set -u, this if [ -z "$OUTPUT_DIR" ] pattern will error on an unset variable. This alternative is robust and a bit shorter.
| if [ -z "$OUTPUT_DIR" ]; then | |
| OUTPUT_DIR=${SOURCE_DIR}/bin | |
| fi | |
| : "${OUTPUT_DIR:=${SOURCE_DIR}/bin}" |
| # Create output directory if it doesn't exist | ||
| mkdir -p "$OUTPUT_DIR" | ||
|
|
||
| BUILD_LOG="${OUTPUT_DIR}/build.log" |
There was a problem hiding this comment.
Minor: >> "$BUILD_LOG" appends across runs, which can make failures confusing. Truncating once at the start keeps the log scoped to the current build.
| BUILD_LOG="${OUTPUT_DIR}/build.log" | |
| BUILD_LOG="${OUTPUT_DIR}/build.log" | |
| : > "$BUILD_LOG" |
| info "Copying binaries from ${BINARY_DIR}..." | ||
| cp "${BINARY_DIR}/${BINARY_NAME}" "${TMP_DIR}/${BINARY_NAME}" | ||
| cp "${BINARY_DIR}/hypeman-token" "${TMP_DIR}/hypeman-token" | ||
| cp "${BINARY_DIR}/.env.example" "${TMP_DIR}/.env.example" | ||
|
|
There was a problem hiding this comment.
Since BINARY_DIR is user-provided, validating the expected files exist before cp makes failures a lot clearer.
| info "Copying binaries from ${BINARY_DIR}..." | |
| cp "${BINARY_DIR}/${BINARY_NAME}" "${TMP_DIR}/${BINARY_NAME}" | |
| cp "${BINARY_DIR}/hypeman-token" "${TMP_DIR}/hypeman-token" | |
| cp "${BINARY_DIR}/.env.example" "${TMP_DIR}/.env.example" | |
| # Copy binaries to TMP_DIR | |
| info "Copying binaries from ${BINARY_DIR}..." | |
| for f in "${BINARY_NAME}" hypeman-token .env.example; do | |
| [ -f "${BINARY_DIR}/${f}" ] || error "Missing ${f} in BINARY_DIR: ${BINARY_DIR}" | |
| done | |
| cp "${BINARY_DIR}/${BINARY_NAME}" "${TMP_DIR}/${BINARY_NAME}" | |
| cp "${BINARY_DIR}/hypeman-token" "${TMP_DIR}/hypeman-token" | |
| cp "${BINARY_DIR}/.env.example" "${TMP_DIR}/.env.example" |
Sayan-
left a comment
Sayan-
left a comment
There was a problem hiding this comment.
bot comments seem relevant - particularly the file validation before cp in the BINARY_DIR path.
separately, what's the relation between install.sh and build-from-source.sh? the build logic (make build + go build gen-jwt + copy .env.example) now exists in both scripts. wondering if BRANCH mode in install.sh could delegate to build-from-source.sh to reduce duplication.
scripts/build-from-source.sh
Outdated
| # Usage: | ||
| # ./scripts/build-from-source.sh | ||
| # | ||
| # Options (via environment variables: |
There was a problem hiding this comment.
nit: missing closing parenthesis - should be (via environment variables):
scripts/build-from-source.sh
Outdated
| # ./scripts/build-from-source.sh | ||
| # | ||
| # Options (via environment variables: | ||
| # OUTPUT_DIR - Full path of directory to place built binaries (optional, default: $(pwd)/bin) |
There was a problem hiding this comment.
nit: comment says $(pwd)/bin but the actual default is ${SOURCE_DIR}/bin (the repo root's bin dir, not cwd). might want to clarify.
There was a problem hiding this comment.
:nod: yep I changed the default but forgot to update comment
|
|
||
| # Count how many of BRANCH, VERSION, BINARY_DIR are set | ||
| count=0 | ||
| [ -n "$BRANCH" ] && ((count++)) || true |
There was a problem hiding this comment.
heads up: if you adopt the bot's suggestion for set -u, these [ -n "$BRANCH" ] checks will fail on unset vars. would need ${BRANCH:-} syntax.
that is actually how I had it originally but then noticed that we suggest I was thinking if |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This is used by
Note
Adds tooling for building and installing Hypeman with more flexible paths.
scripts/build-from-source.sh: Buildshypeman-apiandhypeman-token, writes logs tobuild.log, validates absoluteOUTPUT_DIR, and copies.env.exampleto the output dirscripts/install.sh): AddsBINARY_DIRmode to install from prebuilt binaries (with validation and exec perms), makesBRANCH,VERSION, andBINARY_DIRmutually exclusive with checks, and treatsCLI_VERSION=latestas auto-resolve; keeps build-from-source and release-download paths intactWritten by Cursor Bugbot for commit ebef7e8. This will update automatically on new commits. Configure here.