Re: [gentoo-portage-dev] [PATCH] Add --sync-submodule <glsa|news|profiles> (534070)

[Brian Dolbec]

On Wed, 31 Dec 2014 00:07:19 -0800
Zac Medico <zmedico@g.o> wrote:

> This adds support for a new --sync-submodule option to both emerge and
> emaint. When this option is used with the sync action, only the
> selected submodules are synced. Each submodule is referenced using an
> abstract identifier, which serves to hide the implementation details
> involving the precise locations of specific submodules within each
> repository.
>
> Currently, --sync-submodule has no effect for sync protocols other
> than rsync, but the new SyncBase._get_submodule_paths() method will
> be useful for implementing support in other SyncBase subclasses.
>
> X-Gentoo-Bug: 534070
> X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=534070
> ---
>
> portage.process.spawn(command, diff --git
> a/pym/portage/sync/syncbase.py b/pym/portage/sync/syncbase.py index
> 94d4aab..fcde51f 100644 --- a/pym/portage/sync/syncbase.py
> +++ b/pym/portage/sync/syncbase.py
> @@ -12,6 +12,11 @@ import os
>  import portage
>  from portage.util import writemsg_level
>
> +_SUBMODULE_PATH_MAP = {
> +	'glsa': 'metadata/glsa',
> +	'news': 'metadata/news',
> +	'profiles': 'profiles',
> +}
>
>

It mostly looks good, but...

I don't like the idea of hard-coding this in the base class.

I think this should be configurable per-repo or at least per repo type.

This patch clears the way for other repo types to have sub-modules, but
will they be the same mapping for a git tree with those as
sub-modules?  Maybe I'm just not following how the system works
properly...

Alternatively...if the above is wrong/not feasible.  In the
sync/__init__.py you have the choices hard-coded there independently of
these definitions above.  I would prefer that one get it's options from
the other. Since syncbase.py is not gauranteed to be loaded, I would
think that the above be declared in the sync module's __init__.py and
imported or passed in here.  Then the:

"choices": ("glsa", "news", "profiles"),

could become:

"choices": list(_SUBMODULE_PATH_MAP)

Then there is only one place to edit as options or code change.





-- 
Brian Dolbec <dolsen>