Re: Updated non-US.diff

[Raphael Hertzog <rhertzog@hrnet.fr>]

Le Wed, Jun 14, 2000 at 08:25:45AM +0100, Steve McIntyre écrivait:
> OK, I believe I've covered all of Raphael's changes. This one now
> works fine with !complete.

Ok, you may commit it. I've seen that there's no problem with the
generation of Debian_$CODENAME_nonUS since it's generated in $BDIR and
not in the tasks dir.

I just have 2 remarks :
- I can't generate only one (US or non-US) first CD with
  FORCENONUSONCD1... but that's not a big deal since we don't gain much
  by beeing able to do this ...
- this code may need a little tweak :

> +    foreach (@nonuslist) {
> +        if($complete or $includesrc{$_}) {
> +	  msg(1, "Asked to add non-US source $_ to source CD#1\n");
> +	  $src = $_;
> +        }
> +        if (not exists $included{$src}) {
> +	  msg(0, "ERROR: Non-US source `$src' does not exist ... (ignored)\n");
> +	  next;
> +        }
> +        if($excluded{$src}) {
> +	  msg(1, "...but not doing so - $excluded{$src}\n");
> +	  next;
> +        }
> +        next if $included{$src};
> +        add_src ($src);
> +    }

I'd prefer that you use "foreach $src (@nonuslist)" directly instead of
doing a conditionnal assignement, and you'd better do a next directly if
the package don't have to be included. Otherwise if the assignment is not
done, $src has still the old value and some code maybe executed twice (of
course the included check will fail but that's not always self-evident for
someone reviewing the code).

Something like that should do it (I hope, i'm a bit tired right now ...) :
foreach $src (@nonuslist)
{
	if (! ($complete or $includesrc{$src}))
	{
	  msg(1, "Non-US source package $src doesn't need to be included !\n");
	  next;
	}
        msg(1, "Asked to add non-US source $src to source CD#1\n");
        if (not exists $included{$src}) {
	  msg(0, "ERROR: Non-US source `$src' does not exist ... (ignored)\n");
	  next;
        }
        if ($excluded{$src}) {
	  msg(1, "...but not doing so - $excluded{$src}\n");
	  next;
        }
        next if $included{$src};
        add_src ($src);
}


Apart that everything looks ok, feel free to commit it with this litlle
change. And just add a line about it in debian/changelog (and also update
debian/CONF.sh).

Cheers,
-- 
Raphaël Hertzog -+- http://strasbourg.linuxfr.org/~raphael/