New issue
Advanced search Search tips

Issue 701516 link

Starred by 2 users

Issue metadata

Status: Closed
Merged: issue 584560
Owner: ----
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

gitiles blame is incorrect for some files after move/rename

Project Member Reported by jamescook@chromium.org, Mar 14 2017

Issue description

I recently renamed ~600 files in //ash, then reverted the renames (moving the files back to their original locations).

Example file move: https://codereview.chromium.org/2732813002/ and revert: https://codereview.chromium.org/2731873002/

Usually git can handle tracking blame across these sorts of renames. But gitiles is doing it wrong sometimes, and it makes gitiles blame much less useful for the files I moved.

Locally running "git blame ash/common/system/tray/system_tray.cc" shows multiple authors and useful history:

bfb411316d5d6 ash/system/tray/system_tray.cc        (sadrul@chromium.org    2012-02-27 20:59:27 +0000   1) // Copyright (c) 2012 The Chromium Authors. All rights reserved.
bfb411316d5d6 ash/system/tray/system_tray.cc        (sadrul@chromium.org    2012-02-27 20:59:27 +0000   2) // Use of this source code is governed by a BSD-style license that can be
bfb411316d5d6 ash/system/tray/system_tray.cc        (sadrul@chromium.org    2012-02-27 20:59:27 +0000   3) // found in the LICENSE file.
bfb411316d5d6 ash/system/tray/system_tray.cc        (sadrul@chromium.org    2012-02-27 20:59:27 +0000   4) 
6def4d9d43afe ash/common/system/tray/system_tray.cc (James Cook             2017-03-05 14:13:47 -0800   5) #include "ash/common/system/tray/system_tray.h"
bfb411316d5d6 ash/system/tray/system_tray.cc        (sadrul@chromium.org    2012-02-27 20:59:27 +0000   6) 
ae1d2fc49e3cb ash/common/system/tray/system_tray.cc (bruthig                2016-11-15 16:12:54 -0800   7) #include <algorithm>
ae1d2fc49e3cb ash/common/system/tray/system_tray.cc (bruthig                2016-11-15 16:12:54 -0800   8) #include <map>
ae1d2fc49e3cb ash/common/system/tray/system_tray.cc (bruthig                2016-11-15 16:12:54 -0800   9) #include <vector>

But gitiles just shows all the lines as changed by my rename:

https://chromium.googlesource.com/chromium/src/+blame/master/ash/common/system/tray/system_tray.cc

James Cook	6def4d9	2017-03-05 22:13:47	[diff] [blame]	1	// Copyright (c) 2012 The Chromium Authors. All rights reserved.

(and that commit is used for all following lines that haven't changed since the rename date).

Is something wrong with our gitiles config?

 
Components: -Infra Infra>Git

Comment 2 by msw@chromium.org, Apr 17 2017

Cc: dpranke@chromium.org e...@chromium.org
Labels: Hotlist-Fixit
This would be really nice to fix. It's hard to seek some ash/* git history now.
It takes manually rewriting the URL 3+ times to get a good blame on many moved files:

1) ToT blame on ash/shelf/shelf_model.h shows just James' latest move:
 https://chromium.googlesource.com/chromium/src/+blame/67c41de111691e8cfb173c6cb1f31ee1d24b5dfe/ash/shelf/shelf_model.h
2) Clicking 'blame' and then 'blame^' for that goes to a broken link... I rewrite the path to ash/common/*
 https://chromium.googlesource.com/chromium/src/+blame/1c4d759e44259650dfb2c426a7f997d2d0bc73dc/ash/common/shelf/shelf_model.h
3) The file history there also overwhelmingly points to James' prior move:
 https://chromium.googlesource.com/chromium/src/+blame/643b718269be655bc0a1a6136d6c09f2e92e4bb4/ash/common/shelf/shelf_model.h
4) Again, clicking 'blame^' goes to a broken link... I rewrite the path without 'common' to ash/*
 https://chromium.googlesource.com/chromium/src/+blame/6316a55cb90ab01ff798abfc0555d0b95ff78490/ash/shelf/shelf_model.h
5) Again, clicking 'blame' and 'blame^' goes to a broken link... I rewrite the path with to ash/common/* again!
 https://chromium.googlesource.com/chromium/src/+blame/d2a2b8d292f187116c849444e332a60a86aa4010/ash/common/shelf/shelf_model.h
And finally I have a reasonable blame on the file that's similar to my local "git blame ash/shelf/shelf_model.h"

Elliot or Dirk, can we eliminate James' moves from git history, or minimize the pain here somehow? Who would know how to help?
Cc: primiano@chromium.org iannucci@chromium.org aga...@chromium.org
I'm not sure how many different options you have. We're not going to remove the changes from the git history, we only do that for things like security issues, because rewriting history has all sorts of unpleasant side effects.

You can try `git hyper-blame`, which is a script in depot_tools, which you can tell to ignore certain commits. That's probably your best bet. 

There may be alternatives; I'll see if the git gurus have any advice.
Mergedinto: 584560
Status: Duplicate (was: Untriaged)
> I'm not sure how many different options you have. We're not going to remove the changes from the git history, we only do that for things like security issues, because rewriting history has all sorts of unpleasant side effects.
Fully agree with Dirk. Rewriting commits in the main repo is something afaik unprecedented (when we did the blink merge, we rewrote the history of blink *before* merging it into the main project). Doing that causes all sort of problems with buildbots, perf bisect bots, release scripts etc.

Big renames in a large codebase are natural and the solution is not really into rewriting the git history but in making the tools deal with that. 

> You can try `git hyper-blame`, which is a script in depot_tools, which you can tell to ignore certain commits. That's probably your best bet. 
+1 . At very least you should add your commits to src/.git-blame-ignore-revs, so that hyper-blame will ignore those.
Doing that, however, wouldn't solve the problem with Gitiles.
That is tracked in Issue 584560 (Port git hyper-blame to JGit / gitiles). That is IMHO the real problem to solve.
However, my impression is that such problem is currently not deemed to be prioritary enough, so won't be solved until either somebody manages to escalate its priority or somebody volunteers to fix it. 
Status: Untriaged (was: Duplicate)
I think the root problem is that gitiles shows *different* results than local `git blame`.

git hyper-blame would be nice, but is orthogonal.

Can you can explain why gitiles doesn't match local blame behavior?

> I think the root problem is that gitiles shows *different* results than local `git blame`.

Oh, right, I fully agree with you. Sorry I lost context on this bug, but now I realize that this was the initial point in #0, before we started discussing git history rewrite and hyper-blame.
I am curious as you are about this (and it's the reason, together with its speed, why I personally don't use blame on gitiles). 
However, I am not sure this is the right venue for this bug and I think this should be an internal b/ bug against Gitiles. 

Comment 8 by e...@chromium.org, Mar 9 2018

Cc: -e...@chromium.org
Un-cc-ing me from all bugs on my final day.
Cc: -iannucci@chromium.org iannu...@google.com
Status: Closed (was: Untriaged)
I'm closing this bug since it is better tracked by the Buganizer bug from #7. 

Sign in to add a comment