From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 4752F138A1A for ; Wed, 7 Jan 2015 09:31:25 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 071D0E083E; Wed, 7 Jan 2015 09:31:23 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 75AA4E0839 for ; Wed, 7 Jan 2015 09:31:22 +0000 (UTC) Received: from [192.168.43.17] (99.sub-70-209-195.myvzw.com [70.209.195.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: zmedico) by smtp.gentoo.org (Postfix) with ESMTPSA id 8A14B340693 for ; Wed, 7 Jan 2015 09:31:21 +0000 (UTC) Message-ID: <54ACFCE6.8000104@gentoo.org> Date: Wed, 07 Jan 2015 01:31:18 -0800 From: Zac Medico User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org MIME-Version: 1.0 To: gentoo-portage-dev@lists.gentoo.org Subject: Re: [gentoo-portage-dev] [PATCH] Add --sync-submodule (534070) References: <1420013239-22660-1-git-send-email-zmedico@gentoo.org> <20150106205350.4ff4ca75.dolsen@gentoo.org> In-Reply-To: <20150106205350.4ff4ca75.dolsen@gentoo.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-Archives-Salt: 0b52926f-4cde-40df-ad1f-778daded25f5 X-Archives-Hash: 81f0737e96a1e9a01ea25628e525f966 On 01/06/2015 08:53 PM, Brian Dolbec wrote: > On Wed, 31 Dec 2014 00:07:19 -0800 > Zac Medico 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. The paths should be the same as long as the repo conforms to existing conventions. If the repo doesn't conform to existing conventions, then existing tools won't know where to look for the glsa, news, and profiles directories. > 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... Yes, they should use the same mappings, as explained above. > 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. That's a good idea. I'll make a patch for that. -- Thanks, Zac