xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
	Jacob Shin <jacob.shin@amd.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Xiantao Zhang <xiantao.zhang@intel.com>,
	xen-devel <xen-devel@lists.xen.org>
Subject: Is: git send-email, patch sending, etc Was: Re: [PATCH 1/4] IOMMU: allow MSI message to IRTE propagation to fail
Date: Mon, 6 May 2013 16:25:04 -0400	[thread overview]
Message-ID: <20130506202504.GA23361@phenom.dumpdata.com> (raw)
In-Reply-To: <5154340D02000078000C9345@nat28.tlf.novell.com>

On Thu, Mar 28, 2013 at 11:14:05AM +0000, Jan Beulich wrote:
> >>> On 28.03.13 at 11:33, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> > On 28/03/13 08:25, Jan Beulich wrote:
> >>>>> On 27.03.13 at 15:55, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> >>> Did you look at the mail in your mailreader, or in the raw mail format?
> >> In the mail reader of course (after all I expect you to use a mail
> >> client too). And as said, I saw some damage when looking at the
> >> copy on lists.xen.org.
> >>
> >>> If you're using your mail reader, it's probably interpreting the
> >>> wordwrap stuff properly.  The "raw" mail looks like this:
> >>>
> >>> http://marc.info/?l=xen-devel&m=136428861403115&q=raw 
> >>>
> >>> The above is what GMail sees if I click "show original", and also what
> >>> the Citrix mail system gives me if I save the mail as a file. This
> >>> mangling is apparently called "quoted-printable":
> >>> http://en.wikipedia.org/wiki/Quoted-printable 
> >>>
> >>> The problem is that "patch" (and thus "git apply", "git am", "hg
> >>> import", &c &c), not being a mail-reader, doesn't know how to de-mangle
> >>> stuff.
> >> And rightly so. But your mail client saving the mail should deal with
> >> this properly. (And besides, if you already save the mail, I don't
> >> see why you can't instead save the attachment).
> > 
> > This is already a longer discussion than I really wanted to have, but 
> > just so you have an idea what I'm on about, I'll explain the difference.
> > 
> > The key thing is that "git am" will take an mbox file with a series of 
> > patches and automatically make a commit for each one.  So my algorithm 
> > for reviewing patch series sent in text/plain is as follows:
> > 
> > 1. In Gmail, mark each patch in the series and save it to a special folder.
> 
> And in that operation, the quoted printable encoding should be
> undone. Which makes all of the rest of your mail more or less
> mute.
> 
> > 2. Open up mutt on my local box.  It will connect to gmail and open that 
> > folder.
> > 3. Mark each patch in the series and save it to a local folder in ~/mail/.
> > 4. Use git am to import the whole series as a sequence of commits.
> > 5. View the changeset "in situ" using various git commands ('git meld' 
> > is my favorite ATM).
> > 
> > Marking each one might take 10 seconds, and it's almost entirely 
> > brainless; the main "cognitive load" is just remembering the name that 
> > I've given the local mail folder.  A series of 40 patches takes 
> > basically no more cognitive load to download and import into my git tree 
> > than a single patch.
> > 
> > To view yours at the moment, I have to do the following:
> > 1. For each patch in the series, click to download the attachment and 
> > save it to a separate file
> > 2. Edit each file to include "From: Jan Beulich <JBeulich@suse.com>" at 
> > the top, so it looks sufficiently like an mbox that "git am" won't complain
> > 3. For each patch in the series, run "git am" on it individually.
> > 
> > So #1 is slightly more annoying, as saving is more like 2 seconds per 
> > mail and marking a message is like 0.5 seconds per mail.  But the big 
> > source of cognitive load is having to deal with the different name of 
> > each patch.  It's just that extra little bit when having to open the 
> > file to add the header, and particularly then having to figure out what 
> > order the patches go in.  It doesn't really take that much extra time, 
> > but that it takes attention to remember the filename, and this adds up 
> > for each patch in the series; so the longer the series, the more 
> > cognitive load it generates.
> > 
> > They've done studies that show that even a minimal amount of cognitive 
> > load has an effect on people's endurance for other cognitive 
> > activities.  This is why most successful people instinctively find a way 
> > to make the unimportant decisions in their lives really simple -- 
> > spending time thinking about what to wear or what to eat eats away at 
> > precious energy they would rather spend on running the country or 
> > whatever it is they're doing.
> > 
> > Given the limited amount of head-space I have for arbitrary strings of 
> > things, I'd prefer to spend it on actually understanding the patch, 
> > rather than on patch filenames if I can avoid it; that's why I brought 
> > it up.
> > 
> > It seems like having an automated way to send off an entire patch queue, 
> > rather than cutting and pasting and attaching each mail individually, 
> > would reduce the cognitive load for you as well (not to mention probably 
> > save you several minutes of your day).  git and mercurial both have 
> > really good integrated mechanisms to do that; both also have extensions 
> > that allow you interact with the repository just like you do with 
> > quilt.  I'm not familiar with the git ones, but the mercurial one uses 
> > almost exactly the same commands as quilt, but with "hg" instead of 
> > "quilt" at the front (if I remember quilt correctly -- it's been a long 
> > time since I used it).
> 
> Indeed this might save me some time when sending the patches.
> But would it not require more time when fiddling with the patches
> while putting them together? As an example, I don't normally
> write patch descriptions right away, but do so only when getting
> ready to submit the patches. With git wanting to create a commit
> out of everything, I have to then run extra git commands to get
> the description added to each patch. Whereas with quilt this is a
> simple editing of the patch file, easily cut-n-paste between
> different instances of the patch on different trees (which I
> particularly find myself in need of when producing security fixes
> and their backports).

I had a hit this issue in the past. The way I solved this is by
using a healthy mix of 'git commit -s' (from vim
using :!git add % then !git commit -s), then typing in a short
intro and usually with '*KONRAD* ADD MORE *', then following it
up with other patches. 

If at some point I realized I screwed up or was more happy with
the patches I would do '!git rebase -i HEAD^^^^^^' (the ^ is the
amount of patches back I want to go) and either redo the commits
or alter some of the patches. Sometimes that also meant split
a patch in two which requires hitting the shell right before
the offending git commit (so when you do git rebase -i <something>
you can then choose to hit the shell before a patch), then in the
shell do  bit more of 'git show <the offending git commit>  /tmp/a',
'cp /tmp/a /tmp/orig', then editing /tmp/a for the non-offending bits,
patch -p1 -R < /tmp/a, git add <files>, git commit , and then
contiuning with the original patch that will now have a conflict
(as the git rebase will try apply it).

It is similar to do doing mercurial with qpop, qpush, qrefresh and qdiff.

At the end of the day if I am happy, I can cherry-pick the patch
to other trees (git cherry-pick -x) and call it a day.

Or git send-email.


> 
> Similarly, fixing minor issues (including rejects because of changes
> to earlier files in the series) by editing a patch file is impossible
> with git afaict. And no, using git's merge functionality is of no
> help to me at all, not the least because of the close to unusable
> (to me at least) UIs of any Linux based editors I've come across
> so far. Yet if anything, a merge tool ought to be interactive.
> Merges that just leave awful marks in the merge output are
> pretty pointless imo.

I used kdiff3 for that. It does have a pretty good mechanism of helping
you find the base, local and remote and figuring out what goes where.

  parent reply	other threads:[~2013-05-06 20:25 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-26  8:51 [PATCH 0/4] x86/IOMMU: multi-vector MSI prerequisites Jan Beulich
2013-03-26  9:03 ` [PATCH 1/4] IOMMU: allow MSI message to IRTE propagation to fail Jan Beulich
2013-03-27 11:45   ` George Dunlap
2013-03-27 12:16     ` Jan Beulich
2013-03-27 14:45       ` Boris Ostrovsky
2013-03-27 14:55       ` George Dunlap
2013-03-28  8:25         ` Jan Beulich
2013-03-28  9:46           ` Tim Deegan
2013-03-28  9:49             ` George Dunlap
2013-03-28 10:33           ` George Dunlap
2013-03-28 11:14             ` Jan Beulich
2013-03-28 11:25               ` Tim Deegan
2013-03-28 11:39               ` George Dunlap
2013-03-28 13:47               ` Stefano Stabellini
2013-05-06 20:25               ` Konrad Rzeszutek Wilk [this message]
2013-05-07  8:53                 ` Is: git send-email, patch sending, etc Was: " Ian Campbell
2013-05-07  9:26                 ` Wei Liu
2013-05-07 14:03                   ` Konrad Rzeszutek Wilk
2013-03-27 17:26   ` George Dunlap
2013-03-28  8:27     ` Jan Beulich
2013-03-26  9:03 ` [PATCH 2/4] x86/MSI: cleanup to prepare for multi-vector MSI Jan Beulich
2013-03-26  9:04 ` [PATCH 3/4] AMD IOMMU: allocate IRTE entries instead of using a static mapping Jan Beulich
2013-04-02  8:38   ` [PATCH v2 " Jan Beulich
2013-04-11  0:34     ` Suravee Suthikulpanit
2013-03-26  9:05 ` [PATCH 4/4] AMD IOMMU: untie remap and vector maps Jan Beulich
2013-03-28 12:37   ` George Dunlap
2013-03-28 13:09     ` Jan Beulich
2013-03-28 13:40       ` George Dunlap
2013-03-29  5:18 ` [PATCH 0/4] x86/IOMMU: multi-vector MSI prerequisites Suravee Suthikulpanit
2013-03-29  5:45   ` Suravee Suthikulpanit
2013-04-02  8:39     ` Jan Beulich
2013-04-10 13:55       ` Jan Beulich
2013-04-10 14:38         ` Suravee Suthikulanit
2013-04-10 14:46           ` Jan Beulich
2013-04-11  1:51             ` Suravee Suthikulpanit
2013-04-11  7:13               ` Jan Beulich
2013-04-11 15:40                 ` suravee suthikulpanit
2013-04-11 16:11                   ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130506202504.GA23361@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=jacob.shin@amd.com \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=xen-devel@lists.xen.org \
    --cc=xiantao.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).