Re: [gentoo-portage-dev] [PATCH] egencache --update-changelogs: Respect GIT_DIR

[Zac Medico]

On 11/08/2015 11:24 PM, Michał Górny wrote:
> Allow generating ChangeLogs from external git checkout via using
> GIT_DIR, rather than requiring the repository to be git.
> ---
>  bin/egencache | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/bin/egencache b/bin/egencache
> index 51d115a..eeca255 100755
> --- a/bin/egencache
> +++ b/bin/egencache
> @@ -73,8 +73,6 @@ else:
>   else:
>   from repoman.metadata import parse_metadata_use
>  
> -from repoman.vcs.vcs import FindVCS
> -
>  if sys.hexversion >= 0x300:
>   # pylint: disable=W0622
>   long = int
> @@ -886,7 +884,7 @@ class GenChangeLogs(object):
>   repo_path = self._portdb.porttrees[0]
>   os.chdir(repo_path)
>  
> - if 'git' not in FindVCS():
> + if not os.path.isdir(os.environ.get('GIT_DIR', '.git')):
>   writemsg_level(
>   "ERROR: --update-changelogs supported only in 
> git repos\n",
>   level=logging.ERROR, noiselevel=-1)
> 

Since this code runs inside of a generator method now, it would be much
nicer to avoid the chdir call, and instead do something like this:

if not os.path.isdir(
os.environ.get('GIT_DIR', os.path.join(repo_path, '.git'))):
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] egencache: fix results when GIT_DIR is used in the environment.

[Robin H. Johnson]

On Wed, Nov 11, 2015 at 10:09:42PM -0800, Zac Medico wrote:
> On 11/11/2015 02:30 PM, robb...@gentoo.org wrote:
> > From: "Robin H. Johnson" 
> > 
> > If GIT_DIR is used, and .git is outside the root of the checkout, then
> > --work-tree=... needs to be specified, otherwise any Git command that
> > relies on relative directories to the root will be wrong.
> > 
> > Signed-off-by: Robin H. Johnson 
> > ---
> >  bin/egencache | 31 +++
> >  1 file changed, 23 insertions(+), 8 deletions(-)
> Looks good.
Thanks, merged.

> > +   '--relative=%s' % (cp, ),
> This addition could be mentioned in the commit message.
Added to the commit message.

> > def run(self):
> > -   repo_path = self._portdb.porttrees[0]
> > -   os.chdir(repo_path)
> > +   os.chdir(self._repo_path)
> The chdir calls make me nervous, since we should be careful to ensure
> that they are timed correctly when we introduce parallelization. I'd
> love to add a cwd argument for the grab method to pass into Popen, in
> order to eliminate all of the chdir calls.
Agreed; I like your later patch for it.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Infrastructure Lead, Foundation Trustee
E-Mail : robb...@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

[gentoo-portage-dev] [PATCH] FindVCS: support optional cwd argument

[Zac Medico]

Since os.chdir calls may not be safe to use in threads, iterators,
or event-driven code, make FindVCS support a cwd argument which
defaults to the current working directory.
---
 pym/repoman/vcs/vcs.py | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/pym/repoman/vcs/vcs.py b/pym/repoman/vcs/vcs.py
index 49d3058..a463335 100644
--- a/pym/repoman/vcs/vcs.py
+++ b/pym/repoman/vcs/vcs.py
@@ -39,15 +39,25 @@ _FindVCS_data = (
 )
 
 
-def FindVCS():
-   """ Try to figure out in what VCS' working tree we are. """
+def FindVCS(cwd=None):
+   """
+   Try to figure out in what VCS' working tree we are.
+
+   @param cwd: working directory (default is os.getcwd())
+   @type cwd: str
+   @return: list of strings describing the discovered vcs types
+   @rtype: list
+   """
+
+   if cwd is None:
+   cwd = os.getcwd()
 
outvcs = []
 
def seek(depth=None):
""" Seek for VCSes that have a top-level data directory only. 
"""
retvcs = []
-   pathprep = ''
+   pathprep = cwd
 
while depth is None or depth > 0:
for vcs_type in _FindVCS_data:
@@ -70,10 +80,10 @@ def FindVCS():
return retvcs
 
# Level zero VCS-es.
-   if os.path.isdir('CVS'):
+   if os.path.isdir(os.path.join(cwd, 'CVS')):
outvcs.append('cvs')
if os.path.isdir('.svn'):  # <1.7
-   outvcs.append('svn')
+   outvcs.append(os.path.join(cwd, 'svn'))
 
# If we already found one of 'level zeros', just take a quick look
# at the current directory. Otherwise, seek parents till we get
-- 
2.4.9

[gentoo-portage-dev] [PATCH] ebuild: unset all funcs/vars that start with ___

[Mike Frysinger]

Since the __* (two) namespace is reserved, and ___* (three) has rarely
(if ever) been used in ebuilds, we can nuke all funcs/vars that start
with that.  It makes clean up easier for us.
---
 bin/save-ebuild-env.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bin/save-ebuild-env.sh b/bin/save-ebuild-env.sh
index 31c2d60..ddef1fd 100644
--- a/bin/save-ebuild-env.sh
+++ b/bin/save-ebuild-env.sh
@@ -89,7 +89,9 @@ __save_ebuild_env() {
___eapi_has_package_manager_build_user && unset -f 
package_manager_build_user
___eapi_has_package_manager_build_group && unset -f 
package_manager_build_group
 
-   unset -f $(compgen -A function ___eapi_)
+   # Clear out the triple underscore namespace as it is reserved by the PM.
+   unset -f $(compgen -A function ___)
+   unset ${!___*}
 
# portage config variables and variables set directly by portage
unset ACCEPT_LICENSE BAD BRACKET BUILD_PREFIX COLS \
-- 
2.6.2

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 12 Nov 2015 16:58, Zac Medico wrote:
> On 11/12/2015 04:06 PM, Mike Frysinger wrote:
> > from ebuilds/eclasses that have already stopped using __:
> > __do_sed_fix ()
> > ___ECLASS_RECUR_MULTILIB=yes
> > ___ECLASS_RECUR_TOOLCHAIN_FUNCS=yes
> > __versionator_shopt_toggle ()
> > __versionator__test_version_compare ()
> > __versionator__test_version_is_at_least ()
> > 
> > grepping the tree, i see like two packages and one eclass still using __.
> > both of which are trivial to convert.
> 
> Sure, but do we really want to confuse people who might be ignorant of
> this rule? Having functions disappear from the environment without
> warning is very likely to cause confusion...

that already happens to a degree if you happen to use a name that portage
uses itself.  we can add a repoman check, and if we think it comes up enough,
have portage itself warn when it blows away things it didn't register.

> Also, there's the
> element of backward-compatibility for any __* functions in
> /var/db/pkg/*/*/environment.bz2 of users' installed systems that might
> be needed during pkg_prerm and pkg_postrm.

the ones i highlighted are not needed for those purposes.  you're right it
could be a problem, i think the likelihood is low considering how infrequently
these two are used, and how much we push people to use other phases instead.

> >> Also note that some internals have been intentionally preserved in
> >> environment.bz2. For example, __eapi6_src_install exposes the default
> >> src_install implementation, which someone might examine for debugging
> >> purposes.
> > 
> > is that actually useful ?  i can't see how it would be.
> 
> Shrug, probably not (unless there's a bug in a particular
> implementation, and someone wants to go back and check which
> implementation was used for a particular installed package).

if we want to tag that kind of metadata in a build, we should just explicitly
include something like PORTAGE_VERSION.
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: clear __bashpid & __start_distcc from env

[Zac Medico]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 11/12/2015 05:50 PM, Mike Frysinger wrote:
> These are internal funcs that should not be exported into the env. 
> --- bin/save-ebuild-env.sh | 1 + 1 file changed, 1 insertion(+)
> 
> diff --git a/bin/save-ebuild-env.sh b/bin/save-ebuild-env.sh index
> 8036342..31c2d60 100644 --- a/bin/save-ebuild-env.sh +++
> b/bin/save-ebuild-env.sh @@ -76,6 +76,7 @@ __save_ebuild_env() { 
> __ebuild_arg_to_phase __ebuild_phase_funcs default \ __unpack_tar
> __unset_colors \ __source_env_files __try_source
> __check_bash_version \ +  __bashpid __start_distcc \ __eqaquote
> __eqatag \ ${QA_INTERCEPTORS}
> 
> 

Looks good.
- -- 
Thanks,
Zac
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iEYEARECAAYFAlZFTWIACgkQ/ejvha5XGaPqxACeMhcIu3xuCaws4WVJvyQtmN53
p1YAoMHWpl49BZfSZeoE8qBEX3LPeZ3N
=wkFZ
-END PGP SIGNATURE-

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 12 Nov 2015 21:07, Tim Harder wrote:
> On 2015-11-11 14:42, Zac Medico wrote:
> > Please unset all new internal function inside bin/save-ebuild-env.sh.
> > Note that it already uses this line to unset functions beginning with
> > ___eapi:
> >
> >unset -f $(compgen -A function ___eapi_)
> >
> > However, your __eapi functions will not be matched because they only
> > begin with 2 underscores.
> 
> Just to note another approach, pkgcore generates global and per-eapi
> function lists at install time (or uses the generation scripts when
> running from a checkout) so manually tracking lists of functions isn't
> required.
> 
> The main reason I set that up was because I sometimes forgot to add
> functions to a similar list that was previously used. :)
> 
> I'm not overly familiar with portage's bash code so I'm not sure if
> that's feasible with its current structure.

i was thinking of building lists too, but i'm not entirely sure how to
tease that out of portage.
-mike


signature.asc
Description: Digital signature

[gentoo-portage-dev] [PATCH] GenChangeLogs: parallelize remaining git calls, scale linearly (bug 565540)

[Zac Medico]

Move all git calls to the subprocesses, so performance scales linearly
with --jobs.

X-Gentoo-Bug: 565540
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=565540
---
 bin/egencache | 33 ++---
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/bin/egencache b/bin/egencache
index b44ad11..1cc2f3d 100755
--- a/bin/egencache
+++ b/bin/egencache
@@ -781,6 +781,23 @@ class GenChangeLogs(object):
encoding=_encodings['stdio'], errors='strict')
 
def generate_changelog(self, cp):
+
+   os.chdir(os.path.join(self._repo_path, cp))
+   # Determine whether ChangeLog is up-to-date by comparing
+   # the newest commit timestamp with the ChangeLog timestamp.
+   lmod = self.grab(['git', self._work_tree, 'log', 
'--format=%ct', '-1', '.'])
+   if not lmod:
+   # This cp has not been added to the repo.
+   return
+
+   try:
+   cmod = os.stat('ChangeLog').st_mtime
+   except OSError:
+   cmod = 0
+
+   if float(cmod) >= float(lmod):
+   return
+
try:
output = io.open(self._changelog_output,
mode='w', encoding=_encodings['repo.content'],
@@ -913,21 +930,7 @@ class GenChangeLogs(object):
return
 
for cp in self._portdb.cp_all():
-   os.chdir(os.path.join(self._repo_path, cp))
-   # Determine whether ChangeLog is up-to-date by comparing
-   # the newest commit timestamp with the ChangeLog 
timestamp.
-   lmod = self.grab(['git', self._work_tree, 'log', 
'--format=%ct', '-1', '.'])
-   if not lmod:
-   # This cp has not been added to the repo.
-   continue
-
-   try:
-   cmod = os.stat('ChangeLog').st_mtime
-   except OSError:
-   cmod = 0
-
-   if float(cmod) < float(lmod):
-   yield 
AsyncFunction(target=self.generate_changelog, args=[cp])
+   yield AsyncFunction(target=self.generate_changelog, 
args=[cp])
 
def run(self):
return run_main_scheduler(
-- 
2.4.9

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Tim Harder]

On 2015-11-11 14:42, Zac Medico wrote:
> Please unset all new internal function inside bin/save-ebuild-env.sh.
> Note that it already uses this line to unset functions beginning with
> ___eapi:

>unset -f $(compgen -A function ___eapi_)

> However, your __eapi functions will not be matched because they only
> begin with 2 underscores.

Just to note another approach, pkgcore generates global and per-eapi
function lists at install time (or uses the generation scripts when
running from a checkout) so manually tracking lists of functions isn't
required.

The main reason I set that up was because I sometimes forgot to add
functions to a similar list that was previously used. :)

I'm not overly familiar with portage's bash code so I'm not sure if
that's feasible with its current structure.

Tim


signature.asc
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/12/2015 06:07 PM, Tim Harder wrote:
> On 2015-11-11 14:42, Zac Medico wrote:
>> Please unset all new internal function inside bin/save-ebuild-env.sh.
>> Note that it already uses this line to unset functions beginning with
>> ___eapi:
> 
>>unset -f $(compgen -A function ___eapi_)
> 
>> However, your __eapi functions will not be matched because they only
>> begin with 2 underscores.
> 
> Just to note another approach, pkgcore generates global and per-eapi
> function lists at install time (or uses the generation scripts when
> running from a checkout) so manually tracking lists of functions isn't
> required.
> 
> The main reason I set that up was because I sometimes forgot to add
> functions to a similar list that was previously used. :)
> 
> I'm not overly familiar with portage's bash code so I'm not sure if
> that's feasible with its current structure.
> 
> Tim
> 

That seems like a pretty reasonable approach, at least up until PMS
reserved the __ prefix for package managers. I'm pretty sure that Mike
is intend on exploiting this reserved prefix now. :)
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Tim Harder]

On 2015-11-12 21:25, Zac Medico wrote:
> > Just to note another approach, pkgcore generates global and per-eapi
> > function lists at install time (or uses the generation scripts when
> > running from a checkout) so manually tracking lists of functions isn't
> > required.

> That seems like a pretty reasonable approach, at least up until PMS
> reserved the __ prefix for package managers. I'm pretty sure that Mike
> is intend on exploiting this reserved prefix now. :)

I don't think it's always possible to use a prefix for functions that
shouldn't be saved to the env that are internally implemented in the PM
but externally accessible in ebuilds, e.g. insinto, docinto, inherit,
etc.

Tim


signature.asc
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/12/2015 06:33 PM, Tim Harder wrote:
> On 2015-11-12 21:25, Zac Medico wrote:
>>> Just to note another approach, pkgcore generates global and per-eapi
>>> function lists at install time (or uses the generation scripts when
>>> running from a checkout) so manually tracking lists of functions isn't
>>> required.
> 
>> That seems like a pretty reasonable approach, at least up until PMS
>> reserved the __ prefix for package managers. I'm pretty sure that Mike
>> is intend on exploiting this reserved prefix now. :)
> 
> I don't think it's always possible to use a prefix for functions that
> shouldn't be saved to the env that are internally implemented in the PM
> but externally accessible in ebuilds, e.g. insinto, docinto, inherit,
> etc.
> 
> Tim
> 

That's true, I guess we should look into generating the list automatically.
-- 
Thanks,
Zac

[gentoo-portage-dev] [PATCH] egencache: Delay updating Manifests until all other tasks complete

[Michał Górny]

Since thick Manifests can reference other files (ChangeLogs especially),
their generation should be run as the lask task done by egencache,
followed only by timestamp update.
---
 bin/egencache | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/bin/egencache b/bin/egencache
index 51d115a..e02c690 100755
--- a/bin/egencache
+++ b/bin/egencache
@@ -1095,29 +1095,6 @@ def egencache_main(args):
else:
ret.append(gen_cache.returncode)
 
-   if options.update_manifests:
-
-   cp_iter = None
-   if atoms:
-   cp_iter = iter(atoms)
-
-   event_loop = global_event_loop()
-   scheduler = ManifestScheduler(portdb, cp_iter=cp_iter,
-   gpg_cmd=gpg_cmd, gpg_vars=gpg_vars,
-   force_sign_key=force_sign_key,
-   max_jobs=options.jobs,
-   max_load=options.load_average,
-   event_loop=event_loop)
-
-   signum = run_main_scheduler(scheduler)
-   if signum is not None:
-   sys.exit(128 + signum)
-
-   if options.tolerant:
-   ret.append(os.EX_OK)
-   else:
-   ret.append(scheduler.returncode)
-
if options.update_pkg_desc_index:
if repo_config.writable:
writable_location = repo_config.location
@@ -1153,6 +1130,29 @@ def egencache_main(args):
gen_clogs.run()
ret.append(gen_clogs.returncode)
 
+   if options.update_manifests:
+
+   cp_iter = None
+   if atoms:
+   cp_iter = iter(atoms)
+
+   event_loop = global_event_loop()
+   scheduler = ManifestScheduler(portdb, cp_iter=cp_iter,
+   gpg_cmd=gpg_cmd, gpg_vars=gpg_vars,
+   force_sign_key=force_sign_key,
+   max_jobs=options.jobs,
+   max_load=options.load_average,
+   event_loop=event_loop)
+
+   signum = run_main_scheduler(scheduler)
+   if signum is not None:
+   sys.exit(128 + signum)
+
+   if options.tolerant:
+   ret.append(os.EX_OK)
+   else:
+   ret.append(scheduler.returncode)
+
if options.write_timestamp:
timestamp_path = os.path.join(repo_path, 'metadata', 
'timestamp.chk')
try:
-- 
2.6.3

Re: [gentoo-portage-dev] [PATCH] egencache: Delay updating Manifests until all other tasks complete

[Zac Medico]

On 11/12/2015 08:35 AM, Zac Medico wrote:
> On 11/12/2015 07:00 AM, Michał Górny wrote:
>> Since thick Manifests can reference other files (ChangeLogs especially),
>> their generation should be run as the lask task done by egencache,
>> followed only by timestamp update.
> 
> GenPkgDescIndex needs to execute last, since it accesses the metadata.
> 

Actually, the patch looks good.

I was confusing the manifest generation with the metadata generation.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] egencache: Delay updating Manifests until all other tasks complete

[Zac Medico]

On 11/12/2015 07:00 AM, Michał Górny wrote:
> Since thick Manifests can reference other files (ChangeLogs especially),
> their generation should be run as the lask task done by egencache,
> followed only by timestamp update.

GenPkgDescIndex needs to execute last, since it accesses the metadata.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] egencache: Delay updating Manifests until all other tasks complete

[Brian Dolbec]

On Thu, 12 Nov 2015 08:56:34 -0800
Zac Medico  wrote:

> On 11/12/2015 08:35 AM, Zac Medico wrote:
> > On 11/12/2015 07:00 AM, Michał Górny wrote:  
> >> Since thick Manifests can reference other files (ChangeLogs
> >> especially), their generation should be run as the lask task done
> >> by egencache, followed only by timestamp update.  
> > 
> > GenPkgDescIndex needs to execute last, since it accesses the
> > metadata. 
> 
> Actually, the patch looks good.
> 
> I was confusing the manifest generation with the metadata generation.

actually, looks like Robin made a bug for this after you submitted this
patch.

Can you edit the commit message, add the bug please. Bug 565626

-- 
Brian Dolbec