[git-cr] Rewrite gn references of moved targets (#36209)
This PR introduces support for `cr mv` and `cr follow-renames` to correct `gn` references across the projects when a `BUILD.gn` path changes. This is a modest introduction to references path correction. The rewrite does handle relative references though. In source references are only corrected for the main target, i.e `:basename`, which gets corrected to the new path base name. This new approach also attempts to correct gn references to moved sources, but only for `//` root reference paths. In the future we could potentially introduce some mechanism handles file renames. The baseline for this feature was to get the following to build with no errors: ``` git cr mv components/api_request_helper/ components/api_foo npm run build -- --target=brave:all ``` As a small detour, this PR adds `npm run format` to these commands, when wrapping up. This PR also adds a README.md for the `alias` folder. Resolves https://github.com/brave/brave-browser/issues/55297
This commit is contained in:
cdesouza-chromium
@@ -0,0 +1,196 @@
|
||||
# `git cr` alias for Chromium rebasing tooling
|
||||
|
||||
This alias is maintained by the Chromium Rebase team to simplify many of the
|
||||
rebase tasks. This directory provides the `git cr` git alias and the supporting
|
||||
commit-msg hook used when committing to `brave-core`.
|
||||
|
||||
## Quick start
|
||||
|
||||
From the `brave-core` repository root, install this alias with:
|
||||
|
||||
```sh
|
||||
python3 tools/cr/alias/cmd.py setup-alias
|
||||
git cr install-hook
|
||||
```
|
||||
|
||||
* `setup-alias` writes a `cr` entry to `.git/config`. After this, `git cr`
|
||||
works in this repository.
|
||||
* `install-hook` installs `commit-msg.py` as the repository's `commit-msg`
|
||||
hook. This is necessary to use `git cr commit`
|
||||
|
||||
Once installed, run `git cr` to see the available commands.
|
||||
|
||||
## Subcommands
|
||||
|
||||
### `git cr commit`
|
||||
|
||||
A wrapper around `git commit` that adds three convenience flags. All other
|
||||
arguments are forwarded verbatim to `git commit`.
|
||||
|
||||
```sh
|
||||
git cr commit -m "Fix login button alignment"
|
||||
git cr commit --tagged=WIP,android -m "Work in progress"
|
||||
git cr commit --issue 12345 -m "Resolve crash on launch"
|
||||
git cr commit --culprit abc123,def456 -m "Adjust to upstream API change"
|
||||
```
|
||||
|
||||
[commit-msg hook behaviour](#commit-msg-hook-behaviour) section below for
|
||||
more details.
|
||||
|
||||
### `git cr mv`
|
||||
|
||||
Move a file or directory inside `brave-core` and repair every artefact that
|
||||
references the old path.
|
||||
|
||||
```sh
|
||||
git cr mv components/api_request_helper/ components/api_foo
|
||||
npm run build -- --target=brave:all
|
||||
```
|
||||
|
||||
What it repairs after the rename:
|
||||
|
||||
* C++ include guards in moved `.h` files (regenerated to match the new
|
||||
path).
|
||||
* `#include <…>` shadow-include lines in moved `chromium_src/` files.
|
||||
* `#include` / `#import` directives across `brave-core` (both quoted and
|
||||
angle-bracket forms, including `.mojom` files and their derived
|
||||
`*.mojom.h`, `*.mojom-blink.h`, etc.).
|
||||
* `// path/to/file` style references inside C++ comments and `.gn`/`.gni`
|
||||
files.
|
||||
* `BUILD.gn` / `.gni` source-list entries in the ancestor chain of the
|
||||
moved file.
|
||||
* Quoted GN root references (`"//path/to/file"`) and relative GN
|
||||
references in `.gn` / `.gni` files.
|
||||
* Plaster files (`rewrite/…/foo.h.toml`) and their associated patch files
|
||||
in `patches/`. The new plaster file is re-applied so `patches/` is
|
||||
refreshed.
|
||||
|
||||
### `git cr follow-renames`
|
||||
|
||||
This command has some similarities to `git-cr-mv`, however its main role is
|
||||
to correct move operations that occurred in upstream Chromium.
|
||||
|
||||
```sh
|
||||
# All renames between two Chromium version tags (typical version bump):
|
||||
git cr follow-renames 149.0.7827.1..149.0.7827.4
|
||||
|
||||
# Renames introduced by a single upstream commit:
|
||||
git cr follow-renames 4f093f4239eb2814f57bc97ee593d7acd717ac42
|
||||
```
|
||||
|
||||
This command does all the rewrites done by `git-cr-mv`, but it also does a few
|
||||
extra things:
|
||||
|
||||
1. Move `chromium_src/<old>` to `chromium_src/<new>` and refresh its
|
||||
include guard and shadow `#include` line.
|
||||
2. Move `rewrite/<old>.toml` to `rewrite/<new>.toml`, delete the stale
|
||||
`patches/…` patch file (and `.patchinfo`), then re-run plaster on the
|
||||
new TOML so a fresh patch is produced.
|
||||
3. Update every `#include`, `#import`, `// comment`, `BUILD.gn`, and `.gni`
|
||||
reference across `brave-core` (same logic as `git cr mv`).
|
||||
4. For any patch that is **not** managed by plaster, rewrite the patch's
|
||||
`--- a/old`, `+++ b/new`, and `diff --git` lines, rename the file, and
|
||||
try `git apply --3way` against the Chromium tree so conflicts surface
|
||||
immediately.
|
||||
|
||||
## Why the use of a git commit message hook
|
||||
|
||||
Chromium rebases usually involve hundreds of fixes to Brave. These fixes are
|
||||
the result of the way Chromium evolves. Therefore, `cr` have to provide a
|
||||
significant amount of information regading the project history when introducing
|
||||
these fixes. This is vital when trying to understand when and why certain
|
||||
changes were introduced in the project.
|
||||
|
||||
The hook provides several helpers for commit messages that are relevant to
|
||||
practices we have for `brave-core`. Check `commit-msg.py` for the full
|
||||
documentation. A few examples that we use it for:
|
||||
|
||||
```sh
|
||||
(branch: cr149) $ git commit -m "Disables kVerticalTabsLaunch feature."
|
||||
# Resulting commit message:
|
||||
# [cr149] Disables kVerticalTabsLaunch feature.
|
||||
```
|
||||
|
||||
```sh
|
||||
(branch: canary+fix-toolbar-crash) $ git commit -m "Guard against null TabStripModel"
|
||||
# Resulting commit message:
|
||||
# [canary] Guard against null TabStripModel
|
||||
```
|
||||
|
||||
```sh
|
||||
(branch: cr149+fix-issue-55193) $ git commit -m "Migrate ECDSA_SHA384 patches to plaster"
|
||||
# Resulting commit message:
|
||||
# [cr149] Migrate ECDSA_SHA384 patches to plaster
|
||||
#
|
||||
# Resolves https://github.com/brave/brave-browser/issues/55193
|
||||
```
|
||||
|
||||
### Every commit on a `cr` is expected to have a `[crXXX]` tag
|
||||
|
||||
The main covenience the hook provides, is the enforcement of `[cr149]` prefix
|
||||
when working on `cr` branches, which easily catches one's attention when
|
||||
looking on a blame. This branch tag is important for any commit that is not
|
||||
too obvious to be part of a `cr` branch without it.
|
||||
|
||||
### Using commit tags to provide context
|
||||
|
||||
We should always try to provide tags, as a way to group changes together.
|
||||
For examnple, when a change only matters on a single platform, an OS tag could
|
||||
(e.g. `[android]`, `[ios]`, etc) can provide important context when reviewing
|
||||
the change, and also make it easier similar changes.
|
||||
|
||||
This is strongly encouraged. A single grep then yields every fix that
|
||||
has touched the affected surface, and reviewers triaging a regression may be
|
||||
able to take advantage of such extra context.
|
||||
|
||||
Adding additional tags is very simple:
|
||||
|
||||
```sh
|
||||
(branch: cr149) $ git cr commit --tagged WIP -m "Guard against null TabStripModel"
|
||||
# Resulting commit message:
|
||||
# [cr149][WIP] Guard against null TabStripModel
|
||||
```
|
||||
|
||||
|
||||
### Each change should reference at least one upstream culprit
|
||||
|
||||
Rebase branch can grow to hundreds of changes. For reviewers, and future code
|
||||
archeologists, it is important that we provide the exact cause for introducing
|
||||
a change into the branch. The upstream changes driving our fixes are referred
|
||||
to as **culprits**, and the tooling provides ways to keep them in our history.
|
||||
|
||||
`git cr commit --culprit <hash>` expands each hash into the commit message
|
||||
of the fix being done. This allows us to have a record, but also to use
|
||||
`git log --grep` to find changes based on CL numbers or `crbug` issue numbers.
|
||||
|
||||
|
||||
Auto-generated upgrade messages (`Update from Chromium …`,
|
||||
`Update patches from Chromium …`, `Updated strings for Chromium …`) are
|
||||
left verbatim so they remain easy to grep.
|
||||
|
||||
We add culprits to commits using `git cr commit --culprit=[HASH,]`:
|
||||
|
||||
```sh
|
||||
(branch: cr149) $ git cr commit \
|
||||
--culprit 70cb8d8433679502ede773d828bf31cb0a1bce16 \
|
||||
-m "Adapt to upstream proto changes"
|
||||
# Resulting commit message:
|
||||
# [cr149] Adapt to upstream proto changes
|
||||
#
|
||||
# Chromium changes:
|
||||
# https://chromium.googlesource.com/chromium/src/+/70cb8d8433679502ede773d828bf31cb0a1bce16
|
||||
#
|
||||
# commit 70cb8d8433679502ede773d828bf31cb0a1bce16
|
||||
# Author: Hamda Mare <hmare@google.com>
|
||||
# Date: Tue Apr 28 12:53:24 2026 -0700
|
||||
#
|
||||
# Add proto and type definitions for certificate collection
|
||||
#
|
||||
# This CL adds the necessary proto messages and C++ types to support
|
||||
# certificate collection in signal reports.
|
||||
#
|
||||
# Bug: 502634772
|
||||
# Change-Id: I2136d39ac83c293856690a8acef12d6d2babdeec
|
||||
# Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7795711
|
||||
# …
|
||||
```
|
||||
@@ -37,7 +37,7 @@ import _boot # noqa: F401
|
||||
from incendiary_error_handler import IncendiaryErrorHandler
|
||||
import plaster
|
||||
from plaster import PlasterFile
|
||||
from terminal import console
|
||||
from terminal import console, terminal
|
||||
import repository
|
||||
from alias.source_rewrite import (
|
||||
CPP_EXTENSIONS,
|
||||
@@ -68,6 +68,11 @@ def cmd_follow_renames(args: list[str]) -> int:
|
||||
action='store_true',
|
||||
dest='no_run_plaster',
|
||||
help='Skip running plaster after moving TOML files')
|
||||
parser.add_argument(
|
||||
'--no-format',
|
||||
action='store_true',
|
||||
dest='no_format',
|
||||
help='Skip running `npm run format` after processing renames')
|
||||
parser.add_argument('--verbose',
|
||||
action='store_true',
|
||||
help='Enable verbose logging')
|
||||
@@ -92,10 +97,25 @@ def cmd_follow_renames(args: list[str]) -> int:
|
||||
update_references(old_chromium, new_chromium)
|
||||
_repair_patch_files(old_chromium, new_chromium, parsed.no_git)
|
||||
|
||||
if renames and not parsed.no_format:
|
||||
_run_format()
|
||||
|
||||
console.log(f'[bold green]✔[/] {len(renames)} rename(s) processed')
|
||||
return 0
|
||||
|
||||
|
||||
def _run_format() -> None:
|
||||
"""Runs `npm run format` to clean up files touched by rename repairs.
|
||||
|
||||
Failures are downgraded to warnings: format must not block successful
|
||||
rename processing.
|
||||
"""
|
||||
try:
|
||||
terminal.run_npm_command('format')
|
||||
except (subprocess.CalledProcessError, FileNotFoundError) as e:
|
||||
logging.warning('npm run format failed: %s', e)
|
||||
|
||||
|
||||
def _get_chromium_renames(ref_or_range: str) -> list[_RenamePair]:
|
||||
"""Returns net (original → final) rename pairs from the Chromium git log.
|
||||
|
||||
|
||||
@@ -31,6 +31,9 @@ class _Base(unittest.TestCase):
|
||||
self._repo.setup()
|
||||
self.addCleanup(self._repo.cleanup)
|
||||
# chromium_src/, rewrite/ created by FakeChromiumSrc.setup()
|
||||
# `npm run format` is not available in the fake repo; suppress it.
|
||||
self._format_mock = patch('alias.follow_renames._run_format').start()
|
||||
self.addCleanup(patch.stopall)
|
||||
|
||||
@property
|
||||
def _brave(self) -> Path:
|
||||
@@ -334,6 +337,36 @@ class ReferencesTest(_Base):
|
||||
self.assertIn('#include "B/foo.h"', content)
|
||||
self.assertNotIn('#include "A/foo.h"', content)
|
||||
|
||||
def test_gn_root_reference_updated_on_gni_rename(self) -> None:
|
||||
"""An upstream .gni rename rewrites `"//path"` refs in BUILD.gn."""
|
||||
before = self._chromium_head()
|
||||
self._chromium_commit('tools/grit/repack.gni', '# repack template\n')
|
||||
self._brave_commit('browser/BUILD.gn',
|
||||
'import("//tools/grit/repack.gni")\n')
|
||||
self._chromium_rename('tools/grit/repack.gni', 'build/grit/repack.gni')
|
||||
|
||||
cmd_follow_renames([f'{before}..HEAD'])
|
||||
|
||||
content = (self._brave / 'browser' /
|
||||
'BUILD.gn').read_text(encoding='utf-8')
|
||||
self.assertIn('"//build/grit/repack.gni"', content)
|
||||
self.assertNotIn('"//tools/grit/repack.gni"', content)
|
||||
|
||||
def test_gn_root_reference_updated_on_cpp_rename(self) -> None:
|
||||
"""An upstream C++ rename rewrites `"//path.h"` refs in BUILD.gn."""
|
||||
before = self._chromium_head()
|
||||
self._chromium_commit('base/foo.h', '// header\n')
|
||||
self._brave_commit('browser/BUILD.gn',
|
||||
'sources = [ "//base/foo.h" ]\n')
|
||||
self._chromium_rename('base/foo.h', 'base/sub/foo.h')
|
||||
|
||||
cmd_follow_renames([f'{before}..HEAD'])
|
||||
|
||||
content = (self._brave / 'browser' /
|
||||
'BUILD.gn').read_text(encoding='utf-8')
|
||||
self.assertIn('"//base/sub/foo.h"', content)
|
||||
self.assertNotIn('"//base/foo.h"', content)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Multiple renames in a single range
|
||||
@@ -639,5 +672,38 @@ class PatchFileRepairTest(_Base):
|
||||
self.assertTrue((self._brave / 'patches' / 'B-foo.cc.patch').exists())
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Format step
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class FormatTest(_Base):
|
||||
"""`npm run format` runs once per invocation unless --no-format."""
|
||||
|
||||
def test_format_called_when_renames_exist(self) -> None:
|
||||
before = self._chromium_head()
|
||||
self._chromium_commit('A/foo.h', '// src\n')
|
||||
self._chromium_rename('A/foo.h', 'B/foo.h')
|
||||
|
||||
cmd_follow_renames([f'{before}..HEAD'])
|
||||
|
||||
self._format_mock.assert_called_once()
|
||||
|
||||
def test_format_skipped_with_no_format_flag(self) -> None:
|
||||
before = self._chromium_head()
|
||||
self._chromium_commit('A/foo.h', '// src\n')
|
||||
self._chromium_rename('A/foo.h', 'B/foo.h')
|
||||
|
||||
cmd_follow_renames(['--no-format', f'{before}..HEAD'])
|
||||
|
||||
self._format_mock.assert_not_called()
|
||||
|
||||
def test_format_skipped_when_no_renames(self) -> None:
|
||||
"""No renames in the range -> nothing to format."""
|
||||
before = self._chromium_head()
|
||||
cmd_follow_renames([f'{before}..HEAD'])
|
||||
self._format_mock.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
+21
-1
@@ -14,11 +14,12 @@ from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import logging
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import _boot # noqa: F401
|
||||
from incendiary_error_handler import IncendiaryErrorHandler
|
||||
from terminal import console
|
||||
from terminal import console, terminal
|
||||
import repository
|
||||
import plaster
|
||||
from plaster import PlasterFile
|
||||
@@ -58,6 +59,10 @@ def cmd_mv(args: list[str]) -> int:
|
||||
action='store_true',
|
||||
dest='no_run_plaster',
|
||||
help='Skip running plaster after moving TOML files')
|
||||
parser.add_argument('--no-format',
|
||||
action='store_true',
|
||||
dest='no_format',
|
||||
help='Skip running `npm run format` after the move')
|
||||
parser.add_argument('--verbose',
|
||||
action='store_true',
|
||||
help='Enable verbose logging')
|
||||
@@ -93,10 +98,25 @@ def cmd_mv(args: list[str]) -> int:
|
||||
|
||||
_step5_plaster(file_pairs, parsed.no_git, not parsed.no_run_plaster)
|
||||
|
||||
if not parsed.no_format:
|
||||
_run_format()
|
||||
|
||||
console.log(f'[bold green]✔[/] {parsed.source} → {parsed.destination}')
|
||||
return 0
|
||||
|
||||
|
||||
def _run_format() -> None:
|
||||
"""Runs `npm run format` to clean up files touched by the move.
|
||||
|
||||
Failures are downgraded to warnings: format must not block a successful
|
||||
move.
|
||||
"""
|
||||
try:
|
||||
terminal.run_npm_command('format')
|
||||
except (subprocess.CalledProcessError, FileNotFoundError) as e:
|
||||
logging.warning('npm run format failed: %s', e)
|
||||
|
||||
|
||||
def _step1_move(src: Path, dest: Path, mkdir: bool,
|
||||
no_git: bool) -> list[_FilePair]:
|
||||
"""Validates paths and performs the move.
|
||||
|
||||
@@ -11,6 +11,7 @@ import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import _boot # noqa: F401
|
||||
import repository
|
||||
@@ -28,6 +29,9 @@ class _Base(unittest.TestCase):
|
||||
self.addCleanup(self._repo.cleanup)
|
||||
# chromium_src/, rewrite/ created by FakeChromiumSrc.setup()
|
||||
# patches/ is created by FakeChromiumRepo.__init__
|
||||
# `npm run format` is not available in the fake repo; suppress it.
|
||||
self._format_mock = patch('alias.mv._run_format').start()
|
||||
self.addCleanup(patch.stopall)
|
||||
|
||||
@property
|
||||
def _brave(self) -> Path:
|
||||
@@ -281,6 +285,53 @@ class ReferencesTest(_Base):
|
||||
'user.cc').read_text(encoding='utf-8')
|
||||
self.assertIn('#include <base/feature_list.h>', cc_content)
|
||||
|
||||
def test_directory_move_rewrites_gn_references(self) -> None:
|
||||
"""Moving a directory rewrites root and relative GN references in
|
||||
unrelated BUILD.gn files."""
|
||||
self._commit(
|
||||
'components/api_request_helper/BUILD.gn',
|
||||
'static_library("api_request_helper") {\n'
|
||||
' sources = [ "api_request_helper.cc" ]\n'
|
||||
'}\n'
|
||||
'source_set("test_support") {\n'
|
||||
' public_deps = [ ":api_request_helper" ]\n'
|
||||
'}\n')
|
||||
self._commit('components/api_request_helper/api_request_helper.cc',
|
||||
'// impl\n')
|
||||
# Consumer at an unrelated dir uses a root reference.
|
||||
self._commit(
|
||||
'browser/BUILD.gn',
|
||||
'deps = [ "//brave/components/api_request_helper:test_support" ]\n'
|
||||
)
|
||||
# Sibling under components/ uses a relative reference.
|
||||
self._commit('components/ai_chat/BUILD.gn',
|
||||
'deps = [ "../api_request_helper" ]\n')
|
||||
|
||||
cmd_mv([
|
||||
'--mkdir', 'components/api_request_helper', 'components/api_test'
|
||||
])
|
||||
|
||||
browser_content = (self._brave / 'browser' /
|
||||
'BUILD.gn').read_text(encoding='utf-8')
|
||||
self.assertIn('"//brave/components/api_test:test_support"',
|
||||
browser_content)
|
||||
self.assertNotIn('api_request_helper', browser_content)
|
||||
|
||||
sibling_content = (self._brave / 'components' / 'ai_chat' /
|
||||
'BUILD.gn').read_text(encoding='utf-8')
|
||||
self.assertIn('"../api_test"', sibling_content)
|
||||
self.assertNotIn('api_request_helper', sibling_content)
|
||||
|
||||
# The moved BUILD.gn's directory-name target is renamed too — both
|
||||
# the declaration and same-file label refs. The `api_request_helper.cc`
|
||||
# source-list entry is a stable same-dir reference and must remain.
|
||||
moved_content = (self._brave / 'components' / 'api_test' /
|
||||
'BUILD.gn').read_text(encoding='utf-8')
|
||||
self.assertIn('static_library("api_test")', moved_content)
|
||||
self.assertIn(':api_test"', moved_content)
|
||||
self.assertNotIn('"api_request_helper"', moved_content)
|
||||
self.assertNotIn(':api_request_helper"', moved_content)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Plaster TOML handling tests (Step 5)
|
||||
@@ -498,5 +549,24 @@ class PlasterApplyTest(_Base):
|
||||
self.assertFalse((self._brave / 'patches' / 'B-foo.cc.patch').exists())
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Format step
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class FormatTest(_Base):
|
||||
"""`npm run format` runs after a successful move unless --no-format."""
|
||||
|
||||
def test_format_called_by_default(self) -> None:
|
||||
self._commit('foo/bar.h', '// header\n')
|
||||
cmd_mv(['--mkdir', 'foo/bar.h', 'baz/bar.h'])
|
||||
self._format_mock.assert_called_once()
|
||||
|
||||
def test_format_skipped_with_no_format_flag(self) -> None:
|
||||
self._commit('foo/bar.h', '// header\n')
|
||||
cmd_mv(['--mkdir', '--no-format', 'foo/bar.h', 'baz/bar.h'])
|
||||
self._format_mock.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
@@ -12,6 +12,7 @@ from __future__ import annotations
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
from collections.abc import Iterator
|
||||
from pathlib import Path
|
||||
|
||||
import _boot # noqa: F401
|
||||
@@ -31,6 +32,9 @@ _INCLUDE_EXTENSIONS: frozenset[str] = CPP_EXTENSIONS | frozenset({'.mojom'})
|
||||
_COMMENT_EXTENSIONS: frozenset[str] = CPP_EXTENSIONS | frozenset(
|
||||
{'.gni', '.gn'})
|
||||
|
||||
# Extensions whose files contain GN references in double-quoted strings.
|
||||
_GN_EXTENSIONS: frozenset[str] = frozenset({'.gn', '.gni'})
|
||||
|
||||
# Walk filter rules, relative to the brave-core root.
|
||||
# Each entry is '+' (include) or '-' (exclude) followed by a path.
|
||||
# Bare-name patterns (no '/') match any directory component at any depth.
|
||||
@@ -175,6 +179,8 @@ def update_references(old_path: Path, new_path: Path) -> None:
|
||||
Handles:
|
||||
- #include / #import directives (quoted and angle-bracket) in C++/.mojom
|
||||
- // comment lines in C++ and build files
|
||||
- GN references in .gn/.gni files (root and relative; see
|
||||
_update_gn_references for the per-file-type rules)
|
||||
- BUILD.gn / .gni source-list entries in the ancestor chain of the old file
|
||||
- For moved .mojom files: derived generated-header paths
|
||||
|
||||
@@ -200,45 +206,34 @@ def update_references(old_path: Path, new_path: Path) -> None:
|
||||
mojom_rewrites.append(
|
||||
(pat, r'\g<1>' + new_base + suffix + r'\g<2>'))
|
||||
|
||||
for dirpath, dirnames, filenames in os.walk(repository.BRAVE_CORE_PATH):
|
||||
rel_dir = Path(dirpath).relative_to(
|
||||
repository.BRAVE_CORE_PATH).as_posix()
|
||||
dirnames[:] = [
|
||||
d for d in dirnames
|
||||
if _should_walk_dir(d if rel_dir == '.' else f'{rel_dir}/{d}')
|
||||
]
|
||||
if rel_dir != '.' and _is_path_excluded(rel_dir):
|
||||
for fpath in _walk_brave_core():
|
||||
ext = fpath.suffix.lower()
|
||||
do_includes = ext in _INCLUDE_EXTENSIONS
|
||||
do_comments = ext in _COMMENT_EXTENSIONS
|
||||
if not (do_includes or do_comments):
|
||||
continue
|
||||
for fname in filenames:
|
||||
fpath = Path(dirpath) / fname
|
||||
if fpath.suffix in SEARCH_EXCLUDE_EXTENSIONS:
|
||||
continue
|
||||
|
||||
ext = fpath.suffix.lower()
|
||||
do_includes = ext in _INCLUDE_EXTENSIONS
|
||||
do_comments = ext in _COMMENT_EXTENSIONS
|
||||
if not (do_includes or do_comments):
|
||||
continue
|
||||
content = fpath.read_text(encoding='utf-8')
|
||||
new_content = content
|
||||
|
||||
content = fpath.read_text(encoding='utf-8')
|
||||
new_content = content
|
||||
if do_includes:
|
||||
new_content = include_re.sub(include_sub, new_content)
|
||||
for pat, sub in mojom_rewrites:
|
||||
new_content = pat.sub(sub, new_content)
|
||||
|
||||
if do_includes:
|
||||
new_content = include_re.sub(include_sub, new_content)
|
||||
for pat, sub in mojom_rewrites:
|
||||
new_content = pat.sub(sub, new_content)
|
||||
if do_comments:
|
||||
lines = new_content.splitlines(keepends=True)
|
||||
new_lines = [
|
||||
line.replace(old_posix, new_posix) if
|
||||
line.lstrip().startswith('//') and old_posix in line else line
|
||||
for line in lines
|
||||
]
|
||||
new_content = ''.join(new_lines)
|
||||
|
||||
if do_comments:
|
||||
lines = new_content.splitlines(keepends=True)
|
||||
new_lines = [
|
||||
line.replace(old_posix, new_posix)
|
||||
if line.lstrip().startswith('//') and old_posix in line
|
||||
else line for line in lines
|
||||
]
|
||||
new_content = ''.join(new_lines)
|
||||
if new_content != content:
|
||||
fpath.write_text(new_content, encoding='utf-8', newline='\n')
|
||||
|
||||
if new_content != content:
|
||||
fpath.write_text(new_content, encoding='utf-8', newline='\n')
|
||||
_update_gn_references(old_path, new_path)
|
||||
|
||||
# Update BUILD.gn / .gni source-list entries in the ancestor chain.
|
||||
old_abs = repository.CHROMIUM_SRC_PATH / old_posix
|
||||
@@ -262,6 +257,121 @@ def patch_name_for(chromium_path: Path | str) -> str:
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _walk_brave_core() -> Iterator[Path]:
|
||||
"""Yields every non-excluded file path under brave-core.
|
||||
|
||||
Honours SEARCH_EXCLUDE_DIRS and SEARCH_EXCLUDE_EXTENSIONS via
|
||||
_should_walk_dir / _is_path_excluded.
|
||||
"""
|
||||
for dirpath, dirnames, filenames in os.walk(repository.BRAVE_CORE_PATH):
|
||||
rel_dir = Path(dirpath).relative_to(
|
||||
repository.BRAVE_CORE_PATH).as_posix()
|
||||
dirnames[:] = [
|
||||
d for d in dirnames
|
||||
if _should_walk_dir(d if rel_dir == '.' else f'{rel_dir}/{d}')
|
||||
]
|
||||
if rel_dir != '.' and _is_path_excluded(rel_dir):
|
||||
continue
|
||||
for fname in filenames:
|
||||
fpath = Path(dirpath) / fname
|
||||
if fpath.suffix in SEARCH_EXCLUDE_EXTENSIONS:
|
||||
continue
|
||||
yield fpath
|
||||
|
||||
|
||||
def _gn_token_re(prefix: str) -> re.Pattern[str]:
|
||||
"""Returns a regex matching `"prefix` at a GN reference token boundary.
|
||||
|
||||
Matches inside a double-quoted string, requiring the next character after
|
||||
`prefix` to be `:` (target separator), `/` (sub-path), or `"` (closing
|
||||
quote). The lookahead is zero-width so the substitution only replaces the
|
||||
leading `"prefix` portion.
|
||||
"""
|
||||
return re.compile(r'"' + re.escape(prefix) + r'(?=[:/"])')
|
||||
|
||||
|
||||
def _update_gn_references(old_path: Path, new_path: Path) -> None:
|
||||
"""Rewrites quoted GN references in .gn/.gni files across brave-core.
|
||||
|
||||
Scope is keyed off the moved file's name/extension:
|
||||
- BUILD.gn -> directory rename: rewrite root references
|
||||
(`"//<old_dir>` followed by `:`, `/`, or `"`) and per-file relative
|
||||
references (the relative path from each visited file's dir to the
|
||||
old/new dir, applied with the same token-boundary rule). In the
|
||||
moved BUILD.gn itself, also rewrite the implicit directory-name
|
||||
target -- both the declaration `"<old_basename>"` and any
|
||||
same-file label reference `:<old_basename>"` -- to use the new
|
||||
basename.
|
||||
- .gni or non-BUILD.gn .gn -> single-file move: rewrite the exact
|
||||
quoted root reference `"//<old_path>"`.
|
||||
- C++ source -> single-file move: rewrite the exact quoted root
|
||||
reference `"//<old_path>"`.
|
||||
- Other -> no-op.
|
||||
|
||||
Only edits content inside double-quoted strings. Skips files that fall
|
||||
outside _GN_EXTENSIONS.
|
||||
"""
|
||||
suffix = old_path.suffix.lower()
|
||||
|
||||
if old_path.name == 'BUILD.gn':
|
||||
old_dir = old_path.parent.as_posix()
|
||||
new_dir = new_path.parent.as_posix()
|
||||
if not old_dir or old_dir == '.':
|
||||
return
|
||||
old_root = '//' + old_dir
|
||||
new_root = '//' + new_dir
|
||||
root_re = _gn_token_re(old_root)
|
||||
root_sub = '"' + new_root
|
||||
old_abs_dir = repository.CHROMIUM_SRC_PATH / old_dir
|
||||
new_abs_dir = repository.CHROMIUM_SRC_PATH / new_dir
|
||||
for fpath in _walk_brave_core():
|
||||
if fpath.suffix not in _GN_EXTENSIONS:
|
||||
continue
|
||||
content = fpath.read_text(encoding='utf-8')
|
||||
new_content = root_re.sub(root_sub, content)
|
||||
rel_old = os.path.relpath(old_abs_dir,
|
||||
fpath.parent).replace('\\', '/')
|
||||
rel_new = os.path.relpath(new_abs_dir,
|
||||
fpath.parent).replace('\\', '/')
|
||||
if rel_old and rel_old != '.':
|
||||
rel_re = _gn_token_re(rel_old)
|
||||
new_content = rel_re.sub('"' + rel_new, new_content)
|
||||
if new_content != content:
|
||||
fpath.write_text(new_content, encoding='utf-8', newline='\n')
|
||||
|
||||
# In the moved BUILD.gn, rename the implicit directory-name target.
|
||||
old_basename = old_path.parent.name
|
||||
new_basename = new_path.parent.name
|
||||
if old_basename and old_basename != new_basename:
|
||||
new_build = repository.CHROMIUM_SRC_PATH / new_path
|
||||
if new_build.is_file():
|
||||
content = new_build.read_text(encoding='utf-8')
|
||||
new_content = content.replace(f'"{old_basename}"',
|
||||
f'"{new_basename}"')
|
||||
new_content = new_content.replace(f':{old_basename}"',
|
||||
f':{new_basename}"')
|
||||
if new_content != content:
|
||||
new_build.write_text(new_content,
|
||||
encoding='utf-8',
|
||||
newline='\n')
|
||||
return
|
||||
|
||||
if suffix not in _GN_EXTENSIONS and suffix not in CPP_EXTENSIONS:
|
||||
return
|
||||
|
||||
old_quoted = f'"//{old_path.as_posix()}"'
|
||||
new_quoted = f'"//{new_path.as_posix()}"'
|
||||
for fpath in _walk_brave_core():
|
||||
if fpath.suffix not in _GN_EXTENSIONS:
|
||||
continue
|
||||
content = fpath.read_text(encoding='utf-8')
|
||||
if old_quoted not in content:
|
||||
continue
|
||||
fpath.write_text(content.replace(old_quoted, new_quoted),
|
||||
encoding='utf-8',
|
||||
newline='\n')
|
||||
|
||||
|
||||
def _update_build_ancestors(old_abs: Path, new_abs: Path,
|
||||
brave_root: Path) -> None:
|
||||
"""Updates BUILD.gn/.gni entries in the ancestor dirs of old_abs."""
|
||||
|
||||
@@ -190,5 +190,246 @@ class UpdateReferencesFilterTest(unittest.TestCase):
|
||||
self._read('out/Default/gen/test.cc'))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Integration tests: GN reference rewriting in .gn/.gni files
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class UpdateGnReferencesTest(unittest.TestCase):
|
||||
"""update_references rewrites quoted GN references in .gn/.gni files."""
|
||||
|
||||
def setUp(self):
|
||||
self._repo = FakeChromiumSrc()
|
||||
self._repo.setup()
|
||||
self.addCleanup(self._repo.cleanup)
|
||||
|
||||
def _write(self, rel: str, content: str) -> Path:
|
||||
path = self._repo.brave / rel
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(content, encoding='utf-8')
|
||||
return path
|
||||
|
||||
def _read(self, rel: str) -> str:
|
||||
return (self._repo.brave / rel).read_text(encoding='utf-8')
|
||||
|
||||
# ----- BUILD.gn move: directory rename, root references -----
|
||||
|
||||
def test_build_gn_root_reference_rewritten(self):
|
||||
"""`"//brave/foo"` → `"//brave/bar"` when foo's BUILD.gn moves."""
|
||||
self._write('consumer/BUILD.gn',
|
||||
'deps = [ "//brave/components/api_request_helper" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"//brave/components/api_test"',
|
||||
self._read('consumer/BUILD.gn'))
|
||||
|
||||
def test_build_gn_root_reference_with_target_rewritten(self):
|
||||
"""`"//brave/foo:target"` is rewritten and target preserved."""
|
||||
self._write(
|
||||
'consumer/BUILD.gn',
|
||||
'deps = [ "//brave/components/api_request_helper:test_support" ]\n'
|
||||
)
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"//brave/components/api_test:test_support"',
|
||||
self._read('consumer/BUILD.gn'))
|
||||
|
||||
def test_build_gn_root_reference_with_subpath_rewritten(self):
|
||||
"""`"//brave/foo/sub"` is rewritten and subpath preserved."""
|
||||
self._write(
|
||||
'consumer/BUILD.gn',
|
||||
'sources = [ "//brave/components/api_request_helper/foo.h" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"//brave/components/api_test/foo.h"',
|
||||
self._read('consumer/BUILD.gn'))
|
||||
|
||||
def test_build_gn_similar_prefix_not_rewritten(self):
|
||||
"""`"//brave/foo_v2"` is NOT touched when only `foo` moved."""
|
||||
self._write('consumer/BUILD.gn',
|
||||
'deps = [ "//brave/components/api_request_helper_v2" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"//brave/components/api_request_helper_v2"',
|
||||
self._read('consumer/BUILD.gn'))
|
||||
|
||||
def test_build_gn_root_reference_in_gni_rewritten(self):
|
||||
"""The walk applies to .gni files too, not just BUILD.gn."""
|
||||
self._write(
|
||||
'config/sources.gni',
|
||||
'shared_deps = [ "//brave/components/api_request_helper" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"//brave/components/api_test"',
|
||||
self._read('config/sources.gni'))
|
||||
|
||||
# ----- BUILD.gn move: relative references -----
|
||||
|
||||
def test_build_gn_relative_sibling_rewritten(self):
|
||||
"""`"../api_request_helper"` from a sibling dir is rewritten."""
|
||||
self._write('components/ai_chat/BUILD.gn',
|
||||
'deps = [ "../api_request_helper" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"../api_test"',
|
||||
self._read('components/ai_chat/BUILD.gn'))
|
||||
|
||||
def test_build_gn_relative_from_parent_rewritten(self):
|
||||
"""`"components/api_request_helper:foo"` from brave/BUILD.gn."""
|
||||
self._write(
|
||||
'BUILD.gn',
|
||||
'deps = [ "components/api_request_helper:test_support" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"components/api_test:test_support"',
|
||||
self._read('BUILD.gn'))
|
||||
|
||||
def test_build_gn_implicit_target_renamed_in_moved_file(self):
|
||||
"""In the moved BUILD.gn, the dir-name target is renamed."""
|
||||
# update_references runs after the move; seed the file at its NEW
|
||||
# location.
|
||||
self._write(
|
||||
'components/api_foo/BUILD.gn',
|
||||
'static_library("api_request_helper") {\n'
|
||||
' sources = [ "api.cc" ]\n'
|
||||
'}\n'
|
||||
'source_set("test_support") {\n'
|
||||
' testonly = true\n'
|
||||
'}\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_foo/BUILD.gn'))
|
||||
content = self._read('components/api_foo/BUILD.gn')
|
||||
self.assertIn('static_library("api_foo")', content)
|
||||
self.assertNotIn('"api_request_helper"', content)
|
||||
# Unrelated targets are untouched.
|
||||
self.assertIn('source_set("test_support")', content)
|
||||
|
||||
def test_build_gn_same_file_label_ref_renamed_in_moved_file(self):
|
||||
"""In the moved BUILD.gn, `":<old_basename>"` label refs are renamed."""
|
||||
self._write(
|
||||
'components/api_foo/BUILD.gn',
|
||||
'static_library("api_request_helper") {\n'
|
||||
'}\n'
|
||||
'source_set("test_support") {\n'
|
||||
' public_deps = [ ":api_request_helper" ]\n'
|
||||
' deps = [ ":test_support_data" ]\n'
|
||||
'}\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_foo/BUILD.gn'))
|
||||
content = self._read('components/api_foo/BUILD.gn')
|
||||
self.assertIn('static_library("api_foo")', content)
|
||||
self.assertIn(':api_foo"', content)
|
||||
self.assertNotIn('api_request_helper', content)
|
||||
# Unrelated label refs untouched.
|
||||
self.assertIn(':test_support_data"', content)
|
||||
|
||||
def test_build_gn_implicit_target_not_renamed_in_other_files(self):
|
||||
"""An unrelated BUILD.gn that happens to have a target / label ref
|
||||
with the same name as the old directory must NOT be touched."""
|
||||
self._write('components/api_foo/BUILD.gn',
|
||||
'static_library("api_request_helper") {\n'
|
||||
'}\n')
|
||||
# Unrelated BUILD.gn elsewhere with a same-named target and label.
|
||||
self._write(
|
||||
'unrelated/BUILD.gn', 'static_library("api_request_helper") {\n'
|
||||
'}\n'
|
||||
'group("entry") {\n'
|
||||
' deps = [ ":api_request_helper" ]\n'
|
||||
'}\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_foo/BUILD.gn'))
|
||||
# Moved BUILD.gn renamed.
|
||||
self.assertIn('static_library("api_foo")',
|
||||
self._read('components/api_foo/BUILD.gn'))
|
||||
# Unrelated BUILD.gn untouched (both declaration and label ref).
|
||||
unrelated = self._read('unrelated/BUILD.gn')
|
||||
self.assertIn('static_library("api_request_helper")', unrelated)
|
||||
self.assertIn(':api_request_helper"', unrelated)
|
||||
|
||||
def test_build_gn_implicit_target_no_rewrite_when_basename_unchanged(self):
|
||||
"""Same-basename move (just reparenting) leaves target name alone."""
|
||||
self._write('apps/api_request_helper/BUILD.gn',
|
||||
'static_library("api_request_helper") {\n'
|
||||
'}\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/apps/api_request_helper/BUILD.gn'))
|
||||
self.assertIn('static_library("api_request_helper")',
|
||||
self._read('apps/api_request_helper/BUILD.gn'))
|
||||
|
||||
def test_build_gn_internal_target_ref_untouched(self):
|
||||
"""`":api_request_helper"` (internal target) must NOT be rewritten."""
|
||||
self._write('components/api_request_helper/BUILD.gn',
|
||||
'public_deps = [ ":api_request_helper" ]\n')
|
||||
# Pretend the BUILD.gn is being moved (file already at new location
|
||||
# for the test would be more realistic, but token boundary on the
|
||||
# leading `"` is what we want to verify).
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('":api_request_helper"',
|
||||
self._read('components/api_request_helper/BUILD.gn'))
|
||||
|
||||
# ----- .gni file move: root reference only -----
|
||||
|
||||
def test_gni_root_reference_rewritten(self):
|
||||
"""`"//tools/grit/repack.gni"` is rewritten when that file moves."""
|
||||
self._write('consumer/BUILD.gn', 'import("//tools/grit/repack.gni")\n')
|
||||
update_references(Path('tools/grit/repack.gni'),
|
||||
Path('tools/grit/new_repack.gni'))
|
||||
self.assertIn('"//tools/grit/new_repack.gni"',
|
||||
self._read('consumer/BUILD.gn'))
|
||||
|
||||
def test_gni_relative_reference_not_rewritten(self):
|
||||
"""A relative .gni reference is NOT rewritten on .gni move."""
|
||||
# File is in the same dir as the (hypothetically moved) .gni,
|
||||
# so a relative reference would be `"repack.gni"`.
|
||||
self._write('tools/grit/BUILD.gn', 'import("repack.gni")\n')
|
||||
update_references(Path('tools/grit/repack.gni'),
|
||||
Path('tools/grit/new_repack.gni'))
|
||||
self.assertIn('"repack.gni"', self._read('tools/grit/BUILD.gn'))
|
||||
|
||||
# ----- C++ file move: root reference in .gn/.gni only -----
|
||||
|
||||
def test_cpp_root_reference_in_build_gn_rewritten(self):
|
||||
"""`"//brave/foo/bar.h"` in BUILD.gn is rewritten when bar.h moves."""
|
||||
self._write(
|
||||
'consumer/BUILD.gn', 'sources = [\n'
|
||||
' "//brave/components/api_request_helper/api_request_helper.h"\n'
|
||||
']\n')
|
||||
update_references(
|
||||
Path('brave/components/api_request_helper/api_request_helper.h'),
|
||||
Path('brave/components/api_test/api_test.h'))
|
||||
self.assertIn('"//brave/components/api_test/api_test.h"',
|
||||
self._read('consumer/BUILD.gn'))
|
||||
|
||||
def test_cpp_relative_reference_in_build_gn_not_rewritten(self):
|
||||
"""A relative C++ source-list ref is NOT touched by the new helper.
|
||||
|
||||
(Same-dir BUILD.gn is handled by _update_build_ancestors instead.)
|
||||
"""
|
||||
# Sibling-dir BUILD.gn referencing the moved C++ file via a relative
|
||||
# path. The new helper must not rewrite it (per spec: only roots for
|
||||
# C++ moves). And _update_build_ancestors won't touch it either,
|
||||
# since it only walks ancestor dirs.
|
||||
self._write(
|
||||
'components/ai_chat/BUILD.gn',
|
||||
'sources = [ "../api_request_helper/api_request_helper.h" ]\n')
|
||||
update_references(
|
||||
Path('brave/components/api_request_helper/api_request_helper.h'),
|
||||
Path('brave/components/api_test/api_test.h'))
|
||||
self.assertIn('"../api_request_helper/api_request_helper.h"',
|
||||
self._read('components/ai_chat/BUILD.gn'))
|
||||
|
||||
# ----- Excluded dirs are skipped -----
|
||||
|
||||
def test_excluded_out_dir_skipped(self):
|
||||
"""A BUILD.gn under out/ is not rewritten."""
|
||||
self._write('out/Default/gen/BUILD.gn',
|
||||
'deps = [ "//brave/components/api_request_helper" ]\n')
|
||||
update_references(Path('brave/components/api_request_helper/BUILD.gn'),
|
||||
Path('brave/components/api_test/BUILD.gn'))
|
||||
self.assertIn('"//brave/components/api_request_helper"',
|
||||
self._read('out/Default/gen/BUILD.gn'))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user