On 02/20/2012 08:10 AM, Mr Dash Four wrote:
> Thanks for that. I just had a quick glance over the install.sh script in
> the main shorewall archive (will have a more thorough look towards the
> end of the week):
>
> 1. Line 128:
> case "$LIBEXEC" in
> /*)
> ;;
> *)
> LIBEXEC=/usr/${LIBEXEC}
> #! Why?! My understanding is that the LIBEXEC is specified in full
> (prefix/destdir excluding) - not just the bit after "/usr". This is also
> inconsistent with the same setting in uninstall.sh.Please see http://www.shorewall.net/Install.sh. Prior to Shorewall 4.4.20, LIBEXEC and PERLLIB were assumed to be relative to /usr; the code above exists for backward compatibility. It is also > inconsistent with what you assume in line 380: "mkdir -p > ${DESTDIR}${LIBEXEC}/$PRODUCT" - no "/usr" there! By the time that code is executed, the code at line 128 will have already generated a fully-qualified path name. > #! Besides uninstall.sh has an error on line 115: "rm -rf > $PERLLIB}/Shorewall/*" should really be "rm -rf ${PERLLIB}/Shorewall/*" > or "rm -rf $PERLLIB/Shorewall/*". Corrected: thanks. > #! In uninstall.sh you also use hard-coded values where shorewall was > installed (/sbin/shorewall, /usr/share/shorewall etc) - I think that's > wrong. I am also unsure how uninstall.sh is going to cope with the > arch-specific settings you use when install.sh does its job. > 2. INSTALLSYS and TARGET names: it would be better to rename them as > "BUILD" and "HOST", makes more sense (and is also consistent with their > automake value counterparts). I suspected that there were probably better names for these variables. I'll switch them in Beta 2. > > 3. Line 144: > > if [ -z "$INSTALLSYS" ]; then > case $(uname) in > CYGWIN*) > INSTALLSYS=CYGWIN > ;; > Darwin) > INSTALLSYS=MAC > ;; > *) > if [ -f /etc/debian_version ]; then > INSTALLSYS=DEBIAN > elif [ -f /etc/redhat-release ]; then > if [ -d /etc/sysconfig/network-scripts/ ]; then > #! I have /etc/sysconfig/network-scripts/ present, but my system cannot > be classified as "REDHAT" - it is Fedora. Why the distinction by the > way? Do you need it because of RHEL? I need it in the Shorewall-init installer so I replicated the detection logic on all of the installers for compatibility and to handle any future deviations between RHEL (and derivatives) and Fedora. > 4. Line 301: > > if [ -n "$DESTDIR" ]; then > #![...] > elif [ -z "$DESTDIR" ]; then > #! if .. else - it is either one or the other, there isn't a 3rd > alternative .This is a common use you employ throughout the install.sh > script > Duh -- corrected. Thanks. > 5. Line 355: > > FEDORA|REDHAT) > install_file init.fedora.sh ${DESTDIR}/etc/init.d/$PRODUCT 0544 > #! This WILL fail! Either use "${DESTDIR}${DEST}/$PRODUCT" or > "${DESTDIR}/etc/rc.d/init.d/$PRODUCT" > ;; > Hmmm -- on my Fedora 16 installation, /etc/init.d is a symbolic link to /etc/rc.d/init.d; 'rpm -qf /etc/init.d' indicates that it is installed by chkconfig. So I guess the current code only fails if chkconfig isn't installed. I'll update all install scripts to assume /etc/rc.d/init.d for Fedora and RHEL. -Tom -- Tom Eastep \ When I die, I want to go like my Grandfather who Shoreline, \ died peacefully in his sleep. Not screaming like Washington, USA \ all of the passengers in his car http://shorewall.net \________________________________________________
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2
_______________________________________________ Shorewall-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/shorewall-devel
