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)" <[email protected]> > > > > + 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 <[email protected]>
tif
Description: application/shellscript
brian@big_daddy ~/Dev/git/catalyst $ ./tif "1" YAY, PKGCACHE selected brian@big_daddy ~/Dev/git/catalyst $ ./tif "1" no clst_update_seed = no YAY, PKGCACHE selected brian@big_daddy ~/Dev/git/catalyst $ ./tif "" no clst_update_seed = no NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ ./tif "" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="" ./tif "" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="" ./tif "1" YAY, PKGCACHE selected brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="yes" ./tif "1" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="maybe" ./tif "1" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="maybe" ./tif "1" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="" ./tif "" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ ./tif "1" YAY, PKGCACHE selected brian@big_daddy ~/Dev/git/catalyst $ ./tif "1" no setting clst_update_seed = no YAY, PKGCACHE selected brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="no" ./tif "" NOPE, PKGCACHE disabled brian@big_daddy ~/Dev/git/catalyst $ clst_update_seed="no" ./tif "1" YAY, PKGCACHE selected brian@big_daddy ~/Dev/git/catalyst $
