Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fail git.latest states with uncommitted changes when force_reset=False #34869

Merged
merged 3 commits into from
Jul 22, 2016
Merged

Fail git.latest states with uncommitted changes when force_reset=False #34869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b7b492
Add git.diff function
terminalmage Jul 22, 2016
0604e42
Fail git.latest states with uncommitted changes when force_reset=False
terminalmage Jul 22, 2016
d2de3d4
Add integration test for case where there are uncommitted changes
terminalmage Jul 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions salt/modules/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,145 @@ def describe(cwd, rev='HEAD', user=None, ignore_retcode=False):
ignore_retcode=ignore_retcode)['stdout']


def diff(cwd,
item1=None,
item2=None,
opts='',
user=None,
no_index=False,
cached=False,
paths=None):
'''
.. versionadded:: 2015.8.12,2016.3.3

Interface to `git-diff(1)`_

cwd
The path to the git checkout

item1 and item2
Revision(s) to pass to the ``git diff`` command. One or both of these
arguments may be ignored if some of the options below are set to
``True``. When ``cached`` is ``False``, and no revisions are passed
to this function, then the current working tree will be compared
against the index (i.e. unstaged changes). When two revisions are
passed, they will be compared to each other.

opts
Any additional options to add to the command line, in a single string

.. note::
On the Salt CLI, if the opts are preceded with a dash, it is
necessary to precede them with ``opts=`` (as in the CLI examples
below) to avoid causing errors with Salt's own argument parsing.

user
User under which to run the git command. By default, the command is run
by the user under which the minion is running.

no_index : False
When it is necessary to diff two files in the same repo against each
other, and not diff two different revisions, set this option to
``True``. If this is left ``False`` in these instances, then a normal
``git diff`` will be performed against the index (i.e. unstaged
changes), and files in the ``paths`` option will be used to narrow down
the diff output.

.. note::
Requires Git 1.5.1 or newer. Additionally, when set to ``True``,
``item1`` and ``item2`` will be ignored.

cached : False
If ``True``, compare staged changes to ``item1`` (if specified),
otherwise compare them to the most recent commit.

.. note::
``item2`` is ignored if this option is is set to ``True``.

paths
File paths to pass to the ``git diff`` command. Can be passed as a
comma-separated list or a Python list.

.. _`git-diff(1)`: http://git-scm.com/docs/git-diff


CLI Example:

.. code-block:: bash

# Perform diff against the index (staging area for next commit)
salt myminion git.diff /path/to/repo
# Compare staged changes to the most recent commit
salt myminion git.diff /path/to/repo cached=True
# Compare staged changes to a specific revision
salt myminion git.diff /path/to/repo mybranch cached=True
# Perform diff against the most recent commit (includes staged changes)
salt myminion git.diff /path/to/repo HEAD
# Diff two commits
salt myminion git.diff /path/to/repo abcdef1 aabbccd
# Diff two commits, only showing differences in the specified paths
salt myminion git.diff /path/to/repo abcdef1 aabbccd paths=path/to/file1,path/to/file2
# Diff two files with one being outside the working tree
salt myminion git.diff /path/to/repo no_index=True paths=path/to/file1,/absolute/path/to/file2
'''
if no_index and cached:
raise CommandExecutionError(
'The \'no_index\' and \'cached\' options cannot be used together'
)

command = ['git', 'diff']
command.extend(_format_opts(opts))

if paths is not None and not isinstance(paths, (list, tuple)):
try:
paths = paths.split(',')
except AttributeError:
paths = str(paths).split(',')

ignore_retcode = False
failhard = True

if no_index:
if _LooseVersion(version(versioninfo=False)) < _LooseVersion('1.5.1'):
raise CommandExecutionError(
'The \'no_index\' option is only supported in Git 1.5.1 and '
'newer'
)
ignore_retcode = True
failhard = False
command.append('--no-index')
for value in [x for x in (item1, item2) if x]:
log.warning(
'Revision \'%s\' ignored in git diff, as revisions cannot be '
'used when no_index=True', value
)

elif cached:
command.append('--cached')
if item1:
command.append(item1)
if item2:
log.warning(
'Second revision \'%s\' ignored in git diff, at most one '
'revision is considered when cached=True', item2
)

else:
for value in [x for x in (item1, item2) if x]:
command.append(value)

if paths:
command.append('--')
command.extend(paths)

return _git_run(command,
cwd=cwd,
runas=user,
ignore_retcode=ignore_retcode,
failhard=failhard,
redirect_stderr=True)['stdout']


def fetch(cwd,
remote=None,
force=False,
Expand Down
65 changes: 47 additions & 18 deletions salt/states/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,17 +177,24 @@ def _failed_submodule_update(ret, exc, comments=None):
return _fail(ret, msg, comments)


def _not_fast_forward(ret, pre, post, branch, local_branch, comments):
def _not_fast_forward(ret, pre, post, branch, local_branch,
local_changes, comments):
pre = _short_sha(pre)
post = _short_sha(post)
return _fail(
ret,
'Repository would be updated from {0} to {1}{2}, but this is not a '
'fast-forward merge. Set \'force_reset\' to True to force this '
'update.'.format(
_short_sha(pre),
_short_sha(post),
'Repository would be updated {0}{1}, but {2}. Set \'force_reset\' to '
'True to force this update{3}.'.format(
'from {0} to {1}'.format(pre, post)
if local_changes and pre != post
else 'to {0}'.format(post),
' (after checking out local branch \'{0}\')'.format(branch)
if _need_branch_change(branch, local_branch)
else ''
else '',
'this is not a fast-forward merge'
if not local_changes
else 'there are uncommitted changes',
' and discard these changes' if local_changes else ''
),
comments
)
Expand Down Expand Up @@ -700,6 +707,19 @@ def latest(name,
redact_auth=False)

revs_match = _revs_equal(local_rev, remote_rev, remote_rev_type)
try:
local_changes = bool(
__salt__['git.diff'](target, 'HEAD', user=user)
)
except CommandExecutionError:
# No need to capture the error and log it, the _git_run()
# helper in the git execution module will have already logged
# the output from the command.
log.warning(
'git.latest: Unable to determine if %s has local changes',
target
)
local_changes = False

if remote_rev_type == 'sha1' \
and base_rev is not None \
Expand Down Expand Up @@ -789,13 +809,15 @@ def latest(name,
elif remote_rev_type == 'sha1':
has_remote_rev = True

if not has_remote_rev:
# Either the remote rev could not be found with git
# ls-remote (in which case we won't know more until
# fetching) or we're going to be checking out a new branch
# and don't have to worry about fast-forwarding.
fast_forward = None
else:
# If has_remote_rev is False, then either the remote rev could not
# be found with git ls-remote (in which case we won't know more
# until fetching) or we're going to be checking out a new branch
# and don't have to worry about fast-forwarding. So, we will set
# fast_forward to None (to signify uncertainty) unless there are
# local changes, in which case we will set it to False.
fast_forward = None if not local_changes else False

if has_remote_rev:
# Remote rev already present
if (not revs_match and not update_head) \
and (branch is None or branch == local_branch):
Expand All @@ -809,14 +831,18 @@ def latest(name,
)
return ret

# No need to check if this is a fast_forward if we already know
# that it won't be (due to local changes).
if fast_forward is not False:
if base_rev is None:
# If we're here, the remote_rev exists in the local
# checkout but there is still no HEAD locally. A possible
# reason for this is that an empty repository existed there
# and a remote was added and fetched, but the repository
# was not fast-forwarded. Regardless, going from no HEAD to
# a locally-present rev is considered a fast-forward update.
fast_forward = True
# a locally-present rev is considered a fast-forward
# update, unless there are local changes.
fast_forward = not bool(local_changes)
else:
fast_forward = __salt__['git.merge_base'](
target,
Expand All @@ -833,6 +859,7 @@ def latest(name,
remote_rev,
branch,
local_branch,
local_changes,
comments)
merge_action = 'hard-reset'
elif fast_forward is True:
Expand Down Expand Up @@ -1122,11 +1149,10 @@ def latest(name,
remote_rev,
branch,
local_branch,
local_changes,
comments)

if _need_branch_change(branch, local_branch):
local_changes = __salt__['git.status'](target,
user=user)
if local_changes and not force_checkout:
return _fail(
ret,
Expand Down Expand Up @@ -1168,6 +1194,9 @@ def latest(name,
'\'{0}\' was checked out'.format(checkout_rev)
)

if local_changes:
comments.append('Local changes were discarded')

if fast_forward is False:
__salt__['git.reset'](
target,
Expand Down
47 changes: 47 additions & 0 deletions tests/integration/states/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,53 @@ def test_numeric_rev(self):
finally:
shutil.rmtree(name, ignore_errors=True)

def test_latest_with_local_changes(self):
'''
Ensure that we fail the state when there are local changes and succeed
when force_reset is True.
'''
name = os.path.join(integration.TMP, 'salt_repo')
try:
# Clone repo
ret = self.run_state(
'git.latest',
name='https://{0}/saltstack/salt-test-repo.git'.format(self.__domain),
target=name
)
self.assertSaltTrueReturn(ret)
self.assertTrue(os.path.isdir(os.path.join(name, '.git')))

# Make change to LICENSE file.
with salt.utils.fopen(os.path.join(name, 'LICENSE'), 'a') as fp_:
fp_.write('Lorem ipsum dolor blah blah blah....\n')

# Make sure that we now have uncommitted changes
self.assertTrue(self.run_function('git.diff', [name, 'HEAD']))

# Re-run state with force_reset=False, this should fail
ret = self.run_state(
'git.latest',
name='https://{0}/saltstack/salt-test-repo.git'.format(self.__domain),
target=name,
force_reset=False
)
self.assertSaltFalseReturn(ret)

# Now run the state with force_reset=True, this should succeed
ret = self.run_state(
'git.latest',
name='https://{0}/saltstack/salt-test-repo.git'.format(self.__domain),
target=name,
force_reset=True
)
self.assertSaltTrueReturn(ret)

# Make sure that we no longer have uncommitted changes
self.assertFalse(self.run_function('git.diff', [name, 'HEAD']))

finally:
shutil.rmtree(name, ignore_errors=True)

def test_present(self):
'''
git.present
Expand Down