1 |
On Sun, Oct 18, 2020 at 11:15 AM Felix Bier |
2 |
<Felix.Bier@×××××××××××××.com> wrote: |
3 |
> |
4 |
> This commit fixes the following issues: |
5 |
> |
6 |
> * The PORTDIR_OVERLAY variable has been deprecated by Gentoo. |
7 |
> |
8 |
> With this commit, the variable is no longer written to the |
9 |
> generated make.conf. Instead, a config file |
10 |
> /etc/portage/repos.conf/<repo-name>.conf |
11 |
> is generated for each overlay. The repo name is read from the |
12 |
> overlay using the portage API. Internally, portage parses |
13 |
> metadata/layout.conf and profiles/repo_name to obtain the name. |
14 |
> |
15 |
> References: |
16 |
> https://wiki.gentoo.org/wiki//etc/portage/make.conf |
17 |
> https://wiki.gentoo.org/wiki//etc/portage/repos.conf |
18 |
> |
19 |
> * All overlays were copied into the same target directory. If the |
20 |
> same file name occurred in multiple overlays, the last overlay |
21 |
> would overwrite all previous files with this name. In particular, |
22 |
> only the metadata/layout.conf of the last overlay was retained, |
23 |
> so it was not possible to reference the other overlays e.g. via |
24 |
> the masters entry in the layout.conf or the portage-2 syntax |
25 |
> for specifying a parent profile from another overlay. Also, |
26 |
> this created problems when the overlays contained ebuilds |
27 |
> for the same package, but with differing versions, because |
28 |
> after copying, the target directory contained both versions of the |
29 |
> ebuild but only the manifest file of the last overlay. |
30 |
> |
31 |
> With this commit, each overlay is copied into a separate |
32 |
> sub-directory, e.g. /var/gentoo/repos/local/<repo-name>/. |
33 |
|
34 |
I see this /var/gentoo/repos path used as an example in a few places |
35 |
but not actually in any code. I think our example should match the |
36 |
default structure these days, e.g., /var/db/repos/<repo-name>. |
37 |
|
38 |
> This directory is referenced via the location entry in the |
39 |
> generated /etc/portage/repos.conf/<repo-name>.conf. |
40 |
> --- |
41 |
> catalyst/base/stagebase.py | 72 ++++++++++++++++++++++++++++---------- |
42 |
> catalyst/defaults.py | 1 + |
43 |
> catalyst/support.py | 18 ++++++++++ |
44 |
> 3 files changed, 72 insertions(+), 19 deletions(-) |
45 |
> |
46 |
> diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py |
47 |
> index ac0f4f24..3d4f2a76 100644 |
48 |
> --- a/catalyst/base/stagebase.py |
49 |
> +++ b/catalyst/base/stagebase.py |
50 |
> @@ -1,4 +1,5 @@ |
51 |
> |
52 |
> +import configparser |
53 |
> import copy |
54 |
> import os |
55 |
> import platform |
56 |
> @@ -17,7 +18,8 @@ from DeComp.compress import CompressMap |
57 |
> from catalyst import log |
58 |
> from catalyst.defaults import (confdefaults, MOUNT_DEFAULTS, PORT_LOGDIR_CLEAN) |
59 |
> from catalyst.support import (CatalystError, file_locate, normpath, |
60 |
> - cmd, read_makeconf, ismount, file_check) |
61 |
> + cmd, read_makeconf, get_repo_name, ismount, |
62 |
> + file_check) |
63 |
> from catalyst.base.targetbase import TargetBase |
64 |
> from catalyst.base.clearbase import ClearBase |
65 |
> from catalyst.base.genbase import GenBase |
66 |
> @@ -821,17 +823,48 @@ class StageBase(TargetBase, ClearBase, GenBase): |
67 |
> env=self.env) |
68 |
> self.resume.enable("setup_confdir") |
69 |
> |
70 |
> + def get_repo_conf_path(self, repo_name): |
71 |
> + """ Construct repo conf path: /etc/portage/repos.conf/{name}.conf """ |
72 |
> + return normpath(os.path.join(self.settings['repos_conf'], repo_name + ".conf")) |
73 |
|
74 |
I would rather we use pathlib and not add new uses of normpath. |
75 |
|
76 |
> + |
77 |
> + def get_overlay_location(self, repo_name): |
78 |
> + """ Construct overlay repo path: /var/gentoo/repos/local/{name} """ |
79 |
> + return normpath(os.path.join(self.settings['local_overlay'], repo_name)) |
80 |
> + |
81 |
> + def write_repo_conf(self, repo_name, config): |
82 |
> + """ Write ConfigParser to {chroot}/etc/portage/repo.conf/{name}.conf """ |
83 |
> + repo_conf = self.get_repo_conf_path(repo_name) |
84 |
> + chroot_repo_conf = self.settings['chroot_path'] + repo_conf |
85 |
> + log.info('Creating repo config %s.', chroot_repo_conf) |
86 |
> + ensure_dirs(os.path.dirname(chroot_repo_conf)) |
87 |
|
88 |
Same thing: I'd rather use pathlib and use Path.mkdir and not add new |
89 |
uses of ensure_dirs. |
90 |
|
91 |
> + |
92 |
> + try: |
93 |
> + with open(chroot_repo_conf, 'w') as myf: |
94 |
|
95 |
Let's rename myf. The 'my' prefix pattern in catalyst is not a good |
96 |
one. Even just 'f' or 'file' would be better, IMO. |
97 |
|
98 |
> + config.write(myf) |
99 |
> + except OSError as e: |
100 |
> + raise CatalystError('Could not write {}: {}'.format( |
101 |
> + chroot_repo_conf, e)) from e |
102 |
> + |
103 |
> def portage_overlay(self): |
104 |
|
105 |
Feel free to rename things, like this function if their name no longer |
106 |
matches the current terminology. |
107 |
|
108 |
> - """ We copy the contents of our overlays to /usr/local/portage """ |
109 |
> + """ We copy the contents of our overlays to /var/gentoo/repos/local/{name} """ |
110 |
|
111 |
Let's not use a real path here. It only stands to confuse the reader. |
112 |
|
113 |
> if "portage_overlay" in self.settings: |
114 |
> for x in self.settings["portage_overlay"]: |
115 |
> if os.path.exists(x): |
116 |
> - log.info('Copying overlay dir %s', x) |
117 |
> - ensure_dirs( |
118 |
> - self.settings['chroot_path'] + self.settings['local_overlay']) |
119 |
> - cmd("cp -a " + x + "/* " + self.settings["chroot_path"] + |
120 |
> - self.settings["local_overlay"], |
121 |
> - env=self.env) |
122 |
> + name = get_repo_name(x) |
123 |
> + |
124 |
> + location = self.get_overlay_location(name) |
125 |
> + config = configparser.ConfigParser() |
126 |
> + config[name] = {'location': location} |
127 |
> + self.write_repo_conf(name, config) |
128 |
> + |
129 |
> + chroot_location = normpath( |
130 |
> + self.settings['chroot_path'] + location) |
131 |
> + log.info('Copying overlay dir %s to %s', |
132 |
> + x, chroot_location) |
133 |
> + ensure_dirs(chroot_location) |
134 |
> + cmd('cp -a ' + x + '/* ' + chroot_location, env=self.env) |
135 |
> + else: |
136 |
> + log.warning('Skipping missing overlay %s.', x) |
137 |
> |
138 |
> def root_overlay(self): |
139 |
> """ Copy over the root_overlay """ |
140 |
> @@ -1080,12 +1113,6 @@ class StageBase(TargetBase, ClearBase, GenBase): |
141 |
> varname = x.split('_')[1].upper() |
142 |
> myf.write(f'{varname}="{self.settings[x]}"\n') |
143 |
> |
144 |
> - if setup: |
145 |
> - # Setup the portage overlay |
146 |
> - if "portage_overlay" in self.settings: |
147 |
> - myf.write('PORTDIR_OVERLAY="%s"\n' % |
148 |
> - self.settings["local_overlay"]) |
149 |
> - |
150 |
> # Set default locale for system responses. #478382 |
151 |
> myf.write( |
152 |
> '\n' |
153 |
> @@ -1157,11 +1184,18 @@ class StageBase(TargetBase, ClearBase, GenBase): |
154 |
> log.warning("You've been hacking. Clearing target patches: %s", target) |
155 |
> clear_path(target) |
156 |
> |
157 |
> - # Remove our overlay |
158 |
> - overlay = normpath( |
159 |
> - self.settings["chroot_path"] + self.settings["local_overlay"]) |
160 |
> - if os.path.exists(overlay): |
161 |
> - clear_path(overlay) |
162 |
> + # Remove our overlays |
163 |
> + if "portage_overlay" in self.settings: |
164 |
> + for repo_path in self.settings["portage_overlay"]: |
165 |
> + repo_name = get_repo_name(repo_path) |
166 |
> + |
167 |
> + repo_conf = self.get_repo_conf_path(repo_name) |
168 |
> + chroot_repo_conf = self.settings["chroot_path"] + repo_conf |
169 |
> + clear_path(chroot_repo_conf) |
170 |
> + |
171 |
> + location = self.get_overlay_location(repo_name) |
172 |
> + chroot_location = self.settings['chroot_path'] + location |
173 |
> + clear_path(chroot_location) |
174 |
> |
175 |
> if "sticky-config" not in self.settings["options"]: |
176 |
> # re-write the make.conf to be sure it is clean |
177 |
> diff --git a/catalyst/defaults.py b/catalyst/defaults.py |
178 |
> index c153fcc4..9660a7f3 100644 |
179 |
> --- a/catalyst/defaults.py |
180 |
> +++ b/catalyst/defaults.py |
181 |
> @@ -38,6 +38,7 @@ confdefaults = { |
182 |
> "local_overlay": "/var/db/repos/local", |
183 |
> "port_conf": "/etc/portage", |
184 |
> "make_conf": "%(port_conf)s/make.conf", |
185 |
> + "repos_conf": "%(port_conf)s/repos.conf", |
186 |
> "options": set(), |
187 |
> "pkgdir": "/var/cache/binpkgs", |
188 |
> "port_tmpdir": "/var/tmp/portage", |
189 |
> diff --git a/catalyst/support.py b/catalyst/support.py |
190 |
> index a6a6854a..b8069c7d 100644 |
191 |
> --- a/catalyst/support.py |
192 |
> +++ b/catalyst/support.py |
193 |
> @@ -7,6 +7,8 @@ import shutil |
194 |
> import time |
195 |
> from subprocess import Popen |
196 |
> |
197 |
> +from portage.repository.config import RepoConfig |
198 |
> + |
199 |
> from catalyst import log |
200 |
> |
201 |
> BASH_BINARY = "/bin/bash" |
202 |
> @@ -179,6 +181,22 @@ def read_makeconf(mymakeconffile): |
203 |
> return makeconf |
204 |
> |
205 |
> |
206 |
> +def get_repo_name(repo_path): |
207 |
> + """ Get the name of the repo at the given repo_path. |
208 |
> + |
209 |
> + References: |
210 |
> + https://wiki.gentoo.org/wiki/Repository_format/profiles/repo_name |
211 |
> + https://wiki.gentoo.org/wiki/Repository_format/metadata/layout.conf#repo-name |
212 |
> + """ |
213 |
> + |
214 |
> + repo_config = RepoConfig(None, {"location": repo_path}) |
215 |
> + |
216 |
> + if repo_config.missing_repo_name: |
217 |
> + raise CatalystError("Missing name in repository {}".format(repo_path)) |
218 |
|
219 |
I'd prefer to use f-strings unless there's a good reason not to. |