Hello, On Mon 22 Jul 2019 at 02:40PM +01, Ian Jackson wrote:
> 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.
git-tag alone doesn't check for this, so I'd be inclined not to add that
check. ISTM that git-debpush's checks should all be Debian-specific,
and we assume the user knows the standard git side of things. But I'm
not sure.
Also not helpful if you haven't fetched recently, which is I think when
this problem is most likely to occur.
(Detached heads might not count as standard git. Being behind an
upstream remote branch does.)
> 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.
I'd really like to avoid this. Branch layouts which work without any
layout-specific options with dgit should ideally not lose this property
when it comes to git-debpush.
> (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 ?
This is easy for the user to understand and is helpful. Implemented
(and adhoc tested; it's v. simple).
> 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.
That's better, thanks.
> 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)
It would be good to have a test case, but perhaps you could take a look,
since manipulating the test suite is much easier for you.
Given that it's gathering commits, I've pushed my new series to branch
'series/git-debpush-check-unstitched-v2' of repo
<https://git.spwhitton.name/dgit>.
--
Sean Whitton
signature.asc
Description: PGP signature
