[toolchain] Upload published toolchains with `--upload` by cdesouza-chromium · Pull Request #38688 · brave/brave-core

cdesouza-chromium · 2026-07-31T17:47:04Z

This change unifies the model of how the different toolchain scripts
upload their toolchains, with all of them now providing a --upload
option to upload the produced tarball.

Additionally, this script further flattens the code under
tools/cr/toolchains.

Bug: brave/brave-browser#55812

This change unifies the model of how the different toolchain scripts upload their toolchains, with all of them now providing a `--upload` option to upload the produced tarball. Additionally, this script further flattens the code under `tools/cr/toolchains`. Bug: brave/brave-browser#55812

github-actions · 2026-07-31T17:48:03Z

[puLL-Merge] - brave/brave-core@38688

Description

Refactors toolchain publishing across three builders (Rust, Windows, Xcode). Extracts shared plumbing into new toolchain_publish.py: remote_url_exists, fetch_index, write_index_file, upload_files, brave_core_commit.

Replaces per-platform aggregating index (chronological list, one file per platform+upstream combo) with per-build sibling index (single mapping, one file per exact --brave-subrevision). Removes --force-overwrite collision logic entirely; replaces with _precheck_publishable fail-fast when index already published.

Rust builder gains --upload (previously always published index via CI upload); index writing decoupled from bucket upload. cr/toolchain.py repin now reads per-platform sibling index rather than SHA-fetching each archive directly.

Motivation: consistency across three near-identical publishing paths; simpler uniqueness model (bump subrevision vs. index collision matrix).

Possible Issues

Rust concurrency guard dropped. Old code had a NOTICE about CI not running concurrent same-platform jobs due to shared-index races. New per-build index sidesteps aggregation races, but _precheck_publishable → build → _upload is TOCTOU: two concurrent runs of same subrevision both pass precheck, both upload. S3 sign=False upload likely overwrites without conflict. Acceptable if CI serializes, but no comment documents the new assumption.
_precheck_publishable checks index existence, not archive. Only index URL probed. If archive upload previously succeeded but index write/upload failed, precheck passes and republishes — fine. But if index exists and archive missing (partial upload), precheck refuses forever without manual cleanup. Windows/Xcode share this; not new.
Rust _fetch_index_object strict equality vs. old prefix match. Now requires exact object_name match. Correct given deterministic naming, but any index served with trailing metadata in URL breaks. Intentional tightening.
Lost provenance fields in Rust index. New _write_index drops per-entry aggregation but retains chromium_version, chromium_commit, command_line, brave_core_commit. No timestamp? Retained. OK. Dropped: ability to query "how many versions" — README acknowledges removal.
Test _fake_extra_dep adds condition key to objects not present in production rust_toolchain_extra_dep object dicts (production sets condition only at dep-path level, not per-object). Divergence — test fixture richer than real output. Harmless but could mask assertions.

Changes

`tools/cr/toolchains/toolchain_publish.py` (new)

Shared helpers: brave_core_commit, remote_url_exists, fetch_index, write_index_file, upload_files. DEFAULT_TIMEOUT_SECS=30.

`tools/cr/toolchains/build_rust_toolchain.py`

Removed _latest_index_object (list-tail semantics), _publish_archive_index (collision matrix), _brave_core_commit, --no-index-download, --force-overwrite.
Added toolchain_index_name, _fetch_index_object, _precheck_publishable, _write_index, _upload, _upstream_stem, --upload.
rust_toolchain_extra_dep now takes brave_subrevision; reads exact sibling index.
Split TOOLCHAIN_BUCKET/TOOLCHAIN_BUCKET_PREFIX constants.

`tools/cr/toolchains/build_windows_toolchain.py`

Removed local _remote_url_exists, _brave_core_commit, INDEX_LICENSE_HEADER_TEMPLATE, inline fetch/write/upload. Delegates to toolchain_publish.

`tools/cr/toolchains/build_xcode_toolchain.py`

Same delegation. Removed --force-overwrite, added --upload, TOOLCHAIN_BUCKET/_PREFIX constants, _upload.
run() now always prechecks (no force bypass).

`tools/cr/toolchain.py`

Removed _fetch_archive_info (per-archive SHA download). repin delegates to rust_toolchain_extra_dep.

`tools/cr/toolchain_test.py`

_fake_archive_info → _fake_extra_dep. Patches rust_toolchain_extra_dep instead of _fetch_archive_info. Single-call assertion replaces 4-call.

Recipes

package_rust.py, build_xcode_toolchain.py: added --upload. Xcode dropped force_overwrite proto field + test.

`cherry_picks.py`, `README.md`

Doc updates.

sequenceDiagram
    participant CI as Recipe (--upload)
    participant B as ToolchainBuilder
    participant TP as toolchain_publish
    participant S3 as Bucket

    CI->>B: run(upload=True)
    B->>B: _precheck_publishable()
    B->>TP: remote_url_exists(index_url)
    TP->>S3: HEAD/GET index
    S3-->>TP: 403/404 (absent)
    TP-->>B: False
    Note over B: build + smoke test
    B->>TP: write_index_file(path, index)
    B->>TP: upload_files(bucket, prefix, (archive, index))
    TP->>S3: put archive
    TP->>S3: put index

    Note over Cr: later, repin
    participant Cr as cr/toolchain.py
    Cr->>B: rust_toolchain_extra_dep(stem, subrev)
    loop each platform
        B->>TP: fetch_index(index_url)
        TP->>S3: GET sibling index
        S3-->>TP: mapping
        B->>B: verify object_name match
    end
    B-->>Cr: EXTRA_DEPS entry

brave-builds · 2026-07-31T17:53:09Z

Warning

You have got a presubmit warning. Please address it if possible.

Found 6 lines longer than 80 characters (first 5 shown).

Items:

tools/cr/toolchain_test.py, line 642, 96 chars
tools/cr/toolchains/build_rust_toolchain.py, line 126, 81 chars
tools/cr/toolchains/build_rust_toolchain.py, line 643, 81 chars
tools/cr/toolchains/build_rust_toolchain.py, line 731, 82 chars
tools/cr/toolchains/build_rust_toolchain.py, line 1030, 81 chars

github-actions · 2026-07-31T18:00:41Z

📋 Code Owners Summary

12 file(s) changed, 12 with assigned owners

1 team(s) affected: @brave/chromium-rebase-team

Owners and Their Files

`@brave/chromium-rebase-team` — 12 file(s)

... and 7 more files

github-actions · 2026-07-31T18:03:03Z

+    "absent".
+    """
+    try:
+        with urllib.request.urlopen(url, timeout=timeout):


_{reported by reviewdog 🐶}
[opengrep] Detected a dynamic value being used with urllib. urllib supports 'file://' schemes, so a dynamic value controlled by a malicious actor may allow them to read arbitrary files. Audit uses of urllib calls to ensure user data cannot control the URLs, or consider using the 'requests' library instead.

Source: https://semgrep.dev/r/python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected

Cc @thypon

Please consider an alternative approach that avoids this security concern, or request a review from the sec-team on slack.

github-actions · 2026-07-31T18:03:03Z

+    "absent".
+    """
+    try:
+        with urllib.request.urlopen(url, timeout=timeout):


_{reported by reviewdog 🐶}
[opengrep] The application was found passing in a non-literal value to the urllib methods which issue
requests. urllib supports the file:// scheme, which may allow an adversary who can control
the URL value to read arbitrary files on the file system.

To remediate this issue either hardcode the URLs being used in urllib or use the requests
module instead.

Example using the requests module to issue an HTTPS request:
 import requests # Issue a GET request to https://example.com with a timeout of 10 seconds response = requests.get('https://example.com', timeout=10) # Work with the response object # ... 

Source: https://semgrep.dev/r/gitlab.bandit.B310-1

Cc @thypon

Please consider an alternative approach that avoids this security concern, or request a review from the sec-team on slack.

github-actions · 2026-07-31T18:03:03Z

+    """
+    logging.debug('Fetching %s index %s', description, index_url)
+    try:
+        with urllib.request.urlopen(index_url, timeout=timeout) as response:


_{reported by reviewdog 🐶}
[opengrep] Detected a dynamic value being used with urllib. urllib supports 'file://' schemes, so a dynamic value controlled by a malicious actor may allow them to read arbitrary files. Audit uses of urllib calls to ensure user data cannot control the URLs, or consider using the 'requests' library instead.

Source: https://semgrep.dev/r/python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected

Cc @thypon

Please consider an alternative approach that avoids this security concern, or request a review from the sec-team on slack.

github-actions · 2026-07-31T18:03:03Z

+    """
+    logging.debug('Fetching %s index %s', description, index_url)
+    try:
+        with urllib.request.urlopen(index_url, timeout=timeout) as response:


_{reported by reviewdog 🐶}
[opengrep] The application was found passing in a non-literal value to the urllib methods which issue
requests. urllib supports the file:// scheme, which may allow an adversary who can control
the URL value to read arbitrary files on the file system.

To remediate this issue either hardcode the URLs being used in urllib or use the requests
module instead.

Example using the requests module to issue an HTTPS request:
 import requests # Issue a GET request to https://example.com with a timeout of 10 seconds response = requests.get('https://example.com', timeout=10) # Work with the response object # ... 

Source: https://semgrep.dev/r/gitlab.bandit.B310-1

Cc @thypon

Please consider an alternative approach that avoids this security concern, or request a review from the sec-team on slack.

netzenbot · 2026-08-01T00:19:02Z

+        """Write the sibling YAML index describing the just-built archive.

-        Each entry in the index has these fields:
+        Unlike a shared, ever-growing index, this writes one index per build,


nit: "Unlike a shared, ever-growing index... rather than an aggregated list" only makes sense if you know what this replaced — a first-time reader has no such context. Describing it positively ("Writes one index per build, holding a single mapping...") also fixes the dangling "Each has these fields", which reads as a leftover from "Each entry in the index". (✅ Comments Must Make Sense to Future Readers of the Codebase)

netzenbot · 2026-08-01T00:19:02Z

-            f'{toolchain.build_rust_toolchain.TOOLCHAIN_BUCKET_URL}/'
-            f'{linux_name}')
+        # Every platform's object is read from its sibling index in one call.
+        self.fetch.assert_called_once_with(self.UPSTREAM_STEM,


nit: the old assert_any_call(f'{TOOLCHAIN_BUCKET_URL}/{linux_name}') was the only coverage of the per-platform object-name/URL derivation. That derivation now lives in rust_toolchain_extra_dep (plus the new -{brave_subrevision}.tar.xz suffix and toolchain_index_name), which has no test file, and _fake_extra_dep re-implements the naming rather than exercising it — so the check isn't relocated, just dropped. Consider a small test for rust_toolchain_extra_dep/toolchain_index_name in tools/cr/toolchains/. (✅ Relocate Test Checks When Refactoring)

netzenbot · 2026-08-01T00:19:02Z

+                       capture_output=True).stdout.strip()
+
+
+def remote_url_exists(url: str, timeout: int = DEFAULT_TIMEOUT_SECS) -> bool:


nit: the other shared helper modules in tools/cr/toolchains/ (cherry_picks.py, gitiles.py, upload.py) each have a sibling *_test.py, but this new one doesn't. remote_url_exists's 403/404-vs-raise behaviour and fetch_index's URLError -> RuntimeError wrapping are easy to cover with a urlopen patch and would be worth a small toolchain_publish_test.py. (✅ PRs Should Include Reasonable Test Coverage)

cdesouza-chromium requested a review from a team as a code owner July 31, 2026 17:47

cdesouza-chromium added the CI/skip Do not run CI builds (except noplatform) label Jul 31, 2026

cdesouza-chromium self-assigned this Jul 31, 2026

github-actions Bot added the puLL-Merge label Jul 31, 2026

github-actions Bot reviewed Jul 31, 2026

View reviewed changes

github-actions Bot added the needs-security-review label Jul 31, 2026

netzenbot reviewed Aug 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[toolchain] Upload published toolchains with `--upload` - #38688

[toolchain] Upload published toolchains with `--upload`#38688
cdesouza-chromium wants to merge 1 commit into
masterfrom
toolchain+upload-toolchains-with-script

cdesouza-chromium commented Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026

Uh oh!

brave-builds commented Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026

Uh oh!

github-actions Bot Jul 31, 2026

Uh oh!

github-actions Bot Jul 31, 2026

Uh oh!

github-actions Bot Jul 31, 2026

Uh oh!

github-actions Bot Jul 31, 2026

Uh oh!

netzenbot Aug 1, 2026

Uh oh!

netzenbot Aug 1, 2026

Uh oh!

netzenbot Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		capture_output=True).stdout.strip()


		def remote_url_exists(url: str, timeout: int = DEFAULT_TIMEOUT_SECS) -> bool:

Uh oh!

Conversation

cdesouza-chromium commented Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026

Description

Possible Issues

Changes

tools/cr/toolchains/toolchain_publish.py (new)

tools/cr/toolchains/build_rust_toolchain.py

tools/cr/toolchains/build_windows_toolchain.py

tools/cr/toolchains/build_xcode_toolchain.py

tools/cr/toolchain.py

tools/cr/toolchain_test.py

Recipes

cherry_picks.py, README.md

Uh oh!

brave-builds commented Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026

📋 Code Owners Summary

Owners and Their Files

@brave/chromium-rebase-team — 12 file(s)

Uh oh!

github-actions Bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

netzenbot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

netzenbot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

netzenbot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`tools/cr/toolchains/toolchain_publish.py` (new)

`tools/cr/toolchains/build_rust_toolchain.py`

`tools/cr/toolchains/build_windows_toolchain.py`

`tools/cr/toolchains/build_xcode_toolchain.py`

`tools/cr/toolchain.py`

`tools/cr/toolchain_test.py`

`cherry_picks.py`, `README.md`

`@brave/chromium-rebase-team` — 12 file(s)