Rename original_basename parameter in various file-related modules

original_basename is a private internal parameter so name it
_original_basename to reflect that it is internal.

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):
+    if _original_basename and dest.endswith(os.sep):
-        dest = os.path.join(dest, original_basename)
+        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:
+        if _original_basename:
-            basename = original_basename
+            basename = _original_basename
         dest = os.path.join(dest, basename)
         b_dest = to_bytes(dest, errors='surrogate_or_strict')
 

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
+        # _original_basename is used by other modules that depend on file
-        if params['original_basename']:
+        if params['_original_basename']:
-            basename = params['original_basename']
+            basename = params['_original_basename']
-        elif params['src'] is not None:
+        #elif params['src'] is not None:
-            basename = os.path.basename(params['src'])
+        #    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
+    # src is the source of a hardlink.  We require it if we are creating a new hardlink
-    # or copy module, even if this module never uses it, it is needed to key off some things
+    if src is None and not os.path.exists(b_path):
-    if src is None:
+        raise AnsibleModuleError(results={'msg': 'src and dest are required for creating new hardlinks'})
-        # 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'})
 
+    # 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

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'),

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

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
 }

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['dest'] = dest
-            new_module_args.update(
-                dict(
-                    dest=dest,
-                    original_basename=os.path.basename(src),
-                )
-            )
 
             if path_checksum != dest_stat['checksum']:
 

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',
                 )

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:
+            # remove action plugin only keys
-                # fix file permissions when the copy is done as a different user
+            new_module_args = self._task.args.copy()
-                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
             for key in ('decrypt',):
                 if key in new_module_args:
                     del new_module_args[key]
 
-                # execute the unarchive module now, with the updated args
+            if not remote_src:
-                result.update(self._execute_module(module_args=new_module_args, task_vars=task_vars))
+                # 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:

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"
             )
         )