proj/pkgcore/pkgcore:master commit in: tests/ebuild/, src/pkgcore/ebuild/, /
"Arthur Zamarin" <[email protected]>
| Newsgroups | gmane.linux.gentoo.cvs |
|---|---|
| Message-ID | <1786698041.2ed82e0a09112c9ce97fcfdabfa93d0d1455f079.arthurzam@gentoo> |
commit: 2ed82e0a09112c9ce97fcfdabfa93d0d1455f079
Author: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
AuthorDate: Fri Aug 14 09:00:41 2026 +0000
Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
CommitDate: Fri Aug 14 09:00:41 2026 +0000
URL: https://gitweb.gentoo.org/proj/pkgcore/pkgcore.git/commit/?id=2ed82e0a
manifest: decide freshness from more than the distfile names
Whether a Manifest was current was answered by comparing its DIST entries
against SRC_URI, and nothing else. That is the valid only for a thin
manifest holding exactly the expected distfiles.
A thick manifest also covers the package directory, which the comparison
never looked at, so a package with no distfiles matched trivially and was
skipped: pkgdev manifest wrote no Manifest at all, said nothing, and exited
0, leaving the package masked by corruption. Editing an ebuild was worse,
the file stayed behind recording the previous size and hashes, silently
wrong, until someone passed --force.
A thin repo had its own version of this. A thick Manifest copied in kept its
AUX/EBUILD/MISC entries forever, since the distfiles matched and the rest was
never examined.
So only take the shortcut for a thin manifest that has nothing left to
checksum and carries no thick entries; regenerate thick ones every time,
having no cheaper way to know. To keep that affordable, Manifest.update
now renders the contents first and writes only when they differ, returning
whether it wrote, so an unchanged Manifest costs a comparison rather than
a rewrite and no work gets reported when there is none. Removing a stale
Manifest says so too.
Resolves: https://github.com/pkgcore/pkgdev/issues/108
Resolves: https://github.com/pkgcore/pkgdev/issues/194
Resolves: https://github.com/pkgcore/pkgdev/issues/78
Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org>
NEWS.rst | 14 +++++++++
src/pkgcore/ebuild/digest.py | 67 ++++++++++++++++++++++++----------------
src/pkgcore/ebuild/repository.py | 16 +++++++---
tests/ebuild/test_digest.py | 42 +++++++++++++++++++++++++
4 files changed, 109 insertions(+), 30 deletions(-)
diff --git a/NEWS.rst b/NEWS.rst
index a59e4a587..775b98f07 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -9,6 +9,20 @@ pkgcore 0.12.39 (unreleased)
Fixes
~~~~~
+- manifest generation: a thick Manifest never being written or refreshed
+ unless ``--force`` was passed. Whether the file was current was decided by
+ comparing distfile names alone, but a thick Manifest also covers the package
+ directory, so a repo with no distfiles got no Manifest at all and an edited
+ ebuild left a Manifest silently recording the old size and hashes. Thick
+ manifests are now always regenerated, and ``Manifest.update`` leaves an
+ already correct file untouched, so no work is reported when there is none
+ (Arthur Zamarin, pkgdev#108, pkgdev#194)
+
+- manifest generation: a thick Manifest copied into a repo using thin
+ manifests keeping its ``AUX``/``EBUILD``/``MISC`` entries forever, as the
+ distfiles matched and nothing else was looked at. Such a file is now
+ rewritten down to its ``DIST`` entries (Arthur Zamarin, pkgdev#78)
+
- ``pkgcore.const``: fix the user config, cache and data paths being taken
verbatim from an XDG base dir variable set to an empty (or relative) value,
as is common in containers and root shells, leaving them relative to the
diff --git a/src/pkgcore/ebuild/digest.py b/src/pkgcore/ebuild/digest.py
index 8f8b7856b..e5adf49fb 100644
--- a/src/pkgcore/ebuild/digest.py
+++ b/src/pkgcore/ebuild/digest.py
@@ -17,15 +17,14 @@ from ..package import errors
from . import cpv
-def _write_manifest(handle, chf, filename, chksums):
- """Convenient, internal method for writing manifests"""
+def _manifest_line(chf: str, filename: str, chksums) -> str:
+ """Convenient, internal method for rendering a manifest entry"""
+ chksums = dict(chksums)
size = chksums.pop("size")
- handle.write(f"{chf.upper()} {filename} {size}")
+ line = f"{chf.upper()} {filename} {size}"
for other_chf in sorted(chksums):
- handle.write(
- f" {other_chf.upper()} {get_handler(other_chf).long2str(chksums[other_chf])}"
- )
- handle.write("\n")
+ line += f" {other_chf.upper()} {get_handler(other_chf).long2str(chksums[other_chf])}"
+ return line + "\n"
def convert_chksums(iterable):
@@ -117,15 +116,16 @@ class Manifest:
self._dist, self._aux, self._ebuild, self._misc = data
self._sourced = True
- def update(self, fetchables, chfs=None):
+ def update(self, fetchables, chfs=None) -> bool:
"""Update the related Manifest file.
:param fetchables: fetchables of the package
+ :return: True if the file was written, False if it was already current
"""
if self.thin and not fetchables:
# Manifest files aren't necessary with thin manifests and no distfiles
- return
+ return False
_key_sort = operator.itemgetter(0)
@@ -156,24 +156,39 @@ class Manifest:
)
d[pathname] = dict(obj.chksums)
- with open(self.path, "w") as handle:
- # write it in alphabetical order; aux gets flushed now.
- for path, chksums in sorted(aux.items(), key=_key_sort):
- _write_manifest(handle, "AUX", path, chksums)
-
- # next dist...
- for fetchable in sorted(fetchables, key=operator.attrgetter("filename")):
- _write_manifest(
- handle,
- "DIST",
- os.path.basename(fetchable.filename),
- dict(fetchable.chksums),
- )
+ # write it in alphabetical order; aux gets flushed now.
+ data = "".join(
+ _manifest_line("AUX", path, chksums)
+ for path, chksums in sorted(aux.items(), key=_key_sort)
+ )
+
+ # next dist...
+ data += "".join(
+ _manifest_line(
+ "DIST", os.path.basename(fetchable.filename), fetchable.chksums
+ )
+ for fetchable in sorted(fetchables, key=operator.attrgetter("filename"))
+ )
+
+ # then ebuild and misc
+ for mtype, inst in (("EBUILD", ebuild), ("MISC", misc)):
+ data += "".join(
+ _manifest_line(mtype, path, chksum)
+ for path, chksum in sorted(inst.items(), key=_key_sort)
+ )
+
+ # leave an already correct Manifest alone
+ try:
+ with open(self.path) as handle:
+ if handle.read() == data:
+ return False
+ except OSError:
+ pass
- # then ebuild and misc
- for mtype, inst in (("EBUILD", ebuild), ("MISC", misc)):
- for path, chksum in sorted(inst.items(), key=_key_sort):
- _write_manifest(handle, mtype, path, chksum)
+ with open(self.path, "w") as handle:
+ handle.write(data)
+ self._sourced = False
+ return True
@property
def aux_files(self):
diff --git a/src/pkgcore/ebuild/repository.py b/src/pkgcore/ebuild/repository.py
index e4031499f..f8946ed27 100644
--- a/src/pkgcore/ebuild/repository.py
+++ b/src/pkgcore/ebuild/repository.py
@@ -106,6 +106,7 @@ class repo_operations(_repo_ops.operations):
if os.path.exists(manifest.path):
try:
os.remove(manifest.path)
+ observer.info(f"removing manifest: {key}::{self.repo.repo_id}")
except OSError as exc:
observer.error(
"failed removing old manifest: "
@@ -114,8 +115,15 @@ class repo_operations(_repo_ops.operations):
ret.add(key)
continue
- # Manifest file is current and not forcing a refresh
- if not force and manifest.distfiles.keys() == pkgdir_fetchables.keys():
+ # Manifest file is current and not forcing a refresh; thick manifests
+ # also cover the pkgdir, so they are always regenerated
+ if (
+ not force
+ and manifest_config.thin
+ and not fetchables
+ and manifest.distfiles.keys() == pkgdir_fetchables.keys()
+ and not (manifest.aux_files or manifest.ebuilds or manifest.misc)
+ ):
continue
# fetch distfiles
@@ -162,8 +170,8 @@ class repo_operations(_repo_ops.operations):
if required_chksums.issubset(fetchable.chksums)
}
all_fetchables.update(fetchables)
- observer.info(f"generating manifest: {key}::{self.repo.repo_id}")
- manifest.update(sorted(all_fetchables.values()), chfs=write_chksums)
+ if manifest.update(sorted(all_fetchables.values()), chfs=write_chksums):
+ observer.info(f"generating manifest: {key}::{self.repo.repo_id}")
# edge case: If all ebuilds for a package were masked bad,
# then it was filtered out of the iterator for the above loop,
diff --git a/tests/ebuild/test_digest.py b/tests/ebuild/test_digest.py
index bf54c094e..9da74ce98 100644
--- a/tests/ebuild/test_digest.py
+++ b/tests/ebuild/test_digest.py
@@ -100,3 +100,45 @@ class TestManifest:
class TestManifestDataSource(TestManifest):
convert_source = staticmethod(lambda x: local_source(x))
+
+
+class TestManifestUpdate:
+ chfs = ("size", "blake2b")
+
+ def mk_pkgdir(self, tmp_path):
+ (tmp_path / "pkg-1.ebuild").write_text("EAPI=8\n")
+ (tmp_path / "metadata.xml").write_text("<pkgmetadata/>\n")
+ (tmp_path / "files").mkdir()
+ (tmp_path / "files" / "a.patch").write_text("patch\n")
+ return digest.Manifest(
+ str(tmp_path / "Manifest"), thin=False, allow_missing=True
+ )
+
+ def test_thick_covers_pkgdir(self, tmp_path):
+ manifest = self.mk_pkgdir(tmp_path)
+ assert manifest.update((), chfs=self.chfs)
+ data = (tmp_path / "Manifest").read_text()
+ assert "EBUILD pkg-1.ebuild " in data
+ assert "MISC metadata.xml " in data
+ assert "AUX a.patch " in data
+
+ def test_current_manifest_left_alone(self, tmp_path):
+ manifest = self.mk_pkgdir(tmp_path)
+ assert manifest.update((), chfs=self.chfs)
+ # nothing changed, so nothing to write
+ assert not manifest.update((), chfs=self.chfs)
+
+ def test_stale_manifest_rewritten(self, tmp_path):
+ manifest = self.mk_pkgdir(tmp_path)
+ assert manifest.update((), chfs=self.chfs)
+ before = (tmp_path / "Manifest").read_text()
+ (tmp_path / "pkg-1.ebuild").write_text("EAPI=8\n# changed\n")
+ assert manifest.update((), chfs=self.chfs)
+ assert (tmp_path / "Manifest").read_text() != before
+
+ def test_thin_without_distfiles(self, tmp_path):
+ manifest = digest.Manifest(
+ str(tmp_path / "Manifest"), thin=True, allow_missing=True
+ )
+ assert not manifest.update((), chfs=self.chfs)
+ assert not (tmp_path / "Manifest").exists()