Automatically upgrade old filters instead of requiring —force

Also remove upgrade path for `git-media` since over-general and should
not be needed any more

Author: sinbad

lfs/attribute.go

@@ -2,16 +2,11 @@ package lfs
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 
 	"github.com/github/git-lfs/git"
 )
 
-var (
-	valueRegexp = regexp.MustCompile("\\Agit[\\-\\s]media")
-)
-
 // Attribute wraps the structure and some operations of Git's conception of an
 // "attribute", as defined here: http://git-scm.com/docs/gitattributes.
 type Attribute struct {
@@ -27,6 +22,8 @@ type Attribute struct {
 	// The Properties of an Attribute refer to all of the keys and values
 	// that define that Attribute.
 	Properties map[string]string
+	// Previous values of these attributes that can be automatically upgraded
+	Upgradeables map[string][]string
 }
 
 // InstallOptions serves as an argument to Install().
@@ -44,8 +41,13 @@ type InstallOptions struct {
 // returned immediately, and the rest of the attributes will not be set.
 func (a *Attribute) Install(opt InstallOptions) error {
 	for k, v := range a.Properties {
+		var upgradeables []string
+		if a.Upgradeables != nil {
+			// use pre-normalised key since caller will have set up the same
+			upgradeables = a.Upgradeables[k]
+		}
 		key := a.normalizeKey(k)
-		if err := a.set(key, v, opt); err != nil {
+		if err := a.set(key, v, upgradeables, opt); err != nil {
 			return err
 		}
 	}
@@ -63,7 +65,7 @@ func (a *Attribute) normalizeKey(relative string) string {
 // matching key already exists and the value is not equal to the desired value,
 // an error will be thrown if force is set to false. If force is true, the value
 // will be overridden.
-func (a *Attribute) set(key, value string, opt InstallOptions) error {
+func (a *Attribute) set(key, value string, upgradeables []string, opt InstallOptions) error {
 	var currentValue string
 	if opt.Local {
 		currentValue = git.Config.FindLocal(key)
@@ -73,7 +75,7 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error {
 		currentValue = git.Config.FindGlobal(key)
 	}
 
-	if opt.Force || shouldReset(currentValue) {
+	if opt.Force || shouldReset(currentValue, upgradeables) {
 		var err error
 		if opt.Local {
 			// ignore error for unset, git returns non-zero if missing
@@ -106,11 +108,17 @@ func (a *Attribute) Uninstall() {
 
 // shouldReset determines whether or not a value is resettable given its current
 // value on the system. If the value is empty (length = 0), then it will pass.
-// Otherwise, it will pass if the below regex matches.
-func shouldReset(value string) bool {
+// It will also pass if it matches any upgradeable value
+func shouldReset(value string, upgradeables []string) bool {
 	if len(value) == 0 {
 		return true
 	}
 
-	return valueRegexp.MatchString(value)
+	for _, u := range upgradeables {
+		if value == u {
+			return true
+		}
+	}
+
+	return false
 }

lfs/setup.go

@@ -30,6 +30,10 @@ var (
 			"smudge":   "git-lfs smudge -- %f",
 			"required": "true",
 		},
+		Upgradeables: map[string][]string{
+			"clean":  []string{"git-lfs clean %f"},
+			"smudge": []string{"git-lfs smudge %f"},
+		},
 	}
 
 	passFilters = &Attribute{
@@ -39,6 +43,10 @@ var (
 			"smudge":   "git-lfs smudge --skip -- %f",
 			"required": "true",
 		},
+		Upgradeables: map[string][]string{
+			"clean":  []string{"git-lfs clean %f"},
+			"smudge": []string{"git-lfs smudge --skip %f"},
+		},
 	}
 )
 

test/test-install.sh

@@ -19,12 +19,12 @@ begin_test "install again"
 )
 end_test
 
-begin_test "install with old settings"
+begin_test "install with old (non-upgradeable) settings"
 (
   set -e
 
-  git config --global filter.lfs.smudge "git lfs smudge %f"
-  git config --global filter.lfs.clean "git lfs clean %f"
+  git config --global filter.lfs.smudge "git-lfs smudge --something %f"
+  git config --global filter.lfs.clean "git-lfs clean --something %f"
 
   set +e
   git lfs install 2> install.log
@@ -37,15 +37,29 @@ begin_test "install with old settings"
   grep -E "(clean|smudge) attribute should be" install.log
   [ `grep -c "(MISSING)" install.log` = "0" ]
 
-  [ "git lfs smudge %f" = "$(git config --global filter.lfs.smudge)" ]
-  [ "git lfs clean %f" = "$(git config --global filter.lfs.clean)" ]
+  [ "git-lfs smudge --something %f" = "$(git config --global filter.lfs.smudge)" ]
+  [ "git-lfs clean --something %f" = "$(git config --global filter.lfs.clean)" ]
 
   git lfs install --force
   [ "git-lfs smudge -- %f" = "$(git config --global filter.lfs.smudge)" ]
   [ "git-lfs clean -- %f" = "$(git config --global filter.lfs.clean)" ]
 )
 end_test
 
+begin_test "install with upgradeable settings"
+(
+  set -e
+
+  git config --global filter.lfs.smudge "git-lfs smudge %f"
+  git config --global filter.lfs.clean "git-lfs clean %f"
+
+  # should not need force, should upgrade this old style
+  git lfs install
+  [ "git-lfs smudge -- %f" = "$(git config --global filter.lfs.smudge)" ]
+  [ "git-lfs clean -- %f" = "$(git config --global filter.lfs.clean)" ]
+)
+end_test
+
 begin_test "install updates repo hooks"
 (
   set -e