Merge pull request #619 from ttaylorr/promote-hooks

lfs: promote Hooks and Attributes to types

commands/command_uninit.go

@@ -6,12 +6,14 @@ import (
 )
 
 var (
+	// uninitCmd removes any configuration and hooks set by Git LFS.
 	uninitCmd = &cobra.Command{
 		Use:   "uninit",
 		Short: "Clear the Git LFS configuration",
 		Run:   uninitCommand,
 	}
 
+	// uninitHooksCmd removes any hooks created by Git LFS.
 	uninitHooksCmd = &cobra.Command{
 		Use:   "hooks",
 		Short: "Clear only the Git hooks for the current repository",

lfs/attribute.go

@@ -0,0 +1,88 @@
+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 {
+	// The Section of an Attribute refers to the location at which all
+	// properties are relative to. For example, for a Section with the value
+	// "core", Git will produce something like:
+	//
+	// [core]
+	//	autocrlf = true
+	//	...
+	Section string
+
+	// The Properties of an Attribute refer to all of the keys and values
+	// that define that Attribute.
+	Properties map[string]string
+}
+
+// Install instructs Git to set all keys and values relative to the root
+// location of this Attribute. For any particular key/value pair, if a matching
+// key is already set, it will be overridden if it is either a) empty, or b) the
+// `force` argument is passed as true. If an attribute is already set to a
+// different value than what is given, and force is false, an error will be
+// returned immediately, and the rest of the attributes will not be set.
+func (a *Attribute) Install(force bool) error {
+	for k, v := range a.Properties {
+		key := a.normalizeKey(k)
+		if err := a.set(key, v, force); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// normalizeKey makes an absolute path out of a partial relative one. For a
+// relative path of "foo", and a root Section of "bar", "bar.foo" will be returned.
+func (a *Attribute) normalizeKey(relative string) string {
+	return strings.Join([]string{a.Section, relative}, ".")
+}
+
+// set attempts to set a single key/value pair portion of this Attribute. If a
+// 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, force bool) error {
+	currentValue := git.Config.Find(key)
+	if force || shouldReset(currentValue) {
+		git.Config.UnsetGlobal(key)
+		git.Config.SetGlobal(key, value)
+
+		return nil
+	} else if currentValue != value {
+		return fmt.Errorf("The %s attribute should be \"%s\" but is \"%s\"",
+			key, value, currentValue)
+	}
+
+	return nil
+}
+
+// Uninstall removes all properties in the path of this property.
+func (a *Attribute) Uninstall() {
+	git.Config.UnsetGlobalSection(a.Section)
+}
+
+// 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 {
+	if len(value) == 0 {
+		return true
+	}
+
+	return valueRegexp.MatchString(value)
+}

lfs/hook.go

@@ -0,0 +1,123 @@
+package lfs
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// A Hook represents a githook as described in http://git-scm.com/docs/githooks.
+// Hooks have a type, which is the type of hook that they are, and a body, which
+// represents the thing they will execute when invoked by Git.
+type Hook struct {
+	Type         string
+	Contents     string
+	Upgradeables []string
+}
+
+func (h *Hook) Exists() bool {
+	_, err := os.Stat(h.Path())
+	return err == nil
+}
+
+// Path returns the desired (or actual, if installed) location where this hook
+// should be installed, relative to the local Git directory.
+func (h *Hook) Path() string {
+	return filepath.Join(LocalGitDir, "hooks", string(h.Type))
+}
+
+// Install installs this Git hook on disk, or upgrades it if it does exist, and
+// is upgradeable. It will create a hooks directory relative to the local Git
+// directory. It returns and halts at any errors, and returns nil if the
+// operation was a success.
+func (h *Hook) Install(force bool) error {
+	if !InRepo() {
+		return newInvalidRepoError(nil)
+	}
+
+	if err := os.MkdirAll(filepath.Join(LocalGitDir, "hooks"), 0755); err != nil {
+		return err
+	}
+
+	if h.Exists() && !force {
+		return h.Upgrade()
+	}
+
+	return h.write()
+}
+
+// write writes the contents of this Hook to disk, appending a newline at the
+// end, and sets the mode to octal 0755. It writes to disk unconditionally, and
+// returns at any error.
+func (h *Hook) write() error {
+	return ioutil.WriteFile(h.Path(), []byte(h.Contents+"\n"), 0755)
+}
+
+// Upgrade upgrades the (assumed to be) existing git hook to the current
+// contents. A hook is considered "upgrade-able" if its contents are matched in
+// the member variable `Upgradeables`. It halts and returns any errors as they
+// arise.
+func (h *Hook) Upgrade() error {
+	match, err := h.matchesCurrent()
+	if err != nil {
+		return err
+	}
+
+	if !match {
+		return nil
+	}
+
+	return h.write()
+}
+
+// Uninstall removes the hook on disk so long as it matches the current version,
+// or any of the past versions of this hook.
+func (h *Hook) Uninstall() error {
+	if !InRepo() {
+		return newInvalidRepoError(nil)
+	}
+
+	match, err := h.matchesCurrent()
+	if err != nil {
+		return err
+	}
+
+	if !match {
+		return nil
+	}
+
+	return os.RemoveAll(h.Path())
+}
+
+// matchesCurrent returns whether or not an existing git hook is able to be
+// written to or upgraded. A git hook matches those conditions if and only if
+// its contents match the current contents, or any past "upgrade-able" contents
+// of this hook.
+func (h *Hook) matchesCurrent() (bool, error) {
+	file, err := os.Open(h.Path())
+	if err != nil {
+		return false, err
+	}
+
+	by, err := ioutil.ReadAll(io.LimitReader(file, 1024))
+	file.Close()
+	if err != nil {
+		return false, err
+	}
+
+	contents := strings.TrimSpace(string(by))
+	if contents == h.Contents {
+		return true, nil
+	}
+
+	for _, u := range h.Upgradeables {
+		if u == contents {
+			return true, nil
+		}
+	}
+
+	return false, fmt.Errorf("Hook already exists: %s\n\n%s\n", string(h.Type), contents)
+}

lfs/setup.go

@@ -1,159 +1,69 @@
 package lfs
 
-import (
-	"errors"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"regexp"
-	"strings"
-
-	"github.com/github/git-lfs/git"
-)
-
 var (
-	valueRegexp = regexp.MustCompile("\\Agit[\\-\\s]media")
+	// prePushHook invokes `git lfs push` at the pre-push phase.
+	prePushHook = &Hook{
+		Type:     "pre-push",
+		Contents: "#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting .git/hooks/pre-push.\\n\"; exit 2; }\ngit lfs pre-push \"$@\"",
+		Upgradeables: []string{
+			"#!/bin/sh\ngit lfs push --stdin $*",
+			"#!/bin/sh\ngit lfs push --stdin \"$@\"",
+			"#!/bin/sh\ngit lfs pre-push \"$@\"",
+			"#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository has been set up with Git LFS but Git LFS is not installed.\\n\"; exit 0; }\ngit lfs pre-push \"$@\"",
+			"#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository has been set up with Git LFS but Git LFS is not installed.\\n\"; exit 2; }\ngit lfs pre-push \"$@\"",
+		},
+	}
 
-	prePushHook     = "#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting .git/hooks/pre-push.\\n\"; exit 2; }\ngit lfs pre-push \"$@\""
+	hooks = []*Hook{
-	prePushUpgrades = map[string]bool{
+		prePushHook,
-		"#!/bin/sh\ngit lfs push --stdin $*":     true,
+	}
-		"#!/bin/sh\ngit lfs push --stdin \"$@\"": true,
+
-		"#!/bin/sh\ngit lfs pre-push \"$@\"":     true,
+	filters = &Attribute{
-		"#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository has been set up with Git LFS but Git LFS is not installed.\\n\"; exit 0; }\ngit lfs pre-push \"$@\"": true,
+		Section: "filter.lfs",
-		"#!/bin/sh\ncommand -v git-lfs >/dev/null 2>&1 || { echo >&2 \"\\nThis repository has been set up with Git LFS but Git LFS is not installed.\\n\"; exit 2; }\ngit lfs pre-push \"$@\"": true,
+		Properties: map[string]string{
+			"clean":    "git-lfs clean %f",
+			"smudge":   "git-lfs smudge %f",
+			"required": "true",
+		},
 	}
 )
 
-type HookExists struct {
+// InstallHooks installs all hooks in the `hooks` var.
-	Name     string
-	Path     string
-	Contents string
-}
-
-func (e *HookExists) Error() string {
-	return fmt.Sprintf("Hook already exists: %s\n\n%s\n", e.Name, e.Contents)
-}
-
 func InstallHooks(force bool) error {
-	if !InRepo() {
+	for _, h := range hooks {
-		return newInvalidRepoError(nil)
+		if err := h.Install(force); err != nil {
+			return err
+		}
 	}
 
-	if err := os.MkdirAll(filepath.Join(LocalGitDir, "hooks"), 0755); err != nil {
+	return nil
-		return err
-	}
-
-	hookPath := filepath.Join(LocalGitDir, "hooks", "pre-push")
-	if _, err := os.Stat(hookPath); err == nil && !force {
-		return upgradeHookOrError(hookPath, "pre-push", prePushHook, prePushUpgrades)
-	}
-
-	return ioutil.WriteFile(hookPath, []byte(prePushHook+"\n"), 0755)
 }
 
+// UninstallHooks resmoves all hooks in range of the `hooks` var.
 func UninstallHooks() error {
-	if !InRepo() {
+	for _, h := range hooks {
-		return newInvalidRepoError(nil)
+		if err := h.Uninstall(); err != nil {
-	}
+			return err
-
+		}
-	prePushHookPath := filepath.Join(LocalGitDir, "hooks", "pre-push")
-	file, err := os.Open(prePushHookPath)
-	if err != nil {
-		// hook doesn't exist, our work here is done
-		return nil
-	}
-
-	by, err := ioutil.ReadAll(io.LimitReader(file, 1024))
-	file.Close()
-	if err != nil {
-		return err
-	}
-
-	contents := strings.TrimSpace(string(by))
-	if contents == prePushHook || prePushUpgrades[contents] {
-		return os.RemoveAll(prePushHookPath)
 	}
 
 	return nil
 }
 
-func upgradeHookOrError(hookPath, hookName, hook string, upgrades map[string]bool) error {
+// InstallFilters installs filters necessary for git-lfs to process normal git
-	file, err := os.Open(hookPath)
+// operations. Currently, that list includes:
-	if err != nil {
+//   - smudge filter
-		return err
+//   - clean filter
-	}
+//
-
+// An error will be returned if a filter is unable to be set, or if the required
-	by, err := ioutil.ReadAll(io.LimitReader(file, 1024))
+// filters were not present.
-	file.Close()
-	if err != nil {
-		return err
-	}
-
-	contents := strings.TrimSpace(string(by))
-	if contents == hook {
-		return nil
-	}
-
-	if upgrades[contents] {
-		return ioutil.WriteFile(hookPath, []byte(hook+"\n"), 0755)
-	}
-
-	return &HookExists{hookName, hookPath, contents}
-}
-
 func InstallFilters(force bool) error {
-	if err := setFilter("clean", force); err != nil {
+	return filters.Install(force)
-		return err
-	}
-	if err := setFilter("smudge", force); err != nil {
-		return err
-	}
-	if err := requireFilters(force); err != nil {
-		return err
-	}
-	return nil
 }
 
+// UninstallFilters proxies into the Uninstall method on the Filters type to
+// remove all installed filters.
 func UninstallFilters() error {
-	git.Config.UnsetGlobalSection("filter.lfs")
+	filters.Uninstall()
 	return nil
 }
-
-func setFilter(filterName string, force bool) error {
-	key := fmt.Sprintf("filter.lfs.%s", filterName)
-	value := fmt.Sprintf("git-lfs %s %%f", filterName)
-
-	existing := git.Config.Find(key)
-	if force || shouldReset(existing) {
-		git.Config.UnsetGlobal(key)
-		git.Config.SetGlobal(key, value)
-	} else if existing != value {
-		return fmt.Errorf("The %s filter should be \"%s\" but is \"%s\"", filterName, value, existing)
-	}
-
-	return nil
-}
-
-func requireFilters(force bool) error {
-	key := "filter.lfs.required"
-	value := "true"
-
-	existing := git.Config.Find(key)
-	if force || shouldReset(existing) {
-		git.Config.UnsetGlobal(key)
-		git.Config.SetGlobal(key, value)
-	} else if existing != value {
-		return errors.New("Git LFS filters should be required but are not.")
-	}
-
-	return nil
-}
-
-func shouldReset(value string) bool {
-	if len(value) == 0 {
-		return true
-	}
-	return valueRegexp.MatchString(value)
-}

test/test-init.sh

@@ -30,7 +30,7 @@ begin_test "init with old settings"
 
   [ "$res" = 2 ]
 
-  grep "clean filter should be" init.log
+  grep "clean attribute should be" init.log
   [ `grep -c "(MISSING)" init.log` = "0" ]
 
   [ "git lfs smudge %f" = "$(git config filter.lfs.smudge)" ]