Hi folks,
thanks for comments. I hope I fixed all of your remarks properly.

webrev presentation: http://cr.opensolaris.org/~jf222792/sfwnv_p/

   Jan Forch, Sun Microsystems

On 07/22/09 17:51, Amanda Waite wrote:
> 1. usr/src/pkgdefs/SUNWfreeipmir/prototype_com
>
> You need to change the class of the two files that you've marked as 
> editable.
>
> e none etc/ipmi_monitoring_sensors.conf 0755 root bin
> e none etc/freeipmi.conf 0755 root bin
>
> should be:
>
> e renamenew etc/ipmi_monitoring_sensors.conf 0755 root bin
> e renamenew etc/freeipmi.conf 0755 root bin
>
FIXED
> 2. usr/src/pkgdefs/SUNWfreeipmir/depend
>
> Change the header so that it's the same as 
> usr/src/pkgdefs/SUNWfreeipmiu/depend the default file isn't a good 
> template for the header stuff.
>
FIXED
> 3. usr/src/pkgdefs/SUNWfreeipmir/prototype_sparc
>   usr/src/pkgdefs/SUNWfreeipmir/prototype_i386
>   usr/src/pkgdefs/SUNWfreeipmiu/prototype_sparc
>   usr/src/pkgdefs/SUNWfreeipmiu/prototype_i386
>
> At the end of each file is the package name, and for each you have 
> SUNWfreeipmi, change it to SUNWfreeipmiu or SUNWfreeipmir as appropriate.
>
FIXED
> 4. usr/src/cmd/freeipmi/bmc_intf.h
>
> So where in the build process is this used? It doesn't seem to be 
> pulled in during the build nor is it added in the packaging phase.
>
bmc_intf.h REMOVED
> 5. usr/src/pkgdefs/SUNWfreeipmir/pkginfo.tmpl
>
> You've replaced 'manifest' with 'renamenew' in the CLASSES field, you 
> need all three: "none manifest renamenew"
>
FIXED
> Amanda
>
>
>
> Jan Forch wrote:
>> Hi folks,
>> thanks for comments. I hope I fixed all of your remarks properly.
>>
>> webrev presentation: http://cr.opensolaris.org/~jf222792/sfwnv_p/
>>
>>    Jan Forch, Sun Microsystems
>>
>> --------------------email from 
>> Pradhap.Devarajan at Sun.COM----------------------
>> Hi Michal,
>> Quick run-through found couple of issues.
>>
>> * usr/src/pkgdefs/SUNWfreeipmir/prototype_com
>> - .conf files should be "e" type not "f" type
>>
>> FIXED
>>
>> * usr/src/pkgdefs/SUNWfreeipmiu/depend
>> - check if SUNWlibgpg-error package needs to be added because "ldd 
>> bmc-config" shows libgpg-error library.
>>
>> OUTPUT:
>> make check_deps
>> ==== Checking package dependencies ====
>> /usr/perl5/bin/perl 
>> /builds4/jf222792/sfwnv_o/usr/src/tools/check-deps.pl -e 
>> /builds4/jf222792/sfwnv_o/usr/src/tools/exception_list.check-deps -d 
>> /builds4/jf222792/sfwnv_o/packages/i386/nightly-nd -p 
>> /net/netinstall/export/nv/x/latest/Solaris_11/Product SUNW0sfw 
>> SUNWa2psr SUNWa2psu SUNWaalib SUNWaconf SUNWactivation SUNWaget 
>> SUNWantlr2 SUNWapchd SUNWapchr SUNWapchu SUNWapr13lib SUNWapr13dev 
>> SUNWapr13doc SUNWapu13lib SUNWapu13dev SUNWapu13doc 
>> SUNWapu13dbd-mysql SUNWapu13dbd-pgsql SUNWapu13dbd-sqlite 
>> SUNWapu13-ldap SUNWapch22d SUNWapch22r SUNWapch22u SUNWapch22m-php52 
>> SUNWapch22r-php52 SUNWapch22m-fcgid SUNWapch22r-fcgid SUNWapch22m-jk 
>> SUNWapch22r-jk SUNWapch22m-security SUNWapch22r-security 
>> SUNWapch22m-dtrace SUNWapch22r-dtrace SUNWapch22m-sed SUNWapch22r-sed 
>> SUNWant SUNWautogen SUNWstringtemplate SUNWantlr SUNWareca 
>> SUNWawstats SUNWbash SUNWbcc SUNWbeanshell SUNWbinutils SUNWbison 
>> SUNWbonnieplus SUNWbvi SUNWbwm-ng SUNWbzip SUNWcglib SUNWcimserverr 
>> SUNWcimserveru SUNWclisp SUNWcmake SUNWcommons-collections 
>> SUNWconflict SUNWconmanr SUNWconmanu SUNWconvmv SUNWcupsr SUNWcupsu 
>> SUNWcurl SUNWcvs SUNWdom4j SUNWdoxygen SUNWdosbox SUNWdrools 
>> SUNWdwdiff SUNWejabberdr SUNWejabberdu SUNWelinks SUNWerlang 
>> SUNWerlang-doc SUNWepydoc SUNWexpect SUNWfcgi SUNWfcgi-doc SUNWunrar 
>> SUNWunzip SUNWdvdrw SUNWfetchmail SUNWffiltersr SUNWffiltersu 
>> SUNWfftw2 SUNWfftw3 SUNWflexlex SUNWflexruntime SUNWfping SUNWfpingr 
>> SUNWfoomatic-db SUNWfoomatic-db-enginer SUNWfoomatic-db-engineu 
>> SUNWfppd SUNWfreeipmir SUNWfreeipmiu SUNWgawk SUNWgcc SUNWgccruntime 
>> SUNWgcmn SUNWgdb SUNWgd2 SUNWghostscriptr SUNWghostscriptu 
>> SUNWgimpprint SUNWgpch SUNWggrp SUNWgit SUNWgm4 SUNWgmake 
>> SUNWgnu-automake-19 SUNWgnu-automake-110 SUNWgnu-coreutils 
>> SUNWgnu-dbm SUNWgnu-diffutils SUNWgnu-emacs SUNWgnu-emacs-el 
>> SUNWgnu-emacs-gtk SUNWgnu-emacs-nox SUNWgnu-emacs-x SUNWgnu-gettext 
>> SUNWgnu-gperf SUNWgnu-idn SUNWgnu-mc SUNWgnu-mp SUNWgnu-mpfr 
>> SUNWgnu-nano SUNWgnu-readline SUNWgnu-which SUNWgocr SUNWgrails 
>> SUNWgsed SUNWgsfot SUNWgsfst SUNWgtar SUNWguile SUNWGlib SUNWGtkr 
>> SUNWGtku SUNWgvim SUNWgzip SUNWhal-cups-utilsr SUNWhal-cups-utils 
>> SUNWhexedit SUNWhpijs SUNWhttping SUNWiftop SUNWilmbase SUNWimagick 
>> SUNWImperius SUNWiozone SUNWiperf SUNWipmi SUNWipmir SUNWircii 
>> SUNWjedit SUNWjavamail SUNWjruby SUNWlablgtk SUNWjanino SUNWjarjar 
>> SUNWjaxb SUNWjaxen-core SUNWjdtcore SUNWjdom SUNWjettison 
>> SUNWjodatime SUNWjunit SUNWjxl SUNWkonkretcmpi SUNWlcms SUNWless 
>> SUNWlexpt SUNWlftpr SUNWlftpu SUNWlibconfuse SUNWlibevent 
>> SUNWlibmcrypt SUNWlibmemcached SUNWlibmng SUNWlibnet SUNWlibopenusb 
>> SUNWlibpcap SUNWlibosip2 SUNWlibrsync SUNWlibsigsegv SUNWlibsndfile 
>> SUNWlibtool SUNWlibtorrent SUNWlibxmlrpc-c SUNWlibyaz SUNWlinks 
>> SUNWlogilab-common SUNWlogrotater SUNWlogrotateu SUNWlog4j 
>> SUNWlogilab-astng SUNWlighttpd14r SUNWlighttpd14u SUNWltdl SUNWlua 
>> SUNWlxml SUNWlxmlr SUNWlxml-devel SUNWlxml-python SUNWlxsl 
>> SUNWlxsl-devel SUNWlxsl-python SUNWmeld SUNWmemcached SUNWmemcachedr 
>> SUNWmemcached-java SUNWmercurial SUNWmkcd SUNWmkcdr SUNWmrtg 
>> SUNWmrxvt SUNWmtx SUNWmutt SUNWmuttr SUNWmvel SUNWmysqlr SUNWmysqlt 
>> SUNWmysqlu SUNWmysql5u SUNWmysql5r SUNWmysql5test SUNWmysql5jdbc 
>> SUNWmysql51r SUNWmysql51u SUNWmysql51lib SUNWmysql51test SUNWncft 
>> SUNWneon SUNWnethack SUNWnethackr SUNWnmap SUNWntpr SUNWntpu 
>> SUNWobjectasm SUNWocaml SUNWopenexr SUNWopenldapr SUNWopenldapu 
>> SUNWopenssl-commands SUNWopenssl-include SUNWopenssl-libraries 
>> SUNWopenssl-man SUNWopensslr SUNWopenwsmanr SUNWopenwsmanu SUNWp7zip 
>> SUNWpampkcs11r SUNWpampkcs11u SUNWpampkcs11-docs SUNWpatchutils 
>> SUNWpconsoler SUNWpconsole SUNWpcre SUNWpdsh SUNWpen SUNWpenr 
>> SUNWperl-net-ssleay SUNWperl-pmtools SUNWpgadmin3 
>> SUNWpgbouncer-pg83-config SUNWpgbouncer-pg83-server SUNWphp52d 
>> SUNWphp52u SUNWphp52r SUNWphp52u-mysql SUNWphp52r-mysql 
>> SUNWphp52u-pear SUNWphp52r-pear SUNWphp52u-pgsql SUNWphp52r-pgsql 
>> SUNWpipe-viewer SUNWpmdbi SUNWpmdbdpg SUNWpostgr-82-client 
>> SUNWpostgr-82-contrib SUNWpostgr-82-devel SUNWpostgr-82-docs 
>> SUNWpostgr-82-libs SUNWpostgr-82-pl SUNWpostgr-82-server 
>> SUNWpostgr-82-server-data-root SUNWpostgr-82-jdbc SUNWpostgr-82-tcl 
>> SUNWpostgr-83-client SUNWpostgr-83-contrib SUNWpostgr-83-devel 
>> SUNWpostgr-83-docs SUNWpostgr-83-libs SUNWpostgr-83-pl 
>> SUNWpostgr-83-server SUNWpostgr-83-server-data-root SUNWpostgr-83-tcl 
>> SUNWpostgr-83-tests SUNWpostgr-jdbc SUNWpostgr-upgrade SUNWpowermanr 
>> SUNWpowermanu SUNWprivoxyr SUNWprivoxyu SUNWprocmail SUNWpsutils 
>> SUNWpull-parser SUNWpycups SUNWpylint SUNWpython-pyopenssl 
>> SUNWpython26-pyopenssl SUNWpywbem SUNWpwgen SUNWquiltr SUNWquiltu 
>> SUNWrdiff-backup SUNWrelaxngDatatype SUNWrpm SUNWrsync SUNWrtorrent 
>> SUNWruby18u SUNWruby18r SUNWsaaj SUNWsane-backendr SUNWsane-backendu 
>> SUNWsane-frontend SUNWSblimCimClient SUNWsblimwbemcli SUNWscreen 
>> SUNWscreenrc SUNWserr SUNWseru SUNWserweb SUNWsfdoc SUNWsfinf 
>> SUNWsfman SUNWsfwhea SUNWsg3utilsr SUNWsg3utilsu SUNWshmux 
>> SUNWsimplewbem SUNWslang SUNWslangr SUNWslrn SUNWsmbar SUNWsmbau 
>> SUNWsnack SUNWsnortr SUNWsnortu SUNWsqlite3 SUNWsqlite3docs 
>> SUNWsqlite3tcl SUNWsquidr SUNWsquidu SUNWstunnelr SUNWstunnelu 
>> SUNWsudor SUNWsudou SUNWsvn SUNWsvn-java SUNWsvn-perl SUNWsvn-python 
>> SUNWswig SUNWTcl SUNWtcltls SUNWtcatr SUNWtcatu SUNWtcsh SUNWtexi 
>> SUNWtidy SUNWTk SUNWtop SUNWtor SUNWtor-root SUNWtree SUNWtss 
>> SUNWtss-root SUNWunison SUNWunixodbcr SUNWunixodbc SUNWvim SUNWwgetr 
>> SUNWwgetu SUNWxpp3min SUNWxsdlib SUNWxstream SUNWxom SUNWzlib 
>> SUNWzlibr SUNWzip SUNWzsh SUNWnet-snmp-mgr SUNWnet-snmp-core 
>> SUNWnet-snmp-doc SUNWnet-snmp-utils SUNWnet-snmp-addons SUNWlibusb 
>> SUNWlibusbugen SUNWquaggar SUNWquaggau SUNWbind SUNWbindc SUNWbindr 
>> SUNWslib SUNWwebalizer SUNWwebalizerr SUNWwebminu SUNWwebminr 
>> SUNWwireshark SUNWwiresharkr SUNWwisemanr SUNWwisemanu SUNWwxwidgets 
>> SUNWwxwidgets-devel SUNWusbccid SUNWdiffstat SUNWcsu
>>  *** /usr/lib/cups/backend/hal (SUNWhal-cups-utils) requires
>>      /usr/lib/libdbus-1.so.3 which is found in SUNWdbus-libs, but 
>> that is
>>      not listed as a package dependency.
>>  *** /usr/lib/cups/backend/hal (SUNWhal-cups-utils) requires
>>      /usr/lib/libdbus-1.so.3 which is a link to
>>      /usr/lib/libdbus-1.so.3.4.0, which is found in SUNWdbus-libs, 
>> but that
>>      is not listed as a package dependency.
>> ERROR: TOTAL: 1 errors of type LinkNotInDepend
>> ERROR: TOTAL: 1 errors of type NotInDepend
>>
>> FIXED
>> -> seems SUNWhal-cups-utils pkg is broken but thats not my business 
>> right?
>>
>> * *usr/src/cmd/freeipmi/bmc_intf.h
>> - not sure from where this file originated, if its for special 
>> purpose add it to comments section of METADATA file.
>>
>> FIXED (added comment to METADATA file)
>>
>> * usr/src/cmd/freeipmi/bmc-watchdog.xml
>> - shouldn't it wait for filesystem/local
>>
>> ANSWER:
>> The manifest is almost exact copy of "ipmievd" - I do not see a 
>> reason why it should wait for filesystem/local, if it waits for 
>> filesystem-usr. Do you think that additional dependency on a 
>> filesytem/local is needed?
>>
>> * usr/src/cmd/freeipmi/ipmidetectd.xml.html 
>> usr/src/cmd/freeipmi/svc-ipmidetectd
>> - your prefix is "/usr" but it refers to 
>> /*usr/local/etc/ipmidetectd.conf is it correct ?
>>
>> ANSWER:
>> Because of ARC review, we do not put anything into /etc. Instead, we 
>> have chosen a path for configuration file that at least at certain 
>> point resembles to path used on linux version of freeipmi (on linux, 
>> freeipmi expects the configuration file to be present at 
>> /etc/ipmidetectd.conf). The configuration file has to be created by 
>> an user - and the starter script of the service expects the script to 
>> be in that location, so answer to your question is "yes, it is correct".
>>
>> regards,
>> Pradhap.D
>> --------------------email from 
>> Pradhap.Devarajan at Sun.COM----------------------
>>
>> On 07/22/09 14:58, Amanda Waite wrote:
>>> 1. usr/src/pkgdefs/SUNWfreeipmir/prototype_com
>>>
>>> - If you want to preserve the conf file across pkgrm/pkgadd then you 
>>> need to change it from 'f' to 'e' and then specify which action 
>>> class to use, either renamenew or renameold, i.e.:
>>>
>>> e renamenew etc/ipmi_monitoring_sensors.conf 0755 root bin
>>> e renamenew etc/freeipmi.conf 0755 root bin
>>>
>>> (they probably don't need the execute bits set)
>> FIXED
>>>
>>> - You also need to add the class to the list of 'packaging files' in 
>>> prototype_com
>>>
>>> i i.renamenew
>>>
>>> - and add the class to the classes field in pkginfo.tmpl
>>>
>> FIXED
>>> 2. usr/src/pkgdefs/SUNWfreeipmiu/depend
>>>
>>> - Have you run 'make check_deps'  in ${SRC}/pkgdefs?
>>>
>>> >. edit your sfw-developer.sh script. At the bottom, set SFW_PKGDB
>>> >to point to the package directory of a current Solaris install image.
>>> >> # SFW_PKGDB=/net/netinstall/export/nv/s/latest/Solaris_11/Product
>>> >and use bldenv(1) or just set it in your env.
>>> >2. cd usr/src/pkgdefs
>>> >3. make check_deps.
>>>
>> viz. email from Pradhap Devarajan
>>>
>>> 3. usr/src/pkgdefs/SUNWfreeipmir/prototype_sparc
>>>    usr/src/pkgdefs/SUNWfreeipmir/prototype_i386
>>>    usr/src/pkgdefs/SUNWfreeipmiu/prototype_sparc
>>>    usr/src/pkgdefs/SUNWfreeipmiu/prototype_i386
>>>
>>> - Where are these files? Are they in the workspace but not checked 
>>> in? I've never built anything without at least minimal versions of 
>>> these files, maybe SFW does build without them but I'd prefer to see 
>>> them there.
>>>
>> FIXED
>>> 4.  usr/src/cmd/freeipmi/install-sfw
>>>
>>> - echo "install.sfw"  <- I guess this was left over from the debugging
>>>
>> FIXED
>>> 5. usr/src/cmd/freeipmi/bmc_intf.h
>>>
>>> - I couldn't work out where this file is used
>>>
>> viz. email from Pradhap Devarajan
>>>
>>> Amanda
>>>
>>>
>>>
>>> Michal Bachorik wrote:
>>>> Hi all,
>>>>
>>>> I am in an urgent need to re-review the freeipmi workspace (owned 
>>>> by my colleague Jan Forch, working on freeipmi porting). The 
>>>> freeipmi workspace has already been reviewed by Amanda Waite and 
>>>> Paul Cunnigham almost a month ago, but recently a nightly build 
>>>> process has been changed in a way, that freeipmi workspace nightly 
>>>> build was failing.
>>>>
>>>> The updated workspace has 2 new files that were not reviewed before:
>>>>
>>>> *usr/src/cmd/freeipmi/man-fix
>>>> **usr/src/cmd/freeipmi/man-fix-sed
>>>>
>>>> *The fix applies an extra sed script file on the man pages after 
>>>> untar and before the build of package in a way that words "error" 
>>>> and "error message" are filtered from man pages (the fix is 
>>>> supposed to work with future versions of freeipmi as well).
>>>> *
>>>> *Webrev is located at
>>>> http://cr.opensolaris.org/~jf222792/sfwnv_o/
>>>>
>>>> Thank you in advance,
>>>>
>>>> Michal
>>>>
>>>
>>
>


Reply via email to