git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Tiran Meltser <Tiran.Meltser@mavenir.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Amir Yosef <Amir.Yosef@mavenir.com>
Subject: Re: Request for adding a simple mechanism to exclude files from Git merge operation
Date: Tue, 23 Jun 2020 14:46:22 -0700	[thread overview]
Message-ID: <CABPp-BFwNnD-zZvHjCAvvmzy1wTT3yy-smK5nCtQ937apaNmkQ@mail.gmail.com> (raw)
In-Reply-To: <87sgelpmb2.fsf@osv.gnss.ru>

Hi Sergey,

On Tue, Jun 23, 2020 at 1:19 PM Sergey Organov <sorganov@gmail.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > On Tue, Jun 23, 2020 at 5:47 AM Sergey Organov <sorganov@gmail.com> wrote:
>
> [...]
>
> >>
> >> I believe we basically need support to apply different merge strategies
> >> to different files.
> >>
>
> [...]
>
> >> > Normally merges are symmetric, so if you want non-symmetric behavior,
> >> > you have to define what it's supposed to be.
> >>
> >> Yes, I'm ready to define what it's supposed to be. The problem is that
> >> "git merge" won't let me, due to lack of support to apply different
> >> merge strategies to different files.
> >>
> >> As I see it, first step of improvements could be to support
> >>
> >>   git merge -- <files>
> >>
> >> where selected strategy applies only to <files>, and the rest of files
> >> are kept intact (effectively applying "ours" strategy to them), along
> >> with
> >>
> >>   git merge --exclude=<files>
> >>
> >> , to be able to exclude specific files (apply "ours" only to them)
> >> rather than include.
> >>
> >> [ As a side-note, please notice that after such changes, the "ours"
> >> strategy could be deprecated (not that I think it should), as either:
> >>
> >>    git merge <branch> --
> >>
> >> or
> >>
> >>    git merge --exclude=. <branch>
> >>
> >> would do the trick. ]
> >>
> >> The next step would then be to support
> >>
> >>   git merge --force -- <files>
> >>
> >> that would force to re-merge <files> with given strategy no matter what
> >> their current status in the index is.
> >>
> >> Even though such support would be enough for my specific use-case, it
> >> doesn't provide suitable way to configure the default behavior. As a
> >> more generic solution, a new syntax for "git merge" to specify what
> >> merge strategy to apply to what files could be designed, and then
> >> ability to put that syntax into a file for "git merge" to pick would
> >> solve the problem of quasi-static configuration problem. Alternatively,
> >> even more generic .gitignore way of doing things apparently could be
> >> re-used to some degree by adding support for .gitmerge files.
> >
> > I think you'd have an uphill battle to convince me that this isn't
> > net-negative value:
> >
> >   * You can just do "git merge --no-commit ...; git restore
> > [--source=<side>] -- <pathspec>" to do what you're talking about
> > above.  I don't see the need to add extra functionality to merge,
> > especially not functionality that duplicates restore's functionality.
>
> Yeah, thanks, nice to know! I didn't, as "restore" is rather recent
> addition:
>
> $ git --version
> git version 2.20.1
> $ git help restore
> No manual entry for gitrestore
>
> However, it probably won't help with any other merge strategy anyway,
> right? E.g., think "git merge -X ours".
>
> It's already admittedly better than nothing though!

Well, I mean if we're talking about adding new features to git, then
obviously we're only going to add them to new versions; in that
context, a version from a year and a half ago is ancient.  ;-)

Going on the tangent for a second: if you can, please consider
upgrading (and to something newer than the last release, 2.27.0) in
order to help the community with testing the latest features,
refactors, and performance improvements.

> >   * The "ours" vs. "theirs" wording means you're going to have
> > intrinsic problems with rebases.  Several users will like your choice
> > of what "ours" means, the other half will complain that you've got it
> > all wrong.  I think you need to let the users decide on a case-by-case
> > basis, and we have a handy "git restore" command for letting them do
> > that already.
>
> I don't see how rebases are affected. I only suggested enhancements to
> the merge-the-procedure, the "git merge" user command. Once merge is
> finished and result is committed, there is (fortunately) now way for git
> to know how exactly the resulting content has been achieved.

Sorry, the original email from Tiran wanted to be able to record
"branch-specific" files and have merge automatically handle them
differently.  You also alluded to that when you said

"""
As a
more generic solution, a new syntax for "git merge" to specify what
merge strategy to apply to what files could be designed, and then
ability to put that syntax into a file for "git merge" to pick would
solve the problem of quasi-static configuration problem. Alternatively,
even more generic .gitignore way of doing things apparently could be
re-used to some degree by adding support for .gitmerge files.
"""

Once you record the information for which files it applies to, then
you want it to happen whenever the merge machinery fires, right?
Rebasing, cherry-picking, and reverting are all created via the merge
machinery (even if they end up recording one parent instead of more).
Said another way, if merge automatically handles these special files,
either rebase/cherry-pick/revert also handle the special files
automatically or you've just created a very inconsistent and weird
design.

If you're disclaiming your last paragraph and saying that this would
only be a manual operation where the user specifies which files they
want to specially merge, then a lot of my complaints go away.
Although...

> Nor do I see why to limit decisions to "ours" vs "theirs". I meant to
> support arbitrary merge strategies for different files. Generic feature.
>
> My thought was: if git at all supports different merge strategies, why
> not to support different strategies for different files? I don't see any
> inherent contradiction in adding of such a feature.

If you're interested in re-merging specific files, why not just call
`git merge-file` to handle each one?  It supports e.g. --ours/--theirs
(similar to merge's -Xours/-Xtheirs) and could possibly add more if
there are ones missing.  So, it seems like we already have a command
for this, even if it's less well known?

> >   * The pathspec limiting is going to be a bug factory for renaming
> > handling.  (The simplest form of which is just renaming a special path
> > to a non-special path or vice-versa and modifying both sides of
> > history.)  Rename handling can already get some pretty hairy corner
> > cases without dumping more in the mix.  I'd rather have users decide
> > what to do with paths that switched from being one of the special
> > "ours" paths to being a normal 3-way-conflict marker path.
>
> I admittedly didn't dive into such details, but I didn't suggest to
> attach any additional attributes to paths either, so there is no need to
> care about renames, as far as I'm able to see.
>
> Apparently you talk about some other feature here that I didn't suggest.

Perhaps your comments on creating a ".gitmerge" file means something
different than I understood.  If it indeed does not record pathnames,
then the rename issue goes away (though then I don't understand what
its purpose is nor the rest of your comments in that paragraph where
you suggested it).  But if your .gitmerge comments did imply something
similar to .gitattributes which specified how certain paths were to be
handled, then renaming issues would certainly arise.

> >   * I've run into "branch-specific" files in the wild and even
> > supported repositories that used them for years.  In my opinion, they
> > are almost always nasty code smells that are artifacts from
> > CVS/SVN-like thinking.  Although I wanted to stamp them out
> > immediately, there was opposition to it.  However, over time, people
> > removed those branch-specific files from the repository (and it wasn't
> > just by me or at my prodding either; many were cleaned away by others
> > without my involvement as other folks just found better ways to handle
> > things over time).  Giving special support to bad practices will just
> > enshrine them, which I'd rather avoid.
>
> I didn't suggest any specific support for "branch-specific" files, nor
> to any bad practices, as far as I'm aware.
>
> OTOH, the generic feature I suggest, as any generic feature, could
> indeed be (ab)used in multiple ways, but it's not a problem of the
> feature itself.
>
> > If someone wants to spend their time here, I can't stop them. Just be
> > aware that personally, I think it'd be a bad idea to make any
> > merge-recursive or merge-ort changes to support this kind of thing.
>
> Hopefully your attitude is caused by some misunderstanding of the aim of
> my suggestions.

Well, given that the last paragraph of your first email sounds (to me)
to be contradictory to your statements in this email, it seems quite
likely I am misunderstanding something you have said.

> > (Alternatively, if you're still convinced this is a good idea, you can
> > consider this email a heads up about potential problem areas that you
> > need to address and areas where you'll need to craft some good
> > arguments to win over those who are skeptical.)
>
> I still don't see any potential problems. Could you please give an
> explanatory example?
>
> Let me try to show my point by example. Suppose I've got a merge commit
> where part of files were merged with recursive strategy, part of files
> -- with the same recursive strategy but with -X ours, and the rest --
> with the same recursive strategy and -X theirs. What problems,
> exactly, do you expect?
>
> In fact I even fail to see how you will be able to tell it has been
> achieved with suggested feature rather than by manual resolution of all
> the conflicts, so there must be no additional problems here.
>
> What do I miss?

The problems I was raising were not with the resulting end-state tree
that users can construct or what happens with those trees once
constructed.  My problems were with expected automatic behavior from
the merge machinery coupled with incomplete specifications that
sounded to me like a pile of corner cases and bugs that I'd have to
field while trying to maintain the merge machinery logic.

Oh, and I have a problem with "branch specific" files from the email
you were responding to.  I think those are a code smell.  But my
primary concern was the expectations of some new automatic behavior
out of the merge machinery and how/if it gets configured.

  reply	other threads:[~2020-06-23 21:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-20 18:21 Request for adding a simple mechanism to exclude files from Git merge operation Tiran Meltser
2020-06-21 15:43 ` Philip Oakley
2020-06-22 18:42   ` [E] " Tiran Meltser
2020-06-22 19:41 ` brian m. carlson
2020-06-23 12:44   ` Sergey Organov
2020-06-23 16:16     ` Philip Oakley
2020-06-23 17:23       ` Sergey Organov
2020-06-23 17:08     ` Elijah Newren
2020-06-23 20:19       ` Sergey Organov
2020-06-23 21:46         ` Elijah Newren [this message]
2020-06-23 22:57           ` Chris Torek
2020-06-24 19:15           ` Sergey Organov
2020-06-23 22:38       ` Junio C Hamano
2020-06-24 18:03         ` Sergey Organov
2020-06-24 22:38           ` Junio C Hamano
2020-06-25 11:46             ` Sergey Organov

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

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

  git send-email \
    --in-reply-to=CABPp-BFwNnD-zZvHjCAvvmzy1wTT3yy-smK5nCtQ937apaNmkQ@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=Amir.Yosef@mavenir.com \
    --cc=Tiran.Meltser@mavenir.com \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.net \
    --cc=sorganov@gmail.com \
    /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.
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 NNTP newsgroup(s).