[gentoo-portage-dev] [PATCH v3] bin/doins.py: implement install -p option (bug 642632)

[Zac Medico]

Bug: https://bugs.gentoo.org/642632
---
[PATCH v3] update _is_install_allowed docstring to specify that source_stat
    should be obtained with stat() rather than lstat()

 bin/doins.py                        | 34 +++++++++++++++++++++++++++++-----
 pym/portage/tests/bin/test_doins.py |  6 ++++--
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/bin/doins.py b/bin/doins.py
index 92e450979..9e6566097 100644
--- a/bin/doins.py
+++ b/bin/doins.py
@@ -107,6 +107,7 @@ def _parse_install_options(
 	parser.add_argument('-g', '--group', default=-1, type=_parse_group)
 	parser.add_argument('-o', '--owner', default=-1, type=_parse_user)
 	parser.add_argument('-m', '--mode', default=0o755, type=_parse_mode)
+	parser.add_argument('-p', '--preserve-timestamps', action='store_true')
 	split_options = shlex.split(options)
 	namespace, remaining = parser.parse_known_args(split_options)
 	# Because parsing '--mode' option is partially supported. If unknown
@@ -139,6 +140,24 @@ def _set_attributes(options, path):
 		os.chmod(path, options.mode)
 
 
+def _set_timestamps(source_stat, dest):
+	"""Apply timestamps from source_stat to dest.
+
+	Args:
+		source_stat: stat result for the source file.
+		dest: path to the dest file.
+	"""
+	os.utime(dest, (source_stat.st_atime, source_stat.st_mtime))
+
+
+if sys.version_info >= (3, 3):
+	def _set_timestamps_ns(source_stat, dest):
+		os.utime(dest, ns=(source_stat.st_atime_ns, source_stat.st_mtime_ns))
+
+	_set_timestamps_ns.__doc__ = _set_timestamps.__doc__
+	_set_timestamps = _set_timestamps_ns
+
+
 class _InsInProcessInstallRunner(object):
 	"""Implements `install` command behavior running in a process."""
 
@@ -168,7 +187,9 @@ class _InsInProcessInstallRunner(object):
 			True on success, otherwise False.
 		"""
 		dest = os.path.join(dest_dir, os.path.basename(source))
-		if not self._is_install_allowed(source, dest):
+		# Raise an exception if stat(source) fails, intentionally.
+		sstat = os.stat(source)
+		if not self._is_install_allowed(source, sstat, dest):
 			return False
 
 		# To emulate the `install` command, remove the dest file in
@@ -187,6 +208,8 @@ class _InsInProcessInstallRunner(object):
 				movefile._copyxattr(
 					source, dest,
 					exclude=self._xattr_exclude)
+			if self._parsed_options.preserve_timestamps:
+				_set_timestamps(sstat, dest)
 		except Exception:
 			logging.exception(
 				'Failed to copy file: '
@@ -195,22 +218,23 @@ class _InsInProcessInstallRunner(object):
 			return False
 		return True
 
-	def _is_install_allowed(self, source, dest):
+	def _is_install_allowed(self, source, source_stat, dest):
 		"""Returns if installing source into dest should work.
 
 		This is to keep compatibility with the `install` command.
 
 		Args:
 			source: path to the source file.
+			source_stat: stat result for the source file, using stat()
+				rather than lstat(), in order to match the `install`
+				command
 			dest: path to the dest file.
 
 		Returns:
 			True if it should succeed.
 		"""
 		# To match `install` command, use stat() for source, while
-		# lstat() for dest. Raise an exception if stat(source) fails,
-		# intentionally.
-		source_stat = os.stat(source)
+		# lstat() for dest.
 		try:
 			dest_lstat = os.lstat(dest)
 		except OSError as e:
diff --git a/pym/portage/tests/bin/test_doins.py b/pym/portage/tests/bin/test_doins.py
index 14d7adfa6..9b6c55d85 100644
--- a/pym/portage/tests/bin/test_doins.py
+++ b/pym/portage/tests/bin/test_doins.py
@@ -38,13 +38,15 @@ class DoIns(setup_env.BinTestCase):
 		self.init()
 		try:
 			env = setup_env.env
-			env['INSOPTIONS'] = '-m0644'
+			env['INSOPTIONS'] = '-pm0644'
 			with open(os.path.join(env['S'], 'test'), 'w'):
 				pass
 			doins('test')
 			st = os.lstat(env['D'] + '/test')
 			if stat.S_IMODE(st.st_mode) != 0o644:
 				raise tests.TestCase.failureException
+			if os.stat(os.path.join(env['S'], 'test')).st_mtime != st.st_mtime:
+				raise tests.TestCase.failureException
 		finally:
 			self.cleanup()
 
@@ -145,7 +147,7 @@ class DoIns(setup_env.BinTestCase):
 			env = setup_env.env
 			# Use an option which doins.py does not know.
 			# Then, fallback to `install` command is expected.
-			env['INSOPTIONS'] = '-p'
+			env['INSOPTIONS'] = '-b'
 			with open(os.path.join(env['S'], 'test'), 'w'):
 				pass
 			doins('test')
-- 
2.13.6