New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 598138 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 578344



Sign in to add a comment

clang tooling: refactor run_tool.py to separate edit generation from edit application

Project Member Reported by dcheng@chromium.org, Mar 26 2016

Issue description

Currently, run_tool.py generates a diff for each platform. But then we'll need some way to combine all the diffs together, which could be tricky: if the tool rewrites blocks of code that look like:

#if defined(OS_WIN)
  // stuff
#elif defined(OS_MACOSX)
  // other stuff
#else
  // even more other stuff
#endif

There's the possibility of merge conflicts when we try to combine all the patches.

Instead, if we separate edit generation from edit application, we can run the tool in parallel on Windows, Mac, Linux, CrOS, etc and build a list of all the edits.

Then we can feed all the edits into the edit applier, which can dedupe them and apply them all in pass with no conflicts.

A bonus side effect of splitting this apart means that run_tool.py will be better componentized, hopefully making it easier to debug tooling failures as well. 
 
Labels: -Clang clang
Owner: nasko@chromium.org
Status: Assigned (was: Available)

Comment 2 by dcheng@chromium.org, Dec 21 2016

Cc: -dcheng@chromium.org lukasza@chromium.org nasko@chromium.org
Owner: dcheng@chromium.org
This is blocking the rebase helper, so I'll take this.
Owner: lukasza@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9b89e7bd49caf2c5137bd540528001d36b577bc

commit f9b89e7bd49caf2c5137bd540528001d36b577bc
Author: lukasza <lukasza@chromium.org>
Date: Wed Dec 28 19:43:06 2016

Split run_tool.py into run_tool.py, extract_edits.py and apply_edits.py

The split will allow generation of edits on multiple configs (e.g. linux
vs windows OR rel vs dbg) and merging the edits before applying them once:
    $ tools/clang/scripts/run_tool.py rewrite_to_chrome_style \
        --generate-compdb --all out/rel >run_tool.linux.rel.out
    $ ...
    $ cat run_tool.*.out \
        | tools/clang/scripts/extract_edits.py \
        | tools/clang/scripts/apply_edits.py
        --generate-compdb --all out/rel >run_tool.linux.rel.out

Test steps:
- tools/clang/translation_unit/test_translation_unit.py
- tools/clang/scripts/test_tool.py rewrite_to_chrome_style
- manually running run_tool | extract_edits | apply_edits pipeline
  on WTF and verifying that it still builds after the rename

BUG= 598138 
TEST=See "Test steps" above.

Review-Url: https://codereview.chromium.org/2599193002
Cr-Commit-Position: refs/heads/master@{#440881}

[modify] https://crrev.com/f9b89e7bd49caf2c5137bd540528001d36b577bc/docs/clang_tool_refactoring.md
[add] https://crrev.com/f9b89e7bd49caf2c5137bd540528001d36b577bc/tools/clang/scripts/apply_edits.py
[add] https://crrev.com/f9b89e7bd49caf2c5137bd540528001d36b577bc/tools/clang/scripts/extract_edits.py
[modify] https://crrev.com/f9b89e7bd49caf2c5137bd540528001d36b577bc/tools/clang/scripts/run_tool.py
[modify] https://crrev.com/f9b89e7bd49caf2c5137bd540528001d36b577bc/tools/clang/scripts/test_tool.py
[modify] https://crrev.com/f9b89e7bd49caf2c5137bd540528001d36b577bc/tools/clang/translation_unit/TranslationUnitGenerator.cpp

Comment 5 by dcheng@chromium.org, Dec 29 2016

Status: Fixed (was: Started)

Sign in to add a comment