copy: Reduce calls to stat() in chown_recursive

This reduces the number of times `stat()` or `chown()` are called by the
copy module. Reductions come from

- combining loops for owner and group, thus not looping twice
- not looping over the list of directories returned by `os.walk()`, each
  directory is already handled when `os.walk()` descends into it

Possible further reductions could be achieved by
- exiting early from the check_mode loop
- Replacing `m.set_owner_if_different()` & `m.set_group_if_different()`
  calls with a single method.

I propose to do these separately, if there is interest.
pull/75741/head
Alex Willmer 3 years ago
parent a3b58fb67c
commit 4e654c7f05

@ -0,0 +1,3 @@
minor_changes:
- copy - Reduce the number of disk accesses required to set the owner and
group of copied files and directories

@ -340,62 +340,34 @@ def chown_recursive(path, module):
owner = module.params['owner'] owner = module.params['owner']
group = module.params['group'] group = module.params['group']
if owner is not None: if owner is None and group is None:
if not module.check_mode: return changed
for dirpath, dirnames, filenames in os.walk(path):
owner_changed = module.set_owner_if_different(dirpath, owner, False) if module.check_mode:
if owner_changed is True: uid = pwd.getpwnam(owner).pw_uid if owner is not None else None
changed = owner_changed gid = grp.getgrnam(group).gr_gid if group is not None else None
for dir in [os.path.join(dirpath, d) for d in dirnames]:
owner_changed = module.set_owner_if_different(dir, owner, False) for dirpath, dummy, filenames in os.walk(path):
if owner_changed is True: stat = os.stat(dirpath)
changed = owner_changed changed = changed or (uid is not None and uid != stat.st_uid)
for file in [os.path.join(dirpath, f) for f in filenames]: changed = changed or (gid is not None and gid != stat.st_gid)
owner_changed = module.set_owner_if_different(file, owner, False)
if owner_changed is True: for filename in filenames:
changed = owner_changed filepath = os.path.join(dirpath, filename)
else: stat = os.stat(filepath)
uid = pwd.getpwnam(owner).pw_uid changed = changed or (uid is not None and uid != stat.st_uid)
for dirpath, dirnames, filenames in os.walk(path): changed = changed or (gid is not None and gid != stat.st_gid)
owner_changed = (os.stat(dirpath).st_uid != uid)
if owner_changed is True: return changed
changed = owner_changed
for dir in [os.path.join(dirpath, d) for d in dirnames]: for dirpath, dummy, filenames in os.walk(path):
owner_changed = (os.stat(dir).st_uid != uid) changed = module.set_owner_if_different(dirpath, owner, changed)
if owner_changed is True: changed = module.set_group_if_different(dirpath, group, changed)
changed = owner_changed
for file in [os.path.join(dirpath, f) for f in filenames]: for filename in filenames:
owner_changed = (os.stat(file).st_uid != uid) filepath = os.path.join(dirpath, filename)
if owner_changed is True: changed = module.set_owner_if_different(filepath, owner, changed)
changed = owner_changed changed = module.set_group_if_different(filepath, group, changed)
if group is not None:
if not module.check_mode:
for dirpath, dirnames, filenames in os.walk(path):
group_changed = module.set_group_if_different(dirpath, group, False)
if group_changed is True:
changed = group_changed
for dir in [os.path.join(dirpath, d) for d in dirnames]:
group_changed = module.set_group_if_different(dir, group, False)
if group_changed is True:
changed = group_changed
for file in [os.path.join(dirpath, f) for f in filenames]:
group_changed = module.set_group_if_different(file, group, False)
if group_changed is True:
changed = group_changed
else:
gid = grp.getgrnam(group).gr_gid
for dirpath, dirnames, filenames in os.walk(path):
group_changed = (os.stat(dirpath).st_gid != gid)
if group_changed is True:
changed = group_changed
for dir in [os.path.join(dirpath, d) for d in dirnames]:
group_changed = (os.stat(dir).st_gid != gid)
if group_changed is True:
changed = group_changed
for file in [os.path.join(dirpath, f) for f in filenames]:
group_changed = (os.stat(file).st_gid != gid)
if group_changed is True:
changed = group_changed
return changed return changed

Loading…
Cancel
Save