On Tue, 2013-05-28 at 00:34 -0400, Rick "Zero_Chaos" Farina wrote:
> It has come to light that this patch breaks binpkg use in stage1.
...
> Thanks,
> Zero
> 
> On 03/26/2013 11:09 PM, Jorge Manuel B. S. Vicetto (jmbsvicetto) wrote:
> > From: "Jorge Manuel B. S. Vicetto (jmbsvicetto)" <jmbsvicetto@gentoo.org>
> > 
> > +	elif [ -n "${clst_PKGCACHE}" -a -z "${clst_update_seed}" ]

As it turns out, this one change was incorrect to evaluate all possible
scenarios.  My bash skills are poor, I originally had used nested if's ,
but that got optimized by others to the above line.

That line should be changed to:

elif [ -n "${clst_PKGCACHE}" ] && ( [ -z "${clst_update_seed}" ] || [ "${clst_update_seed}" == "no" ]

Which breaks down to (for those that can't follow it's logic):

# NOTE: clst_update_seed is currently only used for a stage1, so does not exist in the environment for other targets.
# spacing was added to the line so it lines up with the plain English text.

# if clst_PKGCACHE is not null   and  either clst_update_seed doesn't exist  or clst_update_seed is exactly equal to "no"
#if [ -n "${clst_PKGCACHE}" ]    &&  (        [ -z "${clst_update_seed}" ]   ||   [ "${clst_update_seed}" == "no" ] )


I have 3 reasons for wanting this change done this way

     1.  If a spec or config enables the PKGCACHE option.  It will add
        all the binpkg options to the emerge command.  Due to the
        possible problem with using/creating binpkgs during the
        update_seed process. These options should not be set.  Currently
        they are being set, but only --binpkg was being reset to =n in
        the stage1-chroot.sh update_seed command.  The other binpkg
        options were not being removed or reset.  This is poor coding.
        It is far better to just not add the unwanted options in the
        first place.
     2. While it is possible to work around not having the logic in
        setup_myemergeoptions(), it is hacky and complicates the
        stage1-chroot code more than neccessary.  It is also poor
        programming style.  In total it would mean the
        setup_myemergeoptions() would be called three times in total for
        the stage1 run.  Or temporarily saving the emerge options,
        unsetting clst_PKGCACHE, re-running setup_myemergeoptions() to
        get the correct options for update_seed, then later restoring
        the the two variables correctly. Both these coding options are
        poor programming style and more prone to bugs.  With the
        proposed change above it would be run twice.  Once keeping the
        PKGCACHE options off for the update_seed run, then reset
        according to the spec for the target emerge run.
     3. Placing the clst_update_seed logic to toggle the PKGCACHE
        options in the setup_myemergeoptions() simplifies the stage1
        code.  Plus it leaves the option to run an update_seed
        option/command for any future or current target without
        complicating/duplicating code.

I have attached a small test program I used to come up with the
correctly working patch change above.  It can be used to evaluate how
the line's logic is processed.  I have also attached the terminal output
it produced for all the possible conditions that could occur.  

Also I have slightly different PKGCACHE options in my rewrite branch.  I
have added --binpkg-respect-use to them.  It was brought to my attention
early in testing to put them in a config to eliminate the problem of
using binpkgs that were not compiled with them set correctly.

I will be pushing an updated rewrite branch with this and the other
changes made to master soon. So "git pull --force" to update your
checkout with the rewritten commits when I do.
-- 
Brian Dolbec <dolsen@gentoo.org>