Re: [RFC][PATCH] update SubmittingPatches to clarify attachment policy

["Randy.Dunlap" <rddunlap@osdl.org>]

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

On Wed, 04 May 2005 20:52:45 +0200
Krzysztof Halasa <khc@pm.waw.pl> wrote:

> There is MIME "Content-Disposition: inline".
> Personally I think it's at least as good as plain text - it's MIME
> attachment (you can extract automatically, you have well-defined patch
> boundaries, original file name etc.) _and_ mail readers are supposed to
> (and do) display such attachments as normal message parts.
> 
> The message is readable with MIME-unaware readers (scripts etc.) as well.
> 
> Such attachment (raw message data) looks like:
> 
[snip]
> 
> > +code.  If you must use an attachment,
> 
> > verify that it has no
> > +Content-Type-Encoding.
> 
> Content-Transfer-Encoding.
> 
> I'd say "verify that it's binary-encoded - quoted-printable and base64
> encodings are not permitted".
> 
> I.e., it's perfectly fine to specify "Content-Transfer-Encoding: 7bit"
> (or "8bit" or possibly "binary", though I don't exactly know the
> difference between "8bit" and "binary").
> 
> >  A MIME attachment also takes Linus a bit more
> > +time to process, decreasing the likelihood of your MIME-attached
> > +change being accepted.
> 
> I don't think so. Badly formatted MIME attachments, sure. I'd be
> surprised if Linus applies them at all.
> 
> >  Exception:  If your mailer is mangling patches then someone may ask
> > -you to re-send them using MIME.
> > +you to re-send them compressed or using other MIME encodings.
> 
> Rather: "... someone may ask you to re-send them as properly encoded
> MIME attachments".
> 
> 
> In fact I'd encourage using binary-encoded inlined MIME attachments
> at all times, with non-MIME 7bit or 8bit plain text being accepted
> as secondary format.


A couple of days ago, Matt Mackall described/proposed a tool to
check new patches for acceptable content and format:
  http://www.selenic.com/pipermail/kernel-mentors/2005-May/000072.html

I'm attaching a rudimentary version of such a tool (check-patch.pl).
It does not attempt to check for line wrapping or lines that are
> 80 characters.
It dislikes patches that contain attachments that are base64,
quoted-printable, or binary (e.g.).

People can run this script locally, but ideally We (royal) will
have an email address for it so that people can use it to check
if their mail interface munges the patch for them... :(
and can try again until it doesn't.

Yeah, it's not perfect and it's a bit verbose.
Patches accepted (or even complete rewrites :).

---
~Randy

[-- Attachment #2: check-patch.pl --]
[-- Type: application/octet-stream, Size: 2950 bytes --]

#! /usr/bin/perl
# Randy Dunlap <rddunlap@osdl.org>
# 2005-05-02
#
#Matt Mackall wrote:
#I think a mail robot that you could send patches to that would detect:
#- quoted printable
#- line wrapping
#- tab damage
#- wrong directory level
#- non-unidiff
#- added trailing whitespace
#- etc.
#- lines longer than 80 characters
#- use of // style comments
#
#..would be quite useful. Then we could just say "your patch is
#damaged, resend to patch-tester@foo.org until it says it looks ok".
#
#Any volunteers? 
#-- 

$tabcount = 0;
$spaces_run = 0;
$trailing_ws = 0;
$double_slashes = 0;
$bad_encodings = 0;
$not_unidiff = 0;
$not_pdiff = 0;
$bad_subdir = 0;

%kernel_dirs = ("arch", 1, "crypto", 1, "drivers", 1, "fs", 1,
		"include", 1, "init", 1, "ipc", 1, "kernel", 1,
		"lib", 1, "mm", 1, "net", 1, "scripts", 1,
		"security", 1, "sound", 1, "usr", 1, "Documentation", 1);

# get input file name:
$INPUTNAME = $ARGV[0];

if (length ($INPUTNAME) == 0) {
        print "usage: check-patch.pl patchfile\n\n";
        exit 1;
}

if (! open (INPUTNUM, "<$INPUTNAME")) {
        print "cannot open '$INPUTNAME'\n\n";
        exit 2;
}

print "$INPUTNAME :\n";

readpatch: while ($line = <INPUTNUM>)
{
	chomp $line;
	# print "deb: #line = {$line}\n";

	$tabcount++ if $line =~ /\t/;
	$spaces_run++ if $line =~ /    /;
	$trailing_ws++ if $line =~ /^\+.*[ \t]$/;
	$double_slashes++ if $line =~ /\/\//;

	if ($line =~ /Content-Transfer-Encoding:/i) {
		if (($line =~ /base64/i) ||
		    ($line =~ /binary/i) ||
		    ($line =~ /quoted-printable/i)) {
			$bad_encodings++;
			print "bad encoding: $line\n";
		}
	}

	if ($line =~ /^diff/) {
		if ($line !~ /-.*u/) {
			print "appears not to be a -u diff\n";
			$not_unidiff++;
		}
		if ($line !~ /-.*p/) {
			print "appears not to be a -p diff\n";
			$not_pdiff++;
		}
	}

	if ($line =~ /^--- / || $line =~ /^\+\+\+ /) {
		$filename = substr($line, 4);
		@diffline = split(' ', $filename); # filename date time
		@fnparts = split('/', $diffline[0]);
		$subdir = $fnparts[1];
		if ((substr($subdir, length($subdir) - 1, 1) eq '/') &&
		       ($kernel_dirs{$subdir} != 1)) {
			$ending = substr($subdir, length($subdir) - 1, 1);
			print "bad sub-dir level {$ending}[$subdir]: $line\n";
			$bad_subdir++;
		}
	}
} # end readpatch

close (INPUTNUM);

print "  tab count: $tabcount\n";
print "  spaces_run count: $spaces_run\n";

print "  trailing whitespace: $trailing_ws\n";
print "  bad encodings: $bad_encodings\n";
print "  '//' comments: $double_slashes\n";
print "  not -u diff: $not_unidiff\n";
print "  not -p diff: $not_pdiff\n";
print "  bad sub-dir level: $bad_subdir\n";

if ($tabcount == 0 && $spaces_run > 0) {
	print "  WARNING: no tabs found, lots of spaces found; not a good sign\n";
}

$errs = $trailing_ws + $bad_encodings + $double_slashes + $not_unidiff + $not_pdiff
	+ $bad_subdir;

if ($errs == 0) {
	print "  PASS (no errors)\n";
}
else {
	print "  ##### $errs error(s) #####\n";
}

# end.