See my comments below.
Steve C.
Spoorthy H.S wrote:
> Hi Steve,
>
> Please see inline for the comments. If its fine, I'l send the new webrev
> with Targetdirs.
>
> Steven M. Christensen wrote:
>> Spoorthy,
>>
>> I see several issues.
>>
>> You have made changes to usr/src/Targetdirs, but I don't see this in
>> your webrev.
> Added.
OK
>>
>> Also, in your install-sfw you have an /etc/init.d directory, but this
>> is not in Targetdirs. What about the lib/examples directory or
>> lib/conman?
> They are included in Targetdirs as
> /usr/lib/conman \
> /usr/lib/conman/examples \
> /usr/lib/conman/exec \
Sorry, missed those lines. OK
>
>>
>> You have
>>
>> _install E conman ${ROOT}/etc/logrotate.d/conman 555
>> _install E conman ${ROOT}/etc/default/conman 555
>> _install N etc/conman.sysconfig ${ROOT}/etc/default/conman.sysconfig 744
>> _install E conman ${ROOT}/etc/init.d/conman 555
>>
>> in install-sfw.
>>
>> Why are the
>>
>> logrotate.d/conman
>> default/conman
>>
>> files E and 555? These are not executables. Should they not be
>> N 444 or maybe 644? This would carry over into the prototype_com
>> files as well.
>>
> They are the executables.
When I build and install conman, it puts the following file non
executable file in etc/logrotate.d -
##
# ConMan Logrotate Configuration
##
# $Id: conman.logrotate 500 2005-02-10 02:19:46Z dun $
##
##
# Example logrotate entry for ConMan.
# Be sure to create the /var/log/conman/ and /var/log/conman.old/ dirs.
##
# /var/log/conman/* {
# compress
# missingok
# nocopytruncate
# nocreate
# nodelaycompress
# nomail
# notifempty
# olddir /var/log/conman.old/
# rotate 4
# sharedscripts
# size=5M
# weekly
# postrotate
# /usr/bin/killall -HUP conmand
# endscript
it does not put the conman executable there. The conman file I see in
etc/default is what you call conman.sysconfig, but there is no
conman executable in the default directory.
I am just confused about why you have chosen to put the conman
executable in three different directories.
>> Is there a depend file for SUNWconmanr? There is one for SUNWconmanu.
>>
> No depend file for SUNWconmanr. It does not depend on any packages.
> SUNWconmanu depends on root package - SUNWconmanr.
Of course. I see that now. OK
>
> Thanks
> Spoorthy
>> That's all I see right now.
>>
>>
>> Steve C.
> Spoorthy H.S wrote:
>>> Hi Paul,
>>>
>>> New webrev is at http://cr.opensolaris.org/~spoorthy/conmanupd1/
>>> and comments inline..
>>>
>>>
>>> Paul Cunningham wrote:
>>>> Spoorthy,
>>>>
>>>> See below for my comments ...
>>>>
>>>> Paul
>>>>
>>>> Spoorthy H.S wrote:
>>>>>
>>>>> New webrev is at
>>>>> http://cr.opensolaris.org/~spoorthy/conmanupd/
>>>>
>>>> === Start of Comments ===
>>>>
>>>> 1. usr/src/Targetdirs
>>>> Put your new bits in alphabetically ...
>>>> /etc/apache2/2.2 \
>>>> /etc/apache2/2.2/conf.d \
>>>> + /etc/logrotate.d \
>>>> + /etc/default \
>>>> /etc/lighttpd \
>>>> /etc/lighttpd/1.4 \
>>>> /etc/lighttpd/1.4/conf.d \
>>>> /etc/openwsman \
>>>>
>>> Done
>>>> 2. usr/src/cmd/conman/METADATA
>>>> Remove the last blank line
>>>>
>>>> Complete this line ...
>>>> 11 BUGTRAQ:
>>>>
>>> Raised the request. Will update the field.
>>>> 3. usr/src/cmd/conman/Makefile.sfw
>>>> On line 33 you change ...
>>>> --sysconfdir=/etc
>>>> to ..
>>>> --sysconfdir=$(CFGETC)
>>>>
>>>> Cosmetic: move the line-space up a line, eg. ..
>>>> 19 # CDDL HEADER END
>>>> 20 #
>>>> ..
>>>> 21 #
>>>> 23 # Copyright 2009 Sun Microsystems, Inc. All rights reserved.
>>>>
>>> Done
>>>> 4. usr/src/cmd/conman/install-sfw
>>>> Cosmetic: add extra line-space after line 45
>>>>
>>>> On lines 74 to 83 you could use ${LIB} instead of
>>>> ${PREFIX}/lib
>>>>
>>> Declared variable is used.
>>>> 5. Various SUNWconmanr/* and SUNWconmanu/* files
>>>> Change the top of these files so they all conform to
>>>> that in ...
>>>> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/";
>>>>
>>>>
>>>>
>>>> 6. usr/src/pkgdefs/SUNWconmanr/pkginfo.tmpl
>>>> As this is a root package you may want to remove the
>>>> pkg version number on the DESC= line (that way this
>>>> file should not need changing when the src pkg
>>>> is up versioned)
>>>>
>>> Removed the version number.
>>>> 7. usr/src/pkgdefs/SUNWconmanr/prototype_com
>>>> Do any of the installed /etc files need preserving
>>>> over a SUNW pkg upgrade? If so you need to handle that.
>>>>
>>> I dont think it happens.
>>>> 8. pkgdefs/Makefile
>>>> & cmd/Makefile
>>>> The changes to these are not in the webrev
>>>>
>>> Done
>>>> 9. usr/src/pkgdefs/SUNWconmanu/copyright
>>>> Add src pkg owner copyright lines (after sun disclaimer)
>>>> extracted from the src files in the uncompressed tarball,
>>>> see example at ...
>>>> "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright";
>>>>
>>>>
>>>>
>>> The copyright file is "COPYING" in my package. I copied the contents
>>> of it to "copyright" file after SUN disclaimer.
>>>> 10. usr/src/pkgdefs/SUNWconmanu/depend
>>>> Are there any other dependencies?
>>>>
>>> No other dependencies except Root pakage - SUNWconmanr. Do let me
>>> know how can I run the "package dependency checker".
>>>
>>> Thanks
>>> Spoorthy
>>>> === End of Comments =====
>>> _______________________________________________
>>> sfwnv-discuss mailing list
>>> sfwnv-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>>>
>
