From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 473C5138359 for ; Fri, 30 Oct 2020 16:08:17 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 5BC02E087B; Fri, 30 Oct 2020 16:08:16 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 2C141E087B for ; Fri, 30 Oct 2020 16:08:16 +0000 (UTC) Received: by mail-ej1-f41.google.com with SMTP id oq3so7469072ejb.7 for ; Fri, 30 Oct 2020 09:08:14 -0700 (PDT) X-Gm-Message-State: AOAM5337OUHb7JmYKAVg2plezsozVJnfyVfYmN3mhfNSZfer4pLdwyNe ehPcU0EXoBDYCZmBet6fAIvSeAZshuUQ/lJGM0U= X-Google-Smtp-Source: ABdhPJwzNqnGsSSQk165VuzczQByeS02trr7lsCkN8bUA9B/gAfOszZ/vQpt0wiyQFZ+NTbV8y/umBFjc0QZJ450sYg= X-Received: by 2002:a17:906:c407:: with SMTP id u7mr3157256ejz.206.1604074091059; Fri, 30 Oct 2020 09:08:11 -0700 (PDT) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-catalyst@lists.gentoo.org Reply-to: gentoo-catalyst@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 References: <914cce4739709511771f27bed773c7e4e01c3c9a.camel@rohde-schwarz.com> In-Reply-To: From: Matt Turner Date: Fri, 30 Oct 2020 12:07:56 -0400 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [gentoo-catalyst] Re: [PATCH 2/2] Move from PORTDIR_OVERLAY to repos.conf To: gentoo-catalyst@lists.gentoo.org Content-Type: text/plain; charset="UTF-8" X-Archives-Salt: 496aa9e2-565d-4e1f-a172-1b47e34cd16d X-Archives-Hash: 41bc87f21ad943716ff25478c4c2e19c On Sun, Oct 18, 2020 at 11:15 AM Felix Bier wrote: > > This commit fixes the following issues: > > * The PORTDIR_OVERLAY variable has been deprecated by Gentoo. > > With this commit, the variable is no longer written to the > generated make.conf. Instead, a config file > /etc/portage/repos.conf/.conf > is generated for each overlay. The repo name is read from the > overlay using the portage API. Internally, portage parses > metadata/layout.conf and profiles/repo_name to obtain the name. > > References: > https://wiki.gentoo.org/wiki//etc/portage/make.conf > https://wiki.gentoo.org/wiki//etc/portage/repos.conf > > * All overlays were copied into the same target directory. If the > same file name occurred in multiple overlays, the last overlay > would overwrite all previous files with this name. In particular, > only the metadata/layout.conf of the last overlay was retained, > so it was not possible to reference the other overlays e.g. via > the masters entry in the layout.conf or the portage-2 syntax > for specifying a parent profile from another overlay. Also, > this created problems when the overlays contained ebuilds > for the same package, but with differing versions, because > after copying, the target directory contained both versions of the > ebuild but only the manifest file of the last overlay. > > With this commit, each overlay is copied into a separate > sub-directory, e.g. /var/gentoo/repos/local//. I see this /var/gentoo/repos path used as an example in a few places but not actually in any code. I think our example should match the default structure these days, e.g., /var/db/repos/. > This directory is referenced via the location entry in the > generated /etc/portage/repos.conf/.conf. > --- > catalyst/base/stagebase.py | 72 ++++++++++++++++++++++++++++---------- > catalyst/defaults.py | 1 + > catalyst/support.py | 18 ++++++++++ > 3 files changed, 72 insertions(+), 19 deletions(-) > > diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py > index ac0f4f24..3d4f2a76 100644 > --- a/catalyst/base/stagebase.py > +++ b/catalyst/base/stagebase.py > @@ -1,4 +1,5 @@ > > +import configparser > import copy > import os > import platform > @@ -17,7 +18,8 @@ from DeComp.compress import CompressMap > from catalyst import log > from catalyst.defaults import (confdefaults, MOUNT_DEFAULTS, PORT_LOGDIR_CLEAN) > from catalyst.support import (CatalystError, file_locate, normpath, > - cmd, read_makeconf, ismount, file_check) > + cmd, read_makeconf, get_repo_name, ismount, > + file_check) > from catalyst.base.targetbase import TargetBase > from catalyst.base.clearbase import ClearBase > from catalyst.base.genbase import GenBase > @@ -821,17 +823,48 @@ class StageBase(TargetBase, ClearBase, GenBase): > env=self.env) > self.resume.enable("setup_confdir") > > + def get_repo_conf_path(self, repo_name): > + """ Construct repo conf path: /etc/portage/repos.conf/{name}.conf """ > + return normpath(os.path.join(self.settings['repos_conf'], repo_name + ".conf")) I would rather we use pathlib and not add new uses of normpath. > + > + def get_overlay_location(self, repo_name): > + """ Construct overlay repo path: /var/gentoo/repos/local/{name} """ > + return normpath(os.path.join(self.settings['local_overlay'], repo_name)) > + > + def write_repo_conf(self, repo_name, config): > + """ Write ConfigParser to {chroot}/etc/portage/repo.conf/{name}.conf """ > + repo_conf = self.get_repo_conf_path(repo_name) > + chroot_repo_conf = self.settings['chroot_path'] + repo_conf > + log.info('Creating repo config %s.', chroot_repo_conf) > + ensure_dirs(os.path.dirname(chroot_repo_conf)) Same thing: I'd rather use pathlib and use Path.mkdir and not add new uses of ensure_dirs. > + > + try: > + with open(chroot_repo_conf, 'w') as myf: Let's rename myf. The 'my' prefix pattern in catalyst is not a good one. Even just 'f' or 'file' would be better, IMO. > + config.write(myf) > + except OSError as e: > + raise CatalystError('Could not write {}: {}'.format( > + chroot_repo_conf, e)) from e > + > def portage_overlay(self): Feel free to rename things, like this function if their name no longer matches the current terminology. > - """ We copy the contents of our overlays to /usr/local/portage """ > + """ We copy the contents of our overlays to /var/gentoo/repos/local/{name} """ Let's not use a real path here. It only stands to confuse the reader. > if "portage_overlay" in self.settings: > for x in self.settings["portage_overlay"]: > if os.path.exists(x): > - log.info('Copying overlay dir %s', x) > - ensure_dirs( > - self.settings['chroot_path'] + self.settings['local_overlay']) > - cmd("cp -a " + x + "/* " + self.settings["chroot_path"] + > - self.settings["local_overlay"], > - env=self.env) > + name = get_repo_name(x) > + > + location = self.get_overlay_location(name) > + config = configparser.ConfigParser() > + config[name] = {'location': location} > + self.write_repo_conf(name, config) > + > + chroot_location = normpath( > + self.settings['chroot_path'] + location) > + log.info('Copying overlay dir %s to %s', > + x, chroot_location) > + ensure_dirs(chroot_location) > + cmd('cp -a ' + x + '/* ' + chroot_location, env=self.env) > + else: > + log.warning('Skipping missing overlay %s.', x) > > def root_overlay(self): > """ Copy over the root_overlay """ > @@ -1080,12 +1113,6 @@ class StageBase(TargetBase, ClearBase, GenBase): > varname = x.split('_')[1].upper() > myf.write(f'{varname}="{self.settings[x]}"\n') > > - if setup: > - # Setup the portage overlay > - if "portage_overlay" in self.settings: > - myf.write('PORTDIR_OVERLAY="%s"\n' % > - self.settings["local_overlay"]) > - > # Set default locale for system responses. #478382 > myf.write( > '\n' > @@ -1157,11 +1184,18 @@ class StageBase(TargetBase, ClearBase, GenBase): > log.warning("You've been hacking. Clearing target patches: %s", target) > clear_path(target) > > - # Remove our overlay > - overlay = normpath( > - self.settings["chroot_path"] + self.settings["local_overlay"]) > - if os.path.exists(overlay): > - clear_path(overlay) > + # Remove our overlays > + if "portage_overlay" in self.settings: > + for repo_path in self.settings["portage_overlay"]: > + repo_name = get_repo_name(repo_path) > + > + repo_conf = self.get_repo_conf_path(repo_name) > + chroot_repo_conf = self.settings["chroot_path"] + repo_conf > + clear_path(chroot_repo_conf) > + > + location = self.get_overlay_location(repo_name) > + chroot_location = self.settings['chroot_path'] + location > + clear_path(chroot_location) > > if "sticky-config" not in self.settings["options"]: > # re-write the make.conf to be sure it is clean > diff --git a/catalyst/defaults.py b/catalyst/defaults.py > index c153fcc4..9660a7f3 100644 > --- a/catalyst/defaults.py > +++ b/catalyst/defaults.py > @@ -38,6 +38,7 @@ confdefaults = { > "local_overlay": "/var/db/repos/local", > "port_conf": "/etc/portage", > "make_conf": "%(port_conf)s/make.conf", > + "repos_conf": "%(port_conf)s/repos.conf", > "options": set(), > "pkgdir": "/var/cache/binpkgs", > "port_tmpdir": "/var/tmp/portage", > diff --git a/catalyst/support.py b/catalyst/support.py > index a6a6854a..b8069c7d 100644 > --- a/catalyst/support.py > +++ b/catalyst/support.py > @@ -7,6 +7,8 @@ import shutil > import time > from subprocess import Popen > > +from portage.repository.config import RepoConfig > + > from catalyst import log > > BASH_BINARY = "/bin/bash" > @@ -179,6 +181,22 @@ def read_makeconf(mymakeconffile): > return makeconf > > > +def get_repo_name(repo_path): > + """ Get the name of the repo at the given repo_path. > + > + References: > + https://wiki.gentoo.org/wiki/Repository_format/profiles/repo_name > + https://wiki.gentoo.org/wiki/Repository_format/metadata/layout.conf#repo-name > + """ > + > + repo_config = RepoConfig(None, {"location": repo_path}) > + > + if repo_config.missing_repo_name: > + raise CatalystError("Missing name in repository {}".format(repo_path)) I'd prefer to use f-strings unless there's a good reason not to.