llvm dev - Jul 2019 - RFC: changing variable naming rules in LLVM codebase & git-blame

As a very frequent explorer of history, I really don't think this is
as big an issue as it may seem. Even absent refactorings, you often
run into the "wrong" commit when looking at blame (e.g., someone just
added a comma rather than actually changing the code you care about),
and have to look past that, to another previous commit.

Any interactive blame tool ought to have an easy way to do this. For
example, in emacs's annotation mode (which is what I use), you just
press 'a' with the cursor on the line in question to re-annotate at
the commit previous to that.

On Tue, Jul 23, 2019 at 11:25 AM Peter Waller via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> On 7/22/19 6:34 PM, JF Bastien via llvm-dev wrote:
> > * How do we expect to maintain git blame history, if at all?
>
> Hmm. I was hoping git-blame would have a feature which might allow
> ignoring commits, but seemingly not. You can ignore whitespace changes
> with -w, but of course that doesn't help for variable names.
>
> It seems the next best thing is to blame starting at a revision. At
> least if there is "one big change", there is only one revision to
> consider. With many smaller changes that would be harder.
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> On Jul 23, 2019, at 8:30 AM, James Y Knight via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> As a very frequent explorer of history, I really don't think this is
> as big an issue as it may seem. Even absent refactorings, you often
> run into the "wrong" commit when looking at blame (e.g., someone
just
> added a comma rather than actually changing the code you care about),
> and have to look past that, to another previous commit.
> 
> Any interactive blame tool ought to have an easy way to do this. For
> example, in emacs's annotation mode (which is what I use), you just
> press 'a' with the cursor on the line in question to re-annotate at
> the commit previous to that.
You might very well be right, but your answer sounds like you *think* it’ll work
out. For a huge change like this I’d like to *know* for a fact that you’re
right. Changing variable naming will touch every single line except comments, so
it’s quite different from what we usually see today.

Artem mentions a new git feature that’ll do this automatically. Again: sounds
great, do we *know* that it’ll work?

I think the onus is on people who care about doing this renaming to make sure
issues such as this are actually non-issues. Maybe it’ll Just Work if you have
the new feature Artem mentions. It’s the first time I hear of it, so I bet most
people won’t know about it. That seems hostile to most LLVM developers. Maybe we
need something in git-llvm to auto-ignore that huge commit? Or can it be in the
repo’s git config?


Not to dive too deep into this git blame issue: it’s one of a few concerns I
raised, and I have a few more I haven’t raised… This RFC seems like a huge
change and really doesn’t seem ready.

> On Tue, Jul 23, 2019 at 11:25 AM Peter Waller via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> 
>> On 7/22/19 6:34 PM, JF Bastien via llvm-dev wrote:
>>> * How do we expect to maintain git blame history, if at all?
>> 
>> Hmm. I was hoping git-blame would have a feature which might allow
>> ignoring commits, but seemingly not. You can ignore whitespace changes
>> with -w, but of course that doesn't help for variable names.
>> 
>> It seems the next best thing is to blame starting at a revision. At
>> least if there is "one big change", there is only one
revision to
>> consider. With many smaller changes that would be harder.
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On Tue, 23 Jul 2019 at 16:31, James Y Knight via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> As a very frequent explorer of history, I really don't think this is
> as big an issue as it may seem. Even absent refactorings, you often
> run into the "wrong" commit when looking at blame (e.g., someone
just
> added a comma rather than actually changing the code you care about),
> and have to look past that, to another previous commit.
>
> Any interactive blame tool ought to have an easy way to do this. For
> example, in emacs's annotation mode (which is what I use), you just
> press 'a' with the cursor on the line in question to re-annotate at
> the commit previous to that.
This matches my experience too, and I'm also frequently exploring
blame history. Internal APIs get adjusted pretty frequently in LLVM so
I it's incredibly common for any of my blame digging to have to look
through intermediate cleanup commits. Given this experience, I'm a bit
puzzled on why blame history is brought up as a concern so frequently,
but possibly people are using blame in a different way?

Best,

Alex

I tend to use git blame via the Github UI. This provides a single-click
button to go to the commit before the commit in question, so it'll add an
extra click to go back in time (and corresponding page reload time), which
is less than ideal, but isn't the worst thing in the world. I think I've
probably only used git blame directly in LLVM on single-figure occasions,
and nearly always found it easier to then go to github for one reason or
another, so much so that I don't remember how long it's been since I
last
actually used the command line to do it. There are numerous cases I run
into where the code has slightly changed due to a new API parameter, a
variable rename, a re-ordering or refactoring of code or whatever, so one
extra commit I need to ignore is not a big deal.

James

On Wed, 24 Jul 2019 at 10:03, Alex Bradbury via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Tue, 23 Jul 2019 at 16:31, James Y Knight via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >
> > As a very frequent explorer of history, I really don't think this
is
> > as big an issue as it may seem. Even absent refactorings, you often
> > run into the "wrong" commit when looking at blame (e.g.,
someone just
> > added a comma rather than actually changing the code you care about),
> > and have to look past that, to another previous commit.
> >
> > Any interactive blame tool ought to have an easy way to do this. For
> > example, in emacs's annotation mode (which is what I use), you
just
> > press 'a' with the cursor on the line in question to
re-annotate at
> > the commit previous to that.
>
> This matches my experience too, and I'm also frequently exploring
> blame history. Internal APIs get adjusted pretty frequently in LLVM so
> I it's incredibly common for any of my blame digging to have to look
> through intermediate cleanup commits. Given this experience, I'm a bit
> puzzled on why blame history is brought up as a concern so frequently,
> but possibly people are using blame in a different way?
>
> Best,
>
> Alex
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190724/adb753ae/attachment.html>

On Jul 23, 2019, at 9:17 AM, JF Bastien via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> On Jul 23, 2019, at 8:30 AM, James Y Knight via llvm-dev <llvm-dev
at lists.llvm.org> wrote:
>> 
>> As a very frequent explorer of history, I really don't think this
is
>> as big an issue as it may seem. Even absent refactorings, you often
>> run into the "wrong" commit when looking at blame (e.g.,
someone just
>> added a comma rather than actually changing the code you care about),
>> and have to look past that, to another previous commit.
>> 
>> Any interactive blame tool ought to have an easy way to do this. For
>> example, in emacs's annotation mode (which is what I use), you just
>> press 'a' with the cursor on the line in question to
re-annotate at
>> the commit previous to that.
> 
> You might very well be right, but your answer sounds like you *think* it’ll
work out. For a huge change like this I’d like to *know* for a fact that you’re
right. Changing variable naming will touch every single line except comments, so
it’s quite different from what we usually see today.
> 
> Artem mentions a new git feature that’ll do this automatically. Again:
sounds great, do we *know* that it’ll work?
Hi JF,

As discussed on other posts, there is good reason to believe that the
forthcoming git feature will cover this case.  We aren’t the only project that
has been held up by similar concerns about sweeping changes adversely affecting
history spelunking.  Similarly, as James Knight points out, this isn’t really
that big of a problem in practice even without the feature.

Sub projects in LLVM have gone through similar phases, including LLDB which
switched from 4 space indentation to 2 (as well as 80 columns iirc), which
affected literally everything.  This was unpopular, but was a good thing to do,
because commonality and consistency across the project is important.  I’m not
aware of a show-stopper problem even this massive of a change caused.

I think that Rui rolled this out in an incredibly great way with LLD,
incorporating a lot of community feedback and discussion, and (as you say) this
thread has accumulated many posts and a lot of discussion, so I don’t see the
concern about lack of communication.

The reason that I personally pivoted to believe that a “do everything all at
once in one commit generated by Rui’s script” approach was the right thing was
seeing how lld contributors were able to cleanly move forward even when they
have significant out of tree changes - they merged to the N-1 commit, then ran
Rui's script on *their* trees, and were caught up with commit N.  This seems
to have provided a very smooth transition path, where the conversion was merely
a bump in the road, not a major mountain to climb over.

In any case, I hope that we as a community don’t end up in a place where we are
afraid of doing sweeping changes that make the codebase better.  It is important
to continually reinvest in the health of the codebase, and it isn’t bad to
incentivize people to merge their changes upstream.  Being overwhelmingly afraid
of ‘breaking history’ is a path that leads stagnation and ultimate replacement
by newer technologies.  There are many other projects that have suffered this
fate, usually for a combination of reasons including things like this.

-Chris