commands: warn if working copy is dirty

In 'git lfs migrate import' and 'git lfs migrate export', Git LFS makes
destructive changes to a caller's repository and therefore invokes 'git
checkout --force', which throws away local changes.

To prevent this, let's introduce a check that notifies users when they
are going to throw away local changes, and allows for the user to abort
if they do not wish to discard their local changes.

With this, a user can safely migrate over dirty repositories (i.e., in
the case that they wanted to fix a file that should have been in Git
LFS, but wasn't) without having to finagle their repository to get it
into a migrate-able state.

For users with lots of pending migrations, also teach --yes, which
allows a user to avoid the check, and instead simply prints the warning
message to STDERR.
This commit is contained in:
Taylor Blau 2018-07-10 10:18:46 -05:00
parent 9f89dda9b1
commit ad369e55ff
9 changed files with 117 additions and 18 deletions

@ -1,7 +1,9 @@
package commands
import (
"bufio"
"fmt"
"io"
"path/filepath"
"strings"
@ -21,6 +23,10 @@ var (
// in the migration.
migrateExcludeRefs []string
// migrateYes indicates that an answer of 'yes' should be presumed
// whenever 'git lfs migrate' asks for user input.
migrateYes bool
// migrateSkipFetch assumes that the client has the latest copy of
// remote references, and thus should not contact the remote for a set
// of updated references.
@ -294,6 +300,50 @@ func getHistoryRewriter(cmd *cobra.Command, db *gitobj.ObjectDatabase, l *tasklo
githistory.WithFilter(filter), githistory.WithLogger(l))
}
func ensureWorkingCopyClean(in io.Reader, out io.Writer) {
dirty, err := git.IsWorkingCopyDirty()
if err != nil {
ExitWithError(errors.Wrap(err,
"fatal: could not determine if working copy is dirty"))
}
if !dirty {
return
}
var proceed bool
if migrateYes {
proceed = true
} else {
answer := bufio.NewReader(in)
L:
for {
fmt.Fprintf(out, "migrate: override changes in your working copy? [Y/n] ")
s, _, err := answer.ReadLine()
if err != nil {
ExitWithError(errors.Wrap(err,
"fatal: could not read answer"))
}
switch strings.TrimSpace(string(s)) {
case "n", "N":
proceed = false
break L
case "y", "Y":
proceed = true
break L
}
fmt.Fprintf(out, "\n")
}
}
if proceed {
fmt.Fprintf(out, "migrate: changes in your working copy will be overridden ...\n")
} else {
Exit("migrate: working copy must not be dirty")
}
}
func init() {
info := NewCommand("info", migrateInfoCommand)
info.Flags().IntVar(&migrateInfoTopN, "top", 5, "--top=<n>")
@ -321,6 +371,8 @@ func init() {
cmd.PersistentFlags().BoolVar(&migrateEverything, "everything", false, "Migrate all local references")
cmd.PersistentFlags().BoolVar(&migrateSkipFetch, "skip-fetch", false, "Assume up-to-date remote references.")
cmd.PersistentFlags().BoolVarP(&migrateYes, "yes", "y", false, "Don't prompt for answers.")
cmd.AddCommand(exportCmd, importCmd, info)
})
}

@ -17,6 +17,8 @@ import (
)
func migrateExportCommand(cmd *cobra.Command, args []string) {
ensureWorkingCopyClean(os.Stdin, os.Stderr)
l := tasklog.NewLogger(os.Stderr)
defer l.Close()

@ -22,6 +22,8 @@ import (
)
func migrateImportCommand(cmd *cobra.Command, args []string) {
ensureWorkingCopyClean(os.Stdin, os.Stderr)
l := tasklog.NewLogger(os.Stderr)
defer l.Close()

@ -1192,3 +1192,21 @@ func IsFileModified(filepath string) (bool, error) {
return matched, nil
}
// IsWorkingCopyDirty returns true if and only if the working copy in which the
// command was executed is dirty as compared to the index.
//
// If the status of the working copy could not be determined, an error will be
// returned instead.
func IsWorkingCopyDirty() (bool, error) {
bare, err := IsBare()
if bare || err != nil {
return false, err
}
out, err := gitSimple("status", "--porcelain")
if err != nil {
return false, err
}
return len(out) != 0, nil
}

@ -132,7 +132,7 @@ begin_test "migrate export (bare repository)"
md_oid="$(calc_oid "$(cat a.md)")"
txt_oid="$(calc_oid "$(cat a.txt)")"
make_bare
assert_pointer "refs/heads/master" "a.txt" "$txt_oid" "30"
@ -198,7 +198,7 @@ begin_test "migrate export (no filter)"
setup_multiple_local_branches_tracked
git lfs migrate export 2>&1 | tee migrate.log
git lfs migrate export --yes 2>&1 | tee migrate.log
if [ ${PIPESTATUS[0]} -eq 0 ]; then
echo >&2 "fatal: expected git lfs migrate export to fail, didn't"
exit 1
@ -330,7 +330,7 @@ begin_test "migrate export (include/exclude ref)"
--include="*.txt" \
--include-ref=refs/heads/my-feature \
--exclude-ref=refs/heads/master
assert_pointer "refs/heads/master" "a.md" "$md_master_oid" "21"
assert_pointer "refs/heads/master" "a.txt" "$txt_master_oid" "20"
@ -432,7 +432,8 @@ begin_test "migrate export (invalid --remote)"
setup_single_remote_branch_tracked
git lfs migrate export --include="*" --remote="zz" 2>&1 | tee migrate.log
git lfs migrate export --include="*" --remote="zz" --yes 2>&1 \
| tee migrate.log
if [ ${PIPESTATUS[0]} -eq 0 ]; then
echo >&2 "fatal: expected git lfs migrate export to fail, didn't"
exit 1

@ -553,6 +553,28 @@ setup_local_branch_with_symlink() {
git commit -m "add symlink"
}
# setup_local_branch_with_dirty_copy creates a repository as follows:
#
# A
# \
# refs/heads/master
#
# - Commit 'A' has the contents "a.txt\n" in a.txt, and marks a.txt as unclean
# in the working copy.
setup_local_branch_with_dirty_copy() {
set -e
reponame="migrate-single-local-branch-with-dirty-copy"
remove_and_create_local_repo "$reponame"
echo "a.txt" > a.txt
git add a.txt
git commit -m "initial commit"
echo "2" >> a.txt
}
# make_bare converts the existing full checkout of a repository into a bare one,
# and then `cd`'s into it.
make_bare() {
@ -587,4 +609,6 @@ remove_and_create_remote_repo() {
setup_remote_repo "$reponame"
clone_repo "$reponame" "$reponame"
rm clone.log
}

@ -11,7 +11,7 @@ begin_test "migrate import (--fixup)"
txt_oid="$(calc_oid "$(git cat-file -p :a.txt)")"
git lfs migrate import --everything --fixup
git lfs migrate import --everything --fixup --yes
assert_pointer "refs/heads/master" "a.txt" "$txt_oid" "120"
assert_local_object "$txt_oid" "120"
@ -31,7 +31,7 @@ begin_test "migrate import (--fixup, complex nested)"
a_oid="$(calc_oid "$(git cat-file -p :a.txt)")"
b_oid="$(calc_oid "$(git cat-file -p :dir/b.txt)")"
git lfs migrate import --everything --fixup
git lfs migrate import --everything --fixup --yes
assert_pointer "refs/heads/master" "a.txt" "$a_oid" "1"
refute_pointer "refs/heads/master" "b.txt"
@ -53,7 +53,7 @@ begin_test "migrate import (--fixup, --include)"
setup_single_local_branch_tracked_corrupt
git lfs migrate import --everything --fixup --include="*.txt" 2>&1 \
git lfs migrate import --everything --fixup --yes --include="*.txt" 2>&1 \
| tee migrate.log
if [ "${PIPESTATUS[0]}" -eq 0 ]; then
@ -71,7 +71,7 @@ begin_test "migrate import (--fixup, --exclude)"
setup_single_local_branch_tracked_corrupt
git lfs migrate import --everything --fixup --exclude="*.txt" 2>&1 \
git lfs migrate import --everything --fixup --yes --exclude="*.txt" 2>&1 \
| tee migrate.log
if [ "${PIPESTATUS[0]}" -eq 0 ]; then
@ -89,7 +89,7 @@ begin_test "migrate import (--fixup, --no-rewrite)"
setup_single_local_branch_tracked_corrupt
git lfs migrate import --everything --fixup --no-rewrite 2>&1 \
git lfs migrate import --everything --fixup --yes --no-rewrite 2>&1 \
| tee migrate.log
if [ "${PIPESTATUS[0]}" -eq 0 ]; then

@ -12,7 +12,7 @@ begin_test "migrate import --no-rewrite (default branch)"
txt_oid="$(calc_oid "$(git cat-file -p :a.txt)")"
prev_commit_oid="$(git rev-parse HEAD)"
git lfs migrate import --no-rewrite *.txt
git lfs migrate import --no-rewrite --yes *.txt
# Ensure our desired files were imported into git-lfs
assert_pointer "refs/heads/master" "a.txt" "$txt_oid" "120"
@ -46,7 +46,7 @@ begin_test "migrate import --no-rewrite (bare repository)"
txt_oid="$(calc_oid "$(git cat-file -p :a.txt)")"
md_oid="$(calc_oid "$(git cat-file -p :a.md)")"
git lfs migrate import --no-rewrite a.txt a.md
git lfs migrate import --no-rewrite --yes a.txt a.md
# Ensure our desired files were imported
assert_pointer "refs/heads/master" "a.txt" "$txt_oid" "30"
@ -78,7 +78,7 @@ begin_test "migrate import --no-rewrite (multiple branches)"
txt_oid="$(calc_oid "$(git cat-file -p :a.txt)")"
md_feature_oid="$(calc_oid "$(git cat-file -p my-feature:a.md)")"
git lfs migrate import --no-rewrite *.txt *.md
git lfs migrate import --no-rewrite --yes *.txt *.md
# Ensure our desired files were imported
assert_pointer "refs/heads/master" "a.md" "$md_oid" "140"
@ -111,7 +111,7 @@ begin_test "migrate import --no-rewrite (no .gitattributes)"
setup_multiple_local_branches
# Ensure command fails if no .gitattributes files are present
git lfs migrate import --no-rewrite *.txt *.md 2>&1 | tee migrate.log
git lfs migrate import --no-rewrite --yes *.txt *.md 2>&1 | tee migrate.log
if [ ${PIPESTATUS[0]} -eq 0 ]; then
echo >&2 "fatal: expected git lfs migrate import --no-rewrite to fail, didn't"
exit 1
@ -139,7 +139,7 @@ begin_test "migrate import --no-rewrite (nested .gitattributes)"
nested_md_oid="$(calc_oid "$(git cat-file -p :b/a.md)")"
txt_oid="$(calc_oid "$(git cat-file -p :a.txt)")"
git lfs migrate import --no-rewrite a.txt b/a.md
git lfs migrate import --no-rewrite --yes a.txt b/a.md
# Ensure a.txt and subtree/a.md were imported, even though *.md only exists in the
# nested subtree/.gitattributes file
@ -152,7 +152,7 @@ begin_test "migrate import --no-rewrite (nested .gitattributes)"
# Failure should occur when trying to import a.md as no entry exists in
# top-level .gitattributes file
git lfs migrate import --no-rewrite a.md 2>&1 | tee migrate.log
git lfs migrate import --no-rewrite --yes a.md 2>&1 | tee migrate.log
if [ ${PIPESTATUS[0]} -eq 0 ]; then
echo >&2 "fatal: expected git lfs migrate import --no-rewrite to fail, didn't"
exit 1
@ -171,7 +171,7 @@ begin_test "migrate import --no-rewrite (with commit message)"
prev_commit_oid="$(git rev-parse HEAD)"
expected_commit_msg="run git-lfs migrate import --no-rewrite"
git lfs migrate import --message "$expected_commit_msg" --no-rewrite *.txt
git lfs migrate import --message "$expected_commit_msg" --no-rewrite --yes *.txt
# Ensure the git history remained the same
new_commit_oid="$(git rev-parse HEAD~1)"
@ -201,7 +201,7 @@ begin_test "migrate import --no-rewrite (with empty commit message)"
prev_commit_oid="$(git rev-parse HEAD)"
git lfs migrate import -m "" --no-rewrite *.txt
git lfs migrate import -m "" --no-rewrite --yes *.txt
# Ensure the git history remained the same
new_commit_oid="$(git rev-parse HEAD~1)"

@ -417,7 +417,7 @@ begin_test "migrate import (existing .gitattributes)"
txt_master_oid="$(calc_oid "$(git cat-file -p "$master:a.txt")")"
git lfs migrate import --include-ref=refs/heads/master --include="*.txt"
git lfs migrate import --yes --include-ref=refs/heads/master --include="*.txt"
assert_local_object "$txt_master_oid" "120"