Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)

3 messages, 2 authors, 2017-07-13 · open the first message on its own page

Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)

From: Junio C Hamano <hidden>
Date: 2017-07-13 15:23:02

Johannes Schindelin [off-list ref] writes:
I wonder whether the --force-with-lease option would benefit (and counter
the "unsafe because of behind-the-back operations" argument) from doing
some kind of "reverse pull --rebase".

In other words, --force-with-lease could verify that the upstream branch
has no commits that weren't seen in the current branch's reflog, i.e. that
`git rev-parse HEAD@{upstream}` is identical to `git merge-base
--fork-point HEAD HEAD@{upstream}`.

The assumption would be that you typically want to push with a lease only
when following the `pull --rebase` workflow. Meaning that you would only
want to force-push when your local branch had the upstream branch
integrated at some stage [*1*].
I suspect that the problem "--force-with-lease" wants to solve does
not even appear in "pull --rebase" workflow, but let me think aloud
to see where this leads, as the idea sounds interesting, even though
I may have misunderstood what you meant by the above.

If you do a "pull --rebase", you first rewind to the upstream and
then reapply your changes on top, meaning that the result would
normally be based on what was once at the tip of the upstream and
should fast-forward.  The only situation that a result of "pull
--rebase" needs a non-ff push is when somebody else pushed there in
the meantime, adding commits that you haven't seen yet.  And you
do not want to force your push to lose their work with "--force",
with or without any lease.

It does not change if you do an extra "fetch" to update the tip of
your remote-tracking branch with their work.  Using the lazy
"--force-with-lease" is a wrong thing to do here, of course, but
using any "--force", with or without lease, is _unnecessary_ with
"pull --rebase".  The safety afforded by the bog standard push that
requires fast-forward is sufficient.

And the above will equally apply to a non rebasing "pull".

The problem "with-lease" part tries to solve is when you are
actively rewinding the history of the upstream.  Your upstream may
look like:

    o---A---B---C

and you fetch, squash B and C into one with some changes, to come up
with a local history:

          BC
         /
    o---A---B---C

and try to replace the upstream's tip (which is _supposed to be_ at
C) with BC; the resulting history leading to BC is supposed to be a
replacement of the upstream's history, and that is true only when
the upstream's tip is still at C when you do your push.  If somebody
else built D on top of C and pushed there, that would no longer be
true and you'd end up losing it by replacing the upstream's tip with
your BC.  Before the "with lease" option, when you want to fix the
"---B---C" segment of the history that is already published, the
only way to replace it with a single commit BC was with "--force"
and there is no mechanism other than inter-developer coordination to
make sure nobody will push D on top of C that will be lost with your
forced push.  "push --force-with-lease=<ref>:C BC:<ref>" is meant to
be a solution for that issue.

Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)

From: Jeff King <hidden>
Date: 2017-07-13 15:53:36

On Thu, Jul 13, 2017 at 08:22:54AM -0700, Junio C Hamano wrote:
quoted
In other words, --force-with-lease could verify that the upstream branch
has no commits that weren't seen in the current branch's reflog, i.e. that
`git rev-parse HEAD@{upstream}` is identical to `git merge-base
--fork-point HEAD HEAD@{upstream}`.

The assumption would be that you typically want to push with a lease only
when following the `pull --rebase` workflow. Meaning that you would only
want to force-push when your local branch had the upstream branch
integrated at some stage [*1*].
I suspect that the problem "--force-with-lease" wants to solve does
not even appear in "pull --rebase" workflow, but let me think aloud
to see where this leads, as the idea sounds interesting, even though
I may have misunderstood what you meant by the above.

If you do a "pull --rebase", you first rewind to the upstream and
then reapply your changes on top, meaning that the result would
normally be based on what was once at the tip of the upstream and
should fast-forward.  The only situation that a result of "pull
--rebase" needs a non-ff push is when somebody else pushed there in
the meantime, adding commits that you haven't seen yet.  And you
do not want to force your push to lose their work with "--force",
with or without any lease.
That assumes that you're pushing back to the upstream. You could be
doing a "pull --rebase" from upstream and then pushing up to another ref
(or even another repo).

Consider something like a topic branch whose @{upstream} is
origin/master, but you use the "current" push.default. If you do:

  $ git pull --rebase
  $ git push

then the push is a non-ff, because you're pushing the rebased commits.
You'd ideally want a way to force that without clobbering a push that
somebody else made to the topic branch.

I do think Dscho is on to something with the "see if it was ever in our
reflog" idea. What you really want as a default for force-with-lease is
some way to say "I want to change that ref from X to Y, but only for
values of X that I know Y has already considered and incorporated". So
the danger in using the tracking branch is that you may not have
incorporated that value (either because somebody fetched behind your
back, or because you simply forgot to merge it into your local branch
when doing your history-rewriting operation).

So even forgetting "git pull --rebase", doing:

  $ git fetch
  $ git rebase HEAD~2
  $ git push --force-with-lease

is bad, because your rebase didn't take into account what was fetched in
step 1. But imagine force-with-lease rule were more like: when pushing
branch "foo" to remote branch "bar", allow only if the current value of
"bar" is an ancestor of some entry in the reflog of "foo".

That degrades to the usual fast-forward rule if you just check the ref
tip.  And when checking the reflog entries, it shows that at some point
you had your local branch set to something that covered all of the
destination, but then later rewound or rewrote history. We don't have to
care what that operation was. "rebase" is a likely one, but "git reset
--hard HEAD^ && git push" would fall out naturally from the same rule.

It's not quite as safe as a true force-with-lease with a value would be.
For instance, if _upstream_ rewound by a commit, we'd happily undo that
rewind. But then, so does a normal non-forced fast-forward push.

So I'd say this is not really force-with-lease at all, but rather a
special mode that is somewhere between a normal fast-forward and a true
force-with-lease. I don't know if it would make sense to give it a
different name, or just lump it under force-with-lease.

-Peff

Re: What's cooking in git.git (Jul 2017, #03; Mon, 10)

From: Junio C Hamano <hidden>
Date: 2017-07-13 16:32:36

On Thu, Jul 13, 2017 at 8:53 AM, Jeff King [off-list ref] wrote:
So I'd say this is not really force-with-lease at all, but rather a
special mode that is somewhere between a normal fast-forward and a true
force-with-lease. I don't know if it would make sense to give it a
different name, or just lump it under force-with-lease.
Yes. I think these are separate problems that needs separate mechanisms
to solve, and I agree with you that it is also a separate issue if a single UI
"safer non-ff push" that covers both (and other problems that will be
discovered later) problems is desired or if a separate options (e.g.
"--force=lease=...,reflog=...," with nicer names and syntax) is better.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help