stgit 0.13 import mbox problems

stgit 0.13 import mbox problems

[Greg KH]

Hi,

I wanted to see if I could start using stgit instead of quilt, so I
tried to import my current set of kernel patches.

After giving up on the "import a series" option, I just created a mbox
of all of them using quilt and tried to import that.  Unfortunately that
didn't work either:
	$ stg import -M ~/linux/patches/mbox
	Checking for changes in the working directory ... done
	Importing patch "add-my-version-to-the-kernel" ... done
	Importing patch "stupid-patch-for-my-laptop-whi" ... done
	Importing patch "gregs-test-driver-core-sysfs-s" ... done
	Importing patch "detect-atomic-counter-underflo" ... done
	Warning: Message does not contain any diff
	stg import: No diff found inside the patch

I'm using the .13 version if that matters.

The mbox contains 177 kernel patches against Linus's current tree
(2.6.23-rc3-git5), and is available at:
	http://www.kernel.org/pub/linux/kernel/people/gregkh/misc/gregkh-stgit-import-mbox.gz
if anyone wants to test it out and see what I was doing wrong.

Any hints?

Oh, I do have some suggestions as to the naming of the patch from a mail
file, as limiting this to a small number of characters like stgit
currently does will not work out for a lot of my patches, but I'll wait
until I can actually import the thing before I look into that :)

thanks,

greg k-h

Re: stgit 0.13 import mbox problems

[Catalin Marinas]

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

On 23/08/07, Greg KH <greg@kroah.com> wrote:
> I wanted to see if I could start using stgit instead of quilt, so I
> tried to import my current set of kernel patches.
>
> After giving up on the "import a series" option,

Why?

> I just created a mbox
> of all of them using quilt and tried to import that.  Unfortunately that
> didn't work either:
>         $ stg import -M ~/linux/patches/mbox
>         Checking for changes in the working directory ... done
>         Importing patch "add-my-version-to-the-kernel" ... done
>         Importing patch "stupid-patch-for-my-laptop-whi" ... done
>         Importing patch "gregs-test-driver-core-sysfs-s" ... done
>         Importing patch "detect-atomic-counter-underflo" ... done
>         Warning: Message does not contain any diff
>         stg import: No diff found inside the patch

Maybe I should just leave the warning and let it continue. The reason
I added it was that "git-apply --index" fails if there is no diff.

In the meantime, you can try the attached patch for StGIT.

Another hint - quilt can apply patches with fuzz but GIT doesn't allow
this by default. If a patch fails, the diff is dumped to the
.stgit-failed.patch file so that you can apply it manually (with patch
or git-apply) and run 'stg refresh' afterwards.

After an import failure, you can continue importing from the next
patch using the 'stg import --ignore' option.

> I'm using the .13 version if that matters.
>
> The mbox contains 177 kernel patches against Linus's current tree
> (2.6.23-rc3-git5), and is available at:
>         http://www.kernel.org/pub/linux/kernel/people/gregkh/misc/gregkh-stgit-import-mbox.gz
> if anyone wants to test it out and see what I was doing wrong.

I'll give it a try.

One thing you'll notice is the speed difference as stgit has to
generate a git commit during a push operation.

> Oh, I do have some suggestions as to the naming of the patch from a mail
> file, as limiting this to a small number of characters like stgit
> currently does will not work out for a lot of my patches, but I'll wait
> until I can actually import the thing before I look into that :)

We had the full name in the past but the algorithm cause problems with
patches (not e-mails) that didn't have a subject line. It's probably
better to have a config option rather than hard-coded 30 characters.
Note that 'stg series -d' will display the full subject line.

If you don't give up before importing the files :-), please let us
know the user experience, especially related to speed as compared to
quilt.

Karl, maybe it's worth trying this series with your DAG patches as well.

Regards.

-- 
Catalin

[-- Attachment #2: import-empty.patch --]
[-- Type: text/x-patch, Size: 2240 bytes --]

Allow 'import' to apply empty patches

From: Catalin Marinas <catalin.marinas@gmail.com>

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/commands/imprt.py |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/stgit/commands/imprt.py b/stgit/commands/imprt.py
index f972b89..98fe708 100644
--- a/stgit/commands/imprt.py
+++ b/stgit/commands/imprt.py
@@ -201,8 +201,6 @@ def __parse_mail(msg):
     rem_descr, diff = __split_descr_diff(msg_text)
     if rem_descr:
         descr += '\n\n' + rem_descr
-    if not diff:
-        out.warn('Message does not contain any diff')
 
     # parse the description for author information
     descr, descr_authname, descr_authemail, descr_authdate = \
@@ -250,9 +248,6 @@ def __create_patch(filename, message, author_name, author_email,
         # fix possible invalid characters in the patch name
         patch = re.sub('[^\w.]+', '-', patch).strip('-')
 
-    if not diff:
-        raise CmdException, 'No diff found inside the patch'
-
     if options.ignore and patch in crt_series.get_applied():
         out.info('Ignoring already applied patch "%s"' % patch)
         return
@@ -288,14 +283,17 @@ def __create_patch(filename, message, author_name, author_email,
                          committer_name = committer_name,
                          committer_email = committer_email)
 
-    out.start('Importing patch "%s"' % patch)
-    if options.base:
-        git.apply_patch(diff = diff, base = git_id(options.base))
+    if not diff:
+        out.warn('No diff found, creating empty patch')
     else:
-        git.apply_patch(diff = diff)
-    crt_series.refresh_patch(edit = options.edit,
-                             show_patch = options.showpatch)
-    out.done()
+        out.start('Importing patch "%s"' % patch)
+        if options.base:
+            git.apply_patch(diff = diff, base = git_id(options.base))
+        else:
+            git.apply_patch(diff = diff)
+        crt_series.refresh_patch(edit = options.edit,
+                                 show_patch = options.showpatch)
+        out.done()
 
 def __import_file(filename, options, patch = None):
     """Import a patch from a file or standard input

Re: stgit 0.13 import mbox problems

[Catalin Marinas]

On 23/08/07, Catalin Marinas <catalin.marinas@gmail.com> wrote:
> One thing you'll notice is the speed difference as stgit has to
> generate a git commit during a push operation.

I did some performance comparison with Quilt almost two years ago but
I think both StGIT and GIT were greatly improved since then:

http://www.gelato.unsw.edu.au/archives/git/0510/10059.html

-- 
Catalin

Re: stgit 0.13 import mbox problems

[Greg KH]

On Thu, Aug 23, 2007 at 11:19:12AM +0100, Catalin Marinas wrote:
> On 23/08/07, Greg KH <greg@kroah.com> wrote:
> > I wanted to see if I could start using stgit instead of quilt, so I
> > tried to import my current set of kernel patches.
> >
> > After giving up on the "import a series" option,
> 
> Why?

Because it kept dieing too :)

Also, it would import these patches, which are individually in mbox
form, with the incorrect author information.  So I thought I would use
the mbox form to make sure I wasn't just doing something stupid.

> > I just created a mbox
> > of all of them using quilt and tried to import that.  Unfortunately that
> > didn't work either:
> >         $ stg import -M ~/linux/patches/mbox
> >         Checking for changes in the working directory ... done
> >         Importing patch "add-my-version-to-the-kernel" ... done
> >         Importing patch "stupid-patch-for-my-laptop-whi" ... done
> >         Importing patch "gregs-test-driver-core-sysfs-s" ... done
> >         Importing patch "detect-atomic-counter-underflo" ... done
> >         Warning: Message does not contain any diff
> >         stg import: No diff found inside the patch
> 
> Maybe I should just leave the warning and let it continue. The reason
> I added it was that "git-apply --index" fails if there is no diff.

But there was a diff.  Or it was in the file, I don't know what happened
to it :)

> In the meantime, you can try the attached patch for StGIT.
> 
> Another hint - quilt can apply patches with fuzz but GIT doesn't allow
> this by default. If a patch fails, the diff is dumped to the
> .stgit-failed.patch file so that you can apply it manually (with patch
> or git-apply) and run 'stg refresh' afterwards.

Ah, perhaps this is the problem, I'll check it out later today.  A bit
more helpful message would be appreciated, especially as I get a lot of
patches that at first apply, do not go cleanly at all.  I think there's
an override option for git to ignore fuzz somewhere, right?  Perhaps
that could be an option for stgit here?

> After an import failure, you can continue importing from the next
> patch using the 'stg import --ignore' option.

Ok, will try that, and your patch.

> > I'm using the .13 version if that matters.
> >
> > The mbox contains 177 kernel patches against Linus's current tree
> > (2.6.23-rc3-git5), and is available at:
> >         http://www.kernel.org/pub/linux/kernel/people/gregkh/misc/gregkh-stgit-import-mbox.gz
> > if anyone wants to test it out and see what I was doing wrong.
> 
> I'll give it a try.
> 
> One thing you'll notice is the speed difference as stgit has to
> generate a git commit during a push operation.

Oh yeah, I know that it would be slower, but I use git-quiltapply a lot
for sending patches to Linus, and that seems quite fast (actually a lot
faster than stgit for some reason...)

> > Oh, I do have some suggestions as to the naming of the patch from a mail
> > file, as limiting this to a small number of characters like stgit
> > currently does will not work out for a lot of my patches, but I'll wait
> > until I can actually import the thing before I look into that :)
> 
> We had the full name in the past but the algorithm cause problems with
> patches (not e-mails) that didn't have a subject line. It's probably
> better to have a config option rather than hard-coded 30 characters.
> Note that 'stg series -d' will display the full subject line.

Hm, that shows:
	$ stg series -d
	+ add-my-version-to-the-kernel   | Add my version to the kernel.
	+ stupid-patch-for-my-laptop-whi | Stupid patch for my laptop which cant get sysrq-u
	+ gregs-test-driver-core-sysfs-s | Gregs test driver core / sysfs stress test module
	> detect-atomic-counter-underflo | detect atomic counter underflows

Are those spaces really the name of the patch?

Why not just take the Subject: and mangle it to be the full name of the
patch (yeah, I can see problems if you don't have the subject).  I have
a bash script around here from a kernel developer that I use to turn
mbox files into sane file names that works great.

Ah, it's at:
	http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/scripts/rename-patch
if you want to take a look and possibly use the same type of logic.
Hm, Jean seems to have updated it, use the one at:
	http://jdelvare.pck.nerim.net/linux/rename-patch
instead, it's a bit more up to date.

> If you don't give up before importing the files :-), please let us
> know the user experience, especially related to speed as compared to
> quilt.

I really don't want to give up :)

I really do like quilt, but wanted to see how well my current workflow
could be by using stgit as I'm constantly rebasing the main kernel
version against -git snapshots and sometimes that isn't frequent enough.

thanks,

greg k-h

Re: stgit 0.13 import mbox problems

[Josef Sipek]

On Thu, Aug 23, 2007 at 02:22:54AM -0700, Greg KH wrote:
> Hi,
> 
> I wanted to see if I could start using stgit instead of quilt, so I
> tried to import my current set of kernel patches.
 
May I suggest you give guilt [1,2] a spin? It uses the same quilt-like
patch directory format so things should Just Work(tm).

Josef 'Jeff' Sipek.

[1] http://kernel.org/pub/linux/kernel/people/jsipek/guilt/
[2] git://git.kernel.org/pub/scm/linux/kernel/git/jsipek/guilt.git

-- 
You measure democracy by the freedom it gives its dissidents, not the
freedom it gives its assimilated conformists.
		- Abbie Hoffman

Re: stgit 0.13 import mbox problems

[J. Bruce Fields]

On Thu, Aug 23, 2007 at 09:43:22AM -0700, Greg KH wrote:
> I really do like quilt, but wanted to see how well my current workflow
> could be by using stgit as I'm constantly rebasing the main kernel
> version against -git snapshots and sometimes that isn't frequent enough.

So just want to be able to rebase more than once a day?  Then why not
just run quilt on top of git?  Pop off all your quilt patches, git pull,
push them all back on again....

Maybe I'm misunderstanding your use case.

--b.

Re: stgit 0.13 import mbox problems

[Greg KH]

On Thu, Aug 23, 2007 at 02:31:11PM -0400, J. Bruce Fields wrote:
> On Thu, Aug 23, 2007 at 09:43:22AM -0700, Greg KH wrote:
> > I really do like quilt, but wanted to see how well my current workflow
> > could be by using stgit as I'm constantly rebasing the main kernel
> > version against -git snapshots and sometimes that isn't frequent enough.
> 
> So just want to be able to rebase more than once a day?  Then why not
> just run quilt on top of git?  Pop off all your quilt patches, git pull,
> push them all back on again....

Yes, I can do that, but I already keep the patch set in a different git
tree, so that others can sync up with me at times.

I was just thinking that it might be easier to use stgit and then only
have to use one git tree to do everything, as this is what stgit is for
:)

Just always trying to see if I can make my life easier and help out with
making git better overall...

thanks,

greg k-h

Re: stgit 0.13 import mbox problems

[Greg KH]

On Thu, Aug 23, 2007 at 02:06:33PM -0400, Josef Sipek wrote:
> On Thu, Aug 23, 2007 at 02:22:54AM -0700, Greg KH wrote:
> > Hi,
> > 
> > I wanted to see if I could start using stgit instead of quilt, so I
> > tried to import my current set of kernel patches.
>  
> May I suggest you give guilt [1,2] a spin? It uses the same quilt-like
> patch directory format so things should Just Work(tm).

Nice, I didn't realize this was still being worked on.

Hm, is there an "easy" way to take a current tree of quilt patches
(like, say 177 of them) and import them into guilt?  After reading the
guilt documentation I didn't see a way to do it, but I might have missed
something.

Also, this looks exactly like stgit in a way, can anyone point me at the
differences, or is this just two different projects being done by two
different groups/people to do the same thing?  If so, that's fine, just
curious.

thanks,

greg k-h

Re: stgit 0.13 import mbox problems

[Josef Sipek]

On Thu, Aug 23, 2007 at 11:47:58AM -0700, Greg KH wrote:
> On Thu, Aug 23, 2007 at 02:06:33PM -0400, Josef Sipek wrote:
> > On Thu, Aug 23, 2007 at 02:22:54AM -0700, Greg KH wrote:
> > > Hi,
> > > 
> > > I wanted to see if I could start using stgit instead of quilt, so I
> > > tried to import my current set of kernel patches.
> >  
> > May I suggest you give guilt [1,2] a spin? It uses the same quilt-like
> > patch directory format so things should Just Work(tm).
> 
> Nice, I didn't realize this was still being worked on.
> 
> Hm, is there an "easy" way to take a current tree of quilt patches
> (like, say 177 of them) and import them into guilt?  After reading the
> guilt documentation I didn't see a way to do it, but I might have missed
> something.
 
cd gregkh-2.6/
git-checkout master
guilt-init
rm -rf .git/patches/master/
mv quilts-patches-dir/ .git/patches/master/
guilt-series # tada!

> Also, this looks exactly like stgit in a way, can anyone point me at the
> differences, or is this just two different projects being done by two
> different groups/people to do the same thing?  If so, that's fine, just
> curious.

Yep. Two different projects taking different approaches to storing the
patches. I go with the plaintext diff approach ala quilt, while stgit uses
git objects to store the data.

Josef 'Jeff' Sipek.
 
-- 
#endif /* NO LIFE */

Re: stgit 0.13 import mbox problems

[Josef Sipek]

On Thu, Aug 23, 2007 at 11:47:58AM -0700, Greg KH wrote:
> On Thu, Aug 23, 2007 at 02:06:33PM -0400, Josef Sipek wrote:
> > On Thu, Aug 23, 2007 at 02:22:54AM -0700, Greg KH wrote:
> > > Hi,
> > > 
> > > I wanted to see if I could start using stgit instead of quilt, so I
> > > tried to import my current set of kernel patches.
> >  
> > May I suggest you give guilt [1,2] a spin? It uses the same quilt-like
> > patch directory format so things should Just Work(tm).
> 
> Nice, I didn't realize this was still being worked on.
 
Heh, I just noticed this...guilt is younger than stgit - and was created
because of how stgit stores the patches. :)

Josef 'Jeff' Sipek.

-- 
If I have trouble installing Linux, something is wrong. Very wrong.
		- Linus Torvalds

Re: stgit 0.13 import mbox problems

[Catalin Marinas]

On 23/08/07, Josef Sipek <jsipek@fsl.cs.sunysb.edu> wrote:
> Yep. Two different projects taking different approaches to storing the
> patches. I go with the plaintext diff approach ala quilt, while stgit uses
> git objects to store the data.

But guilt also generates a commit object when pushing a patch. Or am I wrong?

-- 
Catalin

Re: stgit 0.13 import mbox problems

[Josef Sipek]

On Thu, Aug 23, 2007 at 09:36:05PM +0100, Catalin Marinas wrote:
> On 23/08/07, Josef Sipek <jsipek@fsl.cs.sunysb.edu> wrote:
> > Yep. Two different projects taking different approaches to storing the
> > patches. I go with the plaintext diff approach ala quilt, while stgit uses
> > git objects to store the data.
> 
> But guilt also generates a commit object when pushing a patch. Or am I wrong?

That's correct. But the git commit/tree/blob objects are a mere cache. For
example, a refresh consists of:

git-update-index ...
git-diff HEAD^ > patchfile
git-reset --hard HEAD^
<internal push patch function - uses git-apply, git-{write,commit}-tree>

Josef 'Jeff' Sipek.

-- 
Research, n.:
  Consider Columbus:
    He didn't know where he was going.
    When he got there he didn't know where he was.
    When he got back he didn't know where he had been.
    And he did it all on someone else's money.

Re: stgit 0.13 import mbox problems

[Catalin Marinas]

On 23/08/07, Josef Sipek <jsipek@fsl.cs.sunysb.edu> wrote:
> On Thu, Aug 23, 2007 at 09:36:05PM +0100, Catalin Marinas wrote:
> > On 23/08/07, Josef Sipek <jsipek@fsl.cs.sunysb.edu> wrote:
> > > Yep. Two different projects taking different approaches to storing the
> > > patches. I go with the plaintext diff approach ala quilt, while stgit uses
> > > git objects to store the data.
> >
> > But guilt also generates a commit object when pushing a patch. Or am I wrong?
>
> That's correct. But the git commit/tree/blob objects are a mere cache. For
> example, a refresh consists of:
>
> git-update-index ...
> git-diff HEAD^ > patchfile
> git-reset --hard HEAD^
> <internal push patch function - uses git-apply, git-{write,commit}-tree>

StGIT does a "git-diff | git-apply" during push as an optimisation.
Guilt moved the diff to the 'refresh' command. It might be a good idea
as the delay during 'push' is much more visible.

As I said in the past, the difference I see is that StGIT performs
three-way merging if simple git-apply fails, with the possibility
(configurable) of automatically starting a three-way merge tool like
xxdiff or emacs. Guilt could probably be modified but this information
needed for the three-way merge might be lost with text-only patches.

The rest is some extra functionality, StGIT being around for longer
(actually I think two months younger than the first official GIT
announcement).

-- 
Catalin

Re: stgit 0.13 import mbox problems

[Catalin Marinas]

On 23/08/07, Greg KH <greg@kroah.com> wrote:
> Also, it would import these patches, which are individually in mbox
> form, with the incorrect author information.  So I thought I would use
> the mbox form to make sure I wasn't just doing something stupid.

If the patches have a "From: ..." line after the subject (as described
in the submitting patches kernel docs), StGIT honours it.

Yet another way (might be even simpler for you) - use git-applymbox
(or git-quiltapply) and 'stg uncommit -n 177'. The former generates
GIT commits and the latter creates StGIT patches (without touching the
tree, HEAD remains the same and corresponds to the top patch). There
is also 'stg uncommit --to=commit-id'.

> > After an import failure, you can continue importing from the next
> > patch using the 'stg import --ignore' option.
>
> Ok, will try that, and your patch.

You can try the tonight snapshot which has this patch included (and
other fixes) - http://homepage.ntlworld.com/cmarinas/stgit/snapshots/stgit-20070823.tar.gz

> > One thing you'll notice is the speed difference as stgit has to
> > generate a git commit during a push operation.
>
> Oh yeah, I know that it would be slower, but I use git-quiltapply a lot
> for sending patches to Linus, and that seems quite fast (actually a lot
> faster than stgit for some reason...)

I should have a look at what git-quiltapply does as most of the StGIT
waiting time on external GIT invocation.

>         $ stg series -d
>         + add-my-version-to-the-kernel   | Add my version to the kernel.
>         + stupid-patch-for-my-laptop-whi | Stupid patch for my laptop which cant get sysrq-u
>         + gregs-test-driver-core-sysfs-s | Gregs test driver core / sysfs stress test module
>         > detect-atomic-counter-underflo | detect atomic counter underflows
>
> Are those spaces really the name of the patch?

No, the patch name is with the dashes, the rest is the patch subject line.

-- 
Catalin