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

Re: And here's the non-US diff



Le Sun, Jun 11, 2000 at 09:42:17PM +0100, Steve McIntyre écrivait:
> I don't know how to get CVS diff to do the "new files" bit, so 4
> attachments: the diff and the 3 new files:

Ok, I have some comments/remarks and some modifications.

> @@ -229,14 +242,42 @@
>  $(BDIR)/1.packages:
>  	@echo "Dispatching the packages on all the CDs ..."
>  	@$(list2cds) $(BDIR)/list $(SIZELIMIT)
> -	
> +ifdef FORCENONUSONCD1
> +	@for file in $(BDIR)/*.packages; do \
> +	    newfile=$${file%%.packages}; \
> +	    newfile=$${newfile##$(BDIR)/.packages}_NONUS.packages; \

This line looks strange for me... didn't you mean this :
newfile=$${newfile}_NONUS.packages;

I guess that the substitution does nothing since $newfile doesn't begin
with "$(BDIR)/.packages" ...

If I'm right then you can merge both lines :
newfile=$${file%%.packages}_NONUS.packages

> +	    newfile=$${file%%.sources}; \
> +	    newfile=$${newfile##$(BDIR)/.sources}_NONUS.sources; \

Again, same problem but for sources.

> @@ -606,14 +630,14 @@
>  bin-image: ok bin-md5list $(OUT)
>  	@echo "Generating the binary iso image n°$(CD) ..."
>  	@test -n "$(CD)" || (echo "Give me a CD=<num> parameter !" && false)
> -	@dir=$(BDIR)/$(CD); cd $(BDIR); opts=`cat $$dir.mkisofs_opts`; \
> +	@dir=$(BDIR)/CD$(CD); cd $(BDIR); opts=`cat $$dir.mkisofs_opts`; \
>  	 volid=`cat $$dir.volid`; rm -f $(OUT)/$(CODENAME)-$(ARCH)-$(CD).raw; \
>  	  $(MKISOFS) $(MKISOFS_OPTS) -V "$$volid" \
>  	  -o $(OUT)/$(CODENAME)-$(ARCH)-$(CD).raw $$opts $(CD)

I'm pretty sure this change is not correct, before you used
$(BDIR)/$$n.mkisofs_opts and here you're using $(BDIR)/CD$$n.mkisofs_opts !

I guess that using opts=`cat $(CD).mkisofs_opts` and 
volid=`cat $(CD).volid` should be ok. I don't see the need for $dir ...

>  src-image: ok src-md5list $(OUT)
>  	@echo "Generating the source iso image n°$(CD) ..."
>  	@test -n "$(CD)" || (echo "Give me a CD=<num> parameter !" && false)
> -	@dir=$(SDIR)/$(CD); cd $(SDIR); opts=`cat $$dir.mkisofs_opts`; \
> +	@dir=$(SDIR)/CD$(CD); cd $(SDIR); opts=`cat $$dir.mkisofs_opts`; \
>  	 volid=`cat $$dir.volid`; rm -f $(OUT)/$(CODENAME)-src-$(CD).raw; \
>           $(MKISOFS) $(MKISOFS_OPTS) -V "$$volid" \
>  	  -o $(OUT)/$(CODENAME)-src-$(CD).raw $$opts $(CD)

Same problem here.

> +#if (FORCENONUSONCD1 == 1)
> +#include "Debian_potato_nonUS"
> +#endif

With this system, we mustn't forget about updating this file ...

> +# Add non-US stuff to CD#1 first if we have been asked to...
> +# "complete" means also add non-US sources that don't have binary packages...
> +if($forcenonusoncd1) {
> +    foreach $p (@nonuslist) {
> +        if($complete) {
> +	  $src = $p;
> +        } else {
> +	  $src = $bin2src{$p};
> +        }
> +        if (not exists $included{$src}) {
> +	  msg(0, "ERROR: Non-US source `$src' does not exist ... (ignored)\n");
> +	  next;
> +        }
> +        next if $excluded{$src};
> +        next if $included{$src};
> +        add_src ($src);
> +    }
> +}

Here you have misunderstood how it works. In @nonuslist you have a
list of ALL the non-US source packages. So it's an error to write
something like $src = $bin2src{$p} since $p is not the name of a binary
package !  %bin2src has the names of binary packages as keys and return the
name of the corresponding source package as value.

So if $complete is not set you must only include the source packages that
have some of their binary packages included on the binary CD set :
This may (check it!) give the good list of source package to include :
map { $bin2src{$_} } (grep { grep (/^$bin2src{$_}$/, @nonuslist) } @list);

Otherwise ($complete is set) you must include them all (all that are
listed in @nonuslist). In any case, take care, some packages of non-US are
in non-free and you may not want them (this should be already ok since
those packages are listed in excluded IIRC).

> @@ -9,8 +9,10 @@
>  
>  #set -e
>  
> -PREFIX=$2
> +PREFIX=`echo $2 | sed 's/CD//'`

What happens if I build my CDs in /home/CD/debian/tmp/ ?
This substiution should be more "specific"... like
s/$(BDIR)\/CD/$(BDIR)\//

(warning: $(BDIR) is not yet defined in this file)

> +
>  NUM=${PREFIX##$TDIR/$CODENAME-$ARCH/}
> +NUM=`echo $NUM | sed 's/_NONUS//g'`
>  if [ -n "$NONFREE" -o -n "$EXTRANONFREE" ]; then
>    SECTIONS="main contrib non-free"
>  else
> @@ -42,7 +44,7 @@
>  	fi
>  	# Install the Packages.cd and Packages.cd.gz files
>  	# Each CD know about all prior CDs
> -	for i in $TDIR/$CODENAME-$ARCH/*.packages; do
> +	for i in $TDIR/$CODENAME-$ARCH/?.packages; do
>  		dir=${i%%.packages}
>  		n=${dir##$TDIR/$CODENAME-$ARCH/}
>  		if [ $n -le $NUM ]; then

Two lines of explication wouldn't hurt. I had to read it 3 times until I
understood everything here ... you're "masquerading" CD1_NONUS as CD1 and
you're using us-safe Packages files to generate the Packages.cd file (as you
announced but it is not self-evident here in this file ...)

[ modifications s in boot-* ]

I wonder why you changed all cp to cp -f but it's not a big deal.

> open(AVAIL, "$apt cache dumpavail |") || die "Can't fork : $!\n";

This will be called one time pro CD. We may be able to optimise this ...
put a non-US.list in the temp directory and use that if it exists ...

> my ($p, $re);
> while (defined($_=<AVAIL>)) {
>     next if not m/^Package: (\S+)\s*$/m;
>     $p = $1;
>     $packages{$p}{"Package"} = $p;
>     foreach $re (qw(Section)) {
>         (m/^$re: (\S+)\s*$/m and $packages{$p}{$re} = $1)
> 	  || msg(1, "Header field '$re' missing for package '$p'.\n");
>         if (m/\/non-US\//mgc) {
> 	  $packages{$p}{"nonus"} = 1;
>         }
>     }
> }

Most of this is totally useles in this script, this one should be enough :
 while (defined($_=<AVAIL>)) {
     next if not m/^Package: (\S+)\s*$/m;
     $p = $1;
     if (m/^(Section|Filename): \S*non-US\S*\s*$/m)
     {
     	$packages{$p}{"nonus"} = 1;
     } else {
     	$packages{$p}{"nonus"} = 0;
     }
 }


Ok, that's it, otherwise it should be ok I guess ... however i've not checked
if you updated everything that had to be updated with the major changes
you did ... don't commit until I can review the corrected version
you'll provide me.

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



Reply to: