Merge branch 'master' into locking/docs

commands/command_unlock.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"os"
 
+	"github.com/git-lfs/git-lfs/git"
+	"github.com/git-lfs/git-lfs/locking"
 	"github.com/spf13/cobra"
 )
 
@@ -31,11 +33,18 @@ func unlockCommand(cmd *cobra.Command, args []string) {
 			Exit("Unable to determine path: %v", err.Error())
 		}
 
+		// This call can early-out
+		unlockAbortIfFileModified(path)
+
 		err = lockClient.UnlockFile(path, unlockCmdFlags.Force)
 		if err != nil {
 			Exit("Unable to unlock: %v", err.Error())
 		}
 	} else if unlockCmdFlags.Id != "" {
+
+		// This call can early-out
+		unlockAbortIfFileModifiedById(unlockCmdFlags.Id, lockClient)
+
 		err := lockClient.UnlockFileById(unlockCmdFlags.Id, unlockCmdFlags.Force)
 		if err != nil {
 			Exit("Unable to unlock %v: %v", unlockCmdFlags.Id, err.Error())
@@ -55,6 +64,43 @@ func unlockCommand(cmd *cobra.Command, args []string) {
 	Print("'%s' was unlocked", args[0])
 }
 
+func unlockAbortIfFileModified(path string) {
+	modified, err := git.IsFileModified(path)
+
+	if err != nil {
+		Exit(err.Error())
+	}
+
+	if modified {
+		if unlockCmdFlags.Force {
+			// Only a warning
+			Error("Warning: unlocking with uncommitted changes because --force")
+		} else {
+			Exit("Cannot unlock file with uncommitted changes")
+		}
+
+	}
+}
+
+func unlockAbortIfFileModifiedById(id string, lockClient *locking.Client) {
+	// Get the path so we can check the status
+	filter := map[string]string{"id": id}
+	// try local cache first
+	locks, _ := lockClient.SearchLocks(filter, 0, true)
+	if len(locks) == 0 {
+		// Fall back on calling server
+		locks, _ = lockClient.SearchLocks(filter, 0, false)
+	}
+
+	if len(locks) == 0 {
+		// Don't block if we can't determine the path, may be cleaning up old data
+		return
+	}
+
+	unlockAbortIfFileModified(locks[0].Path)
+
+}
+
 func init() {
 	if !isCommandEnabled(cfg, "locks") {
 		return

commands/uploader.go

@@ -27,14 +27,14 @@ type uploadContext struct {
 	trackedLocksMu *sync.Mutex
 
 	// ALL verifiable locks
-	ourLocks   map[string]locking.Lock
+	ourLocks   map[string]*locking.Lock
-	theirLocks map[string]locking.Lock
+	theirLocks map[string]*locking.Lock
 
 	// locks from ourLocks that were modified in this push
-	ownedLocks []locking.Lock
+	ownedLocks []*locking.Lock
 
 	// locks from theirLocks that were modified in this push
-	unownedLocks []locking.Lock
+	unownedLocks []*locking.Lock
 }
 
 func newUploadContext(remote string, dryRun bool) *uploadContext {
@@ -45,8 +45,8 @@ func newUploadContext(remote string, dryRun bool) *uploadContext {
 		Manifest:       getTransferManifest(),
 		DryRun:         dryRun,
 		uploadedOids:   tools.NewStringSet(),
-		ourLocks:       make(map[string]locking.Lock),
+		ourLocks:       make(map[string]*locking.Lock),
-		theirLocks:     make(map[string]locking.Lock),
+		theirLocks:     make(map[string]*locking.Lock),
 		trackedLocksMu: new(sync.Mutex),
 	}
 
@@ -61,10 +61,10 @@ func newUploadContext(remote string, dryRun bool) *uploadContext {
 		Error("         Temporarily skipping check ...")
 	} else {
 		for _, l := range theirLocks {
-			ctx.theirLocks[l.Path] = l
+			ctx.theirLocks[l.Path] = &l
 		}
 		for _, l := range ourLocks {
-			ctx.ourLocks[l.Path] = l
+			ctx.ourLocks[l.Path] = &l
 		}
 	}
 

git/git.go

@@ -17,6 +17,7 @@ import (
 	"sync"
 	"time"
 
+	lfserrors "github.com/git-lfs/git-lfs/errors"
 	"github.com/git-lfs/git-lfs/subprocess"
 	"github.com/rubyist/tracerx"
 )
@@ -1092,3 +1093,44 @@ func GetFilesChanged(from, to string) ([]string, error) {
 
 	return files, err
 }
+
+// IsFileModified returns whether the filepath specified is modified according
+// to `git status`. A file is modified if it has uncommitted changes in the
+// working copy or the index. This includes being untracked.
+func IsFileModified(filepath string) (bool, error) {
+
+	args := []string{
+		"-c", "core.quotepath=false", // handle special chars in filenames
+		"status",
+		"--porcelain",
+		"--", // separator in case filename ambiguous
+		filepath,
+	}
+	cmd := subprocess.ExecCommand("git", args...)
+	outp, err := cmd.StdoutPipe()
+	if err != nil {
+		return false, lfserrors.Wrap(err, "Failed to call git status")
+	}
+	if err := cmd.Start(); err != nil {
+		return false, lfserrors.Wrap(err, "Failed to start git status")
+	}
+	matched := false
+	for scanner := bufio.NewScanner(outp); scanner.Scan(); {
+		line := scanner.Text()
+		// Porcelain format is "<I><W> <filename>"
+		// Where <I> = index status, <W> = working copy status
+		if len(line) > 3 {
+			// Double-check even though should be only match
+			if strings.TrimSpace(line[3:]) == filepath {
+				matched = true
+				// keep consuming output to exit cleanly
+				// will typically fall straight through anyway due to 1 line output
+			}
+		}
+	}
+	if err := cmd.Wait(); err != nil {
+		return false, lfserrors.Wrap(err, "Git status failed")
+	}
+
+	return matched, nil
+}

locking/api.go

@@ -224,7 +224,7 @@ type lockVerifiableList struct {
 }
 
 func (c *lockClient) SearchVerifiable(remote string, vreq *lockVerifiableRequest) (*lockVerifiableList, *http.Response, error) {
-	e := c.Endpoints.Endpoint("download", remote)
+	e := c.Endpoints.Endpoint("upload", remote)
 	req, err := c.NewRequest("POST", e, "locks/verify", vreq)
 	if err != nil {
 		return nil, nil, err

test/test-unlock.sh

@@ -72,3 +72,83 @@ begin_test "unlocking a lock without sufficient info"
   assert_server_lock "$reponame" "$id"
 )
 end_test
+
+begin_test "unlocking a lock while uncommitted"
+(
+  set -e
+
+  reponame="unlock_modified"
+  setup_remote_repo_with_file "$reponame" "mod.dat"
+
+  GITLFSLOCKSENABLED=1 git lfs lock "mod.dat" | tee lock.log
+
+  id=$(grep -oh "\((.*)\)" lock.log | tr -d "()")
+  assert_server_lock "$reponame" "$id"
+
+  echo "\nSomething" >> mod.dat
+
+  GITLFSLOCKSENABLED=1 git lfs unlock "mod.dat" 2>&1 | tee unlock.log
+  [ ${PIPESTATUS[0]} -ne "0" ]
+
+  grep "Cannot unlock file with uncommitted changes" unlock.log
+
+  assert_server_lock "$reponame" "$id"
+
+  # should allow after discard
+  git checkout mod.dat
+  GITLFSLOCKSENABLED=1 git lfs unlock "mod.dat" 2>&1 | tee unlock.log
+  refute_server_lock "$reponame" "$id"
+)
+end_test
+
+begin_test "unlocking a lock while uncommitted with --force"
+(
+  set -e
+
+  reponame="unlock_modified_force"
+  setup_remote_repo_with_file "$reponame" "modforce.dat"
+
+  GITLFSLOCKSENABLED=1 git lfs lock "modforce.dat" | tee lock.log
+
+  id=$(grep -oh "\((.*)\)" lock.log | tr -d "()")
+  assert_server_lock "$reponame" "$id"
+
+  echo "\nSomething" >> modforce.dat
+
+  # should allow with --force
+  GITLFSLOCKSENABLED=1 git lfs unlock --force "modforce.dat" 2>&1 | tee unlock.log
+  grep "Warning: unlocking with uncommitted changes" unlock.log
+  refute_server_lock "$reponame" "$id"
+)
+end_test
+
+begin_test "unlocking a lock while untracked"
+(
+  set -e
+
+  reponame="unlock_untracked"
+  setup_remote_repo_with_file "$reponame" "notrelevant.dat"
+
+  git lfs track "*.dat"
+  # Create file but don't add it to git
+  # Shouldn't be able to unlock it
+  echo "something" > untracked.dat
+  GITLFSLOCKSENABLED=1 git lfs lock "untracked.dat" | tee lock.log
+
+  id=$(grep -oh "\((.*)\)" lock.log | tr -d "()")
+  assert_server_lock "$reponame" "$id"
+
+  GITLFSLOCKSENABLED=1 git lfs unlock "untracked.dat" 2>&1 | tee unlock.log
+  [ ${PIPESTATUS[0]} -ne "0" ]
+
+  grep "Cannot unlock file with uncommitted changes" unlock.log
+
+  assert_server_lock "$reponame" "$id"
+
+  # should allow after add/commit
+  git add untracked.dat
+  git commit -m "Added untracked"
+  GITLFSLOCKSENABLED=1 git lfs unlock "untracked.dat" 2>&1 | tee unlock.log
+  refute_server_lock "$reponame" "$id"
+)
+end_test