Hi Jan, You've addressed all my comments and I'm happy.
I think that there's still the one that Pradhap mentioned yesterday which was the usr/local/etc path that's used in one of the properties in ipmidetectd.xml : <property_group name="config" type="application"> <propval name="options" type="astring" value="-c /usr/local/etc/ipmidetectd.conf"/> 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 > > 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 >>>>> >>>> >>> >> >
