Re: [gentoo-portage-dev] [PATCH] bin/doins.py: implement install -p option (bug 642632)
On 12/29/2017 02:13 PM, Mike Gilbert wrote: > On Fri, Dec 29, 2017 at 3:35 PM, Zac Medico wrote: >> + if self._parsed_options.preserve_timestamps: >> + if sys.version_info >= (3, 3): >> + os.utime(dest, >> ns=(sstat.st_mtime_ns, sstat.st_mtime_ns)) >> + else: >> + os.utime(dest, (sstat.st_mtime, >> sstat.st_mtime)) > > It looks like you are copying mtime into both mtime and atime on the > new file. Is that a mistake? > Yeah, that's fixed in v2. -- Thanks, Zac
[gentoo-portage-dev] [PATCH v2] bin/doins.py: implement install -p option (bug 642632)
Bug: https://bugs.gentoo.org/642632
---
[PATCH v2] fix to copy atime, and split out _set_timestamps function
bin/doins.py| 28 +---
pym/portage/tests/bin/test_doins.py | 6 --
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/bin/doins.py b/bin/doins.py
index 92e450979..0d03d8fb2 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,8 @@ 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):
+ 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 +207,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,13 +217,14 @@ 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.
dest: path to the dest file.
Returns:
@@ -210,7 +233,6 @@ class _InsInProcessInstallRunner(object):
# 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)
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.
Re: [gentoo-portage-dev] [PATCH] bin/doins.py: implement install -p option (bug 642632)
On Fri, Dec 29, 2017 at 3:35 PM, Zac Medico wrote: > + if self._parsed_options.preserve_timestamps: > + if sys.version_info >= (3, 3): > + os.utime(dest, ns=(sstat.st_mtime_ns, > sstat.st_mtime_ns)) > + else: > + os.utime(dest, (sstat.st_mtime, > sstat.st_mtime)) It looks like you are copying mtime into both mtime and atime on the new file. Is that a mistake?
[gentoo-portage-dev] [PATCH] bin/doins.py: implement install -p option (bug 642632)
Bug: https://bugs.gentoo.org/642632
---
bin/doins.py| 13 ++---
pym/portage/tests/bin/test_doins.py | 4 ++--
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/bin/doins.py b/bin/doins.py
index 92e450979..dceffee83 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
@@ -168,7 +169,8 @@ 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):
+ 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 +189,11 @@ class _InsInProcessInstallRunner(object):
movefile._copyxattr(
source, dest,
exclude=self._xattr_exclude)
+ if self._parsed_options.preserve_timestamps:
+ if sys.version_info >= (3, 3):
+ os.utime(dest, ns=(sstat.st_mtime_ns,
sstat.st_mtime_ns))
+ else:
+ os.utime(dest, (sstat.st_mtime,
sstat.st_mtime))
except Exception:
logging.exception(
'Failed to copy file: '
@@ -195,13 +202,14 @@ 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.
dest: path to the dest file.
Returns:
@@ -210,7 +218,6 @@ class _InsInProcessInstallRunner(object):
# 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)
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..dd40abf6e 100644
--- a/pym/portage/tests/bin/test_doins.py
+++ b/pym/portage/tests/bin/test_doins.py
@@ -38,7 +38,7 @@ 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')
@@ -145,7 +145,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
