[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/4] IOMMU: allow MSI message to IRTE propagation to fail

On 28/03/13 11:14, Jan Beulich wrote:
On 28.03.13 at 11:33, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
On 28/03/13 08:25, Jan Beulich wrote:
On 27.03.13 at 15:55, George Dunlap <george.dunlap@xxxxxxxxxxxxx> 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:


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":

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

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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.