[gentoo-catalyst] [PATCH] Fix broken seed stage update...

[gentoo-catalyst] [PATCH] Fix broken seed stage update...

[Brian Dolbec]

Strip --usepkg and --buildpkg from emerge options for user defined update_seed_command.
Add a check for the update_seed option to set the correct update options.
Fix default seed stage update command to properly update gcc and it's deps.
Add a seed stage update system command and option.
Add --binpkg-respect-use=y for all cases --usepkg is enabled.

Signed-off-by: Brian Dolbec <dolsen@gentoo.org>
---
 catalyst/targets/stage1.py          |  3 ++-
 doc/catalyst-spec.5.txt             | 12 +++++++++++-
 targets/stage1/stage1-chroot.sh     | 17 ++++++++++++++---
 targets/support/chroot-functions.sh | 11 ++++++++++-
 4 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/catalyst/targets/stage1.py b/catalyst/targets/stage1.py
index e936929..e067c8c 100644
--- a/catalyst/targets/stage1.py
+++ b/catalyst/targets/stage1.py
@@ -18,7 +18,8 @@ class stage1(StageBase):
 	def __init__(self,spec,addlargs):
 		self.required_values=[]
 		self.valid_values=["chost"]
-		self.valid_values.extend(["update_seed","update_seed_command"])
+		self.valid_values.extend(["update_seed","update_seed_command",
+			"update_seed_system"])
 		StageBase.__init__(self,spec,addlargs)
 
 	def set_stage_path(self):
diff --git a/doc/catalyst-spec.5.txt b/doc/catalyst-spec.5.txt
index 4a6e06c..196bdc3 100644
--- a/doc/catalyst-spec.5.txt
+++ b/doc/catalyst-spec.5.txt
@@ -138,9 +138,19 @@ it should update the seed stage or not (valid values: `yes no`).
 *update_seed_command*::
 This is an optional command to pass to emerge for updating the seed
 stage (example: `--update dev-libs/mpfr dev-libs/mpc dev-libs/gmp`)
-If not specified, catalyst will update gcc deps.
+If not specified, catalyst will update gcc's deps, and rebuild gcc if any of
+it's deps are updated with a new version. Even if it itself is not updated.
+This prevents gcc breakage when it's dependency lib sonames have changed.
 This setting requires enabling update_seed.
 
+*update_seed_system*::
+This is an optional setting supported by stage1 to tell catalyst if
+it should update the seed's system packages or not (valid values: `yes no`).
+This is run after any update_seed_command, updating any remaining upgradable
+system packages.
+This setting requires enabling update_seed.
+
+
 Compilation
 ~~~~~~~~~~~
 
diff --git a/targets/stage1/stage1-chroot.sh b/targets/stage1/stage1-chroot.sh
index 97aef7f..65c2d81 100755
--- a/targets/stage1/stage1-chroot.sh
+++ b/targets/stage1/stage1-chroot.sh
@@ -26,12 +26,23 @@ clst_root_path=/ setup_pkgmgr
 # Update stage3
 if [ -n "${clst_update_seed}" ]; then
 	if [ "${clst_update_seed}" == "yes" ]; then
-		echo "Updating seed stage..."
 		if [ -n "${clst_update_seed_command}" ]; then
-			clst_root_path=/ run_merge "--buildpkg=n ${clst_update_seed_command}"
+			echo "--- Updating seed stage with USER defined update_seed_command"
+			update_cmd=${clst_update_seed_command/--usepkg /}
+			update_cmd=${clst_update_seed_command/--buildpkg /}
+			clst_root_path=/ run_merge "${update_cmd}"
 		else
-			clst_root_path=/ run_merge "--buildpkg=n --update --deep --newuse --onlydeps gcc"
+			echo "--- Updating seed stage with DEFAULT update_seed_command"
+			update_cmd="--update --deep --complete-graph --rebuild-if-new-ver gcc"
+			clst_root_path=/ run_merge ${update_cmd}
 		fi
+		if [ "${clst_update_seed_system}" == "yes" ]; then
+			echo "--- Updating seed stage system packages"
+			update_cmd="--update --deep --complete-graph @system"
+			clst_root_path=/ run_merge ${update_cmd}
+		fi
+		# now reset the emerge options for the target
+		clst_update_seed=no setup_myemergeopts
 	elif [ "${clst_update_seed}" != "no" ]; then
 		echo "Invalid setting for update_seed: ${clst_update_seed}"
 		exit 1
diff --git a/targets/support/chroot-functions.sh b/targets/support/chroot-functions.sh
index 2524b4f..69d2923 100755
--- a/targets/support/chroot-functions.sh
+++ b/targets/support/chroot-functions.sh
@@ -133,9 +133,18 @@ setup_myemergeopts(){
 	then
 		export bootstrap_opts="${bootstrap_opts} -f"
 		export clst_myemergeopts="${clst_myemergeopts} -f"
+	# now intercept normal target options if we're updating the seed
+	# to update the seed we do not want binpkgs that may have links to
+	# sonames no longer installed, due to dependency updates.
+	# this function will be re-run later with clst_update_seed=no
+	elif [ "${clst_update_seed}" == "yes" ]
+	then
+		export clst_myemergeopts="${clst_myemergeopts} --newuse"
+		export bootstrap_opts="${bootstrap_opts} -r"
 	elif [ -n "${clst_PKGCACHE}" ]
 	then
-		export clst_myemergeopts="${clst_myemergeopts} --usepkg --buildpkg --newuse"
+		# if you add --usepkg, then also add --binpkg-respect-use=y
+		export clst_myemergeopts="${clst_myemergeopts} --usepkg --binpkg-respect-use=y --buildpkg --newuse"
 		export bootstrap_opts="${bootstrap_opts} -r"
 	fi
 }
-- 
1.8.1.2

[gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

The git branch is located at http://dev.gentoo.org/~dolsen/catalyst/
git checkout the rewrite branch.

For those of you that have already cloned it, use --force in your pull.
I've condensed, rearranged the changes into more logical complete
changes.  Still, I wouldn't think you could cherrypick any single commit
in general.  Some fixes could be cherrypicked to apply to current
master, some would likely have to be hand applied due to other changes.

Next on my todo list, fix doc's creation, create a setup.py and make the
code installable via ebuild.  

I think also development should continue in a branch on the main
catalyst repo on g.o.g.o.  Possibly name it catalyst3b so it does not
conflict with the catalyst3 branch started.  I looked at rebasing my
work on it, but decided against it.  There were far too many changes in
master since it was last updated.

There are far too many patches to individually list them in this mail
list.  Please checkout the branch from my repo to review the changes.
It would also be easier to use gitweb to view them online if it was
pushed to the main repo.


Anyway the latest patch...

======================================================================

Fix broken seed stage update...

Strip --usepkg and --buildpkg from emerge options for user defined
update_seed_command.
Add a check for the update_seed option to set the correct update
options.
Fix default seed stage update command to properly update gcc and it's
deps.
Add a seed stage update system command and option.
Add --binpkg-respect-use=y for all cases --usepkg is enabled.


+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

From 9ffa5b8812403bf20f17eba58543fc4b7c04bc33 Mon Sep 17 00:00:00 2001
From: Brian Dolbec <dolsen@gentoo.org>
Date: Mon, 25 Feb 2013 23:31:41 -0800
Subject: [PATCH] Fix broken seed stage update...

Strip --usepkg and --buildpkg from emerge options for user defined update_seed_command.
Add a check for the update_seed option to set the correct update options.
Fix default seed stage update command to properly update gcc and it's deps.
Add a seed stage update system command and option.
Add --binpkg-respect-use=y for all cases --usepkg is enabled.
---
 catalyst/targets/stage1.py          |  3 ++-
 doc/catalyst-spec.5.txt             | 12 +++++++++++-
 targets/stage1/stage1-chroot.sh     | 17 ++++++++++++++---
 targets/support/chroot-functions.sh | 11 ++++++++++-
 4 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/catalyst/targets/stage1.py b/catalyst/targets/stage1.py
index e936929..e067c8c 100644
--- a/catalyst/targets/stage1.py
+++ b/catalyst/targets/stage1.py
@@ -18,7 +18,8 @@ class stage1(StageBase):
 	def __init__(self,spec,addlargs):
 		self.required_values=[]
 		self.valid_values=["chost"]
-		self.valid_values.extend(["update_seed","update_seed_command"])
+		self.valid_values.extend(["update_seed","update_seed_command",
+			"update_seed_system"])
 		StageBase.__init__(self,spec,addlargs)
 
 	def set_stage_path(self):
diff --git a/doc/catalyst-spec.5.txt b/doc/catalyst-spec.5.txt
index 4a6e06c..196bdc3 100644
--- a/doc/catalyst-spec.5.txt
+++ b/doc/catalyst-spec.5.txt
@@ -138,9 +138,19 @@ it should update the seed stage or not (valid values: `yes no`).
 *update_seed_command*::
 This is an optional command to pass to emerge for updating the seed
 stage (example: `--update dev-libs/mpfr dev-libs/mpc dev-libs/gmp`)
-If not specified, catalyst will update gcc deps.
+If not specified, catalyst will update gcc's deps, and rebuild gcc if any of
+it's deps are updated with a new version. Even if it itself is not updated.
+This prevents gcc breakage when it's dependency lib sonames have changed.
 This setting requires enabling update_seed.
 
+*update_seed_system*::
+This is an optional setting supported by stage1 to tell catalyst if
+it should update the seed's system packages or not (valid values: `yes no`).
+This is run after any update_seed_command, updating any remaining upgradable
+system packages.
+This setting requires enabling update_seed.
+
+
 Compilation
 ~~~~~~~~~~~
 
diff --git a/targets/stage1/stage1-chroot.sh b/targets/stage1/stage1-chroot.sh
index 97aef7f..65c2d81 100755
--- a/targets/stage1/stage1-chroot.sh
+++ b/targets/stage1/stage1-chroot.sh
@@ -26,12 +26,23 @@ clst_root_path=/ setup_pkgmgr
 # Update stage3
 if [ -n "${clst_update_seed}" ]; then
 	if [ "${clst_update_seed}" == "yes" ]; then
-		echo "Updating seed stage..."
 		if [ -n "${clst_update_seed_command}" ]; then
-			clst_root_path=/ run_merge "--buildpkg=n ${clst_update_seed_command}"
+			echo "--- Updating seed stage with USER defined update_seed_command"
+			update_cmd=${clst_update_seed_command/--usepkg /}
+			update_cmd=${clst_update_seed_command/--buildpkg /}
+			clst_root_path=/ run_merge "${update_cmd}"
 		else
-			clst_root_path=/ run_merge "--buildpkg=n --update --deep --newuse --onlydeps gcc"
+			echo "--- Updating seed stage with DEFAULT update_seed_command"
+			update_cmd="--update --deep --complete-graph --rebuild-if-new-ver gcc"
+			clst_root_path=/ run_merge ${update_cmd}
 		fi
+		if [ "${clst_update_seed_system}" == "yes" ]; then
+			echo "--- Updating seed stage system packages"
+			update_cmd="--update --deep --complete-graph @system"
+			clst_root_path=/ run_merge ${update_cmd}
+		fi
+		# now reset the emerge options for the target
+		clst_update_seed=no setup_myemergeopts
 	elif [ "${clst_update_seed}" != "no" ]; then
 		echo "Invalid setting for update_seed: ${clst_update_seed}"
 		exit 1
diff --git a/targets/support/chroot-functions.sh b/targets/support/chroot-functions.sh
index 2524b4f..69d2923 100755
--- a/targets/support/chroot-functions.sh
+++ b/targets/support/chroot-functions.sh
@@ -133,9 +133,18 @@ setup_myemergeopts(){
 	then
 		export bootstrap_opts="${bootstrap_opts} -f"
 		export clst_myemergeopts="${clst_myemergeopts} -f"
+	# now intercept normal target options if we're updating the seed
+	# to update the seed we do not want binpkgs that may have links to
+	# sonames no longer installed, due to dependency updates.
+	# this function will be re-run later with clst_update_seed=no
+	elif [ "${clst_update_seed}" == "yes" ]
+	then
+		export clst_myemergeopts="${clst_myemergeopts} --newuse"
+		export bootstrap_opts="${bootstrap_opts} -r"
 	elif [ -n "${clst_PKGCACHE}" ]
 	then
-		export clst_myemergeopts="${clst_myemergeopts} --usepkg --buildpkg --newuse"
+		# if you add --usepkg, then also add --binpkg-respect-use=y
+		export clst_myemergeopts="${clst_myemergeopts} --usepkg --binpkg-respect-use=y --buildpkg --newuse"
 		export bootstrap_opts="${bootstrap_opts} -r"
 	fi
 }
-- 
1.8.1.2

Re: [gentoo-catalyst] patch, fix broken seed stage update

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

On Tue, Feb 26, 2013 at 08:20:45AM -0800, Brian Dolbec wrote:
> The git branch is located at http://dev.gentoo.org/~dolsen/catalyst/
> git checkout the rewrite branch.

Did you forget to push?

  $ git remote -v | grep dolsen
  dolsen  http://dev.gentoo.org/~dolsen/catalyst (fetch)
  dolsen  http://dev.gentoo.org/~dolsen/catalyst (push)
  $ git fetch dolsen
  $ git log --pretty=fuller dolsen/rewrite
  commit 0fb26090a730f876993c13e23b6da3effdd5d1ed
  Author:     Brian Dolbec <dolsen@gentoo.org>
  AuthorDate: Thu Jan 24 21:09:05 2013 -0800
  Commit:     Brian Dolbec <dolsen@gentoo.org>
  CommitDate: Thu Jan 24 21:13:01 2013 -0800

      add archdir to settings

> Next on my todo list, fix doc's creation, create a setup.py and make the
> code installable via ebuild.  

For what it's worth, I'd rather see this polished up an merged into
master before you add more work on top, mostly because that would make
it easier for me to help out ;).

> I think also development should continue in a branch on the main
> catalyst repo on g.o.g.o.  Possibly name it catalyst3b so it does not
> conflict with the catalyst3 branch started.  I looked at rebasing my
> work on it, but decided against it.  There were far too many changes in
> master since it was last updated.

Avoiding this is another reason to get fairly incremental portions of
your work merged back into master before they grow too unwieldy.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

On Tue, 2013-02-26 at 11:37 -0500, W. Trevor King wrote:
> On Tue, Feb 26, 2013 at 08:20:45AM -0800, Brian Dolbec wrote:
> > The git branch is located at http://dev.gentoo.org/~dolsen/catalyst/
> > git checkout the rewrite branch.
> 
> Did you forget to push?
> 
>   $ git remote -v | grep dolsen
>   dolsen  http://dev.gentoo.org/~dolsen/catalyst (fetch)
>   dolsen  http://dev.gentoo.org/~dolsen/catalyst (push)
>   $ git fetch dolsen
>   $ git log --pretty=fuller dolsen/rewrite
>   commit 0fb26090a730f876993c13e23b6da3effdd5d1ed
>   Author:     Brian Dolbec <dolsen@gentoo.org>
>   AuthorDate: Thu Jan 24 21:09:05 2013 -0800
>   Commit:     Brian Dolbec <dolsen@gentoo.org>
>   CommitDate: Thu Jan 24 21:13:01 2013 -0800
> 
>       add archdir to settings
> 

BAH!, forgot to ssh in and run git update-server-info on it

try again.

another reason to get a branch going on g.o.g.o

Also I've rebased everything on current master including W. Trevor
King's patches.

Re: [gentoo-catalyst] More proposed Catalyst changes

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 6056 bytes --]

On Sat, Feb 02, 2013 at 12:41:32PM -0800, Brian Dolbec wrote:
> On Thu, 2013-01-31 at 14:46 -0500, W. Trevor King wrote:
> > 968d818 Initial creation of a defaults file. Split out hash and
> > contents to their own classes, files
> > 
> > After this, it's not clear to me what the difference is between
> > catalyst.support and catalyst.util.  Perhaps they should be merged.
> 
> I hadn't looked at util until now.
> 
> hmm, I wonder if it would be better to move CatalystError there and
> rename it to error.py  There is only a couple error traceback functions
> in it.  I was thinking it might be good to define a few more specific
> error classes than just the general CatalystError().  But I am not yet
> familiar enough with the code to know for certain.

This sounds good to me too.

> > a4ef493 re-version to "git-rewrite branch"
> > 
> > Why?
> 
> Why not.  It is not intended to be pushed into master.  It also helps to
> confirm that you are running the correct code.  I have fixed it so the
> code can be run from the git checkout.  That way you can have a
> "Production" version installed on the same system.  Now that I am
> testing/debugging, I am comparing catalyst operation and results to
> current -9999 code.  I am not familiar with using catalyst, so that is
> helping me figure out what is wrong.

A fair enough.  Again, it would be nice if there was a FIXME or
something in the commit message so I knew it wasn't destined for
master ;).

> > 9d752a7 move confdefaults out of main.py
> > 
> > Looks good, except, I'm not sure why you changed from
> > `confdefaults.keys()` to `list(confdefaults)` in parse_config() (which
> > should probably be living in catalyst.config anyway).
> 
> keeping a separate defaults file can be helpful in importing some info
> into different modules while keeping imports to a minimum.  Sometimes it
> helps prevents circular import problems.  At this point I opted for a
> separate file, to be determined later if a merge is warranted.

ok.

> As for list(confdefaults), py3 compatibility.  dict.keys() isn't usable
> and 2to3 converts it to list(dict)... something about needing to specify
> the return type.  So is a preemptive change.   One less thing to change
> later.

Really?

  $ python3.3 -c "a = {1:2, 3:4}; print([x for x in a.keys()])"
  [1, 3]

On the other hand, it might be cleaner to just say:

  for x in confdefaults:

But this should still go into a separate commit.

> > c303dae some options cleanup, unifying their use, reducing redundancy.
> > 
> > While I like the general thrust of this, I'd be happier with explicit
> > boolean options instead of a set of boolean options.  For example:
> > 
> >   confdefaults = {
> >     'autoresume': False,
> >     'ccache': False,
> >     …
> > 		}
> 
> Yeah, I removed those.  They were capitalized versions of the values in
> options.  So, I optimized them into options becoming a set which
> eliminates, the duplication and potential problems by changing the value
> of one and not the other.  Believe me keeping 2 different lists in sync
> can be much more difficult than it seems.  It also makes things much
> more difficult to debug.  (the independent booleans like you suggest can
> be considered a list, the other is the options list, set,
> string...whatever form it is in)  

I think I would do something like:

  import collections as _collections
  import ConfigParser as _configparser

  CONFIG = _configparser.ConfigParser(dict_type=_collections.OrderedDict)
  for setting,value in [
          ('ccache', str(False))]:
      CONFIG.set('DEFAULT', setting, value)

although OrderedDict doesn't exist in 2.6, where we should probably
just fall back to dicts.  Use it with:

  if CONFIG.getboolean('DEFAULT', 'ccache'):
      …

Then there's no duplication to worry about, and you get a
configuration object Python developers will be familiar with.

> also using member inclusion is faster and prefered compared to other
> methods like has_key().  In this case, it is just simpler to use
> "options" rather than to individualize them.

I don't think option lookup speed will have much impact on catalyst
execution speed ;).  And with ConfigParser, individual options will
require no additional coding.

> > Can we use logging instead of print?
> 
> YES!!!!,  please :D
> 
> that's been on my wish list too. 

I can add this if you don't want to.  Let me know if you want me to
base my patch against `master`, or against something in your branch.

> > I think keyword arguments are better, because changes to keywords
> > usually occur alongside changes to the argument semantics.  A keyword
> > mismatch is an obvious fix, while changes due to a semantic shift can
> > be more subtle.
> 
> yeah, I was a bit frustrated at that point, debugging code, so chose the
> easy way.  /me fixes.

;).  I've certainly been there too.

> > 923e8a2 remove trailing slash for consistency in variables and remove
> > extra slashes in paths
> > 
> > os.path.join()
> 
> Yes, for sure.  But after I debug my current changes.  But also most of
> those are for the bash side consistency which can not use os.path.join()
> and were adding the slashes again at times.
> 
> I am not a bash programmer, so if someone good at the bash stuff wants
> to work on those... go for it.  I'll try to keep my damage to a minimum.

I can look into this, but I think I need a better description of the
problem first.  Can you give me an example breakage due to using
os.path.join?

> Also if releng want to recode them in python, that's ok with me ;)

I'd be ok with that too, although I don't feel a pressing need for it
;).

> Thank you for the review, it has been informative.  And good to keep me
> from any blunders.

No problem :D

Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] More proposed Catalyst changes

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 7886 bytes --]

On Thu, Jan 31, 2013 at 01:39:22PM -0500, W. Trevor King wrote:
> I'll see if I can get through 968d818, … sometime later today.

I had a second wind ;).  Here are some comments on the remaining
commits:

968d818 Initial creation of a defaults file. Split out hash and contents to their own classes, files

After this, it's not clear to me what the difference is between
catalyst.support and catalyst.util.  Perhaps they should be merged.

I'd also use catalyst.targets.__all__ instead of coding a list of
targets in the apparently unrelated
catalyst.defaults.valid_build_targets.

a4ef493 re-version to "git-rewrite branch"

Why?

9d752a7 move confdefaults out of main.py

Looks good, except, I'm not sure why you changed from
`confdefaults.keys()` to `list(confdefaults)` in parse_config() (which
should probably be living in catalyst.config anyway).

c303dae some options cleanup, unifying their use, reducing redundancy.

While I like the general thrust of this, I'd be happier with explicit
boolean options instead of a set of boolean options.  For example:

  confdefaults = {
    'autoresume': False,
    'ccache': False,
    …
		}

6aeb5e3 Move LockInUse from support.py to lock.py, fix bad execption raising,  pyflakes cleanup

With respect to the exception raising, see my comments on 63a25eb in
the previous email.  Ahh, it looks like you finish that off with
04068a1.

04068a1 massive pyflakes import cleanup and broken CatalystError calls

A lot of this is great stuff (Hooray no `import *`!).  However, I'd
split the `print_traceback=True` changes out into their own commit (or
just change the default value for CatalystError while you're testing).

It looks like an unrelated change to
catalyst.defaults.required_build_targets snuck into this commit.

The hash_map changes should probably be squashed into the earlier hash
reorganization, rather than going into this commit.

c672887 begin splitting up generic_stage_target into smaller code blocks so snapshot_target does not need to import it since most of it was not used or initialized properly.

I'd shift “so snapshot_target…” into the commit body for a shorter
summary.  Not much to say about the code, since I haven't dug though
it yet.

2b29212 some spacing and comment cleanup, etc.

Looks ok.

28888a7 remove redundant /bin/bash additions in cmd() calls

Looks good.

97802b6 clean out the old spawn functions, use subprocess.call() instead

Hooray.  Since you're doing this, I'd drop the earlier commits that
referenced this change (e.g. d23d96b and 1b2597e).

624eb0d move base stage and target files to thier own sub-pkg

s/thier/their/

The diff looks good.

5c689a8 update module loading for the new python structure, rename snapshot_target to snapshot

I think I like this.  It's good to use __import__.  You're missing a
blank line after your import_module docstring summary [1].

I think that the import_module API should match Python 3's
importlib.import_module [2].

669451d fix options being reset by a config file

See my comments on c303dae.  We should really be using ConfigParser's
defaults here, instead of rolling our own default system.

f56017b fix missed PURGEONLY changes

This should be squashed into the settings work that caused it.

c5eb7f7 fix mounts, mountmap hardcoding removal, use normpath to fix paths, add some debug print statements, remove clear & purge code from support.py,

Split the long commit summary.

Can we use logging instead of print?

The except (Failure loading " + x + " plugin) should be squashed into
the commit that caused it and be restricted to only catch
ImportErrors.

The mountmap fix for 'proc' → '/proc' should be squashed into the
commit that caused it.

-               mypath=self.settings["chroot_path"]
+               #mypath=self.settings["chroot_path"]

Just remove this entirely, don't comment it out.

b2ea321 remove a named parameter in generate_hash() call due to a parameter name change

Move “due to …” into the commit message body.

I think keyword arguments are better, because changes to keywords
usually occur alongside changes to the argument semantics.  A keyword
mismatch is an obvious fix, while changes due to a semantic shift can
be more subtle.

a847803 Use normpath from support

Why are we not using os.path.normpath?

Also, remove instead of commenting out.

6108400 chmod +x all sh scripts so they can run from the git checkout

Looks good,

242c17d rename all target .py file and classes without the "_target" so they are the same as the .sh files and work with teh simplified module loading.

Move “so they …” into the commit message body and s/teh/the/.

I was going to suggest this if you hadn't gotten around to it ;).

ecc653f fix targets.sh path

Squash into the commit that caused it.

d563be8 Comment out a small code block causing TypeError, which also was short circuiting another large code block.  FIXME!!!! This whole class seems overly complicated with TOO MANY nested try:excepts:

Long commit summary.

I agree that a few of these methods should be split into helper
functions for easier reading.

e05cc34 Break out a few more repeated (path1 + path2)'s  to doing it once and using the temp variable. comment out some debug print's

Long commit summary.

os.path.join!  Also, convert debug prints to use the logging module
;).

e0738c0 rename a make.conf key to make_conf due to bash variable name restrictions

Squash into causing commit.

db2af17 fix my broken subprocess call to use Popen due to environment passing and return code needs

Squash into causing commit.

ae61e4a reduce 2 operations into one simpler one

Just switch to ConfigParser ;).

b08a757 extend ParserBase to do variable substitution.

ConfigParser does that to ;).

265c8ed add a "shdir" setting to make moving the bash code around easier and have less hardcoded paths in the bash scripts

Long commit summary.

Your comment claims catalyst runtime executables for both sharedir and
shdir.

dc90e40 migrate all target shell scripts to use the new shdir setting

Squash into 265c8ed.

2fa8968 minor cleanup

Squash into earlier (proposed) global whitespace cleanup and import
cleanups.

d7007a9 fix a bug in relative path use.  Add some unique identifiers to echo statements to make identifying the source of a message easier.

Split into two commits (relative path, unique identifiers).  We should
also try and figure out less arbitrary identifiers.

8b9a1c0 add embedded variable substitiution to default settings

ConfigParser does this.

1259fa6 make shdir a complete path to ease it's use. Add port_config path setting to remove more hard coded paths in code.

Squash shdir stuff into the earlier shdir addition.

Squash port_config into wherever you use it…

923e8a2 remove trailing slash for consistency in variables and remove extra slashes in paths

os.path.join()

55a58ee Add a forced debug print statement in cmd() for better debug output

Python logging.

24d69bd Commit my testpath file with instructions to run the git checkout code directly without being installed.

s/caatalyst/catalyst/

It would also probably be a good idea to add an example test.conf with
relative paths.

b7036f3 update gitignore

As I said earlier, the test.* files your ignoring don't seem to exist
in my checkout.

0fb2609 add archdir to settings

Looks good.

Cheers,
Trevor

[1]: http://www.python.org/dev/peps/pep-0257/#multi-line-docstrings
[2]: http://docs.python.org/dev/library/importlib.html#importlib.import_module

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] More proposed Catalyst changes

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 8395 bytes --]

On Sat, Feb 02, 2013 at 10:45:19AM -0800, Brian Dolbec wrote:
> I do think that it will be difficult to make complete commits for
> some changes that do not break catalyst until the remaining commits
> are also merged.

This may be true for some of the shifting commits, we'll see after the
initial reroll.

> Thank you for enough info for me to google about commit summary,
> message body.  Until now I didn't know about needing to separate
> them with a blank line.  I had never come across that info before,
> so had given up trying to format them better.  Everything I tried
> had failed.  Even googling it now it seems to be everywhere but
> git's own help.

Actually, it is in Git's help:

  $ git commit --help | grep -B3 blank
  DISCUSSION
    Though not required, it’s a good idea to begin the commit message with
    a single short (less than 50 character) line summarizing the change,
    followed by a blank line and then a more thorough description. The text
    up to the first blank line in a commit message is treated as the commit

> > 321df3b whitespace cleanup
> > 
> > This looks good, but there are also whitespace cleanups in some other
> > commits (e.g. 2d36d29).  I think we should just run something like:
> > 
> >   for FILE in $(git ls-tree -r --name-only HEAD | grep -v 'bz2$'); do
> >     sed -i 's/[[:space:]]*$//' "$FILE"
> >   done
> > 
> > and have done with it ;).
> 
> yeah, that requires way more knowledge about those tools than I know.
> For me, I open a file, the editor does the cleanup automatically.  If I
> don't separate them when committing, then...  If I didn't edit the
> file...

I've pushed this change to the `whitespace` branch on
git://tremily.us/catalyst.git if you want to rebase your changes onto
that.  I can also submit it as a patch to the mailing list if anyone
wants it that way.

> > 38cd3bb add more configured defaults
> > 
> > List the new settings (distdir, repo_name, packagedir, port_tmpdir,
> > options, snapshot_name) in the commit message so I don't have to read
> > the diff.  Probably explain why you think they should be configurable
> > as well.  `options` is also a pretty ambiguous name.
> 
> Now that I know how to blab on in a message body with a brief summary,
> yeah :)
> 
> "options" was existing and seems comparable to FEATURES for emerge in
> make.conf.  Suggestions for a new name?

In a later comment I suggested breaking each of the options variables
out into their own setting.  Then we don't have to worry about what to
call the boolean settings in aggregate ;).

> > This might also be a good place to move the path construction over to
> > use os.path.join().
> 
> Yes, I agree, but I am still getting accustomed to the code, so was
> holding off that for a bit.  Need to debug changes first.  I am also
> contemplating stealing the path() from layman which joins a list of
> partial paths.  I've noticed it could simplify code in many places that
> join more than 2 partial paths.

Doesn't os.path.join already do that?

  $ python2.7 -c "import os; print(os.path.join('a', 'b', 'c'))"
  a/b/c

> > This might also be a good time to transition from `CatalystError, …`
> > to `CatalystError(…)` where you're touching lines.
> > 
> > I'm not sure which versions of Python Catalyst is trying to support,
> > but I'd be happy to see the beginings of a migration to '{}'.format()
> > for building strings (vs. the current `'' + ''` concatenation)
> 
> yeah, there are a ton of those in the code.
> 
> Since portage is python-2.6+ and python-2.5 is pretty close to being
> removed from the tree... I intend to make it, 2.6, 2.7, 3.2 +
> compatible. 

So the CatalystError(…) updates will be fine, and we should use
'{0}'.format(xyz).

> > 60919e4 Apply all Matt Turner's has_key() replacement patches
> > 
> > This looks like a job for `rebase` ;).  Cherry-picking will just lead
> > to redundant commits.
> 
> That will be looked after in my rebase to master again.  Since I had
> already made a number of changes in my rewrite branch, I could not apply
> his commit cleanly without editing.

Right.  What you should have done is rebase onto the new `master`
branch.  Where Matt's changes clashed with yours, Git would stop and
ask to to fix the conflicts.  Then you could update *your* patches so
that they worked ;).  Since Matt's changes got into `master` first, he
gets to keep them the way they originally were.

> > 7a909b9 cleanup long lines, improve useage() output formatting
> slightly
> > 
> > Can we just switch from getopt to argparse (Python ≥2.7)?
> 
> Yes, we can also add argparse as dep for 2.6 if they want to maintain
> 2.6 compatibility.

Ah, that would be much nicer :).

> > 5eeb8b1 new minimal start script
> > 
> > I think __version__ and __maintainer__ should live in
> > catalyst/__init__.py.
> 
> Can do.  The version can also be set for releases easily using some
> methods from gentoolkit when making the release tarball.  The version
> could then be left as "git" with a partial hash added of the last commit
> when it was merged with.

I do something like this in bugs-everywhere [1], but for Catalyst I
think it may be more work than it's worth.

> > 267dbb6 add __init__.py's to modules and arch sub-pkgs
> > 
> > Added with blank lines?
> 
> I cheated at the time and just used my editor to save an existing one to
> different places.  So it likely saved it with a blank line.  Instead of
> using a terminal and touch.

These should probably get copyright blurbs and descriptive docstrings
anyway, so it's probably not worth worrying over the blank lines.

> > da6cbbf rename shell script targets directory to targets.sh so as to
> not be confused with the new python targets directory
> > 
> > “so as…” should go in the commit message body to get a shorter
> > summary.
> > 
> > I'm not sold on this.  Since they are data files intended to be used
> > by the Python modules, why not move them to catalyst/targets/scripts
> [1]?
>  
> I had been re-thinking that change and was thinking of renaming it back
> to targets now that things have been better rearranged.  There is one py
> file in stage1.  It is also easier to do now since it is just one edit
> and a git mv away.  I've removed that directory name from being hard
> coded in the scripts, so now it just means there is one place to edit.

This sounds reasonable to me.

> > f9f18be update gitignore
> > 
> > The Scratch, catalystc, *.geany, and test* entries would appear to be
> > specific to your local installation and usage.  I'd drop them from the
> > reroll.
> > 
> 
> They are only for my dev space git repo and me.  They will not be part
> of a final ready to merge branch.  But my dev space is also acting as my
> backup, so I committed them.  It makes my git status output cleaner.

Fair enough, but it's probably a good idea to put your local changes
in a clearly marked commit:

  $ git commit -am 'FIXME: local changes to gitignore'

> > d23d96b revert urllib import from commit
> 64c16cae70da13de3c55d8555a2e4c5dcdf2fcad since it is not used anywhere
> in catalyst
> > 
> > 64c16cae does a lot more than import urllib, so I think “revert” gives
> > the wrong impression.  I'd remove the import entirely (instead of
> > commenting it out), and say:
> > 
> >   catalyst/support.py: Remove unused urllib import
> > 
> >   This was introduced in commit
> >   64c16cae70da13de3c55d8555a2e4c5dcdf2fcad but is no longer used.
> 
> Actually urllib was never used.  It was added as a fix for a bug.  It
> was not a correct fix, so just masked what was causing the bug.  I only
> commented it out until I could either find what caused the bug or it
> just didn't return due to other changes.  It is often easier to
> comment/uncomment during testing runs than to remove/re-add code.

Sure.  It's strange that it had any effect at all on a bug if it was
never used, but whatever ;).

Cheers,
Trevor

[1]: http://gitorious.org/be/be/blobs/master/libbe/version.py#line21
     http://gitorious.org/be/be/blobs/master/Makefile#line80

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] More proposed Catalyst changes

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 5880 bytes --]

On Sat, Jan 12, 2013 at 12:55:58AM -0800, Brian Dolbec wrote:
> You can get a git clone of my rewrite work at:
> http://dev.gentoo.org/~dolsen/catalyst/
> 
> rewrite branch

Ah, it's good to see things looking more Pythonic in Catalyst-land ;).
Here's my initial review to push this forward, although I'm not
particularly well-versed in the Catalyst internals.  This is branch is
getting to the stage where rebasing will get annoying, so I think
merging stuff that looks good into the master branch should happen
sooner, rather than later ;).

321df3b whitespace cleanup

This looks good, but there are also whitespace cleanups in some other
commits (e.g. 2d36d29).  I think we should just run something like:

  for FILE in $(git ls-tree -r --name-only HEAD | grep -v 'bz2$'); do
    sed -i 's/[[:space:]]*$//' "$FILE"
  done

and have done with it ;).

38cd3bb add more configured defaults

List the new settings (distdir, repo_name, packagedir, port_tmpdir,
options, snapshot_name) in the commit message so I don't have to read
the diff.  Probably explain why you think they should be configurable
as well.  `options` is also a pretty ambiguous name.

016704a use the new configured snapshot_name and portdir settings

Rather than a separate add (38cd3bb) and use (016704a), I think it
would be better if new options had their addition and use rolled into
a single commit (e.g. add snapshot_name and use it as one commit, add
portdir and use it as another commit).

This might also be a good place to move the path construction over to
use os.path.join().

fefd501 use the portdir setting rather than hard-coded path

Good to merge.

63a25eb Remove self.mounts and self.mountmap's use of paths for keys and paths. Migrate more hardcoded paths to use settings.

That's a long commit summary ;).  Perhaps the “Migrate…” portion
should go into the commit message body, or that could be split out
into a separate commit.

This might also be a good time to transition from `CatalystError, …`
to `CatalystError(…)` where you're touching lines.

I'm not sure which versions of Python Catalyst is trying to support,
but I'd be happy to see the beginings of a migration to '{}'.format()
for building strings (vs. the current `'' + ''` concatenation)

60919e4 Apply all Matt Turner's has_key() replacement patches

This looks like a job for `rebase` ;).  Cherry-picking will just lead
to redundant commits.

7a909b9 cleanup long lines, improve useage() output formatting slightly

Can we just switch from getopt to argparse (Python ≥2.7)?

af6e6b7 Initial rearrangement of the python directories

Hooray!

5eeb8b1 new minimal start script

I think __version__ and __maintainer__ should live in
catalyst/__init__.py.

267dbb6 add __init__.py's to modules and arch sub-pkgs

Added with blank lines?

299e35d update the module loading paths for the new locations

Can we just drop this import manipulation and use __all__?

e26de08 chmod +x

Squash into 5eeb8b1.

78d8be2 fix catalyst_support import to new location and specify imported modules

Looks good.

2d36d29 move catalyst_support, builder, catalyst_lock out of modules, into the catalyst's base namespace

Yay!  There are some whitespace changes in here too though (see my
comments on 321df3b).

bc66f5d rename the modules subpkg to targets, it better reflects what it contains

Looks good.

a7206bb rename files directory to etc to better reflect the directories contents

Actually, files/ is also used for other things (e.g. built man pages,
see MAN_PAGES in the Makefile).  If we keep the rename to etc/, we
might to just build the man pages under doc/.

da6cbbf rename shell script targets directory to targets.sh so as to not be confused with the new python targets directory

“so as…” should go in the commit message body to get a shorter
summary.

I'm not sold on this.  Since they are data files intended to be used
by the Python modules, why not move them to catalyst/targets/scripts [1]?

f9f18be update gitignore

The Scratch, catalystc, *.geany, and test* entries would appear to be
specific to your local installation and usage.  I'd drop them from the
reroll.

d23d96b revert urllib import from commit 64c16cae70da13de3c55d8555a2e4c5dcdf2fcad since it is not used anywhere in catalyst

64c16cae does a lot more than import urllib, so I think “revert” gives
the wrong impression.  I'd remove the import entirely (instead of
commenting it out), and say:

  catalyst/support.py: Remove unused urllib import

  This was introduced in commit
  64c16cae70da13de3c55d8555a2e4c5dcdf2fcad but is no longer used.

ab2c22a comment out some code introduced in commit b3475906d5f51a21ecaf4ff048002a2f44face52 with an undefined variable "s".  The code must always be hitting the general except: pass block.  Seems useless, if not maybe it'll spit out a true error, so the real problem can be fixed

Multiple sentences in a commit summary!  Also, take advantage of your
version control and just rip the code out instead of commenting it
out.  I looked through b347590 (which is a 408-line monster for
catalyst_support.py), but I'm not clear why Eric was messing with that
code.  At any rate, it looks like a good cantidate for replacement
with subprocess.Popen, although see 1b2597e ;).

95704c7 fix an undefined variable

Oops ;).  I'd mention DESIRED_RLIMIT in the commit message.

1b2597e update todo for a spawn() cleanup.

+lots

I'll see if I can get through 968d818, … sometime later today.

Cheers,
Trevor

[1]: http://docs.python.org/2/distutils/setupscript.html#installing-package-data

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] patch, fix broken seed stage update

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 9746 bytes --]

On Tue, Feb 26, 2013 at 08:47:14AM -0800, Brian Dolbec wrote:
> Also I've rebased everything on current master

That should make things easier to merge :).

It looks like some of my earlier comments were addressed by this
reroll, but some are still applicable.  Apologies if we'd resolved any
of this earlier and I just missed the reference in my mailbox.

I map my old comments onto the rebased commits below, but the bulk of
the outstanding suggestions revolve around:

* ConfigParser-based configuration
* Argparse-based command line parsing
* Logging-based debugging output
* os.path.join(), normpath(), … for path manipulation

These are mostly “take advantage of Python's standard library”
changes, and I'd be happy to help implement them on top of the current
master if folks feel like that has a chance of getting merged ;).

On Thu, Jan 31, 2013 at 01:39:22PM -0500, W. Trevor King wrote:
> 38cd3bb add more configured defaults
> 
> List the new settings (distdir, repo_name, packagedir, port_tmpdir,
> options, snapshot_name) in the commit message so I don't have to read
> the diff.  Probably explain why you think they should be configurable
> as well.  `options` is also a pretty ambiguous name.

This still applies to 0a25452.  I made a few comments about using
separate boolean options instead of an aggregate `options` set.
Fixing this should be part of the ConfigParser transition.

> 016704a use the new configured snapshot_name and portdir settings
> 
> Rather than a separate add (38cd3bb) and use (016704a), I think it
> would be better if new options had their addition and use rolled into
> a single commit (e.g. add snapshot_name and use it as one commit, add
> portdir and use it as another commit).
> 
> This might also be a good place to move the path construction over to
> use os.path.join().

This still applies to be2f820.

> 63a25eb Remove self.mounts and self.mountmap's use of paths for keys and paths. Migrate more hardcoded paths to use settings.
> 
> That's a long commit summary ;).  Perhaps the “Migrate…” portion
> should go into the commit message body, or that could be split out
> into a separate commit.

This splitting happened in 77eece8, but…

> This might also be a good time to transition from `CatalystError, …`
> to `CatalystError(…)` where you're touching lines.
> 
> I'm not sure which versions of Python Catalyst is trying to support,
> but I'd be happy to see the beginings of a migration to '{}'.format()
> for building strings (vs. the current `'' + ''` concatenation)

… this still applies.

> 7a909b9 cleanup long lines, improve useage() output formatting slightly
> 
> Can we just switch from getopt to argparse (Python ≥2.7)?

Still applies to 7870959.

> 5eeb8b1 new minimal start script
> 
> I think __version__ and __maintainer__ should live in
> catalyst/__init__.py.

Still applies to baae19f.

> 299e35d update the module loading paths for the new locations
> 
> Can we just drop this import manipulation and use __all__?

Still applies to 79f73a3.

> a7206bb rename files directory to etc to better reflect the directories contents
> 
> Actually, files/ is also used for other things (e.g. built man pages,
> see MAN_PAGES in the Makefile).  If we keep the rename to etc/, we
> might to just build the man pages under doc/.

Still applies to 25f6f1b.

> f9f18be update gitignore
> 
> The Scratch, catalystc, *.geany, and test* entries would appear to be
> specific to your local installation and usage.  I'd drop them from the
> reroll.

They're still in 7fdecf4, but it will be easy to drop this before the
merge with master.

On Thu, Jan 31, 2013 at 02:46:53PM -0500, W. Trevor King wrote:
> 968d818 Initial creation of a defaults file. Split out hash and contents to their own classes, files
> 
> After this, it's not clear to me what the difference is between
> catalyst.support and catalyst.util.  Perhaps they should be merged.
> 
> I'd also use catalyst.targets.__all__ instead of coding a list of
> targets in the apparently unrelated
> catalyst.defaults.valid_build_targets.

Still applies to c97dc3d.

> 9d752a7 move confdefaults out of main.py
> 
> Looks good, except, I'm not sure why you changed from
> `confdefaults.keys()` to `list(confdefaults)` in parse_config() (which
> should probably be living in catalyst.config anyway).

Still applies to 0c2302a.  Additional discussion from a sub-thread:

On Sun, Feb 03, 2013 at 07:44:36AM -0500, W. Trevor King wrote:
> On Sat, Feb 02, 2013 at 12:41:32PM -0800, Brian Dolbec wrote:
> > As for list(confdefaults), py3 compatibility.  dict.keys() isn't usable
> > and 2to3 converts it to list(dict)... something about needing to specify
> > the return type.  So is a preemptive change.   One less thing to change
> > later.
> 
> Really?
> 
>   $ python3.3 -c "a = {1:2, 3:4}; print([x for x in a.keys()])"
>   [1, 3]
> 
> On the other hand, it might be cleaner to just say:
> 
>   for x in confdefaults:
> 
> But this should still go into a separate commit.

I still think this is true ;).

On Thu, Jan 31, 2013 at 02:46:53PM -0500, W. Trevor King wrote:
> c303dae some options cleanup, unifying their use, reducing redundancy.
> 
> While I like the general thrust of this, I'd be happier with explicit
> boolean options instead of a set of boolean options.  For example:
> 
>   confdefaults = {
>     'autoresume': False,
>     'ccache': False,
>     …
> 		}

Still applies to ca85cd4.  Like I said above, I'm happy to work up a
ConfigParser-based solution.

> 04068a1 massive pyflakes import cleanup and broken CatalystError calls
> 
> A lot of this is great stuff (Hooray no `import *`!).  However, I'd
> split the `print_traceback=True` changes out into their own commit (or
> just change the default value for CatalystError while you're testing).
> 
> It looks like an unrelated change to
> catalyst.defaults.required_build_targets snuck into this commit.
> 
> The hash_map changes should probably be squashed into the earlier hash
> reorganization, rather than going into this commit.

Still applies to eed08b1.

> 5c689a8 update module loading for the new python structure, rename snapshot_target to snapshot
> 
> I think I like this.  It's good to use __import__.  You're missing a
> blank line after your import_module docstring summary [1].
> 
> I think that the import_module API should match Python 3's
> importlib.import_module [2].

Still applies to still applies to f733b3f.

> 669451d fix options being reset by a config file
> 
> See my comments on c303dae.  We should really be using ConfigParser's
> defaults here, instead of rolling our own default system.

ConfigParser :).

> c5eb7f7 fix mounts, mountmap hardcoding removal, use normpath to fix paths, add some debug print statements, remove clear & purge code from support.py,
> 
> Split the long commit summary.

This was fixed in 9fabaae, but…

> Can we use logging instead of print?
> 
> The except (Failure loading " + x + " plugin) should be squashed into
> the commit that caused it and be restricted to only catch
> ImportErrors.
> 
> The mountmap fix for 'proc' → '/proc' should be squashed into the
> commit that caused it.
> 
> -               mypath=self.settings["chroot_path"]
> +               #mypath=self.settings["chroot_path"]
> 
> Just remove this entirely, don't comment it out.

… these were not.

> a847803 Use normpath from support
> 
> Why are we not using os.path.normpath?
> 
> Also, remove instead of commenting out.

Still applies to be413a4.

> 242c17d rename all target .py file and classes without the "_target" so they are the same as the .sh files and work with teh simplified module loading.
> 
> Move “so they …” into the commit message body and s/teh/the/.

With 6a86874, the commit body has “This is so they are the named the
same as the target…”, which should probably be “This is so they are
named the same as the target…” (removing an extra “the”).

> e05cc34 Break out a few more repeated (path1 + path2)'s  to doing it once and using the temp variable. comment out some debug print's
> 
> Long commit summary.

This is fixed in 4dd2cb2, but…

> os.path.join!  Also, convert debug prints to use the logging module
> ;).

… these are not.

> e0738c0 rename a make.conf key to make_conf due to bash variable name restrictions
> 
> Squash into causing commit.

Still applies to 253dab7.

> ae61e4a reduce 2 operations into one simpler one
> 
> Just switch to ConfigParser ;).

Still applies to 92c6745.

> b08a757 extend ParserBase to do variable substitution.
> 
> ConfigParser does that to ;).

Still applies to 104c387.

> 265c8ed add a "shdir" setting to make moving the bash code around easier and have less hardcoded paths in the bash scripts
> 
> Long commit summary.

This is fixed in b433a82, but…

> Your comment claims catalyst runtime executables for both sharedir and
> shdir.

… this is not.
 
> 55a58ee Add a forced debug print statement in cmd() for better debug output
> 
> Python logging.

Still applies to 3791efc.

> 24d69bd Commit my testpath file with instructions to run the git checkout code directly without being installed.
> 
> s/caatalyst/catalyst/
> 
> It would also probably be a good idea to add an example test.conf with
> relative paths.

Still applies to 786a01c.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

On Tue, 2013-02-26 at 13:04 -0500, W. Trevor King wrote:
> On Tue, Feb 26, 2013 at 08:47:14AM -0800, Brian Dolbec wrote:
> > Also I've rebased everything on current master
> 
> That should make things easier to merge :).
> 
> It looks like some of my earlier comments were addressed by this
> reroll, but some are still applicable.  Apologies if we'd resolved any
> of this earlier and I just missed the reference in my mailbox.
> 
> I map my old comments onto the rebased commits below, but the bulk of
> the outstanding suggestions revolve around:
> 
> * ConfigParser-based configuration
> * Argparse-based command line parsing
> * Logging-based debugging output
> * os.path.join(), normpath(), … for path manipulation
> 
> These are mostly “take advantage of Python's standard library”
> changes, and I'd be happy to help implement them on top of the current
> master if folks feel like that has a chance of getting merged ;).
> 

All that is for a TODO list.  What I did was rebase my existing changes,
merging fixes to the commits that made the change..., fix the commit
messages, etc...

Same with the other things you listed below, mostly for a TODO and your
wishlist.  Pretty much all I agree with.


My aim for now is to debug the hell out of it, to stabilize all the
existing changes, BEFORE doing a ton more changes.  That will also give
me more experience in how catalyst is used, possibly ideas how to change
it for the better.  I'll fix the doc's generation and make a setup.py
and new 9999 ebuild.

Plus I need to focus more on another project needed for the git tree
migration.


> Cheers,
> Trevor
> 

Re: [gentoo-catalyst] patch, fix broken seed stage update

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]

On Tue, Feb 26, 2013 at 05:30:40PM -0800, Brian Dolbec wrote:
> My aim for now is to debug the hell out of it, to stabilize all the
> existing changes, BEFORE doing a ton more changes.  That will also give
> me more experience in how catalyst is used, possibly ideas how to change
> it for the better.  I'll fix the doc's generation and make a setup.py
> and new 9999 ebuild.

I'm not suggesting additional changes on top of yours, I'm suggesting
new ones underneath yours, or alternatives to your current series.  It
seems a shame to spend time testing a work in progress, since you'll
probably want to re-test the final form before merging.  I'm trying to
help polish your series down before we invest a lot of time testing.

> Plus I need to focus more on another project needed for the git tree
> migration.

I have no problem waiting on the catalyst front if it helps push the
Git migration through.  I've been wishing for that for a while :D.

Instead of making my suggested changes on top of your series, I can
also make them underneath your series, and help rebase your series on
top.  That gets the changes I want in, and lets you focus on other
things at the same time ;).

I'm also ok with just waiting until folks have more time (e.g. the
catalyst-spec man page cooked for five months ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

On Tue, 2013-02-26 at 20:40 -0500, W. Trevor King wrote:
> On Tue, Feb 26, 2013 at 05:30:40PM -0800, Brian Dolbec wrote:
> > My aim for now is to debug the hell out of it, to stabilize all the
> > existing changes, BEFORE doing a ton more changes.  That will also give
> > me more experience in how catalyst is used, possibly ideas how to change
> > it for the better.  I'll fix the doc's generation and make a setup.py
> > and new 9999 ebuild.
> 
> I'm not suggesting additional changes on top of yours, I'm suggesting
> new ones underneath yours, or alternatives to your current series.  It
> seems a shame to spend time testing a work in progress, since you'll
> probably want to re-test the final form before merging.  I'm trying to
> help polish your series down before we invest a lot of time testing.
> 

NNNNNNNNNNNNOOOOOOOOOOOOOOOOOOOOOOOO :(  not more rebasing  I spent
several days just tracking down and fixing rebase errors.  Fix a few,
that in turn caused more errors...

You and I both know that the code is likely to be broken at times while
doing many of the code changes needed to fix it.  The original code was
horrible, not all of it, but...   The time for polish is later when the
majority of the changes are done.  I want to move forward, not churn in
circles, rebasing, fixing conflicts, forever waiting for a commit to be
reviewed & merged.  One of the big reasons I started on catalyst was to
remove the hardcoded paths everywhere that was preventing the tree
migration away from /usr/portage.  That has been largely accomplished,
although not really tested.

The reason I wanted to do more testing, is so far I've only done
snapshot, stage1 thru 3.  I've done the major restructure of the python
layout.  I want to get everything working, not 100% bug free, before
moving on.  That will make it easier to figure out where something got
broken while doing the changes.  Call it a midterm exam ;) to see where
we stand.

So I wouldn't mind working in a branch for now, somewhere we can all
participate in pushing commits to in order to achieve our end goal.
Then later when it's mostly done, we can look to moving it to master.
Finish debugging and polishing it.



> Cheers,
> Trevor
> 

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Matt Turner]

On Tue, Feb 26, 2013 at 6:35 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
> On Tue, 2013-02-26 at 20:40 -0500, W. Trevor King wrote:
>> On Tue, Feb 26, 2013 at 05:30:40PM -0800, Brian Dolbec wrote:
>> > My aim for now is to debug the hell out of it, to stabilize all the
>> > existing changes, BEFORE doing a ton more changes.  That will also give
>> > me more experience in how catalyst is used, possibly ideas how to change
>> > it for the better.  I'll fix the doc's generation and make a setup.py
>> > and new 9999 ebuild.
>>
>> I'm not suggesting additional changes on top of yours, I'm suggesting
>> new ones underneath yours, or alternatives to your current series.  It
>> seems a shame to spend time testing a work in progress, since you'll
>> probably want to re-test the final form before merging.  I'm trying to
>> help polish your series down before we invest a lot of time testing.
>>
>
> NNNNNNNNNNNNOOOOOOOOOOOOOOOOOOOOOOOO :(  not more rebasing  I spent
> several days just tracking down and fixing rebase errors.  Fix a few,
> that in turn caused more errors...

Rebasing can be annoying, but isn't inherently difficult. Typically
you don't rebase until you're ready for others to review and commit
them.

I'm curious if you know how to use interactive rebase to apply fix-ups
to commits? If not, I'll be happy to walk you through it. It greatly
simplifies the workflow.

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Peter Stuge]

Brian Dolbec wrote:
> BAH!, forgot to ssh in and run git update-server-info on it

Add a hook to do it for you in the future.


//Peter

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Rick "Zero_Chaos" Farina]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/26/2013 11:20 AM, Brian Dolbec wrote:
> The git branch is located at http://dev.gentoo.org/~dolsen/catalyst/
> git checkout the rewrite branch.
> 
> For those of you that have already cloned it, use --force in your pull.
> I've condensed, rearranged the changes into more logical complete
> changes.  Still, I wouldn't think you could cherrypick any single commit
> in general.  Some fixes could be cherrypicked to apply to current
> master, some would likely have to be hand applied due to other changes.
> 
> Next on my todo list, fix doc's creation, create a setup.py and make the
> code installable via ebuild.  
> 
> I think also development should continue in a branch on the main
> catalyst repo on g.o.g.o.  Possibly name it catalyst3b so it does not
> conflict with the catalyst3 branch started.  I looked at rebasing my
> work on it, but decided against it.  There were far too many changes in
> master since it was last updated.
In my opinion you can kill the current catalyst 3 branch as nothing uses
it and afaik it doesn't work.  jmbvicetto should be able to
authoritatively agree or not.

- -Zero
> 
> There are far too many patches to individually list them in this mail
> list.  Please checkout the branch from my repo to review the changes.
> It would also be easier to use gitweb to view them online if it was
> pushed to the main repo.
> 
> 
> Anyway the latest patch...
> 
> ======================================================================
> 
> Fix broken seed stage update...
> 
> Strip --usepkg and --buildpkg from emerge options for user defined
> update_seed_command.
> Add a check for the update_seed option to set the correct update
> options.
> Fix default seed stage update command to properly update gcc and it's
> deps.
> Add a seed stage update system command and option.
> Add --binpkg-respect-use=y for all cases --usepkg is enabled.
> 
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
>>From 9ffa5b8812403bf20f17eba58543fc4b7c04bc33 Mon Sep 17 00:00:00 2001
> From: Brian Dolbec <dolsen@gentoo.org>
> Date: Mon, 25 Feb 2013 23:31:41 -0800
> Subject: [PATCH] Fix broken seed stage update...
> 
> Strip --usepkg and --buildpkg from emerge options for user defined update_seed_command.
> Add a check for the update_seed option to set the correct update options.
> Fix default seed stage update command to properly update gcc and it's deps.
> Add a seed stage update system command and option.
> Add --binpkg-respect-use=y for all cases --usepkg is enabled.
> ---
>  catalyst/targets/stage1.py          |  3 ++-
>  doc/catalyst-spec.5.txt             | 12 +++++++++++-
>  targets/stage1/stage1-chroot.sh     | 17 ++++++++++++++---
>  targets/support/chroot-functions.sh | 11 ++++++++++-
>  4 files changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/catalyst/targets/stage1.py b/catalyst/targets/stage1.py
> index e936929..e067c8c 100644
> --- a/catalyst/targets/stage1.py
> +++ b/catalyst/targets/stage1.py
> @@ -18,7 +18,8 @@ class stage1(StageBase):
>  	def __init__(self,spec,addlargs):
>  		self.required_values=[]
>  		self.valid_values=["chost"]
> -		self.valid_values.extend(["update_seed","update_seed_command"])
> +		self.valid_values.extend(["update_seed","update_seed_command",
> +			"update_seed_system"])
>  		StageBase.__init__(self,spec,addlargs)
>  
>  	def set_stage_path(self):
> diff --git a/doc/catalyst-spec.5.txt b/doc/catalyst-spec.5.txt
> index 4a6e06c..196bdc3 100644
> --- a/doc/catalyst-spec.5.txt
> +++ b/doc/catalyst-spec.5.txt
> @@ -138,9 +138,19 @@ it should update the seed stage or not (valid values: `yes no`).
>  *update_seed_command*::
>  This is an optional command to pass to emerge for updating the seed
>  stage (example: `--update dev-libs/mpfr dev-libs/mpc dev-libs/gmp`)
> -If not specified, catalyst will update gcc deps.
> +If not specified, catalyst will update gcc's deps, and rebuild gcc if any of
> +it's deps are updated with a new version. Even if it itself is not updated.
> +This prevents gcc breakage when it's dependency lib sonames have changed.
>  This setting requires enabling update_seed.
>  
> +*update_seed_system*::
> +This is an optional setting supported by stage1 to tell catalyst if
> +it should update the seed's system packages or not (valid values: `yes no`).
> +This is run after any update_seed_command, updating any remaining upgradable
> +system packages.
> +This setting requires enabling update_seed.
> +
> +
>  Compilation
>  ~~~~~~~~~~~
>  
> diff --git a/targets/stage1/stage1-chroot.sh b/targets/stage1/stage1-chroot.sh
> index 97aef7f..65c2d81 100755
> --- a/targets/stage1/stage1-chroot.sh
> +++ b/targets/stage1/stage1-chroot.sh
> @@ -26,12 +26,23 @@ clst_root_path=/ setup_pkgmgr
>  # Update stage3
>  if [ -n "${clst_update_seed}" ]; then
>  	if [ "${clst_update_seed}" == "yes" ]; then
> -		echo "Updating seed stage..."
>  		if [ -n "${clst_update_seed_command}" ]; then
> -			clst_root_path=/ run_merge "--buildpkg=n ${clst_update_seed_command}"
> +			echo "--- Updating seed stage with USER defined update_seed_command"
> +			update_cmd=${clst_update_seed_command/--usepkg /}
> +			update_cmd=${clst_update_seed_command/--buildpkg /}
> +			clst_root_path=/ run_merge "${update_cmd}"
>  		else
> -			clst_root_path=/ run_merge "--buildpkg=n --update --deep --newuse --onlydeps gcc"
> +			echo "--- Updating seed stage with DEFAULT update_seed_command"
> +			update_cmd="--update --deep --complete-graph --rebuild-if-new-ver gcc"
> +			clst_root_path=/ run_merge ${update_cmd}
>  		fi
> +		if [ "${clst_update_seed_system}" == "yes" ]; then
> +			echo "--- Updating seed stage system packages"
> +			update_cmd="--update --deep --complete-graph @system"
> +			clst_root_path=/ run_merge ${update_cmd}
> +		fi
> +		# now reset the emerge options for the target
> +		clst_update_seed=no setup_myemergeopts
>  	elif [ "${clst_update_seed}" != "no" ]; then
>  		echo "Invalid setting for update_seed: ${clst_update_seed}"
>  		exit 1
> diff --git a/targets/support/chroot-functions.sh b/targets/support/chroot-functions.sh
> index 2524b4f..69d2923 100755
> --- a/targets/support/chroot-functions.sh
> +++ b/targets/support/chroot-functions.sh
> @@ -133,9 +133,18 @@ setup_myemergeopts(){
>  	then
>  		export bootstrap_opts="${bootstrap_opts} -f"
>  		export clst_myemergeopts="${clst_myemergeopts} -f"
> +	# now intercept normal target options if we're updating the seed
> +	# to update the seed we do not want binpkgs that may have links to
> +	# sonames no longer installed, due to dependency updates.
> +	# this function will be re-run later with clst_update_seed=no
> +	elif [ "${clst_update_seed}" == "yes" ]
> +	then
> +		export clst_myemergeopts="${clst_myemergeopts} --newuse"
> +		export bootstrap_opts="${bootstrap_opts} -r"
>  	elif [ -n "${clst_PKGCACHE}" ]
>  	then
> -		export clst_myemergeopts="${clst_myemergeopts} --usepkg --buildpkg --newuse"
> +		# if you add --usepkg, then also add --binpkg-respect-use=y
> +		export clst_myemergeopts="${clst_myemergeopts} --usepkg --binpkg-respect-use=y --buildpkg --newuse"
>  		export bootstrap_opts="${bootstrap_opts} -r"
>  	fi
>  }
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRLPDkAAoJEKXdFCfdEflKLykP/Rx73fH878oLHFunNkfpa929
us+Kcf74QNvTjDUH2uzgofICiJFsnCdgUjmF0+gBDUY8DjxOjMYI4CrbqF6FEJ88
Q+rmN5rkcFvqRaVZrh480U9qPhm8JvYvIzviijBBmOqZDOgFQHpOPJMSeRcMAU/5
7fAT5zbmUGduKtecV/juI6UQimo5/eNuWd/z7BY3qAG/eh4skFK1M0JX2JJKnlAO
m5I5GCDKn32FQhhm5OdW+GFgS6c6MczBGGIEnyTwu7Lv50OC6XRm9xP4uiDjGOqW
A+tPhp1pidhamjCYhW0A+1fv5ZYe6DIZy/b8p/KrgB3oDKmhwWn83u1RhCu3GxMe
6Scg1KlnBJgPVa8b7OiS0lbJlrLAUlcQzJJKhvlrH7mRZ8vAqn4R77s31opuU2JU
UvZdchAYN/ziZG9xR/btrDE8yrpzwjiXj4Sc1BncqDMb2Sq/HLrxxYi0Tuk7UNDz
hQdjRBEaC8e/yvuPtdGqpoq9WojKeMH6DhBHI/FYhOlcqr4vHyPtiD6ioHwd92LK
iBAiAfQ5jjk+pDu1yIbOC5/GchWVV6hKjgz2qZd3dZPQq7YixRVImHR5roBXiKwm
4w9FutyE5slZZgFjIjrkcg1lp0emHI8osKfl9hf+wNvVXmgo8AZrrmidqgESNe8B
iQtoJi1P+d7Mee4cqlrI
=Yseo
-----END PGP SIGNATURE-----

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Matt Turner]

On Tue, Feb 26, 2013 at 9:29 AM, Rick "Zero_Chaos" Farina
<zerochaos@gentoo.org> wrote:
> In my opinion you can kill the current catalyst 3 branch as nothing uses
> it and afaik it doesn't work.  jmbvicetto should be able to
> authoritatively agree or not.

If Brian's work is to be the basis for future revisions of catalyst,
it should be the master branch. We can branch current master as a
stable branch. The current catalyst2 branch is totally dead, and
overwriting it doesn't cause us to lose anything.

... but the patches should be sent to the list (with git
format-patch/send-email) and reviewed before being committed to
master.

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

On Tue, 2013-02-26 at 11:39 -0800, Matt Turner wrote:
> On Tue, Feb 26, 2013 at 9:29 AM, Rick "Zero_Chaos" Farina
> <zerochaos@gentoo.org> wrote:
> > In my opinion you can kill the current catalyst 3 branch as nothing uses
> > it and afaik it doesn't work.  jmbvicetto should be able to
> > authoritatively agree or not.
> 
> If Brian's work is to be the basis for future revisions of catalyst,
> it should be the master branch. We can branch current master as a
> stable branch. The current catalyst2 branch is totally dead, and
> overwriting it doesn't cause us to lose anything.
> 

I generally agree that my rewrite should become the master branch, but
it could live in a side branch for a while longer, there are many more
changes needed to clean up the code.  It is likely to be broken at times
some commits by themselves won't be complete, etc...


> ... but the patches should be sent to the list (with git
> format-patch/send-email) and reviewed before being committed to
> master.
> 

I tried using git to send this patch, but it did not work.  I'll look
into how to make it work with email.

But I do question the need to send all patches to this list for review.
Take this patch email for instance.  It was primarily for the last
commit I did to fix the update_seed methods which I found not working
correctly. I also mentioned I had rebased all my other work, condensing
where it made sense...

 Do you see any code suggestions, questions and/or comments in regards
to it... NO

All mails in this thread are about everything else :/

I will continue to spam the list with all my rebased commits, which I
intended to do when I had more time.  I sent this one off this morning
before I went to work.  I know Jorge was interested in the changes.  I'm
back now, so will manually prepare more patch mails.

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Matt Turner]

On Tue, Feb 26, 2013 at 6:04 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
>  Do you see any code suggestions, questions and/or comments in regards
> to it... NO

Sorry, I've had other things to do today in the 10 intervening hours
since the patch was sent. Have a bit of patience.

Re: [gentoo-catalyst] patch, fix broken seed stage update

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]

On Tue, Feb 26, 2013 at 06:04:54PM -0800, Brian Dolbec wrote:
> I tried using git to send this patch, but it did not work.  I'll look
> into how to make it work with email.
> …
> I will continue to spam the list with all my rebased commits, which I
> intended to do when I had more time.  I sent this one off this morning
> before I went to work.  I know Jorge was interested in the changes.  I'm
> back now, so will manually prepare more patch mails.

It looks like you're pasting diffs into your email client, which makes
it harder for others to apply them with `git am`.  When I'm submitting
patches via email, I usually do something like:

  $ git format-patch --cover-letter -v3 -o my-fix $(git merge-base my-fix master)
  $ emacs my-fix/v3-0000-cover-letter.patch
    … fill in subject and summarize changes since v2 …
  $ git send-email --to gentoo-catalyst@lists.gentoo.org \
      --in-reply-to '<message-id-for-v2@me.invalid>' my-fix/v3-*

Where `my-fix` is the name of my feature branch.  The `-v3` option
requires Git ≥1.8.2 for -v/--reroll-count, before that you had to use
something like `--subject-prefix 'PATCH v3'` [1].

By default, `send-email` will try and send the emails with `sendmail`.
If you don't have a working version of sendmail installed, you can
configure it to use SMTP instead [2,3].

By using the format-patch/send-email pair, you avoid copy/paste errors
and tedium.  You also have author information and commit messages
preserved in a form that `git am` understands.  Wins for everybody ;).

Cheers,
Trevor

[1]: http://kernel.org/pub/software/scm/git/docs/git-format-patch.html
[2]: http://kernel.org/pub/software/scm/git/docs/git-send-email.html
[3]: http://kernel.org/pub/software/scm/git/docs/git-config.html

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Matt Turner]

On Tue, Feb 26, 2013 at 8:20 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
> --- a/doc/catalyst-spec.5.txt
> +++ b/doc/catalyst-spec.5.txt
> @@ -138,9 +138,19 @@ it should update the seed stage or not (valid values: `yes no`).
>  *update_seed_command*::
>  This is an optional command to pass to emerge for updating the seed
>  stage (example: `--update dev-libs/mpfr dev-libs/mpc dev-libs/gmp`)
> -If not specified, catalyst will update gcc deps.
> +If not specified, catalyst will update gcc's deps, and rebuild gcc if any of
> +it's deps are updated with a new version. Even if it itself is not updated.
> +This prevents gcc breakage when it's dependency lib sonames have changed.
>  This setting requires enabling update_seed.
>
> +*update_seed_system*::
> +This is an optional setting supported by stage1 to tell catalyst if
> +it should update the seed's system packages or not (valid values: `yes no`).
> +This is run after any update_seed_command, updating any remaining upgradable
> +system packages.
> +This setting requires enabling update_seed.

I don't see any need for an update_seed_system option. If someone
wants to run emerge @system then they can do:

update_seed_command: --options @system

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

On Tue, 2013-02-26 at 18:37 -0800, Matt Turner wrote:
> On Tue, Feb 26, 2013 at 8:20 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
> > --- a/doc/catalyst-spec.5.txt
> > +++ b/doc/catalyst-spec.5.txt
> > @@ -138,9 +138,19 @@ it should update the seed stage or not (valid values: `yes no`).
> >  *update_seed_command*::
> >  This is an optional command to pass to emerge for updating the seed
> >  stage (example: `--update dev-libs/mpfr dev-libs/mpc dev-libs/gmp`)
> > -If not specified, catalyst will update gcc deps.
> > +If not specified, catalyst will update gcc's deps, and rebuild gcc if any of
> > +it's deps are updated with a new version. Even if it itself is not updated.
> > +This prevents gcc breakage when it's dependency lib sonames have changed.
> >  This setting requires enabling update_seed.
> >
> > +*update_seed_system*::
> > +This is an optional setting supported by stage1 to tell catalyst if
> > +it should update the seed's system packages or not (valid values: `yes no`).
> > +This is run after any update_seed_command, updating any remaining upgradable
> > +system packages.
> > +This setting requires enabling update_seed.
> 
> I don't see any need for an update_seed_system option. If someone
> wants to run emerge @system then they can do:
> 
> update_seed_command: --options @system
> 

Well, Jorge last night was saying he wanted -uDN1 @world, but settled
for what was done.

Also if you look at my change for the default update_seed_command.  If
that is just made to @system, the problem I had updating my seed, it
would still be broken.  The problem was that mpc was updated which
changed sonames, so gcc had to be rebuilt or was broken.  Just doing an
emerge -uDN1 @system would not have rebuilt gcc for the new mpc lib.
And most system pkgs can not use EAPI 5 subslots to prevent breakage.

This way gcc will get properly updated, rebuilt if necessary, then if
the user wants, update remaining system pkgs.

It's there, now, only a few lines of code, and for the reasons above,
not as easy to do it all in one command.  We would have to change the
update_seed_command spec option from being a run_merge() to a bash
subshell command.  In that way it can be as complex as the user desires.

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Matt Turner]

On Tue, Feb 26, 2013 at 7:03 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
> On Tue, 2013-02-26 at 18:37 -0800, Matt Turner wrote:
>> On Tue, Feb 26, 2013 at 8:20 AM, Brian Dolbec <dolsen@gentoo.org> wrote:
>> > --- a/doc/catalyst-spec.5.txt
>> > +++ b/doc/catalyst-spec.5.txt
>> > @@ -138,9 +138,19 @@ it should update the seed stage or not (valid values: `yes no`).
>> >  *update_seed_command*::
>> >  This is an optional command to pass to emerge for updating the seed
>> >  stage (example: `--update dev-libs/mpfr dev-libs/mpc dev-libs/gmp`)
>> > -If not specified, catalyst will update gcc deps.
>> > +If not specified, catalyst will update gcc's deps, and rebuild gcc if any of
>> > +it's deps are updated with a new version. Even if it itself is not updated.
>> > +This prevents gcc breakage when it's dependency lib sonames have changed.
>> >  This setting requires enabling update_seed.
>> >
>> > +*update_seed_system*::
>> > +This is an optional setting supported by stage1 to tell catalyst if
>> > +it should update the seed's system packages or not (valid values: `yes no`).
>> > +This is run after any update_seed_command, updating any remaining upgradable
>> > +system packages.
>> > +This setting requires enabling update_seed.
>>
>> I don't see any need for an update_seed_system option. If someone
>> wants to run emerge @system then they can do:
>>
>> update_seed_command: --options @system
>>
>
> Well, Jorge last night was saying he wanted -uDN1 @world, but settled
> for what was done.
>
> Also if you look at my change for the default update_seed_command.  If
> that is just made to @system, the problem I had updating my seed, it
> would still be broken.  The problem was that mpc was updated which
> changed sonames, so gcc had to be rebuilt or was broken.  Just doing an
> emerge -uDN1 @system would not have rebuilt gcc for the new mpc lib.
> And most system pkgs can not use EAPI 5 subslots to prevent breakage.
>
> This way gcc will get properly updated, rebuilt if necessary, then if
> the user wants, update remaining system pkgs.
>
> It's there, now, only a few lines of code, and for the reasons above,
> not as easy to do it all in one command.  We would have to change the
> update_seed_command spec option from being a run_merge() to a bash
> subshell command.  In that way it can be as complex as the user desires.

So from talking on IRC, it seems that the problem that led to this
patch was that a binpkg of gcc linked against an old mpc was used
during stage1, leading to a broken gcc in stage2. Stripping --usepkg
makes sense for this purpose.

Removing --buildpkg I'm not sure about. It seems to me that anything
that is built during the seed-update should be okay to use for the
stage1.

Adding an option for updating @system still seems unnecessary.

Re: [gentoo-catalyst] patch, fix broken seed stage update

[Brian Dolbec]

On Tue, 2013-02-26 at 19:22 -0800, Matt Turner wrote:

> So from talking on IRC, it seems that the problem that led to this
> patch was that a binpkg of gcc linked against an old mpc was used
> during stage1, leading to a broken gcc in stage2. Stripping --usepkg
> makes sense for this purpose.
> 
> Removing --buildpkg I'm not sure about. It seems to me that anything
> that is built during the seed-update should be okay to use for the
> stage1.
> 

Yeah, I thought it should be ok too, but so far opted to leave it out

I thought I'd wait for more feedback.  That's also why I cc'd Zac, since
I didn't know if he was on this list.


> Adding an option for updating @system still seems unnecessary.
> 

 If everyone wants it nuked, is easy todo :)