Hi Brock,
On Thu, 2010-09-02 at 15:20 -0700, Brock Pytlik wrote:
> > http://cr.opensolaris.org/~timf/pkglint-misc-webrev
Thanks for taking a look!
> transforms/defaults:
> 78: Could you add an explanation about what REV is in practice or what
> purpose it serves?
It seems to be a convention we're using, that VERSION attributes in SVR4
packages produced by Sun always have a REV field.
It's not described in the Application Developer's Packaging Guide[1]
though. I suspect it's used to disambiguate different builds of the same
package version (for purposes of targeting patches perhaps? SVR4
historians on this list will know more than I, and why PSTAMP isn't
sufficient?)
I found a reference to it in
/ws/on10-feature-clone/usr/src/pkgdefs/bld_awk_pkginfo.ksh
Looking at what we've got at the moment,
t...@linn[6301] pkg search -r -o action.raw :legacy:: > legacy-actions.txt
t...@linn[6302] wc -l legacy\-actions.txt
19172 legacy-actions.txt
t...@linn[6303] grep -v REV= legacy\-actions.txt | wc -l
311
nearly all legacy actions we've been producing have them.
> lint/pkglint_manifest:
> 292: line needs to be wrapped, also, could the XXX have more explanation
> in it. I don't know whether it's meaning is we're not accounting for
> timestamps and we should, or the other way around.
I should have removed that comment - it was a note to self during
development, to make sure that lint_fmri_successor() could deal with
comparisons where the versions are the same, but the timestamp in the
new package was missing.
For linting, comparing those should result in the timestamp-less package
being deemed a successor to the timestamped package.
That change made it into this webrev, so the comment was wrong and I've
removed it.
> lint/pkglint_action:
> 479, 480: You might consider using a named tuple here, it might make
> lines like 698 a bit more readable. Not a big deal either way.
I didn't know about named tuples, thanks! Fixed this.
> 727: could you add a comment how we'd end up here? My naive reading of
> the code would have lead me to expect that either we'd have a valid mf
> or mf would be None.
You're right. We shouldn't be raising an exception in
engine..get_manifest(..) except for the case where a search returned
more than one package and we haven't asked for a LATEST_SUCCESSOR
search. Returning None should be enough for callers to determine that a
search hasn't yielded any results. Fixed.
I've updated the webrev in-place.
cheers,
tim
[1] http://docs.sun.com/app/docs/doc/817-0406/ch2buildpkg-22939?a=browse
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
