Danek Duvall wrote:
http://cr.opensolaris.org/~richb/pkg-2217-v1/
I think for packages which are updating or introducing new driver actions,
we should have new files in the build 125 directory, which you indicate is
what's actually going on in your workspace. However, for files which have
simply had their formatting changed, they shouldn't be copied.
I could certainly go back and rework create_driver_changes.py to
do this, but it would be a fair bit of work. Also there has been a
fair amount of "hand tuning" of the code base to fixup things
that the script doesn't automatically handle. All these changes
would have to be applied again.
Until it's a deal breaker, I'd like to leave it the way it is (i.e. all
new files under 125/). When I spoke with David on this during a pre-review
he indicated that everything should be in 125.
You don't mention much about devlink.tab, though I see you added at least
one entry for it into a new action. Should that be dropped, too, and have
its "empty" file delivered?
I'll have to check, but it might already be an empty file initially
(apart from a large comment).
SUNWmd: the two driver actions look identical; why have separate sparc and
x86 declarations?
Well the script blindly sees that:
+$(i386_ONLY) perms="admin 644 root sys" \
is different from
+$(sparc_ONLY) perms="admin 0644 root sys" \
and therefore creates two entries. As you point out, these could be
identical, and I'll adjust it accordingly.
SUNWldom: what happened to cnex and drctl?
These should be there. There are SPARC sun4v drivers, and the
create_driver_changes.py worked off intel and SPARC sun4u files,
with the sun4v ones being added in by looking at the results of:
http://defect.opensolaris.org/bz/attachment.cgi?id=2717
But I missed these. I'll add them in
SUNWssad: for consistency, put quotes around the first alias value.
Okay.
SUNWmv88sx: could you sort the aliases?
I was asked by David not too. He wanted them in the same order
as they are in the Nevada files so that it's easier to see what
changes occur from build to build.
I took a quick look at the attachments to 2217 to swee what difference
you've found. I assume a bunch of these are due to non-redistributable
drivers, which makes sense. But where are the extra entries coming from?
Which extra ones do you mean? Can you give an example?
And things like tpm in driver_aliases on sun4v looks like it's wrong (no
quotes?). And there are some drivers that you're saying are missing, but,
I think, shouldn't be (clone, nge, rtls). And is the fact that ata is,
normally, in both scsi and dada classes, but isn't on the opensolaris
image, a problem?
Some of these are "false positives", suggesting that I have a problem
with my compare scripts. The scsi/dada one is one such example.
Both the "standard" image and the modified one have both of these
(as does the IPS package definition file for that driver).
For each one, I checked the actual IPS package definition file
and the entries in the package manifests and I believe they are
correct. I don't know why the compare scripts are flagging them.
I'll spend some more time trying to track that down.
Thanks for the review.
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
