Merge pull request git-lfs#1978 from git-lfs/lock-messages

Improve lock messages

Author: technoweenie

commands/command_lock.go

@@ -42,7 +42,7 @@ func lockCommand(cmd *cobra.Command, args []string) {
 		return
 	}
 
-	Print("\n'%s' was locked (%s)", args[0], lock.Id)
+	Print("Locked %s", args[0])
 }
 
 // lockPaths relativizes the given filepath such that it is relative to the root

commands/command_locks.go

@@ -3,7 +3,11 @@ package commands
 import (
 	"encoding/json"
 	"os"
+	"sort"
+	"strings"
 
+	"github.com/git-lfs/git-lfs/locking"
+	"github.com/git-lfs/git-lfs/tools"
 	"github.com/spf13/cobra"
 )
 
@@ -20,7 +24,6 @@ func locksCommand(cmd *cobra.Command, args []string) {
 	lockClient := newLockClient(lockRemote)
 	defer lockClient.Close()
 
-	var lockCount int
 	locks, err := lockClient.SearchLocks(filters, locksCmdFlags.Limit, locksCmdFlags.Local)
 	// Print any we got before exiting
 
@@ -31,15 +34,25 @@ func locksCommand(cmd *cobra.Command, args []string) {
 		return
 	}
 
+	var maxlen int
+	lockPaths := make([]string, 0, len(locks))
+	locksByPath := make(map[string]locking.Lock)
 	for _, lock := range locks {
-		Print("%s\t%s", lock.Path, lock.Owner)
-		lockCount++
+		lockPaths = append(lockPaths, lock.Path)
+		locksByPath[lock.Path] = lock
+		maxlen = tools.MaxInt(maxlen, len(lock.Path))
+	}
+
+	sort.Strings(lockPaths)
+	for _, lockPath := range lockPaths {
+		lock := locksByPath[lockPath]
+		padding := tools.MaxInt(maxlen-len(lock.Path), 0)
+		Print("%s%s\t%s", lock.Path, strings.Repeat(" ", padding), lock.Owner)
 	}
 
 	if err != nil {
 		Exit("Error while retrieving locks: %v", err)
 	}
-	Print("\n%d lock(s) matched query.", lockCount)
 }
 
 // locksFlags wraps up and holds all of the flags that can be given to the

commands/command_unlock.go

@@ -61,7 +61,7 @@ func unlockCommand(cmd *cobra.Command, args []string) {
 		}
 		return
 	}
-	Print("'%s' was unlocked", args[0])
+	Print("Unlocked %s", args[0])
 }
 
 func unlockAbortIfFileModified(path string) {

test/test-lock.sh

@@ -9,10 +9,8 @@ begin_test "creating a lock"
   reponame="lock_create_simple"
   setup_remote_repo_with_file "$reponame" "a.dat"
 
-  git lfs lock "a.dat" | tee lock.log
-  grep "'a.dat' was locked" lock.log
-
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "a.dat" | tee lock.json
+  id=$(assert_lock lock.json a.dat)
   assert_server_lock "$reponame" "$id"
 )
 end_test
@@ -24,25 +22,22 @@ begin_test "create lock with server using client cert"
   setup_remote_repo_with_file "$reponame" "cc.dat"
 
   git config lfs.url "$CLIENTCERTGITSERVER/$reponame.git/info/lfs"
-  git lfs lock "cc.dat" | tee lock.log
-  grep "'cc.dat' was locked" lock.log
-
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "cc.dat" | tee lock.json
+  id=$(assert_lock lock.json cc.dat)
   assert_server_lock "$reponame" "$id"
 )
 end_test
 
-begin_test "creating a lock (--json)"
+begin_test "creating a lock (with output)"
 (
   set -e
 
-  reponame="lock_create_simple_json"
-  setup_remote_repo_with_file "$reponame" "a_json.dat"
+  reponame="lock_create_simple_output"
+  setup_remote_repo_with_file "$reponame" "a_output.dat"
 
-  git lfs lock --json "a_json.dat" | tee lock.log
-  grep "\"path\":\"a_json.dat\"" lock.log
-
-  id=$(grep -o "\"id\":\".*\"" lock.log | cut -d \" -f 4)
+  git lfs lock "a_output.dat" | tee lock.log
+  grep "Locked a_output.dat" lock.log
+  id=$(grep -oh "\((.*)\)" lock.log | tr -d \(\))
   assert_server_lock "$reponame" "$id"
 )
 end_test
@@ -54,10 +49,8 @@ begin_test "locking a previously locked file"
   reponame="lock_create_previously_created"
   setup_remote_repo_with_file "$reponame" "b.dat"
 
-  git lfs lock "b.dat" | tee lock.log
-  grep "'b.dat' was locked" lock.log
-
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "b.dat" | tee lock.json
+  id=$(assert_lock lock.json b.dat)
   assert_server_lock "$reponame" "$id"
 
   grep "lock already created" <(git lfs lock "b.dat" 2>&1)

test/test-locks.sh

@@ -9,13 +9,13 @@ begin_test "list a single lock"
   reponame="locks_list_single"
   setup_remote_repo_with_file "$reponame" "f.dat"
 
-  git lfs lock "f.dat" | tee lock.log
+  git lfs lock --json "f.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log f.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs locks --path "f.dat" | tee locks.log
-  grep "1 lock(s) matched query" locks.log
+  [ $(wc -l < locks.log) -eq 1 ]
   grep "f.dat" locks.log
   grep "Git LFS Tests" locks.log
 )
@@ -28,9 +28,9 @@ begin_test "list a single lock (--json)"
   reponame="locks_list_single_json"
   setup_remote_repo_with_file "$reponame" "f_json.dat"
 
-  git lfs lock "f_json.dat" | tee lock.log
+  git lfs lock --json "f_json.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log f_json.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs locks --json --path "f_json.dat" | tee locks.log
@@ -62,14 +62,14 @@ begin_test "list locks with a limit"
   git push origin master 2>&1 | tee push.log
   grep "master -> master" push.log
 
-  git lfs lock "g_1.dat" | tee lock.log
-  assert_server_lock "$reponame" "$(get_lock_id "lock.log")"
+  git lfs lock --json "g_1.dat" | tee lock.log
+  assert_server_lock "$reponame" "$(assert_log "lock.log" g_1.dat)"
 
-  git lfs lock "g_2.dat" | tee lock.log
-  assert_server_lock "$reponame" "$(get_lock_id "lock.log")"
+  git lfs lock --json "g_2.dat" | tee lock.log
+  assert_server_lock "$reponame" "$(assert_lock "lock.log" g_2.dat)"
 
   git lfs locks --limit 1 | tee locks.log
-  grep "1 lock(s) matched query" locks.log
+  [ $(wc -l < locks.log) -eq 1 ]
 )
 end_test
 
@@ -99,13 +99,13 @@ begin_test "list locks with pagination"
   grep "master -> master" push.log
 
   for i in $(seq 1 5); do
-    git lfs lock "h_$i.dat" | tee lock.log
-    assert_server_lock "$reponame" "$(get_lock_id "lock.log")"
+    git lfs lock --json "h_$i.dat" | tee lock.log
+    assert_server_lock "$reponame" "$(assert_lock "lock.log" "h_$1.dat")"
   done
 
   # The server will return, at most, three locks at a time
   git lfs locks --limit 4 | tee locks.log
-  grep "4 lock(s) matched query" locks.log
+  [ $(wc -l < locks.log) -eq 4 ]
 )
 end_test
 
@@ -131,23 +131,23 @@ begin_test "cached locks"
   git push origin master 2>&1 | tee push.log
   grep "master -> master" push.log
 
-  git lfs lock "cached1.dat" | tee lock.log
-  assert_server_lock "$(get_lock_id "lock.log")"
+  git lfs lock --json "cached1.dat" | tee lock.log
+  assert_server_lock "$(assert_lock "lock.log" cached1.dat)"
 
-  git lfs lock "cached2.dat" | tee lock.log
-  assert_server_lock "$(get_lock_id "lock.log")"
+  git lfs lock --json "cached2.dat" | tee lock.log
+  assert_server_lock "$(assert_lock "lock.log" cached2.dat)"
 
   git lfs locks --local | tee locks.log
-  grep "2 lock(s) matched query" locks.log
+  [ $(wc -l < locks.log) -eq 2 ]
 
   # delete the remote to prove we're using the local records
   git remote remove origin
 
   git lfs locks --local --path "cached1.dat" | tee locks.log
-  grep "1 lock(s) matched query" locks.log
+  [ $(wc -l < locks.log) -eq 1 ]
   grep "cached1.dat" locks.log
 
   git lfs locks --local --limit 1 | tee locks.log
-  grep "1 lock(s) matched query" locks.log
+  [ $(wc -l < locks.log) -eq 1 ]
 )
 end_test

test/test-pre-push.sh

@@ -470,10 +470,9 @@ begin_test "pre-push with our lock"
 
   git push origin master
 
-  git lfs lock "locked.dat" | tee lock.log
-  grep "'locked.dat' was locked" lock.log
+  git lfs lock --json "locked.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log locked.dat)
   assert_server_lock $id
 
   printf "authorized changes" >> locked.dat
@@ -509,10 +508,8 @@ begin_test "pre-push with their lock on lfs file"
 
   git push origin master
 
-  git lfs lock "locked_theirs.dat" | tee lock.log
-  grep "'locked_theirs.dat' was locked" lock.log
-
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "locked_theirs.dat" | tee lock.log
+  id=$(assert_lock lock.log locked_theirs.dat)
   assert_server_lock $id
 
   pushd "$TRASHDIR" >/dev/null
@@ -552,14 +549,12 @@ begin_test "pre-push with their lock on non-lfs lockable file"
 
   git push origin master
 
-  git lfs lock "tiny_locked_theirs.dat" | tee lock.log
-  grep "'tiny_locked_theirs.dat' was locked" lock.log
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "tiny_locked_theirs.dat" | tee lock.log
+  id=$(assert_lock lock.log tiny_locked_theirs.dat)
   assert_server_lock $id
 
-  git lfs lock "large_locked_theirs.dat" | tee lock.log
-  grep "'large_locked_theirs.dat' was locked" lock.log
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "large_locked_theirs.dat" | tee lock.log
+  id=$(assert_lock lock.log large_locked_theirs.dat)
   assert_server_lock $id
 
   pushd "$TRASHDIR" >/dev/null

test/test-unlock.sh

@@ -9,9 +9,9 @@ begin_test "unlocking a lock by path"
   reponame="unlock_by_path"
   setup_remote_repo_with_file "unlock_by_path" "c.dat"
 
-  git lfs lock "c.dat" | tee lock.log
+  git lfs lock --json "c.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log c.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs unlock "c.dat" 2>&1 | tee unlock.log
@@ -26,8 +26,8 @@ begin_test "force unlocking lock with missing file"
   reponame="force-unlock-missing-file"
   setup_remote_repo_with_file "$reponame" "a.dat"
 
-  git lfs lock "a.dat" | tee lock.log
-  id=$(get_lock_id lock.log)
+  git lfs lock --json "a.dat" | tee lock.log
+  id=$(assert_lock lock.log a.dat)
   assert_server_lock "$reponame" "$id"
 
   git rm a.dat
@@ -52,9 +52,9 @@ begin_test "unlocking a lock (--json)"
   reponame="unlock_by_path_json"
   setup_remote_repo_with_file "$reponame" "c_json.dat"
 
-  git lfs lock "c_json.dat" | tee lock.log
+  git lfs lock --json "c_json.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log c_json.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs unlock --json "c_json.dat" 2>&1 | tee unlock.log
@@ -71,9 +71,9 @@ begin_test "unlocking a lock by id"
   reponame="unlock_by_id"
   setup_remote_repo_with_file "unlock_by_id" "d.dat"
 
-  git lfs lock "d.dat" | tee lock.log
+  git lfs lock --json "d.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log d.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs unlock --id="$id" 2>&1 | tee unlock.log
@@ -88,9 +88,9 @@ begin_test "unlocking a lock without sufficient info"
   reponame="unlock_ambiguous"
   setup_remote_repo_with_file "$reponame" "e.dat"
 
-  git lfs lock "e.dat" | tee lock.log
+  git lfs lock --json "e.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log e.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs unlock 2>&1 | tee unlock.log
@@ -106,9 +106,9 @@ begin_test "unlocking a lock while uncommitted"
   reponame="unlock_modified"
   setup_remote_repo_with_file "$reponame" "mod.dat"
 
-  git lfs lock "mod.dat" | tee lock.log
+  git lfs lock --json "mod.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log mod.dat)
   assert_server_lock "$reponame" "$id"
 
   echo "\nSomething" >> mod.dat
@@ -134,9 +134,9 @@ begin_test "unlocking a lock while uncommitted with --force"
   reponame="unlock_modified_force"
   setup_remote_repo_with_file "$reponame" "modforce.dat"
 
-  git lfs lock "modforce.dat" | tee lock.log
+  git lfs lock --json "modforce.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log modforce.dat)
   assert_server_lock "$reponame" "$id"
 
   echo "\nSomething" >> modforce.dat
@@ -159,9 +159,9 @@ begin_test "unlocking a lock while untracked"
   # Create file but don't add it to git
   # Shouldn't be able to unlock it
   echo "something" > untracked.dat
-  git lfs lock "untracked.dat" | tee lock.log
+  git lfs lock --json "untracked.dat" | tee lock.log
 
-  id=$(get_lock_id lock.log)
+  id=$(assert_lock lock.log untracked.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs unlock "untracked.dat" 2>&1 | tee unlock.log

test/testhelpers.sh

@@ -120,10 +120,21 @@ assert_server_object() {
   }
 }
 
-# Parses the Lock ID returned from a single 'git lfs lock <path>' call
-get_lock_id() {
+# This asserts the lock path and returns the lock ID by parsing the response of
+#
+#   git lfs lock --json <path>
+assert_lock() {
   local log="$1"
-  grep -oh "\((.*)\)" "$log" | tr -d \(\)
+  local path="$2"
+
+  if [ $(grep -c "\"path\":\"$path\"" "$log") -eq 0 ]; then
+    echo "path '$path' not found in:"
+    cat "$log"
+    exit 1
+  fi
+
+  local jsonid=$(grep -oh "\"id\":\"\w\+\"" "$log")
+  echo "${jsonid:3}" | tr -d \"\:
 }
 
 # assert that a lock with the given ID exists on the test server