git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: skottkuk@wp.pl
Cc: git@vger.kernel.org, Atharva Raykar <raykar.ath@gmail.com>
Subject: Re: Logical bug during MERGE or REBASE
Date: Sat, 03 Jul 2021 13:03:50 +0200	[thread overview]
Message-ID: <871r8fab7e.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <1932019063.20210702192555@wp.pl>


On Fri, Jul 02 2021, skottkuk@wp.pl wrote:

> Hello.
>
> I got a strange result in the process of "merge" and/or "rebase".

Atharva already replied to most of this, just adding on this point:

> [...]
> But as for me, it would be logical to consider the construction inside
> {} as something whole, and not just put all the changes into one heap
> with notification what all OK, no conflicts.

Git in general is not aware that your programming language considers {}
to be special, we don't try to do language detection, or to semantically
parse the program.

It's a general merge driver on text lines that works the same whether
you have a language like C# that uses {} braces, or a language like
Emacs Lisp which does not.

There's particular common cases where this logic goes "wrong", I've run
into it the most with repetitive declarations like:

    {
        {
            description => "some thingy",
            callback    => function { foo },
            strict      => 1,
            warn        => 1,
        },
        [... lots of these omitted ... ]
        {
            description => "other thingy",
            callback    => function { bar },
            strict      => 1,
            warn        => 1,
        },
    },

I didn't bother to check this specific example, but in cases *like that*
the merge driver will often append "duplicates" when two branches added
the same "other thingy", since the boilerplate at the end (or beginning,
depending) is repetitive, so a duplication becomes indistinguishable
from an addition for a naïve merge driver).

You can define your own merge driver that's aware of your language, I
think this is probably a too complex and Bad Idea in general.

Custom merge drivers are very useful for e.g. the git-annex case, which
ends up merging really simple "log" files. merges there are always
equivalent to basically a "sort -u". I.e. keep all lines added, remove
duplicates.

But for a programming language a "smart merge" is, I'd like to submit,
simply an impossible task. Even if you had perfect AI you couldn't do
it, even if I had a clone of myself from yesterday we probably couldn't
agree on how to solve all merges.

That's because once you get past the simple cases a merge resolution is
something that requires judgement calls from the programmer. E.g. I
worked on a topic branch, and now I've got a conflict because someone
changed the function signature. I can either do the bare minimum and use
some compatibility interface today, or convert all my work to the "new
API" and not have to convert from the legacy API in the future.

Either one would be a valid resolution, which the perfect AI, or even my
clone from yesterday might do differently.

But most importantly having a textual conflict in a program when you
merge/rebase is almost always the trivial case, having a semantic
conflict is something you always need to check for.

Git (or merge tools in general) can't help you with that, because your
"conflict" is in a conflict between the expectations of your topic
branch, and whether or not they hold given whatever's happened on an
advancing upstream.

So whether you have textual conflicts on merge/rebase from git or not,
your workflow really should be to always assume that you have a semantic
conflict, unless you're already completely familiar with the new code
you're merging into your branch.

I.e. after a merge/rebase look at your patches again to see if they make
sense given what changed on the upstream, compile, run the tests you
have etc.




      parent reply	other threads:[~2021-07-03 11:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02 16:25 Logical bug during MERGE or REBASE skottkuk
2021-07-03  7:57 ` Atharva Raykar
2021-07-04  6:12   ` Bagas Sanjaya
2021-07-04  6:30     ` Atharva Raykar
2021-07-03  9:07 ` martin
2021-07-03 11:03 ` Ævar Arnfjörð Bjarmason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871r8fab7e.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=raykar.ath@gmail.com \
    --cc=skottkuk@wp.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).