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

Re: Updated non-free patch



On Tue, Jun 06, 2000 at 12:10:48PM +0100, Raphael Hertzog wrote:
>
>Le Tue, Jun 06, 2000 at 08:27:56AM +0100, Steve McIntyre écrivait:
>> Much cleaned up and no longer causes a significant slow-down because
>> I'm using a better algorithm. Source is now done too, and the
>> "EXTRANONFREE" option is plumbed into CONF.sh.
>>
>> If nobody objects strenuously I'd like to put this into CVS tonight.
>
>I'd prefer doing it myself since I may want to change some things. I'm a
>litlle concerned by changing this that late because if we change debian-cd
>that much we'll no longer be able to produce the same CD set that has been
>produced for test-cycle 1 & 2 (has they been already generated BTW ?)...

I'm not sure. Phil?

>Anyway I'll look into it ASAP.
>
>>       foreach $re (qw/Binary Version Section Directory/) {
>> -             (m/^$re: (.*?)\s*$/m and $sources{$p}{$re} = $1)
>> -             || msg(1, "Header field '$re' missing for source '$p'\n");
>> +         (m/^$re: (.*?)\s*$/m and $sources{$p}{$re} = $1)
>> +             || msg(1, "Header field '$re' missing for source '$p'\n");
>> +         if (m/\/non-free\//mgc) {
>> +             $sources{$p}{"nonfree"} = 1;
>> +         }
>> +         if (m/\/non-US\//mgc) {
>> +             $sources{$p}{"nonus"} = 1;
>> +         }
>>       }
>
>You may want to test this only for the Directory (or Section ?) header 
>...no ?

Fine. I was trying to minimise changes to the code, at the cost
(probably) of some speed - I'm _not_ a perl hacker and some of the
changes I've put in needed a lot of trial and error.

>>       if (not @{$sources{$p}{"Files"}}) {
>> -             msg(0, "ERROR: Source package $p has no files ...\n");
>> +         msg(0, "ERROR: Source package $p has no files ...\n");
>>       }
>
>Looks like a useless change. :-)

*grin* Yes...

>>  }
>>
>> @@ -150,6 +160,23 @@
>>
>>  }
>>
>> +# And now add the non-free sources to separate non-free CDs
>> +if ($nonfree and $extranonfree) {
>> +    $doing_nonfree=1;
>> +    $cd++;
>> +    $cd_size = 0;
>> +    # New limit
>> +    $limit = $ENV{"SRCSIZELIMIT$cd"} || $deflimit;
>> +    msg(0, "Limit for non-free CD $cd is $limit.\n");
>> +    foreach $p (sort { ($sources{$a}{"Section"} cmp 
>$sources{$b}{"Section"})
>> +                       || ($a cmp $b) }
>> +                grep { not $included{$_} } keys %sources)
>> +    {
>> +     add_src ($p);
>> +    }
>> +}
>> +msg(0, "Non-free source CD $cd filled with $cd_size bytes ...\n");
>
>I wonder why you want to change add_src when the only thing you need to do
>whould be to call add_src just the for non-free packages with something 
>like :
>grep { (! $included{$_}) and $sources{$_}{"nonfree"} } keys %sources
>
>Of course that would require to let non-free out in the precedent run with
>a similar mechanism ...

OK, if you prefer. The reason I did it this way is to try and keep the
changes to list2cds and cds2src as similar as possible. What you're
suggesting here will work fine in list2cds, but not in cds2src because
of dependency-following. Which took me a long time to find! :-(

>> @@ -173,8 +200,12 @@
>>       $size = 0;
>>       msg(2, "+ Trying to add $src ...\n");
>>       foreach (@{$sources{$src}{"Files"}}) {
>> -             $size += $_->[1];
>> -             push @files, $_->[0];
>> +         if(!$doing_nonfree and $extranonfree and 
>$sources{$src}{"nonfree"}) {
>> +             msg(3, "Ignoring non-free source file $_->[0]\n");
>> +             return;
>> +         }
>> +         $size += $_->[1];
>> +         push @files, $_->[0];
>>       }
>
>Even if you keep this method, this test should probably go out of the
>foreach loop (i.e. before the loop). The test concerns the whole source
>package not only a precise file ...

Doh! Of course... Although it will only be very slightly slower this
way, it still is a little silly.

>> +             if (m/\/non-free\//mgc) {
>> +                 $packages{$p}{"nonfree"} = 1;
>> +             }
>> +             if (m/\/non-US\//mgc) {
>> +                 $packages{$p}{"nonus"} = 1;
>> +             }
>
>The same remarks applies for list2cds

OK.

>> +     foreach $q (@dep) {
>> +         if (!$doing_nonfree and $extranonfree and 
>$packages{$q}{"nonfree"}) {
>> +             msg(3, "Ignoring non-free reference to '$q'\n");
>> +         } else {
>> +             # All packages are ok, now check for the size issue
>> +             $size = get_size ([$q]);
>> +
>> +             # Creation of a new CD when needed
>> +             if ($cd_size + $size > $limit) {
>> +               msg(0, "CD $cd filled with $cd_size bytes ... ",
>> +                   "(limit was $limit)\n");
>> +               $cd++;
>> +               $cd_size = 0;
>> +               # New limit
>> +               $limit = $ENV{"SIZELIMIT$cd"} || $deflimit;
>> +               msg(2, "Limit for CD $cd is $limit.\n");
>> +             }
>> +
>> +             msg(2, "  \$cd_size = $cd_size, \$size = $size\n");
>> +
>> +             $cd_size += $size;
>> +             $total_size += $size;
>> +
>> +             add_to_cd ($cd, [$q]);
>> +             $included{$q} = $cd;
>> +         }
>
>This is a fairly large change that I'd probably not have written this way.

So what would you have done? I thought this was an obvious way to do
it, while minimising changes.

>Please don't commit anything yet... I'll look into it this evening.

OK, I'll wait for you.

-- 
Steve McIntyre, Allstor Software         smcintyr@allstor-sw.co.uk
<a href=http://www.rpg-soc.ucam.org/curs/>CURS home page</a>
"Can't keep my eyes from the circling sky,                 
"Tongue-tied & twisted, Just an earth-bound misfit, I..."  



Reply to: