Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 2 additions & 36 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,7 @@ defaults:
shell: bash

jobs:
build:
strategy:
matrix:
include:
- runs-on: ubuntu-latest
arch: amd64
- runs-on: ubuntu-24.04-arm
arch: arm64
runs-on: ${{ matrix.runs-on }}
permissions:
contents: read
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: ${{ github.event_name == 'workflow_dispatch' && inputs.ref || github.ref }}

- name: Install build dependencies
run: sudo apt-get update && sudo apt-get install -y --no-install-recommends libudev-dev libdbus-1-dev

- name: Build binary
run: cargo build --package stellar-cli --release

- name: Upload binary
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: stellar-${{ matrix.arch }}
path: target/release/stellar
retention-days: 1

docker:
needs: build
runs-on: ubuntu-latest
permissions:
contents: read
Expand All @@ -57,12 +27,6 @@ jobs:
ref: ${{ github.event_name == 'workflow_dispatch' && inputs.ref || github.ref }}
fetch-depth: 0

- name: Download binaries
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
pattern: stellar-*
merge-multiple: false

- name: Set up QEMU
uses: docker/setup-qemu-action@ce360397dd3f832beb865e1373c09c0e9f86d70a # v4

Expand Down Expand Up @@ -100,6 +64,8 @@ jobs:
platforms: linux/amd64,linux/arm64
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-arch builds now run under QEMU emulation. The previous workflow built arm64 natively on an ubuntu-24.04-arm runner. With the build matrix removed, the linux/arm64 image is now compiled from source inside QEMU on an amd64 host. Building Rust under QEMU is typically 10–30× slower than native — for a project the size of stellar-cli this can mean very long builds (potentially exceeding the default 6-hour job timeout) and significant CI cost.

Consider keeping the multi-arch matrix and using docker/build-push-action per-arch on native runners, then assembling a manifest with docker buildx imagetools create. Or use buildx with platforms: linux/amd64 on ubuntu-latest and platforms: linux/arm64 on ubuntu-24.04-arm and merge.

Also worth adding cache-from/cache-to (e.g. GHA cache) to avoid re-compiling everything on every push.

push: true
tags: ${{ env.DOCKER_TAGS }}
build-args: |
STELLAR_CLI_REV=${{ github.event_name == 'workflow_dispatch' && inputs.ref || github.ref_name }}
Comment on lines +67 to +68
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For workflow_dispatch, inputs.ref can be a branch, tag, or SHA. For release, github.ref_name is the tag (e.g. v1.2.3). Both are forwarded to cargo install --rev in the Dockerfile, which expects a commit SHA (see Dockerfile comment).

Since the workflow already does actions/checkout at the right ref, the simplest fix is to resolve to a SHA here:

- name: Resolve ref to SHA
  id: rev
  run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"

# ...
build-args: |
  STELLAR_CLI_REV=${{ steps.rev.outputs.sha }}

This makes the build reproducible and avoids the tag-vs-rev ambiguity.

Comment on lines +67 to +68
Comment on lines +67 to +68

- name: Update Docker Hub description
run: |
Expand Down
17 changes: 14 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,26 @@ FROM rust:latest
RUN rustup target add wasm32v1-none

RUN apt-get update && \
apt-get install -y --no-install-recommends ca-certificates libdbus-1-3 libssl3 libudev1 && \
apt-get install -y --no-install-recommends \
build-essential \
ca-certificates \
git \
libdbus-1-dev \
libssl-dev \
libudev-dev \
pkg-config && \
rm -rf /var/lib/apt/lists/*

ARG TARGETARCH
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARG TARGETARCH is no longer used (it was only needed to pick the pre-built binary in stellar-${TARGETARCH}/stellar). It can be removed.

COPY stellar-${TARGETARCH}/stellar /usr/local/bin/stellar
RUN chmod +x /usr/local/bin/stellar
ARG STELLAR_CLI_REV
RUN cargo install --locked \
--git https://github.com/stellar/stellar-cli.git \
--rev "${STELLAR_CLI_REV}" \
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Resolve branch refs before passing them to --rev

For workflow_dispatch runs where ref is a branch name (including the default main), the workflow passes that string through as STELLAR_CLI_REV, but Cargo documents --branch <BRANCH> separately from --rev <SHA> and cargo install --git ... --rev testbranch fails with revspec 'testbranch' not found. This means the manual Docker workflow's default branch build fails in the Dockerfile before pushing; resolve the checked-out ref to a commit SHA for this build arg, or use --branch when the input is a branch.

Useful? React with 👍 / 👎.

stellar-cli
Comment on lines +18 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--rev may break release builds. cargo install --rev is intended for commit SHAs, while the workflow passes github.ref_name for release events — that is the tag name (e.g. v23.1.4). Cargo docs are explicit that --branch, --tag, and --rev are separate selectors with different semantics; relying on --rev to accept a tag name is brittle and may silently break.

Consider resolving the input to a SHA in the workflow before passing it as STELLAR_CLI_REV (e.g. git rev-parse HEAD after checkout, since the workflow already checks out the right ref), or choose the appropriate flag based on whether the input is a tag/branch/SHA.

There is also no default or guard: if someone builds the image locally without --build-arg STELLAR_CLI_REV=..., this becomes cargo install --rev "" stellar-cli, which fails with a confusing error. Adding ARG STELLAR_CLI_REV=main (or a check) would make local builds work and document the intent.

Comment on lines +18 to +21

ENV STELLAR_CONFIG_HOME=/config
ENV STELLAR_DATA_HOME=/data
ENV STELLAR_NO_UPDATE_CHECK=1
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: setting STELLAR_NO_UPDATE_CHECK=1 is a reasonable change for a containerized binary, but it's unrelated to the stated purpose of the PR (build from source). Worth either calling out in the description or splitting into its own commit so the history is clearer.


COPY entrypoint.sh /usr/local/bin/entrypoint.sh

Expand Down
Loading