diff --git a/.bootc-dev-infra-commit.txt b/.bootc-dev-infra-commit.txt index c945bb1..1da9243 100644 --- a/.bootc-dev-infra-commit.txt +++ b/.bootc-dev-infra-commit.txt @@ -1 +1 @@ -3249ff02e990cb856da25dcf44add398202088c0 +d5a5a62c9810a416e4cc98f377c05343393f7c14 diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md new file mode 120000 index 0000000..be77ac8 --- /dev/null +++ b/.claude/CLAUDE.md @@ -0,0 +1 @@ +../AGENTS.md \ No newline at end of file diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..26e62a2 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,28 @@ +{ + "name": "bootc-devenv-debian", + // TODO override this back to prod image + "image": "ghcr.io/bootc-dev/devenv-debian", + "customizations": { + "vscode": { + // Abitrary, but most of our code is in one of these two + "extensions": [ + "rust-lang.rust-analyzer", + "golang.Go" + ] + } + }, + "features": {}, + "runArgs": [ + // Because we want to be able to run podman and also use e.g. /dev/kvm + // among other things + "--privileged" + ], + "postCreateCommand": { + // Our init script + "devenv-init": "sudo /usr/local/bin/devenv-init.sh" + }, + "remoteEnv": { + "PATH": "${containerEnv:PATH}:/usr/local/cargo/bin" + } +} + diff --git a/.gemini/config.yaml b/.gemini/config.yaml index 3c7c033..080ba11 100644 --- a/.gemini/config.yaml +++ b/.gemini/config.yaml @@ -1,3 +1,7 @@ +# NOTE: This file is canonically maintained in +# +# DO NOT EDIT +# # This config mainly overrides `summary: false` by default # as it's really noisy. have_fun: true diff --git a/.github/actions/bootc-ubuntu-setup/action.yml b/.github/actions/bootc-ubuntu-setup/action.yml new file mode 100644 index 0000000..ad39f78 --- /dev/null +++ b/.github/actions/bootc-ubuntu-setup/action.yml @@ -0,0 +1,92 @@ +name: 'Bootc Ubuntu Setup' +description: 'Default host setup' +inputs: + libvirt: + description: 'Install libvirt and virtualization stack' + required: false + default: 'false' +runs: + using: 'composite' + steps: + # The default runners have TONS of crud on them... + - name: Free up disk space on runner + shell: bash + run: | + set -xeuo pipefail + sudo df -h + unwanted_pkgs=('^aspnetcore-.*' '^dotnet-.*' '^llvm-.*' 'php.*' '^mongodb-.*' '^mysql-.*' + azure-cli google-chrome-stable firefox mono-devel) + unwanted_dirs=(/usr/share/dotnet /opt/ghc /usr/local/lib/android /opt/hostedtoolcache/CodeQL) + # Start background removal operations as systemd units; if this causes + # races in the future around disk space we can look at waiting for cleanup + # before starting further jobs, but right now we spent a lot of time waiting + # on the network and scripts and such below, giving these plenty of time to run. + n=0 + runcleanup() { + sudo systemd-run -r -u action-cleanup-${n} -- "$@" + n=$(($n + 1)) + } + runcleanup docker image prune --all --force + for x in ${unwanted_dirs[@]}; do + runcleanup rm -rf "$x" + done + # Apt removals in foreground, as we can't parallelize these + for x in ${unwanted_pkgs[@]}; do + /bin/time -f '%E %C' sudo apt-get remove -y $x + done + # We really want support for heredocs + - name: Update podman and install just + shell: bash + run: | + set -eux + # Require the runner is ubuntu-24.04 + IDV=$(. /usr/lib/os-release && echo ${ID}-${VERSION_ID}) + test "${IDV}" = "ubuntu-24.04" + # plucky is the next release + echo 'deb http://azure.archive.ubuntu.com/ubuntu plucky universe main' | sudo tee /etc/apt/sources.list.d/plucky.list + /bin/time -f '%E %C' sudo apt update + # skopeo is currently older in plucky for some reason hence --allow-downgrades + /bin/time -f '%E %C' sudo apt install -y --allow-downgrades crun/plucky podman/plucky skopeo/plucky just + # This is the default on e.g. Fedora derivatives, but not Debian + - name: Enable unprivileged /dev/kvm access + shell: bash + run: | + set -xeuo pipefail + echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules + sudo udevadm control --reload-rules + sudo udevadm trigger --name-match=kvm + ls -l /dev/kvm + # Used by a few workflows, but generally useful + - name: Set architecture variable + id: set_arch + shell: bash + run: echo "ARCH=$(arch)" >> $GITHUB_ENV + # Install libvirt stack if requested + - name: Install libvirt and virtualization stack + if: ${{ inputs.libvirt == 'true' }} + shell: bash + run: | + set -xeuo pipefail + export BCVK_VERSION=0.9.0 + # see https://github.com/bootc-dev/bcvk/issues/176 + /bin/time -f '%E %C' sudo apt install -y libkrb5-dev pkg-config libvirt-dev genisoimage qemu-utils qemu-kvm virtiofsd libvirt-daemon-system python3-virt-firmware + # Something in the stack is overriding this, but we want session right now for bcvk + echo LIBVIRT_DEFAULT_URI=qemu:///session >> $GITHUB_ENV + td=$(mktemp -d) + cd $td + # Install bcvk + target=bcvk-$(arch)-unknown-linux-gnu + /bin/time -f '%E %C' curl -LO https://github.com/bootc-dev/bcvk/releases/download/v${BCVK_VERSION}/${target}.tar.gz + tar xzf ${target}.tar.gz + sudo install -T ${target} /usr/bin/bcvk + cd - + rm -rf "$td" + + # Also bump the default fd limit as a workaround for https://github.com/bootc-dev/bcvk/issues/65 + sudo sed -i -e 's,^\* hard nofile 65536,* hard nofile 524288,' /etc/security/limits.conf + - name: Cleanup status + shell: bash + run: | + set -xeuo pipefail + systemctl list-units 'action-cleanup*' + df -h diff --git a/.github/actions/setup-rust/action.yml b/.github/actions/setup-rust/action.yml new file mode 100644 index 0000000..f2f5e06 --- /dev/null +++ b/.github/actions/setup-rust/action.yml @@ -0,0 +1,20 @@ +name: 'Setup Rust' +description: 'Install Rust toolchain with caching and nextest' +runs: + using: 'composite' + steps: + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + - name: Install nextest + uses: taiki-e/install-action@v2 + with: + tool: nextest + - name: Setup Rust cache + uses: Swatinem/rust-cache@v2 + with: + cache-all-crates: true + # Only generate caches on push to git main + save-if: ${{ github.ref == 'refs/heads/main' }} + # Suppress actually using the cache for builds running from + # git main so that we avoid incremental compilation bugs + lookup-only: ${{ github.ref == 'refs/heads/main' }} diff --git a/.github/workflows/openssf-scorecard.yml b/.github/workflows/openssf-scorecard.yml new file mode 100644 index 0000000..314a0fa --- /dev/null +++ b/.github/workflows/openssf-scorecard.yml @@ -0,0 +1,50 @@ +# Upstream https://github.com/ossf/scorecard/blob/main/.github/workflows/scorecard-analysis.yml +# Tweaked to not pin actions by SHA digest as I think that's overkill noisy security theater. +name: OpenSSF Scorecard analysis +on: + push: + branches: + - main + +permissions: read-all + +jobs: + analysis: + name: Scorecard analysis + runs-on: ubuntu-24.04 + permissions: + # Needed for Code scanning upload + security-events: write + # Needed for GitHub OIDC token if publish_results is true + id-token: write + + steps: + - name: "Checkout code" + uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: "Run analysis" + uses: ossf/scorecard-action@v2.4.3 + with: + results_file: results.sarif + results_format: sarif + # Scorecard team runs a weekly scan of public GitHub repos, + # see https://github.com/ossf/scorecard#public-data. + # Setting `publish_results: true` helps us scale by leveraging your workflow to + # extract the results instead of relying on our own infrastructure to run scans. + # And it's free for you! + publish_results: true + + - name: "Upload artifact" + uses: actions/upload-artifact@v6 + with: + name: SARIF file + path: results.sarif + retention-days: 5 + + - name: "Upload to code-scanning" + uses: github/codeql-action/upload-sarif@v4 + with: + sarif_file: results.sarif + diff --git a/.github/workflows/rebase.yml b/.github/workflows/rebase.yml new file mode 100644 index 0000000..68f329a --- /dev/null +++ b/.github/workflows/rebase.yml @@ -0,0 +1,45 @@ +name: Automatic Rebase +on: + pull_request: + types: [labeled] + +permissions: + contents: read + +jobs: + rebase: + name: Rebase + if: github.event.label.name == 'needs-rebase' + runs-on: ubuntu-latest + steps: + - name: Generate Actions Token + id: token + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ secrets.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + + - name: Checkout + uses: actions/checkout@v6 + with: + token: ${{ steps.token.outputs.token }} + fetch-depth: 0 + + - name: Automatic Rebase + uses: peter-evans/rebase@v4 + with: + token: ${{ steps.token.outputs.token }} + + - name: Remove needs-rebase label + if: always() + uses: actions/github-script@v8 + with: + github-token: ${{ steps.token.outputs.token }} + script: | + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + name: 'needs-rebase' + }); diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..98ff247 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,41 @@ + + +# Instructions for AI agents + +## CRITICAL instructions for generating commits + +### Signed-off-by + +Human review is required for all code that is generated +or assisted by a large language model. If you +are a LLM, you MUST NOT include a `Signed-off-by` +on any automatically generated git commits. Only explicit +human action or request should include a Signed-off-by. +If for example you automatically create a pull request +and the DCO check fails, tell the human to review +the code and give them instructions on how to add +a signoff. + +### Attribution + +When generating substantial amounts of code, you SHOULD +include an `Assisted-by: TOOLNAME (MODELNAME)`. For example, +`Assisted-by: Goose (Sonnet 4.5)`. + +## Code guidelines + +The [REVIEW.md](REVIEW.md) file describes expectations around +testing, code quality, commit organization, etc. If you're +creating a change, it is strongly encouraged after each +commit and especially when you think a task is complete +to spawn a subagent to perform a review using guidelines (alongside +looking for any other issues). + +If you are performing a review of other's code, the same +principles apply. + +## Follow other guidelines + +Look at the project README.md and look for guidelines +related to contribution, such as a CONTRIBUTING.md +and follow those. diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 0000000..80e6f4d --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,173 @@ +# Code Review Guidelines + +These guidelines are derived from analysis of code reviews across the bootc-dev +organization (October–December 2024). They represent the collective expectations +and standards that have emerged from real review feedback. + +## Testing + +Tests are expected for all non-trivial changes - unit and integration by default. + +If there's something that's difficult to write a test for at the current time, +please do at least state if it was tested manually. + +### Choosing the Right Test Type + +Unit tests are appropriate for parsing logic, data transformations, and +self-contained functions. Use integration tests for anything that involves +running containers or VMs. + +Default to table-driven tests instead of having a separate unit test per +case. Especially LLMs like to generate the latter, but it can become +too verbose. Context windows matter to both humans and LLMs reading the +code later (this applies outside of unit tests too of course, but it's +easy to generate a *lot* of code for unit tests unnecessarily). + +### Separating Parsing from I/O + +A recurring theme is structuring code for testability. Split parsers from data +reading: have the parser accept a `&str`, then have a separate function that +reads from disk and calls the parser. This makes unit testing straightforward +without filesystem dependencies. + +### Test Assertions + +Make assertions strict and specific. Don't just verify that code "didn't crash"— +check that outputs match expected values. When adding new commands or output +formats, tests should verify the actual content, not just that something was +produced. + +## Code Quality + +### Parsing Structured Data + +Never parse structured data formats (JSON, YAML, XML) with text tools like `grep` +or `sed`. + +### Shell Scripts + +Try to avoid having shell script longer than 50 lines. This commonly occurs +in build system and tests. For the build system, usually there's higher +level ways to structure things (Justfile e.g.) and several of our projects +use the `cargo xtask` pattern to put arbitrary "glue" code in Rust using +the `xshell` crate to keep it easy to run external commands. + +### Constants and Magic Values + +Extract magic numbers into named constants. Any literal number that isn't +immediately obvious—buffer sizes, queue lengths, retry counts, timeouts—should +be a constant with a descriptive name. The same applies to magic strings: +deduplicate repeated paths, configuration keys, and other string literals. + +When values aren't self-explanatory, add a comment explaining why that specific +value was chosen. + +### Don't ignore (swallow) errors + +Avoid the `if let Ok(v) = ... { }` in Rust, or `foo 2>/dev/null || true` +pattern in shell script by default. Most errors should be propagated by +default. If not, it's usually appropriate to at least log error messages +at a `tracing::debug!` or equivalent level. + +Handle edge cases explicitly: missing data, malformed input, offline systems. +Error messages should provide clear context for diagnosis. + +### Code Organization + +Separate concerns: I/O operations, parsing logic, and business logic belong in +different functions. Structure code so core logic can be unit tested without +external dependencies. + +It can be OK to duplicate a bit of code in a slightly different form twice, +but having it happen in 3 places asks for deduplication. + +## Commits and Pull Requests + +### Commit Organization + +Break changes into logical, atomic commits. Reviewers appreciate being able to +follow your reasoning: "Especially grateful for breaking it up into individual +commits so I can more easily follow your train of thought." + +Preparatory refactoring should be separate from behavioral changes. Each commit +should tell a clear story and be reviewable independently. Commit messages should +explain the "why" not just the "what," and use imperative mood ("Add feature" +not "Added feature"). + +### PR Descriptions + +PRs should link to the issues they address using `Closes:` or `Fixes:` with +full URLs. One reviewer noted: "I edited this issue just now to have +`Closes: ` but let's try to be sure we're doing that kind of thing in +general in the future." + +Document known limitations and caveats explicitly. When approaches have tradeoffs +or don't fully solve a problem, say so. For complex investigations, use collapsible +`
` sections to include debugging notes without cluttering the main +description. + +Think about broader implications: "But we'll have this problem across all repos +right?" Consider how your change affects the wider ecosystem. + +### Keeping PRs Current + +Keep PRs rebased on main. When CI failures are fixed in other PRs, rebase to +pick up the fixes. Reference the fixing PR when noting that a rebase is needed. + +### Before Merge + +Self-review your diff before requesting review. Catch obvious issues yourself +rather than burning reviewer cycles. + +Do not add `Signed-off-by` lines automatically—these require explicit human +action after review. If code was AI-assisted, include an `Assisted-by:` trailer +indicating the tool and model used. + + +## Architecture and Design + +### Workarounds vs Proper Fixes + +When implementing a workaround, document where the proper fix belongs and link +to relevant upstream issues. Invest time investigating proper fixes before +settling on workarounds. + +### Cross-Project Considerations + +Prefer pushing fixes upstream when the root cause is in a dependency. Reduce +scope where possible; don't reimplement functionality that belongs elsewhere. + +When multiple systems interact (like Renovate and custom sync tooling), be +explicit about which system owns what and how they coordinate. + +### Avoiding Regressions + +Verify that new code paths handle all cases the old code handled. When rewriting +functionality, ensure equivalent coverage exists. + +### Review Requirements + +When multiple contributors co-author a PR, bring in an independent reviewer. + +## Rust-Specific Guidance + +Prefer rustix over `libc`. All `unsafe` code must be very carefully +justified. + +### Dependencies + +New dependencies should be justified. Glance at existing reverse dependencies +on crates.io to see if a crate is widely used. Consider alternatives: "I'm +curious if you did any comparative analysis at all with alternatives?" + +Prefer well-maintained crates with active communities. Consider `cargo deny` +policies when adding dependencies. + +### API Design + +When adding new commands or options, think about machine-readable output early. +JSON is generally preferred for that. + +Keep helper functions in appropriate modules. Move command output formatting +close to the CLI layer, keeping core logic functions focused on their primary +purpose.