diff --git a/bug/bug.go b/bug/bug.go index f84753fa..f1bd1114 100644 --- a/bug/bug.go +++ b/bug/bug.go @@ -321,6 +321,11 @@ func (bug *Bug) Validate() error { return fmt.Errorf("first operation should be a Create op") } + // The bug ID should be the hash of the first commit + if len(bug.packs) > 0 && string(bug.packs[0].commitHash) != bug.id { + return fmt.Errorf("bug id should be the first commit hash") + } + // Check that there is no more CreateOp op it := NewOperationIterator(bug) createCount := 0 @@ -349,7 +354,8 @@ func (bug *Bug) HasPendingOp() bool { // Commit write the staging area in Git and move the operations to the packs func (bug *Bug) Commit(repo repository.ClockedRepo) error { - if bug.staging.IsEmpty() { + + if !bug.NeedCommit() { return fmt.Errorf("can't commit a bug with no pending operation") } @@ -466,6 +472,17 @@ func (bug *Bug) Commit(repo repository.ClockedRepo) error { return nil } +func (bug *Bug) CommitAsNeeded(repo repository.ClockedRepo) error { + if !bug.NeedCommit() { + return nil + } + return bug.Commit(repo) +} + +func (bug *Bug) NeedCommit() bool { + return !bug.staging.IsEmpty() +} + func makeMediaTree(pack OperationPack) []repository.TreeEntry { var tree []repository.TreeEntry counter := 0 diff --git a/bug/bug_actions.go b/bug/bug_actions.go index f214716d..b906b938 100644 --- a/bug/bug_actions.go +++ b/bug/bug_actions.go @@ -4,28 +4,68 @@ import ( "fmt" "strings" + "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/repository" "github.com/pkg/errors" ) +// Note: +// +// For the actions (fetch/push/pull/merge), this package act as a master for +// the identity package and will also drive the needed identity actions. That is, +// if bug.Push() is called, identity.Push will also be called to make sure that +// the dependant identities are also present and up to date on the remote. +// +// I'm not entirely sure this is the correct way to do it, as it might introduce +// too much complexity and hard coupling, but it does make this package easier +// to use. + // Fetch retrieve updates from a remote // This does not change the local bugs state func Fetch(repo repository.Repo, remote string) (string, error) { + stdout, err := identity.Fetch(repo, remote) + if err != nil { + return stdout, err + } + remoteRefSpec := fmt.Sprintf(bugsRemoteRefPattern, remote) fetchRefSpec := fmt.Sprintf("%s*:%s*", bugsRefPattern, remoteRefSpec) - return repo.FetchRefs(remote, fetchRefSpec) + stdout2, err := repo.FetchRefs(remote, fetchRefSpec) + + return stdout + "\n" + stdout2, err } // Push update a remote with the local changes func Push(repo repository.Repo, remote string) (string, error) { - return repo.PushRefs(remote, bugsRefPattern+"*") + stdout, err := identity.Push(repo, remote) + if err != nil { + return stdout, err + } + + stdout2, err := repo.PushRefs(remote, bugsRefPattern+"*") + + return stdout + "\n" + stdout2, err } // Pull will do a Fetch + MergeAll // This function will return an error if a merge fail func Pull(repo repository.ClockedRepo, remote string) error { - _, err := Fetch(repo, remote) + _, err := identity.Fetch(repo, remote) + if err != nil { + return err + } + + for merge := range identity.MergeAll(repo, remote) { + if merge.Err != nil { + return merge.Err + } + if merge.Status == identity.MergeStatusInvalid { + return errors.Errorf("merge failure: %s", merge.Reason) + } + } + + _, err = Fetch(repo, remote) if err != nil { return err } diff --git a/bug/bug_actions_test.go b/bug/bug_actions_test.go index 39438ec7..4d42fb1d 100644 --- a/bug/bug_actions_test.go +++ b/bug/bug_actions_test.go @@ -1,8 +1,11 @@ package bug import ( + "time" + + "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/test" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "testing" ) @@ -11,20 +14,19 @@ func TestPushPull(t *testing.T) { repoA, repoB, remote := test.SetupReposAndRemote(t) defer test.CleanupRepos(repoA, repoB, remote) - err := rene.Commit(repoA) - assert.NoError(t, err) + reneA := identity.NewIdentity("René Descartes", "rene@descartes.fr") - bug1, _, err := Create(rene, unix, "bug1", "message") - assert.NoError(t, err) + bug1, _, err := Create(reneA, time.Now().Unix(), "bug1", "message") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) // A --> remote --> B _, err = Push(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) err = Pull(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) bugs := allBugs(t, ReadAllLocalBugs(repoB)) @@ -33,16 +35,19 @@ func TestPushPull(t *testing.T) { } // B --> remote --> A - bug2, _, err := Create(rene, unix, "bug2", "message") - assert.NoError(t, err) + reneB, err := identity.ReadLocal(repoA, reneA.Id()) + require.NoError(t, err) + + bug2, _, err := Create(reneB, time.Now().Unix(), "bug2", "message") + require.NoError(t, err) err = bug2.Commit(repoB) - assert.NoError(t, err) + require.NoError(t, err) _, err = Push(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) err = Pull(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) bugs = allBugs(t, ReadAllLocalBugs(repoA)) @@ -76,38 +81,43 @@ func _RebaseTheirs(t testing.TB) { repoA, repoB, remote := test.SetupReposAndRemote(t) defer test.CleanupRepos(repoA, repoB, remote) - bug1, _, err := Create(rene, unix, "bug1", "message") - assert.NoError(t, err) + reneA := identity.NewIdentity("René Descartes", "rene@descartes.fr") + + bug1, _, err := Create(reneA, time.Now().Unix(), "bug1", "message") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) // A --> remote _, err = Push(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) // remote --> B err = Pull(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) bug2, err := ReadLocalBug(repoB, bug1.Id()) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message2") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message3") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message4") - assert.NoError(t, err) + reneB, err := identity.ReadLocal(repoA, reneA.Id()) + require.NoError(t, err) + + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message2") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message3") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message4") + require.NoError(t, err) err = bug2.Commit(repoB) - assert.NoError(t, err) + require.NoError(t, err) // B --> remote _, err = Push(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) // remote --> A err = Pull(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) bugs := allBugs(t, ReadAllLocalBugs(repoB)) @@ -116,7 +126,7 @@ func _RebaseTheirs(t testing.TB) { } bug3, err := ReadLocalBug(repoA, bug1.Id()) - assert.NoError(t, err) + require.NoError(t, err) if nbOps(bug3) != 4 { t.Fatal("Unexpected number of operations") @@ -137,49 +147,51 @@ func _RebaseOurs(t testing.TB) { repoA, repoB, remote := test.SetupReposAndRemote(t) defer test.CleanupRepos(repoA, repoB, remote) - bug1, _, err := Create(rene, unix, "bug1", "message") - assert.NoError(t, err) + reneA := identity.NewIdentity("René Descartes", "rene@descartes.fr") + + bug1, _, err := Create(reneA, time.Now().Unix(), "bug1", "message") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) // A --> remote _, err = Push(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) // remote --> B err = Pull(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message2") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message3") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message4") - assert.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message2") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message3") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message4") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message5") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message6") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message7") - assert.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message5") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message6") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message7") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message8") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message9") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message10") - assert.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message8") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message9") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message10") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) // remote --> A err = Pull(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) bugs := allBugs(t, ReadAllLocalBugs(repoA)) @@ -188,7 +200,7 @@ func _RebaseOurs(t testing.TB) { } bug2, err := ReadLocalBug(repoA, bug1.Id()) - assert.NoError(t, err) + require.NoError(t, err) if nbOps(bug2) != 10 { t.Fatal("Unexpected number of operations") @@ -218,83 +230,88 @@ func _RebaseConflict(t testing.TB) { repoA, repoB, remote := test.SetupReposAndRemote(t) defer test.CleanupRepos(repoA, repoB, remote) - bug1, _, err := Create(rene, unix, "bug1", "message") - assert.NoError(t, err) + reneA := identity.NewIdentity("René Descartes", "rene@descartes.fr") + + bug1, _, err := Create(reneA, time.Now().Unix(), "bug1", "message") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) // A --> remote _, err = Push(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) // remote --> B err = Pull(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message2") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message3") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message4") - assert.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message2") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message3") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message4") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message5") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message6") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message7") - assert.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message5") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message6") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message7") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message8") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message9") - assert.NoError(t, err) - _, err = AddComment(bug1, rene, unix, "message10") - assert.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message8") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message9") + require.NoError(t, err) + _, err = AddComment(bug1, reneA, time.Now().Unix(), "message10") + require.NoError(t, err) err = bug1.Commit(repoA) - assert.NoError(t, err) + require.NoError(t, err) bug2, err := ReadLocalBug(repoB, bug1.Id()) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message11") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message12") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message13") - assert.NoError(t, err) - err = bug2.Commit(repoB) - assert.NoError(t, err) + reneB, err := identity.ReadLocal(repoA, reneA.Id()) + require.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message14") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message15") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message16") - assert.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message11") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message12") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message13") + require.NoError(t, err) err = bug2.Commit(repoB) - assert.NoError(t, err) + require.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message17") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message18") - assert.NoError(t, err) - _, err = AddComment(bug2, rene, unix, "message19") - assert.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message14") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message15") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message16") + require.NoError(t, err) err = bug2.Commit(repoB) - assert.NoError(t, err) + require.NoError(t, err) + + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message17") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message18") + require.NoError(t, err) + _, err = AddComment(bug2, reneB, time.Now().Unix(), "message19") + require.NoError(t, err) + err = bug2.Commit(repoB) + require.NoError(t, err) // A --> remote _, err = Push(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) // remote --> B err = Pull(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) bugs := allBugs(t, ReadAllLocalBugs(repoB)) @@ -303,7 +320,7 @@ func _RebaseConflict(t testing.TB) { } bug3, err := ReadLocalBug(repoB, bug1.Id()) - assert.NoError(t, err) + require.NoError(t, err) if nbOps(bug3) != 19 { t.Fatal("Unexpected number of operations") @@ -311,11 +328,11 @@ func _RebaseConflict(t testing.TB) { // B --> remote _, err = Push(repoB, "origin") - assert.NoError(t, err) + require.NoError(t, err) // remote --> A err = Pull(repoA, "origin") - assert.NoError(t, err) + require.NoError(t, err) bugs = allBugs(t, ReadAllLocalBugs(repoA)) @@ -324,7 +341,7 @@ func _RebaseConflict(t testing.TB) { } bug4, err := ReadLocalBug(repoA, bug1.Id()) - assert.NoError(t, err) + require.NoError(t, err) if nbOps(bug4) != 19 { t.Fatal("Unexpected number of operations") diff --git a/bug/bug_test.go b/bug/bug_test.go index 001bfc56..e104f921 100644 --- a/bug/bug_test.go +++ b/bug/bug_test.go @@ -1,6 +1,9 @@ package bug import ( + "time" + + "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/repository" "github.com/stretchr/testify/assert" @@ -12,6 +15,9 @@ func TestBugId(t *testing.T) { bug1 := NewBug() + rene := identity.NewIdentity("René Descartes", "rene@descartes.fr") + createOp := NewCreateOp(rene, time.Now().Unix(), "title", "message", nil) + bug1.Append(createOp) err := bug1.Commit(mockRepo) @@ -28,6 +34,9 @@ func TestBugValidity(t *testing.T) { bug1 := NewBug() + rene := identity.NewIdentity("René Descartes", "rene@descartes.fr") + createOp := NewCreateOp(rene, time.Now().Unix(), "title", "message", nil) + if bug1.Validate() == nil { t.Fatal("Empty bug should be invalid") } @@ -58,6 +67,11 @@ func TestBugValidity(t *testing.T) { func TestBugCommitLoad(t *testing.T) { bug1 := NewBug() + rene := identity.NewIdentity("René Descartes", "rene@descartes.fr") + createOp := NewCreateOp(rene, time.Now().Unix(), "title", "message", nil) + setTitleOp := NewSetTitleOp(rene, time.Now().Unix(), "title2", "title1") + addCommentOp := NewAddCommentOp(rene, time.Now().Unix(), "message2", nil) + bug1.Append(createOp) bug1.Append(setTitleOp) bug1.Append(setTitleOp) diff --git a/bug/op_create_test.go b/bug/op_create_test.go index 065b81c5..c41c5687 100644 --- a/bug/op_create_test.go +++ b/bug/op_create_test.go @@ -12,8 +12,7 @@ import ( func TestCreate(t *testing.T) { snapshot := Snapshot{} - var rene = identity.NewIdentity("René Descartes", "rene@descartes.fr") - + rene := identity.NewBare("René Descartes", "rene@descartes.fr") unix := time.Now().Unix() create := NewCreateOp(rene, unix, "title", "message", nil) diff --git a/bug/op_edit_comment_test.go b/bug/op_edit_comment_test.go index dbdf341d..72f8a168 100644 --- a/bug/op_edit_comment_test.go +++ b/bug/op_edit_comment_test.go @@ -7,30 +7,26 @@ import ( "github.com/MichaelMure/git-bug/identity" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestEdit(t *testing.T) { snapshot := Snapshot{} - var rene = identity.NewIdentity("René Descartes", "rene@descartes.fr") - + rene := identity.NewBare("René Descartes", "rene@descartes.fr") unix := time.Now().Unix() create := NewCreateOp(rene, unix, "title", "create", nil) create.Apply(&snapshot) hash1, err := create.Hash() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) comment := NewAddCommentOp(rene, unix, "comment", nil) comment.Apply(&snapshot) hash2, err := comment.Hash() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) edit := NewEditCommentOp(rene, unix, hash1, "create edited", nil) edit.Apply(&snapshot) diff --git a/bug/op_set_metadata_test.go b/bug/op_set_metadata_test.go index 847164f3..a7f9f313 100644 --- a/bug/op_set_metadata_test.go +++ b/bug/op_set_metadata_test.go @@ -7,13 +7,13 @@ import ( "github.com/MichaelMure/git-bug/identity" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSetMetadata(t *testing.T) { snapshot := Snapshot{} - var rene = identity.NewIdentity("René Descartes", "rene@descartes.fr") - + rene := identity.NewBare("René Descartes", "rene@descartes.fr") unix := time.Now().Unix() create := NewCreateOp(rene, unix, "title", "create", nil) @@ -22,9 +22,7 @@ func TestSetMetadata(t *testing.T) { snapshot.Operations = append(snapshot.Operations, create) hash1, err := create.Hash() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) comment := NewAddCommentOp(rene, unix, "comment", nil) comment.SetMetadata("key2", "value2") @@ -32,9 +30,7 @@ func TestSetMetadata(t *testing.T) { snapshot.Operations = append(snapshot.Operations, comment) hash2, err := comment.Hash() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) op1 := NewSetMetadataOp(rene, unix, hash1, map[string]string{ "key": "override", diff --git a/bug/operation_iterator_test.go b/bug/operation_iterator_test.go index a41120e2..2865d25d 100644 --- a/bug/operation_iterator_test.go +++ b/bug/operation_iterator_test.go @@ -10,14 +10,17 @@ import ( ) var ( - rene = identity.NewIdentity("René Descartes", "rene@descartes.fr") - unix = time.Now().Unix() +// Beware, don't those test data in multi-repo situation ! +// As an example, the Identity would be considered commited after a commit +// in one repo, +// rene = identity.NewIdentity("René Descartes", "rene@descartes.fr") +// unix = time.Now().Unix() - createOp = NewCreateOp(rene, unix, "title", "message", nil) - setTitleOp = NewSetTitleOp(rene, unix, "title2", "title1") - addCommentOp = NewAddCommentOp(rene, unix, "message2", nil) - setStatusOp = NewSetStatusOp(rene, unix, ClosedStatus) - labelChangeOp = NewLabelChangeOperation(rene, unix, []Label{"added"}, []Label{"removed"}) +// createOp = NewCreateOp(rene, unix, "title", "message", nil) +// setTitleOp = NewSetTitleOp(rene, unix, "title2", "title1") +// addCommentOp = NewAddCommentOp(rene, unix, "message2", nil) +// setStatusOp = NewSetStatusOp(rene, unix, ClosedStatus) +// labelChangeOp = NewLabelChangeOperation(rene, unix, []Label{"added"}, []Label{"removed"}) ) func ExampleOperationIterator() { @@ -36,6 +39,15 @@ func ExampleOperationIterator() { func TestOpIterator(t *testing.T) { mockRepo := repository.NewMockRepoForTest() + rene := identity.NewIdentity("René Descartes", "rene@descartes.fr") + unix := time.Now().Unix() + + createOp := NewCreateOp(rene, unix, "title", "message", nil) + setTitleOp := NewSetTitleOp(rene, unix, "title2", "title1") + addCommentOp := NewAddCommentOp(rene, unix, "message2", nil) + setStatusOp := NewSetStatusOp(rene, unix, ClosedStatus) + labelChangeOp := NewLabelChangeOperation(rene, unix, []Label{"added"}, []Label{"removed"}) + bug1 := NewBug() // first pack diff --git a/bug/operation_pack.go b/bug/operation_pack.go index 1ffc1d1a..55fc018e 100644 --- a/bug/operation_pack.go +++ b/bug/operation_pack.go @@ -147,7 +147,7 @@ func (opp *OperationPack) Write(repo repository.Repo) (git.Hash, error) { // First, make sure that all the identities are properly Commit as well for _, op := range opp.Operations { - err := op.base().Author.Commit(repo) + err := op.base().Author.CommitAsNeeded(repo) if err != nil { return "", err } diff --git a/bug/operation_pack_test.go b/bug/operation_pack_test.go index 8a8c7e62..09d159af 100644 --- a/bug/operation_pack_test.go +++ b/bug/operation_pack_test.go @@ -3,27 +3,37 @@ package bug import ( "encoding/json" "testing" + "time" + "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestOperationPackSerialize(t *testing.T) { opp := &OperationPack{} + rene := identity.NewBare("René Descartes", "rene@descartes.fr") + createOp := NewCreateOp(rene, time.Now().Unix(), "title", "message", nil) + setTitleOp := NewSetTitleOp(rene, time.Now().Unix(), "title2", "title1") + addCommentOp := NewAddCommentOp(rene, time.Now().Unix(), "message2", nil) + setStatusOp := NewSetStatusOp(rene, time.Now().Unix(), ClosedStatus) + labelChangeOp := NewLabelChangeOperation(rene, time.Now().Unix(), []Label{"added"}, []Label{"removed"}) + opp.Append(createOp) opp.Append(setTitleOp) opp.Append(addCommentOp) opp.Append(setStatusOp) opp.Append(labelChangeOp) - opMeta := NewCreateOp(rene, unix, "title", "message", nil) + opMeta := NewSetTitleOp(rene, time.Now().Unix(), "title3", "title2") opMeta.SetMetadata("key", "value") opp.Append(opMeta) assert.Equal(t, 1, len(opMeta.Metadata)) - opFile := NewCreateOp(rene, unix, "title", "message", []git.Hash{ + opFile := NewAddCommentOp(rene, time.Now().Unix(), "message", []git.Hash{ "abcdef", "ghijkl", }) @@ -36,7 +46,16 @@ func TestOperationPackSerialize(t *testing.T) { var opp2 *OperationPack err = json.Unmarshal(data, &opp2) - assert.NoError(t, err) + + ensureHash(t, opp) + assert.Equal(t, opp, opp2) } + +func ensureHash(t *testing.T, opp *OperationPack) { + for _, op := range opp.Operations { + _, err := op.Hash() + require.NoError(t, err) + } +} diff --git a/bug/operation_test.go b/bug/operation_test.go index d5cb5090..f9a7d191 100644 --- a/bug/operation_test.go +++ b/bug/operation_test.go @@ -2,6 +2,7 @@ package bug import ( "testing" + "time" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/repository" @@ -11,6 +12,9 @@ import ( ) func TestValidate(t *testing.T) { + rene := identity.NewIdentity("René Descartes", "rene@descartes.fr") + unix := time.Now().Unix() + good := []Operation{ NewCreateOp(rene, unix, "title", "message", nil), NewSetTitleOp(rene, unix, "title2", "title1"), @@ -65,7 +69,8 @@ func TestValidate(t *testing.T) { } func TestMetadata(t *testing.T) { - op := NewCreateOp(rene, unix, "title", "message", nil) + rene := identity.NewIdentity("René Descartes", "rene@descartes.fr") + op := NewCreateOp(rene, time.Now().Unix(), "title", "message", nil) op.SetMetadata("key", "value") @@ -81,7 +86,9 @@ func TestHash(t *testing.T) { } for _, repo := range repos { - b, op, err := Create(rene, unix, "title", "message") + rene := identity.NewBare("René Descartes", "rene@descartes.fr") + + b, op, err := Create(rene, time.Now().Unix(), "title", "message") require.Nil(t, err) h1, err := op.Hash() diff --git a/cache/bug_cache.go b/cache/bug_cache.go index 53c1db64..1d161c76 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -234,8 +234,5 @@ func (c *BugCache) Commit() error { } func (c *BugCache) CommitAsNeeded() error { - if c.bug.HasPendingOp() { - return c.bug.Commit(c.repoCache.repo) - } - return nil + return c.bug.CommitAsNeeded(c.repoCache.repo) } diff --git a/commands/select/select_test.go b/commands/select/select_test.go index fe501d76..003dfc81 100644 --- a/commands/select/select_test.go +++ b/commands/select/select_test.go @@ -1,113 +1,74 @@ package _select import ( - "io/ioutil" - "log" "testing" "github.com/MichaelMure/git-bug/cache" - "github.com/MichaelMure/git-bug/repository" + "github.com/MichaelMure/git-bug/util/test" + "github.com/stretchr/testify/require" ) func TestSelect(t *testing.T) { - repo, err := cache.NewRepoCache(createRepo()) - checkErr(t, err) + repo := test.CreateRepo(false) + defer test.CleanupRepo(repo) - _, _, err = ResolveBug(repo, []string{}) - if err != ErrNoValidId { - t.Fatal("expected no valid id error, got", err) - } + repoCache, err := cache.NewRepoCache(repo) + require.NoError(t, err) - err = Select(repo, "invalid") - checkErr(t, err) + _, _, err = ResolveBug(repoCache, []string{}) + require.Equal(t, ErrNoValidId, err) - _, _, err = ResolveBug(repo, []string{}) - if err == nil { - t.Fatal("expected invalid bug error") - } + err = Select(repoCache, "invalid") + require.NoError(t, err) + + // Resolve without a pattern should fail when no bug is selected + _, _, err = ResolveBug(repoCache, []string{}) + require.Error(t, err) // generate a bunch of bugs for i := 0; i < 10; i++ { - _, err := repo.NewBug("title", "message") - checkErr(t, err) + _, err := repoCache.NewBug("title", "message") + require.NoError(t, err) } - // two more for testing - b1, err := repo.NewBug("title", "message") - checkErr(t, err) - b2, err := repo.NewBug("title", "message") - checkErr(t, err) + // and two more for testing + b1, err := repoCache.NewBug("title", "message") + require.NoError(t, err) + b2, err := repoCache.NewBug("title", "message") + require.NoError(t, err) - err = Select(repo, b1.Id()) - checkErr(t, err) + err = Select(repoCache, b1.Id()) + require.NoError(t, err) // normal select without args - b3, _, err := ResolveBug(repo, []string{}) - checkErr(t, err) - if b3.Id() != b1.Id() { - t.Fatal("incorrect bug returned") - } + b3, _, err := ResolveBug(repoCache, []string{}) + require.NoError(t, err) + require.Equal(t, b1.Id(), b3.Id()) // override selection with same id - b4, _, err := ResolveBug(repo, []string{b1.Id()}) - checkErr(t, err) - if b4.Id() != b1.Id() { - t.Fatal("incorrect bug returned") - } + b4, _, err := ResolveBug(repoCache, []string{b1.Id()}) + require.NoError(t, err) + require.Equal(t, b1.Id(), b4.Id()) // override selection with a prefix - b5, _, err := ResolveBug(repo, []string{b1.HumanId()}) - checkErr(t, err) - if b5.Id() != b1.Id() { - t.Fatal("incorrect bug returned") - } + b5, _, err := ResolveBug(repoCache, []string{b1.HumanId()}) + require.NoError(t, err) + require.Equal(t, b1.Id(), b5.Id()) // args that shouldn't override - b6, _, err := ResolveBug(repo, []string{"arg"}) - checkErr(t, err) - if b6.Id() != b1.Id() { - t.Fatal("incorrect bug returned") - } + b6, _, err := ResolveBug(repoCache, []string{"arg"}) + require.NoError(t, err) + require.Equal(t, b1.Id(), b6.Id()) // override with a different id - b7, _, err := ResolveBug(repo, []string{b2.Id()}) - checkErr(t, err) - if b7.Id() != b2.Id() { - t.Fatal("incorrect bug returned") - } + b7, _, err := ResolveBug(repoCache, []string{b2.Id()}) + require.NoError(t, err) + require.Equal(t, b2.Id(), b7.Id()) - err = Clear(repo) - checkErr(t, err) + err = Clear(repoCache) + require.NoError(t, err) - _, _, err = ResolveBug(repo, []string{}) - if err == nil { - t.Fatal("expected invalid bug error") - } -} - -func createRepo() *repository.GitRepo { - dir, err := ioutil.TempDir("", "") - if err != nil { - log.Fatal(err) - } - - repo, err := repository.InitGitRepo(dir) - if err != nil { - log.Fatal(err) - } - - if err := repo.StoreConfig("user.name", "testuser"); err != nil { - log.Fatal("failed to set user.name for test repository: ", err) - } - if err := repo.StoreConfig("user.email", "testuser@example.com"); err != nil { - log.Fatal("failed to set user.email for test repository: ", err) - } - - return repo -} - -func checkErr(t testing.TB, err error) { - if err != nil { - t.Fatal(err) - } + // Resolve without a pattern should error again after clearing the selected bug + _, _, err = ResolveBug(repoCache, []string{}) + require.Error(t, err) } diff --git a/doc/man/git-bug.1 b/doc/man/git-bug.1 index 123f9a33..f3ad4729 100644 --- a/doc/man/git-bug.1 +++ b/doc/man/git-bug.1 @@ -31,4 +31,4 @@ the same git remote your are already using to collaborate with other peoples. .SH SEE ALSO .PP -\fBgit\-bug\-add(1)\fP, \fBgit\-bug\-bridge(1)\fP, \fBgit\-bug\-commands(1)\fP, \fBgit\-bug\-comment(1)\fP, \fBgit\-bug\-deselect(1)\fP, \fBgit\-bug\-label(1)\fP, \fBgit\-bug\-ls(1)\fP, \fBgit\-bug\-ls\-label(1)\fP, \fBgit\-bug\-pull(1)\fP, \fBgit\-bug\-push(1)\fP, \fBgit\-bug\-select(1)\fP, \fBgit\-bug\-show(1)\fP, \fBgit\-bug\-status(1)\fP, \fBgit\-bug\-termui(1)\fP, \fBgit\-bug\-title(1)\fP, \fBgit\-bug\-version(1)\fP, \fBgit\-bug\-webui(1)\fP +\fBgit\-bug\-add(1)\fP, \fBgit\-bug\-bridge(1)\fP, \fBgit\-bug\-commands(1)\fP, \fBgit\-bug\-comment(1)\fP, \fBgit\-bug\-deselect(1)\fP, \fBgit\-bug\-id(1)\fP, \fBgit\-bug\-label(1)\fP, \fBgit\-bug\-ls(1)\fP, \fBgit\-bug\-ls\-label(1)\fP, \fBgit\-bug\-pull(1)\fP, \fBgit\-bug\-push(1)\fP, \fBgit\-bug\-select(1)\fP, \fBgit\-bug\-show(1)\fP, \fBgit\-bug\-status(1)\fP, \fBgit\-bug\-termui(1)\fP, \fBgit\-bug\-title(1)\fP, \fBgit\-bug\-version(1)\fP, \fBgit\-bug\-webui(1)\fP diff --git a/doc/md/git-bug.md b/doc/md/git-bug.md index a1c1c885..7183dd76 100644 --- a/doc/md/git-bug.md +++ b/doc/md/git-bug.md @@ -29,6 +29,7 @@ git-bug [flags] * [git-bug commands](git-bug_commands.md) - Display available commands * [git-bug comment](git-bug_comment.md) - Display or add comments * [git-bug deselect](git-bug_deselect.md) - Clear the implicitly selected bug +* [git-bug id](git-bug_id.md) - Display or change the user identity * [git-bug label](git-bug_label.md) - Display, add or remove labels * [git-bug ls](git-bug_ls.md) - List bugs * [git-bug ls-id](git-bug_ls-id.md) - List Bug Id diff --git a/identity/bare.go b/identity/bare.go index 5aaa0166..c54277a0 100644 --- a/identity/bare.go +++ b/identity/bare.go @@ -169,6 +169,11 @@ func (i *Bare) Commit(repo repository.Repo) error { return nil } +func (i *Bare) CommitAsNeeded(repo repository.Repo) error { + // Nothing to do, everything is directly embedded + return nil +} + // IsProtected return true if the chain of git commits started to be signed. // If that's the case, only signed commit with a valid key for this identity can be added. func (i *Bare) IsProtected() bool { diff --git a/identity/common.go b/identity/common.go index 00feaa2d..2f2b9042 100644 --- a/identity/common.go +++ b/identity/common.go @@ -29,7 +29,6 @@ func UnmarshalJSON(raw json.RawMessage) (Interface, error) { err := json.Unmarshal(raw, &aux) if err == nil && aux.Id() != "" { return aux, nil - // return identityResolver.ResolveIdentity(aux.Id) } // abort if we have an error other than the wrong type diff --git a/identity/identity.go b/identity/identity.go index 725362f9..a0800bcd 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -241,15 +241,7 @@ func (i *Identity) AddVersion(version *Version) { func (i *Identity) Commit(repo repository.Repo) error { // Todo: check for mismatch between memory and commited data - needCommit := false - for _, v := range i.versions { - if v.commitHash == "" { - needCommit = true - break - } - } - - if !needCommit { + if !i.NeedCommit() { return fmt.Errorf("can't commit an identity with no pending version") } @@ -313,6 +305,23 @@ func (i *Identity) Commit(repo repository.Repo) error { return nil } +func (i *Identity) CommitAsNeeded(repo repository.Repo) error { + if !i.NeedCommit() { + return nil + } + return i.Commit(repo) +} + +func (i *Identity) NeedCommit() bool { + for _, v := range i.versions { + if v.commitHash == "" { + return true + } + } + + return false +} + // Merge will merge a different version of the same Identity // // To make sure that an Identity history can't be altered, a strict fast-forward @@ -379,6 +388,10 @@ func (i *Identity) Merge(repo repository.Repo, other *Identity) (bool, error) { func (i *Identity) Validate() error { lastTime := lamport.Time(0) + if len(i.versions) == 0 { + return fmt.Errorf("no version") + } + for _, v := range i.versions { if err := v.Validate(); err != nil { return err @@ -391,6 +404,11 @@ func (i *Identity) Validate() error { lastTime = v.time } + // The identity ID should be the hash of the first commit + if i.versions[0].commitHash != "" && string(i.versions[0].commitHash) != i.id { + return fmt.Errorf("identity id should be the first commit hash") + } + return nil } diff --git a/identity/identity_stub.go b/identity/identity_stub.go index 6788ce33..e91600b0 100644 --- a/identity/identity_stub.go +++ b/identity/identity_stub.go @@ -80,6 +80,10 @@ func (IdentityStub) Commit(repo repository.Repo) error { panic("identities needs to be properly loaded with identity.ReadLocal()") } +func (i *IdentityStub) CommitAsNeeded(repo repository.Repo) error { + panic("identities needs to be properly loaded with identity.ReadLocal()") +} + func (IdentityStub) IsProtected() bool { panic("identities needs to be properly loaded with identity.ReadLocal()") } diff --git a/identity/interface.go b/identity/interface.go index 1d534eb8..55877c02 100644 --- a/identity/interface.go +++ b/identity/interface.go @@ -30,6 +30,10 @@ type Interface interface { // the Id is properly set. Commit(repo repository.Repo) error + // If needed, write the identity into the Repository. In particular, this + // ensure that the Id is properly set. + CommitAsNeeded(repo repository.Repo) error + // IsProtected return true if the chain of git commits started to be signed. // If that's the case, only signed commit with a valid key for this identity can be added. IsProtected() bool diff --git a/repository/git.go b/repository/git.go index c2f0da0a..10fddac3 100644 --- a/repository/git.go +++ b/repository/git.go @@ -29,7 +29,7 @@ type GitRepo struct { // 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 { - //fmt.Println("Running git", strings.Join(args, " ")) + // fmt.Printf("[%s] Running git %s\n", repo.Path, strings.Join(args, " ")) cmd := exec.Command("git", args...) cmd.Dir = repo.Path