Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.debian.kernel > #61785 > unrolled thread

Issues with using git debrebase for linux

Started byBen Hutchings <ben@decadent.org.uk>
First post2018-08-12 05:00 +0200
Last post2018-08-17 18:00 +0200
Articles 11 — 3 participants

Back to article view | Back to linux.debian.kernel


Contents

  Issues with using git debrebase for linux Ben Hutchings <ben@decadent.org.uk> - 2018-08-12 05:00 +0200
    Re: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-12 18:20 +0200
      Re: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-12 19:30 +0200
        Re: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-12 20:20 +0200
          Re: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-12 23:50 +0200
    Bug#905975: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-12 19:30 +0200
    Re: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-17 18:00 +0200
      Re: Issues with using git debrebase for linux Sean Whitton <spwhitton@spwhitton.name> - 2018-08-17 18:20 +0200
      Re: Issues with using git debrebase for linux Ben Hutchings <ben@decadent.org.uk> - 2018-08-17 19:10 +0200
        Re: Issues with using git debrebase for linux Ian Jackson <ijackson@chiark.greenend.org.uk> - 2018-08-17 19:30 +0200
    Re: Issues with using git debrebase for linux Sean Whitton <spwhitton@spwhitton.name> - 2018-08-17 18:00 +0200

#61785 — Issues with using git debrebase for linux

FromBen Hutchings <ben@decadent.org.uk>
Date2018-08-12 05:00 +0200
SubjectIssues with using git debrebase for linux
Message-ID<wlOLL-4tm-3@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

I think that dgit and git debrebase have the potential to support
maintenance of the linux package in Debian, but I've run into a number
of issues that I think are currently show-stoppers.  I can turn them
into bug reports if you like, but I'm not yet sure which of them are
actually bugs in git debrebase.

For reference, my work can be found on the "use-dgit" branch of
<https://salsa.debian.org/benh/linux.git>.

1. Safe rebasing

linux is team-maintained, and it's normal for multiple developers to
push changes multiple times between releases.  It's therefore not
acceptable to update branches in a non-fast-forward way.  I believe
that git debrebase is supposed to avoid doing that, but it seems quite
easy to defeat the check:

$ git checkout -b use-dgit-test
Switched to a new branch 'use-dgit-test'
$ git branch --set-upstream-to=benh/use-dgit-test
Branch 'use-dgit-test' set up to track remote branch 'use-dgit-test' from 'benh'.
$ git debrebase -i 
OK, you are ahead of refs/remotes/benh/use-dgit-test
Waiting for Emacs...
Successfully rebased and updated refs/heads/use-dgit-test.
$ git rev-list ..benh/use-dgit-test | wc -l
109
$ git debrebase conclude

git-debrebase: error: No ongoing git-debrebase session.
$ git-debrebase status
current branch contents, in git-debrebase terms:
  branch is laundered
key git-debrebase commits:
  anchor
    470915f1011c git-debrebase import: declare upstream
  breakwater
    470915f1011c git-debrebase import: declare upstream
branch and ref status, in git-debrebase terms:
  stitched? (no record of git-debrebase work)

Why was there no pseudo-merge?  Shouldn't the remote tracking branch
have been recorded as ffq-prev?

2. Replacing commits

Over the last 2 days I've prepared the patches and scripts in the
package for conversion to a dgit patches-applied branch.  There are
three of these that import patches from elsewhere, all of which I have
moved to debian/bin/genpatch-<something>.

genpatch-rt can be ignored because it's updating optional patches that
will have to remain as quilt-in-git.  The other two will now need to
effectively replace existing commits.  When we discussed this at
DebConf it was suggested that git rebase, and therefore git debrebase,
could match up commits with corresponding reverts and delete both of
them from the commit series.  I therefore rewrote those scripts to
revert the old imported patches (in reverse order) and then apply the
new patches.  However, I now find that "git debrebase -i --autosquash"
leaves all the commits intact.  So I don't know how I can automate this
commit replacement now.

3. Speed of operation

git debrebase (that is, the default operation) is very slow in the
linux repository.  I don't know whether the size of the tree, or the
number of commits to upstream code, or both, is the problem.  On stable
branches we may have 1000 or more such commits, so if (as I suspect)
the time is proportional to that number then I think we would need at
least a factor of 10 improvement in the speed of operation.

Ben.

-- 
Ben Hutchings
The Peter principle: In a hierarchy, every employee tends to rise to
their level of incompetence.

[toc] | [next] | [standalone]


#61789

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-12 18:20 +0200
Message-ID<wm1fX-3vU-11@gated-at.bofh.it>
In reply to#61785
Ben Hutchings writes ("Issues with using git debrebase for linux"):
> I think that dgit and git debrebase have the potential to support
> maintenance of the linux package in Debian, but I've run into a number
> of issues that I think are currently show-stoppers.  I can turn them
> into bug reports if you like, but I'm not yet sure which of them are
> actually bugs in git debrebase.

Thanks for investigating.  As you know, it's a very new tool so there
will be rough edges etc.

> 1. Safe rebasing
> 
> linux is team-maintained, and it's normal for multiple developers to
> push changes multiple times between releases.  It's therefore not
> acceptable to update branches in a non-fast-forward way.  I believe
> that git debrebase is supposed to avoid doing that, but it seems quite
> easy to defeat the check:
> 
> $ git checkout -b use-dgit-test
> Switched to a new branch 'use-dgit-test'
> $ git branch --set-upstream-to=benh/use-dgit-test
> Branch 'use-dgit-test' set up to track remote branch 'use-dgit-test' from 'benh'.
> $ git debrebase -i 
> OK, you are ahead of refs/remotes/benh/use-dgit-test
> Waiting for Emacs...
> Successfully rebased and updated refs/heads/use-dgit-test.
> $ git rev-list ..benh/use-dgit-test | wc -l
> 109
> $ git debrebase conclude
> 
> git-debrebase: error: No ongoing git-debrebase session.
> $ git-debrebase status
> current branch contents, in git-debrebase terms:
>   branch is laundered
> key git-debrebase commits:
>   anchor
>     470915f1011c git-debrebase import: declare upstream
>   breakwater
>     470915f1011c git-debrebase import: declare upstream
> branch and ref status, in git-debrebase terms:
>   stitched? (no record of git-debrebase work)
> 
> Why was there no pseudo-merge?  Shouldn't the remote tracking branch
> have been recorded as ffq-prev?

I think this is probably a bug.  I will try to reproduce it.  (I'm
still cloning the giant repo.)

> 2. Replacing commits
> 
> Over the last 2 days I've prepared the patches and scripts in the
> package for conversion to a dgit patches-applied branch.  There are
> three of these that import patches from elsewhere, all of which I have
> moved to debian/bin/genpatch-<something>.
> 
> genpatch-rt can be ignored because it's updating optional patches that
> will have to remain as quilt-in-git.  The other two will now need to
> effectively replace existing commits.  When we discussed this at
> DebConf it was suggested that git rebase, and therefore git debrebase,
> could match up commits with corresponding reverts and delete both of
> them from the commit series.  I therefore rewrote those scripts to
> revert the old imported patches (in reverse order) and then apply the
> new patches.  However, I now find that "git debrebase -i --autosquash"
> leaves all the commits intact.  So I don't know how I can automate this
> commit replacement now.

I think the discussion at DC involved some misunderstanding.

I'm not sure exactly what this script of yours is supposed to do.  To
"replace" a commit in your delta queue, I would do something with
git-[deb]rebase -i and a stunt $EDITOR.  It should be fairly
straightforward.  Does that make sense ?

> 3. Speed of operation
> 
> git debrebase (that is, the default operation) is very slow in the
> linux repository.  I don't know whether the size of the tree, or the
> number of commits to upstream code, or both, is the problem.  On stable
> branches we may have 1000 or more such commits, so if (as I suspect)
> the time is proportional to that number then I think we would need at
> least a factor of 10 improvement in the speed of operation.

You have a 1000-commit delta queue ?  Wow.

The time taken is roughly proportiona to the number of commits since
your last anchor, so (roughly) the length of the delta queue plus the
number of packaging commits since the last new-upstream.

There is probably a lot of scope for performance improvement.  I
hadn't given performance much thought yet.  If need be some parts
might be rewritten in C, although I have little experience of
doing git stuff in C.

Anyway, please file a bug about this.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]


#61790

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-12 19:30 +0200
Message-ID<wm2lI-464-9@gated-at.bofh.it>
In reply to#61789
Ian Jackson writes ("Re: Issues with using git debrebase for linux"):
> Ben Hutchings writes ("Issues with using git debrebase for linux"):
> > 1. Safe rebasing
...
> > git-debrebase: error: No ongoing git-debrebase session.
...
> > Why was there no pseudo-merge?  Shouldn't the remote tracking branch
> > have been recorded as ffq-prev?
> 
> I think this is probably a bug.  I will try to reproduce it.  (I'm
> still cloning the giant repo.)

There is definitely a bug here, which I have just filed.

> > 3. Speed of operation
> > 
> > git debrebase (that is, the default operation) is very slow in the
> > linux repository.  I don't know whether the size of the tree, or the
> > number of commits to upstream code, or both, is the problem.  On stable
> > branches we may have 1000 or more such commits, so if (as I suspect)
> > the time is proportional to that number then I think we would need at
> > least a factor of 10 improvement in the speed of operation.
> 
> You have a 1000-commit delta queue ?  Wow.

I see that your delta queue is only 160-odd commits but the changes
since last anchor are more.  It is indeed very slow.

git-debrebase -i is particularly bad.  It can easily be made as fast
as git-debrebase status.

git-debrebase status could do with improvement too.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]


#61792

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-12 20:20 +0200
Message-ID<wm386-4AR-11@gated-at.bofh.it>
In reply to#61790
Ian Jackson writes ("Re: Issues with using git debrebase for linux"):
> git-debrebase -i is particularly bad.  It can easily be made as fast
> as git-debrebase status.

I have a work-in-progress which has achieved this, and which cuts the
tiem for `git-debrebase' on your use-dgit-test branch from 77s to
3.4s, on my laptop.

I need to do a bit more work to make this change properly testable and
releaseable.  I think there's more to be done to make it faster still.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]


#61793

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-12 23:50 +0200
Message-ID<wm6pj-6nr-1@gated-at.bofh.it>
In reply to#61792
Ian Jackson writes ("Re: Issues with using git debrebase for linux"):
> I have a work-in-progress which has achieved this, and which cuts the
> tiem for `git-debrebase' on your use-dgit-test branch from 77s to
> 3.4s, on my laptop.
> 
> I need to do a bit more work to make this change properly testable and
> releaseable.  I think there's more to be done to make it faster still.

I think I can speed up "git-debrebase status" by a lot, too.

I discover that
    git diff-tree --name-only A B
is no faster than
    git diff-tree A B
and both are much much slower than
    git ls-tree A
    git ls-tree B
and some trivial subsequent computation.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]


#61791 — Bug#905975: Issues with using git debrebase for linux

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-12 19:30 +0200
SubjectBug#905975: Issues with using git debrebase for linux
Message-ID<wm2lH-464-7@gated-at.bofh.it>
In reply to#61785
Package: git-debrebase
Version: 6.6
Severity: important

Ben Hutchings writes ("Issues with using git debrebase for linux"):
> 1. Safe rebasing
> 
> linux is team-maintained, and it's normal for multiple developers to
> push changes multiple times between releases.  It's therefore not
> acceptable to update branches in a non-fast-forward way.  I believe
> that git debrebase is supposed to avoid doing that, but it seems quite
> easy to defeat the check:
> 
> $ git checkout -b use-dgit-test
> Switched to a new branch 'use-dgit-test'
> $ git branch --set-upstream-to=benh/use-dgit-test
> Branch 'use-dgit-test' set up to track remote branch 'use-dgit-test' from 'benh'.
> $ git debrebase -i 
> OK, you are ahead of refs/remotes/benh/use-dgit-test
> Waiting for Emacs...
> Successfully rebased and updated refs/heads/use-dgit-test.
> $ git rev-list ..benh/use-dgit-test | wc -l
> 109
> $ git debrebase conclude
> 
> git-debrebase: error: No ongoing git-debrebase session.
> $ git-debrebase status
> current branch contents, in git-debrebase terms:
>   branch is laundered
> key git-debrebase commits:
>   anchor
>     470915f1011c git-debrebase import: declare upstream
>   breakwater
>     470915f1011c git-debrebase import: declare upstream
> branch and ref status, in git-debrebase terms:
>   stitched? (no record of git-debrebase work)
> 
> Why was there no pseudo-merge?  Shouldn't the remote tracking branch
> have been recorded as ffq-prev?

To reproduce: clone the linux repo.  Check out use-dgit-test,
   c6eb63c6ed0694b0d22fbe5aaff953d209a3fc4e
It is a laundered-and-stitched branch.

Run git-debrebase -i, and drop these two commits (near the end):
   Revert "aufs4.x-rcN base patch"
   aufs4.x-rcN base patch
The rebase will complete successfully.

Observe that there is no ffq-prev and git-debrebase conclude does not
work, even though the branch is no longer ff from
origin/use-dgit-test.

Ian.

[toc] | [prev] | [next] | [standalone]


#61831

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-17 18:00 +0200
Message-ID<wnPkl-1Wu-3@gated-at.bofh.it>
In reply to#61785
Sean Whitton writes ("Re: Issues with using git debrebase for linux"):
> Unfortunately, I do not know precisely what conditions are required such
> that git-rebase is able to strip both commits and reversions of those
> commits.  Presumably your script is not meeting those conditions.

I don't think git-rebase ever does that.  I think you are just
confused.

What git-rebase is sometimes able to do is drop a patch when a series
is rebased onto a baseline which already contains an equivalant
patch.  This is simply an effect of the merge algorithm and is subject
to all of the merge algorithm's failings.  I try to avoid relying on
this behaviour.  IME it is very capable of producing mismerges.
In the case of a rebase, that can include throwing away half of a
rebased commit and mangling the other half.

Ben: as I said earlier, I think you just need a new version of your
strip-a-patch script.  It would run something like this:

   export MY_STRIP_A_PATCH_RUNNING_AS_EDITOR=1
   export EDITOR=my-strip-a-patch-script
   unset VISUAL
   git-debrebase -i 

git-debrebase would invoke git-rebase which would reinvoke your
script, thinking it's the editor.  Your script gets the git-rebase
todo list.  Your script just has to find the git-rebase todo list item
corresponding to the commit to be replaced (`pick <old commitid>'),
and change it to say `pick <commitid for new version of patch>'
or maybe `exec git-am <some file>'.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]


#61833

FromSean Whitton <spwhitton@spwhitton.name>
Date2018-08-17 18:20 +0200
Message-ID<wnPDI-2if-13@gated-at.bofh.it>
In reply to#61831

[Multipart message — attachments visible in raw view] — view raw

Hello,

On Fri 17 Aug 2018 at 04:57PM +0100, Ian Jackson wrote:

> Sean Whitton writes ("Re: Issues with using git debrebase for linux"):
>> Unfortunately, I do not know precisely what conditions are required such
>> that git-rebase is able to strip both commits and reversions of those
>> commits.  Presumably your script is not meeting those conditions.
>
> I don't think git-rebase ever does that.  I think you are just
> confused.

Hmm, okay.  Thank you for the correction.

-- 
Sean Whitton

[toc] | [prev] | [next] | [standalone]


#61834

FromBen Hutchings <ben@decadent.org.uk>
Date2018-08-17 19:10 +0200
Message-ID<wnQq5-2Ng-1@gated-at.bofh.it>
In reply to#61831

[Multipart message — attachments visible in raw view] — view raw

On Fri, 2018-08-17 at 16:57 +0100, Ian Jackson wrote:
[...]
> Ben: as I said earlier, I think you just need a new version of your
> strip-a-patch script.  It would run something like this:
> 
>    export MY_STRIP_A_PATCH_RUNNING_AS_EDITOR=1
>    export EDITOR=my-strip-a-patch-script
>    unset VISUAL
>    git-debrebase -i 

I've been setting GIT_EDITOR, which appears to take precedence over
both EDITOR and VISUAL.

> git-debrebase would invoke git-rebase which would reinvoke your
> script, thinking it's the editor.  Your script gets the git-rebase
> todo list.  Your script just has to find the git-rebase todo list item
> corresponding to the commit to be replaced (`pick <old commitid>'),
> and change it to say `pick <commitid for new version of patch>'
> or maybe `exec git-am <some file>'.

I didn't try to match up commits exactly, but dropped all the existing
commits that have a specific "Bug-Debian: ..." pseudo-header.  I added
the new commits at the end of the series.  I'm satisifed that this
works well enough.

Ben.

-- 
Ben Hutchings
It is impossible to make anything foolproof
because fools are so ingenious.


[toc] | [prev] | [next] | [standalone]


#61835

FromIan Jackson <ijackson@chiark.greenend.org.uk>
Date2018-08-17 19:30 +0200
Message-ID<wnQJs-2Tu-5@gated-at.bofh.it>
In reply to#61834
Ben Hutchings writes ("Re: Issues with using git debrebase for linux"):
> I've been setting GIT_EDITOR, which appears to take precedence over
> both EDITOR and VISUAL.

You have implicitly pointed out a bug in my test suite.  I need to
unset GIT_EDITOR as well as VISUAL...

> > git-debrebase would invoke git-rebase which would reinvoke your
> > script, thinking it's the editor.  Your script gets the git-rebase
> > todo list.  Your script just has to find the git-rebase todo list item
> > corresponding to the commit to be replaced (`pick <old commitid>'),
> > and change it to say `pick <commitid for new version of patch>'
> > or maybe `exec git-am <some file>'.
> 
> I didn't try to match up commits exactly, but dropped all the existing
> commits that have a specific "Bug-Debian: ..." pseudo-header.  I added
> the new commits at the end of the series.  I'm satisifed that this
> works well enough.

That sounds reasonable.

I'm afraid I have several other fires to put out first so the faster
git-debrebase won't be in sid right away, and of course there's still
that failure to record ffq-prev bug.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

[toc] | [prev] | [next] | [standalone]


#61832

FromSean Whitton <spwhitton@spwhitton.name>
Date2018-08-17 18:00 +0200
Message-ID<wnPkl-1Wu-1@gated-at.bofh.it>
In reply to#61785

[Multipart message — attachments visible in raw view] — view raw

Hello Ben,

On Sun 12 Aug 2018 at 03:55AM +0100, Ben Hutchings wrote:

> 2. Replacing commits
>
> Over the last 2 days I've prepared the patches and scripts in the
> package for conversion to a dgit patches-applied branch.  There are
> three of these that import patches from elsewhere, all of which I have
> moved to debian/bin/genpatch-<something>.
>
> genpatch-rt can be ignored because it's updating optional patches that
> will have to remain as quilt-in-git.  The other two will now need to
> effectively replace existing commits.  When we discussed this at
> DebConf it was suggested that git rebase, and therefore git debrebase,
> could match up commits with corresponding reverts and delete both of
> them from the commit series.  I therefore rewrote those scripts to
> revert the old imported patches (in reverse order) and then apply the
> new patches.  However, I now find that "git debrebase -i --autosquash"
> leaves all the commits intact.  So I don't know how I can automate this
> commit replacement now.

Hrm.  Sorry about this -- I think this was my suggestion.

Unfortunately, I do not know precisely what conditions are required such
that git-rebase is able to strip both commits and reversions of those
commits.  Presumably your script is not meeting those conditions.

To be clear, --autosquash is not needed for this behaviour; when the
stripping works, it does not require passing any option to trigger it.
--autosquash is for fixup! and squash! commits.

-- 
Sean Whitton

[toc] | [prev] | [standalone]


Back to top | Article view | linux.debian.kernel


csiph-web