[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[PATCH]: Fixes and Features, misc things



I regularly do builds using the parallel capabilities of mksquashfs, and
have never seen any problems. All of the PelicanHPC releases were made this
way, and I haven't received any bug reports.
M.

On Thu, May 22, 2008 at 4:44 PM, Otavio Salvador <otavio at debian.org> wrote:

> Thanatermesis <thanatermesis.ecvs at gmail.com> writes:
>
> > diff -Naur live-helper-new-clean/usr/bin/lh_binary_memtest
> live-helper-new-to-commit/usr/bin/lh_binary_memtest
> > --- live-helper-new-clean/usr/bin/lh_binary_memtest   2008-04-28
> 18:18:28.000000000 +0200
> > +++ live-helper-new-to-commit/usr/bin/lh_binary_memtest       2008-05-20
> 20:10:52.644954763 +0200
> > @@ -56,7 +56,7 @@
> >  if [ "${LH_CHROOT_BUILD}" = "enabled" ]
> >  then
> >
> > -     if [ -f chroot/usr/sbin/grub ] && [ ! -f chroot/boot/grub/menu.lst
> ]
> > +     if [ -f chroot/usr/sbin/grub ] || [ -f chroot/sbin/grub ] && [ ! -f
> chroot/boot/grub/menu.lst ] # Comment: needed if we use grub-gfxboot
> >       then
> >               GRUB="yes"
>
> I nack this one.
>
> We shouldn't add support to things we don't use in Debian and gfxboot
> isn't available on Debian.
>
> If we start to add support to all possible tool available the code
> will get messy.
>
> > diff -Naur live-helper-new-clean/usr/bin/lh_binary_rootfs
> live-helper-new-to-commit/usr/bin/lh_binary_rootfs
> > --- live-helper-new-clean/usr/bin/lh_binary_rootfs    2008-04-28
> 18:18:28.000000000 +0200
> > +++ live-helper-new-to-commit/usr/bin/lh_binary_rootfs        2008-05-20
> 20:19:31.838951797 +0200
> > @@ -223,6 +223,11 @@
> >                       MKSQUASHFS_OPTIONS="${MKSQUASHFS_OPTIONS} -info"
> >               fi
> >
> > +             if [ -n "${LH_PROCESSORS}" ] && [ ! "${LH_PROCESSORS}" =
> "none" ]
> > +             then
> > +                     MKSQUASHFS_OPTIONS="${MKSQUASHFS_OPTIONS}
> -processors ${LH_PROCESSORS}" # Comment: This option do a usage of X
> processors when mksquash'
> > +             fi
> > +
>
> While this looks technically possible, why we'd want to limit it?
>
> >               if [ "${LH_PACKAGES_LISTS}" = "stripped" ] || [
> "${LH_PACKAGES_LISTS}" = "minimal" ]
> >               then
> >                       MKSQUASHFS_OPTIONS="${MKSQUASHFS_OPTIONS} -e $(ls
> chroot/boot/${LINUX}* chroot/boot/initrd.img* chroot/${LINUX}*
> chroot/initrd.img* | sed 's|chroot/||g')"
> > @@ -245,7 +250,7 @@
> >                               ;;
> >
> >                       disabled)
> > -                             mksquashfs chroot
> binary/${INITFS}/filesystem.squashfs ${MKSQUASHFS_OPTIONS}
> > +                             mksquashfs chroot
> binary/${INITFS}/filesystem.squashfs ${MKSQUASHFS_OPTIONS} -no-fragments
> -noappend # Comment: recommended options to use
> >                               ;;
> >               esac
>
> Can you clarify _why_?
>
> > diff -Naur live-helper-new-clean/usr/bin/lh_chroot_sources
> live-helper-new-to-commit/usr/bin/lh_chroot_sources
> > --- live-helper-new-clean/usr/bin/lh_chroot_sources   2008-04-28
> 18:18:28.000000000 +0200
> > +++ live-helper-new-to-commit/usr/bin/lh_chroot_sources       2008-05-20
> 20:32:27.461701423 +0200
> > @@ -44,28 +44,10 @@
> >               # Creating lock file
> >               Create_lockfile .lock
> >
> > -             # Configure custom sources.list
> > -             echo "deb ${LH_MIRROR_CHROOT} ${LH_DISTRIBUTION}
> ${LH_SECTIONS}" > chroot/etc/apt/sources.list
> > -
> > -             if [ "${LH_SOURCE}" = "enabled" ]
> > -             then
> > -                     echo "deb-src ${LH_MIRROR_CHROOT}
> ${LH_DISTRIBUTION} ${LH_SECTIONS}" >> chroot/etc/apt/sources.list
> > -             fi
> > -
> > -             if [ "${LH_SECURITY}" = "enabled" ]
> > -             then
> > -                     if [ "${LH_DISTRIBUTION}" != "sid" ] && [
> "${LH_DISTRIBUTION}" != "unstable" ]
> > -                     then
> > -                             echo "deb ${LH_MIRROR_CHROOT_SECURITY}
> ${LH_DISTRIBUTION}/updates ${LH_SECTIONS}" >> chroot/etc/apt/sources.list
> > -
> > -                             if [ "${LH_SOURCE}" = "enabled" ]
> > -                             then
> > -                                     echo "deb-src
> ${LH_MIRROR_CHROOT_SECURITY} ${LH_DISTRIBUTION}/updates ${LH_SECTIONS}" >>
> chroot/etc/apt/sources.list
> > -                             fi
> > -                     fi
> > -             fi
> > +        # Remove possible existing sources.list
> > +        rm -f chroot/etc/apt/sources.list
>
> Wrong indentation
>
> >               # Update indices from cache
> >               if [ "${LH_CACHE_INDICES}" = "enabled" ] && [ -d
> cache/indices_bootstrap ]
> >               then
> > @@ -196,15 +199,44 @@
> >                       then
> >                               mkdir -p cache/indices_bootstrap
> >
> > -                             cp -f chroot/etc/apt/secring.gpg*
> cache/indices_bootstrap
> > -                             cp -f chroot/etc/apt/trusted.gpg*
> cache/indices_bootstrap
> > +                             if ls chroot/etc/apt/secring.gpg* >
> /dev/null 2>&1
> > +                             then
> > +                                     cp -f chroot/etc/apt/secring.gpg*
> cache/indices_bootstrap
> > +                 fi
> > +                             if lsroot/etc/apt/trusted.gpg* > /dev/null
> 2>&1
>                   ^^^ typo
> > +                             then
> > +                                     cp -f chroot/etc/apt/trusted.gpg*
> cache/indices_bootstrap
> > +                             fi
>
 From my POV this is messy. It would be better to use a for to handle them.
>
> > @@ -239,7 +271,22 @@
> >                       rm -rf chroot/var/lib/apt/lists
> >                       mkdir -p chroot/var/lib/apt/lists/partial
> >
> > -                     echo "deb ${LH_MIRROR_BINARY} ${LH_DISTRIBUTION}
> ${LH_SECTIONS}" > chroot/etc/apt/sources.list
> > +                     # Remove first if exists
> > +            rm -f chroot/etc/apt/sources.list
>
> Wrong indentation
>
> > diff -Naur live-helper-new-clean/usr/bin/lh_config
> live-helper-new-to-commit/usr/bin/lh_config
> > --- live-helper-new-clean/usr/bin/lh_config   2008-04-28
> 18:18:28.000000000 +0200
> > +++ live-helper-new-to-commit/usr/bin/lh_config       2008-05-20
> 20:33:20.446951988 +0200
> > @@ -112,6 +112,7 @@
> >  \t    [--union-filesystem aufs|unionfs]\n\
> >  \t    [--exposed-root enabled|disabled]\n\
> >  \t    [--username NAME]\n\
> > +\t    [--processors NUMBER]\n\
> >  \t    [--verbose]"
> >
> >  Local_arguments ()
> > @@ -582,6 +583,12 @@
> >                               shift 2
> >                               ;;
> >
> > +                     --processors)
> > +                             # Warning: the usage of more than 1
> processor for squashfs FS creation, not seems to be pretty stable, better to
> use it just for tests when fast builds are needed
> > +                             LH_PROCESSORS="${2}"
> > +                             shift 2
> > +                             ;;
> > +
>
> I'm sorry but I fail to reconize that it's true. I use squashfs
> creating in a quad-code machine, daily, and it works fine.
>
> I've opted to give this a first review so your broke-up patch series
> will be near of merging.
>
> Thanks by all the work.
>
> Cheers,
>
> --
>        O T A V I O    S A L V A D O R
> ---------------------------------------------
>  E-mail: otavio at debian.org      UIN: 5906116
>  GNU/Linux User: 239058     GPG ID: 49A5F855
>  Home Page: http://otavio.ossystems.com.br
> ---------------------------------------------
> "Microsoft sells you Windows ... Linux gives
>  you the whole house."
>
> _______________________________________________
> debian-live-devel mailing list
> debian-live-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/debian-live-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.alioth.debian.org/pipermail/debian-live-devel/attachments/20080523/25f8cfaf/attachment-0001.htm 


Reply to: