[35353] tinyca2 Lint Report

[Ryan Schmidt]

On Mar 26, 2008, at 11:28, Anders F Björklund wrote:

> Landon Fuller wrote:
>
>> On Mar 26, 2008, at 12:13 AM, Ryan Schmidt wrote:
>>
>>> Patchfile naming: The old guide was contradictory, and in one place,
>>> recommended the naming scheme "patch-*" while in another place it
>>> recommended "patch-*.diff". The new guide is now consistent in
>>> recommending "patch-*.diff".
>>
>> The original documentation (which I wrote) was originally
>> consistent with the FreeBSD
>> patch specification:
>> 	http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/
>> slow-patch.html
>>
>> We then adopted the semantic of patch-foobar.diff, where 'foobar'
>> was a feature that
>> covered many files -- this was due to the headache of patch-per-
>> file for a sizable set
>> of diffs.
>
> I agree with Landon here, the current lint warning seems misguided ?
>
> This was discussed at length before it was implemented too, with there
> being two different kinds of patches. But apparently "consistency"  
> once
> again triumphed, and one of the types of patch naming was warned  
> about.
> If anything, it should be eased up to allow anything like "patch-*"...
>
> We had "patch-foo.c" and "patch-bar.h", and also "patch-foobar.diff".
> i.e. either patch-FILENAME (BSD style) or patch-ISSUE.diff (multi- 
> file)

Thanks for clarifying that. I didn't realize the .diff extension was  
previously only used for multi-file patches, or rather, that the  
recommendation to use the .diff extension was meant only for multi- 
file patches. I had assumed someone had added something to the guide  
in one place and forgotten to update the recommendation in the other  
place. It seems an arbitrary distinction to me. A diff file is after  
all *always* a diff file, whether it patches one file or five. But at  
least now I understand the previously unexplained discrepancy in  
patchfile name recommendations.

I still believe diff/patch files should always have the .diff  
extension but I'll admit that having port lint warn about it is a bit  
pedantic. Let's change port lint to only check for "patch-*".

The guide doesn't mention the .diff extension for patchfiles at all.  
It has several examples of patchfile used in a port, none of which  
end in .diff. I would like to suggest that we change the guide to  
suggest using the .diff extension always. If people are against this  
suggestion, then we should at least add to the guide the recommended  
patchfile naming scheme for multi-file patches. I thought I had filed  
a ticket about this topic before but I can't find it now, but I would  
be happy to file one once we decide what the documentation should say.


>> My original newline complaint was:
>> 	Warning: Line 2 should be a newline (after RCS tag)
>>
>> 	# $Id: Portfile 35353 2008-03-25 18:13:44Z landonf at macports.org $
>> 	PortSystem      1.0
>>
>> Why does that matter?
>
> Actually the whitespace checks were originally supposed to be  
> optional,
> but I didn't know how to make "port lint" read options in Tcl... :-)
>
> The inspiration for the issued warnings was portlint(1), naturally
> enough.
> http://www.freebsd.org/doc/en/books/porters-handbook/porting-
> portlint.html
>
> Stylistic newlines could be made optional, with an extra port
> parameter ?

I don't remember why whitespace checks were originally put into port  
lint, with the exception of checking for trailing whitespace  
following a backslash which is indeed a fatal error. Unfortunately,  
the error is so fatal that if you put trailing whitespace after a  
backslash, port lint might not even run until you fix it. For  
example, if I put this in a portfile...

checksums   md5 d6ca3009eee24a8e396b8f667b3bd8df \
             sha1 38a94e4eefb3e262fbd0e2c7716ce721a6ecd73c \
             rmd160 86694271fd7b657c7ede77671844376322bb3fef

...but I have a trailing space after the first line, port lint says  
this:

$ port lint
Can't map the URL 'file://.' to a port description file ("wrong #  
args: should be "sha1 action ?file?"").
Please verify that the directory and portfile syntax are correct.
To use the current port, you must be in a port's directory.
(you might also see this message if a pseudo-port such as
outdated or installed expands to no ports).
can't read "portvariants": no such variable
Error: Status 1 encountered during processing.
$

We can change port lint to just check for trailing whitespace after a  
backslash. In light of the above, it seems it won't even get  
executed, but maybe we should still leave a check in there, in case  
we can some day improve port lint to be able to run in spite of such  
a syntax error.

The warnings about missing blank lines are a bit too pedantic even  
for my taste and I would remove them.

I attached new proposed patches to the ticket:

http://trac.macosforge.org/projects/macports/ticket/14799


While we're deciding all this, and until we release a new version of  
MacPorts containing these fixes, let's also turn off automatic port  
lint emails. I filed a ticket for that:

http://trac.macosforge.org/projects/macports/ticket/14817