Hi Danek,

Thanks for taking a look at this.  I've made a few changes to the
Makefile, and updated the whitelist to match the packages changed by
def0057774b7.

On Tue, 2010-09-14 at 14:12 -0700, Danek Duvall wrote:
> I figured, which was why I suggested that you could enable it with an
> environment variable, a la
> 
>     make packages LINT_REF_REPO=file:///.....

> You could find ways to make it faster later on.  I agree that running lint
> against the reference repo is too much for each hudson run, but perhaps
> once a day it'd be worthwhile, even if it took half an hour.

That makes sense alright, I've added that to this webrev: I have this so
that reference repository lint runs won't redirect stdout/stderr and
pkglint errors (not warnings) will cause the build to fail.  I
anticipate that people will use this rarely, and will want to see what's
going on if they're using it.

For consistency, I used PKGLINT_REF_REPO as the variable.

> Having whitelist usage be the default sounds fine to me, at least for now.

Ok, thanks. I'm sure there's ways I can improve performance in the
future.

> pkg/Makefile:
> 
>   - line 135: you don't need the semicolon after the "then".  Though
>     actually, you can just rm -rf the path, and it won't complain if it
>     doesn't exist (though perhaps you care if it exists,  but isn't a
>     directory?).

Nope, I'm not too worried about that, the pkglint unit tests should be
catching that sort of thing, so I've removed the if-statement.

>   - line 143: is there any way you can carry over the exit code of diff to
>     be the exit code for the entire clause?  That way if there are diffs,
>     the build can fail.

Yes, it does that already (there wasn't a continuation '\' after the
pkglint line) - this does have the side effect that we stop the build
even after a pkglint warning is emitted that differed from those we
expected, which does feel rather strange.  If it turns out to be too
annoying, I can revisit that.

Below is an example of me breaking the build (the thing that causes the
build to break is *not* included in this webrev, which is at
http://cr.opensolaris.org/~timf/pkglint-misc-webrev/  :-)

.
.
Repository refresh initiated.
if [ -d pkgtmp/pkglint-cache ] ; then rm -rf pkgtmp/pkglint-cache ; fi
../../proto/root_i386/usr/bin/pkglint -f 
../../proto/root_i386/usr/share/lib/pkg/pkglintrc -c pkgtmp/pkglint-cache \
        -l 
file:///home/timf/projects/ips/pkglint-misc-pkg.hg/src/pkg/../../packages/i386/repo
 2>pkgtmp/pkglint-out.txt > /dev/null
*** Error code 1 (ignored)
cat pkgtmp/pkglint-out.txt | \
        python -c 'import re, sys; print 
"".join(re.sub("@[0-9TZ.:,-]*","",line, count=2) for line in 
sys.stdin.readlines())' \
        > pkgtmp/pkglint-out-noversions.txt 
sort pkgtmp/pkglint-out-noversions.txt > \
        pkgtmp/pkglint-out-noversions-sorted.txt
/usr/bin/diff pkglint_whitelist.txt \
        pkgtmp/pkglint-out-noversions-sorted.txt
1a2,3
> ERROR opensolaris.action001.1     Username pkg$:::$::$:$:$:5srv in 
> pkg://pkg5-nightly/package/pkg > 8 chars
> ERROR opensolaris.action001.3     Username pkg$:::$::$:$:$:5srv in 
> pkg://pkg5-nightly/package/pkg is invalid - see passwd(4)
*** Error code 1
make: Fatal error: Command failed for target `lint'
Current working directory /home/timf/projects/ips/pkglint-misc-pkg.hg/src/pkg
*** Error code 1
The following command caused the error:
cd pkg; pwd; make install check \
        PATH=$(hg root)/proto/root_$(uname -p)/usr/bin:$PATH \
        PYTHONPATH=$(hg root)/proto/root_$(uname 
-p)/usr/lib/python2.6/vendor-packages
make: Fatal error: Command failed for target `packages'
t...@igyo[657] echo $?
1

        cheers,
                        tim

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to