Skip to content

build: drop ninja as dependency and use shared cipd script - #390

Merged
dumbmoron merged 2 commits into
mainfrom
no-more-ninja
Sep 14, 2026
Merged

dumbmoron merged 2 commits into
mainfrom
no-more-ninja

Conversation

@dumbmoron

Copy link
Copy Markdown
Member

No description provided.

@dumbmoron
dumbmoron requested a review from wukko September 14, 2026 15:14
@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown

Retrigger

The PR appears safe to merge based on the reviewed changes.

Reviews (1) · Last reviewed commit: "build: drop ninja as dependency and use ..."

wukko
wukko previously approved these changes Sep 14, 2026

@helium-gardener helium-gardener 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 completed with no actionable findings.

Review walkthrough

Changes

Files Summary
docker/build.Dockerfile
scripts/shared.sh
helium/devutils/install_cipd_deps.py
Ninja is retained only where non-x86 LLVM/Rust builds require it; the shared CIPD installer now provisions host GN, Siso, and TypeScript before GN generation and the Siso build.
helium-chromium
helium/patches/series
patches/series
The submodule advance supplies the shared installer and related Helium updates. The Linux platform patches and their ordering are unchanged from the merge base, so their materialized additions do not represent new platform-patch intent.

Review checks

Check Result Notes
Scope ✅ Passed Comparison against the merge base confirmed that the feature-template hunk in 'change.diff' is base-branch-only; the PR changes only the Dockerfile, shared build script, and submodule pointer.
Project Policy ✅ Passed No upstream Chromium test file, case, data, expectation, or test target is modified by the reviewable change.
Correctness And Safety ✅ Passed Tool provisioning order, failure propagation, host and cross-build architecture handling, and explicit Siso selection were checked without a discrepancy.
Chromium Idioms ✅ Passed The shared installer reads pinned CIPD manifests through Chromium's 'gclient_eval' and installs the DEPS-owned locations consumed by Chromium's GN and Siso wrappers.
Simplicity And Decomposition ✅ Passed The change removes duplicated CIPD version lookup and local GN bootstrap logic in favor of the shared Helium installer.
Patch Organization ✅ Passed The unchanged Linux patch series remains prerequisite-ordered, and the submodule advance is required before 'scripts/shared.sh' can invoke the new installer.

Analysis record

  • Changed files considered: 3
  • Findings: 0 new, 0 retained, 0 resolved
  • Source snapshot: 6af469f2607e
  • Codex: gpt-5.6-sol, reasoning high, fast mode on, web search cached
  • Patch application: clean
  • Chromium context tool calls: none
  • Maintainer conventions applied: script-resource-lifecycle

Limitations

  • None reported.

@coderabbitai

This comment has been minimized.

coderabbitai[bot]

This comment was marked as low quality.

@dumbmoron
dumbmoron merged commit ab7b720 into main Sep 14, 2026
6 checks passed
@dumbmoron
dumbmoron deleted the no-more-ninja branch September 14, 2026 16:03
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.

2 participants