[Findit] Improve output format and cherry-pick bugs fix.

[Findit] Improve output format and cherry-pick bugs fix.

1. For DEPS file parsing, switching from svn to git.

2. For output of testcase https://cluster-fuzz.appspot.com/testcase?key=5097237064450048, now it is:

Lines 1103-1112 of file ThreadState.cpp which potentially caused crash are changed in this cl (frame #2, function "blink::ThreadState::performPendingSweep()").

Lines 1120-1136, 1170-1174 of file Heap.cpp which potentially caused crash are changed in this cl (frame #0, function "blink::ThreadHeap<blink::FinalizedHeapObjectHeader>::sweepNormalPages(blink::HeapStats*)"; frame #1, function "blink::ThreadHeap<blink::FinalizedHeapObjectHeader>::sweep(blink::HeapStats*)").
Minimum distance from crashed line to changed line: 0. (File: ThreadState.cpp, Crashed on: 1100, Changed: 1100).

NOTRY=true

Committed: https://crrev.com/6b6cd502e50d93232afee8f993dc47a563f74c84
Cr-Commit-Position: refs/heads/master@{#291987}

[stgao, 2014-08-26 01:55:54]

stgao@chromium.org changed reviewers:
+ inferno@chromium.org, mbarbella@chromium.org

[aarya, 2014-08-26 02:13:53]

aarya@google.com changed reviewers:
+ aarya@google.com

[aarya, 2014-08-26 02:13:53]

mostly done. some things to fix.

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
File tools/findit/chromium_deps.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
tools/findit/chromium_deps.py:77: http_status_code, content =
utils.GetHttpClient().Get(url, timeout=60)
404 might cause an exception which will cause deps_file_name to not update to
'DEPS'. Did you check that there is no exception thrown. Try with some latest
chromium revision.

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
File tools/findit/chromium_deps_unittest.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
tools/findit/chromium_deps_unittest.py:40: def testGetChromiumComponents(self):
Can you do the testing on these. these test an interesting case where webkit
revision is svn on one, and git on another. if you are using .DEPS.git
correctly, then both should be git hashes.

chromium
745dac28f8e454766824d6fefcd8063f1f11b1cb:744746cc51ef81c8f8d727fafa46b14d1c03fe44

for
https://chromium.googlesource.com/chromium/src/+/745dac28f8e454766824d6fefcd8...
"webkit_revision": "180779",

for
https://chromium.googlesource.com/chromium/src/+/744746cc51ef81c8f8d727fafa46...
'webkit_revision': '04b6022fdfb31a848d67683b58cd785aaeba9a69',

https://codereview.chromium.org/504443004/diff/20001/tools/findit/common/util...
File tools/findit/common/utils.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/common/util...
tools/findit/common/utils.py:34: def JoinLineNumbers(line_numbers,
accepted_gap=1):
Please don't use accepted_gap. When someone modifies a long section, it will
still cause lots of x-y pairs. I know you passed 4, but we want to club as many
as possible. Just go through the loop

5,6,7,8,9,10,11,12,14,15,16,17,18 should be

5-12,14-18 and not groups of 4.

https://codereview.chromium.org/504443004/diff/20001/tools/findit/git_reposit...
File tools/findit/git_repository_parser.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/git_reposit...
tools/findit/git_repository_parser.py:17: }
I fixed this by guess. I was crashing in rename key not found. can you check how
FILE_CHANGE_TYPE_MAP is used and if R makes sense.

[stgao, 2014-08-26 18:35:24]

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
File tools/findit/chromium_deps.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
tools/findit/chromium_deps.py:77: http_status_code, content =
utils.GetHttpClient().Get(url, timeout=60)
On 2014/08/26 02:13:53, aarya wrote:
> 404 might cause an exception which will cause deps_file_name to not update to
> 'DEPS'. Did you check that there is no exception thrown. Try with some latest
> chromium revision.

Good catch. Fix both here and the http_client.

There is a testcase with chromium revision after GIT migration.
I also improved it.

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
File tools/findit/chromium_deps_unittest.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/chromium_de...
tools/findit/chromium_deps_unittest.py:40: def testGetChromiumComponents(self):
On 2014/08/26 02:13:53, aarya wrote:
> Can you do the testing on these. these test an interesting case where webkit
> revision is svn on one, and git on another. if you are using .DEPS.git
> correctly, then both should be git hashes.
> 
> chromium
>
745dac28f8e454766824d6fefcd8063f1f11b1cb:744746cc51ef81c8f8d727fafa46b14d1c03fe44
> 
> for
>
https://chromium.googlesource.com/chromium/src/+/745dac28f8e454766824d6fefcd8...
> "webkit_revision": "180779",
> 
> for
>
https://chromium.googlesource.com/chromium/src/+/744746cc51ef81c8f8d727fafa46...
> 'webkit_revision': '04b6022fdfb31a848d67683b58cd785aaeba9a69',

Good suggestion.
I added a new test testComponentRangeCrossGitMigrationPoint.

https://codereview.chromium.org/504443004/diff/20001/tools/findit/common/util...
File tools/findit/common/utils.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/common/util...
tools/findit/common/utils.py:34: def JoinLineNumbers(line_numbers,
accepted_gap=1):
On 2014/08/26 02:13:53, aarya wrote:
> Please don't use accepted_gap. When someone modifies a long section, it will
> still cause lots of x-y pairs. I know you passed 4, but we want to club as
many
> as possible. Just go through the loop
> 
> 5,6,7,8,9,10,11,12,14,15,16,17,18 should be
> 
> 5-12,14-18 and not groups of 4.
> 

I think I didn't make it clear for the meaning of gap.
I added a more detail comment with example.

If |accepted_gap| = 1, the result of your example is 5-12, 14-18 as you
expected. If |accepted_gap| = 4, the result is 5-18 instead.

The reason I use |accepted_gap| = 4 is: for the testcase we discussed in the
email, the line number sequence is 1120, 1121, 1123, 1124, 1126, 1127, 1128,
1132, 1134, 1135, 1136, 1176, 1170, 1173, 1174, 1175

with gap=1, the result is 1120-1124, 1126-1128, 1132, 1134-1136, 1176, 1170,
1173-1175
with gap=4, the result looks better: 1120-1136, 1170-1174

https://codereview.chromium.org/504443004/diff/20001/tools/findit/git_reposit...
File tools/findit/git_repository_parser.py (right):

https://codereview.chromium.org/504443004/diff/20001/tools/findit/git_reposit...
tools/findit/git_repository_parser.py:17: }
On 2014/08/26 02:13:53, aarya wrote:
> I fixed this by guess. I was crashing in rename key not found. can you check
how
> FILE_CHANGE_TYPE_MAP is used and if R makes sense.

I will check this after lunch.

https://codereview.chromium.org/504443004/diff/40001/tools/findit/git_reposit...
File tools/findit/git_repository_parser.py (right):

https://codereview.chromium.org/504443004/diff/40001/tools/findit/git_reposit...
tools/findit/git_repository_parser.py:16: 'rename': 'R'
I will address this one after lunch.
Just wanted other change get reviewed first.

[aarya, 2014-08-26 19:01:23]

lgtm

https://codereview.chromium.org/504443004/diff/40001/tools/findit/common/http...
File tools/findit/common/http_client_local.py (right):

https://codereview.chromium.org/504443004/diff/40001/tools/findit/common/http...
tools/findit/common/http_client_local.py:213: if timeout is not None:
None timeout is just ignored, i dont think you need if-else

[stgao, 2014-08-26 20:22:36]

Patchset #4 (id:60001) has been deleted

[stgao, 2014-08-26 20:25:23]

Patchset #4 (id:80001) has been deleted

[stgao, 2014-08-26 20:30:47]

Change is done.

Waiting for the result from benchmark of 86 testcases.
When no exception and result looks good, I will commit this Cl and update CF.

https://codereview.chromium.org/504443004/diff/40001/tools/findit/common/http...
File tools/findit/common/http_client_local.py (right):

https://codereview.chromium.org/504443004/diff/40001/tools/findit/common/http...
tools/findit/common/http_client_local.py:213: if timeout is not None:
On 2014/08/26 19:01:23, aarya wrote:
> None timeout is just ignored, i dont think you need if-else

Nice. That makes the code cleaner.

[aarya, 2014-08-26 20:34:19]

lgtm

https://codereview.chromium.org/504443004/diff/100001/tools/findit/git_reposi...
File tools/findit/git_repository_parser.py (right):

https://codereview.chromium.org/504443004/diff/100001/tools/findit/git_reposi...
tools/findit/git_repository_parser.py:214: if file_change_type in ('A', 'R'):
you forgot 'C'

[stgao, 2014-08-26 20:50:35]

https://codereview.chromium.org/504443004/diff/100001/tools/findit/git_reposi...
File tools/findit/git_repository_parser.py (right):

https://codereview.chromium.org/504443004/diff/100001/tools/findit/git_reposi...
tools/findit/git_repository_parser.py:214: if file_change_type in ('A', 'R'):
On 2014/08/26 20:34:18, aarya wrote:
> you forgot 'C'

Done.

[aarya, 2014-08-26 21:14:28]

lgtm

[stgao, 2014-08-26 21:49:25]

The CQ bit was checked by stgao@chromium.org

[commit-bot: I haz the power, 2014-08-26 21:50:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/stgao@chromium.org/504443004/120001

[commit-bot: I haz the power, 2014-08-26 21:53:37]

Committed patchset #5 (120001) as eefbd23c91e94a5df2df3a2c539f03f17b3f69aa

[commit-bot: I haz the power, 2014-09-10 02:45:48]

Patchset 5 (id:??) landed as
https://crrev.com/6b6cd502e50d93232afee8f993dc47a563f74c84
Cr-Commit-Position: refs/heads/master@{#291987}