Bug#903377: convert-from-gbp: `diverged', --overwrite if no previous dgit push
Hello,
On Mon 29 Oct 2018 at 02:02PM GMT, Ian Jackson wrote:
> Sean Whitton writes ("Bug#903377: convert-from-gbp: `diverged', --overwrite
> if no previous dgit push"):
>> On Mon 09 Jul 2018 at 10:48AM +0100, Ian Jackson wrote:
>> > has arguably made things worse for a different use case: if the user
>> > goes straight from (i) gbp without dgit, to (ii) gdr with dgit, then
>> > the new pseudomerge generation cannot work. So there will be a new
>> > message about that.
>>
>> How offputting will this message be? Sorry, I don't have a convenient
>> test case, since I always upload with dgit ;)
>
> It is generated by this code:
>
> + print STDERR f_ < +Cannot confirm dgit view: %s
> +Failed to stitch in dgit view (see messages above).
> +dgit --overwrite will be needed on the first dgit push after conversion.
> +END
>
> If you look in the body of the anon sub $previous_dgit_view you'll see
> the things which might appear in the %s.
>
> This message is not a snag; it just goes to stderr. Unlike dgit, gdr
> is not, in general, very chatty, so the user will probably actually
> see it.
Good. Your
-will require I<--overwrite>. Try pushing without this option first,
-and then dgit will suggest using it if it is needed.
+will have to pass I<--overwrite> to dgit. git-debrebase will normally
+tell you if this is will be needed.
seems right.
>> +In some cases where you used B since
>> +the last upload, it is not possible for dgit to make your history
>> +fast-forwarding from the history on B. In such cases you
>> +will require I<--overwrite>. Try pushing without this option first,
>> +and then dgit will suggest using it if it is needed.
>
> This is not really right. It does not make sense to try first without
> --overwrite, and then pass it if dgit complains that it is needed.
> The purpose of --overwrite is to reassure dgit that everything is OK.
> The only effect is to carry on rather than stopping, in certain cases.
>
> If you are going to pass --overwrite if dgit complains that it wants
> it, you might as well pass it unconditionally (unless you were a dgit
> developer and wanted to check that dgit was correctly flagging this
> issue).
Ah, indeed.
> Anyway, now that git-debrebase convert-from-{gbp,unapplied} will make
> the pseudomerge when it can, I think (hope!) that either (i) gdr will
> make the psuedomerge and --overwrite won't be needed or (ii) gdr will
> not be able to make the psuedomerge but will advise the user that
> --overwrite will be needed.
>
> So I think the user should use --overwrite iff gdr advised them that
> it would be needed. In that case they need to check that the
> changelog doesn't lie.
Right.
> The background to all of this is that --overwrite (without a version
> number) is pretty safe: dgit will check that the version being
> overwritten is mentioned in a not-UNRELEASED entry in your
> debian/changelog. So you can only lose work from --overwrite in a
> scenario which contains something isomorphic to this:
> [...]
> I notice persistent difficulties around when to use --overwrite. I
> wonder if it needs a much longer discussion in one of the more
> discursive manpages.
I think so. Going to file a bug.
--
Sean Whitton
signature.asc
Description: PGP signature
Bug#903377: convert-from-gbp: `diverged', --overwrite if no previous dgit push
Sean Whitton writes ("Bug#903377: convert-from-gbp: `diverged', --overwrite if
no previous dgit push"):
> On Mon 09 Jul 2018 at 10:48AM +0100, Ian Jackson wrote:
> > has arguably made things worse for a different use case: if the user
> > goes straight from (i) gbp without dgit, to (ii) gdr with dgit, then
> > the new pseudomerge generation cannot work. So there will be a new
> > message about that.
>
> How offputting will this message be? Sorry, I don't have a convenient
> test case, since I always upload with dgit ;)
It is generated by this code:
+ print STDERR f_ < > But this is complicated and fiddly. I suggest that we document this
> > problem, instead.
>
> Attached patch does this. Since the case is so precisely specified, I
> opted not to clutter the manpage with an explanation of exactly why
> -fdiverged is needed.
> +If you were not previously using dgit to upload your package (i.e. you
> +were not using the workflow described in dgit-maint-gbp(7)), and you
> +happen to have run B in this clone of the repository,
> +you will need to pass I<--fdiverged> to this command.
This part is fine.
> +In some cases where you used B since
> +the last upload, it is not possible for dgit to make your history
> +fast-forwarding from the history on B. In such cases you
> +will require I<--overwrite>. Try pushing without this option first,
> +and then dgit will suggest using it if it is needed.
This is not really right. It does not make sense to try first without
--overwrite, and then pass it if dgit complains that it is needed.
The purpose of --overwrite is to reassure dgit that everything is OK.
The only effect is to carry on rather than stopping, in certain cases.
If you are going to pass --overwrite if dgit complains that it wants
it, you might as well pass it unconditionally (unless you were a dgit
developer and wanted to check that dgit was correctly flagging this
issue).
Anyway, now that git-debrebase convert-from-{gbp,unapplied} will make
the pseudomerge when it can, I think (hope!) that either (i) gdr will
make the psuedomerge and --overwrite won't be needed or (ii) gdr will
not be able to make the psuedomerge but will advise the user that
--overwrite will be needed.
So I think the user should use --overwrite iff gdr advised them that
it would be needed. In that case they need to check that the
changelog doesn't lie.
The background to all of this is that --overwrite (without a version
number) is pretty safe: dgit will check that the version being
overwritten is mentioned in a not-UNRELEASED entry in your
debian/changelog. So you can only lose work from --overwrite in a
scenario which contains something isomorphic to this:
(i) person P1 finalised the changelog with version V and a package
with contents V. Then they didn't actually upload for some
reason, but instead pushed the tree with the finalised changelog
somewhere (eg, salsa). So (calling that commit C1) salsa has:
commit history ...C1
tree changelog ...V
tree contents V
(ii) person PA takes that commit and makes more changes, and uploads
them, without noticing that they didn't need to finalise the
changelog, and without bumping the version. So (using `A' and
`CA' to refer to the the changes and commits made by PA)
now the archive contains
commit history ...C1-...-CA
tree changelog ...V
tree contents V+A
(iii) person PB fetches from dgit (but not salsa), bumps the version
in the changelog from V to some higher version, wlog V+1. They
maybe make other changes. They now have
commit history ...C1-...-CB
tree changelog ...V;V+1
tree contents V [+B]
Person PB now runs dgit push. dgit will notice that CB is not
fast forward from the archive (which has CA) and stop.
But with --overwrite, dgit will examine the archive's version V
and see that PB's branch contains a finalised changelog entry for
V. dgit will conclude that CB contains all the changes in V, ie
all the changes in the archive. So dgit will make a pseudomerge
overwriting CA. The changes A made by PA in (ii) will be thrown
away.
For this scenario to be realistic, PA != PB but maybe one of them is
the same as P1. This mistake scenario can be avoided by what might be
called `procedural controls': both P1 and PA have realistic options to
avoid the mistake.
IMO P1 should burn the version number by bumping the changelog
version, after deciding not to upload but before pushing. IMO PA
should allocate a new version instead of adding changelog entries to
an already-finalised changelog entry. (Note that if P1 is foolish,
they might unfinalise the changelog after pushing, and then take on
the role of PA, avoiding that procedural control.)
PB has an opportunity to see that something is wrong be
Bug#903377: convert-from-gbp: `diverged', --overwrite if no previous dgit push
control: tag -1 +patch Hello, On Mon 09 Jul 2018 at 10:48AM +0100, Ian Jackson wrote: > My fix for #903132 (which is really nice, thank you) > has arguably made things worse for a different use case: if the user > goes straight from (i) gbp without dgit, to (ii) gdr with dgit, then > the new pseudomerge generation cannot work. So there will be a new > message about that. How offputting will this message be? Sorry, I don't have a convenient test case, since I always upload with dgit ;) I don't think we need to document the expected message explicitly if there is no action the user needs to take other than using --overwrite and possibly -fdiverged. > But also, there will be a new `diverged' snag at the time of git > debrebase convert-from-gbp, if the user has ever done dgit fetch sid. > In that case the user of gdr 5.8 will have to specify -fdiverged to > convert-from-gbp. > > In both cases they will also have to tell dgit --overwrite, just as > they would for 5.7 and earlier. > > It might be possible to find a suitable parent to overwrite with a > pseudomerge, provided the user has done `dgit fetch sid'. The > basic recipe would be to check that the last maintainer view tag > corresponds to dgit/dgit/sid (except for debian/patches). > > But this is complicated and fiddly. I suggest that we document this > problem, instead. Attached patch does this. Since the case is so precisely specified, I opted not to clutter the manpage with an explanation of exactly why -fdiverged is needed. -- Sean Whitton From 2c7d02ff627caaf687e3486d4203e921da88d686 Mon Sep 17 00:00:00 2001 From: Sean Whitton Date: Sat, 27 Oct 2018 13:40:26 -0700 Subject: [PATCH] dgit-maint-debrebase(7): account for case discussed in #903377 Signed-off-by: Sean Whitton --- dgit-maint-debrebase.7.pod | 11 +++ 1 file changed, 11 insertions(+) diff --git a/dgit-maint-debrebase.7.pod b/dgit-maint-debrebase.7.pod index e5abe17..e3d557a 100644 --- a/dgit-maint-debrebase.7.pod +++ b/dgit-maint-debrebase.7.pod @@ -269,6 +269,11 @@ patches applied. Use =back +If you were not previously using dgit to upload your package (i.e. you +were not using the workflow described in dgit-maint-gbp(7)), and you +happen to have run B in this clone of the repository, +you will need to pass I<--fdiverged> to this command. + =item (C) There is a delta queue, and patches are applied. Use @@ -442,6 +447,12 @@ cowbuilder>. Upload with B or B. Remember to pass I<--new> if the package is new in the target suite. +In some cases where you used B since +the last upload, it is not possible for dgit to make your history +fast-forwarding from the history on B. In such cases you +will require I<--overwrite>. Try pushing without this option first, +and then dgit will suggest using it if it is needed. + Right before uploading, if you did not just already do so, you might want to have git-debrebase(1) shuffle your branch such that the Debian delta queue appears right at the tip of the branch you will push: -- 2.11.0 signature.asc Description: PGP signature
