Re: Patch for simple-cdd and debian-cd 3.0

[vagrant@freegeek.org]

On Mon, Feb 26, 2007 at 12:30:10PM -0300, Gustavo Franco wrote:
> On 2/21/07, Raphael Hertzog <hertzog@debian.org> wrote:
> >I just fixed simple-cdd to work with the latest debian-cd.
> 
> It was building only a minimal iso without adding the simple-cdd.task
> that points out what you add on the profile(s).
> 
> >In reality it required some fixes to the debian-cd tree as well, so you
> >should use it in combination with the debian-cd (see debian_cd_dir in
> >simple-cdd.conf) from the svn trunk.
> 
> I see, thanks.
> 
> >If you want to use debian-cd 3.0.0 instead of the svn trunk, you have to
> >add "$debian_cd_dir/tools" in the PATH before calling the various
> >debian-cd targets and you have to manually call the mirrorcheck target
> >before "make images".
> 
> I've added this into build-simple-cdd but using a simpler solution to
> fetch (and set) the ARCH.

i think this should maybe still go in simple-cdd.conf rather than
build-simple-cdd, so that it's possible to over-write it without setting
environment variables ...
 
> >Please apply my patch.

> It needed some more work, read my bzr changelog[0].
...snip... 
> Please test using the "simple-cdd-stratus" branch[1]. Feedback is 
> appreciated.

here goes with the feedback :)

> ------------------------------------------------------------
> revno: 304
> committer: Gustavo Franco <stratus@debian.org>
> branch nick: simple-cdd-stratus
> timestamp: Mon 2007-02-26 11:51:03 -0300
> message:
>  - Add $debian_cd_dir/tools to the $PATH

>  - Remove --keyboard and --force-preseed options

why did you remove those options?

--force-preseed is needed to be able to preseed any values in the
debconf passwd db. i would like a more clean solution to this issue, but
for the moment ...

and i suspect --keyboard is useful as well to anyone making a CD for a
region that typically has a specific keyboard type ...

>  - Remove unneeded generate_di_list and generate_di+k_list related calls
>  - Fix make calls to work with debian-cd 3.0 (breaks debian-cd <= 3.0
> compatibility)
>    Some code merged from Raphael Hertzog patch, thanks!

looking good :)

>  - Comment out check packages call (temp).

why did you include that in the commit? is it actually broken?


also, some comments regarding the isolinux tweaking code in build-simple-cdd:

  if [ -f $simple_cdd_temp/cd-build/$CODENAME/boot1/isolinux/isolinux.cfg ]; then
    isolinuxcfg="$simple_cdd_temp/cd-build/$CODENAME/boot1/isolinux/isolinux.cfg"
  fi
  
  isolinuxcfg="$simple_cdd_temp/$CODENAME-$ARCH/boot1/isolinux/isolinux.cfg"
  if [ "true" = "$use_serial_console" ] && [ -n "$serial_console_speed" ];
  then
      echo "SERIAL 0 $serial_console_speed 0" >> $isolinuxcfg
  fi
  if [ -n "$BOOT_TIMEOUT" ]; then
      sed -ie "s,TIMEOUT.*,TIMEOUT $BOOT_TIMEOUT,g" $isolinuxcfg
  fi
  
so, it's setting the isolinuxcfg variable, and then hard-coding an
overwritten variable... i'm thinking it should be more like:

  isolinuxcfg=$simple_cdd_temp/foo/isolinux.cfg
  if [ -f "$isolinuxcfg" ]; then
     #additional if statements for serial console and BOOT_TIMEOUT
  fi
  
also, do we want $simple_cdd_temp/$CODENAME-$ARCH or
$simple_cdd_temp/cd-build/$CODENAME, or do we really want
$simple_cdd_temp/cd-build/$CODENAME-$ARCH ? it seems debian-cd 3.0.0
just uses $CODENAME, but does 3.0.1~svn use $CODENAME-$ARCH ?


we get all the above figured out, i'll be happy to merge :)

live well,
  vagrant