Skip to content

Commit 01192ca

Browse files
asiyaniasiyani
authored
Merge pull request #38 from utilitywarehouse/as-pathspec-list
avoid mulitple worktrees on same ref by using pathspect list
2 parents 51141a5 + 21994b5 commit 01192ca

8 files changed

+139
-70
lines changed

‎config_test.go

+39-15
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,13 @@ func Test_diffWorktrees(t *testing.T) {
108108
Remote: "[email protected]:path/to/repo1.git",
109109
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
110110
Worktrees: []mirror.WorktreeConfig{
111-
{Link: "link", Ref: "master", Pathspec: ""},
112-
{Link: "link2", Ref: "other-branch", Pathspec: "path"},
111+
{Link: "link", Ref: "master", Pathspecs: nil},
112+
{Link: "link2", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
113113
},
114114
},
115115
wantNewWTCs: []mirror.WorktreeConfig{
116116
{Link: "link", Ref: "master"},
117-
{Link: "link2", Ref: "other-branch", Pathspec: "path"},
117+
{Link: "link2", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
118118
},
119119
wantRemovedWTs: nil,
120120
},
@@ -124,44 +124,68 @@ func Test_diffWorktrees(t *testing.T) {
124124
Remote: "[email protected]:path/to/repo1.git",
125125
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
126126
Worktrees: []mirror.WorktreeConfig{
127-
{Link: "link", Ref: "master", Pathspec: ""},
128-
{Link: "link2", Ref: "other-branch", Pathspec: "path"},
127+
{Link: "link", Ref: "master", Pathspecs: nil},
128+
{Link: "link2", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
129+
{Link: "link3", Ref: "other-branch", Pathspecs: []string{"path"}},
129130
},
130131
},
131132
newRepoConf: &mirror.RepositoryConfig{
132133
Remote: "[email protected]:path/to/repo1.git",
133134
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
134135
Worktrees: []mirror.WorktreeConfig{
135-
{Link: "link", Ref: "master", Pathspec: "new-path"},
136-
{Link: "link2", Ref: "new-branch", Pathspec: "path"},
136+
{Link: "link", Ref: "master", Pathspecs: []string{"new-path"}},
137+
{Link: "link2", Ref: "new-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
138+
{Link: "link3", Ref: "other-branch", Pathspecs: []string{"path", "new-path"}},
137139
},
138140
},
139141
wantNewWTCs: []mirror.WorktreeConfig{
140-
{Link: "link", Ref: "master", Pathspec: "new-path"},
141-
{Link: "link2", Ref: "new-branch", Pathspec: "path"},
142+
{Link: "link", Ref: "master", Pathspecs: []string{"new-path"}},
143+
{Link: "link2", Ref: "new-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
144+
{Link: "link3", Ref: "other-branch", Pathspecs: []string{"path", "new-path"}},
142145
},
143-
wantRemovedWTs: []string{"link", "link2"},
146+
wantRemovedWTs: []string{"link", "link2", "link3"},
147+
},
148+
{
149+
name: "rearrange-path",
150+
initialRepoConf: &mirror.RepositoryConfig{
151+
Remote: "[email protected]:path/to/repo1.git",
152+
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
153+
Worktrees: []mirror.WorktreeConfig{
154+
{Link: "link", Ref: "master", Pathspecs: []string{"a", "b/**/c"}},
155+
{Link: "link2", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
156+
},
157+
},
158+
newRepoConf: &mirror.RepositoryConfig{
159+
Remote: "[email protected]:path/to/repo1.git",
160+
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
161+
Worktrees: []mirror.WorktreeConfig{
162+
{Link: "link", Ref: "master", Pathspecs: []string{"b/**/c", "a"}},
163+
{Link: "link2", Ref: "other-branch", Pathspecs: []string{"path1", "*.c", "path2/**/*.yaml"}},
164+
},
165+
},
166+
wantNewWTCs: nil,
167+
wantRemovedWTs: nil,
144168
},
145169
{
146170
name: "add_new_link",
147171
initialRepoConf: &mirror.RepositoryConfig{
148172
Remote: "[email protected]:path/to/repo1.git",
149173
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
150174
Worktrees: []mirror.WorktreeConfig{
151-
{Link: "link", Ref: "master", Pathspec: ""},
152-
{Link: "link2", Ref: "other-branch", Pathspec: "path"},
175+
{Link: "link", Ref: "master", Pathspecs: nil},
176+
{Link: "link2", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
153177
},
154178
},
155179
newRepoConf: &mirror.RepositoryConfig{
156180
Remote: "[email protected]:path/to/repo1.git",
157181
Root: "/root", Interval: 10 * time.Second, GitGC: "always",
158182
Worktrees: []mirror.WorktreeConfig{
159-
{Link: "link", Ref: "master", Pathspec: ""},
160-
{Link: "link3", Ref: "other-branch", Pathspec: "path"},
183+
{Link: "link", Ref: "master", Pathspecs: nil},
184+
{Link: "link3", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
161185
},
162186
},
163187
wantNewWTCs: []mirror.WorktreeConfig{
164-
{Link: "link3", Ref: "other-branch", Pathspec: "path"},
188+
{Link: "link3", Ref: "other-branch", Pathspecs: []string{"path1", "path2/**/*.yaml", "*.c"}},
165189
},
166190
wantRemovedWTs: []string{"link2"},
167191
},

‎pkg/mirror/config.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ type WorktreeConfig struct {
7474
// are supported. default is HEAD
7575
Ref string `yaml:"ref"`
7676

77-
// Pathspec of the dirs to checkout if required
78-
Pathspec string `yaml:"pathspec"`
77+
// Pathspecs of the dirs to checkout if required
78+
Pathspecs []string `yaml:"pathspecs"`
7979
}
8080

8181
// Auth represents authentication config of the repository

‎pkg/mirror/repository.go

+14-12
Original file line numberDiff line numberDiff line change
@@ -148,13 +148,16 @@ func (r *Repository) AddWorktreeLink(wtc WorktreeConfig) error {
148148
}
149149

150150
wt := &WorkTreeLink{
151-
link: wtc.Link,
152-
linkAbs: linkAbs,
153-
ref: wtc.Ref,
154-
pathspec: wtc.Pathspec,
155-
log: r.log.With("worktree", wtc.Link),
151+
link: wtc.Link,
152+
linkAbs: linkAbs,
153+
ref: wtc.Ref,
154+
pathspecs: wtc.Pathspecs,
155+
log: r.log.With("worktree", wtc.Link),
156156
}
157157

158+
// pathspecs must be sorted for for worktree equality checks
159+
slices.Sort(wt.pathspecs)
160+
158161
r.workTreeLinks[wtc.Link] = wt
159162
return nil
160163
}
@@ -363,10 +366,8 @@ func (r *Repository) cloneByRef(ctx context.Context, dst, ref, pathspec string,
363366

364367
args := []string{"reset", "--hard", ref}
365368
// git reset --hard <ref>
366-
if out, err := runGitCommand(ctx, r.log, nil, dst, args...); err != nil {
369+
if _, err := runGitCommand(ctx, r.log, nil, dst, args...); err != nil {
367370
return "", err
368-
} else {
369-
fmt.Println(out)
370371
}
371372

372373
// get the hash of the repos HEAD
@@ -708,7 +709,7 @@ func (r *Repository) hash(ctx context.Context, ref, path string) (string, error)
708709
// it will remove worktree if tracking ref is removed from the remote
709710
func (r *Repository) ensureWorktreeLink(ctx context.Context, wl *WorkTreeLink) error {
710711
// get remote hash from mirrored repo for the worktree link
711-
remoteHash, err := r.hash(ctx, wl.ref, wl.pathspec)
712+
remoteHash, err := r.hash(ctx, wl.ref, "")
712713
if err != nil {
713714
return fmt.Errorf("unable to get hash for worktree:%s err:%w", wl.link, err)
714715
}
@@ -798,10 +799,11 @@ func (r *Repository) createWorktree(ctx context.Context, wl *WorkTreeLink, hash
798799

799800
// only checkout required path if specified
800801
args := []string{"checkout", hash}
801-
if wl.pathspec != "" {
802-
args = append(args, "--", wl.pathspec)
802+
if len(wl.pathspecs) > 0 {
803+
args = append(args, "--")
804+
args = append(args, wl.pathspecs...)
803805
}
804-
// git checkout <hash> -- <pathspec>
806+
// git checkout <hash> -- <pathspec...>
805807
if _, err := runGitCommand(ctx, wl.log, nil, wtPath, args...); err != nil {
806808
return "", err
807809
}

‎pkg/mirror/repository_test.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,12 @@ func TestRepo_AddWorktreeLink(t *testing.T) {
139139
args args
140140
wantErr bool
141141
}{
142-
{"all-valid", args{wtc: WorktreeConfig{"link", "master", ""}}, false},
143-
{"all-valid-with-path", args{wtc: WorktreeConfig{"link2", "other-branch", "path"}}, false},
144-
{"duplicate-link", args{wtc: WorktreeConfig{"link", "master", ""}}, true},
145-
{"no-link", args{wtc: WorktreeConfig{"", "master", ""}}, true},
146-
{"no-ref", args{wtc: WorktreeConfig{"link3", "", ""}}, false},
147-
{"absLink", args{wtc: WorktreeConfig{"/tmp/link", "tag", ""}}, false},
142+
{"all-valid", args{wtc: WorktreeConfig{"link", "master", []string{}}}, false},
143+
{"all-valid-with-paths", args{wtc: WorktreeConfig{"link2", "other-branch", []string{"path1", "path2/**/*.yaml", "*.c"}}}, false},
144+
{"duplicate-link", args{wtc: WorktreeConfig{"link", "master", []string{}}}, true},
145+
{"no-link", args{wtc: WorktreeConfig{"", "master", []string{}}}, true},
146+
{"no-ref", args{wtc: WorktreeConfig{"link3", "", []string{}}}, false},
147+
{"absLink", args{wtc: WorktreeConfig{"/tmp/link", "tag", []string{}}}, false},
148148
}
149149
for _, tt := range tests {
150150
t.Run(tt.name, func(t *testing.T) {
@@ -155,10 +155,10 @@ func TestRepo_AddWorktreeLink(t *testing.T) {
155155
}
156156
// compare all worktree links
157157
want := map[string]*WorkTreeLink{
158-
"link": {link: "link", linkAbs: "/tmp/root/link", ref: "master"},
159-
"link2": {link: "link2", linkAbs: "/tmp/root/link2", ref: "other-branch", pathspec: "path"},
160-
"link3": {link: "link3", linkAbs: "/tmp/root/link3", ref: "HEAD"},
161-
"/tmp/link": {link: "/tmp/link", linkAbs: "/tmp/link", ref: "tag"},
158+
"link": {link: "link", linkAbs: "/tmp/root/link", ref: "master", pathspecs: []string{}},
159+
"link2": {link: "link2", linkAbs: "/tmp/root/link2", ref: "other-branch", pathspecs: []string{"*.c", "path1", "path2/**/*.yaml"}},
160+
"link3": {link: "link3", linkAbs: "/tmp/root/link3", ref: "HEAD", pathspecs: []string{}},
161+
"/tmp/link": {link: "/tmp/link", linkAbs: "/tmp/link", ref: "tag", pathspecs: []string{}},
162162
}
163163
if diff := cmp.Diff(want, r.workTreeLinks, cmpopts.IgnoreFields(WorkTreeLink{}, "log"), cmp.AllowUnexported(WorkTreeLink{})); diff != "" {
164164
t.Errorf("Repo.AddWorktreeLink() worktreelinks mismatch (-want +got):\n%s", diff)

‎pkg/mirror/worktree.go

+11-7
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,25 @@ import (
55
"fmt"
66
"log/slog"
77
"path/filepath"
8+
"slices"
89
"strings"
910
)
1011

1112
type WorkTreeLink struct {
12-
link string // link name as its specified in config, might not be unique only use it for logging
13-
linkAbs string // the path at which to create a symlink to the worktree dir
14-
ref string // the ref of the worktree
15-
pathspec string // pathspec of the dirs to checkout
16-
log *slog.Logger
13+
link string // link name as its specified in config, might not be unique only use it for logging
14+
linkAbs string // the path at which to create a symlink to the worktree dir
15+
ref string // the ref of the worktree
16+
pathspecs []string // pathspecs of the paths to checkout
17+
log *slog.Logger
1718
}
1819

1920
func (wt *WorkTreeLink) Equals(wtc WorktreeConfig) bool {
21+
sortedConfigPaths := slices.Clone(wtc.Pathspecs)
22+
slices.Sort(sortedConfigPaths)
23+
2024
return wt.link == wtc.Link &&
21-
wt.pathspec == wtc.Pathspec &&
22-
wt.ref == wtc.Ref
25+
wt.ref == wtc.Ref &&
26+
slices.Compare(wt.pathspecs, sortedConfigPaths) == 0
2327
}
2428

2529
// worktreeDirName will generate worktree name for specific worktree link

‎pkg/mirror/z_e2e_race_test.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package mirror
44

55
import (
66
"context"
7-
"log"
87
"os"
98
"path/filepath"
109
"sync"
@@ -31,7 +30,7 @@ func Test_mirror_detect_race(t *testing.T) {
3130

3231
repo := mustCreateRepoAndMirror(t, upstream, root, link1, ref1)
3332
// add worktree for HEAD
34-
if err := repo.AddWorktreeLink(WorktreeConfig{link2, ref2, ""}); err != nil {
33+
if err := repo.AddWorktreeLink(WorktreeConfig{link2, ref2, []string{}}); err != nil {
3534
t.Fatalf("unable to add worktree error: %v", err)
3635
}
3736
// mirror again for 2nd worktree
@@ -51,7 +50,7 @@ func Test_mirror_detect_race(t *testing.T) {
5150
t.Log("TEST-2: forward HEAD")
5251
fileSHA2 := mustCommit(t, upstream, "file", testName+"-2")
5352

54-
t.Run("test-1", func(t *testing.T) {
53+
t.Run("clone-test", func(t *testing.T) {
5554
wg := &sync.WaitGroup{}
5655
// all following assertions will always be true
5756
// this test is about testing deadlocks and detecting race conditions
@@ -60,7 +59,8 @@ func Test_mirror_detect_race(t *testing.T) {
6059
go func() {
6160
defer wg.Done()
6261
if err := repo.Mirror(ctx); err != nil {
63-
log.Fatalf("unable to mirror error: %v", err)
62+
t.Error("unable to mirror", "err", err)
63+
os.Exit(1)
6464
}
6565

6666
assertLinkedFile(t, root, link1, "file", testName+"-2")
@@ -81,7 +81,8 @@ func Test_mirror_detect_race(t *testing.T) {
8181
go func() {
8282
defer wg.Done()
8383
if err := repo.Mirror(ctx); err != nil {
84-
log.Fatalf("unable to mirror error: %v", err)
84+
t.Error("unable to mirror error", "err", err)
85+
os.Exit(1)
8586
}
8687
}()
8788

0 commit comments

Comments
 (0)