diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..7f1dae1e9 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +/testdata/repo/main.go text eol=lf +/testdata/repo2/main.go text eol=lf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b4eaeecf4..c8d8303a1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,27 @@ jobs: - name: test run: go test ./... + test-windows: + runs-on: windows-latest + steps: + - name: checkout + uses: actions/checkout@v7 + + - name: setup go + uses: actions/setup-go@v7 + with: + go-version-file: go.mod + cache: true + + - name: test index + run: go test ./index + + - name: test Git repository paths + run: go test ./gitindex -run '^TestFileKeyFullPathUsesRepositorySeparators$' + + - name: build + run: go build ./cmd/zoekt-webserver ./cmd/zoekt-git-index + shellcheck: name: shellcheck runs-on: ubuntu-latest @@ -99,4 +120,3 @@ jobs: github_token: ${{ secrets.GH_TOKEN }} # Check if the generated code is up-to-date - run: .github/workflows/buf-generate-check.sh - diff --git a/cmd/zoekt-index/main.go b/cmd/zoekt-index/main.go index cc3aab15e..c68739a96 100644 --- a/cmd/zoekt-index/main.go +++ b/cmd/zoekt-index/main.go @@ -182,7 +182,11 @@ func indexArg(arg string, opts index.Options, ignore map[string]struct{}) error }() for f := range comm { - displayName := strings.TrimPrefix(f.name, dir+"/") + displayName, err := filepath.Rel(dir, f.name) + if err != nil { + return err + } + displayName = filepath.ToSlash(displayName) if f.size > int64(opts.SizeMax) && !opts.IgnoreSizeMax(displayName) { if err := builder.Add(index.Document{ Name: displayName, diff --git a/cmd/zoekt-index/main_test.go b/cmd/zoekt-index/main_test.go index 07072f23e..7541c88f8 100644 --- a/cmd/zoekt-index/main_test.go +++ b/cmd/zoekt-index/main_test.go @@ -16,7 +16,10 @@ import ( func TestIndexArgAttachesConfiguredBranches(t *testing.T) { sourceDir := t.TempDir() indexDir := t.TempDir() - if err := os.WriteFile(filepath.Join(sourceDir, "file.txt"), []byte("needle\n"), 0o644); err != nil { + if err := os.Mkdir(filepath.Join(sourceDir, "subdir"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sourceDir, "subdir", "file.txt"), []byte("needle\n"), 0o644); err != nil { t.Fatal(err) } @@ -56,6 +59,9 @@ func TestIndexArgAttachesConfiguredBranches(t *testing.T) { if len(result.Files) != 1 { t.Fatalf("main branch returned %d files, want 1", len(result.Files)) } + if got := result.Files[0].FileName; got != "subdir/file.txt" { + t.Fatalf("file name = %q, want %q", got, "subdir/file.txt") + } if got := result.Files[0].Branches; !reflect.DeepEqual(got, []string{"main"}) { t.Fatalf("branches = %v, want [main]", got) } diff --git a/cmd/zoekt-webserver/main.go b/cmd/zoekt-webserver/main.go index 07e14b677..e80f0fe58 100644 --- a/cmd/zoekt-webserver/main.go +++ b/cmd/zoekt-webserver/main.go @@ -34,6 +34,7 @@ import ( "path/filepath" "strconv" "strings" + "syscall" "time" "github.com/opentracing/opentracing-go" @@ -45,7 +46,6 @@ import ( "github.com/uber/jaeger-client-go" oteltrace "go.opentelemetry.io/otel/trace" "go.uber.org/automaxprocs/maxprocs" - "golang.org/x/sys/unix" "google.golang.org/grpc" "github.com/sourcegraph/zoekt" @@ -348,8 +348,8 @@ func addProxyHandler(mux *http.ServeMux, socket string) { // times you will read the channel (used as buffer for signal.Notify). func shutdownSignalChan(maxReads int) <-chan os.Signal { c := make(chan os.Signal, maxReads) - signal.Notify(c, os.Interrupt) // terminal C-c and goreman - signal.Notify(c, unix.SIGTERM) // Kubernetes + signal.Notify(c, os.Interrupt) // terminal C-c and goreman + signal.Notify(c, syscall.SIGTERM) // Kubernetes and Windows shutdown events return c } @@ -464,7 +464,10 @@ func mustRegisterDiskMonitor(path string) { Help: "Amount of free space disk space.", ConstLabels: prometheus.Labels{"path": path}, }, func() float64 { - usage, _ := disk.Usage(path) + usage, err := disk.Usage(path) + if err != nil { + return 0 + } return float64(usage.Free) })) @@ -473,7 +476,10 @@ func mustRegisterDiskMonitor(path string) { Help: "Amount of total disk space.", ConstLabels: prometheus.Labels{"path": path}, }, func() float64 { - usage, _ := disk.Usage(path) + usage, err := disk.Usage(path) + if err != nil { + return 0 + } return float64(usage.Total) })) } diff --git a/cmd/zoekt-webserver/metrics.go b/cmd/zoekt-webserver/metrics_linux.go similarity index 99% rename from cmd/zoekt-webserver/metrics.go rename to cmd/zoekt-webserver/metrics_linux.go index 160d5ddf9..79e9bb395 100644 --- a/cmd/zoekt-webserver/metrics.go +++ b/cmd/zoekt-webserver/metrics_linux.go @@ -1,3 +1,5 @@ +//go:build linux + package main import ( diff --git a/cmd/zoekt-webserver/metrics_nonlinux.go b/cmd/zoekt-webserver/metrics_nonlinux.go new file mode 100644 index 000000000..e34c6be9c --- /dev/null +++ b/cmd/zoekt-webserver/metrics_nonlinux.go @@ -0,0 +1,10 @@ +//go:build !linux + +package main + +import sglog "github.com/sourcegraph/log" + +func mustRegisterMemoryMapMetrics(sglog.Logger) { + // The memory map metrics are collected via /proc, which + // is only available on linux-based operating systems. +} diff --git a/gitindex/index.go b/gitindex/index.go index 4042f6894..0c479d592 100644 --- a/gitindex/index.go +++ b/gitindex/index.go @@ -26,6 +26,7 @@ import ( "math" "net/url" "os" + "path" "path/filepath" "regexp" "sort" @@ -181,7 +182,7 @@ func getCommit(repo *git.Repository, prefix, ref string) (*object.Commit, error) sha1, err := repo.ResolveRevision(plumbing.Revision(ref)) // ref might be a branch name (e.g. "master") add branch prefix and try again. if err != nil { - sha1, err = repo.ResolveRevision(plumbing.Revision(filepath.Join(prefix, ref))) + sha1, err = repo.ResolveRevision(plumbing.Revision(path.Join(prefix, ref))) } if err != nil { return nil, err @@ -396,7 +397,7 @@ func normalizeSubmoduleRemoteURL(cfg *config.Config) (string, error) { // SetTemplatesFromOrigin fills in templates based on the origin URL. func SetTemplatesFromOrigin(desc *zoekt.Repository, u *url.URL) error { - desc.Name = filepath.Join(u.Host, strings.TrimSuffix(u.Path, ".git")) + desc.Name = path.Join(u.Host, strings.TrimSuffix(u.Path, ".git")) if strings.HasSuffix(u.Host, ".googlesource.com") { return setTemplates(desc, u, "gitiles") @@ -479,7 +480,7 @@ func expandBranches(repo *git.Repository, bs []string, prefix string) ([]string, } name := ref.Name().Short() - if matched, err := filepath.Match(b, name); err != nil { + if matched, err := path.Match(b, name); err != nil { return nil, err } else if !matched { continue @@ -1192,14 +1193,14 @@ func prepareNormalBuildRecurse(options Options, repository *git.Repository, repo for k, repo := range sw { rw.Files[fileKey{ - SubRepoPath: filepath.Join(submodule.Config().Path, k.SubRepoPath), + SubRepoPath: path.Join(submodule.Config().Path, k.SubRepoPath), Path: k.Path, ID: k.ID, }] = repo } for k, v := range subVersions { - branchVersions[filepath.Join(submodule.Config().Path, k)] = v + branchVersions[path.Join(submodule.Config().Path, k)] = v } } } diff --git a/gitindex/tree.go b/gitindex/tree.go index b14f0e252..223309560 100644 --- a/gitindex/tree.go +++ b/gitindex/tree.go @@ -20,7 +20,6 @@ import ( "log" "net/url" "path" - "path/filepath" "strings" "github.com/go-git/go-git/v5" @@ -166,13 +165,13 @@ func (rw *RepoWalker) handleSubmodule(p string, id *plumbing.Hash, branch string } for k, repo := range sw.Files { rw.Files[fileKey{ - SubRepoPath: filepath.Join(p, k.SubRepoPath), + SubRepoPath: path.Join(p, k.SubRepoPath), Path: k.Path, ID: k.ID, }] = repo } for k, v := range subVersions { - subRepoVersions[filepath.Join(p, k)] = v + subRepoVersions[path.Join(p, k)] = v } return nil } @@ -222,7 +221,7 @@ type fileKey struct { } func (k *fileKey) FullPath() string { - return filepath.Join(k.SubRepoPath, k.Path) + return path.Join(k.SubRepoPath, k.Path) } // BlobLocation holds the repo where the blob can be found, plus other information diff --git a/gitindex/tree_test.go b/gitindex/tree_test.go index 440064ca4..6a4dd2767 100644 --- a/gitindex/tree_test.go +++ b/gitindex/tree_test.go @@ -37,6 +37,13 @@ import ( "github.com/sourcegraph/zoekt/search" ) +func TestFileKeyFullPathUsesRepositorySeparators(t *testing.T) { + key := fileKey{SubRepoPath: "submodule", Path: "dir/file.go"} + if got, want := key.FullPath(), "submodule/dir/file.go"; got != want { + t.Fatalf("full path = %q, want %q", got, want) + } +} + func createSubmoduleRepo(dir string) error { if err := os.MkdirAll(dir, 0o755); err != nil { return err diff --git a/go.mod b/go.mod index db33117af..0d6d78bab 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/andygrunwald/go-gerrit v1.0.0 github.com/bmatcuk/doublestar/v4 v4.10.0 github.com/dustin/go-humanize v1.0.1 + github.com/edsrzf/mmap-go v1.2.0 github.com/felixge/fgprof v0.9.5 github.com/fsnotify/fsnotify v1.8.0 github.com/gfleury/go-bitbucket-v1 v0.0.0-20240917142304-df385efaac68 diff --git a/go.sum b/go.sum index 6f54532ee..36c8fdc81 100644 --- a/go.sum +++ b/go.sum @@ -76,6 +76,8 @@ github.com/davidmz/go-pageant v1.0.2 h1:bPblRCh5jGU+Uptpz6LgMZGD5hJoOt7otgT454Wv github.com/davidmz/go-pageant v1.0.2/go.mod h1:P2EDDnMqIwG5Rrp05dTRITj9z2zpGcD9efWSkTNKLIE= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= +github.com/edsrzf/mmap-go v1.2.0 h1:hXLYlkbaPzt1SaQk+anYwKSRNhufIDCchSPkUD6dD84= +github.com/edsrzf/mmap-go v1.2.0/go.mod h1:19H/e8pUPLicwkyNgOykDXkJ9F0MHE+Z52B8EIth78Q= github.com/elazarl/goproxy v1.7.2 h1:Y2o6urb7Eule09PjlhQRGNsqRfPmYI3KKQLFpCAV3+o= github.com/elazarl/goproxy v1.7.2/go.mod h1:82vkLNir0ALaW14Rc399OTTjyNREgmdL2cVoIbS6XaE= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= diff --git a/index/builder.go b/index/builder.go index d221981fc..86355d53e 100644 --- a/index/builder.go +++ b/index/builder.go @@ -39,7 +39,6 @@ import ( "github.com/bmatcuk/doublestar/v4" "github.com/dustin/go-humanize" "github.com/rs/xid" - "golang.org/x/sys/unix" "maps" @@ -98,7 +97,8 @@ type Options struct { // LargeFiles is a slice of glob patterns, including ** for any number // of directories, where matching file paths should be indexed - // regardless of their size. The full pattern syntax is here: + // regardless of their size. Paths and patterns always use forward slashes + // as separators. The full pattern syntax is here: // https://github.com/bmatcuk/doublestar/tree/v4#patterns. LargeFiles []string @@ -337,7 +337,7 @@ func (o *Options) SetDefaults() { if o.RepositoryDescription.Name == "" && o.RepositoryDescription.URL != "" { parsed, _ := url.Parse(o.RepositoryDescription.URL) if parsed != nil { - o.RepositoryDescription.Name = filepath.Join(parsed.Host, parsed.Path) + o.RepositoryDescription.Name = path.Join(parsed.Host, parsed.Path) } } } @@ -492,7 +492,7 @@ func (o *Options) findShard() string { } func (o *Options) findCompoundShard() string { - compoundShards, err := filepath.Glob(path.Join(o.IndexDir, "compound-*.zoekt")) + compoundShards, err := filepath.Glob(filepath.Join(o.IndexDir, "compound-*.zoekt")) if err != nil { return "" } @@ -535,7 +535,7 @@ func (o *Options) IgnoreSizeMax(name string) bool { pattern := strings.TrimSpace(v) negated, validatedPattern := checkIsNegatePattern(pattern) - if m, _ := doublestar.PathMatch(validatedPattern, name); m { + if m, _ := doublestar.Match(validatedPattern, filepath.ToSlash(name)); m { if negated { return false } else { @@ -1127,8 +1127,3 @@ func (e *deltaIndexOptionsMismatchError) Error() string { // umask holds the Umask of the current process var umask os.FileMode - -func init() { - umask = os.FileMode(unix.Umask(0)) - unix.Umask(int(umask)) -} diff --git a/index/builder_test.go b/index/builder_test.go index d8bf8ee13..4c16187e3 100644 --- a/index/builder_test.go +++ b/index/builder_test.go @@ -24,6 +24,19 @@ import ( var update = flag.Bool("update", false, "update golden file") +func TestOptionsSetDefaultsUsesRepositoryPaths(t *testing.T) { + opts := Options{ + RepositoryDescription: zoekt.Repository{ + URL: "https://github.com/sourcegraph/zoekt", + }, + } + opts.SetDefaults() + + if got, want := opts.RepositoryDescription.Name, "github.com/sourcegraph/zoekt"; got != want { + t.Fatalf("repository name = %q, want %q", got, want) + } +} + // ensure we don't regress on how we build v16 func TestBuildv16(t *testing.T) { dir := t.TempDir() @@ -980,6 +993,12 @@ func TestIgnoreSizeMax(t *testing.T) { filePaths []string expected bool }{ + { + name: "star does not match across directories", + largeFiles: []string{"dir/*.md"}, + filePaths: []string{"dir/sub/file.md"}, + expected: false, + }, { name: "empty pattern does nothing", largeFiles: []string{""}, @@ -1179,12 +1198,12 @@ func TestOptions_shardName(t *testing.T) { } t.Setenv("WORKSPACES_API_URL", "") - if got, want := opts.shardNameVersion(16, 0), "/data/a%2Fb_v16.00000.zoekt"; got != want { + if got, want := opts.shardNameVersion(16, 0), filepath.Join("/data", "a%2Fb_v16.00000.zoekt"); got != want { t.Fatalf("expected shard name to be repo name based:\ngot: %q\nwant: %q", got, want) } t.Setenv("WORKSPACES_API_URL", "http://example.com") - if got, want := opts.shardNameVersion(16, 0), "/data/000000123_000000456_v16.00000.zoekt"; got != want { + if got, want := opts.shardNameVersion(16, 0), filepath.Join("/data", "000000123_000000456_v16.00000.zoekt"); got != want { t.Fatalf("expected shard name to be ID based:\ngot: %q\nwant: %q", got, want) } @@ -1196,7 +1215,7 @@ func TestOptions_shardName(t *testing.T) { Name: "a/b", }, } - if got, want := opts.shardNameVersion(16, 0), "/data/000000000_000000000_v16.00000.zoekt"; got != want { + if got, want := opts.shardNameVersion(16, 0), filepath.Join("/data", "000000000_000000000_v16.00000.zoekt"); got != want { t.Fatalf("expected shard name to be with no tenant:\ngot: %q\nwant: %q", got, want) } } diff --git a/index/builder_unix.go b/index/builder_unix.go new file mode 100644 index 000000000..45f772fdc --- /dev/null +++ b/index/builder_unix.go @@ -0,0 +1,14 @@ +//go:build !windows + +package index + +import ( + "os" + + "golang.org/x/sys/unix" +) + +func init() { + umask = os.FileMode(unix.Umask(0)) + unix.Umask(int(umask)) +} diff --git a/index/indexfile_test.go b/index/indexfile_test.go new file mode 100644 index 000000000..e0395db1f --- /dev/null +++ b/index/indexfile_test.go @@ -0,0 +1,38 @@ +package index + +import ( + "os" + "path/filepath" + "testing" +) + +func TestNewIndexFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.zoekt") + if err := os.WriteFile(path, []byte("hello"), 0o600); err != nil { + t.Fatal(err) + } + + f, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + + indexFile, err := NewIndexFile(f) + if err != nil { + t.Fatal(err) + } + t.Cleanup(indexFile.Close) + + if got := indexFile.Name(); got != path { + t.Errorf("Name() = %q, want %q", got, path) + } + if got, err := indexFile.Size(); err != nil || got != 5 { + t.Errorf("Size() = %d, %v; want 5, nil", got, err) + } + if got, err := indexFile.Read(1, 3); err != nil || string(got) != "ell" { + t.Errorf("Read(1, 3) = %q, %v; want %q, nil", got, err, "ell") + } + if _, err := indexFile.Read(3, 3); err == nil { + t.Error("Read(3, 3) unexpectedly succeeded") + } +} diff --git a/index/indexfile.go b/index/indexfile_unix.go similarity index 95% rename from index/indexfile.go rename to index/indexfile_unix.go index 33f04872c..6508a2f4f 100644 --- a/index/indexfile.go +++ b/index/indexfile_unix.go @@ -32,8 +32,8 @@ type mmapedIndexFile struct { } func (f *mmapedIndexFile) Read(off, sz uint32) ([]byte, error) { - if off > off+sz || off+sz > uint32(len(f.data)) { - return nil, fmt.Errorf("out of bounds: %d, len %d, name %s", off+sz, len(f.data), f.name) + if off > off+sz || off+sz > f.size { + return nil, fmt.Errorf("out of bounds: %d, len %d, name %s", off+sz, f.size, f.name) } return f.data[off : off+sz], nil } diff --git a/index/indexfile_windows.go b/index/indexfile_windows.go new file mode 100644 index 000000000..306c211ac --- /dev/null +++ b/index/indexfile_windows.go @@ -0,0 +1,64 @@ +package index + +import ( + "fmt" + "log" + "math" + "os" + + mmap "github.com/edsrzf/mmap-go" +) + +type mmapedIndexFile struct { + name string + size uint32 + data mmap.MMap +} + +func (f *mmapedIndexFile) Read(off, sz uint32) ([]byte, error) { + if off > off+sz || off+sz > f.size { + return nil, fmt.Errorf("out of bounds: %d, len %d, name %s", off+sz, f.size, f.name) + } + return f.data[off : off+sz], nil +} + +func (f *mmapedIndexFile) Size() (uint32, error) { + return f.size, nil +} + +func (f *mmapedIndexFile) Close() { + if err := f.data.Unmap(); err != nil { + log.Printf("WARN failed to unmap %s: %v", f.name, err) + } +} + +func (f *mmapedIndexFile) Name() string { + return f.name +} + +// NewIndexFile returns a new index file. The index file takes +// ownership of the passed in file, and may close it. +func NewIndexFile(f *os.File) (IndexFile, error) { + defer f.Close() + + fi, err := f.Stat() + if err != nil { + return nil, err + } + + sz := fi.Size() + if sz >= math.MaxUint32 { + return nil, fmt.Errorf("file %s too large: %d", f.Name(), sz) + } + + data, err := mmap.MapRegion(f, int(sz), mmap.RDONLY, 0, 0) + if err != nil { + return nil, fmt.Errorf("memory mapping %s: %w", f.Name(), err) + } + + return &mmapedIndexFile{ + name: f.Name(), + size: uint32(sz), + data: data, + }, nil +} diff --git a/index/shard_builder_test.go b/index/shard_builder_test.go index 5359f1a95..76c83f2e3 100644 --- a/index/shard_builder_test.go +++ b/index/shard_builder_test.go @@ -1,6 +1,7 @@ package index import ( + "path/filepath" "strings" "testing" @@ -22,7 +23,7 @@ func TestShardName(t *testing.T) { prefix: "short", version: 1, shardNum: 42, - expected: "index/short_v1.00042.zoekt", + expected: filepath.Join("index", "short_v1.00042.zoekt"), }, { name: "long prefix truncated", @@ -30,7 +31,7 @@ func TestShardName(t *testing.T) { prefix: strings.Repeat("a", 300), version: 2, shardNum: 1, - expected: "index/" + strings.Repeat("a", 200) + "003ef1ba" + "_v2.00001.zoekt", + expected: filepath.Join("index", strings.Repeat("a", 200)+"003ef1ba"+"_v2.00001.zoekt"), }, { name: "empty indexDir",