On Mon, Apr 10, 2017 at 10:08 AM, Jacob Keller <jacob.keller@xxxxxxxxx> wrote:
> On Sun, Apr 9, 2017 at 4:00 AM, Stefan Haller <haller@xxxxxxxxxxx> wrote:
>> Jacob Keller <jacob.keller@xxxxxxxxx> wrote:
>>
>>> Agreed. You "take" a lease whenever you push to the remote or when you
>>> pull from the remote and when you pull into the branch. It should
>>> store something that tracks both the branch and remote branch together
>>> so that you can generalize it to multiple remotes.
>>
>> I don't see why it has to support multiple remotes (but then I don't
>> have much experience with workflows involving multiple remotes, so I may
>> well be missing something). A local branch can only have one remote
>> tracking branch on one remote, and in my view --force-with-lease without
>> arguments works with that remote tracking branch only. Is this view too
>> simple?
>>
>
> I think that's fine. Thinking in terms of only one remote at a time is easier.
>
>>> It doesn't necessarily track perfectly with a branch that contains
>>> extra work such as when doing pull --rebase, but maybe you have an
>>> idea about that?
>>
>> Maybe I wasn't clear enough about that in my proposal, but I propose to
>> always store the commit hash of the remote tracking branch as a new
>> lease after push and pull, not the local branch. This way it works
>> nicely with pull --rebase and a branch that has extra local commits.
>>
>>
>
> Oh right. The main thing it doesn't give is that this doesn't enforce
> that your local branch *has* to have at one point prior to the push
> matched the remote branch that you're overwriting, which would be even
> more evidence that your changes are what you expect and aren't
> deleting anything unexpectedly. However, I think that's not strictly
> necessary to require that since it would also break pull-rebase
> workflow anyways.
>
> So something like:
>
> For a branch, also store its last known "lease" sha1 value, which
> updates once every time that we push that branch to the remote
> tracking branch or any time that we pull into the branch using the
> remote tracking branch.
>
> This value is what we would default to, and we only need to store the
> latest one, since that's the last known good value. If the value is
> wrong then we will error and avoid deleting work, and if it's correct,
> then we know that the remote branch is correct for this specific push
> and is safe.
>
> I think this is straight forward and reasonable approach to solve the
> problem, and makes using force-with-lease much nicer.
Does this proposal require that all the things that can update a ref
be hooked to maintain these lease values?
In lieu of patches it would be nice for us trying to follow along to
at least get some partial list of things that would need to be hooked
up, how the command workflow would look like, what things wouldn't
work that do now, or work that don't now etc.
E.g. now if I:
git fetch
git show origin/master # manually note the sha1
git checkout -b blah <that-sha1>
git commit --amend
git branch --set-upstream-to origin/master
git push --force-with-lease
It'll work unless origin/master was updated in the meantime, but
there's no hint in any log that the branch was created from
origin/master to begin with, just from the sha it happened to point
to.
I think this is a really important property of git, you don't have to
go through some chosen UI that'll record things because you told it
"I'd like to checkout stuff from that branch", you can just copy/paste
the sha1.
How will this work in this proposed "lease" workflow? Will some lease
metadata not get created and I'll need to manually retcon that with
some new command?
I'm not just trying to come up with contrived scenarios, I've had to
do several force pushes (on repos used by many people) and it's
usually due to some giant commit being pushed. At that point the
machine I'm investigating the situation on might not be the machine
where I do the push from, so often I'm just copy/pasting a sha1 across
machines.
To me the *main* feature of --force-with-lease is that it's less
shitty than --force, without imposing too much UI overhead. We have to
be really careful not to make --force-with-lease so complex by default
that people just give u and go back to using --force, which would be
worse than either whatever current problems there are with the
current --force-with-lease behavior, or anything we replace it with.