Skip to content

Conversation

@bandre-ucar
Copy link
Contributor

@bandre-ucar bandre-ucar commented Jan 5, 2018

Untracked files in a git or svn external have the potential to abort a checkout
and leave the sandbox in a poorly defined state that the user needs to resolve.
The initial implementation refused to update if untracked files were present to
prevent this. This restriction has been deamed too restrictive. It is possible
to request svn to do a dry-run and determine if a checkout or switch will be
successful. Git does not have a mechanism to determine if a checkout will be
successful before running, and there are not any clear workarounds.

For now untracked files are being ignored. If problems arise, restrictions and
checks will be re-added to address specific problems.

Closes: GH-51

Testing:
new unit tests added for new git functionality
existing tests were updated to test for new behavior.
python2 - make test - all tests pass, one skip.

@coveralls
Copy link

coveralls commented Jan 5, 2018

Coverage Status

Coverage decreased (-0.07%) to 93.415% when pulling 776b2f4 on bandre-ucar:ignore-untracked-files into 15f60d2 on NCAR:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 93.254% when pulling 776b2f4 on bandre-ucar:ignore-untracked-files into 15f60d2 on NCAR:master.

Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor grammar nit and a question.

if tracked:
# NOTE(bja, 2018-01) brittle hack to obtain repo dir and
# file name
path_data = filename.split('/')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use repo_dir, tracked_file = os.path.split(filename) here?

self._check_container_simple_optional_st_dirty(overall, tree)

def test_container_simple_untracked(self):
"""Verify that a container with simple subrepos and a untracked files
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra 'a'

is_dirty = True
status_list = git_output.split('\0')
for item in status_list:
if item.startswith('??'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to do this would be to add -uno to the git status command. But I'm fine with this approach if you had a reason for doing it this way.

@billsacks
Copy link
Member

Thank you very much for making this change!

@coveralls
Copy link

coveralls commented Jan 10, 2018

Coverage Status

Coverage decreased (-0.2%) to 93.313% when pulling c4e3448 on bandre-ucar:ignore-untracked-files into 15f60d2 on NCAR:master.

bandre-ucar added a commit that referenced this pull request Jan 18, 2018
Merge remote-tracking branch 'wjs/print_command_error'

Two related changes here:

1. For any failed command run via execute_subprocess, print more
information about the problem to stdout

2. After doing an svn switch, check the status of the resulting
repository; if dirty, abort with a helpful message

The immediate purpose of both of these is to give more helpful error
messages if there are failures due to the presence of untracked files,
now that we attempt a checkout even if there are untracked files (via
overwritten by a tracked file in the new version:

git aborts the checkout with an error code; the new information
printed by (1) will help users in this case

svn attempts the checkout and returns a 0 error status, but may
leave you in a conflict state; we check this via (2)

Note that (2) should never happen currently (due to the up-front conservative
check of the repository status, which doesn't even allow untracked files), but
it will become possible after #57.

User interface changes?: No

Testing:
new unit tests added for some utility functions
unit tests: pass
system tests: pass
manual testing: For both git and svn, tested having an untracked file
in the working directory that gets overwritten after checking out a
new branch;
Untracked files in a git or svn external have the potential to abort a checkout
and leave the sandbox in a poorly defined state that the user needs to resolve.
The initial implementation refused to update if untracked files were present to
prevent this. This restriction has been deamed too restrictive. It is possible
to request svn to do a dry-run and determine if a checkout or switch will be
successful. Git does not have a mechanism to determine if a checkout will be
successful before running, and there are not any clear workarounds.

For now untracked files are being ignored. If problems arise, restrictions and
checks will be re-added to address specific problems.

Testing:
  new unit tests added for new git functionality
  existing tests were updated to test for new behavior.
  python2 - make test - all tests pass, one skip.
@bandre-ucar bandre-ucar merged commit 013b82c into ESMCI:master Jan 18, 2018
@bandre-ucar bandre-ucar deleted the ignore-untracked-files branch January 19, 2018 00:24
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 013b82c on bandre-ucar:ignore-untracked-files into ** on NCAR:master**.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 19, 2018

Coverage Status

Changes Unknown when pulling 013b82c on bandre-ucar:ignore-untracked-files into ** on NCAR:master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

checkout_externals should proceed when untracked files are present

4 participants