Hi, Paul, Thank you very much for your comments, please see in line. > Jason, > > This mainly looks good to me, just a few comments .. > > Paul > > Jason Zhao wrote: >> >> I made several changes recently, and I think it had better >> have a review again. The main changes is add a snort.conf >> config file into the package, so the original SUNWsnort package >> was split to 2 packages, SUNWsnortr and SUNWsnortu. SUNWsnortr >> will only deliver snort.conf file to /etc, and SUNWsnortu is >> original SUNWsnort package which deliver all the binaries. >> >> Here is webrev: >> http://cr.opensolaris.org/~jxzhao/snort/webrev > > 1. usr/src/cmd/snort/sunman-stability > Line ... > 20 Availability SUNWsnort > that should probably be now ... > Availability SUNWsnortu SUNWsnortr Done > > 2. usr/src/cmd/snort/Makefile.sfw > Should the 'configure options' explicitly define where the > /etc/.. stuff lives ? In my opinion, no. On configure phase, we could not set a option which could allow snort.conf to be installed during "make install". On configure script, I could not find it.
Actually, snort.conf was not installed by default. Developers of snort would like to have users to copy or create their own snort.conf and other config files if have. Please see the following thread. http://www.snort.org/archive-4-1297.html However I do think a pre-defined snort.conf could simplify the work for customers, so I need to copy it after "make install" in install-sfw. > > 3. usr/src/pkgdefs/SUNWsnortr/pkginfo.tmpl > As this is a root pkg you probably don't need the > pkg version on the DESC= line (that way you may not > need to update this file next time the src pkg version > is updated. Modified. Now the DESC line is ...... DESC="snort - Network Intrusion and Protection Detector (Root)" ...... > > 4. usr/src/pkgdefs/SUNWsnortr/prototype_com > Does this need to be preserver over a package update ? .. > 50 f none etc/snort.conf 0644 root bin Thank you, it does need to be preserved, so the changed line is: ...... e renamenew etc/snort.conf 0644 root bin ...... The class "renamenew" since customer is encouraged to add/remove content in the file. > > 5. usr/src/pkgdefs/SUNWsnortu/depend > Does this have a dependency on the root pkg ? I add a dependency on SUNWsnortr pkg. Now the depend file is ...... P SUNWcar Core Architecture, (Root) P SUNWcakr Core Solaris Kernel Architecture (Root) P SUNWkvm Core Architecture, (Kvm) P SUNWcsr Core Solaris, (Root) P SUNWckr Core Solaris Kernel (Root) P SUNWcnetr Core Solaris Network Infrastructure (Root) P SUNWcsu Core Solaris, (Usr) P SUNWcsd Core Solaris Devices P SUNWcsl Core Solaris Libraries P SUNWgccruntime GCC Runtime P SUNWlibms Math & Microtasking Libraries (Usr) P SUNWlibmsr Math & Microtasking Libraries (Root) P SUNWlibpcap libpcap - a packet capture library P SUNWpcre Perl-Compatible Regular Expression Library P SUNWlibsasl Simple Authentication and Security Layer shared library and plugins P SUNWpr Netscape Portable Runtime Interface P SUNWtls Network Security Services P SUNWsnortr snort - Network Intrusion and Protection Detector (Root) ...... > > 6. usr/src/pkgdefs/SUNWsnortu/pkginfo.tmpl > Put the version number on the DESC= line in > brackets (the more common method), eg .. > DESC="....... (2.8.3.2)" > OK, now it is: ...... DESC="snort - Network Intrusion and Protection Detector (Usr) (2.8.3.2)" ...... The new webrev is updated, please review it again and tell me your comments. http://cr.opensolaris.org/~jxzhao/snort/webrev Thanks in advance! Thanks Jason
