Detect separate git dir and set git config path value appropriately

PR #38016

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

lib/ansible/modules/source_control/git.py

@@ -597,6 +597,30 @@ def is_not_a_branch(git_path, module, dest):
     return False
 
 
+def get_repo_path(dest, bare):
+    if bare:
+        repo_path = dest
+    else:
+        repo_path = os.path.join(dest, '.git')
+    # Check if the .git is a file. If it is a file, it means that the repository is in external directory respective to the working copy (e.g. we are in a
+    # submodule structure).
+    if os.path.isfile(repo_path):
+        with open(repo_path, 'r') as gitfile:
+            data = gitfile.read()
+        ref_prefix, gitdir = data.rstrip().split('gitdir: ', 1)
+        if ref_prefix:
+            raise ValueError('.git file has invalid git dir reference format')
+
+        # There is a possibility the .git file to have an absolute path.
+        if os.path.isabs(gitdir):
+            repo_path = gitdir
+        else:
+            repo_path = os.path.join(repo_path.split('.git')[0], gitdir)
+        if not os.path.isdir(repo_path):
+            raise ValueError('%s is not a directory' % repo_path)
+    return repo_path
+
+
 def get_head_branch(git_path, module, dest, remote, bare=False):
     '''
     Determine what branch HEAD is associated with.  This is partly
@@ -605,31 +629,16 @@ def get_head_branch(git_path, module, dest, remote, bare=False):
     associated with.  In the case of a detached HEAD, this will look
     up the branch in .git/refs/remotes/<remote>/HEAD.
     '''
-    if bare:
-        repo_path = dest
-    else:
-        repo_path = os.path.join(dest, '.git')
-    # Check if the .git is a file. If it is a file, it means that we are in a submodule structure.
-    if os.path.isfile(repo_path):
     try:
-            git_conf = open(repo_path, 'rb')
-            for line in git_conf:
-                config_val = line.split(b(':'), 1)
-                if config_val[0].strip() == b('gitdir'):
-                    gitdir = to_native(config_val[1].strip(), errors='surrogate_or_strict')
-                    break
-            else:
+        repo_path = get_repo_path(dest, bare)
+    except (IOError, ValueError) as err:
         # No repo path found
-                return ''
-
-            # There is a possibility the .git file to have an absolute path.
-            if os.path.isabs(gitdir):
-                repo_path = gitdir
-            else:
-                repo_path = os.path.join(repo_path.split('.git')[0], gitdir)
-        except (IOError, AttributeError):
-            # No repo path found
-            return ''
+        """``.git`` file does not have a valid format for detached Git dir."""
+        module.fail_json(
+            msg='Current repo does not have a valid reference to a '
+            'separate Git dir or it refers to the invalid path',
+            details=str(err),
+        )
     # Read .git/HEAD for the name of the branch.
     # If we're in a detached HEAD state, look up the branch associated with
     # the remote HEAD in .git/refs/remotes/<remote>/HEAD
@@ -1019,10 +1028,17 @@ def main():
         module.fail_json(msg="the destination directory must be specified unless clone=no")
     elif dest:
         dest = os.path.abspath(dest)
-        if bare:
-            gitconfig = os.path.join(dest, 'config')
-        else:
-            gitconfig = os.path.join(dest, '.git', 'config')
+        try:
+            repo_path = get_repo_path(dest, bare)
+        except (IOError, ValueError) as err:
+            # No repo path found
+            """``.git`` file does not have a valid format for detached Git dir."""
+            module.fail_json(
+                msg='Current repo does not have a valid reference to a '
+                'separate Git dir or it refers to the invalid path',
+                details=str(err),
+            )
+        gitconfig = os.path.join(repo_path, 'config')
 
     # create a wrapper script and export
     # GIT_SSH=<path> as an environment variable

test/integration/targets/git/tasks/main.yml

@@ -36,3 +36,6 @@
 - include_tasks: reset-origin.yml
 - include_tasks: ambiguous-ref.yml
 - include_tasks: archive.yml
+- include_tasks: separate-git-dir.yml
+  when:
+    - git_version.stdout is version("1.7.5", '>=')

test/integration/targets/git/tasks/separate-git-dir.yml

@@ -0,0 +1,64 @@
+# test code for repositories with separate git dir updating
+# see https://github.com/ansible/ansible/pull/38016
+# see https://github.com/ansible/ansible/issues/30034
+
+- name: SEPARATE-GIT-DIR | clear checkout_dir
+  file:
+    state: absent
+    path: '{{ checkout_dir }}'
+
+- name: create a tempdir for separate git dir
+  local_action: shell mktemp -du
+  register: tempdir
+
+- name: SEPARATE-GIT-DIR | clone with a separate git dir
+  command: git clone {{ repo_format1 }} {{ checkout_dir }} --separate-git-dir={{ tempdir.stdout }}
+
+- name: SEPARATE-GIT-DIR | update repo the usual way
+  git:
+    repo: "{{ repo_format1 }}"
+    dest: "{{ checkout_dir }}"
+
+- name: SEPARATE-GIT-DIR | set git dir to non-existent dir
+  shell: "echo gitdir: /dev/null/non-existent-dir > .git"
+  args:
+    chdir: "{{ checkout_dir }}"
+
+- name: SEPARATE-GIT-DIR | update repo the usual way
+  git:
+    repo: "{{ repo_format1 }}"
+    dest: "{{ checkout_dir }}"
+  ignore_errors: yes
+  register: result
+
+- name: SEPARATE-GIT-DIR | check update has failed
+  assert:
+    that:
+      - result is failed
+
+- name: SEPARATE-GIT-DIR | set .git file to bad format
+  shell: "echo some text  gitdir: {{ checkout_dir }} > .git"
+  args:
+    chdir: "{{ checkout_dir }}"
+
+- name: SEPARATE-GIT-DIR | update repo the usual way
+  git:
+    repo: "{{ repo_format1 }}"
+    dest: "{{ checkout_dir }}"
+  ignore_errors: yes
+  register: result
+
+- name: SEPARATE-GIT-DIR | check update has failed
+  assert:
+    that:
+      - result is failed
+
+- name: SEPARATE-GIT-DIR | clear separate git dir
+  file:
+    state: absent
+    path: "{{ tempdir.stdout }}"
+
+- name: SEPARATE-GIT-DIR | clear checkout_dir
+  file:
+    state: absent
+    path: '{{ checkout_dir }}'