Merge pull request #2642 from git-lfs/fix-unlock-by-id

ensure files are marked readonly after unlocking by ID

Author: technoweenie

locking/lockable.go

@@ -39,7 +39,6 @@ func (c *Client) ensureLockablesLoaded() {
 // Internal function to repopulate lockable patterns
 // You must have locked the c.lockableMutex in the caller
 func (c *Client) refreshLockablePatterns() {
-
 	paths := git.GetAttributePaths(c.LocalWorkingDir, c.LocalGitDir)
 	// Always make non-nil even if empty
 	c.lockablePatterns = make([]string, 0, len(paths))

locking/locks.go

@@ -144,22 +144,7 @@ func (c *Client) UnlockFile(path string, force bool) error {
 		return fmt.Errorf("Unable to get lock id: %v", err)
 	}
 
-	err = c.UnlockFileById(id, force)
-	if err != nil {
-		return err
-	}
-
-	abs, err := getAbsolutePath(path)
-	if err != nil {
-		return errors.Wrap(err, "make lockpath absolute")
-	}
-
-	// Make non-writeable if required
-	if c.SetLockableFilesReadOnly && c.IsFileLockable(path) {
-		return tools.SetFileWriteFlag(abs, false)
-	}
-	return nil
-
+	return c.UnlockFileById(id, force)
 }
 
 // UnlockFileById attempts to unlock a lock with a given id on the current remote
@@ -181,6 +166,18 @@ func (c *Client) UnlockFileById(id string, force bool) error {
 		return fmt.Errorf("Error caching unlock information: %v", err)
 	}
 
+	if unlockRes.Lock != nil {
+		abs, err := getAbsolutePath(unlockRes.Lock.Path)
+		if err != nil {
+			return errors.Wrap(err, "make lockpath absolute")
+		}
+
+		// Make non-writeable if required
+		if c.SetLockableFilesReadOnly && c.IsFileLockable(unlockRes.Lock.Path) {
+			return tools.SetFileWriteFlag(abs, false)
+		}
+	}
+
 	return nil
 }
 

test/test-unlock.sh

@@ -34,7 +34,7 @@ begin_test "unlocking a file makes it readonly"
 )
 end_test
 
-begin_test "unlocking a file makes ignores readonly"
+begin_test "unlocking a file ignores readonly"
 (
   set -e
 
@@ -99,15 +99,16 @@ begin_test "unlocking a lock by id"
   set -e
 
   reponame="unlock_by_id"
-  setup_remote_repo_with_file "unlock_by_id" "d.dat"
+  setup_remote_repo_with_file "$reponame" "d.dat"
 
   git lfs lock --json "d.dat" | tee lock.log
+  assert_file_writeable d.dat
 
   id=$(assert_lock lock.log d.dat)
   assert_server_lock "$reponame" "$id"
 
   git lfs unlock --id="$id"
-  refute_server_lock "$reponame" "$id"
+  refute_file_writeable d.dat
 )
 end_test