NetBSD Problem Report #46570
From dholland@netbsd.org Sat Jun 9 20:46:00 2012
Return-Path: <dholland@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
by www.NetBSD.org (Postfix) with ESMTP id D974663B882
for <gnats-bugs@gnats.NetBSD.org>; Sat, 9 Jun 2012 20:45:59 +0000 (UTC)
Message-Id: <20120609204559.CA27C14A1D0@mail.netbsd.org>
Date: Sat, 9 Jun 2012 20:45:59 +0000 (UTC)
From: dholland@NetBSD.org
Reply-To: dholland@NetBSD.org
To: gnats-bugs@gnats.NetBSD.org
Subject: infelicities in pkglint
X-Send-Pr-Version: 3.95
>Number: 46570
>Category: pkg
>Synopsis: infelicities in pkglint
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: rillig
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat Jun 09 20:50:00 +0000 2012
>Closed-Date: Sun Jul 10 15:07:42 +0000 2016
>Last-Modified: Sun Jul 10 15:07:42 +0000 2016
>Originator: David A. Holland
>Release: pkgsrc of 20120609
>Organization:
>Environment:
irrelevant
>Description:
pkglint has a number of regrettable characteristics.
- It is written in perl and is thus difficult at best to maintain,
especially now that the original author has left the project.
- It is also 8500 lines of perl in a single file, which is a problem
in its own right.
- It does not understand USE_GAMESGROUP or its accompanying
paraphernalia, and issues a bucket of spurious warnings instead of
guiding correct usage. I looked into fixing this at one point and was
stopped by the first two issues.
- It has a system of permissions for make variable usage in various
contexts, which is fine, but when it warns about violations it reports
the permissions as single letters. The letters are not documented in
the man page. To find out what they mean, you need to UTSL (see points
1 and 2) or notice the -e option. It should use words in the
diagnostics, especially since we encourage novice packagers to use
pkglint.
- It will happily attempt to check gmake makefiles (which can appear
in a package's files/ directly) ... and then when it encounters gmake
syntax, which it doesn't understand, it doesn't even spew errors but
*aborts*.
- If it encounters a file called Makefile.PL it expects it to be a
makefile (rather than a perl script) and *aborts* when it finds the
syntax confusing.
- It doesn't understand the .-include "file" syntax and *aborts* upon
encountering it in a makefile.
- Currently when run in ruby18-base it somehow imagines that it should
be looking at ruby19-base's patches and spews about six screenfuls of
completely nonsensical errors.
- It complains incorrectly about not being in -e mode if it encounters
a makefile recipe with a shell for loop, because of the semicolon in
the for loop syntax.
- It is slow. Running it on a significant portion of the tree takes
minutes (until it crashes out from one of the above issues) at 85-95%
CPU on a decently fast machine.
- There is quite a bit more wrong but the preceding is enough to start
with for now.
>How-To-Repeat:
Run it a lot.
>Fix:
A good start would be rewriting it not in perl.
>Release-Note:
>Audit-Trail:
From: diro@nixsyspaus.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Tue, 26 Jun 2012 15:51:35 -0400
Agreed regarding rewriting it not in Perl. I've seen some inconsistencies in pkglint as well. Here are a few for now and i will trickle more in as i see them:
1) It doesn't understand FETCH_USING:
WARN: Makefile:7: FETCH_USING is defined but not used. Spelling mistake?
2) It did not throw an error on this line:
${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
Non-closed braces need to throw an error.
3) This line needs to throw an error too:
DEPENDS+= itstool[0-9]*:../../textproc/itstool
The lack of a dash between the package and version cause this dependency to
not be detected if it's installed (it's seen as a different package). pkglint
needs to tell the developer that dependencies must include a name and version.
These are seen with pkglint-4.106. Not sure if they've been fixed yet.
From: "Thomas Klausner" <wiz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint/files
Date: Mon, 9 Jul 2012 17:36:59 +0000
Module Name: pkgsrc
Committed By: wiz
Date: Mon Jul 9 17:36:59 UTC 2012
Modified Files:
pkgsrc/pkgtools/pkglint/files: pkglint.1
Log Message:
Document permissions. Addresses part of PR 46570.
To generate a diff of this commit:
cvs rdiff -u -r1.43 -r1.44 pkgsrc/pkgtools/pkglint/files/pkglint.1
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Thomas Klausner <wiz@NetBSD.org>
To: NetBSD bugtracking <gnats-bugs@NetBSD.org>
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 9 Jul 2012 19:42:24 +0200
I'll not comment further about the complaints about the language,
because it is a well-written script in a language intended for
handling text, and does it job well for single packages.
On Sat, Jun 09, 2012 at 08:50:01PM +0000, David Holland wrote:
> - It does not understand USE_GAMESGROUP or its accompanying
> paraphernalia, and issues a bucket of spurious warnings instead of
> guiding correct usage. I looked into fixing this at one point and was
> stopped by the first two issues.
I've just fixed these issues. USE_GAMESGROUP wasn't even documented in
the guide, so I fixed that as welll.
> - It has a system of permissions for make variable usage in various
> contexts, which is fine, but when it warns about violations it reports
> the permissions as single letters. The letters are not documented in
> the man page. To find out what they mean, you need to UTSL (see points
> 1 and 2) or notice the -e option. It should use words in the
> diagnostics, especially since we encourage novice packagers to use
> pkglint.
I've added a reference to -e in the final output line if any problems
were found, and documented the permissions in the man page.
> - It will happily attempt to check gmake makefiles (which can appear
> in a package's files/ directly) ... and then when it encounters gmake
> syntax, which it doesn't understand, it doesn't even spew errors but
> *aborts*.
I don't see how this is a problem, since this is intended for checking
pkgsrc Makefiles and only to be called from pkgsrc/category/program,
where no GNU makefile should ever reside.
> - If it encounters a file called Makefile.PL it expects it to be a
> makefile (rather than a perl script) and *aborts* when it finds the
> syntax confusing.
I don't see how this is a problem, since this is intended for checking
pkgsrc Makefiles and only to be called from pkgsrc/category/program,
where no Perl Makefile.PL should ever reside.
> - It doesn't understand the .-include "file" syntax and *aborts* upon
> encountering it in a makefile.
I don't see how this is a problem, since this is intended for checking
pkgsrc Makefiles and only to be called from pkgsrc/category/program,
where I didn't find any occurrence of ".-include" in the current
pkgsrc tree, nor do I expect any to appear.ever reside.
> - Currently when run in ruby18-base it somehow imagines that it should
> be looking at ruby19-base's patches and spews about six screenfuls of
> completely nonsensical errors.
I can't reproduce this on ruby18-base nor ruby19-base, so I suspect
this has been fixed in the meantime.
> - It complains incorrectly about not being in -e mode if it encounters
> a makefile recipe with a shell for loop, because of the semicolon in
> the for loop syntax.
Can you please give me an example package where I can reproduce this?
Cheers,
Thomas
From: "Thomas Klausner" <wiz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint/files
Date: Mon, 9 Jul 2012 17:47:37 +0000
Module Name: pkgsrc
Committed By: wiz
Date: Mon Jul 9 17:47:37 UTC 2012
Modified Files:
pkgsrc/pkgtools/pkglint/files: makevars.map
Log Message:
Recognize FETCH_USING as user-settable variable, as intended.
Addresses a comment by diro in PR 46570.
To generate a diff of this commit:
cvs rdiff -u -r1.221 -r1.222 pkgsrc/pkgtools/pkglint/files/makevars.map
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Thomas Klausner" <wiz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint
Date: Mon, 9 Jul 2012 18:14:35 +0000
Module Name: pkgsrc
Committed By: wiz
Date: Mon Jul 9 18:14:35 UTC 2012
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: pkglint.pl
Log Message:
Allow "." in package names (needed e.g. for gst-plugins0.10-base).
Check package patterns in DEPENDS.
Requested by diro in PR 46570.
Bump version.
To generate a diff of this commit:
cvs rdiff -u -r1.403 -r1.404 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.834 -r1.835 pkgsrc/pkgtools/pkglint/files/pkglint.pl
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Mon, 09 Jul 2012 18:18:02 +0000
State-Changed-Why:
Waiting for example for shell loop issue.
Responsible-Changed-From-To: pkg-manager->wiz
Responsible-Changed-By: wiz@NetBSD.org
Responsible-Changed-When: Mon, 09 Jul 2012 18:18:41 +0000
Responsible-Changed-Why:
Might as well take it.
From: Thomas Klausner <wiz@NetBSD.org>
To: NetBSD bugtracking <gnats-bugs@NetBSD.org>
Cc: diro@nixsyspaus.org
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Mon, 9 Jul 2012 20:17:26 +0200
> 1) It doesn't understand FETCH_USING:
>
> WARN: Makefile:7: FETCH_USING is defined but not used. Spelling mistake?
I've added support for FETCH_USING to pkglint. (I've marked it as
intended, i.e. currently with -Wall it will complain about packages
that set it, since it shouldn't be used in package Makefiles; that's
just a workaround until the infrastructure supports https better.
Without -Wall pkglint won't complain about it.)
> 2) It did not throw an error on this line:
>
> ${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
>
> Non-closed braces need to throw an error.
It can't be a complete Makefile parser, and even make(1) doesn't complain about
FOO= ${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
I see this as WONTFIX.
> 3) This line needs to throw an error too:
>
> DEPENDS+= itstool[0-9]*:../../textproc/itstool
>
> The lack of a dash between the package and version cause this dependency to
> not be detected if it's installed (it's seen as a different package). pkglint
> needs to tell the developer that dependencies must include a name and version.
I've added pattern checking for DEPENDS lines.
It now complains about all of these cases:
DEPENDS+= broken0.12.1:../../x11/alacarte
DEPENDS+= broken[0-9]*:../../x11/alacarte
DEPENDS+= broken[0-9]*../../x11/alacarte
DEPENDS+= broken>=:../../x11/alacarte
DEPENDS+= broken=0:../../x11/alacarte
DEPENDS+= broken=:../../x11/alacarte
DEPENDS+= broken-:../../x11/alacarte
DEPENDS+= broken>:../../x11/alacarte
Thanks for the suggestions!
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 9 Jul 2012 20:32:25 +0000
On Mon, Jul 09, 2012 at 05:45:02PM +0000, Thomas Klausner wrote:
> I'll not comment further about the complaints about the language,
> because it is a well-written script in a language intended for
> handling text, and does it job well for single packages.
As long as there's someone maintaining it, I don't care what language
it's written in. Heck, translate it to befunge or INTERCAL for all I
care.
However, if (as has been the case for some time before today)
maintenance is left up to passersby and passersby are not able to
successfully modify the thing, it constitutes a problem.
> > - It does not understand USE_GAMESGROUP or its accompanying
> > paraphernalia, and issues a bucket of spurious warnings instead of
> > guiding correct usage. I looked into fixing this at one point and was
> > stopped by the first two issues.
>
> I've just fixed these issues. USE_GAMESGROUP wasn't even documented in
> the guide, so I fixed that as welll.
>
> > - It has a system of permissions for make variable usage in various
> > contexts, which is fine, but when it warns about violations it reports
> > the permissions as single letters. The letters are not documented in
> > the man page. To find out what they mean, you need to UTSL (see points
> > 1 and 2) or notice the -e option. It should use words in the
> > diagnostics, especially since we encourage novice packagers to use
> > pkglint.
>
> I've added a reference to -e in the final output line if any problems
> were found, and documented the permissions in the man page.
That's a start. How about having it output words? Then it wouldn't be
necessary to look them up or explain them.
> > - It will happily attempt to check gmake makefiles (which can appear
> > in a package's files/ directly) ... and then when it encounters gmake
> > syntax, which it doesn't understand, it doesn't even spew errors but
> > *aborts*.
>
> I don't see how this is a problem, since this is intended for checking
> pkgsrc Makefiles and only to be called from pkgsrc/category/program,
> where no GNU makefile should ever reside.
They can and do reside in files/, and when invoked with -Call as is
usually recommended, pkglint takes it upon itself to inspect the
contents of files/ and becomes confused. For example:
% cd archivers/libarchive/
% pkglint -Call
FATAL: files/Makefile.am:175: Unknown line format: ./files/Makefile.am:175: if INC_WINDOWS_FILES
Exit 1
> > - If it encounters a file called Makefile.PL it expects it to be a
> > makefile (rather than a perl script) and *aborts* when it finds the
> > syntax confusing.
>
> I don't see how this is a problem, since this is intended for checking
> pkgsrc Makefiles and only to be called from pkgsrc/category/program,
> where no Perl Makefile.PL should ever reside.
They can and do reside in files/, and when invoked with -Call as is
usually recommended, pkglint takes it upon itself to inspect the
contents of files/ and becomes confused. For example:
valkyrie% cd sysutils/p5-UPS-Nut/
valkyrie% pkglint -Call
WARN: ../../sysutils/ups-nut/Makefile.common:7: Please add a line "# used by sysutils/p5-UPS-Nut/Makefile" here.
FATAL: files/Makefile.PL:4: Unknown line format: ./files/Makefile.PL:4: WriteMakefile(
Exit 1
> > - It doesn't understand the .-include "file" syntax and *aborts* upon
> > encountering it in a makefile.
>
> I don't see how this is a problem, since this is intended for checking
> pkgsrc Makefiles and only to be called from pkgsrc/category/program,
> where I didn't find any occurrence of ".-include" in the current
> pkgsrc tree, nor do I expect any to appear.
You didn't look in files/. Makefiles with this construction can and do
reside in files/, and when invoked with -Call as is usually
recommended, pkglint takes it upon itself to inspect the contents of
files/ and becomes confused. For example:
valkyrie% cd devel/bmake/
valkyrie% pkglint -Call
FATAL: files/Makefile.in:101: Unknown line format: ./files/Makefile.in:101: .-include <bsd.prog.mk>
Exit 1
> > - Currently when run in ruby18-base it somehow imagines that it should
> > be looking at ruby19-base's patches and spews about six screenfuls of
> > completely nonsensical errors.
>
> I can't reproduce this on ruby18-base nor ruby19-base, so I suspect
> this has been fixed in the meantime.
Yes, this has apparently been fixed. It was polluting the weekly check
runs and therefore moderately visible.
> > - It complains incorrectly about not being in -e mode if it encounters
> > a makefile recipe with a shell for loop, because of the semicolon in
> > the for loop syntax.
>
> Can you please give me an example package where I can reproduce this?
Not offhand, because I have been changing them when I find them.
However, you don't need an existing example; just take your favorite
test package and add this:
post-install:
for x in foo bar baz; do \
${INSTALL_PROGRAM} "$$x" ${DESTDIR}${PREFIX}/share; \
done
then run pkglint -Wall. You will get:
WARN: Makefile:37--39: Please switch to "set -e" mode before using a semicolon to separate commands.
On reflection the warning may be coming from the ; before "done"
rather than the one before "do", but the shell won't accept && there
so it's still wrong.
And, btw, another one: pkglint bails out if it encounters a makefile
line of the form "foo : bar", with a space before the colon. Arguably
it's correct to object to this, but it shouldn't crash.
--
David A. Holland
dholland@netbsd.org
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 9 Jul 2012 22:41:11 +0200
Where is it recommended to run pkglint -Call?
I only know of the recommendations to run "pkglint -Wall".
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 9 Jul 2012 20:44:38 +0000
oops, I meant to reply to this part before sending:
On Mon, Jul 09, 2012 at 08:35:02PM +0000, David Holland wrote:
> > > - It does not understand USE_GAMESGROUP or its accompanying
> > > paraphernalia, and issues a bucket of spurious warnings instead of
> > > guiding correct usage. I looked into fixing this at one point and was
> > > stopped by the first two issues.
> >
> > I've just fixed these issues.
It is much better, but I still get e.g.
WARN: Makefile:34: The user-defined variable GAMEMODE is used but not added to BUILD_DEFS.
WARN: Makefile:37: Permission [s] requested for USE_GAMESGROUP, but only [u] is allowed.
(these in maelstrom-x11)
> > USE_GAMESGROUP wasn't even documented in
> > the guide, so I fixed that as welll.
I thought I did that ages ago, but maybe I only started and never
finished, or something? Anyway, thanks.
--
David A. Holland
dholland@netbsd.org
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 9 Jul 2012 23:58:53 +0200
On Mon, Jul 09, 2012 at 08:45:04PM +0000, David Holland wrote:
> WARN: Makefile:34: The user-defined variable GAMEMODE is used but not added to BUILD_DEFS.
> WARN: Makefile:37: Permission [s] requested for USE_GAMESGROUP, but only [u] is allowed.
>
> (these in maelstrom-x11)
Thanks, fixed.
Thomas
From: "Thomas Klausner" <wiz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint
Date: Tue, 10 Jul 2012 09:39:27 +0000
Module Name: pkgsrc
Committed By: wiz
Date: Tue Jul 10 09:39:27 UTC 2012
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: pkglint.pl
Log Message:
Do not parse Makefiles in files/ or patches/
Addresses another part of PR 46570 by David Holland.
Bump version.
To generate a diff of this commit:
cvs rdiff -u -r1.405 -r1.406 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.837 -r1.838 pkgsrc/pkgtools/pkglint/files/pkglint.pl
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Thomas Klausner" <wiz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint/files
Date: Tue, 10 Jul 2012 10:27:23 +0000
Module Name: pkgsrc
Committed By: wiz
Date: Tue Jul 10 10:27:23 UTC 2012
Modified Files:
pkgsrc/pkgtools/pkglint/files: pkglint.pl
Log Message:
Warn about space before colon in dependency line instead of FATALing out.
Addresses part of PR 46570 by David Holland.
To generate a diff of this commit:
cvs rdiff -u -r1.838 -r1.839 pkgsrc/pkgtools/pkglint/files/pkglint.pl
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Tue, 10 Jul 2012 12:28:13 +0200
On Mon, Jul 09, 2012 at 08:35:02PM +0000, David Holland wrote:
> That's a start. How about having it output words? Then it wouldn't be
> necessary to look them up or explain them.
Done (not sure if I already mailed about that).
> They can and do reside in files/, and when invoked with -Call as is
> usually recommended, pkglint takes it upon itself to inspect the
> contents of files/ and becomes confused. For example:
>
> % cd archivers/libarchive/
> % pkglint -Call
> FATAL: files/Makefile.am:175: Unknown line format: ./files/Makefile.am:175: if INC_WINDOWS_FILES
> Exit 1
>
> They can and do reside in files/, and when invoked with -Call as is
> usually recommended, pkglint takes it upon itself to inspect the
> contents of files/ and becomes confused. For example:
>
> valkyrie% cd sysutils/p5-UPS-Nut/
> valkyrie% pkglint -Call
> WARN: ../../sysutils/ups-nut/Makefile.common:7: Please add a line "# used by sysutils/p5-UPS-Nut/Makefile" here.
> FATAL: files/Makefile.PL:4: Unknown line format: ./files/Makefile.PL:4: WriteMakefile(
> Exit 1
>
> You didn't look in files/. Makefiles with this construction can and do
> reside in files/, and when invoked with -Call as is usually
> recommended, pkglint takes it upon itself to inspect the contents of
> files/ and becomes confused. For example:
>
> valkyrie% cd devel/bmake/
> valkyrie% pkglint -Call
> FATAL: files/Makefile.in:101: Unknown line format: ./files/Makefile.in:101: .-include <bsd.prog.mk>
> Exit 1
Makefile* and *mk in files/ and patches/ aren't checked as Makefiles any longer.
> Not offhand, because I have been changing them when I find them.
> However, you don't need an existing example; just take your favorite
> test package and add this:
>
> post-install:
> for x in foo bar baz; do \
> ${INSTALL_PROGRAM} "$$x" ${DESTDIR}${PREFIX}/share; \
> done
>
> then run pkglint -Wall. You will get:
>
> WARN: Makefile:37--39: Please switch to "set -e" mode before using a semicolon to separate commands.
>
> On reflection the warning may be coming from the ; before "done"
> rather than the one before "do", but the shell won't accept && there
> so it's still wrong.
Actually, I like the warning.
I prefer this behavior:
set -e; for i in 1 2 3; do echo $i; false; done
1
*** Error code 1
to:
for i in 1 2 3; do echo $i; false; done
1
2
3
*** Error code 1
> And, btw, another one: pkglint bails out if it encounters a makefile
> line of the form "foo : bar", with a space before the colon. Arguably
> it's correct to object to this, but it shouldn't crash.
Fixed.
I think I've addressed all your points. Anything else?
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sat, 21 Jul 2012 23:53:09 +0000
On Mon, Jul 09, 2012 at 06:20:04PM +0000, Thomas Klausner wrote:
>> 2) It did not throw an error on this line:
>>
>> ${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
>>
>> Non-closed braces need to throw an error.
>
> It can't be a complete Makefile parser, and even make(1) doesn't
> complain about
> FOO= ${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
>
> I see this as WONTFIX.
Well, it could be a complete makefile parser. But even if not, I think
this is actually a moderately important thing to check for, if only
because make *doesn't* always do it right and it's an easy typo to
make.
--
David A. Holland
dholland@netbsd.org
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Sun, 22 Jul 2012 01:59:54 +0000
On Tue, Jul 10, 2012 at 10:30:12AM +0000, Thomas Klausner wrote:
>> Not offhand, because I have been changing them when I find them.
>> However, you don't need an existing example; just take your favorite
>> test package and add this:
>>
>> post-install:
>> for x in foo bar baz; do \
>> ${INSTALL_PROGRAM} "$$x" ${DESTDIR}${PREFIX}/share; \
>> done
>>
>> then run pkglint -Wall. You will get:
>>
>> WARN: Makefile:37--39: Please switch to "set -e" mode before using a semicolon to separate commands.
>>
>> On reflection the warning may be coming from the ; before "done"
>> rather than the one before "do", but the shell won't accept && there
>> so it's still wrong.
>
> Actually, I like the warning.
>
> I prefer this behavior:
>
> set -e; for i in 1 2 3; do echo $i; false; done
> 1
> *** Error code 1
>
> to:
>
> for i in 1 2 3; do echo $i; false; done
> 1
> 2
> 3
> *** Error code 1
Sure. But you may just get
set -e; for i in 1 2 3; do echo $i; false; done
1
2
3
depending on what's actually written in place of the "false". This is
the same sh standards issue that causes some packages' build systems
to not stop on error.
The safest way to do this, AFAICT, is
for i in 1 2 3; do echo $i; false || exit 1; done
pkglint doesn't recognize this idiom.
> I think I've addressed all your points. Anything else?
I'm currently getting a lot of these Perl messages:
Use of uninitialized value in concatenation (.) or string at /usr/pkg/bin/pkglint line 4776.
also, I'm getting the following internal errors from pkglint:
ERROR: emulators/cygwin_lib/Makefile:30--33: Internal pkglint error: SCST_CONT: rest= $${f%.exe}); done
ERROR: fonts/jisx0212fonts/Makefile:29--31: Internal pkglint error: SCST_CONT: rest=$${f%.*}; done
ERROR: games/kanatest/Makefile:21: Internal pkglint error: SCST_CONT: rest= $${a%.po}.mo $$a; done
ERROR: inputmethod/uim/options.mk:162: Internal pkglint error: SWST_PLAIN: rest=\.png
ERROR: lang/gcc3-ada/buildlink3.mk:24: Internal pkglint error: SWST_PLAIN: rest=\buildlink:buildlink/gcc3:
ERROR: lang/gcc34-ada/buildlink3.mk:25: Internal pkglint error: SWST_PLAIN: rest=\buildlink:buildlink/gcc34-ada:
ERROR: lang/swi-prolog-packages/Makefile:61--62: Internal pkglint error: SCST_ECHO: rest= $$! >${WRKDIR}/.Xvfb.pid
ERROR: mail/cyrus-imapd/Makefile:166--173: Internal pkglint error: SWST_PLAIN: rest=\.conf
ERROR: mail/cyrus-imapd/Makefile:166--173: Internal pkglint error: SWST_PLAIN: rest=\.conf
ERROR: mail/cyrus-imapd23/Makefile:116--123: Internal pkglint error: SWST_PLAIN: rest=\.conf
ERROR: mail/cyrus-imapd23/Makefile:116--123: Internal pkglint error: SWST_PLAIN: rest=\.conf
ERROR: mail/cyrus-imapd24/Makefile:115--122: Internal pkglint error: SWST_PLAIN: rest=\.conf
ERROR: mail/cyrus-imapd24/Makefile:115--122: Internal pkglint error: SWST_PLAIN: rest=\.conf
ERROR: mail/dk-milter/Makefile:51--53: Internal pkglint error: SWST_PLAIN: rest=\/
ERROR: mail/sendmail/Makefile:90--91: Internal pkglint error: SWST_PLAIN: rest=\/
ERROR: mail/sendmail/Makefile:93--95: Internal pkglint error: SWST_PLAIN: rest=\/
ERROR: mail/thunderbird/Makefile:78--83: Internal pkglint error: SCST_CONT: rest= '/^ <em:id>/ {sub(".*
ERROR: mail/thunderbird10/Makefile:74--79: Internal pkglint error: SCST_CONT: rest= '/^ <em:id>/ {sub(".*
ERROR: pkgtools/pkg_install/Makefile:197--202: Internal pkglint error: SCST_CONT: rest=$${f%%.[157]}.cat; done
ERROR: sysutils/syslog-ng/Makefile:86--88: Internal pkglint error: SWST_PLAIN: rest=\
ERROR: www/horde/Makefile:121--123: Internal pkglint error: SCST_CONT: rest=$${f%.dist}; done
ERROR: www/seamonkey/Makefile:64--69: Internal pkglint error: SCST_CONT: rest= '/^ <em:id>/ {sub(".*
some other stuff from the pkglint -r run:
ERROR: x11/gtk+extra/buildlink3.mk:5: Package name mismatch between GTK_EXTRA ...
ERROR: x11/gtk+extra/buildlink3.mk:3: ... and gtk+extra.
(this should be easy)
ERROR: x11/py-wxWidgets/buildlink3.mk:5: Package name mismatch between PY_WXWIDGETS ...
ERROR: x11/py-wxWidgets/buildlink3.mk:3: ... and ${PYPKGPREFIX}-wxWidgets.
(I guess it'll need to know about PYPKGPREFIX and other multiversion prefixes)
ERROR: print/teTeX3-bin/patches/patch-ab:69: This code must not be included in patches.
(I have no idea what this warning is intended to mean or what's provoking it.)
ERROR: lang/ruby/buildlink3.mk:14: There is no package in lang/${RUBY_BASE}.
(is it too much to expect it to expand RUBY_BASE?)
WARN: emulators/palmosemulator/Makefile:19: "${LP64PLATFORMS}" is not a valid platform triple.
(it needs to know about this)
WARN: security/ipsec-tools/Makefile:39: The user-defined variable VARBASE is used but not added to BUILD_DEFS.
(surely VARBASE is in BUILD_DEFS by default?)
WARN: graphics/gimp-fix-ca/Makefile:7: "&" is not a valid URL.
WARN: graphics/gimp-fix-ca/Makefile:7: "id=9884" is not a valid URL.
WARN: graphics/gimp-fix-ca/Makefile:7: "&" is not a valid URL.
WARN: graphics/gimp-fix-ca/Makefile:7: "file=" is not a valid URL.
(not clear why it's dicing up the url into substrings, but it's doing
it wrong)
WARN: lang/perl5/packlist.mk:76--81: "BEGIN { destdir = "${DESTDIR}"; gsub(/\/\//, "/", destdir); len_destdir = length(destdir); } { if (index($$1, destdir) == 1) $$1 = substr($$1, len_destdir + 1) }" is not a valid pathname.
(why on earth does it expect this to be a pathname?)
WARN: mail/mail-notification/Makefile:17: "./jb configure" is not a valid pathname.
(it is, however, a valid thing to execute)
WARN: net/uucp/Makefile:52: "/bin /usr/bin ${PREFIX}/bin" is not a valid pathname.
(I guess it's latching onto these because the variable name contains "PATH"?)
WARN: www/opera/Makefile:47: "x86_64" is not valid for EMUL_ARCH. Use one of { i386 none } instead.
(guess it's out of date)
WARN: archivers/unrar/Makefile:49: As .MAKEFLAGS is modified using "+=", its name should indicate plural.
WARN: math/clisp-pari/Makefile:23: As ac_cv_libpari_libs is modified using "+=", its name should indicate plural.
(but it does!)
(this use of .MAKEFLAGS may be abusive, but that doesn't matter)
WARN: shells/zsh/Makefile.common:92: CHECK_BUILTIN.curses is defined but not used. Spelling mistake?
(pkglint should know about the CHECK_BUILTIN idiom)
WARN: x11/gtk3/Makefile:66: Unknown compiler flag "-std=gnu99".
WARN: audio/alsa-lib/Makefile:26: Unknown compiler flag "-std=c99".
(surely it should know about these?)
WARN: chat/pidgin-icb/Makefile:18: Compiler flag "`pkg-config pidgin --cflags`" does not start with a dash.
WARN: chat/pidgin-icb/Makefile:19--20: Linker flag "`pkg-config pidgin --libs`" does not start with a dash.
(and surely at this point it should be taught to recognize pkg-config)
WARN: chat/p5-Net-Goofey/Makefile:19: "test" is not valid for INTERACTIVE_STAGE. Use one of { build configure extract fetch install } instead.
(shouldn't this be legitimate?)
WARN: mail/bulk_mailer/Makefile:17: Compiler flag "%s\"" does not start with a dash.
(it's become confused by quoting)
WARN: databases/py-cassa/Makefile:5: EGG_NAME is defined but not used. Spelling mistake?
(why does it warn only here and not on every other Python module?)
WARN: graphics/clutter/buildlink3.mk:17: Only buildlink variables for clutter, not MesaLib may be set in this file.
(what it's doing is perfectly valid)
WARN: graphics/libkipi-kde3/buildlink3.mk:8: Unknown dependency format: libkipi>=0.1.5<4.0
(it appears to not understand all depends of this form)
That's a bit more than I intended. Lots more where they came from though :-/
and, btw, now at least I can do this without pkglint crashing, thanks.
--
David A. Holland
dholland@netbsd.org
State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 22 Jul 2012 02:04:19 +0000
State-Changed-Why:
New batch of issues submitted.
I've dropped the priority since it's no longer crashing out.
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Mon, 23 Jul 2012 06:17:19 +0000
(sent to gnats instead of gnats-bugs)
------
From: diro@nixsyspaus.org
To: gnats@NetBSD.org
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Thu, 5 Jul 2012 16:38:02 -0400
4) pkglint doesn't report an error on this line:
LICENSE= gnu-gpl-v2 gnu-lgpl-v2 modified-bsd mit
Obviously, pkg_admin/bmake choke on the lack of ANDs/ORs in that line.
5) pkglint needs to not complain about empty PLISTs for pear packages and
other packages in which the PLIST is handled automagically.
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 30 Jul 2012 01:37:05 +0000
On Sun, Jul 22, 2012 at 02:00:14AM +0000, David Holland wrote:
> The following reply was made to PR pkg/46570; it has been noted by GNATS.
> I'm currently getting a lot of these Perl messages:
>
> Use of uninitialized value in concatenation (.) or string at /usr/pkg/bin/pkglint line 4776.
This is caused by e.g. line 38 of x11/antiright/Makefile:
TOOLS_DEPENDS.pkg-config= pkg-config>=0.20:../../devel/pkg-config
this line also generates
WARN: Makefile:38: Permission [set] requested for TOOLS_DEPENDS.pkg-config, but only [] is allowed.
which seems wrong.
--
David A. Holland
dholland@netbsd.org
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sun, 12 Aug 2012 20:25:43 +0200
> 5) pkglint needs to not complain about empty PLISTs for pear packages and
> other packages in which the PLIST is handled automagically.
It doesn't, AFAIK.
# cd /usr/pkgsrc/www/pear-HTTP
# pkglint
ERROR: Makefile: All packages must define their LICENSE.
WARN: Makefile:6: "brook@nmsu.edu pkgsrc-users@NetBSD.org" is not a valid mail address.
1 errors and 1 warnings found. (Use -e for more details.)
# ls
CVS DESCR Makefile distinfo
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 13 Aug 2012 19:56:23 +0000
On Mon, Jul 09, 2012 at 08:45:02PM +0000, Thomas Klausner wrote:
> Where is it recommended to run pkglint -Call?
> I only know of the recommendations to run "pkglint -Wall".
I don't know actually, I remember being told to use that to get it to
print everything.
However, it seems that it does appear in the guide:
"Use e.g. pkglint -Call -Wall for a very thorough check."
--
David A. Holland
dholland@netbsd.org
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Sun, 26 Aug 2012 16:11:26 +0200
On Mon, Jul 30, 2012 at 01:40:06AM +0000, David Holland wrote:
> On Sun, Jul 22, 2012 at 02:00:14AM +0000, David Holland wrote:
> > The following reply was made to PR pkg/46570; it has been noted by GNATS.
> > I'm currently getting a lot of these Perl messages:
> >
> > Use of uninitialized value in concatenation (.) or string at /usr/pkg/bin/pkglint line 4776.
>
> This is caused by e.g. line 38 of x11/antiright/Makefile:
>
> TOOLS_DEPENDS.pkg-config= pkg-config>=0.20:../../devel/pkg-config
I've fixed the warning.
> this line also generates
>
> WARN: Makefile:38: Permission [set] requested for TOOLS_DEPENDS.pkg-config, but only [] is allowed.
>
> which seems wrong.
It says that TOOLS_DEPENDS.pkg-config should not be changed by a
package Makefile. Are you arguing that it should be allowed or that
the error message should have some text between the brackets?
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Sun, 26 Aug 2012 20:54:54 +0000
On Sun, Aug 26, 2012 at 02:15:04PM +0000, Thomas Klausner wrote:
>>> Use of uninitialized value in concatenation (.) or string at /usr/pkg/bin/pkglint line 4776.
>>
>> This is caused by e.g. line 38 of x11/antiright/Makefile:
>>
>> TOOLS_DEPENDS.pkg-config= pkg-config>=0.20:../../devel/pkg-config
>
> I've fixed the warning.
Thanks.
> > this line also generates
> >
> > WARN: Makefile:38: Permission [set] requested for TOOLS_DEPENDS.pkg-config, but only [] is allowed.
> >
> > which seems wrong.
>
> It says that TOOLS_DEPENDS.pkg-config should not be changed by a
> package Makefile. Are you arguing that it should be allowed or that
> the error message should have some text between the brackets?
Well, packages certainly need to be able to specify TOOLS_DEPENDS;
that's practically the whole point of TOOLS_DEPENDS.
Perhaps it should allow += but not =, though.
I've noticed (I think) that sometimes pkglint will report that "only
[] is allowed" but it actually accepts append, but I can't point to
any examples.
(It might also be better if it said "but no uses are allowed" instead
of "only [] is allowed".)
--
David A. Holland
dholland@netbsd.org
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Sun, 26 Aug 2012 23:11:23 +0200
On Sun, Aug 26, 2012 at 08:55:03PM +0000, David Holland wrote:
> > It says that TOOLS_DEPENDS.pkg-config should not be changed by a
> > package Makefile. Are you arguing that it should be allowed or that
> > the error message should have some text between the brackets?
>
> Well, packages certainly need to be able to specify TOOLS_DEPENDS;
> that's practically the whole point of TOOLS_DEPENDS.
>
> Perhaps it should allow += but not =, though.
I guess you get the behaviour you want by changing pkgtools/files/makevars.map:
TOOLS_DEPENDS.* InternalList of DependencyWithPath [$system]
to
TOOLS_DEPENDS.* InternalList of DependencyWithPath [$package_list]
> I've noticed (I think) that sometimes pkglint will report that "only
> [] is allowed" but it actually accepts append, but I can't point to
> any examples.
That shouldn't happen any longer with 4.123. If the allowed flags were
"au" which means append and use, then the warning was printing empty
brackets.
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 27 Aug 2012 04:37:50 +0000
On Sun, Aug 26, 2012 at 09:15:04PM +0000, Thomas Klausner wrote:
>> Well, packages certainly need to be able to specify TOOLS_DEPENDS;
>> that's practically the whole point of TOOLS_DEPENDS.
>>
>> Perhaps it should allow += but not =, though.
>
> I guess you get the behaviour you want by changing
> pkgtools/files/makevars.map:
> TOOLS_DEPENDS.* InternalList of DependencyWithPath [$system]
> to
> TOOLS_DEPENDS.* InternalList of DependencyWithPath [$package_list]
Yes, that silences it. Should I commit?
> > I've noticed (I think) that sometimes pkglint will report that "only
> > [] is allowed" but it actually accepts append, but I can't point to
> > any examples.
>
> That shouldn't happen any longer with 4.123. If the allowed flags were
> "au" which means append and use, then the warning was printing empty
> brackets.
Good :-)
--
David A. Holland
dholland@netbsd.org
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 27 Aug 2012 10:18:26 +0200
On Mon, Aug 27, 2012 at 04:40:04AM +0000, David Holland wrote:
> > I guess you get the behaviour you want by changing
> > pkgtools/files/makevars.map:
> > TOOLS_DEPENDS.* InternalList of DependencyWithPath [$system]
> > to
> > TOOLS_DEPENDS.* InternalList of DependencyWithPath [$package_list]
>
> Yes, that silences it. Should I commit?
Yes, please. Bump version if you want, or not.
Thomas
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Mon, 27 Aug 2012 14:07:04 +0000
On Mon, Aug 27, 2012 at 08:20:05AM +0000, Thomas Klausner wrote:
> On Mon, Aug 27, 2012 at 04:40:04AM +0000, David Holland wrote:
> > > I guess you get the behaviour you want by changing
> > > pkgtools/files/makevars.map:
> > > TOOLS_DEPENDS.* InternalList of DependencyWithPath [$system]
> > > to
> > > TOOLS_DEPENDS.* InternalList of DependencyWithPath [$package_list]
> >
> > Yes, that silences it. Should I commit?
>
> Yes, please. Bump version if you want, or not.
I take it back: that silences it for += but also for =, which probably
isn't desirable.
--
David A. Holland
dholland@netbsd.org
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sun, 9 Sep 2012 20:56:13 +0000
(two mails here, that were accidentally sent to gnats instead of gnats-bugs)
------
From: diro@nixsyspaus.org
To: gnats@NetBSD.org
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Wed, 5 Sep 2012 17:34:51 -0400
Thanks, wiz, for the fixes. Just some more comments and infelicities:
>> 2) It did not throw an error on this line:
>>
>> ${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
> >
> > Non-closed braces need to throw an error.
> It can't be a complete Makefile parser, and even make(1) doesn't complain about
> FOO= ${EGDIR/apparmor.d ${EGDIR/dbus-1/system.d ${EGDIR/pam.d
>
> I see this as WONTFIX.
That seems reasonable. However, bmake _is_ a complete Makefile parser. Doesn't
our make at least need to complain about this (I know it's not realistic to
expect all the *make programs to fix this)?
> I've just fixed these issues. USE_GAMESGROUP wasn't even documented in
> the guide, so I fixed that as well.
I would like to note that there are also many other things which are part of
the pkgsrc framework that are not documented in the guide. It makes it more
difficult for people like me, who jump in every few months, to track the changes
in pkgsrc and to not resort to bothering #pkgsrc for answers if greping
Makefiles in wip/ fails.
Not meaning to turn this into a general complaint ticket about pkgsrc;
however, since we've noticed that some things are not documented, could
anything brought up in this thread, which is not documented already, be documented after discussion?
>> 5) pkglint needs to not complain about empty PLISTs for pear packages and
>> other packages in which the PLIST is handled automagically.
>It doesn't, AFAIK.
> # cd /usr/pkgsrc/www/pear-HTTP
> # pkglint
> ERROR: Makefile: All packages must define their LICENSE.
> WARN: Makefile:6: "brook@nmsu.edu pkgsrc-users@NetBSD.org" is not a valid
> mail address.
> 1 errors and 1 warnings found. (Use -e for more details.)
> # ls
> CVS DESCR Makefile distinfo
Ahh, but it does:
# cd pkgsrc/wip/pear-HTTP_Client/
# pkglint
ERROR: ../../lang/php/pear.mk:69: Cannot read ./../../lang/php5/buildlink3.mk.
WARN: PLIST:1: PLIST files shouldn't be empty.
1 errors and 1 warnings found.
Is is now preferred to not have any PLIST file for pear packages instead of
an empty one?
and:
6) Like the previous behaviour with FETCH_USING (now fixed), it doesn't
recognize RUN_LDCONFIG:
WARN: Makefile:25: RUN_LDCONFIG is defined but not used. Spelling mistake?
0 errors and 1 warnings found.
From: diro@nixsyspaus.org
To: gnats@NetBSD.org
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Fri, 7 Sep 2012 14:31:37 -0400
7) pkglint doesn't understand KDE_LANGUAGE, even though it's defined in the
various buildlink3.mk files that are connected to the package
(x11/kde4-l10n-de, for example).
# pkglint
WARN: Makefile:3: KDE_LANGUAGE is defined but not used. Spelling mistake?
0 errors and 1 warnings found.
This may be getting too package specific, though, and not pkgsrc
framework-specific.
From: diro@nixsyspaus.org
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570
Date: Thu, 27 Sep 2012 07:27:10 -0400
8) pkglint needs to complain when packages have COMMENT set to "TODO: Short
description of the package", a url2pkg default;
Also, i recind my comment that pkglint not being a full Makefile parser is
reasonable. It doesn't detect this:
9) ${INSTALL_DATA} ${WRKSRC}/file.ext ${DESTDIR}${PREFIX}/wherever
${INSTALL_DATA) ${WRKSRC}/file2.ext ${DESTDIR}${PREFIX}/wherever2
(See second INSTALL_DATA).
From: diro@nixsyspaus.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Sat, 10 Nov 2012 18:04:13 -0500
10) BUILD_DEPENDS+= gtk2+>=2.16:../../x11/gtk2
WARN: Makefile:36: Unknown dependency pattern "gtk2+>=2.16".
From: diro@nixsyspaus.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Tue, 13 Nov 2012 09:52:29 -0500
11) pkglint needs to not completely implode on encountering empty options.mk
(or other files):
FATAL: Assertion failed: checklines_mk may only be called with non-empty
lines..
line 8498 called main::main
line 8492 called main::checkitem
line 8461 called main::checkdir_package
line 8368 called main::checkfile
line 7982 called main::checkfile_mk
line 7089 called main::checklines_mk
line 6048 called PkgLint::Util::assert
More helpful would be a warning to the user that X file is empty and needs to
not be.
From: diro@nixsyspaus.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Thu, 15 Nov 2012 19:10:58 -0500
12) pkglint needs to complain if package options appear in both a
PKG_OPTIONS_GROUP and in PKG_SUPPORTED_OPTIONS. This is harmless, but it
causes the package options which appear in both to be shown twice after the
depends stage.
Responsible-Changed-From-To: wiz->pkg-manager
Responsible-Changed-By: wiz@NetBSD.org
Responsible-Changed-When: Sat, 29 Dec 2012 12:28:00 +0000
Responsible-Changed-Why:
Realistically, I'm not going to fix all of these, so open this up
for others as well.
From: diro@nixsyspaus.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Wed, 2 Jan 2013 23:02:15 -0500
13) Not sure if this would cause any problems, but if it doesn't... it would
be nice to have pkglint error out when PKG_OPTIONS.$PKG != $PKGBASE in
options.mk.
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Sat, 19 Jan 2013 20:29:10 +0000
14) pkglint demands that meta-packages have a LICENSE, even though
this is silly.
--
David A. Holland
dholland@netbsd.org
From: Alan Barrett <apb@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570
Date: Sat, 19 Jan 2013 22:41:34 +0200
15) If a package Makefile sets OWNER= then pkglint could complain
about edits made by somebody other than the OWNER. Edits could be
detected by running "cvs diff" or some such, and the current user
name could be found using the mechanism used by "make
changes-entry".
Similarly for MAINTAINER instead of OWNER, but the warning could be
less severe.
From: "Amitai Schlair" <schmonz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint
Date: Sat, 19 Jan 2013 22:51:12 +0000
Module Name: pkgsrc
Committed By: schmonz
Date: Sat Jan 19 22:51:12 UTC 2013
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: pkglint.pl
Added Files:
pkgsrc/pkgtools/pkglint/files: pkglint.t
Log Message:
Make it possible to easily write automated tests:
* minimally adapt pkglint(1) into a "modulino" for testability
* verify it still runs normally as a program
* create a test script with a few very simple test cases
* hook it up to 'make test'
* verify that the tests really fail if I go breaking the code under test
Meta-addresses PR pkg/46570. New BUILD_DEPENDS, but no functional
change, so no PKGREVISION bump. Approved by wiz@.
To generate a diff of this commit:
cvs rdiff -u -r1.423 -r1.424 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.847 -r1.848 pkgsrc/pkgtools/pkglint/files/pkglint.pl
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/pkglint.t
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Sun, 20 Jan 2013 19:39:17 +0000
#16:
------
From: OBATA Akio <obache@netbsd.org>
To: pkg-manager@netbsd.org, gnats-admin@netbsd.org, pkgsrc-bugs@netbsd.org,
yamajun@ofug.net
Cc:
Subject: Re: pkg/47468: Update request: inputmethod/uim, inputmethod/uim-elisp
update to uim-1.8.4
Date: Sat, 19 Jan 2013 08:15:05 +0000 (UTC)
The following reply was made to PR pkg/47468; it has been noted by GNATS.
From: "OBATA Akio" <obache@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/47468: Update request: inputmethod/uim, inputmethod/uim-elisp
update to uim-1.8.4
Date: Sat, 19 Jan 2013 17:13:04 +0900
On Sat, 19 Jan 2013 14:45:00 +0900, <yamajun@ofug.net> wrote:
> pkgsrc changelog:
> * Change value of CHECK_FILES_SKIP for pkglint(1)
It's a false detection, should not be changed.
As noted in mk/check/check-files.mk, it's a list of regular expression,
not shell patterns.
--
OBATA Akio / obache@NetBSD.org
From: Thomas Klausner <wiz@NetBSD.org>
To: NetBSD bugtracking <gnats-bugs@NetBSD.org>,
OBATA Akio <obache@NetBSD.org>
Cc:
Subject: Re: pkg/46570: infelicities in pkglint
Date: Sun, 20 Jan 2013 21:39:36 +0100
On Sun, Jan 20, 2013 at 07:40:05PM +0000, David Holland wrote:
> #16:
>
> From: "OBATA Akio" <obache@netbsd.org>
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: pkg/47468: Update request: inputmethod/uim, inputmethod/uim-elisp
> update to uim-1.8.4
> Date: Sat, 19 Jan 2013 17:13:04 +0900
>
> On Sat, 19 Jan 2013 14:45:00 +0900, <yamajun@ofug.net> wrote:
>
> > pkgsrc changelog:
> > * Change value of CHECK_FILES_SKIP for pkglint(1)
>
> It's a false detection, should not be changed.
> As noted in mk/check/check-files.mk, it's a list of regular expression,
> not shell patterns.
These are the easiest to fix -- just modify the type in pkgtools/pkglint/files/makevars.map.
I don't think there's a special regex-type, so "List of Unchecked [m:a,c:a]" is probably best.
Thomas
From: rodent@NetBSD.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Wed, 27 Mar 2013 07:05:09 -0400
17)
> WARN: audio/festvox-tll/Makefile:12: License no-commercial-use is deprecated.
> WARN: audio/festvox-us1/Makefile:12: License file licenses/no-commercial-use does not exist.
Well, that's not very helpful. What is the new license then? Deprecations need
always specify the new convention.
18)
> WARN: inputmethod/ibus-hangul/PLIST:6: Packages that install a .desktop entry should .include "../../sysutils/desktop-file-utils/desktopdb.mk".
As obache@ pointed out in private mail, this needs to happen only if the
MimeType key exists in the .desktop entry. pkglint needs to tell users to
ignore this warning if this the file doesn't contain such a key.
19) References #10:
> WARN: x11/fltk/buildlink3.mk:8: Unknown dependency format: fltk>=1.1.5rc1<1.3
From: Thomas Klausner <wiz@NetBSD.org>
To: NetBSD bugtracking <gnats-bugs@NetBSD.org>
Cc:
Subject: Re: pkg/46570
Date: Wed, 27 Mar 2013 13:37:50 +0100
On Wed, Mar 27, 2013 at 11:10:07AM +0000, rodent@NetBSD.org wrote:
> 17)
> > WARN: audio/festvox-tll/Makefile:12: License no-commercial-use is deprecated.
> > WARN: audio/festvox-us1/Makefile:12: License file licenses/no-commercial-use does not exist.
>
> Well, that's not very helpful. What is the new license then? Deprecations need
> always specify the new convention.
There never was a license called "no-commercial-use". It was just a
category used before we had proper license handling. There is no
default that could be suggested here, it really depends on the
package. IMO nothing that pkglint can do better in this case.
Thomas
From: rodent@NetBSD.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Fri, 17 May 2013 21:47:27 -0400
20) % cd pkgsrc/textproc/saxon; pkglint
WARN: Makefile:3: The package is being downgraded from 9.4.0.7j to 1J.
0 errors and 1 warnings found. (Use -e for more details.)
% bmake show-var VARNAME=PKGNAME
saxon-9.5.0.1j
% bmake show-var VARNAME=PKGVERSION
9.5.0.1j
From: rodent@NetBSD.org
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570
Date: Sat, 12 Apr 2014 08:46:19 -0400
No, we haven't forgotten about this one. ;) Another complaint:
21) pkglint doesn't grok regex in PKGNAME:
rodent@sammy:~/dev/www/sqtop% pkglint
WARN: Makefile:4: "${DISTNAME:C/([0-9]{4})-([0-9]{2})-([0-9]{2})/\1\2\3/1}" is
not a valid package name. A valid package name has the form
packagename-version, where version consists only of digits, letters and dots.
0 errors and 1 warnings found. (Use -e for more details.)
rodent@sammy:~/dev/www/sqtop% bmake show-var VARNAME=PKGNAME
sqtop-20131217
From: "Roland Illig" <rillig@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/46570 CVS commit: pkgsrc/pkgtools/pkglint
Date: Tue, 12 Jan 2016 01:02:49 +0000
Module Name: pkgsrc
Committed By: rillig
Date: Tue Jan 12 01:02:49 UTC 2016
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile
pkgsrc/pkgtools/pkglint/files: buildlink3.go buildlink3_test.go
category.go check_test.go dir.go distinfo.go distinfo_test.go
expecter.go files.go files_test.go getopt.go globaldata.go
globaldata_test.go globalvars.go licenses.go licenses_test.go
line.go line_test.go logging.go main.go main_test.go mkline.go
mkline_test.go package.go package_test.go patches.go
patches_test.go pkglint.0 pkglint.1 pkglint.go pkglint_test.go
plist.go plist_test.go shell.go shell_test.go substcontext.go
substcontext_test.go toplevel.go toplevel_test.go tree.go util.go
util_test.go vardefs.go vartype.go vartype_test.go vartypecheck.go
vartypecheck_test.go vercmp.go vercmp_test.go
Added Files:
pkgsrc/pkgtools/pkglint/files: category_test.go dir_test.go mklines.go
mklines_test.go parser.go parser_test.go
Removed Files:
pkgsrc/pkgtools/pkglint/files: deprecated.go deprecated_test.go
descr.go descr_test.go makefiles.go makefiles_test.go mkcond.go
mkcond_test.go mkcontext.go pkgcontext.go vars.go vars_test.go
varusecontext.go varusecontext_test.go
Log Message:
Updated pkglint to 5.3
Changes since 5.2.2.2:
* Makefile variables
The warnings about missing permissions sound more natural than before
and give a hint for alternative operators (e.g. set-default instead
of append), or an alternative file where setting this variable is
allowed instead (e.g. PKGREVISION may not be set in Makefile.common,
but in Makefile it is ok).
Warnings about "unknown" allowed permissions are not shown anymore,
since they didn't provide any benefit. To see them again, pkglint must
be run with the -Dunchecked option.
User-defined variables may be used by builtin.mk. They may also be
used during load time, not only during run time, under the assumption
that in most cases the bsd.prefs.mk has already been loaded.
Some individual variables may be defined or used in places where this
was not allowed before. CHECK_BUILTIN.*, BUILDLINK_TARGETS,
TOOLS_DEPENDS.*, BUILDLINK_DEPMETHOD.*, SUBST_CLASSES.
A new parser for Makefile expressions detects and reports more
mistakes than bmake itself. Currently it is only used to check the
basic syntax; more applications are possible.
* PLIST
In PLIST files, conditionals of the form ${PLIST.*} are recognized and
are not part of the pathname. This allows pkglint to better check for
missing manual pages and correctly sorted PLIST files.
In --autofix mode, pkglint can sort PLIST files, which makes these
rather annoying warnings easy to fix.
No more warnings for man pages whose filename doesn't match exactly
the section, e.g. man/man3/exit.3c.
* Patches
The code for checking patch files has been completely rewritten, so
that it is easier understandable and well-structured. As an additional
benefit, it also became faster. Support for context diffs has been
dropped to a minimum, since they are not popular anymore.
Pkglint no longer warns about missing trailing whitespace in a line,
since all patch programs can handle these lines. It also doesn't
request empty lines between multiple diffs in a single file, since
that is simply not necessary.
Pkglint is picky when a patch file continues after the diff with some
text that still looks like a diff, since that means the patch doesn't
do what it looks like on first sight
(example: audio/faad2/patches/patch-au).
* Distinfo
When a patch file listed in distinfo cannot be found in the
filesystem, this is reported clearly instead of complaining about
missing SHA512 hashes (example: audio/libopus).
The inter-package distinfo check that verifies whether a distfile has
different hashes has been enabled. It had been disabled before, but
unintentionally so.
* Misc
- The check for COMMENT has been updated to reflect the changed
default value from url2pkg.
- BUILDLINK_API_DEPENDS.* may be set in buildlink3.mk, even if the
package is not the current one. (The other variables may be only set
for the current package.)
- In shell commands, the escape sequence \. (and similar ones, which
are often seen in sed(1) commands) no longer produces a warning,
since the different shells handle these escape sequences
consistently. (It is the echo(1) implementations that actually
differ, therefore this warning was superfluous.)
- Compiler flags in backticks (typically `pkg-config --cflags`) are
properly recognized.
- Internal pkglint errors when parsing shell commands have been fixed.
- No more warnings about PKGCONFIG_FILE.* being defined but unused.
- Dependencies of the form pkgbase>=1.0<5.0 are recognized.
- Diagnostics use quotes more often to indicate the placeholders.
- The type of GENERATE_PLIST has been changed from List of ShellWord
to ShellCommands, since that is what the variable is really about.
- The type ShellCommand used to mean "a shell command line in a
Makefile", which was confusing. Now it means what the name says,
which reduces the wrong warnings for variables like CC (example:
x11/kdebase3/options.mk).
- Improved buildlink3.mk checks to generate more helpful diagnostics.
- Fixed the parsing of dependency patterns, so that all but the most
exotic ones are properly recognized.
- Fixed the parsing of shell variables of the form ${var%.c}.
- Updated the check for the default COMMENT from url2pkg.
- Many more small improvements.
- Performance has improved again, though only a little bit.
- Unit test coverage has increased from 64.2 % to 78.9 %.
This fixes most of the points mentioned in PR pkg/46570.
To generate a diff of this commit:
cvs rdiff -u -r1.474 -r1.475 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.1 -r1.2 pkgsrc/pkgtools/pkglint/files/buildlink3.go \
pkgsrc/pkgtools/pkglint/files/globalvars.go \
pkgsrc/pkgtools/pkglint/files/licenses_test.go \
pkgsrc/pkgtools/pkglint/files/main_test.go \
pkgsrc/pkgtools/pkglint/files/toplevel_test.go \
pkgsrc/pkgtools/pkglint/files/tree.go \
pkgsrc/pkgtools/pkglint/files/vartype_test.go \
pkgsrc/pkgtools/pkglint/files/vercmp.go \
pkgsrc/pkgtools/pkglint/files/vercmp_test.go
cvs rdiff -u -r1.2 -r1.3 pkgsrc/pkgtools/pkglint/files/buildlink3_test.go \
pkgsrc/pkgtools/pkglint/files/dir.go \
pkgsrc/pkgtools/pkglint/files/expecter.go \
pkgsrc/pkgtools/pkglint/files/files_test.go \
pkgsrc/pkgtools/pkglint/files/licenses.go \
pkgsrc/pkgtools/pkglint/files/main.go \
pkgsrc/pkgtools/pkglint/files/package.go \
pkgsrc/pkgtools/pkglint/files/package_test.go \
pkgsrc/pkgtools/pkglint/files/patches_test.go \
pkgsrc/pkgtools/pkglint/files/toplevel.go \
pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r1.3 -r1.4 pkgsrc/pkgtools/pkglint/files/category.go \
pkgsrc/pkgtools/pkglint/files/getopt.go \
pkgsrc/pkgtools/pkglint/files/globaldata_test.go \
pkgsrc/pkgtools/pkglint/files/line_test.go \
pkgsrc/pkgtools/pkglint/files/logging.go \
pkgsrc/pkgtools/pkglint/files/pkglint_test.go \
pkgsrc/pkgtools/pkglint/files/plist.go \
pkgsrc/pkgtools/pkglint/files/plist_test.go \
pkgsrc/pkgtools/pkglint/files/substcontext_test.go \
pkgsrc/pkgtools/pkglint/files/vardefs.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/category_test.go \
pkgsrc/pkgtools/pkglint/files/dir_test.go \
pkgsrc/pkgtools/pkglint/files/mklines.go \
pkgsrc/pkgtools/pkglint/files/mklines_test.go \
pkgsrc/pkgtools/pkglint/files/parser.go \
pkgsrc/pkgtools/pkglint/files/parser_test.go
cvs rdiff -u -r1.5 -r1.6 pkgsrc/pkgtools/pkglint/files/check_test.go \
pkgsrc/pkgtools/pkglint/files/distinfo.go \
pkgsrc/pkgtools/pkglint/files/globaldata.go \
pkgsrc/pkgtools/pkglint/files/mkline_test.go \
pkgsrc/pkgtools/pkglint/files/pkglint.go
cvs rdiff -u -r1.2 -r0 pkgsrc/pkgtools/pkglint/files/deprecated.go \
pkgsrc/pkgtools/pkglint/files/deprecated_test.go \
pkgsrc/pkgtools/pkglint/files/mkcontext.go \
pkgsrc/pkgtools/pkglint/files/pkgcontext.go
cvs rdiff -u -r1.1 -r0 pkgsrc/pkgtools/pkglint/files/descr.go \
pkgsrc/pkgtools/pkglint/files/descr_test.go \
pkgsrc/pkgtools/pkglint/files/makefiles_test.go
cvs rdiff -u -r1.4 -r1.5 pkgsrc/pkgtools/pkglint/files/distinfo_test.go \
pkgsrc/pkgtools/pkglint/files/files.go \
pkgsrc/pkgtools/pkglint/files/substcontext.go \
pkgsrc/pkgtools/pkglint/files/util.go \
pkgsrc/pkgtools/pkglint/files/vartype.go \
pkgsrc/pkgtools/pkglint/files/vartypecheck_test.go
cvs rdiff -u -r1.7 -r1.8 pkgsrc/pkgtools/pkglint/files/line.go \
pkgsrc/pkgtools/pkglint/files/patches.go \
pkgsrc/pkgtools/pkglint/files/shell.go
cvs rdiff -u -r1.8 -r0 pkgsrc/pkgtools/pkglint/files/makefiles.go
cvs rdiff -u -r1.3 -r0 pkgsrc/pkgtools/pkglint/files/mkcond.go \
pkgsrc/pkgtools/pkglint/files/vars.go \
pkgsrc/pkgtools/pkglint/files/vars_test.go \
pkgsrc/pkgtools/pkglint/files/varusecontext.go \
pkgsrc/pkgtools/pkglint/files/varusecontext_test.go
cvs rdiff -u -r1.4 -r0 pkgsrc/pkgtools/pkglint/files/mkcond_test.go
cvs rdiff -u -r1.6 -r1.7 pkgsrc/pkgtools/pkglint/files/mkline.go \
pkgsrc/pkgtools/pkglint/files/shell_test.go
cvs rdiff -u -r1.35 -r1.36 pkgsrc/pkgtools/pkglint/files/pkglint.0
cvs rdiff -u -r1.49 -r1.50 pkgsrc/pkgtools/pkglint/files/pkglint.1
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/vartypecheck.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: rillig@NetBSD.org
State-Changed-When: Sat, 16 Jan 2016 00:45:41 +0000
State-Changed-Why:
Please check whether your issues are fixed.
I think I have fixed most of them.
Responsible-Changed-From-To: pkg-manager->dholland
Responsible-Changed-By: rillig@NetBSD.org
Responsible-Changed-When: Sat, 16 Jan 2016 00:47:02 +0000
Responsible-Changed-Why:
State-Changed-From-To: feedback->closed
State-Changed-By: rillig@NetBSD.org
State-Changed-When: Sat, 04 Jun 2016 20:24:28 +0000
State-Changed-Why:
Feedback timeout
State-Changed-From-To: closed->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 06 Jun 2016 05:44:41 +0000
State-Changed-Why:
bollocks
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sun, 12 Jun 2016 00:36:48 +0000
On Sat, Jan 16, 2016 at 12:45:41AM +0000, rillig@NetBSD.org wrote:
> Please check whether your issues are fixed.
> I think I have fixed most of them.
43 of 54 issues described in this PR are fixed. The ones remaining
follow. (I've repeated them in detail so there shouldn't be any
further need to look at the parts of the PR before this mail.)
I also discovered a few more issues while testing these, which I've
appended. (I haven't gone *looking* for more; if I do that I'll open a
new PR.)
1. It doesn't know about the "|| exit 1" idiom for making shell loops
safe. For example, it shouldn't complain about this:
post-install:
for x in 1 2 3; do echo "$$x" || exit 1; done
This is ultimately preferred to set -e because there are, at least
according to the wrong language standardized in POSIX, shell
constructs that don't trigger -e when failing. (It is a big ugly
mess.) I wouldn't say it should recommend this over set -e, but it
shouldn't object to it, and it certainly shouldn't be complaining that
the semicolon after the "exit 1" isn't checked for error.
To test this, add the above construct to your favorite test package.
2. It objects if the include guard in a bl3 file doesn't match the
package name when the package name contains characters not legal in
make identifiers. It ought to fold the package name into a legal make
identifier before comparing, transforming - and + and . and probably a
few other things to _.
For example: x11/gtk+extra, where it wants the include guard symbol to
be GTK+EXTRA.
3. In lang/perl5 for some reason it thinks that the awk script at line
76 of packlist.mk ought to be a valid pathname.
4. Shell quoting in mail/bulk_mailer causes it to issue bogus
complaints about compiler arguments. To see this you now need to
retrieve Makefile -r1.11 from CVS, but if you do that it still
happens.
5. It accepts multiple LICENSEs without boolean algebra, but pkgsrc
doesn't. For example:
LICENSE= gnu-gpl-v2 gnu-lgpl-v2 modified-bsd mit
6. It should complain if PKG_OPTIONS_VAR is wrong. It should be
PKG_OPTIONS.pkgbase; currently you can both set it to e.g.
PKG_OPTIONZ.pkgbase or to PKG_OPTIONS.someotherpkg and pkglint doesn't
say anything. There are some potential problems with the latter, but
it won't generate any more false positives than we routinely get from
pkglint from other things and it's probably worth trying to enforce.
7. It still demands a LICENSE for meta-packages, which is silly.
8. apb@ suggested that pkglint should warn if a package with an OWNER
has been modified and the OWNER isn't the current user. And perhaps
also for MAINTAINER (if the MAINTAINER is not pkgsrc-users) but at a
lower level. This seems like a good idea and hasn't been done. Note
that while using cvs diff to check for modifications is unacceptably
expenssive, reading the CVS control files (which pkglint already does
to check for un-CVS'd patches, for example) should let it check by
timestamp.
9. If LICENSE is no-commercial-use it should print something about
what to do instead (that is, extract the package's own specific
license) as well as saying that the setting is deprecated.
10. It should stop demanding desktop.db unless the .desktop file
involved declares a MimeType. This is going to be a pain, but it's
fairly important. It's possible that this check should be migrated out
of pkglint and into the pkgsrc infrastructure as a check after
stage-install, because then the desktop files are at least available
for analysis.
11. In textproc/saxon it announces
WARN: Makefile:3: The package is being downgraded from 9.5.0.1j (see
../../doc/CHANGES-2013:2602) to 1J
which is quite bogus. (This definitely happens with Makefile -r1.24
and probably earlier, but who knows about later.)
12. (new) It doesn't know about the new per-OS settings, e.g:
CONFIGURE_ENV.SunOS is defined by not used. (archivers/libarchive)
CONFIGURE_ARGS.Darwin is defined but not used. (graphics/clutteR)
MAKE_ENV.Interix is defined but not used. (devel/bmake)
13. It doesn't know about the BUILTIN_FIND_HEADERS mechnism:
BUILTIN_FIND_HEADERS_VAR is defined but not used. (archivers/libarchive)
14. It objects to using CHECK_BUILTIN from makefiles, although there's
a couple dozen packages that do this -- maybe they're all abusive, but
this seems like it ought to be discussed.
15. net/uucp/Makefile has a make loop that's used to substitute
variable names, and this confuses it.
--
David A. Holland
dholland@netbsd.org
State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 12 Jun 2016 00:41:08 +0000
State-Changed-Why:
still several things to fix
Responsible-Changed-From-To: dholland->pkg-manager
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Sun, 12 Jun 2016 00:41:53 +0000
Responsible-Changed-Why:
don't change responsible just for feedback
Responsible-Changed-From-To: pkg-manager->rillig
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Sun, 12 Jun 2016 00:52:53 +0000
Responsible-Changed-Why:
to OWNER of pkglint
From: David Holland <dholland-pbugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sun, 12 Jun 2016 00:53:47 +0000
On Sun, Jun 12, 2016 at 12:40:01AM +0000, David Holland wrote:
> 2. It objects if the include guard in a bl3 file doesn't match the
> package name when the package name contains characters not legal in
> make identifiers. It ought to fold the package name into a legal make
> identifier before comparing, transforming - and + and . and probably a
> few other things to _.
>
> For example: x11/gtk+extra, where it wants the include guard symbol to
> be GTK+EXTRA.
patch for this:
Index: files/buildlink3.go
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkglint/files/buildlink3.go,v
retrieving revision 1.5
diff -u -p -r1.5 buildlink3.go
--- files/buildlink3.go 5 Jun 2016 11:24:32 -0000 1.5
+++ files/buildlink3.go 12 Jun 2016 00:51:59 -0000
@@ -83,7 +83,11 @@ func ChecklinesBuildlink3Mk(mklines *MkL
exp.ExpectEmptyLine()
// See pkgtools/createbuildlink/files/createbuildlink, keyword PKGUPPER
- ucPkgbase := strings.ToUpper(strings.Replace(pkgbase, "-", "_", -1))
+
+ tmp := strings.Replace(pkgbase, "-", "_", -1)
+ tmp = strings.Replace(tmp, "+", "_", -1)
+ tmp = strings.Replace(tmp, ".", "_", -1)
+ ucPkgbase := strings.ToUpper(tmp)
if ucPkgbase != pkgupper && !containsVarRef(pkgbase) {
pkgupperLine.Errorf("Package name mismatch between multiple-inclusion guard %q (expected %q) and package name %q (from %s).",
pkgupper, ucPkgbase, pkgbase, pkgbaseLine.ReferenceFrom(pkgupperLine))
--
David A. Holland
dholland@netbsd.org
From: Roland Illig <roland.illig@gmx.de>
To: gnats-bugs@NetBSD.org, dholland@NetBSD.org
Cc:
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sun, 12 Jun 2016 08:22:51 +0200
Am 12.06.2016 um 02:55 schrieb David Holland:
> // See pkgtools/createbuildlink/files/createbuildlink, keyword PKGUPPER
> - ucPkgbase := strings.ToUpper(strings.Replace(pkgbase, "-", "_", -1))
> +
> + tmp := strings.Replace(pkgbase, "-", "_", -1)
> + tmp = strings.Replace(tmp, "+", "_", -1)
> + tmp = strings.Replace(tmp, ".", "_", -1)
> + ucPkgbase := strings.ToUpper(tmp)
Currently pkglint does the same as createbuildlink (see the comment
above the code), and I have no intention of changing that.
And, by the way, + and . are perfectly legal in identifiers:
---snip---
C++FLAGS+= -Wall -Wextra
all:
echo ${CXX} ${C++FLAGS}
---snip---
Roland
State-Changed-From-To: open->closed
State-Changed-By: rillig@NetBSD.org
State-Changed-When: Sun, 10 Jul 2016 15:07:42 +0000
State-Changed-Why:
1. The "set -e" check has been disabled completely.
2. Pkglint is correct and works the same as url2pkg,
so no change necessary.
3. Fixed.
4. Pkglint is correct, the Makefile was wrong.
5. Adding license checks to pkglint should be in a separate PR.
Won't fix.
6. Done.
7. Done.
8. Done.
9. Done, use "pkglint -e" to show the explanation.
10. This check has been removed, since pkglint would need to
extract the distfiles to understand the situation. Won't fix.
11. Done. Pkglint can now parse many more ${DISTFILE:modifiers}
expressions than before.
12. Done.
13. Done.
14. Done.
15. Done. Variables of the form NAME_${OTHERVAR} are no longer
flagged as being defined but not used.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home