lkml.org 
[lkml]   [2023]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [GIT PULL] hardening updates for v6.3-rc1
    On Tue, Feb 21, 2023 at 11:49 AM Kees Cook <kees@kernel.org> wrote:
    >
    > >I've said this before, and apparently I need to say this again: if you
    > >cannot be bothered to explain *WHY* a merge exists, then that merge is
    > >buggy garbage by definition.
    >
    > Okay, understood. This was a merge of the fixes for v6.2. I'll explain that more clearly in the log from now on. :)

    So I really want people to document their merges, not just so that I
    (and others) can see "oh, that's why it exists at all", but also
    because I want to make people think about their merges more in
    general.

    For example, one reason why people do these kinds of merges is because
    they are starting to do some new development for the next release, and
    that new development then depends on fixes or infrastructure that they
    had in another branch (like a "for-linus" branch in case of fixes).

    So then they - mindlessly - just do a "git merge that-branch" and the
    end result looks very much like what you sent me.

    In a slightly better world, they then actually write an explanatory
    commit message for that merge, knowing that I ask for them, and the
    merge commit message ends up being exactly that kind of slightly odd

    "Now I'm starting a new thing that depends on the fixes
    I already sent upstream, so I'm merging that branch"

    Which while certainly better than no explanation at all sounds a bit
    odd, doesn't it? Yeah, add a few details on just what you depend on
    and why, and it gets much better, but it's all going to be a bit
    hand-wavy about future work that you haven't even written yet.

    And *that* will them maybe make you then go "Ahh, I'm doing things wrong".

    Because the "nice git way" to do that kind of thing is to actually
    realize "oh, I'm starting new work that depends on the fixes I already
    sent upstram, so I should just make a new topic branch and start at
    that point that I needed".

    And then - once you've done all the "new work" that depended on that
    state, only at *THAT* point do you merge the topic branch.

    And look - you have exactly the same commits: you have one (or more)
    normal commits that implement the new feature, and you have one merge
    commit, but notice how much easier it is to write the explanation for
    the merge when you do it *after* the work.

    Instead of having to waffle about "future work depends on this feature
    that was in another branch, so I'm merging this branch", your merge
    commit now makes *sense*. You're not merging some old state in order
    to create new features, you are literally just merging the completed
    new feature.

    So *this* is one reason I want people to really think about, and
    explain, their merges. Because it may be that having to explain it
    makes you go "Oh, I'm doing this wrong".

    Now, in your case, I don't actually think you needed that merge for
    any "future new work" at all. I think you just randomly did a merge to
    just get the same warning fixes that you had already sent me. So in
    this case, it smells like the merge was just entirely superfluous.

    Those kinds of superfluous merges can be ok - it's just annoying to
    have a development branch that still shows some artifact that you
    already fixed elsewhere.

    But they still need the explanation. And for that case, I want the
    explanation partly to make it clear that you really *thought* about
    it, and partly just so that I can see why you did it.

    Because we have a very real history where people did mindless daily
    back-merges like this "just because" with absolutely no rhyme or
    reason, just because they wanted to start each day with the most
    recent base, and it really gets very ugly. The development history can
    go from a DAG that actually visualizes the different development
    streams nicely to a spider-net maze of inexplicable merges very
    quickly.

    Linus

    \
     
     \ /
      Last update: 2023-03-27 00:30    [W:3.493 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site