Skip to content

feat: share Unix installation with local development and preserve trusted CAs - #51

Closed
cpunion wants to merge 3 commits into
xgo-dev:mainfrom
cpunion:codex/setup-llgo-consumer-reliability-20261006
Closed

cpunion wants to merge 3 commits into
xgo-dev:mainfrom
cpunion:codex/setup-llgo-consumer-reliability-20261006

Conversation

@cpunion

@cpunion cpunion commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

llcppg replaced setup-llgo in #917 to provide an actionable local/agent installer, not because the action's Go selection was broken. Its #940 addresses a second issue: apt can rebuild the system CA bundle and drop proxy CAs that were appended only to that bundle.

Changes

Use the same LLGo ref resolver, checksum-verified release installer, source builder, and Unix dependencies in CI and locally:

LLGO_VERSION=main GO_VERSION=1.27.0 bash scripts/install.sh
# source the printed env.sh, then:
llgo test -v ./...
Before After
Consumers duplicate installation logic for local agents A standalone Linux/macOS entrypoint reuses the action bundle and shared dependency script
An old Go launcher can reject newer GOEXPERIMENT settings locally Activate the selected toolchain's real bin/go with GOTOOLCHAIN=local
apt can discard bundle-only trusted CAs Validate and preserve existing extra CAs before apt, retaining TLS verification and respecting managed sources
Linux dependency setup uses apt-key and implicit LLVM development dependencies Repository-scoped signing key and explicit LLVM/libclang/libunwind/libc++ development packages

The local entrypoint requires Node.js 20+ and an existing Go 1.21+ launcher. It owns unique installation directories, emits a safely quoted activation file, and does not reset user checkouts or edit profiles. Windows setup and the existing source-build command are unchanged. CA preservation adapts #940 with attribution, fixes symlink traversal, and never introduces a project-supplied CA.

Validation

  • Existing 60 TypeScript tests, TypeScript compilation, ESLint, formatting, ShellCheck and diff checks passed locally.
  • Keep the existing setup-llgo CI matrix; no new test scripts or mock/server framework.
  • llcppg draft #943 pins simplified head 66efbc5 in CI and AGENTS.md. All four downstream jobs passed: action/standalone installation on Linux/macOS, full llcppg coverage tests and post-install HTTPS/Git checks. Standalone jobs start from Go 1.24 and activate the real Go 1.27.0 binary.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review summary

Solid, well-tested PR. It extracts shared Unix dependency setup into scripts/install-dependencies.sh, adds a standalone scripts/install.sh entrypoint, and introduces CA-preservation logic with extensive unit + integration tests. Two genuine improvements stand out: replacing the global-trust apt-key add with a scoped signed-by= keyring, and the carefully-scoped CA preservation (fingerprint-matched, follows symlinks like the real updater, refuses non-CA / managed / disabled certs, never disables TLS). Path-with-spaces handling and errexit-in-conditional pitfalls are tested throughout.

Verified the new __tests__/activate.test.ts mock counts are consistent with src/install.ts: non-Windows source builds call llvm-config 3× then go build (asserted as the 4th call, 5 mocks total), Windows calls go build first (2 mocks). No inconsistency.

Inline findings below. None are blockers.

Minor items not inlined:

  • scripts/install-dependencies.sh:40 (curl ... llvm-snapshot.gpg.key): no --retry, unlike other network paths in the project; a flaky runner fails on the first attempt. Consider --retry 3 --retry-connrefused.
  • scripts/install-dependencies.sh (macOS branch, brew update guard): the "Preserve runner-image formulae on Intel" comment sits directly above the arm64-only refresh, which can momentarily read as applying to the arm64 branch.
  • THIRD_PARTY_NOTICES.md credits "Changjun Ji" while scripts/preserve-extra-ca.sh credits "CarlJi" — same person (correct), but inconsistent form across sibling files.

Comment thread scripts/preserve-extra-ca.sh

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review summary

Solid, well-tested PR. It extracts shared Unix dependency setup into scripts/install-dependencies.sh, adds a standalone scripts/install.sh entrypoint, and introduces CA-preservation logic with extensive unit + integration tests. Two genuine improvements stand out: replacing the global-trust apt-key add with a scoped signed-by= keyring, and the carefully-scoped CA preservation (fingerprint-matched, follows symlinks like the real updater, refuses non-CA / managed / disabled certs, never disables TLS). Path-with-spaces handling and errexit-in-conditional pitfalls are tested throughout.

Verified the new __tests__/activate.test.ts mock counts are consistent with src/install.ts: non-Windows source builds call llvm-config 3× then go build (asserted as the 4th call, 5 mocks total), Windows calls go build first (2 mocks). No inconsistency.

Inline findings below. None are blockers.

(Note: this review re-submits three inline findings whose titles were previously too long; it supersedes the earlier review on this PR.)

Minor items not inlined:

  • scripts/install-dependencies.sh:40 (curl ... llvm-snapshot.gpg.key): no --retry, unlike other network paths in the project; a flaky runner fails on the first attempt. Consider --retry 3 --retry-connrefused.
  • scripts/install-dependencies.sh (macOS branch, brew update guard): the "Preserve runner-image formulae on Intel" comment sits directly above the arm64-only refresh, which can momentarily read as applying to the arm64 branch.
  • THIRD_PARTY_NOTICES.md credits "Changjun Ji" while scripts/preserve-extra-ca.sh credits "CarlJi" — same person (correct), but inconsistent form across sibling files.

Comment thread scripts/install.sh
Comment thread scripts/preserve-extra-ca.sh
Comment thread scripts/install-dependencies.sh Outdated
@cpunion cpunion closed this Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant