Re: Do not raise conflict when a code in a patch was already added

2018-08-21 Thread Igor Djordjevic
Hi Konstantin,

On 21/08/2018 11:37, Konstantin Kharlamov wrote:
> 
> > There's another possibility (and I think it is what happens 
> > actually in Konstantin's case): When one side added lines 1 2 and the 
> > other side added 1 2 3, then the actual conflict is 
> > << 1 2 == 1 2 3 >>, but our merge code is able to move the identical 
> > part out of the conflicted section: 1 2 << == 3 >>. But this is just 
> > a courtesy for the user; the real conflict is the original one. 
> > Without this optimization, the work to resolve the conflict would be 
> > slightly more arduous.
> 
> Yeah, thanks, that's what happens. And I'm wondering, is it really 
> needed to raise a conflict there? Would it be worth to just apply the 
> line "3", possibly with a warning or an interactive question to user 
> (apply/raise) that identical parts were ignored?

I see how this might make sense in the given example of "A added 1 
and 2, B added 1 and 2 and 3", but I'm afraid that might be a too 
narrow view.

What we actually don't know is if A deliberately chose not to include 
3, or even worse, if A started from having "1 and 2 and 3" in there, 
and then decided to remove 3.

In both these situation just applying 3 would be wrong, and raising a 
conflict seems as the most (and only?) sensible solution.

Applying _and_ asking for confirmation might be interesting, but I'm 
afraid it would favor specific use case only, being an annoyance in 
all the others (where it should really be a conflict, and you now 
have additional prompt to deal with).

That said, it would indeed be nice to have a way to communicate to 
`git rebase` that we are just splitting later commit into smaller 
parts preceding it, so situations like this could be resolved 
automatically and without conflicts, as you'd expected - but only 
within that narrow, user-provided/communicated context, not in 
general case.

Regards, Buga


Re: [PATCH v2 02/18] Add a new builtin: branch-diff

2018-05-07 Thread Igor Djordjevic
Hi Stefan,

On 08/05/2018 00:24, Stefan Beller wrote:
> 
> > List, rename, delete -- all these seem more as basic CRUD operations,
> > where comparison is a more complex one. And not to get me wrong - I
> > could see "branch diff" being part of "branch", but not really when
> > "diff" already exists as a separate thing, already doing quite some
> > (but still diff related, and configurable) stuff.
> 
> If we go with "branch --diff", because it has the CRUD operations already
> there for branches, I might ask for "remote --diff" to diff two remotes. ;)
> (That command "remote --diff" would not make any sense, would it?)

I`m not sure if this is a reply to me or in general, and whether you 
support what I sad, or argue against it...? Because what you`re 
saying was (or at least should have been) my exact point there :)

> > Basically, what you (conceptually) call "two versions of the same
> > branch", I simply call "two branches" (from usage standpoint).
> 
> If I diff 2 (topic) branches, which are based on a different version
> from upstream, then I see changes from commits that I don't care
> about, but this tool explicitly excludes them. Instead it includes
> the ordering of the commits as well as its commit messages to
> the diff.

Here, I was merely pointing out that you still need to provide two 
branch heads - which might be expected to resemble "two versions of 
the same topic", but they are still (just) "two branches" in Git world.

> > And you may have a branch that got split, or more of them that got
> > unified, so defining "previous branch version" may not be that
> > straightforward - it`s really just "two commit ranges" (as man page
> > defines it in general), with "two versions of a patch series" only
> > being the most common/expected use case of the former.
> >
> > Finally, if user picks two totally unrelated "branches" to compare,
> > he won`t get a really useful diff - but it`s the same as if he would
> > compare two totally unrelated commits (where tree state massively
> > changed in between, or having unrelated histories, even).
> 
> I used just that, but narrowed down the comparison to one file
> instead of the whole tree.

Again, not sure if this should support the argument, or argue against 
it? :) My point was that there might be other use cases (as you seem 
to have supported now), and as "diff" is pretty forgiving, might be 
"diff branch" should be as well.

> > With something like `git diff --branch ...` you
> > would get yet another "diff look", useful for use case in question
> > here.
> 
> Personally I think this patch series should neither extend git-diff
> nor git-branch.
> 
> It should not extend git-diff, because currently git-diff can diff
> tree-ishs (and does that very well) and comparing to
> worktree/index.

Hmm, are you saying that `git diff` actually has a too generic name 
for its (more specific) purpose?

> It should also not extend git-branch, as that command is for
> CRUD operations that you hinted at earlier (Earlier I proposed
> git-remote --diff for diffing two remote, which makes no sense,
> another one might be git-worktree, which also just does CRUD
> for worktrees. It would be a bad idea to have "git worktree --diff")

Agreed here.

> Hence I propose "git range-diff", similar to topic-diff, that
> was proposed earlier.

I find it strange that we already have both "diff" and "diff-something" 
commands, and yet you still propose "something-diff" naming pattern 
instead (but I guess it`s mainly because of auto-complete concerns).

Please forgive my lack of code base familiarity, but from what I`ve 
seen so far, and at least from end-user perspective, I may rather expect 
`git diff-range` as low level implementation, and possibly exposed 
through `git diff --range` (with a nice single letter abbreviation?).

> * it "diffs ranges" of commits.

Thus "diff-range", as your description says itself :) ("range-diff" 
might sound like it "ranges diffs"...?)

> * it can also deal with out-of-git things like patch series,
>   but that is a mere by product and may not be desired.
>   Just like git-diff can also compare two files outside a git
>   repo, that would not be a good use case.

Hmm, so still follows `git diff` in general... `git diff --range`? :D

> * it autocompletes well.

Only here I`m not sure if something like `git diff --range` (with 
accompanying single letter option) would be considered "auto-complete 
friendly", or not?

Regards, Buga


Re: [PATCH v2 02/18] Add a new builtin: branch-diff

2018-05-07 Thread Igor Djordjevic
Hi Dscho,

On 07/05/2018 03:34, Johannes Schindelin wrote:
> 
> > > I think Todd's idea to shift it from a full-blown builtin to a cmdmode
> > > of `branch` makes tons of sense.
> >
> > I don`t know, I still find it a bit strange that in order to "diff
> > something", you go to "something" and tell it to "diff itself" - not
> > because it`s a weird concept (OOP, anyone? :]), but because we already
> > have "diff" command that can accept different things, thus just teaching
> > it to accept additional "something" (branch, in this case), seems more
> > natural (to me) - "branch diff" being just another "diff" mode of
> > operation.
> 
> You also have to call `git branch` to list branches. And to rename
> branches. And to delete them. So why not also compare them at the same
> time?

Maybe because we already have a command that specifically does 
comparison? :)

List, rename, delete -- all these seem more as basic CRUD operations, 
where comparison is a more complex one. And not to get me wrong - I 
could see "branch diff" being part of "branch", but not really when 
"diff" already exists as a separate thing, already doing quite some 
(but still diff related, and configurable) stuff.

> > What about that side thought you left out from my original message,
> > making it `git diff --branch` instead?
> 
> I really did not like this, as all of the `git diff` options really are
> about comparing two revisions, not two *sets* of revisions.

I see what you mean, but I would argue this being a deliberate user 
choice here, like picking a diff "strategy" - I`d say it still utterly 
does compare two revisions (branch tips, in this case), just putting 
focus on comparing revisions that lead to them (branch history), 
instead of just files found in them (branch files).

> Further, if I put my unsuspecting user hat on, I would ask myself how you
> can compare branches with one another? That is what I would expect `git
> diff --branch` to do, not to compare two versions of *the same* branch.

I totally agree with you here, and thus I have a question - what 
determines "two versions of *the same* branch"? :) Do you still 
explicitly provide both "old" and "new" version branch tips?

I see "multiple versions of the same branch" more as a conceptual 
model, and not something Git is aware of (I think?) - BUT, even if it 
was, I don`t see why this should be a(n artificial) restriction?

Basically, what you (conceptually) call "two versions of the same 
branch", I simply call "two branches" (from usage standpoint).

And you may have a branch that got split, or more of them that got 
unified, so defining "previous branch version" may not be that 
straightforward - it`s really just "two commit ranges" (as man page 
defines it in general), with "two versions of a patch series" only 
being the most common/expected use case of the former.

Finally, if user picks two totally unrelated "branches" to compare, 
he won`t get a really useful diff - but it`s the same as if he would 
compare two totally unrelated commits (where tree state massively 
changed in between, or having unrelated histories, even).

Besides, while I might still not be much into the matter, but isn`t 
"branch" in Git just a pointer to revision? Being so, there is really 
no such thing as "branch" in terms of being a specific (sub)set of 
revisions (commits), other then "everything from branch head/pointer 
to root commit" (in general).

Yes, we do perceive "a branch" being a specific set of topic related 
commits, but which *exact* commits we are interested in ("branch" lower 
bounds) may differ in regards to what we aim for - how far do we consider 
one branch to reach in the past depends solely on the use case.

> So `git diff --branch` does not at all convey the same to me as `git
> branch --diff`, and I find that the latter does match better what this
> patch series tries to achieve.

I agree with the first part, but it seems to me your finding is 
biased due to your (expected) use case.

> > But if "branch diff" is considered to be too special-cased mode of
> > "diff" so that supporting it from `diff` itself would make it feel
> > awkward in both usage and maintenance (in terms of many other regular
> > `diff` specific options being unsupported), I guess I would understand
> > having it outside `diff` altogether (and implemented as proposed `git
> > branch --diff`, or something)... for the time being, at least :)
> 
> The branch diff is not even a special-cased mode of diff. It is *way* more
> complicated than that. It tries to find 1:1 correspondences between *sets*
> of commits, and then only outputs a "sort" of a diff between the commits
> that correspond with each other. I say "sort" of a diff because that diff
> does not look like `git diff  ` at all!

But there is not only one `git diff  ` looks, it 
depends on other options (like --name-status, for example), which is 
my point exactly :)

With something like `git diff --branch ...` you 
would get yet 

Re: [PATCH v2 02/18] Add a new builtin: branch-diff

2018-05-07 Thread Igor Djordjevic
On 07/05/2018 09:48, Jeff King wrote:
> 
> > > Let's, please, not fall into the trap of polluting git-branch with
> > > utterly unrelated functionality, as has happened a few times with
> > > other Git commands. Let's especially not do so merely for the sake of
> > > tab-completion. git-branch is for branch management; it's not for
> > > diff'ing.
> >
> > I totally disagree. `git branch` is *the* command to work with branches.
> > Yes, you can manage branches. But you can also list them. And now you can
> > also compare them.
> 
> One of the things I don't like about "git branch --diff" is that this
> feature is not _just_ about branches at all. E.g., I could do:
> 
>   git tbdiff HEAD~10 HEAD~5 foo
> 
> Or even:
> 
>   git tbdiff v2.16.0 v2.17.0 my-rewritten-v2.17.0
> 
> Those arguments really are just commitishes, not necessarily branches.
> One of the current interface rules for "git branch" is that the branch
> names we hand it are interpreted _exactly_ as branch names. You cannot
> "git branch -m v2.16.0", and there is no ambiguity in "git branch -d
> foo" if "foo" is both a tag and a branch.
> 
> But this new mode does not fit the pattern at all.
> 
> If we were to attach this to an existing command, I think it has more to
> do with "diff" than "branch". But I'm not sure we want to overload
> "diff" either (which has traditionally been about two endpoints, and
> does not really traverse at all, though arguably "foo...bar" is a bit of
> a cheat :) ).
> 
> > > Of the suggestions thus far, Junio's git-topic-diff seems the least
> > > worse, and doesn't suffer from tab-completion problems.
> >
> > Except that this is too limited a view.
> 
> Right, I agree with you. Topic branches are the intended use, but that's
> not what it _does_, and obviously it can be applied in other cases. So
> since "branch" is too specific, I think "topic branch" is even more so.
> 
> It's really "diff-history" or something, I think. That's not very
> catchy, but I think the best name would imply that it was diffing a set
> of commits (so even "diff-commit" would not be right, because that again
> sounds like endpoints).

This is exactly what I feel as well, thanks for concise and 
to-the-point spelling out.

>From user interface perspective, I would expect something like this 
to be possible (and natural):

(1) git diff topic-v1...topic-v2
(2) git diff --branch topic-v1...topic-v2

(1) is what we are all familiar with, providing a diff between two 
revisions with focus on file changes, where (2) shifts focus to 
history changes.

It`s all still a comparison between two revisions (pointed to by 
"topic-v1" and "topic-v2" branch heads in this specific example), but 
it differs in what we are comparing - (1) set of files contained in 
endpoints, or (2) set of revisions contained in (or "leading to") 
endpoints.

Hmm... what about `git diff --history`? :/ It does seem more "true" 
to what it does, though I still like `git diff --branch` more 
(catchier, indeed).

Regards, Buga


Re: [PATCH v2 02/18] Add a new builtin: branch-diff

2018-05-06 Thread Igor Djordjevic
Hi Dscho,

On 06/05/2018 14:10, Johannes Schindelin wrote:
> 
> > > > > This builtin does not do a whole lot so far, apart from showing a
> > > > > usage that is oddly similar to that of `git tbdiff`. And for a
> > > > > good reason: the next commits will turn `branch-diff` into a
> > > > > full-blown replacement for `tbdiff`.
> > > >
> > > > One minor point about the name: will it become annoying as a tab
> > > > completion conflict with git-branch?
> > >
> > > I did mention this in the commit message of 18/18:
> > >
> > > Without this patch, we would only complete the `branch-diff` part but
> > > not the options and other arguments.
> > >
> > > This of itself may already be slightly disruptive for well-trained
> > > fingers that assume that `git braorimas` would expand 
> > > to
> > > `git branch origin/master`, as we now no longer automatically append a
> > > space after completing `git branch`: this is now ambiguous.
> > >
> > > > It feels really petty complaining about the name, but I just want
> > > > to raise the point, since it will never be easier to change than
> > > > right now.
> > >
> > > I do hear you. Especially since I hate `git cherry` every single
> > > time that I try to tab-complete `git cherry-pick`.
> > >
> > > > (And no, I don't really have another name in mind; I'm just
> > > > wondering if "subset" names like this might be a mild annoyance in
> > > > the long run).
> > >
> > > They totally are, and if you can come up with a better name, I am
> > > really interested in changing it before this hits `next`, even.
> >
> > I gave this just a quick glance so might be I`m missing something 
> > obvious or otherwise well-known here, bur why not `diff-branch` instead?
> 
> I think that is just turning the problem from `branch` to `diff`.
> 
> Of course, we have precedent with diff-index and diff-files. Except that
> they don't auto-complete (because they are low-level commands) and I
> *would* like the subcommand discussed in this here patch series to
> auto-complete.

Yeah, I did suspect it might be something like this (those other ones 
not auto-completing, where we do want it here), thanks for elaborating.

> I think Todd's idea to shift it from a full-blown builtin to a cmdmode
> of `branch` makes tons of sense.

I don`t know, I still find it a bit strange that in order to "diff 
something", you go to "something" and tell it to "diff itself" - not 
because it`s a weird concept (OOP, anyone? :]), but because we 
already have "diff" command that can accept different things, thus 
just teaching it to accept additional "something" (branch, in this 
case), seems more natural (to me) - "branch diff" being just another 
"diff" mode of operation.

What about that side thought you left out from my original message, 
making it `git diff --branch` instead?

But if "branch diff" is considered to be too special-cased mode of 
"diff" so that supporting it from `diff` itself would make it feel 
awkward in both usage and maintenance (in terms of many other regular 
`diff` specific options being unsupported), I guess I would understand 
having it outside `diff` altogether (and implemented as proposed `git 
branch --diff`, or something)... for the time being, at least :)

Regards, Buga


Re: [PATCH v2 05/18] branch-diff: also show the diff between patches

2018-05-05 Thread Igor Djordjevic
Hi Johannes,

On 04/05/2018 17:34, Johannes Schindelin wrote:
> Just like tbdiff, we now show the diff between matching patches. This is
> a "diff of two diffs", so it can be a bit daunting to read for the
> beginner.
> 
> And just like tbdiff, we now also accept the `--no-patches` option
> (which is actually equivalent to the diff option `-s`).

A quick nit - would `--no-patch` (singular form) option name be more 
aligned with diff `-s` option it resembles?

Thanks, Buga


Re: [PATCH v2 02/18] Add a new builtin: branch-diff

2018-05-05 Thread Igor Djordjevic
Hi Dscho,

On 05/05/2018 23:57, Johannes Schindelin wrote:
> 
> > > This builtin does not do a whole lot so far, apart from showing a
> > > usage that is oddly similar to that of `git tbdiff`. And for a
> > > good reason: the next commits will turn `branch-diff` into a
> > > full-blown replacement for `tbdiff`.
> >
> > One minor point about the name: will it become annoying as a tab
> > completion conflict with git-branch?
> 
> I did mention this in the commit message of 18/18:
> 
> Without this patch, we would only complete the `branch-diff` part but
> not the options and other arguments.
> 
> This of itself may already be slightly disruptive for well-trained
> fingers that assume that `git braorimas` would expand to
> `git branch origin/master`, as we now no longer automatically append a
> space after completing `git branch`: this is now ambiguous.
> 
> > It feels really petty complaining about the name, but I just want
> > to raise the point, since it will never be easier to change than
> > right now.
> 
> I do hear you. Especially since I hate `git cherry` every single
> time that I try to tab-complete `git cherry-pick`.
> 
> > (And no, I don't really have another name in mind; I'm just
> > wondering if "subset" names like this might be a mild annoyance in
> > the long run).
> 
> They totally are, and if you can come up with a better name, I am
> really interested in changing it before this hits `next`, even.

I gave this just a quick glance so might be I`m missing something 
obvious or otherwise well-known here, bur why not `diff-branch` instead?

>From user interface perspective, I would (personally) rather expect a 
command that does "diff of branches" to belong to "diff family" of 
commands (just operating on branches, instead of "branch" command 
knowing to "diff itself"), and I see we already have `diff-files`, 
`diff-index` and `diff-tree`, for what that`s worth.

Heck, I might even expect something like `git diff --branch ...` to work, 
but I guess that is yet a different matter :)

Thanks, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-19 Thread Igor Djordjevic
Hi Sergey,

On 19/03/2018 06:44, Sergey Organov wrote:
> 
> > > > > > Second side note: if we can fast-forward, currently we prefer
> > > > > > that, and I think we should keep that behavior with -R, too.
> > > > >
> > > > > I agree.
> > > >
> > > > I'm admittedly somewhat lost in the discussion, but are you
> > > > talking fast-forward on _rebasing_ existing merge? Where would it
> > > > go in any of the suggested algorithms of rebasing and why?
> > > >
> > > > I readily see how it can break merges. E.g., any "git merge
> > > > --ff-only --no-ff" merge will magically disappear. So, even if
> > > > somehow supported, fast-forward should not be performed by default
> > > > during _rebasing_ of a merge.
> > >
> > > Hmm, now that you brought this up, I can only agree, of course.
> > >
> > > What I had in my mind was more similar to "no-rebase-cousins", like 
> > > if we can get away without actually rebasing the merge but still 
> > > using the original one, do it. But I guess that`s not what Johannes 
> > > originally asked about.
> > >
> > > This is another definitive difference between rebasing (`pick`?) and 
> > > recreating (`merge`) a merge commit - in the case where we`re rebasing, 
> > > of course it doesn`t make sense to drop commit this time (due to 
> > > fast-forward). This does make sense in recreating the merge (only).
> >
> > Eh, I might take this back. I think my original interpretation (and 
> > agreement) to fast-forwarding is correct.
> >
> > But the confusion here comes from `--no-ff` as used for merging, as 
> > opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant 
> > the latter one.
> >
> > In rebasing, `--no-ff` means that even if a commit inside todo list 
> > isn`t to be changed, do not reuse it but create a new one. Here`s 
> > excerpt from the docs[1]:
> >
> >   --no-ff
> > With --interactive, cherry-pick all rebased commits instead of 
> > fast-forwarding over the unchanged ones. This ensures that the 
> > entire history of the rebased branch is composed of new commits.
> >
> > Without --interactive, this is a synonym for --force-rebase.
> >
> >
> > So fast-forwarding in case of rebasing (merge commits as well) is 
> > something you would want by default, as it wouldn`t drop/lose 
> > anything, but merely reuse existing commit (if unchanged), instead of 
> > cherry-picking (rebasing) it into a new (merge) commit anyway.
> 
> This sounds like breakage. E.g., it seems to be breaking every "-x ours"
> merge out there.

Either you are not understanding how rebase fast-forward works, or 
I`m missing what you are pointing to... Mind explaining how can 
something that`s left unchanged suddenly become a breakage?

> Fast-forwarding existing merge, one way or another, still seems to be
> wrong idea to me, as merge commit is not only about content change, but
> also about joint point at particular place in the DAG.

Not sure what this has to do with rebase fast-forwarding, either - 
nothing changes for fast-forwarded (merge or non-merge) commit in 
question, both content, joint point and everything else stays exactly 
the same. If anything changed, then it can`t/won`t be fast-forwarded, 
being unchanged is a prerequisite.

Let me elaborate a bit. Here`s a starting diagram:

(1) ---X1---X2---X3 (master)
 |\
 | A1---A2---A3
 |\
 | M---C1---C2 (topic)
 |/
 \-B1---B2---B3


With "topic" being active branch, we start interactive rebase with 
`git rebase -i master`. Generated todo list will hold commits A1 to 
A3, B1 to B3, M and C1 to C2.

Now, if we decide to `edit` commit C1, leaving everything else the 
same, fast-forward logic will make the new situation look like this:

(2) ---X1---X2---X3 (master)
 |\
 | A1---A2---A3
 |\
 | M---C1'--C2' (topic)
 |/
 \-B1---B2---B3


Notice how only C1 and C2 changed to C1' and C2'? That`s rebase 
fast-forwarding, noticing earlier commits left unchanged, thus 
reusing original ones.

No matter what, no breakage can happen to M in this case, as it`s 
left (reused) exactly as it was - it`s fast-forward rebased.

If we `edit`-ed commit A2, we would have ended in a situation like this:

(3) ---X1---X2---X3 (master)
 |\
 | A1---A2'--A3'
 |\
 | M'--C1'--C2' (topic)
 |/
 \-B1---B2---B3


This time we have new commits A2', A3', M', C1' and C2' - so 
everything influenced by the change that happened will be changed 
(merge commit as well), where all the rest can still be reused 
(fast-forwarded).

If we had started rebasing with `git rebase -i --no-ff master`, no 
matter which commits we `edit` (or none, even), we would end up with 
this instead:

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-16 Thread Igor Djordjevic
Hi Sergey,

On 16/03/2018 08:31, Sergey Organov wrote:
> 
> > > As I said, putting myself on the user side, I'd prefer entirely
> > > separate first step of the algorithm, exactly as written, with
> > > its own conflict resolution, all running entirely the same way as
> > > it does with non-merge commits. I'm used to it and don't want to
> > > learn something new without necessity. I.e., I'd prefer to
> > > actually see it in two separate stages, like this:
> > >
> > > Rebasing mainline of the merge...
> > > [.. possible conflicts resolution ..]
> > > Merging in changes to side branch(es)...
> > > [.. possible conflicts resolution ..]
> > >
> > > And if the second stage gives non-trivial conflicts, I'd like to
> > > have a simple way to just do "merge -s ours " on top of
> > > already rebased mainline of the merge and go with it. Note that
> > > the latter is significantly different than re-merging everything
> > > from scratch, that would be the only choice with "all-in-one"
> > > approach, and it essentially gives me back those simple "rebase
> > > first parent and just record other parents" semantics when
> > > needed.
> > 
> > [...]
> > 
> > Also note that, for example, in case side branch(es) dropped some 
> > commits (interactively or otherwise), first step alone would still 
> > reintroduce those dropped changes, thus later possible `merge -s ours 
> > ` would be a pretty bad "evil merge" case and a wrong thing to 
> > do in general.
> 
> Except that my presumption is that the second step has been run already
> and has stopped due to conflicts, so I see the conflicting result of
> dropping those commits on side branch(es), check the previous state of
> the right side of the conflicting merge, and decide those state, being
> the result of the fist step after possibly demanding conflicts
> resolution, is fine after all. Thus I just re-merge -x ours the
> branch(es), instead of re-merging everythig from scratch only to finally
> get back to the same result, be it evil or not, the hard way.

Might be my comment missed the point here, it should have been more 
about what you said regarding "first step having its own conflict 
resolution" - in case of dropped commits on side branch(es), you would 
be trying to resolve conflicts using one tree that doesn`t/shouldn`t 
even exist anymore (rebased merge commit first parent changes), which 
might be pretty confusing, only to find the "second stage" later 
removing changes that you might have actually picked as "first stage" 
conflict resolution, making it all even worse.

Only once "huge merge" is done completely (meaning all steps involved 
in merge commit rebasing), user can have a more realistic overview of 
(possibly nested, even) conflicts to resolve (and knowing his resolution 
will actually stick).

Regarding `merge -s ours ` you mention, as you say it would 
happen only after "huge merge" is complete (with possible conflicts), 
I guess it`s unrelated to having "merge commit rebasing" happen in 
one go ("huge merge"), or iteratively, in stages (from user`s 
perspective, unrelated to underlying implementation)...?

Thus I`m questioning use-case for step-by-step merge commit rebasing 
where each stage has its own conflict resolution, in the face of it 
possibly being more confusing than helpful.

Otherwise, I see the point in what you would like to accomplish with 
that `merge -s ours ` (not from scratch), but I`m not sure 
what would be the most sane way to allow it, and if it would be worth 
it in the first place, seeming to be a pretty exotic use case.

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-16 Thread Igor Djordjevic
On 15/03/2018 00:11, Igor Djordjevic wrote:
> 
> > > > Second side note: if we can fast-forward, currently we prefer
> > > > that, and I think we should keep that behavior with -R, too.
> > >
> > > I agree.
> > 
> > I'm admittedly somewhat lost in the discussion, but are you
> > talking fast-forward on _rebasing_ existing merge? Where would it
> > go in any of the suggested algorithms of rebasing and why?
> > 
> > I readily see how it can break merges. E.g., any "git merge
> > --ff-only --no-ff" merge will magically disappear. So, even if
> > somehow supported, fast-forward should not be performed by default
> > during _rebasing_ of a merge.
> 
> Hmm, now that you brought this up, I can only agree, of course.
> 
> What I had in my mind was more similar to "no-rebase-cousins", like 
> if we can get away without actually rebasing the merge but still 
> using the original one, do it. But I guess that`s not what Johannes 
> originally asked about.
> 
> This is another definitive difference between rebasing (`pick`?) and 
> recreating (`merge`) a merge commit - in the case where we`re rebasing, 
> of course it doesn`t make sense to drop commit this time (due to 
> fast-forward). This does make sense in recreating the merge (only).

Eh, I might take this back. I think my original interpretation (and 
agreement) to fast-forwarding is correct.

But the confusion here comes from `--no-ff` as used for merging, as 
opposed to `--no-ff` as used for rebasing. I _think_ Johannes meant 
the latter one.

In rebasing, `--no-ff` means that even if a commit inside todo list 
isn`t to be changed, do not reuse it but create a new one. Here`s 
excerpt from the docs[1]:

  --no-ff
With --interactive, cherry-pick all rebased commits instead of 
fast-forwarding over the unchanged ones. This ensures that the 
entire history of the rebased branch is composed of new commits.

Without --interactive, this is a synonym for --force-rebase.


So fast-forwarding in case of rebasing (merge commits as well) is 
something you would want by default, as it wouldn`t drop/lose 
anything, but merely reuse existing commit (if unchanged), instead of 
cherry-picking (rebasing) it into a new (merge) commit anyway.

The same goes for this part:

> > > > If the user wants to force a new merge, they simply remove that
> > > > -R flag.

This means that using `-R` flag is sensitive to `--no-ff` rebase 
option, original merge commit _reused_ (fast-forwarded) when possible 
(unchanged, and `--no-ff` not provided), or original merge commit 
_rebased_ (when changed, or `--no-ff` provided).

If `-R` flag is removed, then merge commit is always _recreated_, no 
matter if `--no-ff` option is used or not.

p.s. I`m still a bit opposed to `-R` flag in the first place, as 
discussed elsewhere[2][3], but that`s unrelated to this fast-forward 
discussion.

Related to it, though, if `pick` command would be used instead of 
`merge -R` to signal merge commit _rebasing_, it would fit into 
existing logic nicely, where `pick` is already sensitive to `--no-ff` 
option (for rebasing regular commits). Then `merge` alone could be 
naturally (and only) used for _recreating_ merge commits, as 
originally intended (and intuitively expected).

Regards, Buga

[1] https://git-scm.com/docs/git-rebase#git-rebase---no-ff
[2] https://public-inbox.org/git/77b695d0-7564-80d7-d9e6-70a531e66...@gmail.com/
[3] https://public-inbox.org/git/a3d40dca-f508-5853-89bc-1f9ab3934...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-15 Thread Igor Djordjevic
Hi Sergey,

On 15/03/2018 08:52, Sergey Organov wrote:
> 
> > > 2. The U1' == U2' consistency check in RFC that I still think is worth
> > > to be implemented.
> >
> > At the moment, I think we`d appreciate test cases where it actually 
> > proves useful, as the general consensus seems to be leaning towards 
> > it possibly being annoying (over-paranoid).
> 
> As we now have a simple way to actually check it even in this algorithm,
> I'd suggest command-line option to either relax or enforce the check,
> whatever the default is. For the default, I'd still opt for safety, as
> without it we will gather little experience with this new matter.
> 
> Honestly, without this check available, I'd likely vote for at least an
> option for stopping on every rebased merge, on the ground that if
> rebasing a non-merge could be a trouble, rebasing a merge is at least
> double-trouble, and it's not that frequent anyway. So the check we
> discuss is actually a way to make all the process much less paranoid,
> not more.
> 
> By the way, nobody yet commented about "rerere" behavior that basically
> stops rebasing every time it fires. Do you consider it over-paranoid?

I wouldn`t really know, my workflows are usually/still rather simple, I 
don`t think I`ve ever used it on purpose, and I don`t really remember 
I`ve triggered it by accident, not having it stop for amendment, at least.

You did say you find it annoying yourself, though ;) But also accepting 
it as something that probably has a good reason, though (thus not 
considering it over-paranoid, even if annoying).

> As for test cases, I have none myself, but "-s ours" merge may be an
> example of an actual trouble.
> 
> If we don't treat it specially, then changes to side branch will be
> silently propagated over the merge, that's obviously not what is needed,
> provided user keeps his intention to leave the merge "-s ours".
> 
> If we do treat it specially, it could be the case that the merge in
> question only looks like "-s ours" by pure accident, and thus changes to
> the side branch should be propagated.
> 
> I don't see how we can safely proceed without stop for user assistance.
> Had we already achieved some consensus on this issue?

I don`t know, from what Johannes said in the past, I got an 
impression that this is to be expected ("by design"), and not worth 
bothering to stop for. And he is one of the heaviest users of (merge) 
rebasing I know.

Personally, I still feel it would make sense to stop in case like 
this, indeed, but it`s just my humble (and not necessarily much 
educated) opinion.

> > > Finally, here is a sketch of the implementation that I'd suggest to
> > > use:
> > >
> > > git-rebase-first-parent --onto A' M
> > > tree_U1'=$(git write-tree)
> > > git merge-recursive B -- $tree_U1' B'
> > > tree=$(git write-tree)
> > > M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB')
> > > [ $conflicted_last_merge = "yes" ] ||
> > >   trees-match $tree_U1' $tree || 
> > >   stop-for-user-amendment
> >
> > Yes, in case where we would want the "no-op merge" check (equivalent 
> > to U1' == U2' with original approach), this aligns with something I 
> > would expect.
> >
> > Note that all the "rebase merge commit" steps leading to the check 
> > will/should probably be observed as a single one from user`s perspective 
> > (in worst case ending with nested conflicts we discussed), thus 
> > `$conflicted_last_merge` is not related to `merge-recursive` step(s) 
> > only, but `rebase-first-parent`, too (just in case this isn`t implied).
> >
> > Might be easier to reason about simply as `[ $conflicts = "yes" ] || `
> 
> No. For this check it's essential to ensure that no tweaking of the
> content has been performed under the hood after the user has resolved
> conflicts, i.e., after he has been involved last time.
> 
> If all this is done in one "huge merge" step from user point of view,
> then the check belongs to this merge, as this is the last (and the only)
> one. If it's done in steps (and I vote for it), only the last merge
> status is essential for the check, preceding merges don't matter.

"Huge merge" step (from user point of view) is exactly how I perceived 
Johannes` opinion on it, describing it`s already part of Git user 
experience (with possible nested conflicts), while otherwise possibly 
hard to explain where we are precisely at in the moment of stopping for 
(intermediate) conflict resolution.

Thus only `$conflicts`, meaning anything in the "huge merge", as no user 
action/tweaking/involvement can happen until the "huge merge" is done.

> As I said, putting myself on the user side, I'd prefer entirely separate
> first step of the algorithm, exactly as written, with its own conflict
> resolution, all running entirely the same way as it does with non-merge
> commits. I'm used to it and don't want to learn something new without
> necessity. I.e., I'd prefer to actually see it in two separate stages,
> like this:
> 
> Rebasing mainline of the 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-15 Thread Igor Djordjevic
Hi Sergey,

On 15/03/2018 07:00, Sergey Organov wrote:
> 
> > > Thinking about it I've got an idea that what we actually need is
> > > --no-flatten flag that, when used alone, will just tell "git rebase" to
> > > stop flattening history, and which will be implicitly imposed by
> > > --recreate-merges (and --preserve-merges).
> > >
> > > Then the only thing the --recreate-merges will tune is to put 'merge'
> > > directives into the todo list for merge commits, exactly according to
> > > what its name suggests, while the default behavior will be to put 'pick'
> > > with suitable syntax into the todo. And arguments to the
> > > --recreate-merge will specify additional options for the 'merge'
> > > directive, obviously.
> >
> > This seem to basically boil down to what I mentioned previously[2] 
> > through use of new `--rebase-merges` alongside `--recreate-merges`, just 
> > that you named it `--no-flatten` here, but the point is the same - and 
> > not something Johannes liked, "polluting" rebase option space further.
> 
> Not quite so. The problem with --XXX-merges flags is that they do two
> things at once: they say _what_ to do and _how_ to do it. Clean UI
> designs usually have these things separate, and that's what I propose.
> 
> The --[no-]flatten says _what_ (not) to do, and --recreate-merges says
> _how_ exactly it will be performed. In this model --no-flatten could
> have been called, say --preserve-shape, but not --rebase-merges.
> 
> To minimize pollution, the _how_ part could rather be made option value:
> 
> --no-flatten[=]
> 
> where  is 'rebase', 'remerge', etc.
> 
> In this case we will need separate option to specify strategy options,
> if required, that will lead us to something similar to the set of merge
> strategies options.
> 
> > I would agree with him, and settling onto `--rebase-merges` _instead_ of 
> > `--recreate-merges` seems as a more appropriate name, indeed, now that 
> > default behavior is actually merge commit rebasing and not recreating 
> > (recreating still being possible through user editing the todo list).
> 
> I hope he'd be pleased to be able to say --no-flatten=remerge and get
> back his current mode of operation, that he obviously has a good use
> for.

Makes sense, I like it, thanks for elaborating. [ Especially that you 
used "(no) flatten" phrasing, where original `--preserve-merges` 
documentation says it`s used "not to flatten the history", nice touch ;) ]

Not sure if I would prefer original `recreate` instead of `remerge` 
as that particular strategy name, though, but might be I`m just more 
used to the former one at the moment.

But if I think about it more, "recreate" seems straightforward enough - 
create something (again), kind of making it more obvious that it is a 
new thing (that we are now creating, again, based on some old thing).

On the other hand, "remerge" communicates "merge something again", 
which doesn`t necessarily mean creating a new thing (based on, but 
not attached to old thing), and could also be interpreted as "merge 
existing thing again" (and leaves me wondering if it would better 
suite some other strategy possible in the future).

Not sure if that explanation suffices, but it comes to "recreate a 
merge" having more clear meaning than "remerge a merge", being 
somewhat ambiguous, thus confusing (to me, at least).

I don`t know, thinking too much?

Regards, Buga


Re: [bug] git stash push {dir-pathspec} wipes untracked files

2018-03-15 Thread Igor Djordjevic
On 15/03/2018 17:52, Junio C Hamano wrote:
> 
> > Hi, I ran into what I believe is a bug today.  I’m using primarily Git
> > for Windows 2.16.2 and also reproduced the behavior on Git for Windows
> > 2.15.1 and Git 2.14.1 on Ubuntu:
> >
> > Given any repository with at least one subdirectory:
> >
> > 1.   Create some untracked files in the subdir
> > 2.   Modify a tracked file in the subdir
> > 3.   Execute `git stash push subdir`
> > 4.   The untracked files will be removed, without warning.
> >
> > `git stash push` behaves as-expcted and does not touch untracked
> > files.  It’s only when a directory tree is specified as [pathspec]
> > that the problem occurs.
> 
> I wonder if this is the same as the topic on this thread.
> 
>   
> https://public-inbox.org/git/ca+hnv10i7avwxjrqjxxy1lnjtmhr7le4twxhhuybiwtmjco...@mail.gmail.com/
> 
> What is curious is that the fix bba067d2 ("stash: don't delete
> untracked files that match pathspec", 2018-01-06) appeared first in
> 2.16.2, on which Windows 2.16.2 is supposed to be built upon.
> 
> > Here's the precise reproduction case executed on a linux box:
> 
> This does not reproduce for me with v2.16.2-17-g38e79b1fda (the tip
> of 'maint'); I do not have an  install of vanilla v2.16.2 handy, but
> I suspect v2.16.2 would work just fine, too.
> 
> > jake@jake-VirtualBox:~/woot$ git --version
> > git version 2.14.1
> > ...
> > The expected result is that when I do `ls subdir` the file
> > "untracked.txt" still exists.  Alternatively, git stash should warn me
> > before destroying my untracked files, and require I specify --force or
> > similar to invoke destructive behavior.

I can't seem to reproduce this on 2.16.2.windows.1, either:

+ git --version
git version 2.16.2.windows.1
+ git init woot
Initialized empty Git repository in /woot/.git/
+ cd woot
+ mkdir subdir
+ echo test
+ echo test
+ git add meh.txt subdir/meh2.txt
+ git commit '--message=stash bug testing'
[master (root-commit) afec47d] stash bug testing
 2 files changed, 2 insertions(+)
 create mode 100644 meh.txt
 create mode 100644 subdir/meh2.txt
+ git commit '--message=stash bug testing'
On branch master
nothing to commit, working tree clean
+ echo test
+ echo append
+ git stash push subdir
Saved working directory and index state WIP on master: afec47d stash bug testing
+ ls subdir
meh2.txt  untracked.txt

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-14 Thread Igor Djordjevic
Hi Sergey,

On 14/03/2018 08:21, Sergey Organov wrote:
> 
> There are still 2 issues about the implementation that need to be
> discussed though:
> 
> 1. Still inverted order of the second merge compared to RFC.
> 
> It'd be simple to "fix" again, except I'm not sure it'd be better, and
> as there is no existing experiences with this step to follow, it
> probably should be left as in the original, where it means "merge the
> changes made in B' (w.r.t B) into our intermediate version of the
> resulting merge".
> 
> The original Phillip's version seems to better fit the asymmetry between
> mainline and side-branch handling.
> 
> The actual difference will be only in the order of ours vs theirs in
> conflicts though, and thus it's not that critical.

Shouldn`t this be easy to solve just by changing the order of  
and , on passing to `git merge-recursive`, if needed? (or 
that`s what you meant by "simple to fix"?)

> 2. The U1' == U2' consistency check in RFC that I still think is worth
> to be implemented.

At the moment, I think we`d appreciate test cases where it actually 
proves useful, as the general consensus seems to be leaning towards 
it possibly being annoying (over-paranoid).

> In application to the method being discussed, we only need the check if
> the final merge went without conflicts, so the user was not already
> involved, and the check itself is then pretty simple:
> 
>  "proceed without stop only if $tree = $tree_U1'"
> 
> Its equivalence to the U1' == U2' test in the RFC follows from the fact
> that if M' is non-conflicting merge of U1' and U2', then M' == U1' if
> and only if U2' == U1'.

Nicely spot! I`m glad there`s still (kind of) former U1' == U2' check 
in this approach, too, in case it proves useful :)

> Finally, here is a sketch of the implementation that I'd suggest to
> use:
> 
> git-rebase-first-parent --onto A' M
> tree_U1'=$(git write-tree)
> git merge-recursive B -- $tree_U1' B'
> tree=$(git write-tree)
> M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB')
> [ $conflicted_last_merge = "yes" ] ||
>   trees-match $tree_U1' $tree || 
>   stop-for-user-amendment

Yes, in case where we would want the "no-op merge" check (equivalent 
to U1' == U2' with original approach), this aligns with something I 
would expect.

Note that all the "rebase merge commit" steps leading to the check 
will/should probably be observed as a single one from user`s perspective 
(in worst case ending with nested conflicts we discussed), thus 
`$conflicted_last_merge` is not related to `merge-recursive` step(s) 
only, but `rebase-first-parent`, too (just in case this isn`t implied).

Might be easier to reason about simply as `[ $conflicts = "yes" ] || `

> where 'git-rebase-first-parent' denotes whatever machinery is currently
> being used to rebase simple non-merge commit. Handy approximation of
> which for stand-alone scripting is:
> 
> git checkout --detach A' && git cherry-pick -m 1 M
> 
> [As an interesting note, observe how, after all, that original Johannes
> Sixt's idea of rebasing of merge commit by cherry-picking its first
> parent is back there.]

Heh ;) It`s always a bit enlightening when people start from different 
positions, opposing ones, even (or so it may seem, at least), but 
eventually end up in the same place, through means of open (minded) 
discussion.

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-14 Thread Igor Djordjevic
On 14/03/2018 15:24, Sergey Organov wrote:
> 
> > > Second side note: if we can fast-forward, currently we prefer that, and I
> > > think we should keep that behavior with -R, too.
> >
> > I agree.
> 
> I'm admittedly somewhat lost in the discussion, but are you talking
> fast-forward on _rebasing_ existing merge? Where would it go in any of
> the suggested algorithms of rebasing and why?
> 
> I readily see how it can break merges. E.g., any "git merge --ff-only
> --no-ff" merge will magically disappear. So, even if somehow supported,
> fast-forward should not be performed by default during _rebasing_ of a
> merge.

Hmm, now that you brought this up, I can only agree, of course.

What I had in my mind was more similar to "no-rebase-cousins", like 
if we can get away without actually rebasing the merge but still 
using the original one, do it. But I guess that`s not what Johannes 
originally asked about.

This is another definitive difference between rebasing (`pick`?) and 
recreating (`merge`) a merge commit - in the case where we`re rebasing, 
of course it doesn`t make sense to drop commit this time (due to 
fast-forward). This does make sense in recreating the merge (only).

> > > If the user wants to force a new merge, they simply remove that -R
> > > flag.

And this sounds wrong now, too, because we actually have _three_
possible behaviors here - (1) rebase merge commit, which should 
always do what its told (so no fast-forwarding, otherwise the whole 
concept of rebasing a merge commit doesn`t make sense), and recreate 
merge commit, which should (2) by default use fast-forward where 
possible (or whatever the settings say), but (3) also be possible to 
force a new merge as well (through standard `--no-ff`, I guess, or 
something).

> Alternatively, they'd replace 'pick' with 'merge', as they already do
> for other actions. "A plurality is not to be posited without necessity".
> 
> Please, _please_, don't use 'merge' command to 'pick' merge commits!
> It's utterly confusing!

I agree here, as previously discussed[1], but let`s hear Johannes.

> Thinking about it I've got an idea that what we actually need is
> --no-flatten flag that, when used alone, will just tell "git rebase" to
> stop flattening history, and which will be implicitly imposed by
> --recreate-merges (and --preserve-merges).
> 
> Then the only thing the --recreate-merges will tune is to put 'merge'
> directives into the todo list for merge commits, exactly according to
> what its name suggests, while the default behavior will be to put 'pick'
> with suitable syntax into the todo. And arguments to the
> --recreate-merge will specify additional options for the 'merge'
> directive, obviously.

This seem to basically boil down to what I mentioned previously[2] 
through use of new `--rebase-merges` alongside `--recreate-merges`, just 
that you named it `--no-flatten` here, but the point is the same - and 
not something Johannes liked, "polluting" rebase option space further.

I would agree with him, and settling onto `--rebase-merges` _instead_ of 
`--recreate-merges` seems as a more appropriate name, indeed, now that 
default behavior is actually merge commit rebasing and not recreating 
(recreating still being possible through user editing the todo list).

Now, the only thing left seems to be agreeing on actual command to 
use to rebase the merge commit, to `pick` it, so to say... ;)

Regards, Buga

[1] https://public-inbox.org/git/77b695d0-7564-80d7-d9e6-70a531e66...@gmail.com/
[2] https://public-inbox.org/git/b329bb98-f9d6-3d51-2513-465aad2fa...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-13 Thread Igor Djordjevic
Hi Sergey,

On 13/03/2018 17:10, Sergey Organov wrote:
> 
> > Hi Sergey, I've been following this discussion from the sidelines,
> > though I haven't had time to study all the posts in this thread in
> > detail. I wonder if it would be helpful to think of rebasing a merge as
> > merging the changes in the parents due to the rebase back into the
> > original merge. So for a merge M with parents A B C that are rebased to
> > A' B' C' the rebased merge M' would be constructed by (ignoring shell
> > quoting issues)
> >
> > git checkout --detach M
> > git merge-recursive A -- M A'
> > tree=$(git write-tree)
> > git merge-recursive B -- $tree B'
> > tree=$(git write-tree)
> > git merge-recursive C -- $tree C'
> > tree=$(git write-tree)
> > M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB' -pC')
> 
> I wonder if it's OK to exchange the order of heads in the first merge
> (also dropped C for brevity):

It should be, being "left" or "right" hand side ("theirs" or "ours") 
of the three-way merge shouldn`t matter, they`re still both equally 
compared to the merge-base.

> git checkout --detach A'
> git merge-recursive A -- A' M
> tree=$(git write-tree)
> git merge-recursive B -- $tree B'
> tree=$(git write-tree)
> M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB')
> 
> If so, don't the first 2 lines now read: "rebase (first parent of) M on
> top of A'"?

Hmm, lol, yes...? :) So basically, this:

(1) git checkout --detach M
git merge-recursive A -- M A'
tree=$(git write-tree)
...

... is equivalent to this:

(2) git checkout --detach A'
git merge-recursive A -- A' M
tree=$(git write-tree)
...

..., being equivalent to this:

(3) git checkout --detach A'
git cherry-pick -m 1 M
tree=$(git write-tree)
...

..., where in all three cases that `$tree` is equivalent to U1' we 
discussed about so much already :)

I tested it like this as well, slightly modifying previously sent out 
script (like this one[1]), and it still seems to be working ;) Nice!

> If so, then it could be implemented so that it reduces back to regular
> rebase of non-merges when applied to a single-parent commit, similar to
> the method in the RFC, striking out one of advantages of the RFC.

I guess so, but I think it now boils down only to what one finds 
easier to reason about even more.

I`m just glad we got to U1' from this perspective as well, hopefully 
adding even more faith in the overall concept, being beaten from both 
ends and dropping out to be the same (minus minor implementation details).

Regards, Buga

[1] https://public-inbox.org/git/872944c4-ca97-9f55-a424-86d1e3299...@gmail.com/


Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-12 Thread Igor Djordjevic
Hi Dscho,

On 12/03/2018 11:20, Johannes Schindelin wrote:
> 
> > > [...] and cannot introduce ambiguities when rebasing the
> > > changes introduced by M (i.e. the "amendmendts" we talked about).
> >
> > Hmm, not following here, which ambiguities are we talking about?
> 
> U1' vs U2' of course. Those are two things that can be different, even if
> they ideally would have identical trees.
> 
> Phillip's strategy does not leave that room for ambiguity.

Ehm, in Sergey`s approach, this is not an issue, but a feature :)

If U1' != U2', it just means a more complex rebase happened, but it 
doesn`t compromise the result (rebased merge) in any way.

On the other hand, if U1' == U2', we can be pretty sure that merge 
rebasing went as clean as possible.

That`s the idea, at least.

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-12 Thread Igor Djordjevic
Hi Dscho,

On 12/03/2018 11:46, Johannes Schindelin wrote:
> 
> > Sometimes one just needs to read the manual, and I don`t really think
> > this is a ton complicated, but just something we didn`t really have
> > before (real merge rebasing), so it requires a moment to grasp the
> > concept.
> 
> If that were the case, we would not keep getting bug reports about
> --preserve-merges failing to reorder patches.

Not sure where that is heading to, but what I`m arguing about is that 
introducing new commands and concepts (`merge`, and with `-R`) just 
makes the situation even worse (more stuff to grasp).

Reusing existing concepts where possible doesn`t have this problem.

> > Saying in favor of `--rebase-merges`, you mean as a separate option,
> > alongside `--recreate-merges` (once that series lands)?
> 
> No. I am against yet another option. The only reason I pollute the option
> name space further with --recreate-merges is that it would be confusing to
> users if the new mode was called --preserve-merges=v2 (but work *totally
> differently*).

I see. So I take you`re thinking about renaming `--recreate-merges` 
to `--rebase-merges` instead?

That would seem sensible, too, I think, being the default usage mode 
in the first place. Being able to actually (re)create merges, too, 
once user goes interactive, would be "just" an additional (nice and 
powerful) feature on top of it.

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-12 Thread Igor Djordjevic
On 12/03/2018 13:56, Sergey Organov wrote:
> 
> > > I agree with both of you that `pick ` is inflexible 
> > > (not to say just plain wrong), but I never thought about it like that.
> > >
> > > If we are to extract further mentioned explicit old:new merge 
> > > parameter mapping to a separate discussion point, what we`re 
> > > eventually left with is just replacing this:
> > >
> > >   merge -R -C  
> > >
> > > ... with this:
> > >
> > >   pick  
> >
> > I see where you are coming from.
> >
> > I also see where users will be coming from. Reading a todo list in the
> > editor is as much documentation as it is a "program to execute". And I am
> > afraid that reading a command without even mentioning the term "merge"
> > once is pretty misleading in this setting.
> >
> > And even from the theoretical point of view: cherry-picking non-merge
> > commits is *so much different* from "rebasing merge commits" as discussed
> > here, so much so that using the same command would be even more
> > misleading.
> 
> This last statement is plain wrong when applied to the method in the
> [RFC] you are replying to. Using the method in [RFC], "cherry-pick
> non-merge" is nothing more or less than reduced version of generic
> "cherry-pick merge", exactly as it should be.
> 
> Or, in other words, "cherry-pick merge" is generalization of
> "cherry-pick non-merge" to multiple parents.

I think Sergey does have a point here, his approach showing it.

Phillip`s simplification might be further from it, though, but we`re 
talking implementation again - important mental model should just be 
"rebasing a commit" (merge or non-merge), how we`re doing it is 
irrelevant for the user, the point (goal) is the same.


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-12 Thread Igor Djordjevic
Hi Dscho,

On 12/03/2018 11:37, Johannes Schindelin wrote:
> 
> > If we are to extract further mentioned explicit old:new merge 
> > parameter mapping to a separate discussion point, what we`re 
> > eventually left with is just replacing this:
> >
> > merge -R -C  
> >
> > ... with this:
> >
> > pick  
> 
> I see where you are coming from.
> 
> I also see where users will be coming from. Reading a todo list in the
> editor is as much documentation as it is a "program to execute". And I am
> afraid that reading a command without even mentioning the term "merge"
> once is pretty misleading in this setting.
> 
> And even from the theoretical point of view: cherry-picking non-merge
> commits is *so much different* from "rebasing merge commits" as discussed
> here, so much so that using the same command would be even more
> misleading.

I would disagree here, as it seems you`re going too much into 
implementation and theory here, where it shouldn`t really matter from 
the user`s point of view - the point is to rebase a commit, `pick` it 
from one place and plant it elsewhere.

Yes, some commits might have a bit different semantics then others 
(merge vs non-merge), but it should just be an implementation detail, 
in my opinion, no need to leak it in user`s face (more than necessary).

I feel that "merge" is a command that works really well in the 
mindset of (re)creating merges. But if we are "only" rebasing an 
existing merge, `pick` seems much more appropriate (to me, at least), 
and it aligns with what I`m already expecting `pick` to be doing.

Down below, if we are (re)creating the merge, or doing magic to 
somehow just port it over, should be irrelevant. So "rebase" equals 
"pick and plant" (port), not "merge".

> > That is what I had in mind, seeming possibly more straightforward and 
> > beautifully aligned with previously existing (and well known) 
> > `rebase` terminology.
> >
> > Not to say this would make it possible to use other `rebase -i` todo 
> > list commands, too, like if you want to amend/edit merge commit after 
> > it was rebased, you would write:
> >
> > edit  
> >
> > ..., where in case you would simply like to reword its commit 
> > message, it would be just:
> >
> > reword  
> >
> >
> > Even `squash` and `fixup` could have their place in combination with 
> > a (to be rebased) merge commit, albeit in a pretty exotic rebases, 
> > thus these could probably be just disallowed - for the time being, at 
> > least.
> 
> Sure, for someone who read the manual, that would be easy to use. Of
> course, that's the minority.

I`m not following you here - the point is these are already existing 
commands, which would still fit in just nicely, so nothing new to 
learn nor read.

Now, if we are to discuss use cases where people don`t even know what 
they`re doing, I would think that misses the point. Besides, it`s 
always easier to make more mistakes when you introduce yet more 
commands/semantics to think about/learn, and I think it can be avoided 
here, for the better.

> Also: the `edit` command is poorly named to begin with. A much cleaner
> design would be to introduce the `break` command as suggested by Stephan.

This is orthogonal to what we`re discussing. Existing commands might 
not be perfect, but that`s what we have now, so let`s be consistent, 
not putting additional burden on the user there, at least.

But for the record - I tend to agree, I often find myself wondering 
if `edit`-ed commit means `rebase` stops after applying the changes 
and _before_ making the commit itself (so we just edit and 
--continue), or _after_ it (so we edit, `commit --amend` and 
--continue).

> > The real power would be buried in implementation, learning to rebase 
> > merge commits, so user is left with a very familiar interface, slightly 
> > adapted do accommodate a bit different nature of merge commit in 
> > comparison to an ordinary one, also to allow a bit more of interactive 
> > rebase functionality, but it would pretty much stay the same, without 
> > even a need to learn about new `merge`, `-R`, `-C`, and so on.
> >
> > Yes, those would have its purpose, but for real merging then 
> > (creating new merges, or recreating old ones), not necessarily for 
> > merge rebasing.
> >
> > With state of `merge -R -C ...` (that `-R` being the culprit), it 
> > kind of feels like we`re now trying to bolt "rebase merges" 
> > functionality onto a totally different one (recreate merges, serving 
> > a different purpose), making them both needlessly dependent on each 
> > other, further complicating user interface, making it more confusing 
> > and less tunable as per each separate functionality needs (rebase vs. 
> > recreate).
> >
> > I guess I`m the one to pretty much blame here, too, as I really 
> > wanted `--recreate-merges` to handle "rebase merges" better, only to 
> > later realize it might not be the best tool for the job, and that a 
> > more separate approach would be better (at least 

Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-12 Thread Igor Djordjevic
Hi Dscho,

On 11/03/2018 23:04, Igor Djordjevic wrote:
> 
> I`m yet to read (and reason about) your whole (very informative) 
> reply, but I just wanted to address this part first, as it might be a 
> clear end-game situation already, due to a mutual agreement, all the 
> rest being purely academic, interesting, but not any more (that) 
> important to discuss.

Ok, here`s the follow-up.

It`s "for discussion sake only", nothing really groundbreaking in 
here, I would think.

On 11/03/2018 16:40, Johannes Schindelin wrote:
> 
> > > > The main problem with this decision is that we still don't see how
> > > > and when to stop for user amendment using this method. OTOH, the
> > > > original has this issue carefully discussed.
> > >
> > > Why would we want to stop, unless there are merge conflicts?
> >
> > Because we can reliably know that something "unusual" happened - and by
> > that I don`t necessarily mean "wrong", but just might be worth user
> > inspection.
> 
> We have a similar conundrum in recursive merges. Remember how multiple
> merge bases are merged recursively? There can be merge conflicts, too, in
> *any* of the individual merges involved, and indeed, there are (under
> relatively rare circumstances).
> 
> Since we already faced that problem, and we already answered it by
> presenting possibly nested merge conflicts, I am in strong favor of
> keeping our new scenario consistent: present possibly-nested merge
> conflicts.

This is something I didn`t really know (possibly-nested merge 
conflicts already being a regular part of Git user experience), 
thanks for explaining it.

In the light of this, I can only agree, let`s keep it consistent.

If anyone ever decides / finds out there`s a better approach in 
regards to user experience, this might get revised, but it`s a 
different beast altogether, yes.

> As far as I understand, one of the arguments in favor of the current
> approach was: there is no good way to tell the user where they are, and
> how to continue from there. So better just to continue and present the
> user with the entire set of conflicts, and have an obvious way out.

Yes, I see this as the main concern, too. I would have expected that 
being in a kind of a "limbo" for a while shouldn`t be too bad, but I 
guess that`s too academic (and inexperienced) thought, and from a 
practical point of view one may not really know how to approach the 
(iterative) conflicts in the first place, not knowing his exact 
position (nor what`s to come)...?

Or, might be we _can_ provide enough clues on where we currently are 
(even if still inside some intermediate state)...? But, this still 
might be a topic for the future, indeed, and unrelated to rebasing 
merges alone (as you pointed out already).

> > For example, situation like this (M is made on A3 with `-s ours`, 
> > obsoleting Bx commits):
> >
> > (1) ---X8--X9 (master)
> >|\
> >| A1---A2---A3
> >| \
> >|  M (topic)
> >| /
> >\-B1---B2---B3
> >
> > ... where we want to rebase M onto X9 is what I would call "usual 
> > stuff", but this situation (M is still made on A3 with `-s ours`, 
> > obsoleting Bx commits, but note cherry-picked B2'):
> >
> > (2) ---X8--B2'--X9 (master)
> >|\
> >| A1---A2---A3
> >| \
> >|  M (topic)
> >| /
> >\-B1---B2---B3
> >
> > ... where we still want to rebase M onto X9 is what we might consider 
> > "unusual", because we noticed that something that shouldn`t be part 
> > of the rebased merge commit (due to previous `-s ours`) actually got 
> > in there (due to later cherry-pick), and just wanting the user to 
> > check and confirm.
> 
> We already have those scenarios when performing a regular interactive
> rebase, where a patch was already applied upstream. In the normal case,
> the user is not even shown B2, thanks to the --cherry-pick option used in
> generating the todo list.
> 
> Granted, in some cases --cherry-pick does not detect that, and then we
> generate a todo list including B2, and when that patch is applied, the
> interactive rebase stops, saying that there are no changes to be
> committed.
> 
> And this behavior is exactly the same with --recreate-merges!
> 
> So I do not think that it would make sense to bother the user *again* when
> rebasing the merge commit.

This seems fair enough. Phillip also pointed out it might be more 
annoyance then help, but as no one was really sure of the possibilities 
we`re discussing here, I tho

Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-11 Thread Igor Djordjevic
Hi Dscho,

I`m yet to read (and reason about) your whole (very informative) 
reply, but I just wanted to address this part first, as it might be a 
clear end-game situation already, due to a mutual agreement, all the 
rest being purely academic, interesting, but not any more (that) 
important to discuss.

On 11/03/2018 16:40, Johannes Schindelin wrote:
> 
> > For myself, I do actually favor Sergey`s approach in general, but 
> > _implemented_ through what Phillip described (or a mixture of both, to 
> > be precise). But, let me explain... :)
> 
> So as you explained later in this sub-thread, Sergey's approach is
> essentially the same as Phillip's.
> 
> I still do not understand Sergey's approach on a fundamental level. I
> mean, I can follow his instructions how to implement his algorithm, but it
> is as if I had a blindfold on and somebody guided me through a maze: I
> understand *what* I am supposed to do, but I have no clue *why*.
> 
> And admittedly, I got very frustrated when a document was thrown my way
> that is too long to read in one sitting, and all of my attempts at getting
> clear and comprehensible answers to specific questions were met with "go
> and read that document, I am sure you will understand then".
> 
> For something as fundamental to my daily workflow as an interactive rebase
> (*especially* when trying to maintain the branch topology), this is no
> good at all.
> 
> Since you already confirmed that there is essentially no difference
> between the two approaches, I will simply go with the one I understand, in
> particular I understand *why* it works.
> 
> But let's read on, maybe I will change my mind based on your explanations
> (which do answer my questions, thank you so much for that)...

No problem, I learned much myself trying to write those explanations 
in the first place, and I still need to read on yet myself, seeing 
how well my explanations actually fared :) Thank you for still 
holding on, though.

But I just wanted to point out that you can really just go with what 
Phillip described if you find that easier to reason about (and/or 
implement), there`s even no need for mind changing, as essentially, 
and in my opinion, it seems to be just a bit different implementation 
of the same concept (but not requiring temporary commits).

That said, *if* we decide we like temporary commit U1' == U2' consistency 
check (especially for non-interactive rebase, maybe), we can produce 
these after the fact for the sake of the check only.

I will come with a follow-up, but all the rest might be less important.

Regards, Buga


Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-11 Thread Igor Djordjevic
Hi Dscho,

On 11/03/2018 16:47, Johannes Schindelin wrote:
> 
> > > > Phillip's method is essentially merging the new tips into the original
> > > > merge, pretending that the new tips were not rebased but merged into
> > > > upstream.
> > >
> > > [...]
> > >
> > > Here`s a starting point, two commits A and B, merged into M:
> > >
> > > (3) ---A
> > > \
> > >  M
> > > /
> > > ---B
> > >
> > >
> > > According the "patch theory"[1] (which might not be too popular 
> > > around here, but should serve the purpose for what I`m trying to 
> > > explain), each merge commit can be "transformed" to two non-merge 
> > > commits, one on top of each of the merge parents, where new commit 
> > > brings its original merge parent commit tree to the state of the 
> > > merge commit tree:
> > >
> > > (4) ---A---U1
> > >
> > >
> > >
> > > ---B---U2
> > >
> > >
> > > Now, we have two new commits, U1 and U2, each having the same tree as 
> > > previous merge commit M, but representing changes in regards to 
> > > specific parents - and this is essentially what Sergey`s original 
> > > approach was using (whether he knew it, or not).
> > >
> > > When it comes to rebasing, it`s pretty simple, too. As this:
> > >
> > > (5) ---X1---o---o---o---o---o---X2 (master)
> > >|\
> > >| A1---A2---A3
> > >| \
> > >|  M
> > >| /
> > >\-B1---B2---B3
> > >
> > > ... actually equals this:
> > >
> > > (6) ---X1---o---o---o---o---o---X2 (master)
> > >|\
> > >| A1---A2---A3---U1
> > >|
> > >|
> > >|
> > >\-B1---B2---B3---U2
> > >
> > > ... where trees of M, U1 and U2 are same, and we can use the regular 
> > > rebase semantics and rebase it to this:
> > >
> > > (7) ---X1---o---o---o---o---o---X2 (master)
> > > |\
> > > | A1'--A2'--A3'--U1'
> > > |
> > > |
> > > |
> > > \-B1'--B2'--B3'--U2'
> > >
> > > ... which is essentially this again:
> > >
> > > (8) ---X1---o---o---o---o---o---X2 (master)
> > > |\
> > > | A1'--A2'--A3'
> > > |\
> > > | M'
> > > |/
> > > \-B1'--B2'--B3'
> > >
> >
> > Having explained all this, I realized this is the same "essentially 
> > merging the new tips into the original pretending that the new tips 
> > were not rebased but merged into upstream" as Phillip`s one, just 
> > that we have additional temporary commits U1 and U2 (as per mentioned 
> > "patch theory") :)
> 
> But if the old tips had been merged into upstream (resulting in the new
> tips), then the merge bases would be *the old tips*.

Exactly, and that is what part you`ve cut out of the quote was 
showing :) By Phillip`s implementation, we would start with *old tips* 
as merge bases, indeed (old tips being U1 and U2 in this case), where 
it further gets transformed as previously written:

>   git merge-recursive U1 -- M U1'
>   tree="$(git write-tree)"
>   git merge-recursive U2 -- $tree U2'
>   tree="$(git write-tree)"
> 
> ..., where we know U1 = U2 = M (in regards to trees), so this is the 
> same as:
> 
>   git merge-recursive M -- M U1'
>   tree="$(git write-tree)"
>   git merge-recursive M -- $tree U2'
>   tree="$(git write-tree)"

Here, `git merge-recursive M -- M U1'` simply equals to U1' tree 
(being a fast-forward merge), so we can write the two merges above as
a single merge, too:

>   git merge-recursive M -- U1' U2'
>   tree="$(git write-tree)"
> 
> ... which is exactly what Sergey`s (updated) approach suggests, 
> merging U1' and U2' with M as merge-base (and shown inside that 
> sample implementation script I provided[1]) :)

So from *old tips* being the rebased merge base (Phillip), we got to 
*old merge commit* being the rebased merge base (Sergey), or vice 
versa. Does this shed a bit more light on it now? Or you wanted to 
point out something else in the first place...?

> I am still not sure for what scenarios Phillip's strategy is the same as
> Sergey's (updated) one, as the former strategy can do completely without
> temporary commits [...]

I think the root of misunderstanding might be coming from the fact 
that Sergey was mainly describing a general concept (without a 
strictly defined implementation strategy, not being restricted to a 
specific one), where Phillip came up with a solution that eventually 
seems to use the same concept (as those transformations above should 
show), but simplifying it further inside a concrete implementation.

By saying that Phillip "simplified it", even though transformations 
shown above 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-11 Thread Igor Djordjevic
Hi Dscho,

On 11/03/2018 13:11, Johannes Schindelin wrote:
> 
> > > I did wonder about using 'pick ' for rebasing merges
> > > and keeping 'merge ...' for recreating them but I'm not sure if that
> > > is a good idea. It has the advantage that the user cannot specify the
> > > wrong parents for the merge to be rebased as 'git rebase' would work
> > > out if the parents have been rebased, but maybe it's a bit magical to
> > > use pick for merge commits. Also there isn't such a simple way for the
> > > user to go from 'rabase this merge' to 'recreate this merge' as they'd
> > > have to write the whole merge line themselves (though I guess
> > > something like emacs' git-rebase.el would be able to help with that)
> >
> > Since the ultimate commit hashes of newly rebased commits would be
> > unknown at the time of writing the todo file, I'm not sure how this
> > would work to specify the parents?
> 
> I agree with Phillip's follow-up that the `pick ` syntax
> would pose a problem, but for different reasons: We already tried it, with
> --preserve-merges, and it is just a really stupid syntax that does not
> allow the user even to reorder commits. Or drop commits (except at the
> very end of the todo list).

Hehe, please excuse me, but in the light of that other explicit (or 
not) parent mapping discussion[1], I would take a chance to be really 
sneaky here and say that being non-explicit "is just a really stupid 
syntax that does not allow the user even to reorder rebased merge 
parents. Or drop parents (except at the very end of the parent list)." ;)

[1] https://public-inbox.org/git/b329bb98-f9d6-3d51-2513-465aad2fa...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-11 Thread Igor Djordjevic
Hi Dscho,

On 11/03/2018 13:08, Johannes Schindelin wrote:
> 
> > Hmm, funny enough, `pick ` was something I though about
> > originally, too, feeling that it might make more sense in terms on
> > what`s really going on, but I guess I wanted it incorporated into
> > `--recreate-merges` too much that I tried really hard to fit it in,
> > without changing it much :/
> 
> The `pick ` syntax is too limited to allow reordering, let
> alone changing the parents.

I agree, `pick ` syntax alone is never what I had in 
mind, so it`s missing further context here, touched in that other 
subthread[1]. My fault, sorry for confusion.

> >   pick  :HEAD 
> > :
> 
> I do not really like it, as it makes things a ton less intuitive. If you
> did not know about this here discussion, and you did not read the manual
> (and let's face it: a UI that does not require users to read the manual is
> vastly superior to a UI that does), and you encountered this command:
> 
>   merge deadbeef cafecafe:download-button
> 
> what would you think those parameters would mean?
> 
> Granted, encountering
> 
>   merge -R -C deadbeef download-button # Merge branch 'download-button'
> 
> is still not *quite* as intuitive as I would wish. Although, to be honest,
> if I encountered this, I would think that I should probably leave the -R
> and the -C deadbeef alone, and that I could change what is getting merged
> by changing the `download-button` parameter.

Agreed, encountering mapping is slightly more complicated, but I 
would argue it`s much more powerful at the same time, too, thus 
pretty much worth it.

Without it, actually, it seems like we`re repeating the mistake of 
`--preserve-merges`, where we`re assuming too much (order of new and 
old parents being the same, and I guess number of them, too).

Oh, and as we`re still discussing in terms of `merge` command, using 
(elsewhere mentioned[1]) `pick` instead, it might be even less 
non-intuitive, as we`re not married to `merge` semantics any more:

pick deadbeef cafecafe:download-button


And might be calling it "non-intuitive" is unfair, I guess it would 
rather be "not familiar yet", being case with any new functionality, 
let alone a very powerful one, where getting a clue on what it does 
at the beginning could do wonders later.

Sacrificing that power for a bit of perceived simplicity, where it 
actually assumes stuff on its own (trying to stay simple for the 
user), doesn`t seem as a good way to go in the long run.

Sometimes one just needs to read the manual, and I don`t really think 
this is a ton complicated, but just something we didn`t really have 
before (real merge rebasing), so it requires a moment to grasp the 
concept.

But I`m still not sure if there isn`t a better way to present 
explicit mapping, just that : seemed as the most straightforward 
one to pass on the point for the purpose of discussing it.

And I`m not reluctant to simplifying user interface, or for dropping 
the explicit mapping altogether, even, just for carefully measuring 
what we could lose without explicit mapping - think flexibility, but 
ease and correctness of implementation, too, as we need to guess the 
old merge parents and which new one they should correspond to.

> > p.s. Are we moving towards `--rebase-merges` I mentioned in that 
> > other topic[1], as an add-on series after `--recreate-merges` hits 
> > the mainstream (as-is)...? :P
> 
> That's an interesting question. One that I do not want to answer alone,
> but I would be in favor of `--rebase-merges` as it is IMHO a much better
> name for what this option is all about.

Saying in favor of `--rebase-merges`, you mean as a separate option, 
alongside `--recreate-merges` (once that series lands)?

That does seem as the most clean, intuitive and straightforward 
solution. Depending on the option you provide (recreate vs rebase), 
todo list would be populated accordingly by default - but important 
thing is "todo list parser" would know to parse both, so one can 
still adapt todo list to both recreate (some) and rebase (some other) 
merges at the same time.

Of course, this only once `--rebase-merges` gets implemented, too, 
but as we had waited for it for so long, it won`t hurt to wait a bit 
more and possibly do it more properly, than rush it now and make a 
confusing user interface, needlessly welding two functionalities 
together (rebase vs. recreate).

But I guess you already knew my thoughts, so let`s see what other`s 
think, too ;)

Regards, Buga

[1] https://public-inbox.org/git/f4e6237a-84dc-1aa8-150d-041806e24...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-11 Thread Igor Djordjevic
Hi Dscho,

On 11/03/2018 13:00, Johannes Schindelin wrote:
> 
> > I actually like `pick` for _rebasing_ merge commits, as `pick` is 
> > already used for rebasing non-merge commits, too, so it feels natural.
> 
> Phillip is right, though: this would repeat the design mistake of
> --preserve-merges.
> 
> We must not forget that the interactive mode is the target here, and that
> the syntax (as well as the generated todo list) must allow for easy
> modification. The `pick ` approach does not allow that, so we
> cannot use it.
> 
> The `merge -R -C  ` approach is a lot better:
> it offers the flexibility, without sacrificing the ease when not modifying
> the todo list.

Eh, I`m afraid the quote you took is missing the rest of its 
(important) context, where I mentioned already proposed format for 
`pick` in that other subthread[1], including other parameters beside 
merge commit to pick, as that parent mapping.

I agree with both of you that `pick ` is inflexible 
(not to say just plain wrong), but I never thought about it like that.

If we are to extract further mentioned explicit old:new merge 
parameter mapping to a separate discussion point, what we`re 
eventually left with is just replacing this:

merge -R -C  

... with this:

pick  


That is what I had in mind, seeming possibly more straightforward and 
beautifully aligned with previously existing (and well known) 
`rebase` terminology.

Not to say this would make it possible to use other `rebase -i` todo 
list commands, too, like if you want to amend/edit merge commit after 
it was rebased, you would write:

edit  

..., where in case you would simply like to reword its commit 
message, it would be just:

reword  


Even `squash` and `fixup` could have their place in combination with 
a (to be rebased) merge commit, albeit in a pretty exotic rebases, 
thus these could probably be just disallowed - for the time being, at 
least.

The real power would be buried in implementation, learning to rebase 
merge commits, so user is left with a very familiar interface, slightly 
adapted do accommodate a bit different nature of merge commit in 
comparison to an ordinary one, also to allow a bit more of interactive 
rebase functionality, but it would pretty much stay the same, without 
even a need to learn about new `merge`, `-R`, `-C`, and so on.

Yes, those would have its purpose, but for real merging then 
(creating new merges, or recreating old ones), not necessarily for 
merge rebasing.

With state of `merge -R -C ...` (that `-R` being the culprit), it 
kind of feels like we`re now trying to bolt "rebase merges" 
functionality onto a totally different one (recreate merges, serving 
a different purpose), making them both needlessly dependent on each 
other, further complicating user interface, making it more confusing 
and less tunable as per each separate functionality needs (rebase vs. 
recreate).

I guess I`m the one to pretty much blame here, too, as I really 
wanted `--recreate-merges` to handle "rebase merges" better, only to 
later realize it might not be the best tool for the job, and that a 
more separate approach would be better (at least not through the same 
`merge` todo list command)...

Regards, Buga

[1] https://public-inbox.org/git/f3872fb9-01bc-b2f1-aee9-cfc0e4db7...@gmail.com/


Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-08 Thread Igor Djordjevic
On 08/03/2018 20:58, Igor Djordjevic wrote:
> 
> > Phillip's method is essentially merging the new tips into the original
> > merge, pretending that the new tips were not rebased but merged into
> > upstream.
> 
> [...]
> 
> Here`s a starting point, two commits A and B, merged into M:
> 
> (3) ---A
> \
>  M
> /
> ---B
> 
> 
> According the "patch theory"[1] (which might not be too popular 
> around here, but should serve the purpose for what I`m trying to 
> explain), each merge commit can be "transformed" to two non-merge 
> commits, one on top of each of the merge parents, where new commit 
> brings its original merge parent commit tree to the state of the 
> merge commit tree:
> 
> (4) ---A---U1
> 
> 
> 
> ---B---U2
> 
> 
> Now, we have two new commits, U1 and U2, each having the same tree as 
> previous merge commit M, but representing changes in regards to 
> specific parents - and this is essentially what Sergey`s original 
> approach was using (whether he knew it, or not).
> 
> When it comes to rebasing, it`s pretty simple, too. As this:
> 
> (5) ---X1---o---o---o---o---o---X2 (master)
>|\
>| A1---A2---A3
>| \
>|  M
>| /
>\-B1---B2---B3
> 
> ... actually equals this:
> 
> (6) ---X1---o---o---o---o---o---X2 (master)
>|\
>| A1---A2---A3---U1
>|
>|
>|
>\-B1---B2---B3---U2
> 
> ... where trees of M, U1 and U2 are same, and we can use the regular 
> rebase semantics and rebase it to this:
> 
> (7) ---X1---o---o---o---o---o---X2 (master)
> |\
> | A1'--A2'--A3'--U1'
> |
> |
> |
> \-B1'--B2'--B3'--U2'
> 
> ... which is essentially this again:
> 
> (8) ---X1---o---o---o---o---o---X2 (master)
> |\
> | A1'--A2'--A3'
> |\
> | M'
> |/
> \-B1'--B2'--B3'
> 

Having explained all this, I realized this is the same "essentially 
merging the new tips into the original pretending that the new tips 
were not rebased but merged into upstream" as Phillip`s one, just 
that we have additional temporary commits U1 and U2 (as per mentioned 
"patch theory") :)

Merging U1' and U2' with M as a base can initially be presented like 
this as well (what Phillip`s approach would yield):

git merge-recursive U1 -- M U1'
tree="$(git write-tree)"
git merge-recursive U2 -- $tree U2'
tree="$(git write-tree)"

..., where we know U1 = U2 = M (in regards to trees), so this is the 
same as:

git merge-recursive M -- M U1'
tree="$(git write-tree)"
git merge-recursive M -- $tree U2'
tree="$(git write-tree)"

..., which can be further simplified, being the same as:

git merge-recursive M -- U1' U2'
tree="$(git write-tree)"

... which is exactly what Sergey`s (updated) approach suggests, 
merging U1' and U2' with M as merge-base (and shown inside that 
sample implementation script I provided[1]) :)

With all this said, I think it`s safe to say these two approaches are 
exactly the same, just Sergey`s being simplified (thus harder to 
initially understand), and the only actual question is whether we 
value U1' and U2' enough, as possible "something suspicious happened" 
indicators, to use them, or not.

I would think yes, but I would be open for more samples of where do 
they become useful for reporting "suspicious activity", too.

Regards, Buga

[1] https://public-inbox.org/git/b11785bd-5c96-43c1-95d8-b28eccfd1...@gmail.com/


Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-08 Thread Igor Djordjevic
On 08/03/2018 21:27, Igor Djordjevic wrote:
> 
> > git merge-recursive U1' -- M U2'
> > tree="$(git write-tree)"
> > # in case of original merge being octopus, we would continue like:
> > # git merge-recursive $tree -- M U3'
> > # tree="$(git write-tree)"
> > # git merge-recursive $tree -- M U4'
> > # ... and so on, then finally:
> > git merge-recursive $tree -- "$(git merge-base U1' U2' B1')" B1'
> > # in more general case, it would be:
> > # git merge-recursive $tree -- "$(git merge-base 
> > )" B1'
> > tree="$(git write-tree)"
> > git tag M' "$(git log --pretty=%B -1 M | git commit-tree $tree -p B3' 
> > -p B4 -p B1')"
> 
> That last line should obviously read just:
> 
>   git log --pretty=%B -1 M | git commit-tree $tree -p B3' -p B4 -p B1'
> 
> ..., above mentioned `git tag M'` part being a leftover from my other test 
> script.

Eh, pardon me, I managed to mess up all the merge-recursive lines, 
too, in regards to where the merge-base commit goes... Here`s a 
complete (and corrected) sample:

git merge-recursive M -- U1' U2'
tree="$(git write-tree)"
# in case of original merge being octopus, we would continue like:
# git merge-recursive M -- $tree U3'
# tree="$(git write-tree)"
# git merge-recursive M -- $tree U4'
# ... and so on, then finally:
git merge-recursive "$(git merge-base U1' U2' B1')" -- $tree B1'
# ... or even:
# git merge-recursive "$(git merge-base B3' B4 B1')" -- $tree B1'
# as in more general case, it would be:
# git merge-recursive "$(git merge-base 
)" -- $tree B1'
tree="$(git write-tree)"
git log --pretty=%B -1 M | git commit-tree $tree -p B3' -p B4 -p B1'


Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-08 Thread Igor Djordjevic
On 08/03/2018 20:58, Igor Djordjevic wrote:
> 
>   git merge-recursive U1' -- M U2'
>   tree="$(git write-tree)"
>   # in case of original merge being octopus, we would continue like:
>   # git merge-recursive $tree -- M U3'
>   # tree="$(git write-tree)"
>   # git merge-recursive $tree -- M U4'
>   # ... and so on, then finally:
>   git merge-recursive $tree -- "$(git merge-base U1' U2' B1')" B1'
>   # in more general case, it would be:
>   # git merge-recursive $tree -- "$(git merge-base 
> )" B1'
>   tree="$(git write-tree)"
>   git tag M' "$(git log --pretty=%B -1 M | git commit-tree $tree -p B3' 
> -p B4 -p B1')"

That last line should obviously read just:

git log --pretty=%B -1 M | git commit-tree $tree -p B3' -p B4 -p B1'

..., above mentioned `git tag M'` part being a leftover from my other test 
script.


Re: [RFC v2] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-08 Thread Igor Djordjevic
Hi Johannes,

On 07/03/2018 15:08, Johannes Schindelin wrote:
> 
> > > Didn't we settle on Phillip's "perform successive three-way merges
> > > between the original merge commit and the new tips with the old tips
> > > as base" strategy?
> >
> > It seems you did, dunno exactly why.
> 
> That is not true. You make it sound like I was the only one who liked
> this, and not Phillip and Buga, too.

For myself, I do actually favor Sergey`s approach in general, but 
_implemented_ through what Phillip described (or a mixture of both, to 
be precise). But, let me explain... :)

> > The main problem with this decision is that we still don't see how and
> > when to stop for user amendment using this method. OTOH, the original
> > has this issue carefully discussed.
> 
> Why would we want to stop, unless there are merge conflicts?

Because we can reliably know that something "unusual" happened - and 
by that I don`t necessarily mean "wrong", but just might be worth 
user inspection.

For example, situation like this (M is made on A3 with `-s ours`, 
obsoleting Bx commits):

(1) ---X8--X9 (master)
   |\
   | A1---A2---A3
   | \
   |  M (topic)
   | /
   \-B1---B2---B3

... where we want to rebase M onto X9 is what I would call "usual 
stuff", but this situation (M is still made on A3 with `-s ours`, 
obsoleting Bx commits, but note cherry-picked B2'):

(2) ---X8--B2'--X9 (master)
   |\
   | A1---A2---A3
   | \
   |  M (topic)
   | /
   \-B1---B2---B3

... where we still want to rebase M onto X9 is what we might consider 
"unusual", because we noticed that something that shouldn`t be part 
of the rebased merge commit (due to previous `-s ours`) actually got 
in there (due to later cherry-pick), and just wanting the user to 
check and confirm.

This is the major reason why I would prefer Sergey`s approach in 
general... and might be I also have a good explanation on *why* it 
works, but let`s get there ;)

(p.s. This is only one, but certainly not the only case)

> > "rebase sides of the merge commit and then three-way merge them back
> > using original merge commit as base"
> 
> And that is also wrong, as I had proved already! Only Buga's addition made
> it robust against dropping/modifying commits, and that addition also makes
> it more complicated.

No, this is actually right, that sentence nicely describing _how_ it 
works. That addition of mine was just including the correct merge 
base (being the original merge commit that we are "rebasing"), and 
that`s what Sergey is talking about.

> And it still has no satisfactory simple explanation why it works.

Getting there... :)

> > > - it is *very easy* to reason about, once it is pointed out that
> > > rebases and merges result in the same trees.
> >
> > The original is as easy to reason about, if not easier, especially as
> > recursive merge strategy is not being used there in new ways.
> 
> So do it. I still have to hear a single-sentence, clear and obvious
> explanation why it works.
> 
> And please do not describe why your original version works, because it
> does not work. Describe why the one amended with Buga's hack works.
> 
> > I honestly don't see any advantages of Phillip's method over the
> > original, except personal preferences. At the same time, I have no
> > objection of using it either, provided consistency check problem is
> > solved there as well.
> 
> Okay, let me reiterate then, because I do not want this point to be
> missed:
> 
> Phillip's method is essentially merging the new tips into the original
> merge, pretending that the new tips were not rebased but merged into
> upstream.
> 
> So it exploits the duality of the rebase and merge operation, which both
> result in identical trees (potentially after resolving merge conflicts).
> 
> I cannot think of any such interpretation for your proposal augmented by
> Buga's fix-ups. And I haven't heard any such interpretation from your
> side, either.

Ok here it goes (I might be still wrong, but please bare with me).

What Sergey originally described also uses the "duality of the rebase 
and merge operation", too ;) Just in a bit different way (and might 
be a more straightforward one?).

Here`s a starting point, two commits A and B, merged into M:

(3) ---A
\
 M
/
---B


According the "patch theory"[1] (which might not be too popular 
around here, but should serve the purpose for what I`m trying to 
explain), each merge commit can be "transformed" to two non-merge 
commits, one on top of each of the merge parents, where new commit 
brings its original merge parent commit tree to the state of the 
merge commit tree:

(4) ---A---U1



---B---U2


Now, we have two new commits, U1 and U2, each having the same tree as 
previous merge commit M, but representing changes in regards to 
specific parents - and this is essentially what Sergey`s original 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-08 Thread Igor Djordjevic
On 06/03/2018 12:45, Sergey Organov wrote:
> 
> > > The only thing I wonder of here is how would we check if the 
> > > "rebased" merge M' was "clean", or should we stop for user amendment? 
> > > With that other approach Sergey described, we have U1'==U2' to test with.
> >
> > I think (though I haven't rigorously proved to myself) that in the
> > absence of conflicts this scheme has well defined semantics (the merges
> > can be commuted), so the result should be predicable from the users
> > point of view so maybe it could just offer an option to stop.
> 
> Yes, hopefully it's predictable, but is it the intended one? We don't
> know, so there is still some level of uncertainty.
> 
> When in doubt, I try to find similar cases. There are two I'm aware of:
> 
> 1. "git merge" just commits the result when there are no conflicts.
> However, it supposedly has been run by the user just now, and thus user
> can amend what he gets. That's effectively a stop for amendment from our
> POV.
> 
> 2. When rebasing, "rerere", when fires, stages the changes, and rebasing
> stops for amendment. For me "rerere" behavior is rather annoying (I've
> never in fact amended what it prepared), but I always assumed there are
> good reasons it behaves this way.
> 
> Overall, to be consistent, it seems we do need to stop at U1' != U2', at
> least by default. Additional options could be supported then to specify
> user intentions, both on the command level and in the todo list,
> provided it proves to be useful.

Just to say I agree with this, `if U1' == U2' then proceed else stop` 
seems as a good sanity check.


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-08 Thread Igor Djordjevic
On 08/03/2018 16:16, Igor Djordjevic wrote:
> 
> > Unless we reimplement the octopus merge (which works quite a bit
> > differently from the "rebase merge commit" strategy, even if it is
> > incremental, too), which has its own challenges: if there are merge
> > conflicts before merging the last MERGE_HEAD, the octopus merge will exit
> > with status 2, telling you "Should not be doing an octopus.". While we
> > will want to keep merge conflict markers and continue with the "rebase the
> > original merge commit" strategy.
> >
> > [...]
> 
> The thing is, in my opinion, as long as we are _rebasing_, you can`t 
> pick any merge strategy, as it doesn`t really make much sense. If you 
> do want a specific strategy, than that`s _recreating_ a merge, and it 
> goes fine with what you already have for `--recreate-merges`.
> 
> On merge rebasing, the underlying strategy we decide to use is just an 
> implementation detail, picking the one that works best (or the only 
> one that works, even), user should have nothing to do with it.

Just to add, if not already assumable, that I think we should stop 
and let user react on conflicts on each of the "rebase the original 
commit" strategy steps (rebase first parent, rebase second parent... 
merge parents).

I guess this stresses not using real "octopus merge" strategy even in 
case where we`re rebasing octopus merge commit even more (and aligns 
nicely with what you seem to expect already).


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-08 Thread Igor Djordjevic
On 08/03/2018 13:16, Phillip Wood wrote:
> 
> > Side note: I wonder whether we really need to perform the additional check
> > that ensures that the  refers to the rewritten version of the
> > original merge commit's parent.
> > 
> > [...]
> 
> Oops that was referring to the first side note. I think fast forwarding
> is a good idea. I'm not so sure about checking that  refers
> to the rewritten version of the original merge commit's parent any more
> though. Having thought some more, I think we would want to allow the
> user to rearrange a topic branch that is the parent of a merge and that
> would require allowing a different parent as the old parent could be
> dropped or swapped with another commit in the branch. I can't think of a
> way to mechanically check that the new parent is 'somehow derived from'
> the old one.

Exactly, we must not depend on exact parent commits, but on parent 
"branches" (so to say).

And that is why I think explicit mapping would be pretty helpful (if 
not the only approach).

> > I did wonder about using 'pick ' for rebasing merges and
> > keeping 'merge ...' for recreating them but I'm not sure if that is a
> > good idea. It has the advantage that the user cannot specify the wrong
> > parents for the merge to be rebased as 'git rebase' would work out if
> > the parents have been rebased, but maybe it's a bit magical to use pick
> > for merge commits. Also there isn't such a simple way for the user to go
> > from 'rabase this merge' to 'recreate this merge' as they'd have to
> > write the whole merge line themselves (though I guess something like
> > emacs' git-rebase.el would be able to help with that)
> 
> Scrub that, it is too magical and I don't think it would work with
> rearranged commits - it's making the --preserve-merges mistake all over
> again. It's a shame to have 'merge' mean 'recreate the merge' and
> 'rebase the merge' but I don't think there is an easy way round that.

I actually like `pick` for _rebasing_ merge commits, as `pick` is 
already used for rebasing non-merge commits, too, so it feels natural.

Then `merge` is left to do what it is meant for - merging (or 
"recreate the merge", in the given context).

I tried to outline a possible user interface in that other reply[1], 
elaborating it a bit, too,

Regards, Buga

[1] https://public-inbox.org/git/f3872fb9-01bc-b2f1-aee9-cfc0e4db7...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-08 Thread Igor Djordjevic
Hi Phillip and Johannes, 

On 08/03/2018 12:20, Phillip Wood wrote:
> 
> I did wonder about using 'pick ' for rebasing merges and
> keeping 'merge ...' for recreating them but I'm not sure if that is a
> good idea. It has the advantage that the user cannot specify the wrong
> parents for the merge to be rebased as 'git rebase' would work out if
> the parents have been rebased, but maybe it's a bit magical to use pick
> for merge commits. Also there isn't such a simple way for the user to go
> from 'rabase this merge' to 'recreate this merge' as they'd have to
> write the whole merge line themselves (though I guess something like
> emacs' git-rebase.el would be able to help with that)

Hmm, funny enough, `pick ` was something I though 
about originally, too, feeling that it might make more sense in terms 
on what`s really going on, but I guess I wanted it incorporated into 
`--recreate-merges` too much that I tried really hard to fit it in, 
without changing it much :/

And now that I said this in a previous reply:

> The thing is, in my opinion, as long as we are _rebasing_, you can`t 
> pick any merge strategy, as it doesn`t really make much sense. If you 
> do want a specific strategy, than that`s _recreating_ a merge, and it 
> goes fine with what you already have for `--recreate-merges`.
> 
> On merge rebasing, the underline strategy we decide to use is just an 
> implementation detail, picking the one that works best (or the only 
> one that works, even), user should have nothing to do with it.

The difference between "rebase merge commit" and "recreate merge 
commit" might starting to be more evident.

So... I might actually go for this one now. And (trying to stick with 
explicit mappings, still :P), now that we`re not married to `merge` 
expectations a user may already have, maybe a format like this:

  pick  :HEAD :


Here, original-merge is a _commit_, where original-parent and 
new-parent are _labels_ (in terms of `--recreate-merges`).

Everything else I previously said still holds - one is allowed to 
change or drop mappings, and add or drop new merge parents. Yes, in 
case user does something "stupid", he`ll get a lot of conflicts, but 
hey, we shouldn`t judge.

p.s. Are we moving towards `--rebase-merges` I mentioned in that 
other topic[1], as an add-on series after `--recreate-merges` hits 
the mainstream (as-is)...? :P

Regards, Buga

[1] https://public-inbox.org/git/bc9f82fb-fd18-ee45-36a4-921a1381b...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-08 Thread Igor Djordjevic
Hi Dscho,

On 07/03/2018 08:26, Johannes Schindelin wrote:
> 
> > So, it could be something like:
> >
> > merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed
> 
> I like where this is heading, too, but I do not think that we can do this
> on a per-MERGE_HEAD basis. The vast majority of merge commits, in
> practice, have two parents. So the `merge` command would actually only
> have one revision to merge (because HEAD is the implicit first parent). So
> that is easy.
> 
> But as soon as you go octopus, you can either perform an octopus merge, or
> rebase the original merge commit. You cannot really mix and match here.
> 
> Unless we reimplement the octopus merge (which works quite a bit
> differently from the "rebase merge commit" strategy, even if it is
> incremental, too), which has its own challenges: if there are merge
> conflicts before merging the last MERGE_HEAD, the octopus merge will exit
> with status 2, telling you "Should not be doing an octopus.". While we
> will want to keep merge conflict markers and continue with the "rebase the
> original merge commit" strategy.
> 
> And it would slam the door shut for adding support for *other* merge
> strategies to perform a more-than-two-parents merge.

The thing is, in my opinion, as long as we are _rebasing_, you can`t 
pick any merge strategy, as it doesn`t really make much sense. If you 
do want a specific strategy, than that`s _recreating_ a merge, and it 
goes fine with what you already have for `--recreate-merges`.

On merge rebasing, the underline strategy we decide to use is just an 
implementation detail, picking the one that works best (or the only 
one that works, even), user should have nothing to do with it.

> Also, I do not think that it makes a whole lot of sense in practice to let
> users edit what will be used for "original parent". If the user wants to
> do complicated stuff, they can already do that, via `exec`. The `merge`
> command really should be about facilitating common workflows, guiding the
> user to what is sane.

I thought of a situation like this:

(1) ---o---o---o---M--- (master)
\ /
 X1--X2--X3 (topic)


Merge M was done with `-s ours`, obsoleting "topic" branch. But, I 
later realized that I actually do want that X2 commit in master.

Now, I guess the most obvious approach is just cherry-picking it, but 
what if I would like to do something like this instead, with an 
interactive rebase (and rebasing the merge, not recreating it):

(2) ---o---o---o---M'--- (master)
   |\ /|
   | X1'-X3'-/ | (topic)
   |   |
   \--X2'--/ (new)


This way, and having "topic" inherit original merge behavior due to 
merge rebasing, X1' and X3' would still be missing from M' as they 
did originally from M, but X2' would now be included, as it`s coming 
from a new branch, forged during interactive rebase.

(note - implementation wise, this still wouldn`t be an octopus merge ;)

> The vast majority of merge commits, in practice, have two parents. So
> the `merge` command would actually only have one revision to merge
> (because HEAD is the implicit first parent).

Now, this is something I actually overlooked :( 

I guess order of parent commits could then be used to map to old 
commit parents, being a limitation in comparison to direct 
old-parent:new-parent mapping, but might be a more straightforward 
user experience...

Though in case of octopus merge, where one would like to drop a 
branch from the middle, being merged with `-s ours`, that would be 
impossible, as then the next branch would be taking over dropped 
branch merge parent, yielding an incorrect result.

So in this case:

(3) ---o---o---o---M--- (master)
   |\ /|
   | X1--X3--/ | (topic)
   |   |
   \--X2---/ (new)

... where "topic" was merged using `-s ours`, we wouldn`t be able to 
just remove whole "topic" branch from the rebased merge without 
influencing it incorrectly.

With (any kind of) explicit old-parent:new-parent mapping, this is 
possible (and shouldn`t be any harder, implementation wise).

Now, it`s a different story if we`re interested in such exotic 
scenarios in the first place, but if possible, I would be all for 
it... :)

> Currently my favorite idea is to introduce a new flag: -R (for "rebase the
> original merge commit"). It would look like this:
> 
>   merge -R -C   # 
> 
> This flag would of course trigger the consistency check (does the number
> of parents of the original merge commit agree with the parameter list? Was
> an original merge commit specified to begin with?), and it would not fall
> back to the recursive merge, but error out if that check failed.
> 
> Side note: I wonder whether we really need to perform the additional check
> that ensures that the  refers to the rewritten version of the
> original merge commit's parent.

No, and even worse - I think we must not do that, as that merge 
parent might be moved elsewhere, which should be 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-06 Thread Igor Djordjevic
Hi Johannes,

On 05/03/2018 18:29, Johannes Schindelin wrote:
> 
> > By the way, is there documentation for `git merge-recursive` 
> > anywhere, besides the code itself...? :$
> 
> I am not aware of any. The commit message adding the command is not very
> illuminating (https://github.com/git-for-windows/git/commit/720d150c4):
> 
> Add a new merge strategy by Fredrik Kuivinen.
> 
> I really wanted to try this out, instead of asking for an adjustment
> to the 'git merge' driver and waiting.  For now the new strategy is
> called 'fredrik' and not in the list of default strategies to be tried.
> 
> The script wants Python 2.4 so this commit also adjusts Debian and RPM
> build procecure files.
> 
> Digging through https://public-inbox.org/git/ during that time frame comes
> up with this hit, though:
> 
> https://public-inbox.org/git/20050907164734.ga20...@c165.ib.student.liu.se/
> 
> which is still not a good documentation of the algorithm. You can probably
> dig further yourself, but I think I can describe it very quickly here:
> 
> To merge two commits recursively, you first have to find their "merge
> bases". If there was an obvious branch point, then that is the merge base.
> But when you start a branch off of master, then work a bit, then merge
> master, you already have two merge bases.
> 
> The trick about the recursive merge is to reduce the number of merge bases
> iteratively to one. It does that by taking two merge bases, and performing
> a recursive merge on them, which generates a "virtual" commit, the
> condensed merge base. That one is then merged recursively with the next
> merge base, until there is only one left.
> 
> A recursive merge of two commits with exactly one merge base is simply a
> three-way merge.
> 
> I vaguely remember that there was something funny about the order in which
> order you want to process the merge bases: if you did it in one
> (chronological) direction, it worked beautifully, in the other direction
> it would generate tons of merge conflicts or something like that.

Thanks, this is very informative (together with the linked discussion).

Not remembering seeing this before, I wasn`t really sure if this was 
some undocumented core Git (plumbing?) utility (used withing Git 
itself, too), or just a leftover (yet still useful) sample tool.

Regards, Buga


Re: [PATCH v2 1/3] add -p: select individual hunk lines

2018-03-06 Thread Igor Djordjevic
On 06/03/2018 21:29, Igor Djordjevic wrote:
> 
> > diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> > index f83e7450ad..a273b41e95 100755
> > --- a/git-add--interactive.perl
> > +++ b/git-add--interactive.perl
> > 
> > [...]
> > 
> > @@ -1255,6 +1382,7 @@ j - leave this hunk undecided, see next undecided hunk
> >  J - leave this hunk undecided, see next hunk
> >  k - leave this hunk undecided, see previous undecided hunk
> >  K - leave this hunk undecided, see previous hunk
> > +l - select hunk lines to use
> 
> s/select hunk lines to use/stage hunk lines/

I was wrong here - in the context of Junio`s remark, I now think this 
might even belong to context-aware "help_patch_modes" instead, 
phrased accordingly in there (stage/stash/unstage... etc.).


Re: [PATCH v2 0/3] add -p: select individual hunk lines

2018-03-06 Thread Igor Djordjevic
Hi Junio,

On 06/03/2018 22:03, Junio C Hamano wrote:
> 
> > A small nitpick - I see you use phrasing like "select lines", where 
> > the other commands usually talk about "staging", instead, so "stage 
> > lines" might be more aligned with the existing text.
> 
> Isn't this machinery shared across "add -p" and "reset -p"?  What is
> done to the selected lines when you are using this UI while running
> "reset -p"?  I hope it is not "staging".  If the interface only
> "selects lines" and what is done to the selected lines depends on
> what operation is using this backend, then the current phrasing is
> perfectly fine and saying "staging" makes it actively worse.

Hmm, if that is the case, I agree, but I was merely trying to review 
the files being changed - for example, inside "Documentation/git-add.txt":

   y - stage this hunk
   n - do not stage this hunk
   q - quit; do not stage this hunk or any of the remaining ones
   a - stage this hunk and all later hunks in the file
   d - do not stage this hunk or any of the later hunks in the file
   g - select a hunk to go to
   / - search for a hunk matching the given regex
   j - leave this hunk undecided, see next undecided hunk
   J - leave this hunk undecided, see next hunk
   k - leave this hunk undecided, see previous undecided hunk
   K - leave this hunk undecided, see previous hunk
   s - split the current hunk into smaller hunks
   e - manually edit the current hunk
   ? - print help


In there, adding "l" should follow "stage" phrasing, I would think.

But you are right for "git-add--interactive.perl", for example - in 
there, I didn`t notice the line (seems to be?) added inside the shared 
"help_patch_cmd".

But if so, I guess it should then be moved to more context-related 
"help_patch_modes", being phrased accordingly in there.

Thanks for pointing this out, let me recheck my comments.

Regards, Buga


Re: [PATCH v2 2/3] add -p: allow line selection to be inverted

2018-03-06 Thread Igor Djordjevic
On 06/03/2018 11:17, Phillip Wood wrote:
> From: Phillip Wood 
> 
> If the list of lines to be selected begins with '^' select all the
> lines except the ones listed.

s/to be selected begins with '^' select all/to be staged begins with '^' stage 
all/

> 
> Signed-off-by: Phillip Wood 
> ---
>  Documentation/git-add.txt  |  3 ++-
>  git-add--interactive.perl  | 17 -
>  t/t3701-add-interactive.sh |  2 +-
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index ad33fda9a2..0e2c11e97b 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -341,7 +341,8 @@ If you press "l" then the hunk will be reprinted with 
> each insertion
>  or deletion labelled with a number and you will be prompted to enter
>  which lines you wish to select. Individual line numbers should be
>  separated by a space or comma, to specify a range of lines use a dash
> -between them.
> +between them. To invert the selection prefix it with "\^" so "^3-5,8"
> +will select everything except lines 3, 4, 5 and 8.

Hmm, here, first "selection" seems to make sense as it is (I guess),
but might still be better to later say 
s/will select everything/will stage everything/ ...?

That said, might be "to invert the selection" could rather be "to unstage," 
instead? Not sure, though.

>  +
>  After deciding the fate for all hunks, if there is any hunk
>  that was chosen, the index is updated with the selected hunks.
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index a273b41e95..6fa3d0a87c 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1085,9 +1085,21 @@ sub check_hunk_label {
>  sub parse_hunk_selection {
>   local $_;
>   my ($hunk, $line) = @_;
> - my $max_label = $hunk->{MAX_LABEL};
> + my ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);
>   my @selected = (0) x ($max_label + 1);
>   my @fields = split(/[,\s]+/, $line);
> + if ($fields[0] =~ /^\^(.*)/) {
> + $invert = 1;
> + if ($1 ne '') {
> + $fields[0] = $1;
> + } else {
> + shift @fields;
> + unless (@fields) {
> + error_msg __("no lines to invert\n");
> + return undef;
> + }
> + }
> + }
>   for (@fields) {
>   if (/^([0-9]*)-([0-9]*)$/) {
>   if ($1 eq '' and $2 eq '') {
> @@ -1110,6 +1122,9 @@ sub parse_hunk_selection {
>   return undef;
>   }
>   }
> + if ($invert) {
> + @selected = map { !$_ } @selected;
> + }
>   return \@selected;
>  }
>  
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 65c8c3354b..89c0e73f2b 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '
>  '
>  
>  test_expect_success 'can reset individual lines of patch' '
> - printf "%s\n" l 2 |
> + printf "%s\n" l "^1 3" |
>   EDITOR=: git reset -p 2>error &&
>   test_must_be_empty error &&
>   git diff --cached HEAD >actual &&
> 


Re: [PATCH v2 3/3] add -p: optimize line selection for short hunks

2018-03-06 Thread Igor Djordjevic
On 06/03/2018 11:17, Phillip Wood wrote:
> From: Phillip Wood 
> 
> If there are fewer than ten changes in a hunk then make spaces
> optional when selecting individual lines. This means that for short

Not sure if using s/selecting individual lines/staging individual lines/ 
would make sense here, too, but not that important (as you later do 
say "to stage lines").

> hunks one can just type -357 to stage lines 1, 2, 3, 5 & 7.
> 
> Signed-off-by: Phillip Wood 
> ---
>  Documentation/git-add.txt  |  3 ++-
>  git-add--interactive.perl  | 30 ++
>  t/t3701-add-interactive.sh |  2 +-
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index 0e2c11e97b..d52acfc722 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -340,7 +340,8 @@ patch::
>  If you press "l" then the hunk will be reprinted with each insertion
>  or deletion labelled with a number and you will be prompted to enter
>  which lines you wish to select. Individual line numbers should be
> -separated by a space or comma, to specify a range of lines use a dash
> +separated by a space or comma (these can be omitted if there are fewer
> +than ten labelled lines), to specify a range of lines use a dash
>  between them. To invert the selection prefix it with "\^" so "^3-5,8"
>  will select everything except lines 3, 4, 5 and 8.
>  +
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 6fa3d0a87c..9a6bcd5085 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1082,6 +1082,33 @@ sub check_hunk_label {
>   return 1;
>  }
>  
> +sub split_hunk_selection {
> + local $_;
> + my @fields = @_;
> + my @ret;
> + for (@fields) {
> + if (/^(-[0-9])(.*)/) {
> + push @ret, $1;
> + $_ = $2;
> + }
> + while ($_ ne '') {
> + if (/^[0-9]-$/) {
> + push @ret, $_;
> + last;
> + } elsif (/^([0-9](?:-[0-9])?)(.*)/) {
> + push @ret, $1;
> + $_ = $2;
> + } else {
> + error_msg sprintf
> + __("invalid hunk line '%s'\n"),
> + substr($_, 0, 1);
> + return ();
> + }
> + }
> + }
> + return @ret;
> +}
> +
>  sub parse_hunk_selection {
>   local $_;
>   my ($hunk, $line) = @_;
> @@ -1100,6 +1127,9 @@ sub parse_hunk_selection {
>   }
>   }
>   }
> + if ($max_label < 10) {
> + @fields = split_hunk_selection(@fields) or return undef;
> + }
>   for (@fields) {
>   if (/^([0-9]*)-([0-9]*)$/) {
>   if ($1 eq '' and $2 eq '') {
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 89c0e73f2b..d3bce154da 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '
>  '
>  
>  test_expect_success 'can reset individual lines of patch' '
> - printf "%s\n" l "^1 3" |
> + printf "%s\n" l ^13 |
>   EDITOR=: git reset -p 2>error &&
>   test_must_be_empty error &&
>   git diff --cached HEAD >actual &&
> 


Re: [PATCH v2 1/3] add -p: select individual hunk lines

2018-03-06 Thread Igor Djordjevic
On 06/03/2018 11:17, Phillip Wood wrote:
> From: Phillip Wood 
> 
> When I end up editing hunks it is almost always because I want to
> stage a subset of the lines in the hunk. Doing this by editing the
> hunk is inconvenient and error prone (especially so if the patch is
> going to be reversed before being applied). Instead offer an option
> for add -p to stage individual lines. When the user presses 'l' the
> hunk is redrawn with labels by the insertions and deletions and they
> are prompted to enter a list of the lines they wish to stage. Ranges
> of lines may be specified using 'a-b' where either 'a' or 'b' may be
> omitted to mean all lines from 'a' to the end of the hunk or all lines
> from 1 upto and including 'b'.
> 
> Signed-off-by: Phillip Wood 
> ---
>  Documentation/git-add.txt  |   7 +++
>  git-add--interactive.perl  | 143 
> +
>  t/t3701-add-interactive.sh |  65 +
>  3 files changed, 215 insertions(+)
> 
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index d50fa339dc..ad33fda9a2 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -332,10 +332,17 @@ patch::
> J - leave this hunk undecided, see next hunk
> k - leave this hunk undecided, see previous undecided hunk
> K - leave this hunk undecided, see previous hunk
> +   l - select hunk lines to use

Might be more surrounding context aligned to say "stage hunk lines" 
here (phrase "stage", instead of "select to use").

> s - split the current hunk into smaller hunks
> e - manually edit the current hunk
> ? - print help
>  +
> +If you press "l" then the hunk will be reprinted with each insertion
> +or deletion labelled with a number and you will be prompted to enter
> +which lines you wish to select. Individual line numbers should be

Likewise, s/you wish to select/you wish to stage/.

> +separated by a space or comma, to specify a range of lines use a dash
> +between them.
> ++
>  After deciding the fate for all hunks, if there is any hunk
>  that was chosen, the index is updated with the selected hunks.
>  +
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index f83e7450ad..a273b41e95 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1013,6 +1013,133 @@ sub color_diff {
>   } @_;
>  }
>  
> +sub label_hunk_lines {
> + local $_;
> + my $hunk = shift;
> + my $i = 0;
> + my $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];
> + if ($i > 1) {
> + @{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);
> + return 1;
> + }
> + return 0;
> +}
> +
> +sub select_hunk_lines {

This is just something I`ve spotted, but I have no actual idea if 
renaming this to "stage_hunk_lines" might be better, too, or not 
(depending on the surrounding code context), so please take this with 
a big grain of salt.

> + my ($hunk, $selected) = @_;
> + my ($text, $labels) = @{$hunk}{qw(TEXT LABELS)};
> + my ($i, $o_cnt, $n_cnt) = (0, 0, 0);
> + my ($push_eol, @newtext);
> + # Lines with this mode will become context lines if they are
> + # not selected
> + my $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';
> + for $i (1..$#{$text}) {
> + my $mode = substr($text->[$i], 0, 1);
> + if ($mode eq '\\') {
> + push @newtext, $text->[$i] if ($push_eol);
> + undef $push_eol;
> + } elsif ($labels->[$i] and $selected->[$labels->[$i]]) {
> + push @newtext, $text->[$i];
> + if ($mode eq '+') {
> + $n_cnt++;
> + } else {
> + $o_cnt++;
> + }
> + $push_eol = 1;
> + } elsif ($mode eq ' ' or $mode eq $context_mode) {
> + push @newtext, ' ' . substr($text->[$i], 1);
> + $o_cnt++; $n_cnt++;
> + $push_eol = 1;
> + } else {
> + undef $push_eol;
> + }
> + }
> + my ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =
> + parse_hunk_header($text->[0]);
> + unshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
> + my $newhunk = {
> + TEXT => \@newtext,
> + DISPLAY => [ color_diff(@newtext) ],
> + OFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,
> + TYPE => $hunk->{TYPE},
> + USE => 1,
> + };
> + # If this hunk has previously been edited add the offset delta
> + # of the old hunk to get the real delta from the original
> + # hunk.
> + if ($hunk->{OFS_DELTA}) {
> + $newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};
> + }
> + 

Re: [PATCH v2 0/3] add -p: select individual hunk lines

2018-03-06 Thread Igor Djordjevic
Hi Phillip,

On 06/03/2018 11:17, Phillip Wood wrote:
> 
> From: Phillip Wood 
> 
> I've added some documentation to git-add.txt for the new selection
> mode and cleaned up some style issues, otherwise these are unchanged
> since v1.  These patches build on top of the recount fixes in [1]. The
> commit message for the first patch describes the motivation:
> 
> "When I end up editing hunks it is almost always because I want to
> stage a subset of the lines in the hunk. Doing this by editing the
> hunk is inconvenient and error prone (especially so if the patch is
> going to be reversed before being applied). Instead offer an option
> for add -p to stage individual lines. When the user presses 'l' the
> hunk is redrawn with labels by the insertions and deletions and they
> are prompted to enter a list of the lines they wish to stage. Ranges
> of lines may be specified using 'a-b' where either 'a' or 'b' may be
> omitted to mean all lines from 'a' to the end of the hunk or all lines
> from 1 upto and including 'b'."
> 
> [1] 
> https://public-inbox.org/git/xmqqbmg29x1n@gitster-ct.c.googlers.com/T/#m01d0f1af90f32b698e583b56f8e53b986bcec7c6

Nice, thank you :)

A small nitpick - I see you use phrasing like "select lines", where 
the other commands usually talk about "staging", instead, so "stage 
lines" might be more aligned with the existing text.

I`ll quickly go through the patches regarding this (not being of much 
help for the code itself at the moment, sorry!).

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-06 Thread Igor Djordjevic
On 06/03/2018 19:12, Johannes Schindelin wrote:
> 
> > > And I guess being consistent is pretty important, too - if you add new
> > > content during merge rebase, it should always show up in the merge,
> > > period. 
> >
> > Yes, that should make it easy for the user to know what to expect from
> > rebase.
> 
> Indeed. We have seen time and time again that consistent behavior is the
> only thing that lets us adhere to the Law of Least Surprise.
> 
> And here lies the rub: do we really want to let `merge -C ` behave
> completely differently than `merge`? Granted, in one case we provide a
> template merge commit, in the other case, we do not. And the idea is
> already to behave differently, although that difference only extends to
> the commit message so far.
> 
> But given the benefit (i.e. that the strategy to transform the original
> merge commit into the new merge commit), I am willing to run that risk,
> especially since I foresee only few users wanting to create new merge
> commits from scratch using the `merge` todo command.
> 
> Of course, even then we need to be careful: the user might have
> *changed* or *moved* the original `merge` command. For example, if the
> merge command read:
> 
>   merge -C deadbee cafecafe bedbedbed
> 
> and the user switched the order of the merged branches into
> 
>   merge -C deadbee bedbedbed cafecafe
> 
> we would have to detect the changed order of the arguments so that we
> could still find the original branch tips.
> 
> But the user might also have changed the branch(es) to merge completely,
> in which case we might not even be able to find original branch tips.
> 
> My preferred solution would be to let the `merge` command figure out
> whether the passed arguments correspond to the rewritten versions of the
> original merge parents. And only in that case would we use the fancy
> strategy, in all other cases we would fall back to performing a regular
> recursive (or octopus) merge.
> 
> How does that sound?
> 
> It will be slightly inconsistent. But in a defendable way, I think.

I like where this discussion is heading, and here`s what I thought 
about it :)

First, starting from non-interactive rebase, I guess we may now agree 
that _rebasing_ merges is an actually expected behavior, not recreating 
them (thus keeping manual conflict resolutions and amendments, not 
losing them).

Now, interactive rebase is a totally different story, we already said 
user can change pretty much about everything, making merge 
_recreation_ to be a more sane choice, but let`s leave this other 
extreme for a brief moment.

In the least interesting situation, though, user could just review 
and close todo list, without changing anything - and in that case it 
would be important, consistency wise, to behave exactly like in case 
of non-interactive rebase, meaning still rebasing merges, not 
recreating them.

Ok, so that still aligns with what`s written so far - we need to be 
able to rebase merges interactively, too (not just recreate them), to 
stay consistent in less complex interactive rebases.

But, what if user really wants to _recreate_ merges, for whatever 
reason? Come on, this is interactive rebase we`re talking about, why 
being restrictive? :)

Here`s a twist - not letting `merge` trying to be too smart by 
figuring out whether passed arguments correspond to rewritten 
versions of the original merge parents (which would be too 
restrictive, too, I`m afraid), but just be explicit about it, instead!

So, it could be something like:

merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed


The format is still something to think about, but the point is rather 
simple - explicitly map old and new merge parents, showing this 
inside todo list by default.

This makes it much easier for later processing (and correct, no need 
to guess which one goes where), but also gives more power to the 
user, being able to decide which merge parents get "rebased", and 
which ones should go into the merge just like "new".

So if a user gets an interactive todo list like that and just closes 
it, we still have exact situation like non-interactive rebase (and no 
guessing on implementation side).

But, user might still decide to introduce new merge parents into the 
mix, even, where we could then just be merging those (as there is no 
old merge parent to actually rebase from):

merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed new-branch

Here, "new-branch" is something new, introduced inside interactive 
rebase, and it will be just merged into the other two (which are 
still being rebased).

Also, another example - if original merge parent "123abc" was merged 
from the other side using `-s ours` strategy, that means all the 
content this branch originally had will still be missing from the 
rebased merge (expect for what`s been cherry-picked elsewhere).

But, I would argue it`s quite legit to want to revise that decision, 
and let that content in this time. To make that 

Re: [PATCH v5 00/12] rebase -i: offer to recreate merge commits

2018-03-05 Thread Igor Djordjevic
Hi Johannes,

On 26/02/2018 22:29, Johannes Schindelin wrote:
> 
> Once upon a time, I dreamt of an interactive rebase that would not
> flatten branch structure, but instead recreate the commit topology
> faithfully.
> 
> My original attempt was --preserve-merges, but that design was so
> limited that I did not even enable it in interactive mode.
> 
> Subsequently, it *was* enabled in interactive mode, with the predictable
> consequences: as the --preserve-merges design does not allow for
> specifying the parents of merge commits explicitly, all the new commits'
> parents are defined *implicitly* by the previous commit history, and
> hence it is *not possible to even reorder commits*.
> 
> This design flaw cannot be fixed. Not without a complete re-design, at
> least. This patch series offers such a re-design.
> 
> Think of --recreate-merges as "--preserve-merges done right".

First of all, thanks for this wonderful improvement to existing `git 
rebase` functionality, I`m really excited to have this in the mainline! :)

But in the light of "--preserve-merges done right", I would like to 
hear your opinion on a topic that might be considered more or less 
important, and thus tackled in a few different ways... :$

Rebasing amended merges :( Even though documentation is quite clear 
about merge conflicts and manual amendments not recreated 
automatically, this might be considered quite an issue (a bug, even), 
as even in case of non-interactive rebase, amended content will be 
dropped - and even worse, it all happens silently, without alerting 
the user (for whom we presume to know what he`s doing, I guess).

Now, might be this is considered the least interesting use case, in 
comparison to all the power of more serious interactive rebases, but 
I would argue it could be the one most used by less adventurous users 
that would simply like to stay up-to-date with upstream, rebasing their 
current work on top of it (think `git pull --rebase=recreate`, even).

As it currently is, and that was the case with `--preserve-merges`, 
too, this will cause them to silently lose their work (amended merge 
content). And while documentation is clear about it, these might be 
less knowledgeable users, too, and thus potentially be the ones we 
should (try to) protect even more, if possible.

Now, in the light of that other, ongoing "merge rebasing" topic[1], 
it seems we really might be able to do much better, actually 
_rebasing_ merges (and keeping manual conflict resolutions/amendments), 
instead of _recreating_ them (and silently loosing content), and doing 
so reliably (or stopping for possible user inspection, but not silently 
doing the wrong thing, even if documented).

This concerns non-interactive rebase the most, but I have ideas on 
making it aligned with interactive one, too, where user could 
actually decide whether to rebase or (re)create the merge (rebase 
becoming the default, intuitively aligned with non-interactive rebase).

But before elaborating, I would like to hear your opinion on whether 
you find it worth to pursue that goal here, before `--recreate-merges` 
hits the mainstream, or it might be just fine as a possible later
improvement, too (if accepted, that is).

My main concern, and why I raised the question inside this topic in 
the first place, is default behavior. With `--recreate-merges` just 
being introduced, we have no backwards compatibility to think about, 
being a unique chance to make default behave as needed (not to say 
"correct", even), and might be really ticking one more of 
"--preserve-merges done right" boxes, and could be a pretty important 
one, too.

But once this becomes widely available, I guess it will be hard to 
improve (fix?) this merge rebasing silent content losing behavior 
(even if we would acknowledge it as a bug), without introducing 
additional options - and without a possibility to make possibly 
"right" behavior a default one, thus further complicating user 
experience.

So, I wanted to hear your stance on this :(

Knowing how much this means to you, it is really not my wish to drag 
this topic further, and if you find it that we`re good here as it is, 
I wouldn`t have any objections - I guess later new `--rebase-merges` 
option is a possibility, too, might be a wrapper around 
`--recreate-merges`, but with actual merge rebasing being a default 
(where merge recreation would still be possible, too)...

Otherwise, if you have any interest in it now, I can further elaborate 
what I`m thinking about, where it might help improve both user 
experience and rebase possibilities, for what might not be too much 
extra work... hopefully :P

Whatever ends up being your response, I`m really grateful for your 
work on this matter so far, and thank you for everything you did.

p.s. lol, now that I said it, and after writing all this, I might 
actually even like the idea of (later) having `--rebase-merges` 
alongside `--recreate-merges`, too, each one clearly communicating 
its 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-03 Thread Igor Djordjevic
On 02/03/2018 19:14, Igor Djordjevic wrote:
> 
> > > It is interesting to think what it means to faithfully rebase a '-s
> > > ours' merge. In your example the rebase does not introduce any new
> > > changes into branch B that it doesn't introduce to branch A. Had it
> > > added a fixup to branch B1 for example or if the topology was more
> > > complex so that B ended up with some other changes that the rebase did
> > > not introduce into A, then M' would contain those extra changes whereas
> > > '--recreate-merges' with '-s ours' (once it supports it) would not.
> >
> > Unless the method of merging was stored, I don't think we *can*
> > correctly automate resolving of "-s ours" because all we store is the
> > resulting content, and we don't know how or why the user generated it
> > as such. I believe the "correct" solution in any case would be to take
> > the content we DO know and then ask the user to stop for amendments.
>  
> I agree with Jake, and for the exact same reasons.
> 
> That said, I`d like to see what mentioned '--recreate-merges' with 
> '-s ours' does (or would do, once it supports it), would you have a 
> pointer for me where to look at?
> 
> But if that`s something yet to come, might be it`s still open for 
> discussion. I mean, even this topic started inside original 
> `--recreate-merges` one[1], and hopefully it can still bring 
> improvements there (sooner or later).
> 
> [1] 
> https://public-inbox.org/git/cover.1516225925.git.johannes.schinde...@gmx.de/

Ok, I went through mentioned `--recreate-merges` topic again, and I 
think I see one crucial merge commit handling difference.

In there, as it is at the moment, merge commits are really to be 
_recreated_... as the option name seems to imply ;)

In terms of interactive rebasing, it actually comes from "todo list" 
becoming much more powerful, gaining ability to create (new) merges, 
which is wonderful from the aspect of history rewriting (what rebase 
is all about).

But, I would argue it is a different concept from actually _rebasing_ 
existing merge commits, being what we`re discussing about here.

Yes, you can use merge commit (re)creation to "rebase" existing merge 
commit so the end result is the same, being what `--recreate-merges` 
now does, but that only goes for some (uninteresting?) merge commits 
where not knowing the deeper context of how the merge commit is 
originally created is not important as no content is to be lost (due 
to missing that deeper and utterly unknown context).

But as soon as you try to do that with more complex merge commits, 
like holding manual amendments and conflict resolutions, it doesn`t 
really work as expected - which I demonstrated in my original example 
script[1] in this topic, original merge commit amendment lost on 
rebase, and even worse - that happened silently.

Now, not to get misinterpreted to pick sides in "(re)create" vs 
"rebase" merge commit discussion, I just think these two (should) have 
a different purpose, and actually having both inside interactive rebase 
is what we should be aiming for.

And that`s what I think is important to understand before any further 
discussion - _(re)creating_ existing merge commits is not the same as 
_rebasing_ them, even though the former can sometimes be used to 
achieve the latter.

Regards, Buga

[1] https://public-inbox.org/git/bbe64321-4d3a-d3fe-8bb9-58b600fab...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-02 Thread Igor Djordjevic
Hi Phillip,

On 02/03/2018 12:31, Phillip Wood wrote:
> 
> > Thinking about it overnight, I now suspect that original proposal had a
> > mistake in the final merge step. I think that what you did is a way to
> > fix it, and I want to try to figure what exactly was wrong in the
> > original proposal and to find simpler way of doing it right.
> >
> > The likely solution is to use original UM as a merge-base for final
> > 3-way merge of U1' and U2', but I'm not sure yet. Sounds pretty natural
> > though, as that's exactly UM from which both U1' and U2' have diverged
> > due to rebasing and other history editing.
> 
> Hi Sergey, I've been following this discussion from the sidelines,
> though I haven't had time to study all the posts in this thread in
> detail. I wonder if it would be helpful to think of rebasing a merge as
> merging the changes in the parents due to the rebase back into the
> original merge. So for a merge M with parents A B C that are rebased to
> A' B' C' the rebased merge M' would be constructed by (ignoring shell
> quoting issues)
> 
> git checkout --detach M
> git merge-recursive A -- M A'
> tree=$(git write-tree)
> git merge-recursive B -- $tree B'
> tree=$(git write-tree)
> git merge-recursive C -- $tree C'
> tree=$(git write-tree)
> M'=$(git log --pretty=%B -1 M | git commit-tree -pA' -pB' -pC')
> 
> This should pull in all the changes from the parents while preserving
> any evil conflict resolution in the original merge. It superficially
> reminds me of incremental merging [1] but it's so long since I looked at
> that I'm not sure if there are any significant similarities.
> 
> [1] https://github.com/mhagger/git-imerge

Interesting, from quick test[3], this seems to produce the same 
result as that other test I previously provided[2], where temporary 
commits U1' and U2' are finally merged with original M as a base :)

Just that this looks like even more straight-forward approach...?

The only thing I wonder of here is how would we check if the 
"rebased" merge M' was "clean", or should we stop for user amendment? 
With that other approach Sergey described, we have U1'==U2' to test with.

By the way, is there documentation for `git merge-recursive` 
anywhere, besides the code itself...? :$

Thanks, Buga

[2] https://public-inbox.org/git/f1a960dc-cc5c-e7b0-10b6-39e551665...@gmail.com/
[3] Quick test script:
-- >8 --
#!/bin/sh

# rm -rf ./.git
# rm -f ./test.txt

git init

touch ./test.txt
git add -- test.txt

# prepare repository
for i in {1..8}
do
echo X$i >>test.txt
git commit -am "X$i"
done

# prepare branch A
git checkout -b A
sed -i '2iA1' test.txt
git commit -am "A1"
sed -i '4iA2' test.txt
git commit -am "A2"
sed -i '6iA3' test.txt
git commit -am "A3"

# prepare branch B
git checkout -b B master
sed -i '5iB1' test.txt
git commit -am "B1"
sed -i '7iB2' test.txt
git commit -am "B2"
sed -i '9iB3' test.txt
git commit -am "B3"

git checkout -b topic A
git merge -s ours --no-commit B # merge A and B with `-s ours`
sed -i '8iM' test.txt   # amend merge commit ("evil merge")
git commit -am "M"
git tag M #original-merge

# master moves on...
git checkout master
git cherry-pick B^ # cherry-pick B2 into master
sed -i "1iX9" test.txt # add X9
git commit -am "X9"

# (0) ---X8--B2'--X9 (master)
#|\
#| A1---A2---A3 (A)
#| \
#|  M (topic)
#| /
#\-B1---B2---B3 (B)

# simple/naive demonstration of proposed merge rebasing logic
# using iterative merge-recursive, preserving merge commit manual
# amendments, testing `-s ours` merge with cherry-picking from
# obsoleted part, but still respecting interactively rebased
# added/modified/dropped/cherry-picked commits :)

git checkout -b A-prime A
git reset --hard HEAD^ # drop A3 from A
sed -i '/A1/c\A12' test.txt# amend A1 to A12
git commit -a --amend --no-edit
git rebase master  # rebase A onto master
git cherry-pick B  # cherry-pick B3 into A

git checkout -b B-prime B
git rebase master  # rebase B onto master
sed -i '12iB4' test.txt# add B4
git commit -am "B4"

git checkout --detach M
git merge-recursive A -- M A-prime
tree="$(git write-tree)"
git merge-recursive B -- $tree B-prime
tree="$(git write-tree)"
git tag M-prime "$(git log --pretty=%B -1 M | git commit-tree $tree -p A-prime 
-p B-prime)"

git update-ref refs/heads/topic "$(git rev-parse M-prime)"
git checkout topic

# (1) ---X8--B2'--X9 (master)
# |\
# | A12--A2'---B3' (A)
# | \
# |  M' (topic)
# | /
# \-B1'--B3'---B4  (B)

# show resulting graph
# echo
# git log --all --decorate --oneline --graph

# in comparison to original merge commit M, rebased merge commit 
# M' is expected to:
#
# - Add X9, from updated "master"
# - Have A1 changed to A12, due to A12 commit 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-02 Thread Igor Djordjevic
Hi Phillip,

> On Fri, Mar 2, 2018 at 4:36 AM, Phillip Wood wrote:
> > 
> > > It is interesting to think what it means to faithfully rebase a '-s
> > > ours' merge.
> > 
> > I should have explained that I mean is a faithful rebase one that
> > adheres to the semantics of '-s ours' (i.e. ignores any changes in the
> > side branch) or one that merges new changes from the side branch into
> > the content of the original merge? In your example you add B4 to B. If
> > M' preserves the semantics of '-s ours' then it will not contain the
> > changes in B4. I think your result does (correct me if I'm wrong) so it
> > is preserving the content of the original merge rather than the
> > semantics of it.

Yeah, I understood what you mean, and I see you noticed that B4 
commit, for which I did anticipate possibly bringing up a discussion 
like this ;)

I agree with Jake here, my thoughts exactly (what I wrote in that 
other subthread[1], too):

On 02/03/2018 17:02, Jacob Keller wrote:
> 
> We only have the content, and we don't know the semantics (nor, I
> think, should we attempt to understand or figure out the semantics).

Hmm, I wanted to elaborate a bit here, but that sentence seems to 
summarize the pure essence of it, and whatever I write looks like 
just repeating the same stuff again...

That`s just it. And stopping to give the user a chance to 
review/amend the result, where he might decide he actually did want 
something else - so all good.

Otherwise, I would be interested to learn how context/semantics 
guessing could provide a better default action (without introducing 
more complexity for might not be much benefit, if any).

But in the end, I guess we can just discuss the "most sane default" 
to present to the user (introduce or obsolete that new commit B4, in 
the discussed example[2]), as we should definitely stop for amending 
anyway, not proceeding automatically whenever U1' != U2'.

Oh, and what about situations where we introduce new or drop existing 
branches (which should be possible with new `--recreate-merges`)...? 
"Preserving old branch semantics" may have even less sense here - the 
(possibly heavily reorganized) content is the only thing we have, 
where context will (and should) be provided by the user.

And I guess being consistent is pretty important, too - if you add 
new content during merge rebase, it should always show up in the 
merge, period. It seems pretty confusing to find out one of the 
branches "declared special" (even more if it`s based on uncertain 
guess-work), so when you add something to it it`s just "swallowed", 
as the whole branch is always obsoleted, for now and ever.

I might even see a value in such behavior, but only as a conscious 
user action, not something done automatically... I guess? :)

Regards, Buga

[1] https://public-inbox.org/git/f26cdbe2-1bc3-02ff-7b99-12a6ebab5...@gmail.com/
[2] https://public-inbox.org/git/f1a960dc-cc5c-e7b0-10b6-39e551665...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

2018-03-02 Thread Igor Djordjevic
Hi Phillip,

On 02/03/2018 17:00, Jacob Keller wrote:
> 
> > It is interesting to think what it means to faithfully rebase a '-s
> > ours' merge. In your example the rebase does not introduce any new
> > changes into branch B that it doesn't introduce to branch A. Had it
> > added a fixup to branch B1 for example or if the topology was more
> > complex so that B ended up with some other changes that the rebase did
> > not introduce into A, then M' would contain those extra changes whereas
> > '--recreate-merges' with '-s ours' (once it supports it) would not.
> 
> Unless the method of merging was stored, I don't think we *can*
> correctly automate resolving of "-s ours" because all we store is the
> resulting content, and we don't know how or why the user generated it
> as such. I believe the "correct" solution in any case would be to take
> the content we DO know and then ask the user to stop for amendments.
 
I agree with Jake, and for the exact same reasons.

That said, I`d like to see what mentioned '--recreate-merges' with 
'-s ours' does (or would do, once it supports it), would you have a 
pointer for me where to look at?

But if that`s something yet to come, might be it`s still open for 
discussion. I mean, even this topic started inside original 
`--recreate-merges` one[1], and hopefully it can still bring 
improvements there (sooner or later).

Thanks, Buga

[1] 
https://public-inbox.org/git/cover.1516225925.git.johannes.schinde...@gmx.de/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-02 Thread Igor Djordjevic
Hi Sergey,

On 02/03/2018 06:40, Sergey Organov wrote:
> 
> > So... In comparison to original merge commit M, rebased merge commit 
> > M' is expected to:
> > 
> >  - Add X9, from updated "master"
> >  - Have A1 changed to A12, due to A12 commit amendment
> >  - Keep A2, rebased as A2'
> >  - Remove A3, due to dropped A3 commit
> >  - Keep amendment from original (evil) merge commit M
> >  - Miss B1' like M does B, due to original `-s ours` merge strategy
> >  - Add B2, cherry-picked as B2' into "master"
> >  - Add B3, cherry-picked as B3' into "A"
> >  - Add B4, added to "B"
> >  - Most important, provide safety mechanism to "fail loud", being 
> >aware of non-trivial things going on, allowing to stop for user 
> >inspection/decision
> > 
> > 
> > There, I hope I didn`t miss any expectation. And, it _seems_ to work 
> > exactly as expected :D
> 
> That's very nice, to the level of being even suspect! :-)

Heh, indeed :) I`m already thinking of some even more complex 
situations. The more use/test cases, the merrier.

Like if number of merge parents (branches) gets changed during 
interactive rebase...

> To avoid falling into euphoria though, we need to keep in mind that
> "expectations" is rather vague concept, and we likely still need to stop
> for user amendment unless we absolutely sure nothing surprising happens.
> I.e., we better require U1'==U2' test to succeed to proceed non-stop
> automatically. Besides, it will be somewhat inline with what 'rerere'
> does.

I totally agree, and I think whatever we come up with, we`ll always 
be missing some deeper context of the original merge, so U1'==U2' 
test is a must - if it fails, even if we didn`t get any conflicts and 
could otherwise proceed automatically, better stop for user sanity check.

I`m still thinking if there could be a situation where test passes, 
but result might still be suspicious (and worth inspecting), though.

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-03-01 Thread Igor Djordjevic
Hi Sergey,

On 01/03/2018 06:39, Sergey Organov wrote:
> 
> > > (3) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> > >
> > 
> > Meh, I hope I`m rushing it now, but for example, if we had decided to 
> > drop commit A2 during an interactive rebase (so losing A2' from 
> > diagram above), wouldn`t U2' still introduce those changes back, once 
> > U1' and U2' are merged, being incorrect/unwanted behavior...? :/
> > 
> > [...]
> 
> Yeah, I now see it myself. I'm sorry for being lazy and not inspecting
> this more carefully in the first place.

No problem, that`s why we`re discussing it, and I`m glad we`re 
aligned now, so we can move forward :)

> > So while your original proposal currently seems like it could be 
> > working nicely for non-interactive rebase (and might be some simpler 
> > interactive ones), now hitting/acknowledging its first real use 
> > limit, my additional quick attempt[1] just tries to aid pretty 
> > interesting case of complicated interactive rebase, too, where we 
> > might be able to do better as well, still using you original proposal 
> > as a base idea :)
> 
> Yes, thank you for pushing me back to reality! :-) The work and thoughts
> you are putting into solving the puzzle are greatly appreciated!

You`re welcome, and I am enjoying it :)

> Thinking about it overnight, I now suspect that original proposal had a
> mistake in the final merge step. I think that what you did is a way to
> fix it, and I want to try to figure what exactly was wrong in the
> original proposal and to find simpler way of doing it right.
> 
> The likely solution is to use original UM as a merge-base for final
> 3-way merge of U1' and U2', but I'm not sure yet. Sounds pretty natural
> though, as that's exactly UM from which both U1' and U2' have diverged
> due to rebasing and other history editing.

Yes, this might be it...! ;)

To prove myself it works, I`ve assembled a pretty crazy `-s ours`  
merge interactive rebase scenario, and it seems this passes the test, 
ticking all the check boxes (I could think of) :P

Let`s see our starting situation:

 (0) ---X8--B2'--X9 (master)
|\
| A1---A2---A3 (A)
| \
|  M (topic)
| /
\-B1---B2---B3 (B)


Here, merge commit M is done with `-s ours` (obsoleting branch "B"), 
plus amended to make it an "evil merge", where a commit B2 from 
obsoleted branch "B" is cherry picked to "master".

Now, we want to rebase "topic" (M) onto updated "master" (X9), but to 
make things more interesting, we`ll do it interactively, with some 
amendments, drops, additions and even more cherry-picks!

This is what the final result looks like:

 (1) ---X8--B2'--X9 (master)
 |\
 | A12--A2'---B3' (A)
 | \
 |  M' (topic)
 | /
 \-B1'--B3'---B4  (B)


During interactive rebase, on branch "A", we amended A1 into A12, 
dropped A3 and cherry-picked B3. On branch "B", B4 is added, B2' being 
omitted automatically as already present in "master".

So... In comparison to original merge commit M, rebased merge commit 
M' is expected to:

 - Add X9, from updated "master"
 - Have A1 changed to A12, due to A12 commit amendment
 - Keep A2, rebased as A2'
 - Remove A3, due to dropped A3 commit
 - Keep amendment from original (evil) merge commit M
 - Miss B1' like M does B, due to original `-s ours` merge strategy
 - Add B2, cherry-picked as B2' into "master"
 - Add B3, cherry-picked as B3' into "A"
 - Add B4, added to "B"
 - Most important, provide safety mechanism to "fail loud", being 
   aware of non-trivial things going on, allowing to stop for user 
   inspection/decision


There, I hope I didn`t miss any expectation. And, it _seems_ to work 
exactly as expected :D

Not to leave this to imagination only, and hopefully helping others 
to get to speed and possibly discuss this, pointing to still possible 
flaws, I`m adding a demo script[1], showing how this exact example 
works.

Note that script _is_ coined to avoid rebase conflicts, as they`re not 
currently important for the point to be made here.

In real life, except for usual possibility for conflicts during 
commit rebasing, we might experience _three_ possible conflict 
situations once "rebased" merge itself is to be created - two when 
rebasing each of temporary merge helper commits, and one on the 
"rebased" merge itself. This is something where we might think about 
user experience, not introducing (too much) confusion...

Regards, Buga

[1] Demonstration script:
-- >8 --
#!/bin/sh

# rm -rf ./.git
# rm -f ./test.txt

git init

touch ./test.txt
git add -- test.txt

# 

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-28 Thread Igor Djordjevic
On 28/02/2018 21:25, Igor Djordjevic wrote:
> 
> But U1' and U2' are really to be expected to stay the same in 
> non-interactive rebase case only...

Just to rephrase to "could be expected" here, meaning still not 
necessarily in this case, either - I`ve just witnessed 
non-interactive rebase Johannes previously described[1], merge with 
`-s ours` (dropping B* patches), plus B1 cherry-picked between 
X1..X2, eventually coming up with different U1' and U2', too (which 
would produce a wrong end result, if continued).

But I guess this should go to the "complex history" pile, good thing 
still being that diff safety check between U1' and U2' works as 
expected, thus automatic merge rebase can be aborted and command 
given back to the user for closer inspection.

[1] 
https://public-inbox.org/git/nycvar.qro.7.76.6.1802272330290...@zvavag-6oxh6da.rhebcr.pbec.zvpebfbsg.pbz/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-28 Thread Igor Djordjevic
Hi Sergey,

On 28/02/2018 07:14, Sergey Organov wrote:
> 
> > > Would additional step as suggested in [1] (using R1 and R2 to "catch" 
> > > interactive rebase additions/amendments/drops, on top of U1' and 
> > > U2'), make more sense (or provide an additional clue, at least)?
> > > 
> > > [1] 
> > > https://public-inbox.org/git/8829c395-fb84-2db0-9288-f7b28fa0d...@gmail.com/
> > 
> > Anyway, from (yet another) ad hoc test, additional step mentioned in 
> > [1] above seems to handle this case, too (merge with `-s ours` 
> > dropping B* patches, plus B1 cherry-picked between X1..X2)
> > 
> > ...
> > 
> > And not just that - it worked with additional interactive rebase 
> > adding, amending and removing commits, on top of all this still 
> > preserving original `-s ours` merge commit evil-merge amendment, too, 
> > all as expected (or so it seems) :P
> 
> Great! I do believe that once we start from some sensible approach, many
> kinds of improvements are possible. I'll definitely need to take close
> look at what you came up with, thanks!
> 
> I'd like to say though that nobody should expect miracles from automated
> rebasing of merges when we get to complex history editing. It will need
> to retreat to manual merge, sooner or later.

I agree, and as I really liked "the feeling" of the original approach 
you described, it felt bad to (presumably) see it failing in what 
doesn`t seem to be neither too complex nor rare situation of dropping 
a commit during an interactive rebase, thus motivation to try to 
improve it, if possible, wasn`t lacking :)
 
Eh, might be I`m just naively ignorant at the moment as well, but I`m 
trying to work my way through it... ;)

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-28 Thread Igor Djordjevic
Hi Sergey,

On 28/02/2018 06:19, Sergey Organov wrote:
> 
> > > (3) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> > >
> >
> > Meh, I hope I`m rushing it now, but for example, if we had decided to 
> > drop commit A2 during an interactive rebase (so losing A2' from 
> > diagram above), wouldn`t U2' still introduce those changes back, once 
> > U1' and U2' are merged, being incorrect/unwanted behavior...? :/
> 
> I think the method will handle this nicely.

That`s what I thought as well. And then I made a test. And then the 
test broke... Now, might be the test was flawed in the first place, 
but thinking about it a bit more, it does seem to make sense not to 
handle this case nicely :/

> When you "drop" A2, will A3' change, or stay intact?

When you "drop" commit A2 from rebasing, patch (diff) A3' does stay 
the same - but resulting tree (snapshot) after applying it does not.

By removing commit A2 from your rebase, you`re saying that changes 
introduced in that commit shouldn`t ever happen in the rebased tree, 
so trees/snapshots of all rebased commits coming after dropped A2 
will have these changes missing, in comparison to trees of original 
commits they`re being rebased from.

> If it changes, say, to A3'', the U1' will change to U1'', and the method
> will propagate the change automatically.
> 
> If it A3' doesn't change, then there are no changes to take.

All true, but note what I wrote in the original message - the issue 
of dropping A2 is not U1, but U2, and that one doesn`t change.

In this case, U1' will correctly represent U1 rebased on top of A3' 
(where A2 is now missing), no problem there.

But on the other end, as U2 holds changes between original merge and 
B3 (being A1, A2, A3 and evil-merge M), it will also still hold 
changes introduced by original A2.

Rebasing it onto B3' brings all these changes along, and once merged 
with U1' to produce "rebased" merge it unexpectedly introduces 
supposedly dropped commit A2 changes in their full glory.

Yes, considering this situation a conflict, as originally proposed, 
by simply noticing that U1' and U2' differ, helps this to fail 
loudly without doing the wrong thing.

But U1' and U2' are really to be expected to stay the same in 
non-interactive rebase case only, where it doesn`t help interactive 
rebase case at all if it is to fail most of the time (where U1' and 
U2' don`t have to be, but should be expected to possibly be different).

So while your original proposal currently seems like it could be 
working nicely for non-interactive rebase (and might be some simpler 
interactive ones), now hitting/acknowledging its first real use 
limit, my additional quick attempt[1] just tries to aid pretty 
interesting case of complicated interactive rebase, too, where we 
might be able to do better as well, still using you original proposal 
as a base idea :)

Regards, Buga

[1] https://public-inbox.org/git/8829c395-fb84-2db0-9288-f7b28fa0d...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-28 Thread Igor Djordjevic
Hi Sergey,

On 28/02/2018 06:44, Sergey Organov wrote:
> 
> > > Here`s our starting position:
> > >
> > > (0) ---X1---o---o---o---o---o---X2 (master)
> > >|\
> > >| A1---A2---A3
> > >| \
> > >|  M (topic)
> > >| /
> > >\-B1---B2---B3
> > >
> > >
> > > Now, we want to rebase merge commit M from X1 onto X2. First, rebase
> > > merge commit parents as usual:
> > >
> > > (1) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3   | A1'--A2'--A3'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3   \-B1'--B2'--B3'
> > >
> > >
> > > That was commonly understandable part.
> >
> > Good. Let's assume that I want to do this interactively (because let's
> > face it, rebase is boring unless we shake up things a little). And let's
> > assume that A1 is my only change to the README, and that I realized that
> > it was incorrect and I do not want the world to see it, so I drop A1'.
> >
> > Let's see how things go from here:
> >
> > > Now, for "rebasing" the merge commit (keeping possible amendments), we
> > > do some extra work. First, we make two temporary commits on top of old
> > > merge parents, by using exact tree (snapshot) of commit M:
> > >
> > > (2) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'
> >
> > Okay, everything would still be the same except that I still have dropped
> > A1'.
> >
> > > So here, in terms of _snapshots_ (trees, not diffs), U1 = U2 = M.
> > >
> > > Now, we rebase these temporary commits, too:
> > >
> > > (3) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> >
> > I still want to drop A1 in this rebase, so A1' is still missing.
> >
> > And now it starts to get interesting.
> >
> > The diff between A3 and U1 does not touch the README, of course, as I said
> > that only A1 changed the README.  But the diff between B3 and U2 does
> > change the README, thanks to M containing A1 change.
> >
> > Therefore, the diff between B3' and U2' will also have this change to the
> > README. That change that I wanted to drop.
> >
> > > As a next step, we merge these temporary commits to produce our
> > > "rebased" merged commit M:
> > >
> > > (4) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > >| \  |  \
> > >|  M |   M'
> > >| /  |  /
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> >
> > And here, thanks to B3'..U2' changing the README, M' will also have that
> > change that I wanted to see dropped.
> >
> > Note that A1' is still dropped in my example.
> >
> > > Finally, we drop temporary commits, and record rebased commits A3' 
> > > and B3' as our "rebased" merge commit parents instead (merge commit 
> > > M' keeps its same tree/snapshot state, just gets parents replaced):
> > >
> > > (5) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'
> > >| \  | \
> > >|  M |  M'
> > >| /  | /
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'
> >
> > Now, thanks to U2' being dropped (and A1' *still* being dropped), the
> > change in the README that is still in M' is really only in M'. No other
> > rebased commit has it. That makes it look as if M' introduced this change
> > in addition to the changes that were merged between the merge parents.
> 
> Except that original proposal suggests to cosider this a conflict and
> to stop for amendment, as U1' and U2' now differ.

Thanks for bringing this one up again - I focused on a mere example 
of how the approach could work, not stressing enough the simplicity 
of recognizing when it really doesn`t, which is an important aspect 
to know as well, indeed, supporting much needed trust that no bad 
things could happen behind our back.

Either work as expected, or fail loudly, yes.

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-28 Thread Igor Djordjevic
Hi Sergey,

On 28/02/2018 06:21, Sergey Organov wrote:
> 
> > > > > (3) ---X1---o---o---o---o---o---X2
> > > > >|\   |\
> > > > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > > > >| \  |
> > > > >|  M |
> > > > >| /  |
> > > > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> > > > >
> > > >
> > > > Meh, I hope I`m rushing it now, but for example, if we had decided to
> > > > drop commit A2 during an interactive rebase (so losing A2' from
> > > > diagram above), wouldn`t U2' still introduce those changes back, once
> > > > U1' and U2' are merged, being incorrect/unwanted behavior...? :/
> > >
> > > In that case, the method won't work well at all, so I think we need a
> > > different approach.
> > >
> >
> > Hmm, still rushing it, but what about adding an additional step, 
> > something like this:
> 
> I think it's unneeded, as it should work fine without it, see another
> reply.

Unfortunately, I have a broken test case saying different - it could 
very well be a flawed test, too, but let`s elaborate in that 
other sub-thread[1], indeed.

Regards, Buga

[1] https://public-inbox.org/git/87606hoflx@javad.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
On 28/02/2018 03:12, Igor Djordjevic wrote:
> 
> Would additional step as suggested in [1] (using R1 and R2 to "catch" 
> interactive rebase additions/amendments/drops, on top of U1' and 
> U2'), make more sense (or provide an additional clue, at least)?
> 
> [1] 
> https://public-inbox.org/git/8829c395-fb84-2db0-9288-f7b28fa0d...@gmail.com/

Heh, no sleeping tonight :P

Anyway, from (yet another) ad hoc test, additional step mentioned in 
[1] above seems to handle this case, too (merge with `-s ours` 
dropping B* patches, plus B1 cherry-picked between X1..X2):

On 28/02/2018 00:27, Johannes Schindelin wrote:
> 
> - One of the promises was that the new way would also handle merge
>   strategies other than recursive. What would happen, for example, if M
>   was generated using `-s ours` (read: dropping the B* patches' changes)
>   and if B1 had been cherry-picked into the history between X1..X2?
> 
>   Reverting R would obviously revert those B1 changes, even if B1' would
>   obviously not even be part of the rebased history!
> 
> Yes, I agree that this `-s ours` example is quite concocted, but the point
> of this example is not how plausible it is, but how easy it is to come up
> with a scenario where this design to "rebase merge commits" results in
> very, very unwanted behavior.

And not just that - it worked with additional interactive rebase 
adding, amending and removing commits, on top of all this still 
preserving original `-s ours` merge commit evil-merge amendment, too, 
all as expected (or so it seems) :P

Again, for the brave ones, here`s another messy test script (not 
tightly related to the last discussed diagrams, but based on my 
original "demonstration" script[2])... Hopefully I get to make a 
proper (and sane) sample soon... if not missing something here in the 
first place ;)

Regards, Buga

[2] https://public-inbox.org/git/bbe64321-4d3a-d3fe-8bb9-58b600fab...@gmail.com/

-- 8< --
#!/bin/sh

# rm -rf ./.git
# rm -f ./test.txt

git init

touch ./test.txt
git add -- test.txt

for i in {1..20}
do
echo A$i >>test.txt
git commit -am "A$i"
done

git checkout -b b1
sed -i '3iB11' test.txt
git commit -am "B11"
sed -i '7iB12' test.txt
git commit -am "B12"

git checkout -b b2 HEAD^
sed -i '16iB21' test.txt
git commit -am "B21"
sed -i '18iB22' test.txt
git commit -am "B22"

git checkout -b merge b1
git merge -s ours --no-commit b2
sed -i '12iX' test.txt # amend merge commit
git commit -am "M"
git tag original-merge

git checkout master
git cherry-pick b2
for i in {1..5}
do
j=`expr "$i" + 20`
sed -i "${i}iA${j}" test.txt
git commit -am "A$j"
done

# simple/naive demonstration of proposed merge rebasing logic
# using described "Trivial Merge" (TM, or "Angel Merge"),
# preserving merge commit manual amendments, but still respecting
# interactively rebased added/modified/dropped commits :)

# read -p "Press enter to continue"
git checkout b1
git cherry-pick -m1 original-merge && git tag U1
git reset --hard HEAD^^ # drop U1 and last b1 commit
sed -i '/B11/c\B' test.txt
git commit -a --amend --no-edit
git rebase master
git cherry-pick U1 && git tag U1-prime

# read -p "Press enter to continue"
git checkout b2
git cherry-pick -m2 original-merge && git tag U2
git reset --hard HEAD^ # drop U2
git rebase master
sed -i '20iBX' test.txt
git commit -am "BX" # add new commit
git cherry-pick U2 && git tag U2-prime

git diff U1 U1-prime | git apply --3way && git commit -m "U2-second" && git tag 
U2-second
git checkout b1
git diff U2 U2-prime | git apply --3way && git commit -m "U1-second" && git tag 
U1-second

# read -p "Press enter to continue"
git branch -f merge b1
git checkout merge
git merge b2 --no-commit
git commit -a --reuse-message original-merge
git tag angel-merge

# read -p "Press enter to continue"
git reset --hard b1^
git read-tree --reset angel-merge
git update-ref refs/heads/merge "$(git show -s --format=%B original-merge | git 
commit-tree "$(git write-tree)" -p "$(git rev-parse b1^^)" -p "$(git rev-parse 
b2^^)")"
git tag -f angel-merge
git checkout angel-merge .
git branch -f b1 b1^^
git branch -f b2 b2^^

# show resulting graph
echo
git log --all --decorate --oneline --graph

# comparison between original merge and rebased merge,
# showing merge commit amendment "X" being preserved during rebase
# (not shown in diff)
echo
echo 'diff original-merge angel-merge:'
git diff original-merge angel-merge


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
Hi Junio,

On 28/02/2018 01:10, Junio C Hamano wrote:
> 
> > > (3) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> > >
> >
> > Meh, I hope I`m rushing it now, but for example, if we had decided to 
> > drop commit A2 during an interactive rebase (so losing A2' from 
> > diagram above), wouldn`t U2' still introduce those changes back, once 
> > U1' and U2' are merged, being incorrect/unwanted behavior...? :/
> 
> As long as we are talking about rebase that allows us users to
> adjust and make changes ("rebase -i" being the prime and most
> flexible example), it is easy to imagine that A1'--A3' and B1'--B3'
> have almost no relation to their original counterparts.  After all,
> mechanical merge won't be able to guess the intention of the change
> humans make, so depending on what happend during X1 and X2 that
> happend outside of these two topics, what's required to bring series
> A and B to series A' and B' can be unlimited.  So from that alone,
> it should be clear that replaying difference between M and A3 (and M
> and B3) on top of U1' and U2' is hopeless as a general solution.

Yeah, I`ve encountered it in my (trivial) test case :(

> It is acceptable as long as a solution fails loudly when it does the
> wrong thing, but I do not think the apporach can produce incorrect
> result silently, as your example shows above.

Hmm, I think my example doesn`t even try to prevent failing, but it 
should otherwise be perfectly capable of doing so (and doing it 
loudly) - for example, it`s enough to diff U1' and U2' - if not the 
same, user might want to confirm the "rebased" merge outcome, as 
either something went wrong, or interactive rebase happened... or 
both :) (it`s what Sergey originally explained, seeming to be a solid 
safety net, though more testing would be good)

> What you _COULD_ learn from an old merge is to compute:
> 
> - a trial and mechanical merge between A3 and B3; call that pre-M
> 
> - diff to bring pre-M to M (call that patch evil-M); which is
>   what the person who made M did to resolve the textual and
>   semantic conflicts necessary to merge these two topics.
> 
> Then when merging A3' and B3', you could try to mechanically merge
> them (call that pre-M'), and apply evil-M, which may apply cleanly
> on top of pre-M', or it may not.  When there aren't so huge a
> difference between series A and A' (and series B and B'), the result
> would probably be a moral equivalent of Sergay's "replay" (so this
> approach will also silently produce a wrong result without human
> supervision).  One edge the evil-M approach has over Sergey's "dual
> cherry pick" is that it separates and highlights non-mechanical
> conflict resolution out of mechanical merges in a human readable
> form (i.e. the patch evil-M).

This seems to be what Johannes wrote about[1], too, but also 
something I think would be good to avoid, if possible, not 
complicating it too much :P

Maybe something like that latest thought[2] could help, using 
additional R1 and R2 commits to handle interactive rebase 
additions/amendments/drops, on top of U1' and U2'? Yeah, and 
that`s not complicating it... ;) :D

Regards, Buga

[1] 
https://public-inbox.org/git/nycvar.qro.7.76.6.1802272330290...@zvavag-6oxh6da.rhebcr.pbec.zvpebfbsg.pbz/
[2] https://public-inbox.org/git/8829c395-fb84-2db0-9288-f7b28fa0d...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
Hi Johannes,

On 28/02/2018 00:27, Johannes Schindelin wrote:
> 
> thank you for making this a lot more understandable to this thick
> developer.

Hehe, no problem, it primarily served fighting my own thickness ;)

> > Finally, we drop temporary commits, and record rebased commits A3' 
> > and B3' as our "rebased" merge commit parents instead (merge commit 
> > M' keeps its same tree/snapshot state, just gets parents replaced):
> >
> > (5) ---X1---o---o---o---o---o---X2
> >|\   |\
> >| A1---A2---A3---U1  | A1'--A2'--A3'
> >| \  | \
> >|  M |  M'
> >| /  | /
> >\-B1---B2---B3---U2  \-B1'--B2'--B3'
> 
> ...
> 
> In my example, where I dropped A1' specifically so that that embarrasingly
> incorrect change to the README would not be seen by the world, though, the
> evil merge would be truly evil: it would show said change to the world.
> The exact opposite of what I wanted.

Yeah, I`m afraid that`s what my testing produced as well :( Back to 
the drawing board...

> It would have been nice to have such a simple solution ;-)

Eh, the worst thing is the feeling I have, like it`s just around the 
corner, but we`re somehow missing it :P

> So the most obvious way to try to fix this design would be to recreate the
> original merge first, even with merge conflicts, and then trying to use the
> diff between that and the actual original merge commit.

For simplicity sake, this is something I would like to avoid (if 
possible), and also for the reasons you mentioned yourself:

> Now, would this work?
> 
> I doubt it, for at least two reasons:
> 
> - if there are merge conflicts between A3/B3 and between A3'/B3', those
>   merge conflicts will very likely look very different, and the conflicts
>   when reverting R will contain those nested conflicts: utterly confusing.
>   And those conflicts will look even more confusing if a patch (such as
>   A1') was dropped during an interactive rebase.
> 
> - One of the promises was that the new way would also handle merge
>   strategies other than recursive. What would happen, for example, if M
>   was generated using `-s ours` (read: dropping the B* patches' changes)
>   and if B1 had been cherry-picked into the history between X1..X2?
> 
>   Reverting R would obviously revert those B1 changes, even if B1' would
>   obviously not even be part of the rebased history!
> 
> ...
> 
> But maybe I missed something obvious, and the design can still be fixed
> somehow?

Would additional step as suggested in [1] (using R1 and R2 to "catch" 
interactive rebase additions/amendments/drops, on top of U1' and 
U2'), make more sense (or provide an additional clue, at least)?

It`s late here, and I`m really rushing it now, so please forgive me if 
it`s a stupid one... :$

Regards, Buga

[1] https://public-inbox.org/git/8829c395-fb84-2db0-9288-f7b28fa0d...@gmail.com/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
On 28/02/2018 02:33, Igor Djordjevic wrote:
> 
> This seems to be working inside my (too trivial?) test case, for 
> interactive adding, dropping, and amending of rebased commits, 
> resulting "rebased" merge containing all the added/modified/dropped 
> changes, plus the original merge amendment, all as expected :P

In case anyone has a wish to examine my (now pretty messy) test 
script, here it is - sorry for not having time to clean it up! :(

What I get when I diff original and "rebased" merge is this:

diff --git a/test.txt b/test.txt
index a82470b..d458032 100644
--- a/test.txt
+++ b/test.txt
@@ -1,10 +1,14 @@
+A21
+A22
+A23
+A24
+A25
 A1
 A2
-B11
+B
 A3
 A4
 A5
-B12
 A6
 A7
 A8
@@ -14,6 +18,7 @@ A10
 A11
 A12
 A13
+BX
 A14
 B2
 A15


... where A21 to A25 are additions due to new base, B11 was 
interactively amended to B, B12 was interactively dropped, and BX 
interactively added :)

We don`t see line X here, being an "evil merge" amendment being 
correctly preserved from original merge commit (thus not a 
difference). If we do `git show` of the "rebased" merge, we get this, 
as expected:

diff --cc test.txt
index b173cef,fad39a8..d458032
--- a/test.txt
+++ b/test.txt
@@@ -13,6 -13,6 +13,7 @@@ A
  A7
  A8
  A9
++X
  A10
  A11
  A12


Regards, Buga

-- 8< --
#!/bin/sh

# rm -rf ./.git
# rm -f ./test.txt

git init

touch ./test.txt
git add -- test.txt

for i in {1..20}
do
echo A$i >>test.txt
git commit -am "A$i"
done

git checkout -b b1
sed -i '3iB11' test.txt
git commit -am "B11"
sed -i '7iB12' test.txt
git commit -am "B12"

git checkout -b b2 HEAD^
sed -i '16iB2' test.txt
git commit -am "B2"

git checkout -b merge b1
git merge --no-commit b2
sed -i '12iX' test.txt # amend merge commit
git commit -am "M"
git tag original-merge

git checkout master
for i in {1..5}
do
j=`expr "$i" + 20`
sed -i "${i}iA${j}" test.txt
git commit -am "A$j"
done

# simple/naive demonstration of proposed merge rebasing logic
# using described "Trivial Merge" (TM, or "Angel Merge"),
# preserving merge commit manual amendments, but still respecting
# interactively rebased added/modified/dropped commits :)

# read -p "Press enter to continue"
git checkout b1
git cherry-pick -m1 original-merge && git tag U1
git reset --hard HEAD^^ # drop U1 and last b1 commit
sed -i '/B11/c\B' test.txt
git commit -a --amend --no-edit
git rebase master
git cherry-pick U1 && git tag U1-prime

# read -p "Press enter to continue"
git checkout b2
git cherry-pick -m2 original-merge && git tag U2
git reset --hard HEAD^ # drop U2
git rebase master
sed -i '20iBX' test.txt
git commit -am "BX" # add new commit
git cherry-pick U2 && git tag U2-prime

git diff U1 U1-prime | git apply --3way && git commit -m "U2-second" && git tag 
U2-second
git checkout b1
git diff U2 U2-prime | git apply --3way && git commit -m "U1-second" && git tag 
U1-second

# read -p "Press enter to continue"
git branch -f merge b1
git checkout merge
git merge b2 --no-commit
git commit -a --reuse-message original-merge
git tag angel-merge

# read -p "Press enter to continue"
git reset --hard b1^
git read-tree --reset angel-merge
git update-ref refs/heads/merge "$(git show -s --format=%B original-merge | git 
commit-tree "$(git write-tree)" -p "$(git rev-parse b1^^)" -p "$(git rev-parse 
b2^^)")"
git tag -f angel-merge
git checkout angel-merge .
git branch -f b1 b1^^
git branch -f b2 b2^^

# show resulting graph
echo
git log --all --decorate --oneline --graph

# comparison between original merge and rebased merge,
# showing merge commit amendment "X" being preserved during rebase
# (not shown in diff)
echo
echo 'diff original-merge angel-merge:'
git diff original-merge angel-merge


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
On 28/02/2018 01:36, Jacob Keller wrote:
> 
> > > (3) ---X1---o---o---o---o---o---X2
> > >|\   |\
> > >| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
> > >| \  |
> > >|  M |
> > >| /  |
> > >\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> > >
> >
> > Meh, I hope I`m rushing it now, but for example, if we had decided to
> > drop commit A2 during an interactive rebase (so losing A2' from
> > diagram above), wouldn`t U2' still introduce those changes back, once
> > U1' and U2' are merged, being incorrect/unwanted behavior...? :/
> 
> In that case, the method won't work well at all, so I think we need a
> different approach.
> 

Hmm, still rushing it, but what about adding an additional step, 
something like this: 
 
 (4) ---X1---o---o---o---o---o---X2
|\   |\
| A1---A2---A3---U1  | A1'--A2'--A3'--U1'--R1
| \  |
|  M |
| /  |
\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'--R2


... where:

  R1 = git diff U2 U2' | git apply --3way
  R2 = git diff U1 U1' | git apply --3way

(this is just to explain the point, might be there is a better way to 
produce Rx)

So, we still use Ux' to preserve merge commit M amendments, but also 
Rx to catch any changes happening between Ux and Ux' caused by 
interactive rebase commit manipulation (add/amend/drop).

Note that R*1* is produced by applying diff from U*2*' side, and vice 
versa (as it`s the other side that can erroneously introduce dropped 
commit changes back, like U2' in case of dropped A2').

>From here we continue as before - merging R1 and R2, then rewriting 
merge commit parents to point to A3' and B3' (dropping Ux` and Rx).

This seems to be working inside my (too trivial?) test case, for 
interactive adding, dropping, and amending of rebased commits, 
resulting "rebased" merge containing all the added/modified/dropped 
changes, plus the original merge amendment, all as expected :P

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
On 27/02/2018 20:59, Igor Djordjevic wrote:
> 
> (3) ---X1---o---o---o---o---o---X2
>|\   |\
>| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
>| \  |
>|  M |
>| /  |
>\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
> 

Meh, I hope I`m rushing it now, but for example, if we had decided to 
drop commit A2 during an interactive rebase (so losing A2' from 
diagram above), wouldn`t U2' still introduce those changes back, once 
U1' and U2' are merged, being incorrect/unwanted behavior...? :/

p.s. Looks like Johannes already elaborated on this in the meantime, 
let`s see... (goes reading that other e-mail[1])

[1] 
https://public-inbox.org/git/nycvar.qro.7.76.6.1802272330290...@zvavag-6oxh6da.rhebcr.pbec.zvpebfbsg.pbz/


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
On 27/02/2018 19:55, Igor Djordjevic wrote:
> 
> It would be more along the lines of "(1) rebase old merge commit parents, 
> (2) generate separate diff between old merge commit and each of its 
> parents, (3) apply each diff to their corresponding newly rebased 
> parent respectively (as a temporary commit, one per rebased parent), 
> (4) merge these temporary commits to generate 'rebased' merge commit, 
> (5) drop temporary commits, recording their parents as parents of 
> 'rebased' merge commit (instead of dropped temporary commits)".
> 
> Implementation wise, steps (2) and (3) could also be done by simply 
> copying old merge commit _snapshot_ on top of each of its parents as 
> a temporary, non-merge commit, then rebasing (cherry-picking) these 
> temporary commits on top of their rebased parent commits to produce 
> rebased temporary commits (to be merged for generating 'rebased' 
> merge commit in step (4)).

For those still tagging along (and still confused), here are some 
diagrams (following what Sergey originally described). Note that 
actual implementation might be even simpler, but I believe it`s a bit 
easier to understand like this, using some "temporary" commits approach.

Here`s our starting position:

(0) ---X1---o---o---o---o---o---X2 (master)
   |\
   | A1---A2---A3
   | \
   |  M (topic)
   | /
   \-B1---B2---B3


Now, we want to rebase merge commit M from X1 onto X2. First, rebase
merge commit parents as usual:

(1) ---X1---o---o---o---o---o---X2
   |\   |\
   | A1---A2---A3   | A1'--A2'--A3'
   | \  |
   |  M |
   | /  |
   \-B1---B2---B3   \-B1'--B2'--B3'


That was commonly understandable part. Now, for "rebasing" the merge 
commit (keeping possible amendments), we do some extra work. First, 
we make two temporary commits on top of old merge parents, by using 
exact tree (snapshot) of commit M:

(2) ---X1---o---o---o---o---o---X2
   |\   |\
   | A1---A2---A3---U1  | A1'--A2'--A3'
   | \  |
   |  M |
   | /  |
   \-B1---B2---B3---U2  \-B1'--B2'--B3'


So here, in terms of _snapshots_ (trees, not diffs), U1 = U2 = M.

Now, we rebase these temporary commits, too:

(3) ---X1---o---o---o---o---o---X2
   |\   |\
   | A1---A2---A3---U1  | A1'--A2'--A3'--U1'
   | \  |
   |  M |
   | /  |
   \-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'


As a next step, we merge these temporary commits to produce our 
"rebased" merged commit M:

(4) ---X1---o---o---o---o---o---X2
   |\   |\
   | A1---A2---A3---U1  | A1'--A2'--A3'--U1'
   | \  |  \
   |  M |   M'
   | /  |  /
   \-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'


Finally, we drop temporary commits, and record rebased commits A3' 
and B3' as our "rebased" merge commit parents instead (merge commit 
M' keeps its same tree/snapshot state, just gets parents replaced):

(5) ---X1---o---o---o---o---o---X2
   |\   |\
   | A1---A2---A3---U1  | A1'--A2'--A3'
   | \  | \
   |  M |  M'
   | /  | /
   \-B1---B2---B3---U2  \-B1'--B2'--B3'


And that`s it, our merge commit M has been "rebased" to M' :)

(6) ---X1---o---o---o---o---o---X2 (master)
|\
| A1'--A2'--A3'
| \
|  M' (topic)
| /
\-B1'--B2'--B3'


Important thing to note here is that in our step (3) above, still in 
terms of trees/snapshots (not diffs), U1' could still be equal to 
U2', produced merge commit M' tree thus being equal to both of them 
as well (merge commit introducing no changes to either of its 
parents, originally described by Sergey as "angel merge").

But it doesn`t have to be so - if any of the rebased commits A1 to A3 
or B1 to B3 was dropped or modified (or extra commits added, even), 
that would influence the trees (snapshots) produced after rebasing U1 
and U2 to U1' and U2', final merge M' reflecting all these changes as 
well, besides keeping original merge commit M amendments (preserving 
"evil merge").


Well, that`s some theory, now to hopefully confirm/test/polish all 
this... or trash it, if flawed beyond correction :P

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-27 Thread Igor Djordjevic
Hi Dscho,

On 27/02/2018 17:21, Johannes Schindelin wrote:
> 
> Do you have any way to describe the idea in a simple, 3-5 lines long 
> paragraph?
> 
> So far, I just know that it is some sort of confusing criss-cross 
> cherry-picking and merging and stuff, but nothing in those steps
> shouts out to me what the *idea* is.
> 
> If it would be something like "recreate the old merge, with merge 
> conflicts and all, then generate the diff to the actual tree of the
> merge commit, then apply that to the newly-generated merge", I would
> understand.

It would be more along the lines of "(1) rebase old merge commit parents, 
(2) generate separate diff between old merge commit and each of its 
parents, (3) apply each diff to their corresponding newly rebased 
parent respectively (as a temporary commit, one per rebased parent), 
(4) merge these temporary commits to generate 'rebased' merge commit, 
(5) drop temporary commits, recording their parents as parents of 
'rebased' merge commit (instead of dropped temporary commits)".

Implementation wise, steps (2) and (3) could also be done by simply 
copying old merge commit _snapshot_ on top of each of its parents as 
a temporary, non-merge commit, then rebasing (cherry-picking) these 
temporary commits on top of their rebased parent commits to produce 
rebased temporary commits (to be merged for generating 'rebased' 
merge commit in step (4)).

Regards, Buga


Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

2018-02-19 Thread Igor Djordjevic
Hi Sergey,

On 16/02/2018 14:08, Sergey Organov wrote:
> 
> By accepting the challenges raised in recent discussion of advanced
> support for history rebasing and editing in Git, I hopefully figured out
> a clean and elegant method of rebasing merges that I think is "The Right
> Way (TM)" to perform this so far troublesome operation. ["(TM)" here has
> second meaning: a "Trivial Merge (TM)", see below.]
> 
> Let me begin by outlining the method in git terms, and special thanks
> here must go to "Johannes Sixt"  for his original bright
> idea to use "cherry-pick -m1" to rebase merge commits.
> 
> End of preface -- here we go.
> 
> Given 2 original branches, b1 and b2, and a merge commit M that joins
> them, suppose we've already rebased b1 to b1', and b2 to b2'. Suppose
> also that B1' and B2' happen to be the tip commits on b1' and b2',
> respectively.
> 
> To produce merge commit M' that joins b1' and b2', the following
> operations will suffice:
> 
> 1. Checkout b2' and cherry-pick -m2 M, to produce U2' (and new b2').
> 2. Checkout b1' and cherry-pick -m1 M, to produce U1' (and new b1').
> 3. Merge --no-ff new b2' to current new b1', to produce UM'.
> 4. Get rid of U1' and U2' by re-writing parent references of UM' from
>U1' and U2' to  B1' and B2', respectively, to produce M'.
> 5. Mission complete.
> 
> Let's now see why and how the method actually works.
> 
> Firs off, let me introduce you to my new friend, the Trivial Merge, or
> (TM) for short. By definition, (TM) is a merge that introduces
> absolutely no differences to the sides of the merge. (I also like to
> sometimes call him "Angel Merge", both as the most beautiful of all
> merges, and as direct antithesis to "evil merge".)
> 
> One very nice thing about (TM) is that to safely rebase it, it suffices
> to merge its (rebased) parents. It is safe in this case, as (TM) itself
> doesn't posses any content changes, and thus none could be missed by
> replacing it with another merge commit.
> 
> I bet most of us have never seen (TM) in practice though, so let's see
> how (TM) can help us handle general case of some random merge. What I'm
> going to do is to create a virtual (TM) and see how it goes from there.
> 
> Let's start with this history:
> 
>   M
>  / \
> B1  B2
> 
> And let's transform it to the following one, contextually equivalent to
> the original, by introducing 2 simple utility commits U1 and U2, and a
> new utility merge commit UM:
> 
>   UM
>  /  \
> U1   U2
> ||
> B1   B2
> 
> Here content of any of the created UM, U1, and U2 is the same, and is
> exact copy of original content of M. I.e., provided [A] denotes
> "content of commit A", we have:
> 
> [UM] = [U1] = [U2] = [M]
> 
> Stress again how these changes to the history preserve the exact content
> of the original merge ([UM] = [M]), and how U1 an U2 represent content
> changes due to merge on either side[*], and how neither preceding nor
> subsequent commits content would be affected by the change of
> representation.
> 
> Now observe that as [U1] = [UM], and [U2] = [UM], the UM happens to be
> exactly our new friend -- the "Trivial Merge (TM)" his true self,
> introducing zero changes to content.
> 
> Next we rebase our new representation of the history and we get:
> 
>   UM'
>  /  \
> U1'  U2'
> ||
> B1'  B2'
> 
> Here UM' is bare merge of U1' and U2', in exact accordance with the
> method of rebasing a (TM) we've already discussed above, and U1' and U2'
> are rebased versions of U1 and U2, obtained by usual rebasing methods
> for non-merge commits.
> 
> (Note, however, that at this point UM' is not necessarily a (TM)
> anymore, so in real implementation it may make sense to check if UM' is
> not a (TM) and stop for possible user amendment.)
> 
> Finally, to get to our required merge commit M', we get the content of
> UM' and record two actual parents of the merge:
> 
>   M'
>  / \
> B1' B2'
> 
> Where [M'] = [UM'].
> 
> That's it. Mission complete.
> 
> I expect the method to have the following nice features:
> 
> - it carefully preserves user changes by rebasing the merge commit
> itself, in a way that is semantically similar to rebasing simple
> (non-merge) commits, yet it allows changes made to branches during
> history editing to propagate over corresponding merge commit that joins
> the branches, even automatically when the changes don't conflict, as
> expected.
> 
> - it has provision for detection of even slightest chances of ending up
> with surprising merge (just check if UM' is still (TM)), so that
> implementation could stop for user inspection and amendment when
> appropriate, yet it is capable of handling trivial cases smoothly and
> automatically.
> 
> - it never falls back to simple invocation of merge operation on rebased
> original branches themselves, thus avoiding the problem of lack of
> knowledge of how the merge at hand has been performed in the first
> place. It doesn't prevent implementation from letting user to manually
> 

Re: [PATCH v2 10/14] commit-graph: add core.commitgraph setting

2018-01-31 Thread Igor Djordjevic
Hi Derrick,

On 30/01/2018 22:39, Derrick Stolee wrote:
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 0e25b2c92b..5b63559a2b 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -898,6 +898,9 @@ core.notesRef::
>  This setting defaults to "refs/notes/commits", and it can be overridden by
>  the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
>  
> +core.commitgraph::
 ^^^
A small style nitpick - you may want to use "core.commitGraph" 
throughout the series (note capital "G"), making it more readable and 
aligning with the rest of `git config` variable names (using "bumpyCaps" 
as per coding guidelines[1], and as seen a few lines below, at the 
end of this very patch, too, "core.sparseCheckout").

> + Enable git commit graph feature. Allows reading from .graph files.
> +
>  core.sparseCheckout::
>   Enable "sparse checkout" feature. See section "Sparse checkout" in
>   linkgit:git-read-tree[1] for more information.
> diff --git a/cache.h b/cache.h
> index d8b975a571..e50e447a4f 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -825,6 +825,7 @@ extern char *git_replace_ref_base;
>  extern int fsync_object_files;
>  extern int core_preload_index;
>  extern int core_apply_sparse_checkout;
> +extern int core_commitgraph;
^^^
Similar nit here, might be "core_commit_graph" (throughout the 
series) would align better with existing variable names around it 
(note additional underscore between "commit" and "graph"), but also 
with your own naming "scheme" used for cmd_commit_graph(), 
builtin_commit_graph_usage[], construct_commit_graph(), etc.

>  extern int precomposed_unicode;
>  extern int protect_hfs;
>  extern int protect_ntfs;
> diff --git a/config.c b/config.c
> index e617c2018d..99153fcfdb 100644
> --- a/config.c
> +++ b/config.c
> @@ -1223,6 +1223,11 @@ static int git_default_core_config(const char *var, 
> const char *value)
>   return 0;
>   }
>  
> + if (!strcmp(var, "core.commitgraph")) {
> + core_commitgraph = git_config_bool(var, value);
> + return 0;
> + }
> +
>   if (!strcmp(var, "core.sparsecheckout")) {
>   core_apply_sparse_checkout = git_config_bool(var, value);
>   return 0;
> diff --git a/environment.c b/environment.c
> index 63ac38a46f..faa4323cc5 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -61,6 +61,7 @@ enum object_creation_mode object_creation_mode = 
> OBJECT_CREATION_MODE;
>  char *notes_ref_name;
>  int grafts_replace_parents = 1;
>  int core_apply_sparse_checkout;
> +int core_commitgraph;
>  int merge_log_config = -1;
>  int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
>  unsigned long pack_size_limit_cfg;
> 

Thanks, Buga

[1] https://github.com/git/git/blob/master/Documentation/CodingGuidelines

  Externally Visible Names
  
  ...
  
  The section and variable names that consist of multiple words are
  formed by concatenating the words without punctuations (e.g. `-`),
  and are broken using bumpyCaps in documentation as a hint to the
  reader.


Re: git stash ; git stash pop unstages all staged changes

2018-01-30 Thread Igor Djordjevic
Hi Marten,

On 30/01/2018 11:05, Martin Häcker wrote:
> 
> I just lost quite some work, because `git stash; git stash pop` 
> doesn’t seem to understand the concept of the index correctly.

I`m afraid it actually seems you`re not fully understanding how `git 
stash pop` works - but the good news is none of your work is lost, as 
long as you have the stash sha1 (or you can find/restore it through 
`git fsck`).

> What git stash does get right is that it does remove everything that 
> is stashed from the current state of the repo, but what it doesn’t 
> get right is restoring that state fatefully in `git stash pop`.

According `git stash pop`[1] documentation, this is as expected, but 
I see the confusing part:

  Remove a single stashed state from the stash list and apply it on top 
  of the current working tree state, i.e., do the inverse operation of 
  `git stash push`. The working directory must match the index.

It clearly says that stash is applied "on top of the current _working 
tree_ state" (so _not_ index), but then it says "i.e., do the inverse 
operation of `git stash push`", which is not exactly true (in regards 
to index handling, no options provided explicitly).

Later below, it notes what to do to restore index as well:

  If the `--index` option is used, then tries to reinstate not only the 
  working tree’s changes, but also the index’s ones. However, this can 
  fail, when you have conflicts (which are stored in the index, where 
  you therefore can no longer apply the changes as they were originally).

> As a user, I would expect that `git stash pop` undoes the change that 
> `git stash` inflicted.

This seems as a fair expectation, especially when documentation 
itself states that `git stash pop` is reverse operation of `git stash 
push`...

As to why `git stash pop` behaves like this, I would leave to someone 
more informed to answer, what I myself managed to dig out so far is that 
it seems `--index` was originally discussed to be a default indeed, 
with `--skip-index` to turn it off if needed, see "[PATCH] Teach 
git-stash to "apply --index""[2] (2007-07-02, Johannes Schindelin) 
(note that `git stash pop` is essentially `git stash apply && git 
stash drop`).

For the reference, here are some similar topics:
 - "[BUG] git stash doesn't use --index as default"[3] (2013-10-11, James)
 - "Asymmetric default behavior of git stash"[4] (2013-08-27, Uwe Storbeck)
 - "Newbie question: Is it possible to undo a stash?"[5] (2008-05-14, Iván V.)

Finally, here`s your demo script, just modified to use `git stash 
pop --index` instead, yielding desired results:

  mkdir test
  cd test
  git init
  touch foo
  git add foo
  git commit -m "initial commit"
  echo "bar" >> foo
  git status
  git add foo
  git status
  echo "baz" >> foo
  git diff
  git status
  git stash
  git status
  git stash pop --index
  git diff


Regards, Buga

[1] https://git-scm.com/docs/git-stash#git-stash-pop--index-q--quietltstashgt
[2] 
https://public-inbox.org/git/pine.lnx.4.64.0707021213350.4...@racer.site/T/#u
[3] 
https://public-inbox.org/git/1381467430.4130.38.ca...@freed.purpleidea.com/T/#u
[4] https://public-inbox.org/git/20130827132210.ga14...@ibr.ch/T/#u
[5] 
https://public-inbox.org/git/509f40850805141256gce6ac1brf5ced6436f81d...@mail.gmail.com/T/#u


Re: [BUG] git pull with pull.rebase and rebase.autoStash is not working anymore in 2.16

2018-01-24 Thread Igor Djordjevic
Hi Dimitriy,

On 24/01/2018 13:19, Dimitriy wrote:
> 
> Looks like regression in 2.16.
> Worked fine before update.
> Seems like git stash is not always working.
> Any ideas?

Could this be the same one as reported as Git for Windows issue 
#1437[1] ("`git status` reports (non-existent) modifications after 
`git stash push`", 2018-01-20), fixed in Git for Windows v2.16.1...?

Care to try it out? :)

Regards, Buga

[1] https://github.com/git-for-windows/git/issues/1437

> $ git --version
> git version 2.16.0.windows.2
> 
> $ git config pull.rebase
> true
> 
> $ git config rebase.autoStash
> true
> 
> $ git status
> On branch develop
> Your branch is behind 'origin/develop' by 3 commits, and can be 
> fast-forwarded.
>   (use "git pull" to update your local branch)
> 
> Changes not staged for commit:
>   (use "git add ..." to update what will be committed)
>   (use "git checkout -- ..." to discard changes in working directory)
> 
> modified:   source_work/x.cpp
> 
> no changes added to commit (use "git add" and/or "git commit -a")
> 
> $ git pull
> Created autostash: 7ddcdf2ba
> HEAD is now at ba14a4c3f some commit
> Cannot rebase: You have unstaged changes.
> Please commit or stash them.
> 
> $ git status
> On branch develop
> Your branch is behind 'origin/develop' by 3 commits, and can be 
> fast-forwarded.
>   (use "git pull" to update your local branch)
> 
> You are currently rebasing.
>   (all conflicts fixed: run "git rebase --continue")
> 
> Changes not staged for commit:
>   (use "git add ..." to update what will be committed)
>   (use "git checkout -- ..." to discard changes in working directory)
> 
> modified:   source_work/x.cpp
> 
> no changes added to commit (use "git add" and/or "git commit -a")
> 
> $ git rebase --abort
> Applied autostash.
> 
> $ git stash
> Saved working directory and index state WIP on develop: ba14a4c3f  some commit
> 
> $ git status
> On branch develop
> Your branch is behind 'origin/develop' by 3 commits, and can be 
> fast-forwarded.
>   (use "git pull" to update your local branch)
> 
> Changes not staged for commit:
>   (use "git add ..." to update what will be committed)
>   (use "git checkout -- ..." to discard changes in working directory)
> 
> 
> modified:   source_work/x.cpp
> 
> no changes added to commit (use "git add" and/or "git commit -a")
> 
> $ git stash
> Saved working directory and index state WIP on develop: ba14a4c3f  some commit
> 
> $ git status
> On branch develop
> Your branch is behind 'origin/develop' by 3 commits, and can be 
> fast-forwarded.
>   (use "git pull" to update your local branch)
> 
> nothing to commit, working tree clean


Re: [PATCH v2 6/7] wt-status.c: handle worktree renames

2017-12-27 Thread Igor Djordjevic
p.s. An extra note for the casual reader, just in case:

On 28/12/2017 01:50, Igor Djordjevic wrote:
> 
> ... it totally slipped me that documentation is/was pretty strict 
> about  being HEAD path (exclusively), where I was still 
> expecting it to show renamed working tree "from" value as  
> in case of working tree (double) rename, too - where that exact 
> (already renamed in index) path wasn`t to be found inside HEAD at 
> all, so the working tree rename couldn`t really be shown as "source" 
> and "target" rename pair, strictly following the "porcelain v2" 
> specification... :/
> 
> ...
> 
> I repeat that this looks like the correct approach, making fully 
> described working tree rename detection possible in porcelain in the 
> first place, but also aligning output of "status" --porcelain 
> variants with its default (--long) form.

Wherever "working tree rename detection" is discussed above, it`s all 
still about renamed file `git add -N` case only (where old name, 
appearing "deleted" from HEAD/index, is not staged yet), not some new 
functionality where renamed file inside working tree is instantly / 
magically recognized without being staged in the first place.


Re: [PATCH v3 0/6] Renames in git-status "changed not staged" section

2017-12-27 Thread Igor Djordjevic
Hi Duy,

On 27/12/2017 11:18, Nguyễn Thái Ngọc Duy wrote:
> 
> v3 more or less goes back to v1 after my discussion with Igor about
> porcelain formats. So 7/7 is not needed anymore. 4/7 becomes 5/6. The
> meat is still in 6/6, now with some more updates in git-status.txt and
> to address the comment from Torsten.

Albeit a tiny concern expressed in that last e-mail[1], this now 
seems fine, and a few tests I did came back as expected. Thanks!

Regards, Buga

[1] 
https://public-inbox.org/git/CACsJy8A=jz9laum50gvjnt5gtdiyymymupbsrjfo4lmkvqs...@mail.gmail.com/T/#m18b4e2cb2b7685fcc9650f3fb71b2191ef74cbe1


Re: [PATCH v2 6/7] wt-status.c: handle worktree renames

2017-12-27 Thread Igor Djordjevic
On 27/12/2017 02:06, Duy Nguyen wrote:
> 
> >   ... 
> >
> >The pathname. In a renamed/copied entry, this
> >  is the path in the index and in the working tree.
> 
> Gaah.. as you may see in the other mail when I quoted this
> (incorrectly). I must have modified this file at some point and
> thought it was true (my version did not have "and in the worktree").

Ah, this explains a lot... :) I got really confused with your v2, it 
felt as the series took a strange turn, and in a kind of a subtle way :P

> The "and" is still problematic if you take this very seriously
> (because in this case index name and worktree name are different) but
> I think it's ok to ignore that "and" and switch it to "or".

Yes, I agree, and the change does feel like a good thing. But, I also 
now hope this doesn`t break any expectations, because... (read below)

> >The pathname in the commit at HEAD. This is only
> >  present in a renamed/copied entry, and tells
> >  where the renamed/copied contents came from.
> >
> > If I`m reading this correctly, it should be vice-versa - value from
> > HEAD, being "original-file", should come last, where value from
> > working tree ("new-file") should be first.

... it totally slipped me that documentation is/was pretty strict 
about  being HEAD path (exclusively), where I was still 
expecting it to show renamed working tree "from" value as  
in case of working tree (double) rename, too - where that exact 
(already renamed in index) path wasn`t to be found inside HEAD at 
all, so the working tree rename couldn`t really be shown as "source" 
and "target" rename pair, strictly following the "porcelain v2" 
specification... :/

I see now that your initial reply[1] was talking about this, but I 
didn`t focus on it much as you replied to it yourself shortly 
afterwards, and later v2 of the series came up.

Might be this is where you changed your offline documentation 
version, too, as that is what the sample patch was about :)

> Yeah I think the "where the renamed/copied contents came from" clears
> up my confusion in this format. Back to v1 it is!

I see you addressed this by loosening the restriction here a bit, too,
making  be "pathname in the commit at HEAD _or in the index_",
in your "[PATCH v3 6/6] wt-status.c: handle worktree renames"[2].

I repeat that this looks like the correct approach, making fully 
described working tree rename detection possible in porcelain in the 
first place, but also aligning output of "status" --porcelain 
variants with its default (--long) form.

Hopefully, on top of everything positive, it also doesn`t break 
anything (too much?)... :P Latest revision should now provide all the 
necessary ingredients to resolve what happened, for the (small?) 
price of tweaking previous expectations a bit.

Regards, Buga

[1] 
https://public-inbox.org/git/CACsJy8A=jz9laum50gvjnt5gtdiyymymupbsrjfo4lmkvqs...@mail.gmail.com/T/#mf2f5ae672ec6f4e1abecbd5fe65283e9d8fbed57
[2] https://public-inbox.org/git/20171227101839.26427-7-pclo...@gmail.com/T/#u


Re: [PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format

2017-12-27 Thread Igor Djordjevic
On 27/12/2017 01:49, Duy Nguyen wrote:
> 
> > I lost you a bit here, partially because of what seems to be an
> > incomplete setup script, partially because of this last sentence, as
> > Git v2.15.1 doesn`t seem to be showing this, so not sure about "like
> > we have been showing until now" part...?
> 
> Yeah I missed a "git add -N third" in the setup. And "until now" was a
> poor choice of words. It should have been "before 425a28e0a4", where
> "new files" could not show up, which prevented rename detection in the
> "Changed bot not staged for commit" section in the first place.
> ...
> Sorry again about "now". Before 425a28e0a4 rename detection would not
> kick in to find "second -> third" so people wouldn't know about rename
> anyway.

Yeah, no worries, I had I hunch that it might be what you meant, but 
got too involved in all the rest that I forgot to bring that one up, 
too. Thanks for clarifying, though.

Regards, Buga


Re: Bring together merge and rebase

2017-12-26 Thread Igor Djordjevic
Very interesting topic, just this one part I wanted to comment on:

On 26/12/2017 02:28, Jacob Keller wrote:
> 
> What about some way to take the reflog and turn it into a commit-based
> linkage and export that? Rather than tying it into the individual
> commit history, keep track of it outside the commit, possibly via
> something like notes, or some other mechanism.

This seems like the most useful approach, might be not touching reflog 
per se, but having some kind of "cherry-picked commits source" log 
(where rebasing is a subset of cherry-picking). What Johannes 
mentioned, a mapping between "old" and "new" commits. Might be notes 
could fit in nicely, but I`m not competent to comment on that at the 
moment.

For me, the most interesting use case is not even tied to code review 
(thus no review comments to think about), but a situation where one 
might be rebasing a set of downstream patches on top of updating 
upstream - it might be possible for a bug to slip through due to some 
upstream changes, even where there are no conflicts and test suite is 
executed regularly (might be test reveling the bug is yet to be added).

In that situation, instead of just going back in "regular" history 
(single dimension) and eventually finding the offending (rebased) 
commit (its N-th rebased version, that is), it might be great to 
actually keep drilling down the "rebase history" now (second 
dimension), finding the exact rebase iteration / rebased commit 
version where the error first appeared.

Carl, you described this well in your document[1], and Johannes 
provided a valuable first-hand experience[2] from working around the 
very same native Git limitation for years, mentioning using (fragile, 
costly and not very automatible) rebased commits message search to 
drill down the second dimension (rebase iterations), which seems to 
be the only possible approach at the moment, with "vanilla" Git, at 
least.

So this might be much more interesting case, if code review one is 
less appropriate because of review comments being also relevant to 
commit rebase iterations (which should be then stored somewhere, too, 
relating to corresponding commits, not to lose context).

Regards, Buga

p.s. "Merging rebase" and "shears.sh" script[3] seem to be orthogonal 
to this - really great on their own in improving rebase itself and 
making it smarter and much more powerful and useful, where I guess 
they would benefit from native Git "cherry-picked (rebased) commits 
iterations tracking" (old/source <> new/destination commit mapping), 
too, as would other Git tools.

[1] http://blog.episodicgenius.com/post/merge-or-rebase--neither/
[2] 
https://public-inbox.org/git/20171226040843.h7o6txkrp6zlv...@glandium.org/T/#m2e5079488bed2968d4ea52a10051a06c06ff61e0
[3] 
https://github.com/git-for-windows/build-extra/blob/af9cff5005/shears.sh#L12-L18


Re: [PATCH v2 7/7] wt-status.c: avoid double renames in short/porcelain format

2017-12-26 Thread Igor Djordjevic
Hi Duy,

On 26/12/2017 10:10, Nguyễn Thái Ngọc Duy wrote:
> 
> The presence of worktree rename leads to an interesting situation,
> what if the same index entry is renamed twice, compared to HEAD and to
> worktree? We can have that with this setup
> 
> echo first > first && git add first && git commit -m first
> git mv first second  # rename reported in "diff --cached"
> mv second third  # rename reported in "diff-files"
> 
> For the long format this is fine because we print two "->" rename
> lines, one in the "updated" section, one in "changed" one.
> 
> For other output formats, it gets tricky because they combine both
> diffs in one line but can only display one rename per line. The result
> "XY" column of short format, for example, would be "RR" in that case.
> 
> This case either needs some extension in short/porcelain format
> to show something crazy like
> 
> RR first -> second -> third
> 
> or we could show renames as two lines instead of one, for example
> something like this for short form:
> 
> R  first -> second
>  R second -> third
> 
> But for now it's safer and simpler to just break the "second -> third"
> rename pair and show
> 
> RD first -> second
>  A third
> 
> like we have been showing until now.
> 

I lost you a bit here, partially because of what seems to be an 
incomplete setup script, partially because of this last sentence, as 
Git v2.15.1 doesn`t seem to be showing this, so not sure about "like 
we have been showing until now" part...?

Here, with your setup script, with plain Git v2.15.1, we have:

$ git status
On branch master
Changes to be committed:
  (use "git reset HEAD ..." to unstage)

renamed:first -> second

Changes not staged for commit:
  (use "git add/rm ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

deleted:second

Untracked files:
  (use "git add ..." to include in what will be committed)

third

Might be an additional `git add -N -- third` is needed here, to show 
what (I assume) you wanted...? If so:

$ git add -N third
(1) $ git status
On branch master
Changes to be committed:
  (use "git reset HEAD ..." to unstage)

renamed:first -> second

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

renamed:second -> second
  ^^
Now we can see two renames I believe you were talking about...? (Note 
original bug showing above, which started this thread.) Now, still 
using v2.15.1, let`s see porcelain statuses:

(2) $ git status --porcelain
RR first -> second

(3) $ git status --porcelain=v2
2 RR N... 100644 100644 00 9c59e24b8393179a5d712de4f990178df5734d99 
9c59e24b8393179a5d712de4f990178df5734d99 R100 secondfirst

Here, they both report renames in _both_ index and working tree (RR), 
but they show "index" renamed path only ("second", in comparison to 
original value in HEAD, "first").

I`m inclined to say this doesn`t align with what `git status` shows, 
disrespecting `add -N` (or respecting it only partially, through that 
second R, but not showing the actual working tree rename, "third").

Without influencing porcelain format, and to fully respect `add -N`, 
I believe showing two renames (index and working tree) as two lines 
would be the correct approach - and that`s what default `git status` 
does, too.

Now, let`s examine this patch series v2 outputs:

(1) $ git status
On branch master
Changes to be committed:
  (use "git reset HEAD ..." to unstage)

renamed:first -> second

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

renamed:second -> third

(2) $ git status --porcelain
RD first -> second
 A third

(3) $ git status --porcelain=v2
2 RD N... 100644 100644 00 9c59e24b8393179a5d712de4f990178df5734d99 
9c59e24b8393179a5d712de4f990178df5734d99 R100 secondfirst
1 .A N... 00 00 100644  
 third

Here, porcelain statuses make situation a bit better, as now at least 
`add -N` is respected, showing new "tracked" path appearing in the 
working tree.

But, we now lost any idea about the rename that happened there as 
well - which Git v2.15.1 porcelain was partially showing (through 
RR), and which `git status` still reports correctly - and which we 
still differ from.
 
I don`t think this looks like what we have been showing until now 
(unless I misunderstood which exact "now" are we talking about), so I 
don`t see that as a valid argument to support this 

Re: [PATCH v2 6/7] wt-status.c: handle worktree renames

2017-12-26 Thread Igor Djordjevic
Hi Duy,

On 26/12/2017 10:10, Nguyễn Thái Ngọc Duy wrote:
> Before 425a28e0a4 (diff-lib: allow ita entries treated as "not yet exist
> in index" - 2016-10-24) there are never "new files" in the index, which
> essentially disables rename detection because we only detect renames
> when a new file appears in a diff pair.
> 
> After that commit, an i-t-a entry can appear as a new file in "git
> diff-files". But the diff callback function in wt-status.c does not
> handle this case and produces incorrect status output.
> 
> PS. The reader may notice that this patch adds a new xstrdup() but not
> a free(). Yes we leak memory (the same for head_path). But wt_status
> so far has been short lived, this leak should not matter in
> practice.
> 
> Noticed-by: Alex Vandiver <ale...@dropbox.com>
> Helped-by: Igor Djordjevic <igor.d.djordje...@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
>  t/t2203-add-intent.sh | 28 
>  wt-status.c   | 72 
> +++
>  wt-status.h   |  4 +--
>  3 files changed, 85 insertions(+), 19 deletions(-)
> 
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 878e73fe98..e5bfda1853 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -162,5 +162,33 @@ test_expect_success 'commit: ita entries ignored in 
> empty commit check' '
>   )
>  '
>  
> +test_expect_success 'rename detection finds the right names' '
> + git init rename-detection &&
> + (
> + cd rename-detection &&
> + echo contents > first &&
> + git add first &&
> + git commit -m first &&
> + mv first third &&
> + git add -N third &&
> +
> + git status | grep -v "^?" >actual.1 &&
> + test_i18ngrep "renamed: *first -> third" actual.1 &&
> +
> + git status --porcelain | grep -v "^?" >actual.2 &&
> + cat >expected.2 <<-\EOF &&
> +  R first -> third
> + EOF
> + test_cmp expected.2 actual.2 &&
> +
> + oid=12f00e90b6ef79117ce6e650416b8cf517099b78 &&
> + git status --porcelain=v2 | grep -v "^?" >actual.3 &&
> + cat >expected.3 <<-EOF &&
> + 2 .R N... 100644 100644 100644 $oid $oid R100 first third
> + EOF
> + test_cmp expected.3 actual.3
> + )
> +'
> +
>  test_done

I`m afraid "--porcelain=v2" test might be incorrect here, as `git 
status --porcelain=v2` output seems to be too, with this v2 series 
applied. Test I sent previously[1] fails, and it looks valid.

This is output I now get, with old/deleted file unstaged and 
new/created file staged with `git add -N`:

$ git status --porcelain=v2
2 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 
12f00e90b6ef79117ce6e650416b8cf517099b78 R100 original-file new-file

Note "original-file" listed first, where "new-file" listed second 
(last). According the "v2" documentation[2] (excerpt):

  ... 

   The pathname. In a renamed/copied entry, this
 is the path in the index and in the working tree.
  ...
   The pathname in the commit at HEAD. This is only
 present in a renamed/copied entry, and tells
 where the renamed/copied contents came from.


If I`m reading this correctly, it should be vice-versa - value from 
HEAD, being "original-file", should come last, where value from 
working tree ("new-file") should be first.

If I stage both files and try again, output is as expected 
("new-file" comes first):

$ git status --porcelain=v2
2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 
12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-file  original-file

> diff --git a/wt-status.c b/wt-status.c
> index c124d7589c..d5bdf4c2e9 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -376,6 +376,8 @@ static void wt_longstatus_print_change_data(struct 
> wt_status *s,
>   strbuf_addch(, ')');
>   }
>   status = d->worktree_status;
> + if (d->worktree_path)
> + two_name = d->worktree_path;
>   break;
>   default:
>   die("BUG: unhandled change_type %d in 
> wt_longstatus_print_change_data",
> @@ -460,6 +462,12 @@ static void wt_status_collect_changed_

Re: [PATCH] status: handle worktree renames

2017-12-25 Thread Igor Djordjevic
On 25/12/2017 20:45, Igor Djordjevic wrote:
> 
> I guess an additional test for this would be good, too.

... aaand here it is. Again based on your test, but please double 
check, I`m not sure if it`s ok to compare file modes like that, 
expecting them to be the same (hashes should be fine, I guess).

---
 t/t2203-add-intent.sh | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 41a8874e6..394b1047c 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -165,5 +165,20 @@ test_expect_success 'rename detection finds the right 
names' '
)
 '
 
+test_expect_success 'rename detection finds the right names (porcelain v2)' '
+   git init rename-detection-v2 &&
+   (
+   cd rename-detection-v2 &&
+   echo contents > original-file &&
+   git add original-file &&
+   git commit -m first-commit &&
+   mv original-file new-file &&
+   git add -N new-file &&
+   git status --porcelain=v2 | grep -v actual >actual &&
+   echo "2 .R N... 100644 100644 100644 
12f00e90b6ef79117ce6e650416b8cf517099b78 
12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-fileoriginal-file" 
>expected &&
+   test_cmp expected actual
+   )
+'
+
 test_done
 
-- 
2.15.1.windows.2


Re: [PATCH] status: handle worktree renames

2017-12-25 Thread Igor Djordjevic
On 25/12/2017 19:26, Igor Djordjevic wrote:
> 
> But I`ve noticed that "--porcelain=v2" output might still be buggy - 
> this is what having both files staged shows:
> 
> $ git status --porcelain=v2
> 2 R. N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 
> 12f00e90b6ef79117ce6e650416b8cf517099b78 R100 new-fileoriginal-file
> 
> ..., where having old/deleted file unstaged, and new/created file 
> staged with `git add -N` shows this:
> 
> $ git status --porcelain=v2
> 1 .R N... 100644 100644 100644 12f00e90b6ef79117ce6e650416b8cf517099b78 
> 12f00e90b6ef79117ce6e650416b8cf517099b78 new-file
> 
> So even though unstaged value is correctly recognized as "R" (renamed), 
> first number is "1" (instead of "2" to signal rename/copy), and both 
> rename score and original file name are missing.

As an exercise, might be something like this as a fixup on top of 
your patch could work.

I`ve tried to follow your lead on what you did yourself, but please 
note that, besides being relatively new to Git codebase, this is my 
first C code for almost 10 years (since university), so... :)

I guess an additional test for this would be good, too.

Regards, Buga

---
 wt-status.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index f0b5b3d46..55c0ad249 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -2050,7 +2050,7 @@ static void wt_porcelain_v2_print_changed_entry(
const char *path_head = NULL;
char key[3];
char submodule_token[5];
-   char sep_char, eol_char;
+   char sep_char, eol_char, score_char;
 
wt_porcelain_v2_fix_up_changed(it, s);
wt_porcelain_v2_submodule_state(d, submodule_token);
@@ -2059,6 +2059,8 @@ static void wt_porcelain_v2_print_changed_entry(
key[1] = d->worktree_status ? d->worktree_status : '.';
key[2] = 0;
 
+   path_head = d->head_path ? d->head_path : d->worktree_path;
+   score_char = d->index_status ? key[0] : key[1];
if (s->null_termination) {
/*
 * In -z mode, we DO NOT C-quote pathnames.  Current path is 
ALWAYS first.
@@ -2067,7 +2069,6 @@ static void wt_porcelain_v2_print_changed_entry(
sep_char = '\0';
eol_char = '\0';
path_index = it->string;
-   path_head = d->head_path;
} else {
/*
 * Path(s) are C-quoted if necessary. Current path is ALWAYS 
first.
@@ -2078,8 +2079,8 @@ static void wt_porcelain_v2_print_changed_entry(
sep_char = '\t';
eol_char = '\n';
path_index = quote_path(it->string, s->prefix, _index);
-   if (d->head_path)
-   path_head = quote_path(d->head_path, s->prefix, 
_head);
+   if (path_head)
+   path_head = quote_path(path_head, s->prefix, _head);
}
 
if (path_head)
@@ -2087,7 +2088,7 @@ static void wt_porcelain_v2_print_changed_entry(
key, submodule_token,
d->mode_head, d->mode_index, d->mode_worktree,
oid_to_hex(>oid_head), 
oid_to_hex(>oid_index),
-   key[0], d->score,
+   score_char, d->score,
path_index, sep_char, path_head, eol_char);
else
fprintf(s->fp, "1 %s %s %06o %06o %06o %s %s %s%c",
-- 
2.15.1.windows.2


Re: [PATCH] status: handle worktree renames

2017-12-25 Thread Igor Djordjevic
Hi Duy,

On 25/12/2017 11:37, Nguyễn Thái Ngọc Duy wrote:
> Before 425a28e0a4 (diff-lib: allow ita entries treated as "not yet exist
> in index" - 2016-10-24) there are never "new files" in the index, which
> essentially disables rename detection because we only detect renames
> when a new file appears in a diff pair.
> 
> After that commit, an i-t-a entry can appear as a new file in "git
> diff-files". But the diff callback function in wt-status.c does not
> handle this case and produces incorrect status output.
> 
> Handle this rename case. While at there make sure unhandled diff status
> code is reported to catch cases like this easier in the future.
> 
> The reader may notice that this patch adds a new xstrdup() but not a
> free(). Yes we leak memory (the same for head_path). But wt_status so
> far has been short lived, this leak should not matter in practice.
> 
> Noticed-by: Alex Vandiver 
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  t/t2203-add-intent.sh | 15 +++
>  wt-status.c   | 24 +++-
>  wt-status.h   |  1 +
>  3 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
> index 1bdf38e80d..41a8874e60 100755
> --- a/t/t2203-add-intent.sh
> +++ b/t/t2203-add-intent.sh
> @@ -150,5 +150,20 @@ test_expect_success 'commit: ita entries ignored in 
> empty commit check' '
>   )
>  '
>  
> +test_expect_success 'rename detection finds the right names' '
> + git init rename-detection &&
> + (
> + cd rename-detection &&
> + echo contents > original-file &&
> + git add original-file &&
> + git commit -m first-commit &&
> + mv original-file new-file &&
> + git add -N new-file &&
> + git status --porcelain | grep -v actual >actual &&
> + echo " R original-file -> new-file" >expected &&
> + test_cmp expected actual
> + )
> +'
> +
>  test_done
>  
> diff --git a/wt-status.c b/wt-status.c
> index ef26f07446..f0b5b3d46a 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -376,6 +376,8 @@ static void wt_longstatus_print_change_data(struct 
> wt_status *s,
>   strbuf_addch(, ')');
>   }
>   status = d->worktree_status;
> + if (d->worktree_path)
> + one_name = d->worktree_path;
>   break;
>   default:
>   die("BUG: unhandled change_type %d in 
> wt_longstatus_print_change_data",
> @@ -432,7 +434,7 @@ static void wt_status_collect_changed_cb(struct 
> diff_queue_struct *q,
>   struct wt_status_change_data *d;
>  
>   p = q->queue[i];
> - it = string_list_insert(>change, p->one->path);
> + it = string_list_insert(>change, p->two->path);
>   d = it->util;
>   if (!d) {
>   d = xcalloc(1, sizeof(*d));
> @@ -459,6 +461,12 @@ static void wt_status_collect_changed_cb(struct 
> diff_queue_struct *q,
>   /* mode_worktree is zero for a delete. */
>   break;
>  
> + case DIFF_STATUS_COPIED:
> + case DIFF_STATUS_RENAMED:
> + d->worktree_path = xstrdup(p->one->path);
> + d->score = p->score * 100 / MAX_SCORE;
> + /* fallthru */
> +
>   case DIFF_STATUS_MODIFIED:
>   case DIFF_STATUS_TYPE_CHANGED:
>   case DIFF_STATUS_UNMERGED:
> @@ -467,8 +475,8 @@ static void wt_status_collect_changed_cb(struct 
> diff_queue_struct *q,
>   oidcpy(>oid_index, >one->oid);
>   break;
>  
> - case DIFF_STATUS_UNKNOWN:
> - die("BUG: worktree status unknown???");
> + default:
> + die("BUG: unhandled worktree status '%c'", p->status);
>   break;
>   }
>  
> @@ -548,6 +556,10 @@ static void wt_status_collect_updated_cb(struct 
> diff_queue_struct *q,
>* values in these fields.
>*/
>   break;
> +
> + default:
> + die("BUG: unhandled worktree status '%c'", p->status);
> + break;
>   }
>   }
>  }
> @@ -1724,8 +1736,10 @@ static void wt_shortstatus_status(struct 
> string_list_item *it,
>   } else {
>   struct strbuf onebuf = STRBUF_INIT;
>   const char *one;
> - if (d->head_path) {
> - one = quote_path(d->head_path, s->prefix, );
> +
> + one = d->head_path ? d->head_path : d->worktree_path;
> + if (one) {
> + one = quote_path(one, s->prefix, );
>   if (*one != '"' && strchr(one, ' ') != NULL) {
>   putchar('"');
>  

Re: Need help migrating workflow from svn to git.

2017-12-21 Thread Igor Djordjevic
Hi Josef,

On 20/12/2017 12:43, Josef Wolf wrote:
> 
>> $ git add -u
>> $ git reset
> 
> This would be added after the "git checkout -m -B master FETCH_HEAD" 
> command?

Yes, so it would be something like this:

git fetch origin master &&  #1
git checkout -m -B master FETCH_HEAD && #2
git add -u &&   #3
git reset   #4

But it actually depends on what kind of default `git diff` output 
you prefer.

In order to avoid failure on subsequent script runs, in case where 
conflicts still exist, you need to ensure #3 and #4 are executed 
before #1 and #2 are executed _again_.

So you may put #3 and #4 in front of #1 and #2, too, that would work 
just as well, where `git diff` would now be showing "combined diff"[2] 
as long as the script isn`t executed again (and it would keep showing 
new "combined diff" from that point on).

>> Yes, `git diff` won`t be the same as if conflicts were still in, but 
>> it might be worth it in this specific case, conflicting parts still 
>> easily visible between conflict markers.
> 
> That means, the conflict is still there, but git would think this is 
> an ordinary modification?

Yes, as by that `git add -u` you confirm all merge conflicts are 
resolved, and `git diff` output changes accordingly. You can read 
more about "diff format for merges"[1] and "combined diff format"[2] 
from `git-diff`[3] documentation.

Here are some examples from my test repositories. Local repo 
introduces line "A1" (local modification, uncommitted), where remote 
repo introduced line "B1" (commit). Steps #1 and #2 get executed, merge 
conflicts shown with `git diff`, before `git add -u` and `git reset`:

$ git diff
diff --cc A
index 5314b4f,1e2b966..000
--- a/A
+++ b/A
@@@ -12,5 -12,5 +12,9 @@@
  2
  3
  4
++<<< FETCH_HEAD
 +B1
++===
+ A1
++>>> local
  5

... and after `git add -u` and `git reset` (note line "B1" not 
showing as changed anymore):

$ git diff
diff --git a/A b/A
index 5314b4f..8ea9600 100644
--- a/A
+++ b/A
@@ -12,5 +12,9 @@ A
 2
 3
 4
+<<< FETCH_HEAD
 B1
+===
+A1
+>>> local
 5


Now, without any commits yet made locally (except commit pulled from 
remote repo), local repo adds line "A2" where remote repo introduces 
line "B2" (commit). Steps #1 and #2 get executed again, merge 
conflicts shown with `git diff`, before `git add -u` and `git reset`:

$ git diff
diff --cc A
index 424ae9e,4aac880..000
--- a/A
+++ b/A
@@@ -2,7 -2,7 +2,11 @@@
  1
  2
  3
++<<< FETCH_HEAD
 +B2
++===
+ A2
++>>> local
  4
  5
  6

... and after `git add -u` and `git reset` (note showing line "B2" as 
unchanged, and now showing leftover "conflicts" around "A1" here as 
well, where previous "combined" diff discarded it as uninteresting 
due to implied "--cc"[4] flag):

$ git diff
diff --git a/A b/A
index 424ae9e..77ad8e6 100644
--- a/A
+++ b/A
@@ -2,7 +2,11 @@ A
 1
 2
 3
+<<< FETCH_HEAD
 B2
+===
+A2
+>>> local
 4
 5
 6
@@ -13,5 +17,9 @@ A3
 2
 3
 4
+<<< FETCH_HEAD
 B1
+===
+A1
+>>> local
 5


Hope that helps. As usual, best to give it some try on your own :)

Regards, Buga

[1] https://git-scm.com/docs/git-diff#_diff_format_for_merges
[2] https://git-scm.com/docs/git-diff#_combined_diff_format
[3] https://git-scm.com/docs/git-diff
[4] https://git-scm.com/docs/git-diff-tree#git-diff-tree---cc


Re: [PATCH v2 1/5] core.aheadbehind: add new config setting

2017-12-21 Thread Igor Djordjevic
Hi Jeff,

On 21/12/2017 20:09, Jeff Hostetler wrote:
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 9593bfa..c78d6be 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -895,6 +895,14 @@ core.abbrev::
>   abbreviated object names to stay unique for some time.
>   The minimum length is 4.
>  
> +core.aheadbehind::
^^^
A small nitpick - you may want to use "core.aheadBehind" throughout 
the series (note capital "B"), making it more readable and aligning 
with the rest of `git config` variable names (using "bumpyCaps" as 
per coding guidelines[1], and as seen at the end of this very patch, 
too, "add.ignoreErrors").

> + If true, tells commands like status and branch to print ahead and
> + behind counts for the branch relative to its upstream branch.
> + This computation may be very expensive when there is a great
> + distance between the two branches.  If false, these commands
> + only print that the two branches refer to different commits.
> + Defaults to true.
> +
>  add.ignoreErrors::
>  add.ignore-errors (deprecated)::
>   Tells 'git add' to continue adding files when some files cannot be

Regards, Buga

[1] https://github.com/git/git/blob/master/Documentation/CodingGuidelines

  Externally Visible Names
  
  ...
  
  The section and variable names that consist of multiple words are
  formed by concatenating the words without punctuations (e.g. `-`),
  and are broken using bumpyCaps in documentation as a hint to the
  reader.


Re: feature-request: git "cp" like there is git mv.

2017-12-17 Thread Igor Djordjevic
Hi Simon,

On 12/12/2017 11:53, Simon Doodkin wrote:
> 
> please develop a new feature, git "cp" like there is git mv 
> tomovefile1 tofile2 (to save space).
> 
> there is a solution in https://stackoverflow.com/a/44036771/466363
> however, it is not single easy command.

While having `git cp` alongside `git mv` would make sense, I`m afraid 
that is not what you are really after, nor it would help in your case.

Looking at referenced "Stack Overflow" post[1], it tries to address 
`git blame` not following "file copy", where it does "file rename".

Proposed steps seem to be "solution" from your perspective, and while 
that may be absolutely valid and acceptable for your specific case, I 
would argue it`s the wrong approach in general - because `git blame` 
already supports what you want (just not by default), and making 
three additional, unneeded and possibly confusing commits (one being 
a merge), just to "bend" `git blame` to fit your (out of line?) usage 
expectations doesn`t seem right.

I would say a better direction might be using `git blame` "-C[]" 
option[2], where desired effect is achieved without any artificial 
history fiddling.

Example being worth more than plain words, I`m providing a script[3] 
that demonstrates what I`m talking about :)

Regards, Buga

[1] https://stackoverflow.com/a/44036771/466363
[2] https://git-scm.com/docs/git-blame#git-blame--Cltnumgt
[3] Demo script showing how using (multiple) "-C" option(s), with 
specified numeric value, can make `git blame` provide desired 
information, recognizing "file copy" operation (line copy, actually, 
but that is what we are really interested in, using `git blame`):
--- >8 ---
git init

echo a >A
echo b >>A
echo c >>A

git add A
git commit -m "create file A"

git mv A B
git commit -m "rename file A -> B"

# ---
# (A) regular flow
cp B C
git add C
git commit -m "copy file B -> C"
# ---

# ---
# (B) proposed "solution", https://stackoverflow.com/a/44036771/466363
# git mv B C
# git commit -n -m "rename file B -> C"
# SAVED=`git rev-parse HEAD`
# git reset --hard HEAD^
# git mv B B-temp
# git commit -n -m "rename file B -> B-temp"
# git merge $SAVED # This will generate conflicts
# git commit -a -n --no-edit # Trivially resolved like this
# git mv B-temp B
# git commit -n -m "rename file B-temp -> B"
# ---

echo
echo '(1) blames B back to A, as expected:'
git blame -- B
# git blame shows that file B has a history (back to file A)...

echo
echo '(2) blames C only, missing B and A:'
git blame -- C
# ... while file C doesn't have a history

echo
echo '(3) blames C back to A, as expected:'
git blame -C -C3 -- C
# git blame shows that file C has a history (back to file A)


Re: Need help migrating workflow from svn to git.

2017-12-15 Thread Igor Djordjevic
On 15/12/2017 20:09, Junio C Hamano wrote:
> 
> > Junio, what about consecutive runs, while merge conflicts are still 
> > unresolved?
> 
> The impression I got was that the original running with svn does not
> deal with conflicting situation anyway, so I did not think about it
> at all, and I personally do not care ;-)

Heh, fair enough :) Though I was genuinely interested if there is a 
better way to accomplish it than `git add -u && git reset`, but I 
guess I can take that as "whatever works" ;)


Re: Need help migrating workflow from svn to git.

2017-12-15 Thread Igor Djordjevic
On 15/12/2017 17:33, Junio C Hamano wrote:
> 
> $ git fetch  
> $ git checkout -m -B  FETCH_HEAD

... aaand that`s how you do it[1] without a temporary branch :)

Junio, what about consecutive runs, while merge conflicts are still 
unresolved?

Seeing Josef having a pretty relaxed flow, and his cron job running 
every 15 minutes, would adding something like:

$ git add -u
$ git reset

... to the mix, to "silence" actually still unresolved merge 
conflicts, making next script execution possible, make sense?

Yes, `git diff` won`t be the same as if conflicts were still in, but 
it might be worth it in this specific case, conflicting parts still 
easily visible between conflict markers.

Regards, Buga

[1] On 15/12/2017 19:24, Igor Djordjevic wrote:
> 
> git checkout -b temp &&   #1
> git fetch &&  #2
> git branch -f master origin/master && #3
> git checkout -m master && #4
> git add -u && #5
> git reset &&  #6
> git branch -d temp#7
> 
> Explanation:
>  1. Create temporary branch where we are, switching to it, so we can 
> update "master" without local modifications
>  2. Fetch latest updates
>  3. Update "master" to fetched "origin/master"
>  4. Switch to updated "master", merging local modifications
>  5. Mark any pending merge conflicts as resolved by staging them...
>  6. ... and unstage them right away
>  7. Delete temporary branch
> 
> Step (4) is what merges your local modifications with remote updates 
> (leaving conflicts, if any), where steps (5) and (6) are not needed 
> for a single run, but in case you don`t resolve conflicts before next 
> cron job executes this script again, step (1) will now fail without 
> them because of (still) unresolved merge conflicts.
> 
> So, as you seem to be pretty at ease with your flow, you might prefer 
> leaving those two steps (5, 6) in.


Re: Need help migrating workflow from svn to git.

2017-12-15 Thread Igor Djordjevic
Hi Josef,

Thank you for your patient answers. From what you said here and in 
that other reply[1], it looks like you know what you`re doing, you`re 
aware of circumstances, and you still prefer doing it that way.

So, here it goes... :)

On 15/12/2017 13:47, Josef Wolf wrote:
> 
> > I`m thinking of a workflow involving (scripted) creation of a 
> > temporary branch at fetched remote branch position, and using 
> > something like `git checkout --merge ` to merge your 
> > local modifications to latest changes fetched from remote (ending
> > up with conflicts inside working tree, if any),
> 
> But this would require local modifications to be committed?
 
Nope :) Here`s a script you can test to see if it works for you, 
simulating `svn update` (at least how I perceived it).

Feel free to adapt as you feel like it (I used local "master" branch 
and remote "origin/master", for example), or to speak up if any 
additional info is needed.

git checkout -b temp &&   #1
git fetch &&  #2
git branch -f master origin/master && #3
git checkout -m master && #4
git add -u && #5
git reset &&  #6
git branch -d temp#7

Explanation:
 1. Create temporary branch where we are, switching to it, so we can 
update "master" without local modifications
 2. Fetch latest updates
 3. Update "master" to fetched "origin/master"
 4. Switch to updated "master", merging local modifications
 5. Mark any pending merge conflicts as resolved by staging them...
 6. ... and unstage them right away
 7. Delete temporary branch

Step (4) is what merges your local modifications with remote updates 
(leaving conflicts, if any), where steps (5) and (6) are not needed 
for a single run, but in case you don`t resolve conflicts before next 
cron job executes this script again, step (1) will now fail without 
them because of (still) unresolved merge conflicts.

So, as you seem to be pretty at ease with your flow, you might prefer 
leaving those two steps (5, 6) in.

This does seem ugly and hacky, but if it works for you, I don`t judge :) 
Please note that there might be better ways to accomplish this, I 
repeat, I`m not an expert, but hopefully this could do the job.

Also, if I missed something, I hope someone will correct me.

Regards, Buga

[1] https://public-inbox.org/git/20171215130645.gd18...@raven.inka.de/


Re: Need help migrating workflow from svn to git.

2017-12-14 Thread Igor Djordjevic
On 14/12/2017 23:27, Igor Djordjevic wrote:
> 
> As you basically have a flow where two users (you and cron job) can 
> edit same files at the same time, desired outcome might be a bit 
> ambiguous, especially when scheduled execution of those files is 
> added to the mix.

This said, and without having you to change your habits too much (nor 
use Git in possibly awkward ways), I`m thinking you may actually 
benefit of using `git worktree add `[1] to create a 
temporary working tree ("working copy", as you say), alongside a 
temporary branch, where you could hack and test as much as you want, 
unaffected by cron job updating and executing the original working 
copy/branch (also not stepping on cron job`s toes yourself).

Once you`re satisfied and you commit/merge/push your changes from 
within the temporary working copy/branch, you can just delete it 
(both temporary working copy and its branch), and you`re good :)

p.s. Even if you`re not familiar with Git branching and merging, it 
shouldn`t take too much effort to wrap your head around it, and it`s 
definitely worth it - and actually pretty easy, even more if you`re 
working alone.

[1] https://git-scm.com/docs/git-worktree


Re: Need help migrating workflow from svn to git.

2017-12-14 Thread Igor Djordjevic
Hi Josef,

I`m not a Git expert, and I know less of Subversion, but following 
your explanation, I might try to help, at least until more 
experienced people join.

On 14/12/2017 14:09, Josef Wolf wrote:
> 
> Every machine has a working copy of the repository in a specific 
> directory. A cron job (running every 15 minutes) executes "svn
> update" and executes the scripts which are contained in this working
> copy.
> ...
> Sometimes, I need to fix a problem on some host or need to implement
> a new feature. For this, I go to the working copy of a host where the
> change needs to be done and start haking. With svn, I don't need to
> stop the cron job. "svn update" will happily merge any in-coming
> changes and leave alone the files which were not modified upstream.
> Conflicts with my local modifications which I am currently hacking on
> are extremely rare, because the scripts are pretty much independent.
> So I'm pretty much happy with this mode of operation.

Aside "update and merge" working copy while you`re hacking on it, 
what happens with "execute" part? It seems really strange that you 
don`t mind cron job running the same scripts which you are actively 
working on, thus being in an inconsistent state, if not broken, even.

> With git, by contrast, this won't work. Git will refuse to pull
> anything as long as there are ANY local modifications.

Not sure what`s happening at your end, but "ANY" part shouldn`t be 
true - you can have local modifications and still execute `git pull` 
successfully.

Only if you have local modifications in files that _also_ changed on 
the remote end, `git pull` aborts (fetch of the remote branch 
succeeds, actually, just merge with local branch is aborted).

Now, having in mind you said conflicts are extremely rare in your 
flow anyway, would this be enough for you? Of course, provided that 
issue you`re having with being unable to `git pull` with ANY local 
modifications, as you wrote, is resolved first.

> The cron job would need to
> 
>git stash
>git pull
>git stash pop
> 
> But this will temporarily remove my local modifications. If I happen
> to do a test run at this time, the test run would NOT contain the
> local modifications which I was about to test. Even worse: if I
> happen to save one of the modified files while the modifications are
> in the stash, the "git stash pop" will definitely cause a conflict,
> although nothing really changed.

Is `git stash pop` causing conflicts your only concern here? How 
about a situation where you save one of the modified files _after_ 
`git stash pop` was successful, effectively discarding any updates 
introduced by `git pull` from remote end...?

As you basically have a flow where two users (you and cron job) can 
edit same files at the same time, desired outcome might be a bit 
ambiguous, especially when scheduled execution of those files is 
added to the mix.

> So, how would I get this workflow with git? Is it possible to emulate
> the behavior of "svn update"?
> 
> Any ideas?

I`m thinking of a workflow involving (scripted) creation of a 
temporary branch at fetched remote branch position, and using 
something like `git checkout --merge ` to merge your 
local modifications to latest changes fetched from remote (ending up 
with conflicts inside working tree, if any), which would seem to 
simulate `svn update` as desired (if I understand it correctly), but 
it might be good to address some of the concerns I raised above first.

Regards, Buga


Re: Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Igor Djordjevic
Hi Thomas,

On 14/12/2017 00:14, Thomas Gummerer wrote:
> 
> > For what it`s worth, using `git stash save ` instead seems
> > to (still) work as expected...
> 
> I think that depends on what you expect ;) 'git stash save ' 
> will create a stash of the whole working directory with the message 
> "". So while it would indeed work for the presumably 
> simplified example Reid provided, it would not do what you'd expect
> if there are any tracked and modified files outside of the .
> 
> In that case 'git stash save ' would include the tracked
> files outside of , while what I assume Reid wanted is to keep
> them in place, and only stash the files in .

Indeed, I didn`t pay enough attention to the fact that even `git 
stash save\push` produced different output messages, the difference 
being exactly automatic (push) versus provided (save) stash message.

And I did use `git stash save ` in the past... :$ Not too 
often, I guess.

> > but on the other hand, `git-stash`[1] manpage seems not to mention
> > this usage ("save" with "pathspec")?
> 
> "stash save" with "pathspec" doesn't exist, and it will probably
> never exist. We decided to introduce a new "push" verb for 'git
> stash' because the command line for 'git stash save' takes a message
> as its last argument, instead of taking the message with a -m flag
> like other commands do. Introducing a pathspec argument for "git
> stash save" would have either broken backward compatibility, or it
> would have had some syntax that's very inconsistent with other git
> commands.

Yeah, I`m aware of the "transition", thus teaching myself to use `git 
stash push` lately. That`s also what made me curious to try out the 
"old" `git stash save` behavior, but obviously in a bit hasty manner.

Sorry for the confusion, and thanks, for both clarification 
and your work.

Regards, Buga


Re: Apparent bug in 'git stash push ' loses untracked files

2017-12-13 Thread Igor Djordjevic
Hi Reid,

On 13/12/2017 18:32, Reid Price wrote:
> 
> When running 'git stash push ' if there are both tracked and
> untracked files in this subdirectory, the tracked files are stashed
> but the untracked files are discarded.

I can reproduce this as well (git version 2.15.1.windows.2).

For what it`s worth, using `git stash save ` instead seems to 
(still) work as expected... but on the other hand, `git-stash`[1] 
manpage seems not to mention this usage ("save" with "pathspec")?

Regards, Buga

[1] https://git-scm.com/docs/git-stash


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking tree file changes)

2017-12-10 Thread Igor Djordjevic
Hi Philip,

On 10/12/2017 13:22, Phillip Wood wrote:
> 
> Sorry I should have been clearer. The point I was somewhat obliquely 
> making was that 'rebase --onto' succeeds where 'rebase --autosquash' 
> fails not because it is smarter but because it is doing something 
> different. Specifically it avoids the conflicting merge to create A'
> as the user has already created that commit in the temporary branch

No problem, and thanks for clarifying, I understand and agree to all 
that with you. I was just pointing that it wasn`t something I was 
commenting to (nor specially interested in), because of what Alexei 
actually wrote - here`s his quote (emphasis mine):

  "And then I often find that "rebase -i --autosquash" _fails to apply
  the commit B_ because it expects slightly different context around
  the changed lines."

>From there, it seemed pretty clear he perceived the failure not 
coming from creating A', but applying B on top of it, and that is 
what got my attention. But, read below...

> > - but you`re mentioning `git _commit_ --onto` instead, comparing it
> > with `rebase`... and which one of the two ("--autosquash", I
> > assume)?
> 
> Yes because in an earlier message you said
> 
> > If you mind enough to be bothered testing it out, might be even 
> > existing/initial state of originally proposed `git commit 
> > --onto-parent` script would work for you, as it does incorporate
> > some trivial three-way merge resolution.
> >
> > In your starting situation:
> >
> > ---A---B
> >
> >  you would just do something like:
> >
> > git commit --onto-parent A
> >
> >  hopefully ending up in the desired state (hopefully =
> > conflicts automatically resolved):
> >
> > ---A---C---B'
> 
> and I was pointing out that this would involve performing the same
> merge as 'rebase --autosquash' which has conflicts

Yeah, what I assumed (and agreed to), thanks for confirmation. What 
made me a bit uncertain was that you left that part of my earlier 
message quoted _after_ your inline reply to it, thus making overall 
context a bit difficult to be exactly sure in :P

> I understood Alexei to mean that it was merging the f!A into A that 
> caused conflicts due to the fact that f!A has conflicting context
> that was introduced in B. After all B' the rebased B is merge A A' B
> whether it is created by 'rebase --autosquash' or 'rebase --onto'. A'
> must be the same in both cases or one is applying a different fix.

Yes, I understand and agree you might be right, what you are talking 
about being what he actually _meant_, but because that is not what he 
_wrote_, I wanted to see an example of it, (still?) hoping that he 
really did mean what he wrote (commit B being the problematic one), 
as then there would be a possibility for improvement.

And your analysis seems correct, and that`s what I was afraid of as 
well - but wasn`t really sure, especially as I seem to remember 
something similar from my own (humble) experience, thus leaving a 
possibility for an example to prove differently.

But if that is absolutely impossible, as you claim, like not even due 
to some commit squashing, some edge case, or something - and I don`t 
feel like I have enough knowledge/experience to judge that myself at 
the moment - then you have to be right, and what he wrote is really 
not what he meant... nor what I thought I remembered from my own past 
experience, either :/ Nor there is any chance for improvement here, 
unfortunately, I guess.

Still, I hope for that example...! :D

> I've found conflicts arising from moving fixups can be quite common,
> so these days I tend to edit the commit to be fixed up directly. I
> have a script git-amend that does something like
> 
> target=$(git rev-parse --verify "$1") && GIT_SEQUENCE_EDITOR="sed -i
> s/^pick $target/edit $target/" rebase -ik $target^
> 
> so I can just type 'git amend ' to make this easier

This is useful, thanks. I have something like `git commit --amend 
` on my wish list for quite some time :) Still not getting to 
look into it, though.

> > In that (very?) specific case, proposed `git commit
> > --onto-parent`[1] doesn`t suffer from this, as once f!A is
> > successfully applied onto A (either squashed in with --amend, or on
> > top of it), we take original f!A _snapshot_ (not patch!) made on
> > top of B, and just "declare" it B` (being equal to B + f!A, which
> > we already know, and being correct), without a need to (try to)
> > apply B patch on top of fixed-up A to create B', as `rebase` does
> > (and fails).
> 
> Ah I understand, but that only works when you're fixing up HEAD~1.
> If you had A-B-C-f!A you have to recreate B with a merge.

Yes, and thus the notion of what he mentioned as being a "(very?) 
specific case" ;) That initial/draft version of "git commit 
--onto-parent" script I sent to the list[1] operates on the first 
parent commit only, indeed, though its main point/purpose had nothing 
to do with smarter merges, but just not touching the 

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, noworking tree file changes)

2017-12-09 Thread Igor Djordjevic
Hi Philip,

On 09/12/2017 20:01, Phillip Wood wrote:
> 
> > But thanks for clarifying, anyway, it does feel like `git rebase
> > -i --autosquash` could be smarter in this regards, if `git rebase 
> > --onto` does it better...?
> 
> Creating the fixup directly on A rather than on top of B avoids the 
> conflicting merge B f!A A. Creating the fixup on top of B and then
> using git commit --onto A would suffer from the same conflicts as
> rebase does.

I`m a bit confused here, as you`re replying to the part where we 
strictly discussed `rebase --autosquash` versus `rebase --onto`, 
having the latter succeed where the former fails - but you`re 
mentioning `git _commit_ --onto` instead, comparing it with `rebase`... 
and which one of the two ("--autosquash", I assume)?

Even further, while I do seem to understand (and agree with) what 
you`re talking about with `commit --onto` and `rebase --autosquah` 
suffering from the same conflicts in attempt to take f!A, originally 
created on top of B, and apply it on top of A - the thing is that 
Alexei actually pointed to B being the problematic one, failing to 
rebase on top of already (successfully) autosquashed A' (where A' = A 
+ f!A, fixup applied through --autosquash), while it doesn`t fail 
rebasing --onto f!A when f!A is being committed on top of A directly 
(and not through --autosquash).

In that (very?) specific case, proposed `git commit --onto-parent`[1] 
doesn`t suffer from this, as once f!A is successfully applied onto A 
(either squashed in with --amend, or on top of it), we take original 
f!A _snapshot_ (not patch!) made on top of B, and just "declare" it 
B` (being equal to B + f!A, which we already know, and being 
correct), without a need to (try to) apply B patch on top of fixed-up 
A to create B', as `rebase` does (and fails).

> I don't think there is any way for 'git rebase --autosquash' to
> avoid the conflicts unless it used a special fixup merge strategy
> that somehow took advantage of the DAG to resolve the conflicts by
> realizing they come from a later commit. However I don't think that
> could be implemented reliably as sometimes one wants those
> conflicting lines from the later commit to be moved to the earlier
> commit with the fixup.

I think I agree on this part being tricky (if possible at all), but I 
also think this is not what Alexei was complaining about, nor what we 
were discussing (as I tried to explain above) - but please do correct 
me if I misunderstood you.

That said, and what I mentioned already, we might really benefit from 
simple test case(s), showing "rebase --autosquash" failing where 
"rebase --onto" works, as Alexei explained, giving some more (and 
firm) context to the discussion.

I *think* I`ve experienced this in the past myself, but now I can`t 
seem to wrap my head around a reproducible example just yet... :$

Regards, Buga

[1] 
https://public-inbox.org/git/4a92e34c-d713-25d3-e1ac-100525011...@talktalk.net/T/#m72f45ad7a8f1c733266a875bca087ee82cc781e7


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-12-08 Thread Igor Djordjevic
Hi Alexei,

On 09/12/2017 03:18, Alexei Lozovsky wrote:
> 
> > Chris reported in this very topic[1] that sometimes, due to
> > conflicts with later commits, "checkout > commit > [checkout >]
> > rebase --onto" is "much easier to do", where "commit --fixup >
> > rebase -i" "breaks" (does not apply cleanly).
> 
> It was more of a rant about conflict resolution by rebase rather than
> a concern about modification time of files. While I'd prefer git to
> not touch the source tree unnecessarily, it's not really a big deal
> for me if it does and some parts of the project need to be rebuilt.

Nevertheless, I found it valuable in supporting the case where 
"commit --fixup > rebase -i" seems to require even more work than 
otherwise necessary :)

But thanks for clarifying, anyway, it does feel like `git rebase -i 
--autosquash` could be smarter in this regards, if `git rebase 
--onto` does it better...?

Even though your explanation seems clear, having a real, easily 
reproducible case would help as well, I guess.

> I kinda hoped that you may know this magic and incorporate it into 
> "commit --onto" which will allow to immediately get to the result of 
> the rebase:
> 
>   ---A---f!A---B'
> 
> without spelling it all manually.

If you mind enough to be bothered testing it out, might be even 
existing/initial state of originally proposed `git commit 
--onto-parent` script would work for you, as it does incorporate some 
trivial three-way merge resolution.

In your starting situation:

---A---B

... you would just do something like:

git commit --onto-parent A

... hopefully ending up in the desired state (hopefully = conflicts 
automatically resolved):

---A---C---B'

You could even do this instead:

git commit --onto-parent A --amend

... ending up with:

---A'---B'

... as that is basically what you wanted in the first place ;)

> (And yeah, I'm actually Alexei, not Chris. That was my MUA being
> dumb and using an old pseudonym than Google insists I'm called by.)

Ah, sorry for the confusion :)

Regards, Buga


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-12-08 Thread Igor Djordjevic
On 08/12/2017 17:24, Junio C Hamano wrote:
> 
> > To get back on track, and regarding what`s already been said,
> > would having something like this(1) feel useful?
> >
> > (1) git commit --onto 
> 
> Are you asking me if _I_ find it useful?  It is not a very useful
> question to ask, as I've taken things that I do not find useful
> myself.

It was also (kind of shy and subtle) "would you take it?", indeed, 
but I do value your personal opinion here, too, being a recognized 
developer, and one really knowing the Git (mailing list) community on 
top of it, so I appreciate you addressed both sides of the question.

And it was partly addressed to Hannes, but more for a confirmation, I 
guess, him being the one to favor such a flow in the first place, 
over what I initially suggested.

> Having said that, I do not see me personally using it. You keep
> claiming that committing without ever materializing the exact state
> that is committed in the working tree is a good thing.
> 
> I do not subscribe to that view.  

No - and I find it an important difference to note - just that it 
might be acceptable / more preferable _in certain situations_, where 
the only alternative seems to be wasting (significant) amount of time 
on needless rebuilds of many files (just because of branch switching, 
otherwise untouched by the changes we`re interested in).

If this is perceived a too uncommon/exotic case to worth addressing 
is a different matter, though.

> I'd rather do a quick fix-up on top (which ensures that at least the
> fix-up works in the context of the tip), and then "rebase -i" to
> move it a more appropriate place in the history (during which I have
> a chance to ensure that the fix-up works in the context it is
> intended to apply to).

Chris reported in this very topic[1] that sometimes, due to conflicts 
with later commits, "checkout > commit > [checkout >] rebase --onto" 
is "much easier to do", where "commit --fixup > rebase -i" "breaks" 
(does not apply cleanly).

> I know that every time I say this, people who prefer to commit
> things that never existed in the working tree will say "but we'll
> test it later after we make these commit without having their state
> in the working tree".  But I also know better that "later" often do
> not come, ever, at least for people like me ;-).

No comment here ;)

> The amount of work _required_ to record the fix-up at its final
> resting place deeper in the history would be larger with "rebase -i"
> approach, simply because approaches like "commit --onto" and "git
> post" that throw a new commit deep in the history would not require
> ever materializing it in the working tree.  But because I care about
> what I am actually committing, and because I am just lazy as any
> other human (if not more), I'd prefer an apporach that _forces_ me
> to have a checkout of the exact state that I'd be committing.  That
> would prod me to actually looking at and testing the state after the
> change in the context it is meant to go.

All that I agree with, too.

But that said, I do find `git add --patch` invaluable (for example), 
where one can still opt to commit right away (and test later ;)), or 
do a proper `git stash push --keep-index` first in order to actually 
check/test the exact state/context before committing.

One of the biggest advantages I see in using Git is that it provides 
so many possibilities, where there is not necessarily a single 
"correct" way to do something - depending on the (sub)context, the 
decision on "_the_ correct" way can be deferred to the user himself.

Git (usually) does not judge, except in cases where something is 
considered "plain wrong" - still different than "might not be the 
best approach", but not necessarily a wrong one, either.

But I do realize it also means more chances for beginner users to 
shoot themselves in the foot, blaming it on Git, so even if just a 
matter of personal taste, a more restrictive preference from the Git 
maintainer is understandable :)

Regards, Buga

[1] 
https://public-inbox.org/git/CAPc5daWupO6DMOMFGn=xjucg-jmyc4eyo8+tmasdwcaohxz...@mail.gmail.com/T/#m989306ab9327e15f14027cfd74ae8c5bf487affb


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-12-07 Thread Igor Djordjevic
On 06/12/2017 19:34, Johannes Sixt wrote:
> 
> I am sorry for not responding in detail. I think we've reached a 
> mutual understanding of our workflows.

No problem, thanks for your time so far.

There might be one more thing I should address, possibly left unclear 
from my previous message, but I`ll leave that for a follow-up e-mail, 
not being that important at the moment for the topic itself.

On 06/12/2017 19:40, Junio C Hamano wrote:
> 
> > Though, from the ideas you tossed around most recently, you seem to
> > want to make git-commit into a kitchen-sink for everything. I have
> > my doubts that this will be a welcome change. Just because new
> > commits are created does not mean that the feature must live in
> > git-commit.
> 
> Nicely put.

Yeah, I understand that might have felt cluttering, besides also 
being out of scope of the original topic idea. Thanks for the reality 
check (to both).

To get back on track, and regarding what`s already been said, would 
having something like this(1) feel useful?

(1) git commit --onto 

So in previously mentioned situation:

(2) ...A...C<- topics A, C
\   \
  ---o---o---o---o I<- integration <- HEAD
/   /
...B...D<- topics B, D

... it would allow committing changes F inside HEAD on top of B 
directly, no checkout / branch switching needed, getting to:

(3) ...A...C<- topics A, C
\   \
  ---o---o---o---o I<- integration <- HEAD
/   /
...B...D<- topic D
\
 F  <- topic B

So the most conservative approach, where changes F are removed from 
HEAD index and working tree, leaving it up to the user to decide if 
he will then merge them back in (or do something else).

I stress the major selling point here still being avoiding branch 
switching back and forth in order to commit a fixup on a different 
branch, which could otherwise trigger needless rebuilds, being 
significant in large projects.

And thanks to that `git-merge-one-file--cached`[1] script, we are 
also able to resolve some more of trivial conflicts when applying F 
onto B, using three-way file merge when needed, but still not 
touching working tree (contrary to original `git-merge-one-file`).

Regards, Buga

[1] 
https://public-inbox.org/git/CAPc5daWupO6DMOMFGn=xjucg-jmyc4eyo8+tmasdwcaohxz...@mail.gmail.com/T/#mcb3953542dc265516e3ab1bff006ff1b5b85126a


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-12-03 Thread Igor Djordjevic
Hi Hannes,

On 01/12/2017 18:23, Johannes Sixt wrote:
> 
> > To work with `--onto-parent` and be able to commit on top of any of
> > the topic branches, you would need a situation like this instead:
> >
> >   (1)  ...C  <- topic C
> >   |
> >  ...A |  <- topic A
> >  \|
> >...o I<- integration
> >  /|
> >  ...B |  <- topic B
> >   |
> >...D  <- topic D
> 
> This is a very, VERY exotic workflow, I would say. How would you
> construct commit I when three or more topics have conflicts?
> merge-octopus does not support this use-case.

But I`m not interested in constructing commit I in the first place, 
only help working with it once it`s already there (which shouldn`t be 
too uncommon in case of unrelated, non-conflicting topics) - but I 
already agreed my example could have been a bit too esoteric, 
distracting attention from the important part :)

I`ll continue on this further below, where you commented on those 
more common scenarios.

Here, let`s try to look at the whole situation from a "patch queue" 
perspective instead, starting from something like this:

(5) ---O---I  <- integration <- HEAD

... and then making progress like this - first, commit A1 onto O, 
starting "topicA" branch at the same time, even, maybe using syntax 
like `git commit --onto-parent O -b topicA`:

(6) ---O---J  <- integration <- HEAD   [ J = I + A1 ]
\ /
 A1   <- topicA

..., then commit B1 onto O to start "topicB" branch:

(7)  B1   <- topicB
/ \
---O---K  <- integration <- HEAD   [ K = J + B1 ]
\ /
 A1   <- topicA

..., then add one more commit (patch) onto B1:

(8)  B1---B2   <- topicB
/  \
---OL  <- integration <- HEAD  [ L = K + B2 ]
\  /
 A1---/<- topicA

..., and one more, B3:

(9)  B1---B2---B3   <- topicB
/   \
---O-M  <- integration <- HEAD [ M = L + B3 ]
\   /
 A1/<- topicA

We can also start a new topic branch (queue), commit C1:

(10) B1---B2---B3   <- topicB
/   \
---O-N  <- integration <- HEAD [ N = M + C1 ]
   |\   /|
   | A1/ /  <- topicA
   \/
C1-/<- topicC

And lets make one more "topicA" related commit A2:

(11) B1---B2---B3   <- topicB
/   \
---O-P  <- integration <- HEAD [ P = N + A2 ]
   |\   /|
   | A1---A2---/ /  <- topicA
   \/
C1-/<- topicC


Notice how HEAD never leaves "integration" branch, and underlying 
commit is recreated each time? It`s like a live branch we`re working 
on, but we`re not actually committing to.

No branch switching (and no working tree file changes caused by it), 
and we`re working on multiple topics/branches simultaneously, being 
able to instantly test their mutual interaction as we go, but also 
creating their separate (and "clean") histories at the same time.

I guess this would make most sense with simple topics we _could_ 
practically work on at the same time without making our life too 
complicated - what stands for "git add --patch", too, when working on 
multiple commits at the same time.

Once satisfied, of course each topic branch would need to be tested 
separately, and each commit, even - all the same as with "git add 
--patch" commits.

And "git add --patch" can still be used here, too, to distribute 
partial changes, currently existing together inside the working tree, 
to different topic branches, at the time of making the commit itself.

Does this approach make more sense in regards to "git commit 
--onto-parent" functionality I`m having in mind? Or I`m dreaming too 
much here...? :)

> > Once there, starting from your initial position:
> >
> > > ...A...C<- topics A, C
> > > \   \ E
> > >   ---o---o---o---o I<- integration <- HEAD
> > > /   /
> > > ...B...D<- topics B, D
> >
> > ... and doing something like `git commit --onto B --merge` would
> > yield:
> > 
> > (3) ...A...C<- topics A, C
> >   \   \ E
> > ---o---o---o---o I'   <- integration
> >   /   /|
> >   ...B...D |  <- topic D
> >   \|
> >f---'  <- topic B
> >
> > ... where (I' = I + f) is still true.
> 
> I am not used to this picture. I would not think that it is totally
> unacceptable, but it still has a hmm-factor.

Main idea is not to pile up uninteresting merge commits inside 
(throwaway) integration branch, needlessly complicating history, but 
pretend as we just made the integration merge, where fixup commit f 
was already existing in its topic branch prior the merge.

> > If that`s preferred in some
> > cases, it could even look like this 

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-12-03 Thread Igor Djordjevic
Hi Chris,

On 30/11/2017 23:40, Chris Nerwert wrote:
> 
> I'm actually doing the described workflow quite often with git rebase
> when working on a topic. Given the following structure:
> 
>   ---o   (master)
>   \
>o---A---B---C (topic)
> 
> When I want to make changes to commit A one option is to make them
> directly on topic, then do "git commit --fixup A", and then eventual
> interactive rebase onto master will clean them up:
> 
>   ---o (master)
>   \
>o---A---B---C---f!A (topic)
> 
> However, sometimes this breaks when changes in B or C conflict
> somehow with A (which may happen quite a lot during development of a
> topic), so the rebase will not apply cleanly. So sometimes I make a
> temporary branch from A, commit the fixup there:
> 
>   ---o   (master)
>   \
>o---A---B---C (topic)
> \
>  f!A (temp)
> 
> and then use "git rebase --onto temp A topic" to move the topic back
> on track:
> 
>   ---o (master)
>   \
>o---A---f!A (temp)
>   \
>B'---C' (topic)
> 
> after which the final cleanup rebase is much easier to do.
> 
> Obviously, all the branch switching and rebasing does take its tall
> on file modifications.

>From use case you described (and which I often experience myself), it 
seems plain "git commit --onto A" would be of help here, committing 
fixup onto A directly, without a need to switch to it (branch or 
not), a case I`m discussing with Hannes in that other sub-thread[1] of 
this e-mail, too.

But from there, your flow takes a different direction, using rebase, 
while this whole thread started around some merge-like functionality.

I can imagine a user interface doing what you (and I) would like, 
something like:

(1) git commit --onto A --rebase

..., where your changes would first be committed onto commit A, and 
then commits from A (excluded) to HEAD (included) rebased onto this 
new commit.

BUT, as far as it seems to me, rebase currently touches working tree 
for each operation (am I wrong here?), so once the rebase sequence is 
initiated, it would internally still need to checkout to your new 
fixup commit (on top of A), and then proceed applying changes and 
changing working tree with each commit being rebased, overall failing 
to address your main concern - needless (untouched) file 
modifications, even in case of no conflicts.

I find this scenario quite interesting as well, but I`m afraid it may 
currently be out of scope of what I`m trying to accomplish with "git 
commit --onto[-parent]", for the most part because it looks like it 
would need "index only rebase" first (not touching working tree, that 
is)...?

If we had that, it would/should be pretty easy to add it into the mix 
with "git commit --onto" here, ending up with something as imagined 
in command line (1) above :) I`ll make a note of it, thanks.

Any further help appreciated, of course :)

Regards, Buga

[1] 
https://public-inbox.org/git/0f30bef8-a9f7-43b9-bc89-4b9cd7af3...@gmail.com/T/#me830a80d745df60ae8bd6a2e67eee4bd4dabf56c


Re: [ANNOUNCE] Git for Windows 2.15.1

2017-11-29 Thread Igor Djordjevic
Hi Johannes,

On 29/11/2017 14:57, Johannes Schindelin wrote:
> 
>   * It is now possible to configure nano or Notepad++ as Git's
> default editor instead of vim.

This seems as a really nice option, as it could\should greatly help 
Windows people in lowering friction in first encounter with Git (for 
Windows).

Being pretty unfamiliar with Linux and its tools at the time, I 
remember the initial frustration in trying to do what otherwise felt 
as a no-brain, simple and trivial task - write the damn commit 
message after `git commit`, lol. Even had to kill the bash window a 
few times, not knowing what to do, where it was clear it was 
expecting something from me :$

I later learned about vim, like getting started with Git wasn`t hard 
enough... :) As soon as I found it being a possibility, I`ve set 
Notepad++ as my default editor.

That said, what is the Notepad++ as default editor option doing, just 
setting:

[core]
editor = 'F:/Install/Notepad++/notepad++.exe' -multiInst -notabbar 
-nosession

... inside users` .gitconfig (`git config --global`)? Asking because 
I already had it there, and seems the option made no difference, so 
I`m not sure if it actually worked.

Otherwise, I guess I can dig the answer up from the installer code as 
well... :)

Thanks for yet another Git for Windows release.

Regards, Buga

p.s. Ok, It seems it actually went to `git config --system` instead, 
like:

[core]
editor = 'F:\\Install\\Notepad++\\notepad++.exe' -multiInst -notabbar 
-nosession -noPlugin

I removed my original (user) configuration, and it still works (minus 
plugins, due to that last parameter), so all good :)

Plugins do come in handy for me, though, like spell-check when 
writing commit messages, so I`ll just stick with my option for now.


Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-11-29 Thread Igor Djordjevic
Hi Hannes,

On 29/11/2017 20:11, Johannes Sixt wrote:
> 
> Ok, then please explain, how this process should work in my workflow
> and with the `commit --onto-parent` feature that you have in mind. I
> have an integration branch (which is a throw-away type, so you can
> mangle it in any way you want); it is a series of merges:
> 
>  ...A...C<- topics A, C
>  \   \ E
>---o---o---o---o I<- integration
>  /   /
>  ...B...D<- topics B, D
> 
> Now I find a bug in topic B. Assume that the merges of C and D have
> textual conflicts with the integration branch (but not with B) and/or
> may be evil. What should I do?
> 
> With git-post, I make a fixup commit commit on the integration
> branch, then `git post B && git merge B`:
> 
>  ...A...C  <- topics A, C
>  \   \
>---o---o---o---o---f---F<- integration
>  /   /   /
>  ...B...D   /  <- topic D
>  \ /
>   f'--'<- topic B
> 
> The merge F does not introduce any changes on the integration branch,
> so I do not need it, but it helps keep topic B off radar when I ask
> `git branch --no-merged` later.

But you`re not committing (posting) on your HEAD`s (direct) parent in 
the first place (topic B), so `commit --onto-parent` isn`t right tool 
for the job... yet :)

To make it easier to explain, I marked your integration branch 
initial head with "I" in the quote above (commit merging-in branch 
D), and marked commit merging-in branch C with "E".

HEAD being currently on commit "I", you can only use `--onto-parent` 
option to commit onto "E" or "D", being parents of "I".

To work with `--onto-parent` and be able to commit on top of any of 
the topic branches, you would need a situation like this instead:

 (1)  ...C  <- topic C
 |
...A |  <- topic A
\|
  ...o I<- integration
/|
...B |  <- topic B
 |
  ...D  <- topic D

With `commit --onto-parent` you would skip `git post B && git merge 
B` steps, where "fixup commit" would be done with `--onto-parent B`, 
So you end up in situation like this:

 (2)  ...C  <- topic C
 |
...A |  <- topic A
\|
  ...o I'   <- integration
/|
...B---f |  <- topic B
 |
  ...D  <- topic D

State of index and working tree files in your F and my I' commit is 
exactly the same (I' = I + f), where in my case (2) history looks 
like "f" was part of topic B from the start, before integration 
merge happened.

BUT, all this said, I understand that your starting position, where 
not all topic branches are merged at the same time (possibly to keep 
conflict resolution sane), is probably more often to be encountered 
in real-life work... and as existing `--onto-parent` machinery should 
be able to support it already, I`m looking forward to make it happen :)

Once there, starting from your initial position:

>...A...C<- topics A, C
>\   \ E
>  ---o---o---o---o I<- integration <- HEAD
>/   /
>...B...D<- topics B, D

... and doing something like `git commit --onto B --merge` would yield:
 
 (3) ...A...C<- topics A, C
 \   \ E
   ---o---o---o---o I'   <- integration
 /   /|
 ...B...D |  <- topic D
 \|
  f---'  <- topic B

... where (I' = I + f) is still true. If that`s preferred in some 
cases, it could even look like this instead:

 (4) ...A...C <- topics A, C
 \   \ E  I
   ---o---o---o---o---F   <- integration
 /   /   /
 ...B...D   / <- topic D
 \ /
  f---'   <- topic B

... where F(4) = I'(3), so similar situation, just that we don`t 
discard I but post F on top of it.

Good thing is all necessary logic should already be in place, I just 
need to think a bit about the most sane user interface, and get back 
to you. Thanks for invaluable input so far :)

Of course, do feel free to drop any ideas you come up with as well, 
on how `git commit` user interface/options leading to (3) or (4) 
should look like (they could both be supported).

> > Like working on multiple branches at the same time, in the manner
> > similar to what `git add --patch` allows in regards to working on
> > multiple commits at the same time. This just takes it on yet another
> > level... hopefully :)
> 
> 'kay, I'm not eagerly waiting for this particular next level (I
> prefer to keep things plain and simple), but I would never say this
> were a broken workflow. ;)

Hehe, thanks, I guess :) Simplicity, but user-oriented, is the major 
point here, where you can work on unrelated patch series at the same 
time (patch queues?), without a need to switch branches, while you 
still 

Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-11-27 Thread Igor Djordjevic
Hi Johannes,

On 27/11/2017 22:54, Johannes Sixt wrote:
> 
> I my opinion, putting the focus on integration merge commits and the
> desire to automate the re-merge step brings in a LOT of complexity in
> the implementation for a very specific use-case that does not
> necessarily help other cases.

It might seem more complex than it is, until you examine the guts to 
see how it really works :)

Basic concept is pretty simple, as I actually don`t automate 
anything, at least not in terms of what would manual user steps look 
like - for example, there`s no real re-merge step in terms of 
actually redoing the merge, I just reuse what was already there in a 
very clean way, I would think (supported by my current, humble 
knowledge, still).

The only merge that could possibly ever happen is upon committing 
desired subset of changes onto parent, and that shouldn`t be too 
complex by definition, otherwise that commit doesn`t really belong 
there in the first place, if it can`t be meaningfully applied where 
we want it (for now, at least).

That said, the whole operation of "posting on parent and re-merging 
everything", the way it looks like from the outside, could end just 
with a simple diff-apply-commit-commit internally, no merges at all. 
Only if simple `git apply` fails, we try some trivial merging - and 
all that inside separate (parent) index only, not touching original 
HEAD index nor working tree, staying pristine for the whole process, 
untouched.

Once done, you should be in the very same situation you started from, 
nothing changed, just having your history tweaked a bit to tell a 
different story on how you got there (now including a commit you 
posted on your HEAD`s parent).

Otherwise, I agree that explained use case might be a bit specific, 
but that is only because I recognized that one to be the most 
interesting to initially present (not to say one of more complex 
cases) - to me, at least, but it is certainly not the only one.

Don`t let "usual/preferred/recommended" Git workflow distract you too 
much - one of the reasons I made this is because it also allows _kind 
of_ "vanilla Git" patch queue, where you can quickly work on top of 
the merge head, pushing commits onto parents below, being tips of 
your "queues", putting you up to speed without a need to ever switch 
a branch (hypothetically), until satisfied with what you have, where 
you can slow down and polish each branch separately, as usual.

Like working on multiple branches at the same time, in the manner 
similar to what `git add --patch` allows in regards to working on 
multiple commits at the same time. This just takes it on yet another 
level... hopefully :)

> For example, in my daily work, I have encountered situations where,
> while working on one topic, I made a hot-fix for a different topic.
> There is no demand for a merge step in this scenario.
> 
> In your scenario above, it would certainly not be too bad if you
> forgo the automatic merge and have the user issue a merge command
> manually. The resulting history could look like this:
> 
> (3) o---o---A---X(topicA)
>/ \   \
>   /   M1--M2 (test, HEAD)
>  /   /||
>  ---o---o---M---' || (master)
>  \   \   / |
>   \   o-B /  (topicB)
>\ /
> o---o---C(topicC)
> 
> I.e., commit --onto-parent A produced commit X, but M2 was then a
> regular manual merge. (Of course, I am assuming that the merge
> commits are dispensible, and only the resulting tree is of
> interest.)

I see - and what you`re asking for is what I already envisioned and 
hoped to get some more feedback about, here`s excerpt from 
[SCRIPT/RFC 3/3] git-commit--onto-parent.sh[1] (I guess you didn`t 
have time to checked that one yet?):

  For example, it might make sense to separate commit creation (on 
  current HEAD`s parent) and its actual re-merging into integration 
  test branch, where "--remerge" (or something) parameter would be used 
  on top of "--onto-parent" to trigger both, if/when desired.
  
  Another direction to think in might be introducing more general 
  "--onto" parameter, too (or instead), without "parent" restriction, 
  allowing to record a commit on top of any arbitrary commit (other 
  than HEAD). This could even be defaulted to "git commit " 
  (no option needed), where current "git commit" behaviour would then 
  just be a special case of omitted  defaulting to HEAD, 
  aligning well with other Git commands sharing the same behaviour.

So I definitely look forward decoupling these two ((1) commit to 
parent and (2) remerge), with enough discussion flowing :)

Heck, even "to parent" is an artificial/imposed restriction now, in 
reality you could commit on top of any other commit you want (without 
switching branches)... but let`s take one step at a time.

Just note that omitting the remerge step is what actually makes the 
logic more complex, as we 

[SCRIPT/RFC 1/3] setup.sh

2017-11-26 Thread Igor Djordjevic
On 26/11/2017 23:35, Igor Djordjevic wrote:
> 
> This is what we end up with once "master" and topic branches are 
> merged in merge commit M1 inside temporary "test" branch for further 
> integration testing:
> 
> (2)o---o---A (topicA)
>   / \
>  /   M1 (test, HEAD)
> /   /||
> ---o---o---M---/ || (master)
> \   \   / |
>  \   o---B-/  | (topicB)
>   \   |
>o---o---C--/ (topicC)

To begin with, you can use provided "setup.sh"[*1*] script, putting 
you straight into position shown on graph (2) above, with addition of 
tag "A" and remote branch "origin/topicA" so you could try using 
these as "--onto-parent" values, too.

As seen in there, change "X" is already made and staged, so you can 
now just run something like:

git-commit--onto-parent.sh --onto-parent topicA

... to see the logic in action.
 
Instead of "topicA", you may try providing tag "A", remote branch 
"origin/topicA" or even plain commit hash. It`s interesting to add 
"--amend" option into the mix, too, and see what happens. Also, you 
can try using "topicB" and see the commit fail (as it doesn`t merge 
cleanly).

All this while "test.txt" file doesn`t get modified on disk (nor 
would any other file) - being desired behaviour, as we didn`t 
actually change anything inside the working tree, but just amended 
history of how we got here, so recompilation isn`t needlessly 
triggered :)

p.s. Note these two lines near the end:

sed -i '4iX1' test.txt  # works with simple patch apply
sed -i '17iX2' test.txt # needs three-way file merge

You can play with it, commenting out one or the other and observing 
how it influences "git commit --onto-parent" in regards of the parent 
provided.

Regards, Buga

[*1*] "setup.sh", can clean previous setup run as well, but commented 
 out here for safety, not to unexpectedly delete something for unwary 
 user.
--- 8< ---
#!/bin/sh

#rm -rf ./.git
#rm -f ./test.txt

git init

touch ./test.txt
git add -- test.txt

for i in {1..10}
do
echo $i >>test.txt  
git commit -am "$i"
done

echo M >>test.txt
git commit -am "M"

git checkout -b topicA HEAD~2

for i in 1 2
do
sed -i "${i}iA${i}" test.txt
git commit -am "A$i"
done
sed -i '3iA' test.txt
git commit -am "A"
git tag A

# simulate remote branch
mkdir -p ./.git/refs/remotes/origin &&
echo $(git rev-parse HEAD^0) >$_/topicA

git checkout -b topicB master^

sed -i '4iB1' test.txt
git commit -am "B1"
sed -i '5iB' test.txt
git commit -am "B"

git checkout -b topicC master~2

for i in 1 2
do
j=`expr "$i" + 5`
sed -i "${j}iC${i}" test.txt
git commit -am "C$i"
done
sed -i "8iC" test.txt
git commit -am "C"

git checkout -b test master
git merge --no-edit topicA topicB topicC

sed -i '4iX1' test.txt  # works with simple patch apply
sed -i '17iX2' test.txt # needs three-way file merge
git add -- test.txt

echo
git log --all --decorate --oneline --graph
echo
git diff --cached


[SCRIPT/RFC 2/3] git-merge-one-file--cached

2017-11-26 Thread Igor Djordjevic
Original "git-merge-one-file" script is slightly tweaked here into 
"git-merge-one-file--cached"[*1*] to allow (still trivial) _index-only_ 
three-way file merge, not touching files inside working tree.

Proof of concept, not thoroughly tested, original content left in, 
commented out. Feel free to comment/polish.

To make it available, I guess it would be best placed beside existing 
"git-merge-one-file" script...?

Regards, Buga

[*1*] "git-merge-one-file--cached"
--- 8< ---
#!/bin/sh
#
# Copyright (c) Linus Torvalds, 2005
#
# ---
# Original "git-merge-one-file" script slightly tweaked into
# "git-merge-one-file--cached" to allow (still trivial) index-only
# three-way file merge, not touching files inside working tree.
#
# Proof of concept, not thoroughly tested, original content left in,
# commented out.
# ---
#
# This is the git per-file merge script, called with
#
#   $1 - original file SHA1 (or empty)
#   $2 - file in branch1 SHA1 (or empty)
#   $3 - file in branch2 SHA1 (or empty)
#   $4 - pathname in repository
#   $5 - original file mode (or empty)
#   $6 - file in branch1 mode (or empty)
#   $7 - file in branch2 mode (or empty)
#
# Handle some trivial cases.. The _really_ trivial cases have
# been handled already by git read-tree, but that one doesn't
# do any merges that might change the tree layout.

USAGE='   '
USAGE="$USAGE   "
LONG_USAGE="usage: git merge-one-file $USAGE

Blob ids and modes should be empty for missing files."

SUBDIRECTORY_OK=Yes
. git-sh-setup
cd_to_toplevel
require_work_tree

if test $# != 7
then
echo "$LONG_USAGE"
exit 1
fi

case "${1:-.}${2:-.}${3:-.}" in
#
# Deleted in both or deleted in one and unchanged in the other
#
"$1.." | "$1.$1" | "$1$1.")
if { test -z "$6" && test "$5" != "$7"; } ||
   { test -z "$7" && test "$5" != "$6"; }
then
echo "ERROR: File $4 deleted on one branch but had its" >&2
echo "ERROR: permissions changed on the other." >&2
exit 1
fi

if test -n "$2"
then
echo "Removing $4"
# else
# read-tree checked that index matches HEAD already,
# so we know we do not have this path tracked.
# there may be an unrelated working tree file here,
# which we should just leave unmolested.  Make sure
# we do not have it in the index, though.
# exec git update-index --remove -- "$4"
fi
# if test -f "$4"
# then
# rm -f -- "$4" &&
# rmdir -p "$(expr "z$4" : 'z\(.*\)/')" 2>/dev/null || :
# fi &&
exec git update-index --remove --cacheinfo "$6","$2","$4"
;;

#
# Added in one.
#
".$2.")
# the other side did not add and we added so there is nothing
# to be done, except making the path merged.
exec git update-index --add --cacheinfo "$6","$2","$4"
;;
"..$3")
echo "Adding $4"
if test -f "$4"
then
echo "ERROR: untracked $4 is overwritten by the merge." >&2
exit 1
fi
git update-index --add --cacheinfo "$7","$3","$4" # &&
# exec git checkout-index -u -f -- "$4"
;;

#
# Added in both, identically (check for same permissions).
#
".$3$2")
if test "$6" != "$7"
then
echo "ERROR: File $4 added identically in both branches," >&2
echo "ERROR: but permissions conflict $6->$7." >&2
exit 1
fi
echo "Adding $4"
git update-index --add --cacheinfo "$6","$2","$4" # &&
# exec git checkout-index -u -f -- "$4"
;;

#
# Modified in both, but differently.
#
"$1$2$3" | ".$2$3")

case ",$6,$7," in
*,12,*)
echo "ERROR: $4: Not merging symbolic link changes." >&2
exit 1
;;
*,16,*)
echo "ERROR: $4: Not merging conflicting submodule changes." >&2
exit 1
;;
esac

src1=$(git unpack-file $2)
src2=$(git unpack-file $3)
case "$1" in
'')
echo "Added $4 in both, but differently."
orig=$(git unpack-file e69de29bb2d1d6434b8b29ae775ad8c2e48c5391)
;;
*)
echo "Auto-merging $4"
orig=$(git unpack-file $1)
;;
esac

git merge-file "$src1" "$orig" "$src2"
ret=$?
msg=
if test $ret != 0 || test -z "$1"
then
msg='content conflict'
ret=1
fi

# Create the working tree file, using "our tree" version from the
# index, and then store the result of the merge.
# git checkout-index -f --stage=2 -- "$4" && cat "$src1" >"$4" || exit 1
# rm -f -- "$orig" "$src1" "$src2"

if test "$6" != "$7"
   

[SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)

2017-11-26 Thread Igor Djordjevic
Hi to all,

Here`s my humble attempt on (once more) scratching a use case which 
seems to be desired occasionally through the years, obviously not 
enough to be actually implemented yet, but might be worth some more 
attention... :)

For the reference, some past mentions of (more or less) similar ideas 
are listed at the end of this e-mail[*1*], where I`m also Cc-ing 
people previously involved, might be still sharing interest - as this 
is my first "new thread" list message, I`m sorry if this is not an 
encouraged practice, please let me know.

Approach discussed here could have a few more useful applications, 
but one seems to be standing out the most - in case where multiple 
topic branches are temporarily merged for integration testing, it 
could be very useful to be able to post "hotfix" commits to merged 
branches directly, _without actually switching to them_ (and thus 
without touching working tree files), and still keeping everything 
merged, in one go.

Example starting point is "master" branch with 3 topic branches (A, 
B, C), to be (throwaway) merged for integration testing inside 
temporary "test" branch:

(1)o---o---A (topicA)
  /
 /
/
---o---o---M (master, test, HEAD)
\   \
 \   o---B (topicB)
  \
   o---o---C (topicC)


This is what we end up with once "master" and topic branches are 
merged in merge commit M1 inside temporary "test" branch for further 
integration testing:

(2)o---o---A (topicA)
  / \
 /   M1 (test, HEAD)
/   /||
---o---o---M---/ || (master)
\   \   / |
 \   o---B-/  | (topicB)
  \   |
   o---o---C--/ (topicC)


Upon discovery of a fix needed inside "topicA", hotfix changes X 
should be committed to "topicA" branch and re-merged inside merge 
commit M2 on temporary integration "test" branch (previous temporary 
merge commit M1 is thrown away as uninteresting):

(3)o---o---A---X (topicA)
  / \
 /   M2 (test, HEAD)
/   /||
---o---o---M---/ || (master)
\   \   / |
 \   o---B-/  | (topicB)
  \  /
   o---o---C/ (topicC)


Now, usually advised approach to get from (2) to (3), where one is 
expected to switch to desired topic branch, commit the change, and 
then re-merge everything again into integration testing branch, is 
arguably a bit tiresome, but where it actually fails short is the 
fact that branch switching back and forth (for each hotfix commit) 
could possibly keep changing a lot of files otherwise untouched by 
the hotfix changes we`re committing (but different between branches), 
causing build systems to needlessly waste what could be significant 
time compiling them again and again.

Example script proposes using something like this instead:

(4) git commit --onto-parent topicA
 
... in above-mentioned case (2) to commit X onto "topicA" branch 
directly, re-merging all previously merged integration testing topic 
branches at the same time, reaching state (3) without any 
intermediate branch switching (and without touching working tree, 
thus without needless recompilation).

Once integration tests pass, integration test branch will be thrown 
away and new commits on each topic branch should still be properly 
tested - we`re just deferring it not to do it in the middle of 
integration testing, saving some (or a lot) needless rebuild cycles 
at the same time (or in the first place, even).

Scripts in series:
  [1/3]: setup.sh
  [2/3]: git-merge-one-file--cached
  [3/3]: git-commit--onto-parent.sh

Regards, Buga

[*1*] Some previous list mentions of similar functionality, in order 
 of appearance, latest on top (I kind of remember seeing more, but 
 can`t find them now, please feel free to add here, or notify more 
 people interested in the past):

 - [PATCH/RFC] git-post: the opposite of git-cherry-pick (2017-10-05)
   https://public-inbox.org/git/c6b52120-98bf-d685-6dc0-3c83e9e80...@kdbg.org/

 - "groups of files" in Git? (2017-07-11)
   
https://public-inbox.org/git/caeceraz3vyekvj8sm1ffdavsp3lmvqa1o3yojvthvg-0fpt...@mail.gmail.com/

 - Making a (quick) commit to another branch (2013-04-27)
   https://public-inbox.org/git/517bdb6d.8040...@cedarsoft.com/

 - Commit to other branch (2010-05-31)
   https://public-inbox.org/git/4c03d9c1.1060...@cedarsoft.com/

 - [RFC] git commit --branch (2006-05-29)
   https://public-inbox.org/git/20060529202851.ge14...@admingilde.org/

 - n-heads and patch dependency chains (2006-04-03)
   https://public-inbox.org/git/4430d352.4010...@vilain.net/

 - Multi-headed branches (hydra? :)) for basic patch calculus (2006-04-02)
   https://public-inbox.org/git/1143950852.21233.23.camel@localhost.localdomain/


  1   2   >