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 change uses a new label for plaster reapply commits, as the
previous name had `Regen` in it, which can cause confusion with the
similar command `brockit regen`.
This change moves the script used to generate the rust WASM toolchain
under `tools/cr/toolchains`, as tooling packaging is a Chromium rebase
concern, and it is better kept under the rebase's team purview. A
separate PR has been submitted to fix CI.
This PR is pretty much just a rearranging of code, with no practical
function changes. Files have been renamed to be less repetitive and more
intuitive. There are few minor changes though:
1. `_boot.py` has been introduced to allow `alias/` sources to be able
to include `tools/cr` into path and make themselves a submodule. This
is not the best posssible solution, but it does avoid having to mess
with PATH or PYTHONPATH and any other similar constant before calling
these scripts.
2. `PRESUBMIT.py` has been updated to run tests in subdirectories.
This PR adds integration for `plaster` into `brockit`. This involves
primarily changes to how 3way apply works. With this change, whenever
3way apply fails due to conflicts or a broken patch, we take a next step
attempting to fix the issue by rerunning the plaster.
Why not rerun plaster always for any patch managed with a plaster?
It is important to maintain changes in a granular state that they can be
inspected. We don't want to do blanket `plaster` runs because that may
create a huge committable change, that would be harder to inspect and
specially harder to catch unintended plaster new interpretations based
in underlying Chromium changes.
This PR introduces a two step process for plaster:
1. The plaster re-run commit.
This commit is similar to the use of `Conflict-resolved` commits. It
will be commited with a message:
```
Regen-fixed 🩹 patches from Chromium VERSION to VERSION
```
This will be a pinned commit, and it will helps keep track of the
patches that did require a `plaster` rerun to apply again. This commit
follows the same rules of the `Conflict-Resolved` commit when it comes
to fixups for changes touching the patching in the same dev cycle.
2. The introduction of `plaster check`
At the end of the lift, we run `plaster check`. For the vast majority of
the cases, what the plaster rerun would have generated is exactly what
we got with simply reapplying the patch. However, for cases where the
patch re-applied, and yet a plaster re-run would somehow produce
something different, we want to alert about that, and have that
investigated, and committed with a culprit, to make such a change
visible during review.
It is important to notice that `plaster check` also runs during the
`presubmit` step, so even if not caught at this stage, it would be
brought to someone's attention at some point.
- Broken plasters
A new section in the continuation report has been added for broken
plasters, which are required to be either fixed or deleted before
calling `--continue`.
Resolves https://github.com/brave/brave-browser/issues/55188
These are helpers for the rebase process, in particular involve file
renames. These helpers are supposed to be intuitive on their use. These
commands do something similar, and there's overlap between them, but
they are for different purposes.
What happens when you use `git cr mv`:
* Move source to destination
* Correct shadowed file inclusion for `chromium_src`
* Corrects header guards for the new path
* Corrects all references elsewhere for that file inclusion
* Deletes stale patches if moving plasters.
What happens when you use `git cr follow-renames`:
Follow renames has a narrow purpose of taking git changes, checking
which files got renamed, and moving only those particular files to a new
location. It will do nearly everything else identical to `git cr mv`.
Resolves https://github.com/brave/brave-browser/issues/55189
This change simplifies a lot of the code around `Patchfile`, but
expecting instances of `Patchfile` to be provided with the expected
`source` value for the source file to be patched. This eliminates a lot
of the creative ways how we were detecting that information.
We can use a single git call to retrieve the underlying source for all
patches, which this change does.
Additionally, there is some sprucing up of the codebase, to make
consitent uses of `Path`. The use of `PurePath` creates the need for a
lot of conversions, so we are making this more consistent.
Resolves https://github.com/brave/brave-browser/issues/55177
The way `brockit` was handling `--3way` apply failures was always
assuming that the first line would have error. This has proven wrong,
and caused a crash when whitespace warnings were print first. This
change corrects the logic to look for the error line.
Resolves https://github.com/brave/brave-browser/issues/55170
Before this change, we had to pass the environment variables manually
before every commit:
```
tags=WIP issue=1234 git commit -m "message"
```
This was fragile in several ways: typos like `tgas=test` are
silently swallowed by the shell and the hook never sees them,
`key=value` prefixes break shell autocompletion from the common
command-line starter, and the hook was traditionally copied into
.git/hooks/ where it went stale whenever commit-msg.py changed
upstream.
git_cr.py fixes all three problems with a single script that
registers as `git cr` via `git config --local alias.cr`:
```
git cr commit --tagged WIP --issue 1234 -m "My change"
```
The --tagged, --issue, and --culprit flags are parsed and
injected as the environment variables the hook expects ($tags,
$issue, $culprit). Unknown flags are forwarded verbatim to
git commit so git's own error messages are preserved.
git cr install-hook — creates a symlink on POSIX (or a bash
shim on Windows) so the hook tracks
changes to commit-msg.py automatically.
git cr setup-alias — writes alias.cr to .git/config without
touching any shell config files.
cmd_commit performs a sanity check before running git commit:
if the hook is absent or not executable it prints a clear
message pointing the user to `git cr install-hook`.
Resolved: https://github.com/brave/brave-browser/issues/55078
Patch files flagged for conflict resolution in most cases will end up
committed by `brockit` in a "Conflict-Resolved" change. These
"Conflict-Resolved" commits tend to occur daily with brockit, and they
eventually get squashed when calling `rebase --squash-minor-bumps`. The
issue this PR solves, is that when a patch is added/changed during the
`cr` development cycle, that patch may end up becoming a conflict
resolution candidate, and added to a "Conflict-Resolved" commit, which
will eventually be picked up with `rebase --squash-minor-bumps` for
squashing, and this will cause the commit to be moved prior to the patch
being added/modified, resulting in a rebase conflict.
This PR adds additional handling committing conflict-resolved patches,
where a lookup for the patch in the branch history is carried out, to
verify if any changes in the branch dev cycle are relevant for that
patch, and if so, to commit the conflict-resolved patch as a `fixup!`
for that change. This way, the next `rebase --squash-minor-bumps`
squahses the fixup to the relevant changes, and no conflicts arise from
having a "Conflict-resolved" patch being squashed with changes that have
not been introduced yet.
This approach is only employed for lifts in branches doing major version
upgrades, as these branches are worked on an incremental schedule, where
the need for this type of handling becomes apparent.
Resolves https://github.com/brave/brave-browser/issues/54994
This chnage is adding logs to this `temp_dir.cleanup()` call, to provide
more detailed errors when this call fails. This call has started failing
recently on TeamCity, and it is not very clear why.
Bug https://github.com/brave/brave-browser/issues/55013
This change drops the `--vscode` from `brockit lift` and introduces an
auto-detect approach that checks for the presence of a vscode terminal
session socket, and uses that socket to open files if detected as
opening sources that require conflict attention is most likely always
desired.
This change also abandons the use of `code` command call to open
sources, and uses the socket for that. This fixes the problem that code
would send the command to open the sources to whatever was the
foreground vscode window the user had, which would be the wrong one when
multitasking.
Resolves https://github.com/brave/brave-browser/issues/54989
This PR fixes `lift` being broken due to git status output being
trimmed, and tilting the xy axis for git status. On top of that, this PR
overhauls `GitStatus` to be more accurate and comprehensive, as there
were a few corner cases.
This PR adds the following labels to `brockit`, for fetching the latest
available tags in Chromium:
```
+---------------------------+------------------------------------------------+
| Labels | Description |
+---------------------------+------------------------------------------------+
| @latest-tag | Latest tag in the Chromium repo |
| @latest-m{MAJOR} | Latest tag for the given major version |
| @latest-for-branch | Latest tag for the major inferred from the |
+---------------------------+------------------------------------------------+
```
This will simplify the way how the tag is retrieved for the daily
rebase, as it doesn't really matter if the tag is a canary tag yet or
not.
This PR does a series of clean ups to this script on the initialisation
process, by removing complex env and const initialisation from the
global scope, and keeping them closer to where they are used, and behind
the conditionals that guard their use.
This change also removes all code from `ToolchainBuilder` that is not
strictly involved with field value initialisations, so we we are left
with guarantees that all member data fields are initialised by the time
they are supposed to be used.
There are removal for unnecessary checks for `mkdir` operations that
will fail anyway if they cannot perform, as well as for checks if a
directory is a file, which can be avoided by checking an expected file
under the directory.
Finally, this change corrects our use of `depot_tools`, by making sure
we deploy a copy of it as a sibling path to `src`, rather than using the
one under `src/third_party` as that one should be left pinned to a
particular version chose by Chromium, and using its `gclient`
innevitably leads to it self-updating.
This PR introduces a check in brockit, to go over all modified patch
files that are about to be committed, be it as updated patches, or
conflict-resolved, and checks if any of them have a different number of
hunks, and if so, stops the process for intervention, the same way it is
done when deleted patches are detected.
This check will prevent cases where hunks of a patch file are being
dropped, either through `apply_patches`, or through 3way resolution,
which should be of interest, and definitely should be submitted
separately as an individual fix.
Resolves https://github.com/brave/brave-browser/issues/47717
* Upgrade jest to 30.3.0
Remove use of https://github.com/brave/jest
# Conflicts:
# package-lock.json
* more improvements that can be made with jest upgrade
* format
This PR adds several Windows-specific features to this script:
- Adds git's `bin` path to `PATH`, which is required by
`build_rust.py`. This is only done this `sh.exe` is not resolveable.
- Adds support for using hermetic toolchain when checking out
Chromim, so we don't have to install VS Studio in the CI node.
- Initialises git global configs, which may be required by Chromium's
upstream toolchain scripts to cherry-pick/commit changes into the
downloaded repositories.
Additinaly this changes adds `--with-git-cache`, which has the script
setting up the presumed git cache path to user's home directory.
Finally, this change also improves `depot_tools`'s booststraping
routines to be able to pick up a previous installation of `depot_tools`
under `src` and add it to `PATH` if it is not there yet.
Bug: https://github.com/brave/brave-browser/issues/54478
This PR changes the script for posting presubmit reports to github, to
print the `stderr` output from process that have exited with an error
code. This will let us see why a particular API call failed at a given
moment.
This PR adds special handling to how we launch processes with
`subprocess` when running on Windows, to take into account any cases
where it is required to pass `.bat` to the process name.
This is a safer and better workaround to this issues than using
`shell=True`.
Bug: https://github.com/brave/brave-browser/issues/54478
This PR introduces support for checking out a clean Chromium repo using
`build_rust_toolchain_standalone.py`. Two options have been introduced:
- `--clone-chromium`: With this option the script will proceed to
cloning Chromium if a checkout is not found on the path provided as
`--chromium-src`.
- `--use-ref`: This option indicates which Chromium git ref should be
used to generate the toolchain.
Bug: https://github.com/brave/brave-browser/issues/54478
This PR adds a script that permits us to generate a wasm rust toolchain
for Brave, relying only on a vanilla Chromium checkout. Functionally,
this script is meant to serve as a replacement to
`build_rust_toolchain_aux`. It can be called simply with:
```sh
./tools/rust/build_rust_toolchain_standalone.py --out-dir=./out/ --chromium-src=../
```
Long term, this script will let us move away from the complexities of
checking out brave when generating this toolchain, which adds extra
complexity on top, when trying to figure out failures during this
process. It is also more natural to approach this problem this way, as
toolchain updates in Chromium cause sync breakages in Brave, however
with `npm run build_rust_toolchain_aux` there's an expectation for a
full valid sync, which creates a chicken-and-egg type of problem.
Additional work will be made to roll this approach into CI.
Bug: https://github.com/brave/brave-browser/issues/54478
This PR adds authorship reassingment to brockit. This feature will allow
anyone to reassing the author of any given commit in a `cr` branch. This
is important to maintain our git history closer to the reality of who
should have attribution for each change.
The basics are:
```sh
tools/cr/brockit.py reassign <commit_hash>
```
The `reassign` command creates an empty commit with a `reassing!`
prefix, similar to a simple `fixup` change. However, this is only then
picked up when running `rebase` with `--squash-minor-bumps`.
```
tools/cr/brockit.py rebase --squash-minor-bumps
```
At this point the reassign change is moved above the original change,
and the original change is then squashed into the reassign change,
resulting on a natural change of authors as the original commit is
collapsed into the reassignment one.
Notes on discussions relating to this feature design:
In this particular implementation for this PR, it is necessary to call
rebase `--squash-minor-bumps` to have the necessary
reordering/squashing dance to take place that results in the authorship
being swapped (it is perfectly possible to drop `--squash-minor-bumps`
as a requirement, and make reassignment handling integral to regular
brockit rebase but I kept it behind that flag for now for the sake of
expediency).
- Pros: with this method, one can flag a commit for reassignment, but
doesn't need to immediately force push the branch, as the
reassignment only takes place once the daily rebase occurs.
- Cons: Brockit handles the commit message readjustment that is
necessary when squashing the original commit into the reassignment
commit, however if brockit rebase `--squash-minor-bumps` runs into
rebase conflicts, this breaks the editing chain, and requires the
user to manually call `git rebase --continue` and manually fix the
commit message for the squash. This is not a big deal as we already
do this for the minor bumps messages whenever conflicts occur, but
it is a break on automation flow. There could be room for something
like `brockit rebase --continue` to be honest. The other issue is
that we are adding extra tasks to the whole rebase thing, and
unknown bugs could end up interacting with each other (hopefully
won't ever be the case). Of less importance, but still of notice,
there are particular cases I have not completely tested, like
reassigning a bump commit (e.g. Conflict-resolved patches from
Chromium, etc) that is supposed to be squashed when squashing bumps,
so there could be concerns about both types of squash interacting,
which would either require better rebase checks for these corner
cases, or preventing users from reassigning authorship for brockit
generated commits (both options a no biggie).
There is a different way of going about this that this PR has not
pursued: `brockit reassing` immediately rebases doing the whole
reordering/squashing. This means each call to reassign does create a
`reassign!` commit but it immediately runs an interactive rebase to
squash it with the change it is targeting.
- Pros: no chance for conflicts during this process, which means that
the user would never have to deal with occasionally having to edit
the the commit message due to rebase conflicts. It also removes any
of these concerns from brockit's rebasing code, leaving less room
for bugs, as no squashing of fixups would be taking place.
- Cons: An imediate `rebase --interactive` would mean partial
rewrites of the tree, which requires force pushes to origin.
This alternative approach seemed a bit more work to implement. Later,
if preferred, we could also provide this second option with a flag
(e.g. `brockit reassign --now`) and reuse most of the implementation
details for the rebase case.
Resolves https://github.com/brave/brave-browser/issues/54151
* Add --update-crate flag to update a single vendored tool crate
Adds a --update-crate=<name>@<version> option to tools/crates/update.py
that updates one crate in-place without running cargo vendor. It downloads
the tarball directly from crates.io, replaces the vendor directory, patches
Cargo.lock, and regenerates .cargo-checksum.json. This avoids collateral
version bumps from a full re-vendor and is useful for targeted security
advisory fixes.
Exposes the flag via `npm run update_brave_tools_crates -- --update-crate=`.
* [Plaster] Fixes for Windows.
- When running commands make sure the output is in UTF-8, otherwise on
Windows the output is using cp1252 which then can't be handled by
UTF-8 decoder.
- When updating files (patches after applying plaster, or target source
files) make sure to use LF line endings on Windows. The repo files are
checked out as LF and we shouldn't introduce CRLF line ending during
plaster apply operation.
* [Brockit] Fixes Win toolchain message.
When checking for Windows toolchain changes also check TOOLCHAIN_HASH.
The SDK version doesn't always get updated while the hash can change and
contain a different minor SDK update.
* Review feedback fix.
This change adds support for file paths to be listed when passing
`plaster apply`. This changes `apply` to go over all files, and just
apply specific ones.
This change will allow us to integrate plaster calls in brockit for
files that fail to reapply.
Resolves https://github.com/brave/brave-browser/issues/53512
cdesouza-chromium