repository: remove tie to Bug, improved and reusable testing

- allow the creation of arbitrary Lamport clocks, freeing the way to new entities and removing Bug specific (upper layer) code.
- generalize the memory-only and persisted Lamport clocks behind a common interface
- rework the tests to provide reusable testing code for a Repo, a Clock, a Config, opening a path to add a new Repo implementation more easily
- test previously untested components with those new tests

Note: one problem found during this endeavor is that `identity.Version` also need to store one time + Lamport time for each other Entity (Bug, config, PR ...). This could possibly done without breaking change but it would be much easier to wait for https://github.com/MichaelMure/git-bug-migration to happen.

1 files changed, 75 insertions, 127 deletions

diff --git a/repository/git.go b/repository/git.go
index 1eb3b8b5..e4cb57e6 100644
--- a/repository/git.go
+++ b/repository/git.go
@@ -8,30 +8,25 @@ import (
 	"os/exec"
 	"path"
 	"strings"
-
-	"github.com/pkg/errors"
+	"sync"
 
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
 const (
-	createClockFile = "/git-bug/create-clock"
-	editClockFile   = "/git-bug/edit-clock"
-)
-
-var (
-	// ErrNotARepo is the error returned when the git repo root wan't be found
-	ErrNotARepo = errors.New("not a git repository")
+	clockPath = "git-bug"
 )
 
 var _ ClockedRepo = &GitRepo{}
+var _ TestedRepo = &GitRepo{}
 
 // GitRepo represents an instance of a (local) git repository.
 type GitRepo struct {
-	Path        string
-	createClock *lamport.Persisted
-	editClock   *lamport.Persisted
+	path string
+
+	clocksMutex sync.Mutex
+	clocks      map[string]lamport.Clock
 }
 
 // LocalConfig give access to the repository scoped configuration
@@ -46,19 +41,14 @@ func (repo *GitRepo) GlobalConfig() Config {
 
 // Run the given git command with the given I/O reader/writers, returning an error if it fails.
 func (repo *GitRepo) runGitCommandWithIO(stdin io.Reader, stdout, stderr io.Writer, args ...string) error {
-	repopath := repo.Path
-	if repopath == ".git" {
-		// seeduvax> trangely the git command sometimes fail for very unknown
-		// reason wihtout this replacement.
-		// observed with rev-list command when git-bug is called from git
-		// hook script, even the same command with same args runs perfectly
-		// when called directly from the same hook script.
-		repopath = ""
-	}
-	// fmt.Printf("[%s] Running git %s\n", repopath, strings.Join(args, " "))
+	// make sure that the working directory for the command
+	// always exist, in particular when running "git init".
+	path := strings.TrimSuffix(repo.path, ".git")
+
+	// fmt.Printf("[%s] Running git %s\n", path, strings.Join(args, " "))
 
 	cmd := exec.Command("git", args...)
-	cmd.Dir = repopath
+	cmd.Dir = path
 	cmd.Stdin = stdin
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
@@ -93,8 +83,11 @@ func (repo *GitRepo) runGitCommand(args ...string) (string, error) {
 
 // NewGitRepo determines if the given working directory is inside of a git repository,
 // and returns the corresponding GitRepo instance if it is.
-func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
-	repo := &GitRepo{Path: path}
+func NewGitRepo(path string, clockLoaders []ClockLoader) (*GitRepo, error) {
+	repo := &GitRepo{
+		path:   path,
+		clocks: make(map[string]lamport.Clock),
+	}
 
 	// Check the repo and retrieve the root path
 	stdout, err := repo.runGitCommand("rev-parse", "--git-dir")
@@ -107,28 +100,22 @@ func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
 	}
 
 	// Fix the path to be sure we are at the root
-	repo.Path = stdout
-
-	err = repo.LoadClocks()
-
-	if err != nil {
-		// No clock yet, trying to initialize them
-		err = repo.createClocks()
-		if err != nil {
-			return nil, err
+	repo.path = stdout
+
+	for _, loader := range clockLoaders {
+		allExist := true
+		for _, name := range loader.Clocks {
+			if _, err := repo.getClock(name); err != nil {
+				allExist = false
+			}
 		}
 
-		err = witnesser(repo)
-		if err != nil {
-			return nil, err
+		if !allExist {
+			err = loader.Witnesser(repo)
+			if err != nil {
+				return nil, err
+			}
 		}
-
-		err = repo.WriteClocks()
-		if err != nil {
-			return nil, err
-		}
-
-		return repo, nil
 	}
 
 	return repo, nil
@@ -136,13 +123,12 @@ func NewGitRepo(path string, witnesser Witnesser) (*GitRepo, error) {
 
 // InitGitRepo create a new empty git repo at the given path
 func InitGitRepo(path string) (*GitRepo, error) {
-	repo := &GitRepo{Path: path + "/.git"}
-	err := repo.createClocks()
-	if err != nil {
-		return nil, err
+	repo := &GitRepo{
+		path:   path + "/.git",
+		clocks: make(map[string]lamport.Clock),
 	}
 
-	_, err = repo.runGitCommand("init", path)
+	_, err := repo.runGitCommand("init", path)
 	if err != nil {
 		return nil, err
 	}
@@ -152,13 +138,12 @@ func InitGitRepo(path string) (*GitRepo, error) {
 
 // InitBareGitRepo create a new --bare empty git repo at the given path
 func InitBareGitRepo(path string) (*GitRepo, error) {
-	repo := &GitRepo{Path: path}
-	err := repo.createClocks()
-	if err != nil {
-		return nil, err
+	repo := &GitRepo{
+		path:   path,
+		clocks: make(map[string]lamport.Clock),
 	}
 
-	_, err = repo.runGitCommand("init", "--bare", path)
+	_, err := repo.runGitCommand("init", "--bare", path)
 	if err != nil {
 		return nil, err
 	}
@@ -168,7 +153,7 @@ func InitBareGitRepo(path string) (*GitRepo, error) {
 
 // GetPath returns the path to the repo.
 func (repo *GitRepo) GetPath() string {
-	return repo.Path
+	return repo.path
 }
 
 // GetUserName returns the name the the user has used to configure git
@@ -385,93 +370,56 @@ func (repo *GitRepo) GetTreeHash(commit git.Hash) (git.Hash, error) {
 	return git.Hash(stdout), nil
 }
 
-// AddRemote add a new remote to the repository
-// Not in the interface because it's only used for testing
-func (repo *GitRepo) AddRemote(name string, url string) error {
-	_, err := repo.runGitCommand("remote", "add", name, url)
-
-	return err
-}
-
-func (repo *GitRepo) createClocks() error {
-	createPath := path.Join(repo.Path, createClockFile)
-	createClock, err := lamport.NewPersisted(createPath)
-	if err != nil {
-		return err
+// GetOrCreateClock return a Lamport clock stored in the Repo.
+// If the clock doesn't exist, it's created.
+func (repo *GitRepo) GetOrCreateClock(name string) (lamport.Clock, error) {
+	c, err := repo.getClock(name)
+	if err == nil {
+		return c, nil
 	}
-
-	editPath := path.Join(repo.Path, editClockFile)
-	editClock, err := lamport.NewPersisted(editPath)
-	if err != nil {
-		return err
+	if err != ErrClockNotExist {
+		return nil, err
 	}
 
-	repo.createClock = createClock
-	repo.editClock = editClock
+	repo.clocksMutex.Lock()
+	defer repo.clocksMutex.Unlock()
 
-	return nil
-}
-
-// LoadClocks read the clocks values from the on-disk repo
-func (repo *GitRepo) LoadClocks() error {
-	createClock, err := lamport.LoadPersisted(repo.GetPath() + createClockFile)
-	if err != nil {
-		return err
-	}
+	p := path.Join(repo.path, clockPath, name+"-clock")
 
-	editClock, err := lamport.LoadPersisted(repo.GetPath() + editClockFile)
+	c, err = lamport.NewPersistedClock(p)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	repo.createClock = createClock
-	repo.editClock = editClock
-	return nil
+	repo.clocks[name] = c
+	return c, nil
 }
 
-// WriteClocks write the clocks values into the repo
-func (repo *GitRepo) WriteClocks() error {
-	err := repo.createClock.Write()
-	if err != nil {
-		return err
-	}
+func (repo *GitRepo) getClock(name string) (lamport.Clock, error) {
+	repo.clocksMutex.Lock()
+	defer repo.clocksMutex.Unlock()
 
-	err = repo.editClock.Write()
-	if err != nil {
-		return err
+	if c, ok := repo.clocks[name]; ok {
+		return c, nil
 	}
 
-	return nil
-}
-
-// CreateTime return the current value of the creation clock
-func (repo *GitRepo) CreateTime() lamport.Time {
-	return repo.createClock.Time()
-}
-
-// CreateTimeIncrement increment the creation clock and return the new value.
-func (repo *GitRepo) CreateTimeIncrement() (lamport.Time, error) {
-	return repo.createClock.Increment()
-}
+	p := path.Join(repo.path, clockPath, name+"-clock")
 
-// EditTime return the current value of the edit clock
-func (repo *GitRepo) EditTime() lamport.Time {
-	return repo.editClock.Time()
-}
-
-// EditTimeIncrement increment the edit clock and return the new value.
-func (repo *GitRepo) EditTimeIncrement() (lamport.Time, error) {
-	return repo.editClock.Increment()
+	c, err := lamport.LoadPersistedClock(p)
+	if err == nil {
+		repo.clocks[name] = c
+		return c, nil
+	}
+	if err == lamport.ErrClockNotExist {
+		return nil, ErrClockNotExist
+	}
+	return nil, err
 }
 
-// WitnessCreate witness another create time and increment the corresponding clock
-// if needed.
-func (repo *GitRepo) WitnessCreate(time lamport.Time) error {
-	return repo.createClock.Witness(time)
-}
+// AddRemote add a new remote to the repository
+// Not in the interface because it's only used for testing
+func (repo *GitRepo) AddRemote(name string, url string) error {
+	_, err := repo.runGitCommand("remote", "add", name, url)
 
-// WitnessEdit witness another edition time and increment the corresponding clock
-// if needed.
-func (repo *GitRepo) WitnessEdit(time lamport.Time) error {
-	return repo.editClock.Witness(time)
+	return err
 }