> On Sun 21 Jul 2019 at 03:02PM +01, Ian Jackson wrote:
> > Look for the ffq-prev ref. Any gdr unstitched branch will have such a
> > corresponding ref. See STITCHING, PSEUDO-MERGES, FFQ RECORD in
> > git-debrebase(5).
>
> Thanks. Patch enclosed.
Nice work.
LGTM. One tiny code comment. Also a big digression...
> > For a detached head, this is not determinable, and gdr conclude is not
> > possible either. So this means that git-debpush of a non-branch
> > commitish must either be a forceable fail, or must skip this check.
>
> I prefer the latter is better because I don't want to impose anything
> extra on non-gdr users.
Right.
Another way to detect this would be that the branch is not ff of where
it is going to be pushed. Indeed, it will be not ff of the local
tracking branch for the remote. This might be a useful thing to check
anyway: it seems undesirable to make a signed tag, and then
predictably fall over during push.
But this doesn't help in the gdr case because with a detached HEAD, it
wouldn't be pushed anywhere.
I worry about a bad failure mode. You detach your head, in the middle
of gdr work, to do some kind of something. You forget to reattach and
instead git-debpush. Because we have always split the view, dgit on
the git-debpush server might make a pseudomerge binding in your
unstitched branch. If you then go away, you'll leave a history
divergence between salsa and dgit.
Re this:
(i) I think but I am not sure that dgit in split view mode will still
try to check that you are ff of something. But I think "something" is
the last maintainer upload. So that won't stop this. And anyway we
always have --overwrite so even that check will be gone.
(ii) Maybe we should have a non-splitting "gdr" "quilt mode" for
git-debpush, where we can avoid this problem more easily.
(iii) Also you could do all of the above mid-rebase (!) I think this
is a general problem. Certainly we should check for that. And, when
I look at it like that I think maybe the answer is this:
git-debpush should not push a detached HEAD unless you explicitly say
HEAD or name the commitish or something. Ie the "just push what I am
looking at" should require --force if you are not on a branch.
I'm not sure whether we want even git-debpush HEAD to require --force.
Some people may have a tendency to use that because it is "more
explicit and therefore safer" and if we take it to mean "push HEAD
even if it's detached" when the default is not to push detached
things, we are interpreting them in the opposite way to what they
meant. So I would be inclined to require --force for any case where
what we push starts out as literally precisely HEAD (whether
explicitly specified, or defaulted) and is detached. (So "HEAD~0"
would not need forcing.)
What do you think ?
As for the code...
> +# ---- git-debrebase branch format checks
> +
> +# only check branches, since you can't run `git debrebase conclude` on
> +# non-branches
> +case "$branch" in
> + refs/heads/*)
> + # see "STITCHING, PSEUDO-MERGES, FFQ RECORD" in git-debrebase(5)
> + ffq_prev_ref="${branch/refs/refs/ffq-prev}"
Don't you find this very confusing ? I would have written
+ ffq_prev_ref="refs/ffq-prev/${branch#refs/}"
Up to you.
Apart from that LGTM. Do you think we should have a test case ?
We could add a failing git-debpush invocation to some random gdr-test.
(adding the dependency and regenerating d/t/control)
Ian.
