[gentoo-portage-dev] [PATCH v3] egencache: add --stable-mtime option

[Zac Medico]

Since the Manifest "stable mtime" behavior could have undiscovered
bugs, disable it by default, and add a corresponding egencache option.

Suggested-by: Michał Górny <mgorny@g.o>
---
[PATCH v3] fixes the commit message and adds a comment block to
Manifest.write in order to document subtle issues involving the mtime
of self.pkgdir that must be accounted for.

 bin/egencache                                      |  6 +++++-
 man/egencache.1                                    |  3 +++
 pym/portage/manifest.py                            | 23 +++++++++++++++++-----
 .../ebuild/_parallel_manifest/ManifestProcess.py   |  6 ++++--
 .../ebuild/_parallel_manifest/ManifestScheduler.py |  7 +++++--
 .../ebuild/_parallel_manifest/ManifestTask.py      |  8 +++++---
 6 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/bin/egencache b/bin/egencache
index 7e3387e..07665e8 100755
--- a/bin/egencache
+++ b/bin/egencache
@@ -120,6 +120,9 @@ def parse_args(args):
 		choices=('y', 'n'),
 		metavar="<y|n>",
 		help="manually override layout.conf sign-manifests setting")
+	common.add_argument("--stable-mtime",
+		action="store_true",
+		help="apply stable mtime to generated manifests (for rsync)")
 	common.add_argument("--strict-manifests",
 		choices=('y', 'n'),
 		metavar="<y|n>",
@@ -1151,7 +1154,8 @@ def egencache_main(args):
 			force_sign_key=force_sign_key,
 			max_jobs=options.jobs,
 			max_load=options.load_average,
-			event_loop=event_loop)
+			event_loop=event_loop,
+			manifest_kwargs=dict(stable_mtime=options.stable_mtime))
 
 		signum = run_main_scheduler(scheduler)
 		if signum is not None:
diff --git a/man/egencache.1 b/man/egencache.1
index 7fd17c2..081e8c1 100644
--- a/man/egencache.1
+++ b/man/egencache.1
@@ -100,6 +100,9 @@ Manually override layout.conf sign-manifests setting.
 .BR "\-\-strict\-manifests< y | n >"
 Manually override "strict" FEATURES setting.
 .TP
+.BR "\-\-stable\-mtime"
+Apply stable mtime to generated manifests (for rsync).
+.TP
 .BR "\-\-thin\-manifests< y | n >"
 Manually override layout.conf thin-manifests setting.
 .TP
diff --git a/pym/portage/manifest.py b/pym/portage/manifest.py
index f696f84..6efd6e5 100644
--- a/pym/portage/manifest.py
+++ b/pym/portage/manifest.py
@@ -128,7 +128,7 @@ class Manifest(object):
 	def __init__(self, pkgdir, distdir=None, fetchlist_dict=None,
 		manifest1_compat=DeprecationWarning, from_scratch=False, thin=False,
 		allow_missing=False, allow_create=True, hashes=None,
-		find_invalid_path_char=None):
+		find_invalid_path_char=None, stable_mtime=False):
 		""" Create new Manifest instance for package in pkgdir.
 		    Do not parse Manifest file if from_scratch == True (only for internal use)
 			The fetchlist_dict parameter is required only for generation of
@@ -145,6 +145,7 @@ class Manifest(object):
 			find_invalid_path_char = _find_invalid_path_char
 		self._find_invalid_path_char = find_invalid_path_char
 		self.pkgdir = _unicode_decode(pkgdir).rstrip(os.sep) + os.sep
+		self.stable_mtime = stable_mtime
 		self.fhashdict = {}
 		self.hashes = set()
 
@@ -283,7 +284,16 @@ class Manifest(object):
 			myentries = list(self._createManifestEntries())
 			update_manifest = True
 			preserved_stats = {}
-			preserved_stats[self.pkgdir.rstrip(os.sep)] = os.stat(self.pkgdir)
+			if self.stable_mtime:
+				# The pre-existing mtime of self.pkgdir is included in the
+				# max mtime calculation in order to account for anything
+				# that may have been renamed or removed in this directory
+				# (including the Manifest itself). Note that the mtime of
+				# this directory will be always be bumped as a side-effect
+				# of writing the Manifest (since write_atomic uses a rename
+				# operation for atomicity), therefore it must be preserved
+				# before the Manifest is written.
+				preserved_stats[self.pkgdir.rstrip(os.sep)] = os.stat(self.pkgdir)
 			if myentries and not force:
 				try:
 					f = io.open(_unicode_encode(self.getFullname(),
@@ -291,7 +301,8 @@ class Manifest(object):
 						mode='r', encoding=_encodings['repo.content'],
 						errors='replace')
 					oldentries = list(self._parseManifestLines(f))
-					preserved_stats[self.getFullname()] = os.fstat(f.fileno())
+					if self.stable_mtime:
+						preserved_stats[self.getFullname()] = os.fstat(f.fileno())
 					f.close()
 					if len(oldentries) == len(myentries):
 						update_manifest = False
@@ -313,7 +324,8 @@ class Manifest(object):
 					# non-empty for all currently known use cases.
 					write_atomic(self.getFullname(), "".join("%s\n" %
 						_unicode(myentry) for myentry in myentries))
-					self._apply_max_mtime(preserved_stats, myentries)
+					if self.stable_mtime:
+						self._apply_max_mtime(preserved_stats, myentries)
 					rval = True
 				else:
 					# With thin manifest, there's no need to have
@@ -450,7 +462,8 @@ class Manifest(object):
 			fetchlist_dict=self.fetchlist_dict, from_scratch=True,
 			thin=self.thin, allow_missing=self.allow_missing,
 			allow_create=self.allow_create, hashes=self.hashes,
-			find_invalid_path_char=self._find_invalid_path_char)
+			find_invalid_path_char=self._find_invalid_path_char,
+			stable_mtime=self.stable_mtime)
 		pn = os.path.basename(self.pkgdir.rstrip(os.path.sep))
 		cat = self._pkgdir_category()
 
diff --git a/pym/portage/package/ebuild/_parallel_manifest/ManifestProcess.py b/pym/portage/package/ebuild/_parallel_manifest/ManifestProcess.py
index 44e2576..01595a3 100644
--- a/pym/portage/package/ebuild/_parallel_manifest/ManifestProcess.py
+++ b/pym/portage/package/ebuild/_parallel_manifest/ManifestProcess.py
@@ -10,14 +10,16 @@ from portage.util._async.ForkProcess import ForkProcess
 
 class ManifestProcess(ForkProcess):
 
-	__slots__ = ("cp", "distdir", "fetchlist_dict", "repo_config")
+	__slots__ = ("cp", "distdir", "fetchlist_dict", "manifest_kwargs",
+		"repo_config")
 
 	MODIFIED = 16
 
 	def _run(self):
 		mf = self.repo_config.load_manifest(
 			os.path.join(self.repo_config.location, self.cp),
-			self.distdir, fetchlist_dict=self.fetchlist_dict)
+			self.distdir, fetchlist_dict=self.fetchlist_dict,
+			**(self.manifest_kwargs or {}))
 
 		try:
 			mf.create(assumeDistHashesAlways=True)
diff --git a/pym/portage/package/ebuild/_parallel_manifest/ManifestScheduler.py b/pym/portage/package/ebuild/_parallel_manifest/ManifestScheduler.py
index 38ac482..8a1c1d0 100644
--- a/pym/portage/package/ebuild/_parallel_manifest/ManifestScheduler.py
+++ b/pym/portage/package/ebuild/_parallel_manifest/ManifestScheduler.py
@@ -12,11 +12,13 @@ from .ManifestTask import ManifestTask
 class ManifestScheduler(AsyncScheduler):
 
 	def __init__(self, portdb, cp_iter=None,
-		gpg_cmd=None, gpg_vars=None, force_sign_key=None, **kwargs):
+		gpg_cmd=None, gpg_vars=None, force_sign_key=None,
+		manifest_kwargs=None, **kwargs):
 
 		AsyncScheduler.__init__(self, **kwargs)
 
 		self._portdb = portdb
+		self._manifest_kwargs = manifest_kwargs
 
 		if cp_iter is None:
 			cp_iter = self._iter_every_cp()
@@ -79,7 +81,8 @@ class ManifestScheduler(AsyncScheduler):
 				yield ManifestTask(cp=cp, distdir=distdir,
 					fetchlist_dict=fetchlist_dict, repo_config=repo_config,
 					gpg_cmd=self._gpg_cmd, gpg_vars=self._gpg_vars,
-					force_sign_key=self._force_sign_key)
+					force_sign_key=self._force_sign_key,
+					manifest_kwargs=self._manifest_kwargs)
 
 	def _task_exit(self, task):
 
diff --git a/pym/portage/package/ebuild/_parallel_manifest/ManifestTask.py b/pym/portage/package/ebuild/_parallel_manifest/ManifestTask.py
index 0ee2b91..fb5e16e 100644
--- a/pym/portage/package/ebuild/_parallel_manifest/ManifestTask.py
+++ b/pym/portage/package/ebuild/_parallel_manifest/ManifestTask.py
@@ -18,8 +18,8 @@ from .ManifestProcess import ManifestProcess
 
 class ManifestTask(CompositeTask):
 
-	__slots__ = ("cp", "distdir", "fetchlist_dict", "gpg_cmd",
-		"gpg_vars", "repo_config", "force_sign_key", "_manifest_path")
+	__slots__ = ("cp", "distdir", "fetchlist_dict", "force_sign_key",
+		"gpg_cmd", "gpg_vars", "manifest_kwargs", "repo_config", "_manifest_path")
 
 	_PGP_HEADER = b"BEGIN PGP SIGNED MESSAGE"
 	_manifest_line_re = re.compile(r'^(%s) ' % "|".join(MANIFEST2_IDENTIFIERS))
@@ -30,7 +30,9 @@ class ManifestTask(CompositeTask):
 		self._manifest_path = os.path.join(self.repo_config.location,
 			self.cp, "Manifest")
 		manifest_proc = ManifestProcess(cp=self.cp, distdir=self.distdir,
-			fetchlist_dict=self.fetchlist_dict, repo_config=self.repo_config,
+			fetchlist_dict=self.fetchlist_dict,
+			manifest_kwargs=self.manifest_kwargs,
+			repo_config=self.repo_config,
 			scheduler=self.scheduler)
 		self._start_task(manifest_proc, self._manifest_proc_exit)
 
-- 
2.4.10