Allow "dirty" local repositories to be safely modified
This patch set rolls back previously introduced behavior in https://review.openstack.org/#/c/584482/ which forbids users from basically performing any Pegleg command that references a dirty local repository. This is annoying, forcing users to create temporary commits before executing a Pegleg command. Fortunately with https://review.openstack.org/#/c/577886/ Pegleg copies over all repositories to temporary folders, within which dirty repos can have their changes temporarily committed, allowing different references to then be safely checked out, without ever modifying any local repositories. Change-Id: I2142ae434f8ad57d0ab81cb104e21d952dc23148
This commit is contained in:
Felipe Monteiro
parent
922083448c
commit
73fbf264ca
|
|
@ -45,8 +45,8 @@ Git Exceptions
|
|||
:members:
|
||||
:show-inheritance:
|
||||
:undoc-members:
|
||||
* - GitDirtyRepoException
|
||||
- .. autoexception:: pegleg.engine.exceptions.GitDirtyRepoException
|
||||
* - GitConfigException
|
||||
- .. autoexception:: pegleg.engine.exceptions.GitConfigException
|
||||
:members:
|
||||
:show-inheritance:
|
||||
:undoc-members:
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ class GitAuthException(BaseGitException):
|
|||
self._ssh_key_path = ssh_key_path
|
||||
|
||||
self._message = ('Failed to authenticate for repo %s with ssh-key at '
|
||||
'path %s.' % (self._repo_url, self._ssh_key_path))
|
||||
'path %s' % (self._repo_url, self._ssh_key_path))
|
||||
|
||||
super(GitAuthException, self).__init__(self._message)
|
||||
|
||||
|
|
@ -62,7 +62,7 @@ class GitProxyException(BaseGitException):
|
|||
|
||||
def __init__(self, location):
|
||||
self._location = location
|
||||
self._message = ('Could not resolve proxy [%s].' % self._location)
|
||||
self._message = ('Could not resolve proxy [%s]' % self._location)
|
||||
|
||||
super(GitProxyException, self).__init__(self._message)
|
||||
|
||||
|
|
@ -73,14 +73,12 @@ class GitSSHException(BaseGitException):
|
|||
def __init__(self, ssh_key_path):
|
||||
self._ssh_key_path = ssh_key_path
|
||||
|
||||
self._message = ('Failed to find specified SSH key: %s.' %
|
||||
self._message = ('Failed to find specified SSH key: %s' %
|
||||
(self._ssh_key_path))
|
||||
|
||||
super(GitSSHException, self).__init__(self._message)
|
||||
|
||||
|
||||
class GitDirtyRepoException(BaseGitException):
|
||||
"""Exception that occurs when trying to checkout ref from dirty repo."""
|
||||
message = ("Failed to checkout ref=%(ref)s from repo_url=%(repo_url)s. "
|
||||
"Please manually clean all tracked/untracked files from repo "
|
||||
"before proceeding.")
|
||||
class GitConfigException(BaseGitException):
|
||||
"""Exception that occurs when reading Git repo config fails."""
|
||||
message = ("Failed to read Git config file for repo_url=%(repo_url)s")
|
||||
|
|
|
|||
|
|
@ -12,7 +12,6 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import atexit
|
||||
import logging
|
||||
import os
|
||||
import tempfile
|
||||
|
|
@ -27,11 +26,7 @@ from pegleg.engine import exceptions
|
|||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
__all__ = [
|
||||
'git_handler',
|
||||
]
|
||||
|
||||
__MODIFIED_REPOS = []
|
||||
__all__ = ('git_handler', )
|
||||
|
||||
|
||||
def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None):
|
||||
|
|
@ -98,11 +93,15 @@ def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None):
|
|||
|
||||
repo = Repo(repo_url, search_parent_directories=True)
|
||||
if repo.is_dirty(untracked_files=True):
|
||||
LOG.error('The locally cloned repo_url=%s is dirty. Manual clean '
|
||||
'up of tracked/untracked files required.', repo_url)
|
||||
# Raise an exception and force the user to clean up the repo.
|
||||
# This is the safest approach to avoid data loss/corruption.
|
||||
raise exceptions.GitDirtyRepoException(ref=ref, repo_url=repo_url)
|
||||
# NOTE(felipemonteiro): This code should never be executed on a
|
||||
# real local repository. Wrapper logic around this module will
|
||||
# only perform this functionality against a temporary replica of
|
||||
# local repositories, making the below operations safe.
|
||||
LOG.info('Replica of local repo=%s is dirty. Committing all '
|
||||
'tracked/untracked changes to ref=%s',
|
||||
repo_name(repo_url), ref)
|
||||
repo.git.add(all=True)
|
||||
repo.index.commit('Temporary Pegleg commit')
|
||||
|
||||
try:
|
||||
# Check whether the ref exists locally.
|
||||
|
|
@ -365,8 +364,8 @@ def repo_name(repo_url_or_path):
|
|||
if config_reader.has_section(section):
|
||||
repo_url = config_reader.get_value(section, option)
|
||||
return repo_url.split('/')[-1].split('.git')[0]
|
||||
raise click.ClickException(
|
||||
"Repo=%s is not a valid Git repository" % repo_url_or_path)
|
||||
|
||||
raise exceptions.GitConfigException(repo_url=repo_url_or_path)
|
||||
|
||||
|
||||
def normalize_repo_path(repo_path):
|
||||
|
|
@ -403,12 +402,3 @@ def normalize_repo_path(repo_path):
|
|||
"repository" % orig_repo_path)
|
||||
|
||||
return repo_path, sub_path
|
||||
|
||||
|
||||
@atexit.register
|
||||
def clean_repo():
|
||||
global __MODIFIED_REPOS
|
||||
|
||||
for r in __MODIFIED_REPOS:
|
||||
repo = Repo(r)
|
||||
repo.head.reset(index=True, working_tree=True)
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import requests
|
|||
import tempfile
|
||||
|
||||
import fixtures
|
||||
from git import Repo
|
||||
import mock
|
||||
import pytest
|
||||
|
||||
|
|
@ -337,6 +338,61 @@ def test_git_checkout_none_ref_checks_out_master(mock_log):
|
|||
_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)
|
||||
def test_git_checkout_dirty_repo_tracked_file_committed(mock_log):
|
||||
"""Validate a dirty tracked file is committed."""
|
||||
# Clone the openstack-helm repo and automatically checkout patch 73.
|
||||
ref = 'refs/changes/54/457754/73'
|
||||
repo_url = 'https://github.com/openstack/openstack-helm'
|
||||
git_dir = git.git_handler(repo_url, ref)
|
||||
_validate_git_clone(git_dir, ref)
|
||||
|
||||
# Simulate a dirty repo by removing the first file in it we encounter.
|
||||
dirty_file = next(
|
||||
os.path.join(git_dir, x) for x in os.listdir(git_dir)
|
||||
if os.path.isfile(os.path.join(git_dir, x)))
|
||||
os.remove(dirty_file)
|
||||
|
||||
# Sanity check that the repo has a dirty tracked file.
|
||||
repo = Repo(git_dir)
|
||||
assert repo.is_dirty()
|
||||
|
||||
git.git_handler(git_dir, ref)
|
||||
|
||||
# Validate that the tracked file is committed.
|
||||
repo = Repo(git_dir)
|
||||
assert not repo.is_dirty()
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not is_connected(), reason='git clone requires network connectivity.')
|
||||
@mock.patch.object(git, 'LOG', autospec=True)
|
||||
def test_git_checkout_dirty_repo_untracked_file_committed(mock_log):
|
||||
"""Validate a dirty untracked file is committed."""
|
||||
# Clone the openstack-helm repo and automatically checkout patch 73.
|
||||
ref = 'refs/changes/54/457754/73'
|
||||
repo_url = 'https://github.com/openstack/openstack-helm'
|
||||
git_dir = git.git_handler(repo_url, ref)
|
||||
_validate_git_clone(git_dir, ref)
|
||||
|
||||
# Simulate a dirty repo by adding in a new untracked file.
|
||||
with open(os.path.join(git_dir, test_utils.rand_name('test_file')),
|
||||
'w') as f:
|
||||
f.write('whatever')
|
||||
|
||||
# Sanity check that the repo has an untracked file.
|
||||
repo = Repo(git_dir)
|
||||
assert len(repo.untracked_files)
|
||||
|
||||
git.git_handler(git_dir, ref)
|
||||
|
||||
# Validate that the untracked file is committed.
|
||||
assert not len(repo.untracked_files)
|
||||
assert not repo.is_dirty(untracked_files=True)
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not is_connected(), reason='git clone requires network connectivity.')
|
||||
@mock.patch.object(git, 'LOG', autospec=True)
|
||||
|
|
@ -357,51 +413,6 @@ def test_git_clone_existing_directory_raises_exc_for_invalid_ref(mock_log):
|
|||
_assert_check_out_from_local_repo(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_dirty_repo_tracked_file_raises_exception(mock_log):
|
||||
"""Validate Git throws an error when attempting to checkout a ref from
|
||||
a dirty repo, with modified tracked file.
|
||||
"""
|
||||
# Clone the openstack-helm repo and automatically checkout patch 73.
|
||||
ref = 'refs/changes/54/457754/73'
|
||||
repo_url = 'https://github.com/openstack/openstack-helm'
|
||||
git_dir = git.git_handler(repo_url, ref)
|
||||
_validate_git_clone(git_dir, ref)
|
||||
|
||||
# Simulate a dirty repo by removing the first file in it we encounter.
|
||||
dirty_file = next(
|
||||
os.path.join(git_dir, x) for x in os.listdir(git_dir)
|
||||
if os.path.isfile(os.path.join(git_dir, x)))
|
||||
os.remove(dirty_file)
|
||||
|
||||
with pytest.raises(exceptions.GitDirtyRepoException):
|
||||
git.git_handler(git_dir, ref)
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not is_connected(), reason='git clone requires network connectivity.')
|
||||
@mock.patch.object(git, 'LOG', autospec=True)
|
||||
def test_git_checkout_dirty_repo_untracked_file_raises_exception(mock_log):
|
||||
"""Validate Git throws an error when attempting to checkout a ref from
|
||||
a dirty repo, with untracked file.
|
||||
"""
|
||||
# Clone the openstack-helm repo and automatically checkout patch 73.
|
||||
ref = 'refs/changes/54/457754/73'
|
||||
repo_url = 'https://github.com/openstack/openstack-helm'
|
||||
git_dir = git.git_handler(repo_url, ref)
|
||||
_validate_git_clone(git_dir, ref)
|
||||
|
||||
# Simulate a dirty repo by adding in a new untracked file.
|
||||
with open(os.path.join(git_dir, test_utils.rand_name('test_file')),
|
||||
'w') as f:
|
||||
f.write('whatever')
|
||||
|
||||
with pytest.raises(exceptions.GitDirtyRepoException):
|
||||
git.git_handler(git_dir, ref)
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not is_connected(), reason='git clone requires network connectivity.')
|
||||
def test_git_clone_empty_url_raises_value_error():
|
||||
|
|
|
|||
Loading…
Reference in New Issue