build: add native custom-gcl installer - #201
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the local linting experience by introducing a native Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a native installer for custom-gcl, which is a great improvement for the local development workflow. The changes are well-structured, splitting the logic into dedicated scripts and updating the Makefile and documentation accordingly. I've found a few areas for improvement in the new shell scripts to enhance their robustness and maintainability. Specifically, I've suggested adding a validation check for the destination path, simplifying some environment variable handling, and removing a hardcoded version string from the fallback script.
| exit 1 | ||
| fi | ||
|
|
||
| dest="$1" |
There was a problem hiding this comment.
The script doesn't handle the case where the provided destination is an existing directory. If <destination> is a directory, mv will place the binary inside it, but the success message will point to the directory itself, which is misleading. Furthermore, subsequent scripts attempting to execute the destination path will fail. You should add a check to ensure the destination is not a directory.
| dest="$1" | |
| dest="$1" | |
| if [ -d "$dest" ]; then | |
| echo "error: destination cannot be a directory: $dest" >&2 | |
| exit 1 | |
| fi |
| if [ -n "${GOFLAGS:-}" ]; then | ||
| export GOFLAGS="${GOFLAGS} -buildvcs=false" | ||
| else | ||
| export GOFLAGS="-buildvcs=false" | ||
| fi |
There was a problem hiding this comment.
The logic to append -buildvcs=false to GOFLAGS is a bit verbose. You can simplify this using POSIX parameter expansion for better readability and conciseness.
| if [ -n "${GOFLAGS:-}" ]; then | |
| export GOFLAGS="${GOFLAGS} -buildvcs=false" | |
| else | |
| export GOFLAGS="-buildvcs=false" | |
| fi | |
| export GOFLAGS="${GOFLAGS}${GOFLAGS:+ }-buildvcs=false" |
| cat >"$dest" <<'EOF2' | ||
| #!/bin/sh | ||
|
|
||
| set -eu | ||
|
|
||
| run_golangci() { | ||
| exec go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.5 "$@" | ||
| } | ||
|
|
||
| if [ "${1:-}" = "run" ]; then | ||
| shift | ||
|
|
||
| tmpdir="$(mktemp -d "${TMPDIR:-/tmp}/custom-gcl.XXXXXX")" | ||
| cfg="$tmpdir/custom-gcl.yml" | ||
| trap 'rm -rf "$tmpdir"' EXIT | ||
|
|
||
| awk ' | ||
| $0 ~ /^linters-settings:[[:space:]]*$/ { | ||
| in_ls = 1 | ||
| next | ||
| } | ||
| in_ls && $0 ~ /^ custom:[[:space:]]*$/ { | ||
| skip_custom = 1 | ||
| next | ||
| } | ||
| skip_custom && $0 ~ /^ [A-Za-z0-9_-]+:[[:space:]]*$/ { | ||
| skip_custom = 0 | ||
| } | ||
| in_ls && $0 ~ /^[^[:space:]]/ { | ||
| in_ls = 0 | ||
| } | ||
| skip_custom { | ||
| next | ||
| } | ||
| $0 ~ /^[[:space:]]*-[[:space:]]*ll[[:space:]]*$/ { | ||
| sub(/ll/, "lll") | ||
| next | ||
| } | ||
| { | ||
| } | ||
| ' .golangci.yml >"$cfg" | ||
|
|
||
| run_golangci run --config "$cfg" "$@" | ||
| fi | ||
|
|
||
| run_golangci "$@" | ||
| EOF2 |
There was a problem hiding this comment.
The fallback script hardcodes the golangci-lint version (v1.64.5 on line 33). This could become out of sync if the version in tools/.custom-gcl.yml is updated, leading to inconsistencies. The generated script should dynamically determine the version from tools/.custom-gcl.yml at runtime. This would make the fallback mechanism more robust and easier to maintain.
cat >"$dest" <<'EOF2'
#!/bin/sh
set -eu
# This script is a fallback. It assumes it is located in tools/ and that
# the repo root is one level up.
repo_root=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
config_file="$repo_root/tools/.custom-gcl.yml"
gcl_version=""
if [ -f "$config_file" ]; then
gcl_version=$(sed -n 's/^version:[[:space:]]*//p' "$config_file" | head -n 1)
fi
# Fallback to a known-good version if parsing fails.
gcl_version=${gcl_version:-v1.64.5}
run_golangci() {
exec go run "github.com/golangci/golangci-lint/cmd/golangci-lint@${gcl_version}" "$@"
}
if [ "${1:-}" = "run" ]; then
shift
tmpdir="$(mktemp -d "${TMPDIR:-/tmp}/custom-gcl.XXXXXX")"
cfg="$tmpdir/custom-gcl.yml"
trap 'rm -rf "$tmpdir"' EXIT
awk '
$0 ~ /^linters-settings:[[:space:]]*$/ {
in_ls = 1
print
next
}
in_ls && $0 ~ /^ custom:[[:space:]]*$/ {
skip_custom = 1
next
}
skip_custom && $0 ~ /^ [A-Za-z0-9_-]+:[[:space:]]*$/ {
skip_custom = 0
}
in_ls && $0 ~ /^[^[:space:]]/ {
in_ls = 0
}
skip_custom {
next
}
$0 ~ /^[[:space:]]*-[[:space:]]*ll[[:space:]]*$/ {
sub(/ll/, "lll")
print
next
}
{
print
}
' .golangci.yml >"$cfg"
run_golangci run --config "$cfg" "$@"
fi
run_golangci "$@"
EOF2
438f2ce to
bc9abd7
Compare
- Add scripts/install-custom-gcl.sh to build a native custom-gcl binary for the current macOS/Linux host using the repo-local ll plugin. - Read the golangci-lint version and plugin module path from tools/.custom-gcl.yml so the native installer stays aligned with the Docker-based tooling config. - Move the local-custom-gcl bootstrap logic out of the Makefile and into dedicated scripts so the native and fallback paths are easier to follow. - Update scripts/local-custom-gcl.sh to prefer a preinstalled binary, reuse an existing local build, and then bootstrap a native build before falling back to the lll approximation. - Add make install-custom-gcl so developers can install the local linter explicitly without having to remember the script path. - Document the native install path in AGENTS.md and CLAUDE.md so local no-Docker linting can use the real ll plugin after setup. - Extend tools/AGENTS.md and tools/CLAUDE.md with the same local-lint guidance so the workflow is discoverable near the linter sources.
bc9abd7 to
e3869c7
Compare
Two PR #201 review fixes: Audit-only UTXO diff. applyUTXODiff no longer books external_deposit / external_withdrawal ledger legs; it writes wallet_utxo_log audit rows and nothing else. Every treasury_wallet movement produced by a round is already booked by handleRoundConfirmed (RecordCapitalCommitted, RecordMiningFee); every sweep movement by handleSweepCompleted (RecordRoundSweep, RecordMiningFee). Booking UTXO-level external_* legs on top would double-count round-change and round-funding outputs on every block. The RecordExternal* helpers stay defined for the follow-up classifier PR, which will distinguish round/sweep-attributable outpoints from genuinely external operator movements via an attribution table populated by the round/sweep handlers. Drop utxoTracker.mu. Every access runs on the durable actor's single-consumer receive loop (per the top-of-CLAUDE.md serialization invariant); reseedUTXOSnapshot runs inside Start before the mailbox opens. The previous mutex rationale ("future diagnostics readers") was speculative. If a reader path ever lands, a Snapshot() accessor can introduce its own lock without re-scoping the whole struct. Docs updated to match: ledger/ and fees/ CLAUDE.md + AGENTS.md, docs/fee_ledger.md, ARCHITECTURE.md. Tests rewritten to assert audit-only behavior on the diff paths.
ledgeractor: add durable ledger accounting actor
Extend the ledger actor's UTXO diff subsystem so every wallet movement gets a definitive classification and gets booked as either a round / sweep attribution (no external leg) or an operator external_deposit / external_withdrawal -- the piece that was left at 'audit-only' in PR #201. Rather than stand up a separate attribution table, piggyback on the existing wallet_utxo_log: * New source_id BYTEA NULL column carries round_id / batch_id when a round or sweep handler pre-inserts an attributed row. * New classifications: withdrawal (spent-side deposit), sweep_consumption (spent-side sweep_return), round_change (naming parity with round_funding), and pending (the two-phase limbo the diff loop uses before reconciliation). * New sqlc helpers PromotePendingWalletUTXOLog (atomic flip of stale pending rows into deposit/withdrawal) and InsertWalletUTXOLog now carries source_id and returns the rowcount so the diff loop can tell a genuine insert from a silent no-op against an already-attributed row. Classifier loop lives in two passes on each BlockEpochMsg: 1. reconcilePendingAuditRows promotes any pending row left behind by the previous block's diff to deposit or withdrawal and books the matching external_* ledger leg via fees.RecordExternalDeposit / RecordExternalWithdrawal. 2. applyUTXODiff inserts the current block's diff rows as pending; pre-inserts from round / sweep handlers short-circuit via the UNIQUE (hash, index, event) constraint so the classifier never double-books. The one-block grace window covers the narrow race where a BlockEpochMsg lands on the ledger actor's mailbox before the matching RoundConfirmedMsg / SweepCompletedMsg from a simultaneously-confirmed round or sweep. Follow-up commits wire the round / sweep producers to populate the new TLV outpoint slices and the handlers to pre-insert the attributed audit rows.
2 participants
Summary
This splits the native local-linter bootstrap into a dedicated tooling PR.
Changes
scripts/install-custom-gcl.shto build a host-nativecustom-gclMakefileinto dedicated scriptsscripts/local-custom-gcl.shto prefer PATH, reuse an existing localbinary, try a native build, and only then fall back to the
lllapproximation
make install-custom-gclAGENTS.md,CLAUDE.md, and thecorresponding
tools/docsWhy
The existing non-Docker lint path works, but it falls back to stock
golangci-lintand disables the repo-localllplugin. This keeps the localworkflow fast while letting
make lint-localandmake lint-changed-localuse the real plugin after one install step.
Validation
shellcheck scripts/install-custom-gcl.sh scripts/local-custom-gcl.sh./scripts/install-custom-gcl.sh ./tools/custom-gcl-native-test./tools/custom-gcl-native-test versionmake local-custom-gclmake lint-changed-local base=HEAD~1 workers=1