threads / patch / 4166

patchImplement git-quiltimport

Subject: [PATCH] Implement git-quiltimport

## tl;dr

24 messages between May 16, 2006 and Jun 2, 2006. Diffs are folded; open one to read it.

replies: 23people: 4as markdown or json

Eric W. Biederman· May 16, 2006, 16:51 UTC · lore

Importing a quilt patch series into git is not very difficult but parsing the patch descriptions and all of the other minutia take a bit of effort to get right, so this automates it.

Since git and quilt complement each other it makes sense to make it easy to go back and forth between the two.

---
Eric
 Documentation/git-quiltimport.txt |   50 +++++++++++++++++++++
 Makefile                          |    2 -
 git-quiltimport.sh                |   88 +++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/git-quiltimport.txt
 create mode 100644 git-quiltimport.sh
2256c7e9b3913732a5c3a2e54cdea20fc951b76d
Show changes to 3 files +139 −1

Documentation/git-quiltimport.txt, Makefile, git-quiltimport.sh

diff --git a/Documentation/git-quiltimport.txt b/Documentation/git-quiltimport.txt
new file mode 100644
index 0000000..8ea20eb
--- /dev/null
+++ b/Documentation/git-quiltimport.txt
@@ -0,0 +1,50 @@
+git-quiltimport(1)
+================
+
+NAME
+----
+git-quiltimport - Applies a quilt patchset onto the current branch
+
+
+SYNOPSIS
+--------
+[verse]
+'git-quiltimport' [--author <author>] [--patches <dir>]
+
+
+DESCRIPTION
+-----------
+Applies a quilt patchset onto the current git branch, preserving
+the patch boundaries, patch order, and patch descriptions present
+in the quilt patchset.
+
+For each patch the code attempts to extract the author from the 
+patch description.  If that fails it falls back to the author
+specified with --author.  If the --author flag was not given
+the the author is recorded as unknown.
+
+The patch name is preserved as the 1 line subject in the git
+description.
+
+OPTIONS
+-------
+--author Author Name <Author Email>::
+	The author name and email address to use when no author
+	information can be found in the patch description.
+
+--patches <dir>::
+	The directory to find the quilt patches and the
+	quilt series file.
+
+Author
+------
+Written by Eric Biederman <ebiederm@lnxi.com>
+
+Documentation
+--------------
+Documentation by Eric Biederman <ebiederm@lnxi.com>
+
+GIT
+---
+Part of the gitlink:git[7] suite
+
diff --git a/Makefile b/Makefile
index 37fbe78..1f4abe6 100644
--- a/Makefile
+++ b/Makefile
@@ -125,7 +125,7 @@ SCRIPT_SH = \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh git-grep.sh \
-	git-lost-found.sh
+	git-lost-found.sh git-quiltimport.sh
 
 SCRIPT_PERL = \
 	git-archimport.perl git-cvsimport.perl git-relink.perl \
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
new file mode 100644
index 0000000..534be82
--- /dev/null
+++ b/git-quiltimport.sh
@@ -0,0 +1,88 @@
+#!/bin/sh
+USAGE='--author <author> --patches </path/to/quilt/patch/directory>'
+SUBDIRECTORY_ON=Yes
+. git-sh-setup
+
+quilt_author="Unknown <unknown>"
+while case "$#" in 0) break;; esac
+do
+	case "$1" in
+	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
+		quilt_author=$(expr "$1" : '-[^=]*\(.*\)')
+		shift
+		;;
+	
+	--au|--aut|--auth|--autho|--author)
+		case "$#" in 1) usage ;; esac
+		shift
+		quilt_author="$1"
+		shift
+		;;
+
+	--pa=*|--pat=*|--patc=*|--patch=*|--patche=*|--patches=*)
+		QUILT_PATCHES=$(expr "$1" : '-[^=]*\(.*\)')
+		shift
+		;;
+	
+	--pa|--pat|--patc|--patch|--patche|--patches)
+		case "$#" in 1) usage ;; esac
+		shift
+		QUILT_PATCHES="$1"
+		shift
+		;;
+	
+	*)
+		break
+		;;
+	esac
+done
+
+# Quilt Author
+quilt_author_name=$(expr "z$quilt_author" : 'z\(.*[^ ]\) *<.*') &&
+quilt_author_email=$(expr "z$quilt_author" : '.*\(<.*\)') &&
+test '' != "$quilt_author_name" &&
+test '' != "$quilt_author_email" ||
+die "malformatted --author parameter"
+
+# Quilt patch directory
+: ${QUILT_PATCHES:=patches}
+if ! [ -d "$QUILT_PATCHES" ] ; then
+	echo "The \"$QUILT_PATCHES\" directory does not exist."
+	exit 1
+fi
+
+# Temporay directories
+tmp_dir=.dotest
+tmp_msg="$tmp_dir/msg"
+tmp_patch="$tmp_dir/patch"
+tmp_info="$tmp_dir/info"
+
+
+# Find the intial commit
+commit=$(git-rev-parse HEAD)
+
+mkdir $tmp_dir || exit 2
+cat "$QUILT_PATCHES/series" | grep -v '^#' | 
+while read line ; do 
+	echo $line
+	(cat $QUILT_PATCHES/$line | git-mailinfo "$tmp_msg" "$tmp_patch" > "$tmp_info") || exit 3
+	
+	# Parse the author information
+	export GIT_AUTHOR_NAME=$(sed -ne 's/Author: //p' "$tmp_info")
+	export GIT_AUTHOR_EMAIL=$(sed -ne 's/Email: //p' "$tmp_info")
+	if [ -z "$GIT_AUTHOR_EMAIL" ] ; then
+		GIT_AUTHOR_NAME=$quilt_author_name
+		GIT_AUTHOR_EMAIL=$quilt_author_email
+	fi
+	export GIT_AUTHOR_DATE=$(sed -ne 's/Date: //p' "$tmp_info")
+	export SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
+	if [ -z "$SUBJECT" ] ; then
+		SUBJECT=$(echo $line | sed -e 's/.patch$//')
+	fi
+
+	git-apply --index -C1 "$tmp_patch" &&
+	tree=$(git-write-tree) &&
+	commit=$((echo "$SUBJECT"; echo; cat "$tmp_msg") | git-commit-tree $tree -p $commit) &&
+	git-update-ref HEAD $commit || exit 4
+done
+rm -rf $tmp_dir || exit 5
-- 
1.3.2.g2256
Linus Torvalds· May 16, 2006, 17:03 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport

On Tue, 16 May 2006, Eric W. Biederman wrote:
>
> If the --author flag was not given the the author is recorded as 
> unknown.

Please don't do this. Just error out. It would be horrible to have a quilt import "succeed", and then later notice that some of the patches had incorrect authorship attribution just because the import script didn't check it, but just made it "unknown".

An un-attributed patch is simply not acceptable in any serious project. It's much better to consider it an error than to say "ok".

		Linus
Eric W. Biederman· May 16, 2006, 17:53 UTC · re: Linus Torvalds · lore

Re: [PATCH] Implement git-quiltimport

Linus Torvalds <torvalds@osdl.org> writes:
Show 12 quoted lines
> On Tue, 16 May 2006, Eric W. Biederman wrote:
>>
>> If the --author flag was not given the the author is recorded as 
>> unknown.
>
> Please don't do this. Just error out. It would be horrible to have a quilt 
> import "succeed", and then later notice that some of the patches had 
> incorrect authorship attribution just because the import script didn't 
> check it, but just made it "unknown".
>
> An un-attributed patch is simply not acceptable in any serious project. 
> It's much better to consider it an error than to say "ok".
There are two practical problems with this.
1) quilt does not force any authorship information to be preserved,
   in the description, so this probably a common case.  Although for
   most users just needing to specify --author sounds reasonable.
2) There are currently 84 out of roughly 1322 patches in
   2.6.17-rc4-mm1 that git-mailinfo cannot compute the author for.
   Generally the information is there but in such an irregular form
   that it cannot be automatically detected.
   If we can resolve that problem I am willing to make it an error.
   If we can't then sucking quilt patches into a git tree is much
   less useful.  
   Given the ugliness in -mm making it an error to have an
   non-attributed patch would result in people specifying --author
   when they really don't know who the author is, giving us much
   less reliable information.
   Possibly what we need is an option to not make it an error so that
   people doing this kind of thing in their own trees have useful
   information.

The list of patches that git-mailinfo cannot find authorship information for from 2.6.17-rc4-mm1 is included below. Mostly these are either git trees splatted into a single file, or simply fixes added by Andrew. But there are some like: gregkh-usb-usb-gotemp that have no description at all and only the patch name records who made the patch.

A really ugly case is acx1xx-wireless-driver patch, which appears to have multiple authors and a serious history before Andrew got it.

Show 96 quoted lines
>From acx1xx-wireless-driver.patch
> acx100.sourceforge.net (Andreas Mohr <andi@rhlx01.fht-esslingen.de>) ->
>   -> Denis Vlasenko <vda@ilport.com.ua>
>      -> Jeff Garzik <jgarzik@pobox.com>
>         -> me
> 
> DESC
> acx1xx-wireless-driver-usb-is-bust
> EDESC
> From: Andrew Morton <akpm@osdl.org>
> 
> drivers/net/wireless/tiacx/usb.c:1116: `URB_ASYNC_UNLINK' undeclared (first use in this function)
> 
> Cc: Denis Vlasenko <vda@ilport.com.ua>
> DESC
> acx1xx-allow-modular-build
> EDESC
> From: Andrew Morton <akpm@osdl.org>
> DESC
> acx1xx-wireless-driver-spy_offset-went-away
> EDESC
> From: Andrew Morton <akpm@osdl.org>
> 
> Cc: Denis Vlasenko <vda@ilport.com.ua>
> DESC
> acx update
> EDESC
> From: Denis Vlasenko <vda@ilport.com.ua>
> 
> > > Attached is a patch which updates acx. All your changes are
> > > included too. allyesconfig build is fixed by unifying
> > > PCI and USB modules into one. 'acx_debug' parameter is renamed back
> > > to just 'debug' (because all previous versions used it and
> > > we don't want to add to user confusion).
> > >
> > > Please apply.
> > >
> > > Signed-off-by: Denis Vlasenko <vda@ilport.com.ua>
> >
> > I missed a spy_offset fix. Updated patch is attached.
> > Also it is at
> > http://195.66.192.167/linux/acx_patches/linux-2.6.13-mm2acx-2.patch.bz2
> 
> Oh no. Yes. I forgot to remove some standalone build aids.
> 
> DESC
> acx-update 2
> EDESC
> From: Denis Vlasenko <vda@ilport.com.ua>
> 
> [20051016] 0.3.13
> * Revert 20051013 fix, we have one which actually works.
>   Thanks Jacek Jablonski <yacek87@gmail.com> for testing!
> 
> [20051013]
> * trying to fix "yet another similar bug"
> * usb fix by Carlos Martin
> 
> [20051012] 0.3.12
> * acx_l_clean_tx_desc bug fixed - was stopping tx completely
>   at high load. (It seems there exists yet another similar bug!)
> * "unknown IE" dump was 2 bytes too short - fixed
> * DUP logging made less noisy
> * another usb fix by Carlos Martin <carlosmn@gmail.com>
> 
> [20051003]
> * several usb fixes by Carlos Martin <carlosmn@gmail.com> - thanks!
> * unknown IE logging made less noisy
> * few unknown IEs added to the growing collection
> * version bump to 0.3.11
> 
> [20050916]
> * fix bogus MTU handling, add ability to change MTU
> * fix WLAN_DATA_MAXLEN: 2312 -> 2304
> * version bump to 0.3.10
> 
> [20050915]
> * by popular request default mode is 'managed'
> * empty handler for EID 7 (country info) is added
> * fix 'timer not started - iface is not up'
> * tx[host]desc micro optimizations
> * version bump to 0.3.9
> 
> [20050914]
> * tx[host]desc ring workings brought a bit back to two-hostdesc
>   scheme. This is an attempt to fix weird WG311v2 bug.
>   I still fail to understand how same chip with same fw can
>   work for me but do not work for a WG311v2 owner. Mystery.
> * README updated
> * version bump to 0.3.8
> 
> [20050913]
> * variable and fields with awful names renamed
> * a few fields dropped (they had constant values)
> * small optimization to acx_l_clean_tx_desc()
> * version bump to 0.3.7
      origin
      git-acpi
      git-agpgart
      git-alsa
      git-block
      git-cfq
      git-cifs
      git-dvb
      git-gfs2
      git-ia64
      git-ieee1394
      git-infiniband
      git-intelfb
      sane-menuconfig-colours
      git-klibc
      git-hdrcleanup
      git-hdrinstall
      git-libata-all
      libata_resume_fix
      git-mips
      git-mtd
      git-netdev-all
      git-nfs
      git-ocfs2
      git-powerpc
      git-rbtree
      git-sas
      gregkh-pci-acpiphp-configure-_prt-v3
      gregkh-pci-acpiphp-hotplug-slot-hotplug
      gregkh-pci-acpiphp-host-and-p2p-hotplug
      gregkh-pci-acpiphp-turn-off-slot-power-at-error-case
      gregkh-pci-pci-legacy-i-o-port-free-driver-changes-to-generic-pci-code
      gregkh-pci-pci-legacy-i-o-port-free-driver-update-documentation-pci_txt
      gregkh-pci-pci-legacy-i-o-port-free-driver-make-intel-e1000-driver-legacy-i-o-port-free
      gregkh-pci-pci-64-bit-resources-drivers-pci-changes
      gregkh-pci-pci-64-bit-resources-drivers-media-changes
      gregkh-pci-pci-64-bit-resources-drivers-net-changes
      gregkh-pci-pci-64-bit-resources-drivers-pcmcia-changes
      gregkh-pci-pci-64-bit-resources-drivers-others-changes
      gregkh-pci-pci-msi-abstractions-and-support-for-altix
      git-pcmcia
      git-scsi-target
      gregkh-usb-usb-gotemp
      git-supertrak
      git-watchdog
      x86_64-mm-defconfig-update
      x86_64-mm-memset-always-inline
      x86_64-mm-amd-core-cpuid
      x86_64-mm-amd-cpuid4
      x86_64-mm-alternatives
      x86_64-mm-ia32-unistd-cleanup
      x86_64-mm-topology-comment
      x86_64-mm-new-compat-ptrace
      x86_64-mm-disable-agp-resource-check
      x86_64-mm-new-northbridge
      x86_64-mm-iommu-warning
      x86_64-mm-i386-up-generic-arch
      x86_64-mm-iommu-enodev
      x86_64-mm-compat-printk
      x86_64-mm-i386-numa-summit-check
      x86_64-mm-fix-b44-checks
      x86_64-mm-nommu-warning
      git-cryptodev
      mm
      acx1xx-wireless-driver
      reiser4-export-find_get_pages
      kgdb-core-lite
      kgdb-8250
      kgdb-netpoll_pass_skb_to_rx_hook
      kgdb-eth
      kgdb-i386-lite
      kgdb-cfi_annotations
      kgdb-sysrq_bugfix
      kgdb-module
      kgdb-core
      kgdb-i386
      journal_add_journal_head-debug
      list_del-debug
      unplug-can-sleep
      firestream-warnings
      git-viro-bird-m32r
      git-viro-bird-m68k
      git-viro-bird-frv
      git-viro-bird-upf
      git-viro-bird-volatile
Eric
Junio C Hamano· May 16, 2006, 19:01 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport

ebiederm@xmission.com (Eric W. Biederman) writes:
Show 8 quoted lines
>    Given the ugliness in -mm making it an error to have an
>    non-attributed patch would result in people specifying --author
>    when they really don't know who the author is, giving us much
>    less reliable information.
>
>    Possibly what we need is an option to not make it an error so that
>    people doing this kind of thing in their own trees have useful
>    information.

I agree it is probably a good way to error by default, optinally allowing to say "don't care". I do not think Linus would pull from such a tree or trees branched from it into his official tree, so I do not think we would need to worry about commits with incomplete information propagating for this particular "gitified mm" usage. But as a general purpose tool to produce "gitified quilt series" tree, we would.

It depends on the expected use of the resulting gitified mm tree.

If it is for an individual developer to futz with and tweak upon, and the end result from the work leaves such a "gitified quilt series" repository only as a patch form, then not having to figure out and specify authorship information to many patches is probably a plus; the information will not be part of the official history recorded elsewhere anyway.

However, if it is to produce a reference git tree to point people at, (i.e. the quiltimport script is run once per a series by somebody and the result is published for public use), I would imagine we would want to have the attribution straight, so if the tool has to "guess", it should either error out or go interactive and ask.

Eric W. Biederman· May 17, 2006, 05:17 UTC · re: Junio C Hamano · lore

Re: [PATCH] Implement git-quiltimport

Junio C Hamano <junkio@cox.net> writes:
Show 28 quoted lines
> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>>    Given the ugliness in -mm making it an error to have an
>>    non-attributed patch would result in people specifying --author
>>    when they really don't know who the author is, giving us much
>>    less reliable information.
>>
>>    Possibly what we need is an option to not make it an error so that
>>    people doing this kind of thing in their own trees have useful
>>    information.
>
> I agree it is probably a good way to error by default, optinally
> allowing to say "don't care".  I do not think Linus would pull
> from such a tree or trees branched from it into his official
> tree, so I do not think we would need to worry about commits
> with incomplete information propagating for this particular
> "gitified mm" usage.  But as a general purpose tool to produce
> "gitified quilt series" tree, we would.
>
> It depends on the expected use of the resulting gitified mm
> tree.
>
> If it is for an individual developer to futz with and tweak
> upon, and the end result from the work leaves such a "gitified
> quilt series" repository only as a patch form, then not having
> to figure out and specify authorship information to many patches
> is probably a plus; the information will not be part of the
> official history recorded elsewhere anyway.

So what we need for this case really is a way to mark the commit objects in such a way that git-fetch or git-merge would choke on the commit objects and refuse to merge. That way the changes could not easily propagate in the wild. Every user would at least have to specify a non-default option, that Linus at least would never specify.

This scenario is how I have been primarily using such a tree.
Show 6 quoted lines
> However, if it is to produce a reference git tree to point
> people at, (i.e. the quiltimport script is run once per a series
> by somebody and the result is published for public use), I would
> imagine we would want to have the attribution straight, so if
> the tool has to "guess", it should either error out or go
> interactive and ask.

Reasonable. Going interactive is probably the best way since it appears that the patches do have sufficient information to derive the user information from. I know people have at various times in the past made the Andrews tree available in git form so you could do things like git-bisect, etc. So I think we need to address this issue. Probably by at least asking Andrew about it.

I will take a look at the policy and see what I can do. At the very least the default we be to error on such a tree.

..

A infrastructure question came to me when looking at this: several of the patches are from a branch with several authors. How do we specify a commit in git with several authors?

There are cases when you have enough collaboration that even a single patch could have multiple authors, contributing equally.

Eric
Junio C Hamano· May 17, 2006, 05:31 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport

ebiederm@xmission.com (Eric W. Biederman) writes:
Show 6 quoted lines
> A infrastructure question came to me when looking at this:
> several of the patches are from a branch with several authors.
> How do we specify a commit in git with several authors?
>
> There are cases when you have enough collaboration that even
> a single patch could have multiple authors, contributing equally.

The object format allows one author and one committer, but they are only used for human consumption and log summarizing purposes by the core. We could extend it to support more than one but I doubt it is worth it.

I would say it would be best to place the primary contact person, incase somebody has a problem with that particular patch done by such a group, on the author line. Listing everybody involved to give credits to them at the end of the log message would also be a good idea, and that's where we usually record attribution, sign-offs and acked-bys.

Eric W. Biederman· May 17, 2006, 18:44 UTC · re: Junio C Hamano · lore

[PATCH] Implement git-quiltimport (take 2)

Importing a quilt patch series into git is not very difficult but parsing the patch descriptions and all of the other minutia take a bit of effort to get right, so this automates it.

Since git and quilt complement each other it makes sense to make it easy to go back and forth between the two.

If a patch is encountered that it cannot derive the author from the user is asked.

---
 Documentation/git-quiltimport.txt |   55 +++++++++++++++++++
 Makefile                          |    2 -
 git-quiltimport.sh                |  106 +++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/git-quiltimport.txt
 create mode 100644 git-quiltimport.sh
5041c213c1090007dac9c03049c18a1433ccbefc
Show changes to 3 files +162 −1

Documentation/git-quiltimport.txt, Makefile, git-quiltimport.sh

diff --git a/Documentation/git-quiltimport.txt b/Documentation/git-quiltimport.txt
new file mode 100644
index 0000000..e694537
--- /dev/null
+++ b/Documentation/git-quiltimport.txt
@@ -0,0 +1,55 @@
+git-quiltimport(1)
+================
+
+NAME
+----
+git-quiltimport - Applies a quilt patchset onto the current branch
+
+
+SYNOPSIS
+--------
+[verse]
+'git-quiltimport' [--author <author>] [--patches <dir>]
+
+
+DESCRIPTION
+-----------
+Applies a quilt patchset onto the current git branch, preserving
+the patch boundaries, patch order, and patch descriptions present
+in the quilt patchset.
+
+For each patch the code attempts to extract the author from the 
+patch description.  If that fails it falls back to the author
+specified with --author.  If the --author flag was not given
+the patch description is displayed and the user is asked to
+interactively enter the author of the patch.
+
+If a subject is not found in the patch description the patch name is
+preserved as the 1 line subject in the git description.
+
+OPTIONS
+-------
+--author Author Name <Author Email>::
+	The author name and email address to use when no author
+	information can be found in the patch description.
+
+--patches <dir>::
+	The directory to find the quilt patches and the
+	quilt series file.
+
+        The default for the patch directory is patches
+	or the value of the $QUILT_PATCHES environment
+	variable.
+
+Author
+------
+Written by Eric Biederman <ebiederm@lnxi.com>
+
+Documentation
+--------------
+Documentation by Eric Biederman <ebiederm@lnxi.com>
+
+GIT
+---
+Part of the gitlink:git[7] suite
+
diff --git a/Makefile b/Makefile
index 37fbe78..1f4abe6 100644
--- a/Makefile
+++ b/Makefile
@@ -125,7 +125,7 @@ SCRIPT_SH = \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
 	git-merge-resolve.sh git-merge-ours.sh git-grep.sh \
-	git-lost-found.sh
+	git-lost-found.sh git-quiltimport.sh
 
 SCRIPT_PERL = \
 	git-archimport.perl git-cvsimport.perl git-relink.perl \
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
new file mode 100644
index 0000000..be43f9d
--- /dev/null
+++ b/git-quiltimport.sh
@@ -0,0 +1,106 @@
+#!/bin/sh
+USAGE='--author <author> --patches </path/to/quilt/patch/directory>'
+SUBDIRECTORY_ON=Yes
+. git-sh-setup
+
+quilt_author=""
+while case "$#" in 0) break;; esac
+do
+	case "$1" in
+	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
+		quilt_author=$(expr "$1" : '-[^=]*\(.*\)')
+		shift
+		;;
+	
+	--au|--aut|--auth|--autho|--author)
+		case "$#" in 1) usage ;; esac
+		shift
+		quilt_author="$1"
+		shift
+		;;
+
+	--pa=*|--pat=*|--patc=*|--patch=*|--patche=*|--patches=*)
+		QUILT_PATCHES=$(expr "$1" : '-[^=]*\(.*\)')
+		shift
+		;;
+	
+	--pa|--pat|--patc|--patch|--patche|--patches)
+		case "$#" in 1) usage ;; esac
+		shift
+		QUILT_PATCHES="$1"
+		shift
+		;;
+	
+	*)
+		break
+		;;
+	esac
+done
+
+# Quilt Author
+if [ -n "$quilt_author" ] ; then
+	quilt_author_name=$(expr "z$quilt_author" : 'z\(.*[^ ]\) *<.*') &&
+	quilt_author_email=$(expr "z$quilt_author" : '.*<\([^>]*\)') &&
+	test '' != "$quilt_author_name" &&
+	test '' != "$quilt_author_email" ||
+	die "malformatted --author parameter"
+fi
+
+# Quilt patch directory
+: ${QUILT_PATCHES:=patches}
+if ! [ -d "$QUILT_PATCHES" ] ; then
+	echo "The \"$QUILT_PATCHES\" directory does not exist."
+	exit 1
+fi
+
+# Temporay directories
+tmp_dir=.dotest
+tmp_msg="$tmp_dir/msg"
+tmp_patch="$tmp_dir/patch"
+tmp_info="$tmp_dir/info"
+
+
+# Find the intial commit
+commit=$(git-rev-parse HEAD)
+
+mkdir $tmp_dir || exit 2
+for patch_name in $(cat "$QUILT_PATCHES/series" | grep -v '^#'); do
+	echo $patch_name
+	(cat $QUILT_PATCHES/$patch_name | git-mailinfo "$tmp_msg" "$tmp_patch" > "$tmp_info") || exit 3
+	
+	# Parse the author information
+	export GIT_AUTHOR_NAME=$(sed -ne 's/Author: //p' "$tmp_info")
+	export GIT_AUTHOR_EMAIL=$(sed -ne 's/Email: //p' "$tmp_info")
+	while test -z "$GIT_AUTHOR_EMAIL" && test -z "$GIT_AUTHOR_NAME" ; do
+		if [ -n "$quilt_author" ] ; then
+			GIT_AUTHOR_NAME="$quilt_author_name";
+			GIT_AUTHOR_EMAIL="$quilt_author_email";
+		else
+			echo "No author found in $patch_name";
+			echo "---"
+			cat $tmp_msg
+			echo -n "Author: ";
+			read patch_author
+
+			echo "$patch_author"
+
+			patch_author_name=$(expr "z$patch_author" : 'z\(.*[^ ]\) *<.*') &&
+			patch_author_email=$(expr "z$patch_author" : '.*<\([^>]*\)') &&
+			test '' != "$patch_author_name" &&
+			test '' != "$patch_author_email" &&
+			GIT_AUTHOR_NAME="$patch_author_name" &&
+			GIT_AUTHOR_EMAIL="$patch_author_email"
+		fi
+	done
+	export GIT_AUTHOR_DATE=$(sed -ne 's/Date: //p' "$tmp_info")
+	export SUBJECT=$(sed -ne 's/Subject: //p' "$tmp_info")
+	if [ -z "$SUBJECT" ] ; then
+		SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
+	fi
+
+	git-apply --index -C1 "$tmp_patch" &&
+	tree=$(git-write-tree) &&
+	commit=$((echo "$SUBJECT"; echo; cat "$tmp_msg") | git-commit-tree $tree -p $commit) &&
+	git-update-ref HEAD $commit || exit 4
+done
+rm -rf $tmp_dir || exit 5
-- 
1.3.2.g2256-dirty
Junio C Hamano· May 17, 2006, 18:51 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

ebiederm@xmission.com (Eric W. Biederman) writes:
Show 9 quoted lines
> Importing a quilt patch series into git is not very difficult
> but parsing the patch descriptions and all of the other
> minutia take a bit of effort to get right, so this automates it.
>
> Since git and quilt complement each other it makes sense
> to make it easy to go back and forth between the two.
>
> If a patch is encountered that it cannot derive the author
> from the user is asked.

What's the expected workflow for you to work on a 1300 patch series you get from Andrew in the next installment to deal with 88 unattributed patches? Answer the question 88 times and make sure you get the answers right every time? Or abort and hand-edit them to help mailinfo to notice the correct attribution and re-run?

I know I am guilty of suggesting "going interactive", but I have a feeling that having an optional file that maps patch-name to author might be easier to work with. If the old patches are recycled in the updated -mm set, you probably can reuse the mapping for them, adding entries for newly introduced "unnamed" patches as needed.

Eric W. Biederman· May 17, 2006, 19:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Junio C Hamano <junkio@cox.net> writes:
Show 18 quoted lines
> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Importing a quilt patch series into git is not very difficult
>> but parsing the patch descriptions and all of the other
>> minutia take a bit of effort to get right, so this automates it.
>>
>> Since git and quilt complement each other it makes sense
>> to make it easy to go back and forth between the two.
>>
>> If a patch is encountered that it cannot derive the author
>> from the user is asked.
>
> What's the expected workflow for you to work on a 1300 patch
> series you get from Andrew in the next installment to deal with
> 88 unattributed patches?  Answer the question 88 times and make
> sure you get the answers right every time?  Or abort and
> hand-edit them to help mailinfo to notice the correct
> attribution and re-run?

For the internal consumption case it isn't a big deal. I can specify --author with something bogus and it works.

There are a few tweaks that can be made to git-mailinfo to make it better at parsing information out of patches. I cut the list down to about 49 that way. I had it all of the way down to 1. But then I realized that the first Singed-off-by really doesn't accurately reflect the author. I suspect a few of my other teaks are equally suspicious.

Show 6 quoted lines
> I know I am guilty of suggesting "going interactive", but I have
> a feeling that having an optional file that maps patch-name to
> author might be easier to work with.  If the old patches are
> recycled in the updated -mm set, you probably can reuse the
> mapping for them, adding entries for newly introduced "unnamed"
> patches as needed.

Short of getting the script where it has a sane restart in the middle mode going interactive and asking questions makes a lot of sense. Especially with smaller trees.

For Andrews tree before I play anymore with technical solutions I need to talk to Andrew and see if we can improve the situation upstream. Possibly with a quilt-audit script that finds problem patches.

Eric
Junio C Hamano· May 17, 2006, 23:34 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

ebiederm@xmission.com (Eric W. Biederman) writes:
Show 11 quoted lines
> Junio C Hamano <junkio@cox.net> writes:
>
>> What's the expected workflow for you to work on a 1300 patch
>> series you get from Andrew in the next installment to deal with
>> 88 unattributed patches?  Answer the question 88 times and make
>> sure you get the answers right every time?  Or abort and
>> hand-edit them to help mailinfo to notice the correct
>> attribution and re-run?
>
> For the internal consumption case it isn't a big deal.  I
> can specify --author with something bogus and it works. 
Yes.
Show 10 quoted lines
>> I know I am guilty of suggesting "going interactive", but I have
>> a feeling that having an optional file that maps patch-name to
>> author might be easier to work with.  If the old patches are
>> recycled in the updated -mm set, you probably can reuse the
>> mapping for them, adding entries for newly introduced "unnamed"
>> patches as needed.
>
> Short of getting the script where it has a sane restart in the
> middle mode going interactive and asking questions makes a lot
> of sense.  Especially with smaller trees.

Yes perhaps on smaller trees, but that does not mean much. For smaller trees and/or smaller patch series almost anything would do.

How about doing something like this, so that the user can record the fixup information, especially with --dry-run patch? Then the next round from the updated -mm tree the user would not have to retype them again ("then..fi" part should be indented in the final version, but I did not want indentation changes to distract you):

 	# Parse the author information
 	export GIT_AUTHOR_NAME=$(sed -ne 's/Author: //p' "$tmp_info")
 	export GIT_AUTHOR_EMAIL=$(sed -ne 's/Email: //p' "$tmp_info")
+	already_tried_fixup=
 	while test -z "$GIT_AUTHOR_EMAIL" && test -z "$GIT_AUTHOR_NAME" ; do
 		if [ -n "$quilt_author" ] ; then
 			GIT_AUTHOR_NAME="$quilt_author_name";
 			GIT_AUTHOR_EMAIL="$quilt_author_email";
 		else
+			if test -z "$already_tried_fixup"
+			then
+				patch_author=`grep author-fixup "$patch_name"`
+				already_tried_fixup=t
+			fi
+			if test -z "$patch_author"
+			then
 			echo "No author found in $patch_name";
 			echo "---"
 			cat $tmp_msg
 			echo -n "Author: ";
 			read patch_author
+			fi
 
 			echo "$patch_author"
> For Andrews tree before I play anymore with technical solutions I
> need to talk to Andrew and see if we can improve the situation
> upstream.  Possibly with a quilt-audit script that finds problem
> patches.
Yes, that sounds very sensible.
Eric W. Biederman· May 18, 2006, 10:48 UTC · re: Junio C Hamano · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Junio C Hamano <junkio@cox.net> writes:
Show 30 quoted lines
> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Junio C Hamano <junkio@cox.net> writes:
>>
>>> What's the expected workflow for you to work on a 1300 patch
>>> series you get from Andrew in the next installment to deal with
>>> 88 unattributed patches?  Answer the question 88 times and make
>>> sure you get the answers right every time?  Or abort and
>>> hand-edit them to help mailinfo to notice the correct
>>> attribution and re-run?
>>
>> For the internal consumption case it isn't a big deal.  I
>> can specify --author with something bogus and it works. 
>
> Yes.
>
>>> I know I am guilty of suggesting "going interactive", but I have
>>> a feeling that having an optional file that maps patch-name to
>>> author might be easier to work with.  If the old patches are
>>> recycled in the updated -mm set, you probably can reuse the
>>> mapping for them, adding entries for newly introduced "unnamed"
>>> patches as needed.
>>
>> Short of getting the script where it has a sane restart in the
>> middle mode going interactive and asking questions makes a lot
>> of sense.  Especially with smaller trees.
>
> Yes perhaps on smaller trees, but that does not mean much.  For
> smaller trees and/or smaller patch series almost anything would
> do.

Yes, a smaller patch series, that is what I meant. Most quilt trees that I know about are in needed small.

Andrews is the only one I know of that has gets as far as sucking in other quilt trees.

Show 6 quoted lines
> How about doing something like this, so that the user can record
> the fixup information, especially with --dry-run patch?  Then
> the next round from the updated -mm tree the user would not have
> to retype them again ("then..fi" part should be indented in the
> final version, but I did not want indentation changes to
> distract you):

This might be a sane work flow. My imagination actually had the user making a copy of the quilt tree and editing it by hand. My --dry-run doesn't ask the question it just throws errors so --dry-run isn't quite the right name.

So I guess with something like --dry-run there isn't a restart problem. The question is if we don't edit the patches themselves where do we put your author-fixup tag? .dotest?

Eric
Greg KH· May 19, 2006, 23:58 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

On Thu, May 18, 2006 at 04:48:26AM -0600, Eric W. Biederman wrote:
Show 35 quoted lines
> Junio C Hamano <junkio@cox.net> writes:
> 
> > ebiederm@xmission.com (Eric W. Biederman) writes:
> >
> >> Junio C Hamano <junkio@cox.net> writes:
> >>
> >>> What's the expected workflow for you to work on a 1300 patch
> >>> series you get from Andrew in the next installment to deal with
> >>> 88 unattributed patches?  Answer the question 88 times and make
> >>> sure you get the answers right every time?  Or abort and
> >>> hand-edit them to help mailinfo to notice the correct
> >>> attribution and re-run?
> >>
> >> For the internal consumption case it isn't a big deal.  I
> >> can specify --author with something bogus and it works. 
> >
> > Yes.
> >
> >>> I know I am guilty of suggesting "going interactive", but I have
> >>> a feeling that having an optional file that maps patch-name to
> >>> author might be easier to work with.  If the old patches are
> >>> recycled in the updated -mm set, you probably can reuse the
> >>> mapping for them, adding entries for newly introduced "unnamed"
> >>> patches as needed.
> >>
> >> Short of getting the script where it has a sane restart in the
> >> middle mode going interactive and asking questions makes a lot
> >> of sense.  Especially with smaller trees.
> >
> > Yes perhaps on smaller trees, but that does not mean much.  For
> > smaller trees and/or smaller patch series almost anything would
> > do.
> 
> Yes, a smaller patch series, that is what I meant.
> Most quilt trees that I know about are in needed small.

$ quilt series | wc -l 207

And that is about "normal" for me. Sometimes it grows to about 500+ patches, but that only happens when there's a longer kernel release cycle.

Another tree that I work on all the time is about 700+ patches, and yet another 2000+. So you might re-evaluate your statement about "small" quilt series :)

In looking at your script, it doesn't seem to be able to handle patches in quilt that are in mbox format. Any thoughts to allow this to handle the attribution properly?

Right now my development flow has me converting my quilt tree to one big mbox file and then using 'git applymbox' to import it before asking Linus to pull from it.

With your script I could skip at least one step, which would save me some time...

thanks,
greg k-h
Eric W. Biederman· May 20, 2006, 02:42 UTC · re: Greg KH · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Greg KH <greg@kroah.com> writes:
Show 15 quoted lines
> On Thu, May 18, 2006 at 04:48:26AM -0600, Eric W. Biederman wrote:
>> 
>> Yes, a smaller patch series, that is what I meant.
>> Most quilt trees that I know about are  small.
>
> $ quilt series | wc -l
> 207
>
> And that is about "normal" for me.  Sometimes it grows to about 500+
> patches, but that only happens when there's a longer kernel release
> cycle.
>
> Another tree that I work on all the time is about 700+ patches, and yet
> another 2000+.  So you might re-evaluate your statement about "small"
> quilt series :)

Sure. On fixing the upstream attribution issue you and Andi Kleen look like people that are worth talking to, as there were several patches in Andrews tree from both of you that were lacking attribution.

> In looking at your script, it doesn't seem to be able to handle patches
> in quilt that are in mbox format.  Any thoughts to allow this to handle
> the attribution properly?
Mbox format but one patch per file, or multiple patches in one mbox file?

If it is one patch per file but with mbox headers, it is relatively simple to teach git-mailinfo to parse things in a slightly more intelligent way. I played with that but I didn't have any patches that helped with.

Show 6 quoted lines
> Right now my development flow has me converting my quilt tree to one big
> mbox file and then using 'git applymbox' to import it before asking
> Linus to pull from it.
>
> With your script I could skip at least one step, which would save me
> some time...
Sure. That is the point of making it generic.
Eric
Greg KH· May 20, 2006, 21:32 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

On Fri, May 19, 2006 at 08:42:38PM -0600, Eric W. Biederman wrote:
Show 21 quoted lines
> Greg KH <greg@kroah.com> writes:
> 
> > On Thu, May 18, 2006 at 04:48:26AM -0600, Eric W. Biederman wrote:
> >> 
> >> Yes, a smaller patch series, that is what I meant.
> >> Most quilt trees that I know about are  small.
> >
> > $ quilt series | wc -l
> > 207
> >
> > And that is about "normal" for me.  Sometimes it grows to about 500+
> > patches, but that only happens when there's a longer kernel release
> > cycle.
> >
> > Another tree that I work on all the time is about 700+ patches, and yet
> > another 2000+.  So you might re-evaluate your statement about "small"
> > quilt series :)
> 
> Sure.  On fixing the upstream attribution issue you and Andi Kleen 
> look like people that are worth talking to, as there were several
> patches in Andrews tree from both of you that were lacking attribution.

Yes, I know I don't put any headers on patches I create until they are ready to be sent upstream to Linus. I'll try to be better about that in the future, sorry.

Show 5 quoted lines
> > In looking at your script, it doesn't seem to be able to handle patches
> > in quilt that are in mbox format.  Any thoughts to allow this to handle
> > the attribution properly?
> 
> Mbox format but one patch per file, or multiple patches in one mbox file?

The patches are already in mbox format in each file, but I dump them all together into one big one, to preserve the proper order as defined in the quilt series file so that git gets it right.

> If it is one patch per file but with mbox headers, it is relatively
> simple to teach git-mailinfo to parse things in a slightly more intelligent
> way.  I played with that but I didn't have any patches that helped with.
Hm, I'll try playing with that.

If you want, just grab my quilt trees from kernel.org and play with them, they should all be in mbox format for the individual patches (with some exceptions as noted above, just kick me about them to get me to fix them...)

thanks,
greg k-h
Eric W. Biederman· May 21, 2006, 00:36 UTC · re: Greg KH · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Greg KH <greg@kroah.com> writes:
Show 12 quoted lines
> On Fri, May 19, 2006 at 08:42:38PM -0600, Eric W. Biederman wrote:
>
>> If it is one patch per file but with mbox headers, it is relatively
>> simple to teach git-mailinfo to parse things in a slightly more intelligent
>> way.  I played with that but I didn't have any patches that helped with.
>
> Hm, I'll try playing with that.
>
> If you want, just grab my quilt trees from kernel.org and play with
> them, they should all be in mbox format for the individual patches (with
> some exceptions as noted above, just kick me about them to get me to fix
> them...)
So I just grabbed the gregkh-2.6 set of patches and with an unmodified
git-mailinfo I only have problems with the following patches:
	gregkh/gkh-version.patch
	gregkh/sysfs-test.patch
	gregkh/gregkh-usb-minors.patch
	gregkh/gregkh-debugfs_example.patch
	gregkh/gpl_future-test.patch
	usb/usb-gotemp.patch
None of which actually have from headers.

Currently git-mailinfo distinguishes headers and non-headers by the presence of the first blank line. So it seems to work just fine on mbox format patches.

Eric
Junio C Hamano· May 21, 2006, 00:41 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

ebiederm@xmission.com (Eric W. Biederman) writes:
> Currently git-mailinfo distinguishes headers and non-headers by the
> presence of the first blank line.  So it seems to work just fine on
> mbox format patches.

The program was designed to be fed one e-mail a time (the intended way for it to work is that a wrapper script uses git-mailsplit to break mbox up and call git-mailinfo one by one).

Eric W. Biederman· May 21, 2006, 00:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Junio C Hamano <junkio@cox.net> writes:
Show 10 quoted lines
> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Currently git-mailinfo distinguishes headers and non-headers by the
>> presence of the first blank line.  So it seems to work just fine on
>> mbox format patches.
>
> The program was designed to be fed one e-mail a time (the
> intended way for it to work is that a wrapper script uses
> git-mailsplit to break mbox up and call git-mailinfo one by
> one).

In this case what is meant is a leading "From " header (no colon) at the start of the patch.

Where git-mailinfo is likely to fall down is more in the quilt patches from Andi Kleen. If you look at my quoted patch header below you will see the subject is a plain line, followed by a space followed by a from. On this example git-mailinfo works (except for picking up the subject) but it appears to be a fluke.

>From x86_64-mm-add-abilty-to-enable-disable-nmi-watchdog-from-sysfs.patch:
Show 20 quoted lines
> Add abilty to enable/disable nmi watchdog with sysctl
> 
> From: dzickus <dzickus@redhat.com>
> 
> Adds a new /proc/sys/kernel/nmi call that will enable/disable the nmi
> watchdog.
> 
> Signed-off-by:  Don Zickus <dzickus@redhat.com>
> Signed-off-by: Andi Kleen <ak@suse.de>
> 
> ---
>  arch/i386/kernel/nmi.c   |   52 +++++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86_64/kernel/nmi.c |   48 +++++++++++++++++++++++++++++++++++++++++++
>  include/asm-i386/nmi.h   |    1
>  include/asm-x86_64/nmi.h |    1
>  include/linux/sysctl.h   |    1
>  kernel/sysctl.c          |   11 +++++++++
>  6 files changed, 114 insertions(+)
> 
> Index: linux/arch/i386/kernel/nmi.c
Eric
Junio C Hamano· May 21, 2006, 01:02 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

ebiederm@xmission.com (Eric W. Biederman) writes:
Show 8 quoted lines
> Where git-mailinfo is likely to fall down is more in the quilt
> patches from Andi Kleen. If you look at my quoted patch header below
> you will see the subject is a plain line, followed by a space followed
> by a from.  On this example git-mailinfo works (except for picking up
> the subject) but it appears to be a fluke.
>
> From x86_64-mm-add-abilty-to-enable-disable-nmi-watchdog-from-sysfs.patch:
>

Yeah, that's right, but in a real mailbox wouldn't that line be prefixed with a '>' ;-)?

Eric W. Biederman· May 21, 2006, 01:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Junio C Hamano <junkio@cox.net> writes:
Show 13 quoted lines
> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Where git-mailinfo is likely to fall down is more in the quilt
>> patches from Andi Kleen. If you look at my quoted patch header below
>> you will see the subject is a plain line, followed by a space followed
>> by a from.  On this example git-mailinfo works (except for picking up
>> the subject) but it appears to be a fluke.
>>
>> From x86_64-mm-add-abilty-to-enable-disable-nmi-watchdog-from-sysfs.patch:
>>
>
> Yeah, that's right, but in a real mailbox wouldn't that line be
> prefixed with a '>' ;-)?

That last from line was my attribution. The first quoted line was the first line of the patch. In this context that was probably a little confusing.

Eric
Greg KH· Jun 1, 2006, 19:23 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport (take 2)

On Sat, May 20, 2006 at 06:36:53PM -0600, Eric W. Biederman wrote:
Show 25 quoted lines
> Greg KH <greg@kroah.com> writes:
> 
> > On Fri, May 19, 2006 at 08:42:38PM -0600, Eric W. Biederman wrote:
> >
> >> If it is one patch per file but with mbox headers, it is relatively
> >> simple to teach git-mailinfo to parse things in a slightly more intelligent
> >> way.  I played with that but I didn't have any patches that helped with.
> >
> > Hm, I'll try playing with that.
> >
> > If you want, just grab my quilt trees from kernel.org and play with
> > them, they should all be in mbox format for the individual patches (with
> > some exceptions as noted above, just kick me about them to get me to fix
> > them...)
> 
> So I just grabbed the gregkh-2.6 set of patches and with an unmodified
> git-mailinfo I only have problems with the following patches:
> 	gregkh/gkh-version.patch
> 	gregkh/sysfs-test.patch
> 	gregkh/gregkh-usb-minors.patch
> 	gregkh/gregkh-debugfs_example.patch
> 	gregkh/gpl_future-test.patch
> 	usb/usb-gotemp.patch
> 
> None of which actually have from headers.

Oops, sorry for the delay. I've now fixed up these patches (the ones in the gregkh/ directory are not ever going to be sent upstream, that's why they were missing headers, same for the gotemp driver.)

thanks,
greg k-h
Eric W. Biederman· Jun 2, 2006, 00:24 UTC · re: Greg KH · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Greg KH <greg@kroah.com> writes:
Show 15 quoted lines
>> 
>> So I just grabbed the gregkh-2.6 set of patches and with an unmodified
>> git-mailinfo I only have problems with the following patches:
>> 	gregkh/gkh-version.patch
>> 	gregkh/sysfs-test.patch
>> 	gregkh/gregkh-usb-minors.patch
>> 	gregkh/gregkh-debugfs_example.patch
>> 	gregkh/gpl_future-test.patch
>> 	usb/usb-gotemp.patch
>> 
>> None of which actually have from headers.
>
> Oops, sorry for the delay.  I've now fixed up these patches (the ones in
> the gregkh/ directory are not ever going to be sent upstream, that's
> why they were missing headers, same for the gotemp driver.)
No problem. I'm mostly on vacation at the moment.

Things like usb/usb-gotemp.path have been sucked into the -mm tree. So sometimes these things after being published do wind up down stream...

Eric
Eric W. Biederman· May 19, 2006, 09:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] Implement git-quiltimport (take 2)

Junio C Hamano <junkio@cox.net> writes:
Show 6 quoted lines
>> For Andrews tree before I play anymore with technical solutions I
>> need to talk to Andrew and see if we can improve the situation
>> upstream.  Possibly with a quilt-audit script that finds problem
>> patches.
>
> Yes, that sounds very sensible.

Anyway I had a first pass conversation with Andrew and it looks like he will start addressing the problem patches there. So things should be resolvable.

The final piece on my agenda for getting the proper authors from the -mm tree is coping with Andrew's git patches. They have enough information to determine the Author but they are really several different patches. Ideally I will be able recognize they are performing a pull from a git repository and get the same results.

Otherwise some other trick will be necessary.
Eric
Eric W. Biederman· May 17, 2006, 20:10 UTC · re: Junio C Hamano · lore

[PATCH] Implement a --dry-run option to git-quiltimport

Since large quilt trees like -mm can easily have patches without clear authorship information, add a --dry-run option to make the problem patches easy to find.

---

This patch should make it easy to communicate to Andrew and others exactly which patches there are problems with, and should make it possible to easily edit those patches before they are imported.

 Documentation/git-quiltimport.txt |    8 +++++++-
 git-quiltimport.sh                |   24 ++++++++++++++++++------
 2 files changed, 25 insertions(+), 7 deletions(-)
cb0ff8090e1492f177a521b01cf987c16b125d81
Show changes to 2 files +25 −7

Documentation/git-quiltimport.txt, git-quiltimport.sh

diff --git a/Documentation/git-quiltimport.txt b/Documentation/git-quiltimport.txt
index e694537..97f4071 100644
--- a/Documentation/git-quiltimport.txt
+++ b/Documentation/git-quiltimport.txt
@@ -9,7 +9,7 @@ git-quiltimport - Applies a quilt patchs
 SYNOPSIS
 --------
 [verse]
-'git-quiltimport' [--author <author>] [--patches <dir>]
+'git-quiltimport' [--dry-run] [--author <author>] [--patches <dir>]
 
 
 DESCRIPTION
@@ -29,6 +29,12 @@ preserved as the 1 line subject in the g
 
 OPTIONS
 -------
+--dry-run::
+	Walk through the patches in the series and warn
+	if we cannot find all of the necessary information to commit
+	a patch.  At the time of this writing only missing author
+	information is warned about.
+
 --author Author Name <Author Email>::
 	The author name and email address to use when no author
 	information can be found in the patch description.
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index be43f9d..476e078 100644
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
-USAGE='--author <author> --patches </path/to/quilt/patch/directory>'
+USAGE='--dry-run --author <author> --patches </path/to/quilt/patch/directory>'
 SUBDIRECTORY_ON=Yes
 . git-sh-setup
 
+dry_run=""
 quilt_author=""
 while case "$#" in 0) break;; esac
 do
@@ -19,6 +20,11 @@ do
 		shift
 		;;
 
+	--dry-run)
+		shift
+		dry_run=1
+		;;
+
 	--pa=*|--pat=*|--patc=*|--patch=*|--patche=*|--patches=*)
 		QUILT_PATCHES=$(expr "$1" : '-[^=]*\(.*\)')
 		shift
@@ -75,8 +81,12 @@ for patch_name in $(cat "$QUILT_PATCHES/
 		if [ -n "$quilt_author" ] ; then
 			GIT_AUTHOR_NAME="$quilt_author_name";
 			GIT_AUTHOR_EMAIL="$quilt_author_email";
+	    	elif [ -n "$dry_run" ]; then
+			echo "No author found in $patch_name" >&2;
+			GIT_AUTHOR_NAME="dry-run-not-found";
+			GIT_AUTHOR_EMAIL="dry-run-not-found";
 		else
-			echo "No author found in $patch_name";
+			echo "No author found in $patch_name" >&2;
 			echo "---"
 			cat $tmp_msg
 			echo -n "Author: ";
@@ -98,9 +108,11 @@ for patch_name in $(cat "$QUILT_PATCHES/
 		SUBJECT=$(echo $patch_name | sed -e 's/.patch$//')
 	fi
 
-	git-apply --index -C1 "$tmp_patch" &&
-	tree=$(git-write-tree) &&
-	commit=$((echo "$SUBJECT"; echo; cat "$tmp_msg") | git-commit-tree $tree -p $commit) &&
-	git-update-ref HEAD $commit || exit 4
+	if [ -z "$dry_run" ] ; then
+		git-apply --index -C1 "$tmp_patch" &&
+		tree=$(git-write-tree) &&
+		commit=$((echo "$SUBJECT"; echo; cat "$tmp_msg") | git-commit-tree $tree -p $commit) &&
+		git-update-ref HEAD $commit || exit 4
+	fi	
 done
 rm -rf $tmp_dir || exit 5
-- 
1.3.2.g5041c-dirty
Linus Torvalds· May 17, 2006, 14:28 UTC · re: Eric W. Biederman · lore

Re: [PATCH] Implement git-quiltimport

On Tue, 16 May 2006, Eric W. Biederman wrote:
Show 7 quoted lines
> 
> So what we need for this case really is a way to mark the
> commit objects in such a way that git-fetch or git-merge
> would choke on the commit objects and refuse to merge.
> That way the changes could not easily propagate in the wild.
> Every user would at least have to specify a non-default option,
> that Linus at least would never specify.
No.

What we need is for your git-quiltimport to not generate the bogus commits in the first place!

Make it a damn error already.

If you want to have some stupid default to a non-existing and non-supported user, then do

	git-quiltimport --author="Bad User <baduser@nosuchuser.com>"

and get over it. Don't make the quiltimport script _default_ to a totally idiotic model that will just screw people over.

Or keep it as your personal script. I'm just saying that it should sure as hell not be used by any sane person in the format it is in now. Because, _by_default_, it now does something insane, which is bad bad bad. Which means that it shouldn't be merged by Junio.

		Linus

← back to recent threads