diff --git a/lib/ansible/modules/files/copy.py b/lib/ansible/modules/files/copy.py index a2516e0b07..42bdb95470 100644 --- a/lib/ansible/modules/files/copy.py +++ b/lib/ansible/modules/files/copy.py @@ -283,7 +283,7 @@ def main(): # not checking because of daisy chain to file module argument_spec=dict( src=dict(type='path'), - original_basename=dict(type='str'), # used to handle 'dest is a directory' via template, a slight hack + _original_basename=dict(type='str'), # used to handle 'dest is a directory' via template, a slight hack content=dict(type='str', no_log=True), dest=dict(type='path', required=True), backup=dict(type='bool', default=False), @@ -307,7 +307,7 @@ def main(): b_dest = to_bytes(dest, errors='surrogate_or_strict') backup = module.params['backup'] force = module.params['force'] - original_basename = module.params.get('original_basename', None) + _original_basename = module.params.get('_original_basename', None) validate = module.params.get('validate', None) follow = module.params['follow'] remote_src = module.params['remote_src'] @@ -344,8 +344,8 @@ def main(): ) # Special handling for recursive copy - create intermediate dirs - if original_basename and dest.endswith(os.sep): - dest = os.path.join(dest, original_basename) + if _original_basename and dest.endswith(os.sep): + dest = os.path.join(dest, _original_basename) b_dest = to_bytes(dest, errors='surrogate_or_strict') dirname = os.path.dirname(dest) b_dirname = to_bytes(dirname, errors='surrogate_or_strict') @@ -367,8 +367,8 @@ def main(): if os.path.isdir(b_dest): basename = os.path.basename(src) - if original_basename: - basename = original_basename + if _original_basename: + basename = _original_basename dest = os.path.join(dest, basename) b_dest = to_bytes(dest, errors='surrogate_or_strict') diff --git a/lib/ansible/modules/files/file.py b/lib/ansible/modules/files/file.py index 09b8ed0291..45d15982b9 100644 --- a/lib/ansible/modules/files/file.py +++ b/lib/ansible/modules/files/file.py @@ -191,11 +191,11 @@ def additional_parameter_handling(params): if (params['state'] not in ("link", "absent") and os.path.isdir(to_bytes(params['path'], errors='surrogate_or_strict'))): basename = None - # original_basename is used by other modules that depend on file - if params['original_basename']: - basename = params['original_basename'] - elif params['src'] is not None: - basename = os.path.basename(params['src']) + # _original_basename is used by other modules that depend on file + if params['_original_basename']: + basename = params['_original_basename'] + #elif params['src'] is not None: + # basename = os.path.basename(params['src']) if basename: params['path'] = params['path'] = os.path.join(params['path'], basename) @@ -513,18 +513,18 @@ def ensure_symlink(path, src, follow, force): elif prev_state == 'hard': changed = True if not force: - raise AnsibleModuleError(results={'msg': 'Cannot link, different hard link exists at destination', + raise AnsibleModuleError(results={'msg': 'Cannot link because a hard link exists at destination', 'dest': path, 'src': src}) elif prev_state == 'file': changed = True if not force: - raise AnsibleModuleError(results={'msg': 'Cannot link, %s exists at destination' % prev_state, + raise AnsibleModuleError(results={'msg': 'Cannot link because a file exists at destination', 'dest': path, 'src': src}) elif prev_state == 'directory': changed = True if os.path.exists(b_path): if not force: - raise AnsibleModuleError(results={'msg': 'Cannot link, different hard link exists at destination', + raise AnsibleModuleError(results={'msg': 'Cannot link because a file exists at destination', 'dest': path, 'src': src}) else: raise AnsibleModuleError(results={'msg': 'unexpected position reached', 'dest': path, 'src': src}) @@ -575,27 +575,35 @@ def ensure_hardlink(path, src, follow, force): prev_state = get_state(b_path) file_args = module.load_file_common_arguments(module.params) - # source is both the source of a symlink or an informational passing of the src for a template module - # or copy module, even if this module never uses it, it is needed to key off some things - if src is None: - # Note: Bug: if hard link exists, we shouldn't need to check this - raise AnsibleModuleError(results={'msg': 'src and dest are required for creating hardlinks'}) + # src is the source of a hardlink. We require it if we are creating a new hardlink + if src is None and not os.path.exists(b_path): + raise AnsibleModuleError(results={'msg': 'src and dest are required for creating new hardlinks'}) + # Toshio: Begin suspect block + # I believe that this block of code is wrong for hardlinks. + # src may be relative. + # If it is relative, it should be relative to the cwd (so just use abspath). + # This is different from symlinks where src is relative to the symlink's path. + + # Why must src be an absolute path? if not os.path.isabs(b_src): - raise AnsibleModuleError(results={'msg': "absolute paths are required"}) + raise AnsibleModuleError(results={'msg': "src must be an absolute path"}) + # If this is a link, then it can't be a dir so why is it in the conditional? if not os.path.islink(b_path) and os.path.isdir(b_path): relpath = path else: b_relpath = os.path.dirname(b_path) relpath = to_native(b_relpath, errors='strict') + # Why? This does nothing because src was checked to be absolute above? absrc = os.path.join(relpath, src) b_absrc = to_bytes(absrc, errors='surrogate_or_strict') if not force and not os.path.exists(b_absrc): raise AnsibleModuleError(results={'msg': 'src file does not exist, use "force=yes" if you' ' really want to create the link: %s' % absrc, 'path': path, 'src': src}) + # Toshio: end suspect block diff = initial_diff(path, 'hard', prev_state) changed = False @@ -676,7 +684,7 @@ def main(): argument_spec=dict( state=dict(choices=['file', 'directory', 'link', 'hard', 'touch', 'absent'], default=None), path=dict(aliases=['dest', 'name'], required=True, type='path'), - original_basename=dict(required=False), # Internal use only, for recursive ops + _original_basename=dict(required=False), # Internal use only, for recursive ops recurse=dict(default=False, type='bool'), force=dict(required=False, default=False, type='bool'), # Note: Should not be in file_common_args in future follow=dict(required=False, default=True, type='bool'), # Note: Different default than file_common_args diff --git a/lib/ansible/modules/files/unarchive.py b/lib/ansible/modules/files/unarchive.py index 438da154cd..7796f3f06f 100644 --- a/lib/ansible/modules/files/unarchive.py +++ b/lib/ansible/modules/files/unarchive.py @@ -787,7 +787,6 @@ def main(): # not checking because of daisy chain to file module argument_spec=dict( src=dict(type='path', required=True), - original_basename=dict(type='str'), # used to handle 'dest is a directory' via template, a slight hack dest=dict(type='path', required=True), remote_src=dict(type='bool', default=False), creates=dict(type='path'), diff --git a/lib/ansible/modules/windows/win_copy.ps1 b/lib/ansible/modules/windows/win_copy.ps1 index e142e5a5a2..01718ea1b9 100644 --- a/lib/ansible/modules/windows/win_copy.ps1 +++ b/lib/ansible/modules/windows/win_copy.ps1 @@ -25,7 +25,7 @@ $src = Get-AnsibleParam -obj $params -name "src" -type "path" -failifempty ($cop $dest = Get-AnsibleParam -obj $params -name "dest" -type "path" -failifempty $true # used in single mode -$original_basename = Get-AnsibleParam -obj $params -name "original_basename" -type "str" +$original_basename = Get-AnsibleParam -obj $params -name "_original_basename" -type "str" # used in query and remote mode $force = Get-AnsibleParam -obj $params -name "force" -type "bool" -default $true diff --git a/lib/ansible/modules/windows/win_file.ps1 b/lib/ansible/modules/windows/win_file.ps1 index 5ff30982ab..f6441b095c 100644 --- a/lib/ansible/modules/windows/win_file.ps1 +++ b/lib/ansible/modules/windows/win_file.ps1 @@ -16,7 +16,7 @@ $path = Get-AnsibleParam -obj $params -name "path" -type "path" -failifempty $tr $state = Get-AnsibleParam -obj $params -name "state" -type "str" -validateset "absent","directory","file","touch" # used in template/copy when dest is the path to a dir and source is a file -$original_basename = Get-AnsibleParam -obj $params -name "original_basename" -type "str" +$original_basename = Get-AnsibleParam -obj $params -name "_original_basename" -type "str" if ((Test-Path -Path $path -PathType Container) -and ($null -ne $original_basename)) { $path = Join-Path -Path $path -ChildPath $original_basename } diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index f8e95c8811..6fb394bd96 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -134,13 +134,7 @@ class ActionModule(ActionBase): for opt in ['remote_src', 'regexp', 'delimiter', 'ignore_hidden', 'decrypt']: if opt in new_module_args: del new_module_args[opt] - - new_module_args.update( - dict( - dest=dest, - original_basename=os.path.basename(src), - ) - ) + new_module_args['dest'] = dest if path_checksum != dest_stat['checksum']: diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index f81a4620fd..b0502bfff4 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -39,7 +39,7 @@ from ansible.utils.hashing import checksum # Supplement the FILE_COMMON_ARGUMENTS with arguments that are specific to file # FILE_COMMON_ARGUMENTS contains things that are not arguments of file so remove those as well REAL_FILE_ARGS = frozenset(FILE_COMMON_ARGUMENTS.keys()).union( - ('state', 'path', 'original_basename', 'recurse', 'force', + ('state', 'path', '_original_basename', 'recurse', 'force', '_diff_peek', 'src')).difference( ('content', 'decrypt', 'backup', 'remote_src', 'regexp', 'delimiter', 'directory_mode', 'unsafe_writes')) @@ -304,7 +304,7 @@ class ActionModule(ActionBase): dict( src=tmp_src, dest=dest, - original_basename=source_rel, + _original_basename=source_rel, follow=follow ) ) @@ -338,7 +338,7 @@ class ActionModule(ActionBase): new_module_args.update( dict( dest=dest, - original_basename=source_rel, + _original_basename=source_rel, recurse=False, state='file', ) diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 8686c2ac3a..c221af8fa9 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -90,33 +90,19 @@ class ActionModule(ActionBase): # handle diff mode client side # handle check mode client side - if not remote_src: - # fix file permissions when the copy is done as a different user - self._fixup_perms2((self._connection._shell.tmpdir, tmp_src)) - # Build temporary module_args. - new_module_args = self._task.args.copy() - new_module_args.update( - dict( - src=tmp_src, - original_basename=os.path.basename(source), - ), - ) - - else: - new_module_args = self._task.args.copy() - new_module_args.update( - dict( - original_basename=os.path.basename(source), - ), - ) - - # remove action plugin only key + # remove action plugin only keys + new_module_args = self._task.args.copy() for key in ('decrypt',): if key in new_module_args: del new_module_args[key] - # execute the unarchive module now, with the updated args - result.update(self._execute_module(module_args=new_module_args, task_vars=task_vars)) + if not remote_src: + # fix file permissions when the copy is done as a different user + self._fixup_perms2((self._connection._shell.tmpdir, tmp_src)) + new_module_args['src'] = tmp_src + + # execute the unarchive module now, with the updated args + result.update(self._execute_module(module_args=new_module_args, task_vars=task_vars)) except AnsibleAction as e: result.update(e.result) finally: diff --git a/lib/ansible/plugins/action/win_copy.py b/lib/ansible/plugins/action/win_copy.py index da70973978..df85e14a1a 100644 --- a/lib/ansible/plugins/action/win_copy.py +++ b/lib/ansible/plugins/action/win_copy.py @@ -274,7 +274,7 @@ class ActionModule(ActionBase): dict( dest=dest, src=tmp_src, - original_basename=source_rel, + _original_basename=source_rel, _copy_mode="single" ) )