xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@eu.citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH 1/4] IOMMU: allow MSI message to IRTE propagation to fail
Date: Thu, 28 Mar 2013 11:39:04 +0000	[thread overview]
Message-ID: <51542BD8.1000202@eu.citrix.com> (raw)
In-Reply-To: <5154340D02000078000C9345@nat28.tlf.novell.com>

On 28/03/13 11:14, 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.

I'm not sure why you think so.  First of all, when I say "special 
folder" I mean, "special gmail folder", which mutt access via the gmail 
IMAP interface.

Secondly, gmail's default is to treat all e-mail like just plain English 
text, in which whitespace and line breaks don't really matter that 
much.  So if I do copy+paste, or a normal "save", it mucks up all the 
whitespace, and the patch won't apply.

The alternate which gmail gives you is "show original", which shows the 
entire e-mail exactly as it received it -- including quoted-printable; 
in which case again the patch doesn't apply.

It seems perfectly reasonable to me for gmail to behave in this manner.  
FWIW Thunderbird behaves exactly the same way -- if you click "save 
e-mail" then it saves the mail exactly as it received it; if you do a 
copy-and-paste, it mucks up the whitespace.

> 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).
>
> 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.

Actually I completely agree with your assessment of git -- I'm sort of 
adapting to the "history rewriting" way of doing things, but I much 
prefer mercurial queues.

Mercurial queues, as I said, are almost exactly like quilt.  When you 
want to make a new patch, you type "hg qnew [filename]".  It will create 
the file in .hg/patches/ (instead of in patches/, as quilt does IIRC).  
The patches in the series are found in .hg/patches/series (instead of in 
patches/series.  You can directly edit this file and re-order or comment 
out patches which are not currently applied.  It will not complain if 
there is no description (just like quilt); however, if there *is* a 
description at the top of the patch, it will import that into the 
changeset when you do "hg qpush".  (Alternately, you can use "hg 
qrefresh -e" to edit the message at the top of the commit.  I find this 
more convenient.)

So basically, if you use hg patchqueues, you can use your existing 
workflow almost exactly:
* hg qnew / hg qrefresh / hg qpush / hg qpop just like quilt
* Edit patches directly, re-order patches by editing series, add patch 
comments directly, just like in quilt

But additionally, after you get your patch series the way you want it, 
you can just type "hg email -o", and it will e-mail the whole lot to the 
list.

The main thing to keep in mind is that if you edit the patches directly, 
you need to qpop and qpush them to get the changes into the mercurial 
changesets -- and that goes for the commit messages as well.  "hg 
e-mail" sends the *commits*, not the *patches*.  So if you edit the 
patches directly, you just need to pop and push all the paches first, 
which is as easy as "hg qpop -a ; hg qpush -a ; hg email -o"

There are similar options for git (guilt and stgit are some ones I've 
heard of), but I don't really know much about them.

>
>> If you're not willing to find a way to send it text/plain, could you
>> maybe at least name the attachments "01-[whatever].patch"
>> "02-[whatever].patch"?  I think that would help reduce the cognitive
>> load quite a bit.
> That would be possible, but making the patch mail composition
> more tedious for me. And while I'm all in favor of making others'
> work easier looking at stuff I'm interested in getting reviewed, I
> have some difficulty justifying my own effort needing to further
> increase to do so. If there was a way to make your and my life
> easier in this regard...

Well, the justification should be that it's more efficient for you to do 
it once per patch, than for every other reviewer to have to do it once 
per patch. :-)

But that may not actually be necessary; I've been mucking about with git 
and found some vestigal components that seem not to be in use anymore 
that actually understand quoted-printable.  They should be fairly simple 
to compose into a script that would do what I need.

  -George

  parent reply	other threads:[~2013-03-28 11:39 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 [this message]
2013-03-28 13:47               ` Stefano Stabellini
2013-05-06 20:25               ` Is: git send-email, patch sending, etc Was: " Konrad Rzeszutek Wilk
2013-05-07  8:53                 ` 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=51542BD8.1000202@eu.citrix.com \
    --to=george.dunlap@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=xen-devel@lists.xen.org \
    /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).