Re: [Gnewsense-dev] Progress in testing a patched Builder on newer Ubuntu releases!

[Bake Timmons]

"Karl Goetz" <address@hidden> writes:

> On Sun, August 30, 2009 04:46, Bake Timmons wrote:
>> Hi,
>>
>> Attached is a patch to Builder[1] that enables me to build a LiveCD
>
> Comments inline, and preceeded with 'kk '
[2. application/octet-stream; builder-patch-for-jaunty-reviewd]...

    diff -ru a/do-update b/do-update
    --- a/do-update     2009-08-25 12:38:40.000000000 -0400
    +++ b/do-update     2009-08-29 13:27:04.000000000 -0400
    @@ -77,8 +77,8 @@
            DIR=$(reprepro -b $REPODST dumpreferences | fgrep 
"/$1_${NEW_VERSION}_" | cut -d/ -f 2-4 | head -1)
            if [ -n "$DIR" ]; then 
                    # In another component - use that
    -           for i in $REPODST/pool/$DIR/*_"${NEW_VERSION}"_*.deb; do
    -                   reprepro -b $REPODST includedeb $4 $i
    +           for j in $REPODST/pool/$DIR/*_"${NEW_VERSION}"_*.deb; do
    +                   reprepro -b $REPODST includedeb $4 $j

    kk This change seems totally gratuitous.

Yes, it does seem that way, but in fact it fixes bug #326 in which that
$i loop variable gets clobbered by an outer $i loop variable.  One of
the ugly pitfalls of shell programming it seems. ;)

                    done
            else
                    # Build it
    @@ -118,7 +118,6 @@
            # Non-Free, or no valid use in gNewSense
            # Paramater to remove_package is the source package name
            remove_package kubuntu-desktop
    -   remove_package ubuntu-artwork

    kk Why this removal (more below)?

>From the TODO: comment below, I explain that I reposition line this as a
workaround so as to allow gnewsense-artwork to build w/ Jaunty.  Of
course, I could also try *understanding* what's going on to get a proper
fix. :)


            remove_package ubuntu-desktop
            remove_package nvidia-xconfig
            remove_package linux-wlan-ng-firmware
    @@ -182,15 +181,27 @@
                    # ensure_updated kubuntu-desktop k$DISTRONAME_L-desktop 
$RELEASE$i ./gen-kmeta $KMETA_VERSION

                    FOUND=0
    +           case $(echo $MIRRORDIST | tr A-Z a-z) in
    +               "hardy")
    +                   export KERNEL_RELEASE="2.6.24"
    +                   ;;
    +               "jaunty")
    +                   export KERNEL_RELEASE="2.6.28"
    +                   ;;
    +               *)
    +                   echo "* Unhandled distribution: " $MIRRORDIST
    +                   exit 1
    +           esac
    +

    kk This should go in config.auto

Right, I see what you mean.

    @@ -215,6 +225,12 @@
                    fi

            done
    +
    +   # TODO: figure out why, with a Jaunty snapshot,
    +   # gnewsense-artwork fails to build when the following line
    +   # precedes the ensure_updated ubuntu-desktop ... line above
    +
    +   remove_package ubuntu-artwork
     done

    kk I'm not sure what this note is telling me.

FWIW, now that I read that comment again, I see that I meant that
gnewsense-artwork fails to build when the remove_package ubuntu-artwork
line precedes the ensure_updated ubuntu-artwork ... line.

     # Extra meta packages
    diff -ru a/gen-artwork b/gen-artwork
    --- a/gen-artwork   2009-05-29 16:21:46.000000000 -0400
    +++ b/gen-artwork   2009-08-20 00:46:36.000000000 -0400
    @@ -17,6 +17,7 @@
     #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
USA
     #

    +set -ex

    kk This seems an unneeded change.

Oops--left that in mistakenly--sorry.

     . config

    diff -ru a/gen-kernel b/gen-kernel
    --- a/gen-kernel    2009-08-21 10:58:27.000000000 -0400
    +++ b/gen-kernel    2009-08-28 12:47:01.000000000 -0400
    @@ -20,45 +20,131 @@

     . config

    -DEBLOB=$PWD/firmware/deblob-2.6.24.4.1
     rm -rf $WORKINGDIR
     mkdir -p $WORKINGDIR
     cd $WORKINGDIR

    -apt-get source linux-image-2.6.24-$KERNEL_VERSIONPART-generic
    -apt-get --yes build-dep linux-image-2.6.24-$KERNEL_VERSIONPART-generic
    +case $(echo $MIRRORDIST | tr A-Z a-z) in
    +    "hardy")
    +   DEBLOB=$PWD/firmware/deblob-2.6.24.4.1

    -function clean_kconfig {
    +   apt-get source linux-image-2.6.24-$KERNEL_VERSIONPART-generic
    +   apt-get --yes build-dep linux-image-2.6.24-$KERNEL_VERSIONPART-generic

    kk should probably try and use $KERNEL_RELEASE here to keep it generic. 
might lead to wrapping the code
    kk in a function, or moving some config details into config.auto

Right.

    +
    +   function clean_kconfig {

    kk (trim lots of indentation)

    +
    +    "jaunty")
    +   apt-get -d source 
linux-image-$KERNEL_RELEASE-$KERNEL_VERSIONPART-generic

    kk Is there a point in -d here?

Yes, indeed.  Normally, we do want to unpack and patch the source
package.  But the patching we do here, including possibly with the
linux-libre code, does not *directly* use an unpacked tarball.  Of
course, the .tar.gz file will be unnecessary if a linux-libre tarball
can simply be fetched--no harm done though by the apt-get command
otherwise getting the diff and dsc.

    +   apt-get --yes build-dep 
linux-image-$KERNEL_RELEASE-$KERNEL_VERSIONPART-generic
    +   apt-get --yes install wget

    kk wget not installed in the build chroots already? 
    kk You might want to add it to the chroot setup script instead of here

OK.

    +
    +   mkdir old
    +   if wget $LINUXLIBRE_TARBALL_URL
    +   then
    +       EXTRAVERSION=$(echo $LINUXLIBRE_TARBALL_URL | sed 
's/.*\(-libre[^.]*\).*/\1/')
    +   else
    +       echo "Note: Avoid the (repeated) lengthy building of 
linux-$KERNEL_RELEASE-libre.tar.gz here"
    +       echo "by generating the file yourself and installing it so that it 
can be served up"
    +       echo "as 
http://localhost/linux-libre/linux-$KERNEL_RELEASE-libre.tar.gz";

    kk This goes away when you fix your code, so thats "ok"

    +       gunzip < linux_$KERNEL_RELEASE.orig.tar.gz | bzip2 > 
linux-$KERNEL_RELEASE.tar.bz2
    +       mv linux_$KERNEL_RELEASE.orig.tar.gz old

    I'm not sure what the point of this 'old' directory is.

    +       wget http://localhost/linux-libre/deblob-{2.6.28,check,main}
    +       chmod u+x deblob*
    +       sed -i '/^xdelta delta/,$ d' deblob-main    # delete some unneeded 
commands
    +       gzip *tar
    +       EXTRAVERSION=$(sed -n "s/.*extra=\(.*\)/\1/p" 
deblob-$KERNEL_RELEASE)
    +       bash -x deblob-main $KERNEL_RELEASE $EXTRAVERSION
    +   fi
    +
    +   mv *libre*tar.gz linux_$KERNEL_RELEASE.orig.tar.gz

    kk Some of this stuff would go away if you would (say) put a deblobbed 
linux in the builder source tree.
    kk For example, under packages/<release>/ (in the correct format, obviously)

OK--I like it.

    +
    +   DIFF_FILE=$(basename *diff.gz .gz)
    +   mv *diff.gz old
    +   gunzip < old/*diff.gz > $DIFF_FILE.orig
    +
    +   # Use different patches for different Debian diff files or just use sed
    +   cat > patch_debian_diff.sed<<EOF

    kk Couldnt read this diff, will have to try again later.

It's ugly, but IMO it's far less tedious than managing different patches
for the various diff files that might be applicable here now (and in the
future perhaps?).

    +EOF
    +   sed -f patch_debian_diff.sed $DIFF_FILE.orig > $DIFF_FILE
    +   gzip $DIFF_FILE
    +
    +   TARFILE_INFO=$(find . -maxdepth 1 -name 
linux_$KERNEL_RELEASE.orig.tar.gz -printf "%s %f") 
    +   DIFF_INFO=$(find . -maxdepth 1 -name $DIFF_FILE.gz -printf "%s %f")
    +

     kk (trim)

     rename 's/linux/linux-nofirmware/' linux_*.orig.tar.gz

    @@ -68,10 +154,18 @@
     # Need to go to previous revision for ABI stuff - long way of saying 
second last word
     sed -i 's/^prev_revision :=.*$/prev_revision := $(word $(words $(wordlist 
2,$(words $(prev_revisions)),$(prev_revisions))),$(prev_revisions))/' 
debian/rules.d/0-common-vars.mk
     echo 'skipmodule = true' >> debian/rules.d/0-common-vars.mk # Having less 
modules is okay for us
    +echo 'skipabi = true' >> debian/rules.d/0-common-vars.mk
     sed -i '/\^-\[\^-\]/,/^fi/d' debian/scripts/abi-check      # Having less 
symbols is okay for us

    kk Does this add a new line even if it exists already?

With this line, my do-update log shows:

WW: 6 symbols changed hash and weren't ignored
II: Module hash change summary...
    drivers/media/dvb/frontends/tda1004x    : 2
    drivers/media/dvb/frontends/or51211     : 1
    drivers/media/dvb/frontends/sp887x      : 1
    drivers/media/dvb/frontends/sp8870      : 1
    drivers/media/dvb/frontends/bcm3510     : 1

Without this line, that WW warning above becomes a fatal error.

    +case $(echo $MIRRORDIST | tr A-Z a-z) in
    +    "hardy")
     # Xen hunk mismatches cause patches to *.orig not to apply
    -sed -i 's/\(patch -p1\)/\1 --no-backup-if-mismatch/' 
debian/rules.d/6-binary-custom.mk
    +   sed -i 's/\(patch -p1\)/\1 --no-backup-if-mismatch/' 
debian/rules.d/6-binary-custom.mk
    +   ;;
    +    *)
    +   echo Further cases here...
    +   ;;
    +esac

    kk An if statement looks like it would better fit here.

Probably.  I just felt at the time that there might be other special case
code that could neatly fit in there.

     echo | dch -D $RELEASE -v $(sed -n 
'1s#^.*(\(.*\)).*#\1'${DISTRONAME_L}${KERNEL_VERSION}'#p' debian/changelog)  
'Removed firmware'

    Only in b: gen-kernel-linux-libre

    kk Whats in this file? ( gen-kernel-linux-libre)

Sorry, I forgot to delete that obsolete file.

    diff -ru a/gen-launchpad-integration b/gen-launchpad-integration
    --- a/gen-launchpad-integration     2009-05-29 16:21:46.000000000 -0400
    +++ b/gen-launchpad-integration     2009-08-29 08:07:08.000000000 -0400
    @@ -34,4 +34,5 @@

     echo | dch -D $RELEASE -v $(sed  -n 
'1s#^.*(\(.*\)).*#\1'${DISTRONAME_L}${LAUNCHPAD_INTEGRATION_VERSION}'#p' 
debian/changelog)  "Changed to reflect $RELEASE"

    -dpkg-buildpackage $DPKGOPTS
    +# This script seems to fail when parallel tasks are enabled:
    +dpkg-buildpackage $DPKGOPTS -j1

    kk Sounds like a bug, you might want to report it upstream if its 
reproducable without running via builder

OK.

    --- a/gen-linux-ubuntu-modules      2009-05-29 16:21:46.000000000 -0400
    +++ b/gen-linux-ubuntu-modules      2009-08-25 12:57:07.000000000 -0400
    @@ -24,9 +24,9 @@

     function clean_kconfig {
            #$1 = filename $2 = things to remove

    kk Have you verified the freedom of this package? or is that a TODO?
    kk I'm also wondering if these modules will work with the l-l kernel, since 
its somewhat different to that shipped by Ubuntu

That's a TODO.  I am afraid I know little so far about the module
vs. l-l kernel issue.

    diff -ru a/gen-livecd b/gen-livecd
    --- a/gen-livecd    2009-08-07 10:57:21.000000000 -0400
    +++ b/gen-livecd    2009-08-19 21:36:30.000000000 -0400
    @@ -66,6 +66,13 @@
     Config: $RELEASE
     EOF

    +# In Jaunty, this script fails, complaining about missing update-rc.d,
    +# which is found in the sysv-rc package.  The following edits are a 
workaround,
    +# forcing sysv-rc to be installed early.
    +
    +sed -i -e '1,/Packages: *apt/ s/Packages: *apt/Packages:\n apt\n sysv-rc/' 
cdebootstrap/generic/packages
    +sed -i -e '1,/What: *dpkg/ s/\(Action: *dpkg-install\)/\1\nWhat: 
sysv-rc\n\n\1/' cdebootstrap/generic/action 
    +

    kk Does 'this script' mean gen-livecd or chroots() ?
    kk If it means gen-livecd a simple `chroots apt-get install sysv-rc` might 
be enough to fix it

this script = gen-livecd, and, yes, chroots method looks good.

     chroots ()
     {
            # Execute commands in chroot
    @@ -73,7 +80,8 @@
            PATH=/usr/sbin:/usr/bin:/sbin:/bin DEBIAN_PRIORITY=critical ${1}
     }
     cp -r cdebootstrap/generic/ cdebootstrap/$RELEASE
    -cdebootstrap -v --allow-unauthenticated --keyring= --arch=$ARCHTOBUILD 
--flavour=minimal -c cdebootstrap/ $RELEASE chroot-$ARCHTOBUILD $REPOAPT
    +
    +cdebootstrap --debug -v --allow-unauthenticated --keyring= 
--arch=$ARCHTOBUILD --flavour=minimal -c cdebootstrap/ $RELEASE 
chroot-$ARCHTOBUILD $REPOAPT

    kk Don't need --debug in general.

Oops--left it in.

     # Now have full chroot

     mkdir -p cdroot-$ARCHTOBUILD/image/casper/

    diff -ru a/gen-ubiquity b/gen-ubiquity
    --- a/gen-ubiquity  2009-05-29 16:21:46.000000000 -0400
    +++ b/gen-ubiquity  2009-08-29 14:23:37.000000000 -0400
    @@ -97,7 +97,35 @@

     cd d-i/source/apt-setup/
     # Ignore these for now 
    -echo -e '#!/bin/sh\ncat $1 >> $2' > apt-setup-verify
    +#echo -e '#!/bin/sh\ncat $1 >> $2' > apt-setup-verify
    +# Handle variable number of args to newest versions of apt-setup-verify
    +
    +cat <<EOFXW > apt-setup-verify
    +#!/bin/bash
    +set -e
    +
    +case "\$#" in
    +
    +    "2" )
    +   ;;
    +    "3" )
    +   if [ "\$1" = "--invalid" ]; then
    +       shift
    +   else
    +       exit 1
    +   fi
    +   ;;
    +    "6" )
    +   shift 4
    +   ;;
    +    *   )
    +   exit 1
    +   ;;
    +esac
    +
    +cat \$1 >> \$2
    +EOFXW
    +

     cat <<EOFXX > generators/50mirror.$DISTRONAME_L
     #!/bin/bash
    diff -ru a/gen-yelp b/gen-yelp
    --- a/gen-yelp      2009-05-29 16:21:46.000000000 -0400
    +++ b/gen-yelp      2009-08-11 08:34:57.000000000 -0400
    @@ -26,9 +26,18 @@

     apt-get source yelp$VERSION
     apt-get --yes build-dep yelp$VERSION
    -cd yelp*
    +cd yelp*/debian/patches

    -rm 
debian/patches/{01_lpi,04_new_ubuntu_layout,05_menu_tooltip,07_rosetta_translations_update,99_change_help_URL}.patch
    +if [ -f 99_change_help_URL.patch ]; then     # found in hardy
    +    PATCHES='01_lpi.patch 04_new_ubuntu_layout.patch 05_menu_tooltip.patch 
07_rosetta_translations_update.patch 99_change_help_URL.patch'
    +elif [ -f 06_ubuntu_online_url.patch ]; then     # found in jaunty
    +    PATCHES='01_lpi.patch 04_new_ubuntu_layout.patch 05_menu_tooltip.patch 
06_ubuntu_online_url.patch 07_rosetta_translations_update.patch'
    +    PATCHGROUP="\($(echo $PATCHES | sed 's/ /\\|/g')\)"
    +    sed -i -e "/[^#]*$PATCHGROUP/d" series     # see quilt(1)
    +fi
    +rm $PATCHES
    +
    +cd ../..

    kk Personally I'd prefer something along the lines of
    kk if debian/patches/example.patch
    kk  rm debian/patches/{name,name,name}.patch
    kk elif 2nd-example.patch
    kk  rm debian/patches/{name,name,name}.patch
    kk fi
    kk Which is neater, saves us changing our directory, and setting up lots of 
variables

Right, but note that we still need a way to tell sed what patch file
names to delete from the series file that quilt uses.  Hence my use of
variables.

     echo | dch -D $RELEASE -v $(sed  -n 
'1s#^.*(\(.*\)).*#\1'${DISTRONAME_L}${YELP_VERSION}'#p' debian/changelog)  
"Removed Ubuntu-specifc patches"

    diff -ru a/packages/deltah/gnewsense-theme/debian/control 
b/packages/deltah/gnewsense-theme/debian/control
    --- a/packages/deltah/gnewsense-theme/debian/control        2009-05-29 
16:21:39.000000000 -0400
    +++ b/packages/deltah/gnewsense-theme/debian/control        2009-08-16 
07:42:43.000000000 -0400
    @@ -11,7 +11,7 @@
     Conflicts: human-cursors-theme (<= 0.5)
     Replaces: 
     Provides: gnewsense-gtk-theme
    -Depends: dmz-cursor-theme, human-icon-theme, gtk2-engines-ubuntulooks, 
gtk2-engines-murrine, human-theme
    +Depends: dmz-cursor-theme, human-icon-theme, gtk2-engines-murrine, 
human-theme

    kk no g-e-m? Has it been removed, or is unneeded, or...?

You mean, no g-e-u, which is unneeded (as a dependency for human-theme).
It also caused a conflict error in my attempts to build with Jaunty.

     Description: gNewSense theme
      The default gNewSense theme. At the moment the package contains
       - the theme definitions.
    diff -ru a/packages/deltah/usplash-theme-gnewsense/Makefile 
b/packages/deltah/usplash-theme-gnewsense/Makefile
    --- a/packages/deltah/usplash-theme-gnewsense/Makefile      2009-05-29 
16:21:39.000000000 -0400
    +++ b/packages/deltah/usplash-theme-gnewsense/Makefile      2009-08-14 
07:11:44.000000000 -0400
    @@ -8,10 +8,14 @@
     INSTALL_DATA = $(INSTALL) -m 644
     INSTALL_PROGRAM = $(INSTALL) -m 755

    -CONVERT=convert
    +CONVERT:=convert
    +# As of version 6.4.2-9 of convert: new dithering selection option,
    +# -dither Floyd-Steinberg or -dither Riemersma.
    +CONVERT_VER := $(shell $(CONVERT) -version | sed -n 
's/[Vv]ersion:[^0-9]*\([0-9.]*\).*/\1/p')
    +DITHER_FLAG := $(shell dpkg --compare-versions "$(CONVERT_VER)" lt 
"6.4.2-9" && echo '-dither' || echo '-dither Floyd-Steinberg')

    kk What does this new dithering option give us?

It's just a syntax requirement of newer convert programs.  As of version
6.4.2-9 of convert, a simple, naked -dither flag cannot be used as it is
now, so I just used it with "Floyd-Steinberg", which is the same method
used by the older -dither usage.