From d16432f246fae3bca6b7c7d981c94392ad450b5c Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Thu, 9 Aug 2018 19:39:27 +0100 Subject: [PATCH] git: Use currently checked out ref when none supplied This patch set adds logic to examine the currently checked out reference in a repository if ref=None is passed to git.git_handler. This makes it convenient to simply reuse the revision that is currently checked out without having to re-supply it. Note that ref is still a required parameter so providing None will have to be done explicitly. Defaults for ref='master' have been changed to ref=None. Turns out the GitPython library has logic to use the repo's Git config's fetch refspec by default [0], which means that by default 'master' will be checked out for many repositories. Also adds a ``is_repository`` function to ``git`` module to more robustly check whether a directory is a repo. Unit tests and doc strings are updated as well. [0] https://github.com/gitpython-developers/GitPython/blob/a8591a094a768d73e6efb5a698f74d354c989291/git/remote.py#L779 Change-Id: Ib1736eb20a51996cf03fa7f6187f078b39595267 --- src/bin/pegleg/pegleg/engine/util/git.py | 58 ++++++++++++++----- .../pegleg/tests/unit/engine/util/test_git.py | 30 +++++++++- 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/src/bin/pegleg/pegleg/engine/util/git.py b/src/bin/pegleg/pegleg/engine/util/git.py index 4b6c3176..917e84a1 100644 --- a/src/bin/pegleg/pegleg/engine/util/git.py +++ b/src/bin/pegleg/pegleg/engine/util/git.py @@ -30,7 +30,7 @@ __all__ = [ ] -def git_handler(repo_url, ref, proxy_server=None, auth_key=None): +def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None): """Handle directories that are Git repositories. If ``repo_url`` is a valid URL for which a local repository doesn't @@ -46,7 +46,8 @@ def git_handler(repo_url, ref, proxy_server=None, auth_key=None): :param repo_url: URL of remote Git repo or path to local Git repo. If no local copy exists, clone it. Afterward, check out ``ref`` in the repo. - :param ref: branch, commit or reference in the repo to clone. + :param ref: branch, commit or reference in the repo to clone. None causes + the currently checked out reference to be used (if repo exists). :param proxy_server: optional, HTTP proxy to use while cloning the repo. :param auth_key: If supplied results in using SSH to clone the repository with the specified key. If the value is None, SSH is not used. @@ -65,8 +66,8 @@ def git_handler(repo_url, ref, proxy_server=None, auth_key=None): except Exception as e: raise ValueError('repo_url=%s is invalid. Details: %s' % (repo_url, e)) - if not ref: - raise ValueError('ref=%s must be a non-empty, valid Git ref' % ref) + if ref is None: + ref = _get_current_ref(repo_url) if not os.path.exists(repo_url): # we need to clone the repo_url first since it doesn't exist and then @@ -115,12 +116,31 @@ def git_handler(repo_url, ref, proxy_server=None, auth_key=None): return repo_url -def _try_git_clone(repo_url, ref='master', proxy_server=None, auth_key=None): +def _get_current_ref(repo_url): + """If no ``ref`` is provided, then the most logical assumption is that + the current revision in the checked out repo should be used. If the + repo hasn't been cloned yet, None will be returned, in which case GitPython + will use the repo's config's fetch refspec instead. + + :param repo_url: URL of remote Git repo or path to local Git repo. + + """ + + try: + repo = Repo(repo_url) + current_ref = repo.head.ref.name + LOG.debug('ref for repo_url=%s not specified, defaulting to currently ' + 'checked out ref=%s', repo_url, current_ref) + return current_ref + except Exception as e: + return None + + +def _try_git_clone(repo_url, ref=None, proxy_server=None, auth_key=None): """Try cloning Git repo from ``repo_url`` using the reference ``ref``. :param repo_url: URL of remote Git repo or path to local Git repo. - :param ref: branch, commit or reference in the repo to clone. Default is - 'master'. + :param ref: branch, commit or reference in the repo to clone. :param proxy_server: optional, HTTP proxy to use while cloning the repo. :param auth_key: If supplied results in using SSH to clone the repository with the specified key. If the value is None, SSH is not used. @@ -176,8 +196,7 @@ def _get_clone_env_vars(repo_url, ref, auth_key): """Generate environment variables include SSH command for Git clone. :param repo_url: URL of remote Git repo or path to local Git repo. - :param ref: branch, commit or reference in the repo to clone. Default is - 'master'. + :param ref: branch, commit or reference in the repo to clone. :param auth_key: If supplied results in using SSH to clone the repository with the specified key. If the value is None, SSH is not used. :returns: Dictionary of key-value pairs for Git clone. @@ -207,21 +226,20 @@ def _get_clone_env_vars(repo_url, ref, auth_key): return env_vars -def _try_git_checkout(repo, repo_url, ref, fetch=True): +def _try_git_checkout(repo, repo_url, ref=None, fetch=True): """Try to checkout a ``ref`` from ``repo``. Local branches are created for multiple variations of the ``ref``, including its refpath and hexpath (i.e. commit ID). - This is to locally "memoize" references that would otherwise require + This is to locally "cache" references that would otherwise require resolution upstream. We increase performance by creating local branches for these other ``ref`` formats when the ``ref`` is fetched remotely for the first time only. :param repo: Git Repo object. :param repo_url: URL of remote Git repo or path to local Git repo. - :param ref: branch, commit or reference in the repo to clone. Default is - 'master'. + :param ref: branch, commit or reference in the repo to clone. :param fetch: Whether to fetch the ``ref`` from remote before checkout or to use the already-cloned local repo. :raises GitException: If ``ref`` could not be checked out. @@ -285,3 +303,17 @@ def _create_local_ref(g, branches, ref, newref, reftype=None): reftype, ref) g.checkout('FETCH_HEAD', b=newref) branches.append(newref) + + +def is_repository(path): + """Checks whether the directory ``path`` is a Git repository. + + :param str path: Directory path to check. + :returns: True if ``path`` is a repo, else False. + :rtype: boolean + """ + try: + Repo(path).git_dir + return True + except git_exc.InvalidGitRepositoryError: + return False diff --git a/src/bin/pegleg/tests/unit/engine/util/test_git.py b/src/bin/pegleg/tests/unit/engine/util/test_git.py index 8e848d58..700e4125 100644 --- a/src/bin/pegleg/tests/unit/engine/util/test_git.py +++ b/src/bin/pegleg/tests/unit/engine/util/test_git.py @@ -86,7 +86,7 @@ def _validate_git_clone(repo_dir, fetched_ref=None, checked_out_ref=None): assert os.path.isdir(repo_dir) # Assert that the directory is a Git repo. - assert os.path.isdir(os.path.join(repo_dir, '.git')) + assert git.is_repository(repo_dir) if fetched_ref: # Assert the FETCH_HEAD is at the fetched_ref ref. with open(os.path.join(repo_dir, '.git', 'FETCH_HEAD'), 'r') \ @@ -283,6 +283,24 @@ def test_git_clone_existing_directory_checks_out_next_local_ref( _assert_repo_url_was_cloned(mock_log, git_dir) +@pytest.mark.skipif( + not is_connected(), reason='git clone requires network connectivity.') +@mock.patch.object(git, 'LOG', autospec=True) +def test_git_checkout_without_reference_defaults_to_current( + mock_log, clean_git_repo): + """Validate that the currently checked out ref is defaulted to when + ref=None is passed to ``git.git_handler``. + """ + url = 'https://github.com/openstack/airship-armada' + commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d' + git_dir = git.git_handler(url, commit) + _validate_git_clone(git_dir, commit) + + git_dir = git.git_handler(git_dir, ref=None) # Defaults to commit ref. + _validate_git_clone(git_dir, commit) # Validate with the original ref. + _assert_repo_url_was_cloned(mock_log, git_dir) + + @pytest.mark.skipif( not is_connected(), reason='git clone requires network connectivity.') @mock.patch.object(git, 'LOG', autospec=True) @@ -313,6 +331,16 @@ def test_git_clone_delete_repo_and_reclone(mock_log, clean_git_repo): mock_log.debug.assert_any_call('Cloning [%s]', repo_url) +@pytest.mark.skipif( + not is_connected(), reason='git clone requires network connectivity.') +@mock.patch.object(git, 'LOG', autospec=True) +def test_git_checkout_none_ref_checks_out_master(mock_log, clean_git_repo): + """Validate that ref=None checks out master.""" + url = 'https://github.com/openstack/airship-armada' + git_dir = git.git_handler(url, ref=None) + _validate_git_clone(git_dir, 'master') + + @pytest.mark.skipif( not is_connected(), reason='git clone requires network connectivity.') @mock.patch.object(git, 'LOG', autospec=True)