Karen Tung wrote:
> Hi Evan,
>
> Thanks for your response. I still have questions about item 3. See below.
>
> Evan Layton wrote:
>> Karen Tung wrote:
>>> Hi Evan,
>>>
>>> - I checked bug 7838, it doesn't contain any detail of your
>>> implementation.
>>> I was hoping to understand more why you implemented things the way
>>> you do there.
>>> So, I guess I will ask my question here. :-)
>>>
>>> 1) Why add the "entire" package to the AI manifests, and why make it
>>> the first package?
>>> I think it is sufficient to get the version number from SUNWcsd or
>>> SUNWcs. I believe
>>> those are always there in any image or installation.
>>
>> The reason the entire package is used here is because SUNWcsd and
>> SUNWcs don't always change when there is an update however for any
>> change the entire package does change. In order to be able to
>> differentiate between these the entire package was the only thing I
>> could think of to use.
> OK, this makes sense.
>
>>
>>>
>>> 2) At this time, the version number for the "Install running on", and
>>> the "System installed with:"
>>> are printed at 2 separate locations in the log file. I think it is
>>> much better for people to find
>>> the information if they are printed one following the other.
>>>
>>
>> OK that's not difficult to do I can just move the first call to
>> log_bld_info to just before the second one.
> OK
>>
>>> 3) In log_bld_info(), you used temp file to capture the output of
>>> "pkg.....". I think it is much simpler
>>> to use "popen()" to execute the command and get the output and
>>> completely avoid the temp file.
>>
>> definitely not simpler... ;-)
>>
>> While this is a bit complicated I really don't want to have to figure
>> out the pid, do a popen(), attach the pipe, run system() and then grab
>> the output from the package command. I'd much rather leave it the way
>> it is...
> Actually, looking at the popen(3C) man page, or googling for popen gives
> many examples on how to use
> it. Why does it involve figuring out the pid, attaching the pipe and
> running system() in this case?
umm you're right, it doesn't...
I guess I could change this to use something more like:
void
log_bld_info(char *mntpnt, char *comment)
{
char rel_str[BUFSIZ] = {0};
char cmd[MAXPATHLEN] = {0};
FILE *fp = NULL;
int fd = 0, i = 0;
snprintf(cmd, MAXPATHLEN, "%s info entire | grep FMRI",
PKG_PATH);
if ((fp = popen(cmd, "r")) != NULL) {
if (fgets(rel_str, BUFSIZ, fp) != NULL)
om_log_print("%s\n%s\n", comment, rel_str);
(void) pclose(fp);
return;
}
om_debug_print(OM_DBGLVL_WARN, "log_bld_info: Unable to "
"retrieve build version information\n");
return;
}
>>
>>>
>>> 4) The format of the output contains the name of the package you used
>>> for querying the
>>> output, should we parse the output to not include the name of the
>>> package?
>>
>> I left that there so that it's obvious what is being compared. This
>> way we know that this is the entire package that we're looking at and
>> don't have to attempt to figure out what we're actually comparing.
>>
> OK, this makes sense.
>
> Thanks,
>
> --Karen
>
>> Thanks,
>> -evan
>>
>>>
>>> Thanks,
>>>
>>> --Karen
>>>
>>> Evan Layton wrote:
>>>> I need two reviewers for:
>>>>
>>>> 7838 Log installer build number and installed software build number
>>>> in install_log
>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7838
>>>>
>>>> and
>>>>
>>>> 6810 AI image missing D-Trace toolkit
>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=6810
>>>>
>>>> Webrev:
>>>>
>>>> http://cr.opensolaris.org/~evanl/7838/
>>>>
>>>>
>>>> I've tested this with both an AI and slim CD image and get the
>>>> proper output.
>>>>
>>>> In the log you'll see lines similar to:
>>>> <OM Apr 15 19:14:09> Install running on:
>>>>
>>>> FMRI: pkg:/entire at 0.5.11,5.11-0.111:20090331T092149Z
>>>>
>>>> and
>>>>
>>>> <OM Apr 15 19:40:42> System installed with:
>>>>
>>>> FMRI: pkg:/entire at 0.5.11,5.11-0.111:20090415T070733Z
>>>>
>>>>
>>>> Thanks,
>>>> -evan
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>>
>
