Jack,
The logic and coding all looks fine to me. These are mostly minor issues:
auto_install.h
==========
all good
auto_install.c
=========
1848 * booted environment. This must be done before semantic
1849 * validation, since this may add required devices which
1850 * are needed to pass validation.
Have you re-instated this comment accidentally? Semantic
validation is no longer done in AI. And in any event, if it was
being done, it would be done from the call to ai_setup_manifest_image
which is now called before this section of code.
1854 * environment. It is possible that a package missing
here will
1855 * already included in the target install, so let the
insert "be" after "already".
1864 /* FAILURE or PKG_NOT_FND */
This comment is a little cryptic. Could you expand it slightly?
auto_ddu_lib.c
===========
110 char **type, char **location, char **name, char **descr, char
**inf_link,
It looks like I incorrectly renamed some "origin" variables
to "location" previously. Thanks for correcting this. But...
115 static int ai_du_process_manual_pkg(py_state_t *py_state_p,
116 PyObject *pPackageList, char *location, char *type, char *name,
117 char *noinstall);
...I don't think this is one of those? The variable you have
renamed from "origin" to "location" here really is an "origin"
value from the AI Schema.
I think only the variables used to store the "pkg_location"
value from DDU should be called location/locn. I believe all
the others should remain as "origin". Do you agree?
196 PyErr_Print();
Could this be replaced with a call to ai_dump_python_exception()
as you have done elsewhere?
line 208:
- what is the reason for moving the Python thread initialization
from before the importing 'til after? Most other places that
import Py code do it the other way round.
1310 * <search_all addadd="false">
s/addadd/addall/
(I recognize this typo! I think you corrected it in an
early draft of my previous changes?!?)
1319 * publisher name and origin are both optional, but both must be
specified
1320 * together.
Again - this looks familiar. I think Evan had asked me to
change this to something like:
* publisher and origin are both optional, but if one is specified then the
* other must also be specified.
1908 * it be that that the package is not found or there was an issue during
s/that that/that/
- Dermot
On 08/23/10 19:53, Jack Schwartz wrote:
Hi everyone.
Please review a webrev for the following bugs:
6978976 AI manifest<searchall> should be more forgiving of missing
packages.
6978978 DDU install should log what pkg repo they are trying to get
packages from
6978979 AI Driver Update code needs to check for duplicate driver
requests
6971871 Handling of expected DDU Python exceptions shouldn't display
tracebacks in AI logfiles
6971872 Preface Driver Update messages with more useful<add_drivers>
instead of function names
http://strongheart.sfbay/public_webrevs/100820.1/webrev/
Please review if possible by Weds COB 8/25. I would like to get this
back to slim_source before I leave on vacation, to push on Friday 8/27
AM. Unfortunately, I had to wait until now to send this out, for the
manifest schema changes to go back first. I've been testing for the
last few days however.
Changes:
Changes to auto_install.h are mostly reformatting to pull in line
lengths to< 80 chars. There are three other changes to that file, all
trivial: (line 49, 372, 374)
Changes to auto_install.c are for better messaging and handling for
6978976
Changes to default.xml are to uncomment the<add_drivers> clause again.
Changes to auto_ddu_lib.c can be roughly broken down as follows:
- 163 - 238: revamp of auto_ddu_lib_init/fini since they handle more
modules now.
- 245 - 280: the guts of the fix for 6978971 regarding Python exceptions.
- 401 - 449: the fix for 6978979
- 506 - 545: origin of the new return code for when searched packages
are missing
- 815 - 874: beef up the routine that returns info for checking for
duplicate devs
- 1487 - 1512: list repo packages are taken from (6978978)
Throughout: propagation of error to distinguish the package not found
error from the others; fixing of error messages for 6978972, lint and
other cleanup.
Thanks,
Jack
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
