From f04f859ba8f22fa6ad8d189b96ab4f0d02e67ae5 Mon Sep 17 00:00:00 2001 From: Erik Seliger Date: Thu, 18 Apr 2024 23:48:18 +0200 Subject: [PATCH] gitserver: Remove revspec updates (#61861) Revspec updates never worked reliably. When there was _any_ operation in flight, we would just not fetch this revspec and return "all good". This is only used by packages, an experimental feature. Gitserver should not modify the authoritative data source for what versions exist. Instead, callers should add the version to the list of desired versions, and they'll eventually be part of the package when the next fetch occurred. Anyhow, experimental, didn't work reliably, will be removed for now. Test plan: Existing test suites. --- cmd/gitserver/internal/ensurerevision.go | 2 +- cmd/gitserver/internal/server.go | 6 +- cmd/gitserver/internal/server_test.go | 6 +- cmd/gitserver/internal/vcssyncer/git.go | 2 +- .../internal/vcssyncer/instrumented_syncer.go | 16 +++-- cmd/gitserver/internal/vcssyncer/mock.go | 29 ++++---- .../internal/vcssyncer/packages_syncer.go | 72 +------------------ .../vcssyncer/packages_syncer_test.go | 54 ++------------ cmd/gitserver/internal/vcssyncer/perforce.go | 2 +- cmd/gitserver/internal/vcssyncer/syncer.go | 8 +-- 10 files changed, 41 insertions(+), 156 deletions(-) diff --git a/cmd/gitserver/internal/ensurerevision.go b/cmd/gitserver/internal/ensurerevision.go index 88500a9e9b2..906b49c164f 100644 --- a/cmd/gitserver/internal/ensurerevision.go +++ b/cmd/gitserver/internal/ensurerevision.go @@ -32,7 +32,7 @@ func (s *Server) EnsureRevision(ctx context.Context, repo api.RepoName, rev stri } // Revision not found, update before returning. - err := s.doRepoUpdate(ctx, repo, rev) + _, _, err := s.RepoUpdate(ctx, repo) if err != nil { if ctx.Err() == nil { ensureRevisionCounter.WithLabelValues("update_failed").Inc() diff --git a/cmd/gitserver/internal/server.go b/cmd/gitserver/internal/server.go index 9d7edc4d383..2db490eb662 100644 --- a/cmd/gitserver/internal/server.go +++ b/cmd/gitserver/internal/server.go @@ -371,7 +371,7 @@ func (s *Server) repoUpdateOrClone(ctx context.Context, repoName api.RepoName) e repoClonedCounter.Inc() logger.Info("cloned repo", log.String("repo", string(repoName))) } else { - if err := s.doRepoUpdate(ctx, repoName, ""); err != nil { + if err := s.doRepoUpdate(ctx, repoName); err != nil { // The repo update might have failed due to the repo being corrupt s.LogIfCorrupt(ctx, repoName, err) @@ -691,7 +691,7 @@ var ( var doBackgroundRepoUpdateMock func(api.RepoName) error -func (s *Server) doRepoUpdate(ctx context.Context, repo api.RepoName, revspec string) (err error) { +func (s *Server) doRepoUpdate(ctx context.Context, repo api.RepoName) (err error) { logger := s.logger.Scoped("repoUpdate").With(log.String("repo", string(repo))) if doBackgroundRepoUpdateMock != nil { @@ -722,7 +722,7 @@ func (s *Server) doRepoUpdate(ctx context.Context, repo api.RepoName, revspec st // ensure the background update doesn't hang forever fetchCtx, cancelTimeout := context.WithTimeout(ctx, fetchTimeout) defer cancelTimeout() - output, err := syncer.Fetch(fetchCtx, repo, dir, revspec) + output, err := syncer.Fetch(fetchCtx, repo, dir) // best-effort update the output of the fetch if err := s.db.GitserverRepos().SetLastOutput(ctx, repo, string(output)); err != nil { s.logger.Warn("Setting last output in DB", log.Error(err)) diff --git a/cmd/gitserver/internal/server_test.go b/cmd/gitserver/internal/server_test.go index afbfe06086f..471618cafde 100644 --- a/cmd/gitserver/internal/server_test.go +++ b/cmd/gitserver/internal/server_test.go @@ -1141,7 +1141,7 @@ type mockVCSSyncer struct { mockTypeFunc func() string mockIsCloneable func(ctx context.Context, repoName api.RepoName) error mockClone func(ctx context.Context, repo api.RepoName, targetDir common.GitDir, tmpPath string, progressWriter io.Writer) error - mockFetch func(ctx context.Context, repoName api.RepoName, dir common.GitDir, revspec string) ([]byte, error) + mockFetch func(ctx context.Context, repoName api.RepoName, dir common.GitDir) ([]byte, error) } func (m *mockVCSSyncer) Type() string { @@ -1168,9 +1168,9 @@ func (m *mockVCSSyncer) Clone(ctx context.Context, repo api.RepoName, targetDir return errors.New("no mock for Clone() is set") } -func (m *mockVCSSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir, revspec string) ([]byte, error) { +func (m *mockVCSSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir) ([]byte, error) { if m.mockFetch != nil { - return m.mockFetch(ctx, repoName, dir, revspec) + return m.mockFetch(ctx, repoName, dir) } return nil, errors.New("no mock for Fetch() is set") diff --git a/cmd/gitserver/internal/vcssyncer/git.go b/cmd/gitserver/internal/vcssyncer/git.go index 4aeb1f445e4..bea53a01a11 100644 --- a/cmd/gitserver/internal/vcssyncer/git.go +++ b/cmd/gitserver/internal/vcssyncer/git.go @@ -162,7 +162,7 @@ func (s *gitRepoSyncer) Clone(ctx context.Context, repo api.RepoName, _ common.G } // Fetch tries to fetch updates of a Git repository. -func (s *gitRepoSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir, _ string) ([]byte, error) { +func (s *gitRepoSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir) ([]byte, error) { source, err := s.getRemoteURLSource(ctx, repoName) if err != nil { return nil, errors.Wrapf(err, "failed to get remote URL source for %s", repoName) diff --git a/cmd/gitserver/internal/vcssyncer/instrumented_syncer.go b/cmd/gitserver/internal/vcssyncer/instrumented_syncer.go index 2bdbd7e5937..badcf2eb980 100644 --- a/cmd/gitserver/internal/vcssyncer/instrumented_syncer.go +++ b/cmd/gitserver/internal/vcssyncer/instrumented_syncer.go @@ -2,14 +2,16 @@ package vcssyncer import ( "context" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/sourcegraph/sourcegraph/cmd/gitserver/internal/common" - "github.com/sourcegraph/sourcegraph/internal/api" "io" "strconv" "strings" "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + + "github.com/sourcegraph/sourcegraph/cmd/gitserver/internal/common" + "github.com/sourcegraph/sourcegraph/internal/api" ) // fetchBuckets are the buckets used for the fetch and clone duration histograms. @@ -94,9 +96,9 @@ func (i *instrumentedSyncer) Clone(ctx context.Context, repo api.RepoName, targe return i.base.Clone(ctx, repo, targetDir, tmpPath, progressWriter) } -func (i *instrumentedSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir, revspec string) (output []byte, err error) { +func (i *instrumentedSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir) (output []byte, err error) { if !i.shouldObserve() { - return i.base.Fetch(ctx, repoName, dir, revspec) + return i.base.Fetch(ctx, repoName, dir) } start := time.Now() @@ -107,7 +109,7 @@ func (i *instrumentedSyncer) Fetch(ctx context.Context, repoName api.RepoName, d metricFetchDuration.WithLabelValues(i.formattedTypeLabel, strconv.FormatBool(succeeded)).Observe(duration) }() - return i.base.Fetch(ctx, repoName, dir, revspec) + return i.base.Fetch(ctx, repoName, dir) } func (i *instrumentedSyncer) shouldObserve() bool { diff --git a/cmd/gitserver/internal/vcssyncer/mock.go b/cmd/gitserver/internal/vcssyncer/mock.go index 21b7e718e32..b3230810b80 100644 --- a/cmd/gitserver/internal/vcssyncer/mock.go +++ b/cmd/gitserver/internal/vcssyncer/mock.go @@ -44,7 +44,7 @@ func NewMockVCSSyncer() *MockVCSSyncer { }, }, FetchFunc: &VCSSyncerFetchFunc{ - defaultHook: func(context.Context, api.RepoName, common.GitDir, string) (r0 []byte, r1 error) { + defaultHook: func(context.Context, api.RepoName, common.GitDir) (r0 []byte, r1 error) { return }, }, @@ -71,7 +71,7 @@ func NewStrictMockVCSSyncer() *MockVCSSyncer { }, }, FetchFunc: &VCSSyncerFetchFunc{ - defaultHook: func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error) { + defaultHook: func(context.Context, api.RepoName, common.GitDir) ([]byte, error) { panic("unexpected invocation of MockVCSSyncer.Fetch") }, }, @@ -223,23 +223,23 @@ func (c VCSSyncerCloneFuncCall) Results() []interface{} { // VCSSyncerFetchFunc describes the behavior when the Fetch method of the // parent MockVCSSyncer instance is invoked. type VCSSyncerFetchFunc struct { - defaultHook func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error) - hooks []func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error) + defaultHook func(context.Context, api.RepoName, common.GitDir) ([]byte, error) + hooks []func(context.Context, api.RepoName, common.GitDir) ([]byte, error) history []VCSSyncerFetchFuncCall mutex sync.Mutex } // Fetch delegates to the next hook function in the queue and stores the // parameter and result values of this invocation. -func (m *MockVCSSyncer) Fetch(v0 context.Context, v1 api.RepoName, v2 common.GitDir, v3 string) ([]byte, error) { - r0, r1 := m.FetchFunc.nextHook()(v0, v1, v2, v3) - m.FetchFunc.appendCall(VCSSyncerFetchFuncCall{v0, v1, v2, v3, r0, r1}) +func (m *MockVCSSyncer) Fetch(v0 context.Context, v1 api.RepoName, v2 common.GitDir) ([]byte, error) { + r0, r1 := m.FetchFunc.nextHook()(v0, v1, v2) + m.FetchFunc.appendCall(VCSSyncerFetchFuncCall{v0, v1, v2, r0, r1}) return r0, r1 } // SetDefaultHook sets function that is called when the Fetch method of the // parent MockVCSSyncer instance is invoked and the hook queue is empty. -func (f *VCSSyncerFetchFunc) SetDefaultHook(hook func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error)) { +func (f *VCSSyncerFetchFunc) SetDefaultHook(hook func(context.Context, api.RepoName, common.GitDir) ([]byte, error)) { f.defaultHook = hook } @@ -247,7 +247,7 @@ func (f *VCSSyncerFetchFunc) SetDefaultHook(hook func(context.Context, api.RepoN // Fetch method of the parent MockVCSSyncer instance invokes the hook at the // front of the queue and discards it. After the queue is empty, the default // hook function is invoked for any future action. -func (f *VCSSyncerFetchFunc) PushHook(hook func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error)) { +func (f *VCSSyncerFetchFunc) PushHook(hook func(context.Context, api.RepoName, common.GitDir) ([]byte, error)) { f.mutex.Lock() f.hooks = append(f.hooks, hook) f.mutex.Unlock() @@ -256,19 +256,19 @@ func (f *VCSSyncerFetchFunc) PushHook(hook func(context.Context, api.RepoName, c // SetDefaultReturn calls SetDefaultHook with a function that returns the // given values. func (f *VCSSyncerFetchFunc) SetDefaultReturn(r0 []byte, r1 error) { - f.SetDefaultHook(func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error) { + f.SetDefaultHook(func(context.Context, api.RepoName, common.GitDir) ([]byte, error) { return r0, r1 }) } // PushReturn calls PushHook with a function that returns the given values. func (f *VCSSyncerFetchFunc) PushReturn(r0 []byte, r1 error) { - f.PushHook(func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error) { + f.PushHook(func(context.Context, api.RepoName, common.GitDir) ([]byte, error) { return r0, r1 }) } -func (f *VCSSyncerFetchFunc) nextHook() func(context.Context, api.RepoName, common.GitDir, string) ([]byte, error) { +func (f *VCSSyncerFetchFunc) nextHook() func(context.Context, api.RepoName, common.GitDir) ([]byte, error) { f.mutex.Lock() defer f.mutex.Unlock() @@ -310,9 +310,6 @@ type VCSSyncerFetchFuncCall struct { // Arg2 is the value of the 3rd argument passed to this method // invocation. Arg2 common.GitDir - // Arg3 is the value of the 4th argument passed to this method - // invocation. - Arg3 string // Result0 is the value of the 1st result returned from this method // invocation. Result0 []byte @@ -324,7 +321,7 @@ type VCSSyncerFetchFuncCall struct { // Args returns an interface slice containing the arguments of this // invocation. func (c VCSSyncerFetchFuncCall) Args() []interface{} { - return []interface{}{c.Arg0, c.Arg1, c.Arg2, c.Arg3} + return []interface{}{c.Arg0, c.Arg1, c.Arg2} } // Results returns an interface slice containing the results of this diff --git a/cmd/gitserver/internal/vcssyncer/packages_syncer.go b/cmd/gitserver/internal/vcssyncer/packages_syncer.go index 7f1dddeef8b..e5fa9ce2fcd 100644 --- a/cmd/gitserver/internal/vcssyncer/packages_syncer.go +++ b/cmd/gitserver/internal/vcssyncer/packages_syncer.go @@ -8,7 +8,6 @@ import ( "path" "sort" "strings" - "time" "github.com/sourcegraph/log" @@ -98,14 +97,14 @@ func (s *vcsPackagesSyncer) Clone(ctx context.Context, repo api.RepoName, _ comm // The Fetch method is responsible for cleaning up temporary directories. // TODO: We should have more fine-grained progress reporting here. tryWrite(s.logger, progressWriter, "Fetching package revisions\n") - if _, err := s.Fetch(ctx, repo, common.GitDir(tmpPath), ""); err != nil { + if _, err := s.Fetch(ctx, repo, common.GitDir(tmpPath)); err != nil { return errors.Wrapf(err, "failed to fetch repo for %s", repo) } return nil } -func (s *vcsPackagesSyncer) Fetch(ctx context.Context, repo api.RepoName, dir common.GitDir, revspec string) ([]byte, error) { +func (s *vcsPackagesSyncer) Fetch(ctx context.Context, repo api.RepoName, dir common.GitDir) ([]byte, error) { source, err := s.getRemoteURLSource(ctx, repo) if err != nil { return nil, errors.Wrap(err, "getting remote URL source") @@ -128,76 +127,9 @@ func (s *vcsPackagesSyncer) Fetch(ctx context.Context, repo api.RepoName, dir co return nil, err } - if revspec != "" { - return nil, s.fetchRevspec(ctx, name, dir, versions, revspec) - } - return nil, s.fetchVersions(ctx, name, dir, versions) } -// fetchRevspec fetches the given revspec if it's not contained in -// existingVersions. If download and upserting the new version into database -// succeeds, it calls s.fetchVersions with the newly-added version and the old -// ones, to possibly update the "latest" tag. -func (s *vcsPackagesSyncer) fetchRevspec(ctx context.Context, name reposource.PackageName, dir common.GitDir, existingVersions []string, revspec string) error { - // Optionally try to resolve the version of the user-provided revspec (formatted as `"v${VERSION}^0"`). - // This logic lives inside `vcsPackagesSyncer` meaning this repo must be a package repo where all - // the git tags are created by our npm/crates/pypi/maven integrations (no human commits/branches/tags). - // Package repos only create git tags using the format `"v${VERSION}"`. - // - // Unlike other versions, we silently ignore all errors from resolving requestedVersion because it could - // be any random user-provided string, with no guarantee that it's a valid version string that resolves - // to an existing dependency version. - // - // We assume the revspec is formatted as `"v${VERSION}^0"` but it could be any random string or - // a git commit SHA. It should be harmless if the string is invalid, worst case the resolution fails - // and we silently ignore the error. - requestedVersion := strings.TrimSuffix(strings.TrimPrefix(revspec, "v"), "^0") - - for _, existingVersion := range existingVersions { - if existingVersion == requestedVersion { - return nil - } - } - - dep, err := s.source.ParseVersionedPackageFromNameAndVersion(name, requestedVersion) - if err != nil { - // Invalid version. Silently ignore error, see comment above why. - return nil - } - - // if the next check passes, we know that any filters added/updated before this timestamp did not block it - instant := time.Now() - - if allowed, err := s.svc.IsPackageRepoVersionAllowed(ctx, s.scheme, dep.PackageSyntax(), dep.PackageVersion()); !allowed || err != nil { - // if err == nil && !allowed, this will return nil - return errors.Wrap(err, "error checking if package repo version is allowed") - } - - err = s.gitPushDependencyTag(ctx, string(dir), dep) - if err != nil { - // Package could not be downloaded. Silently ignore error, see comment above why. - return nil - } - - if _, _, err = s.svc.InsertPackageRepoRefs(ctx, []dependencies.MinimalPackageRepoRef{ - { - Scheme: dep.Scheme(), - Name: dep.PackageSyntax(), - Versions: []dependencies.MinimalPackageRepoRefVersion{{Version: dep.PackageVersion(), LastCheckedAt: &instant}}, - LastCheckedAt: &instant, - }, - }); err != nil { - // We don't want to ignore when writing to the database failed, since - // we've already downloaded the package successfully. - return err - } - - existingVersions = append(existingVersions, requestedVersion) - - return s.fetchVersions(ctx, name, dir, existingVersions) -} - // fetchVersions checks whether the given versions are all valid version // specifiers, then checks whether they've already been downloaded and, if not, // downloads them. diff --git a/cmd/gitserver/internal/vcssyncer/packages_syncer_test.go b/cmd/gitserver/internal/vcssyncer/packages_syncer_test.go index 83850d3708e..42ed4cbe514 100644 --- a/cmd/gitserver/internal/vcssyncer/packages_syncer_test.go +++ b/cmd/gitserver/internal/vcssyncer/packages_syncer_test.go @@ -66,7 +66,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { depsSource.Add("foo@0.0.1") t.Run("one version from service", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) require.NoError(t, err) s.assertRefs(t, dir, map[string]string{ @@ -89,7 +89,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { oneVersionOneDownload := map[string]int{"foo@0.0.1": 1, "foo@0.0.2": 1} t.Run("two versions, service and config", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) require.NoError(t, err) s.assertRefs(t, dir, allVersionsHaveRefs) @@ -99,7 +99,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { depsSource.Delete("foo@0.0.2") t.Run("cached tag not re-downloaded (404 not found)", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) require.NoError(t, err) // v0.0.2 is still present in the git repo because we didn't send a second download request. @@ -111,7 +111,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { depsSource.download["foo@0.0.1"] = errors.New("401 unauthorized") t.Run("cached tag not re-downloaded (401 unauthorized)", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) // v0.0.1 is still present in the git repo because we didn't send a second download request. require.NoError(t, err) s.assertRefs(t, dir, allVersionsHaveRefs) @@ -126,7 +126,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { } t.Run("service version deleted", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) require.NoError(t, err) s.assertRefs(t, dir, onlyV2Refs) @@ -136,7 +136,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { s.configDeps = []string{} t.Run("all versions deleted", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) require.NoError(t, err) s.assertRefs(t, dir, map[string]string{}) @@ -148,7 +148,7 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { depsService.Add("foo@0.0.2") depsSource.Add("foo@0.0.2") t.Run("error aggregation", func(t *testing.T) { - _, err := s.Fetch(ctx, "", dir, "") + _, err := s.Fetch(ctx, "", dir) require.ErrorContains(t, err, "401 unauthorized") // The foo@0.0.1 tag was not created because of the 401 error. @@ -159,49 +159,9 @@ func TestVcsDependenciesSyncer_Fetch(t *testing.T) { s.assertDownloadCounts(t, depsSource, map[string]int{"foo@0.0.1": 2, "foo@0.0.2": 2}) }) - bothV2andV3Refs := map[string]string{ - // latest branch has been updated to point to 0.0.3 instead of 0.0.2 - "refs/heads/latest": "c93e10f82d5d34341b2836202ebb6b0faa95fa71", - "refs/tags/v0.0.2": "7e2e4506ef1f5cd97187917a67bfb7a310f78687", - "refs/tags/v0.0.2^{}": "6cff53ec57702e8eec10569a3d981dacbaee4ed3", - "refs/tags/v0.0.3": "ba94b95e16bf902e983ead70dc6ee0edd6b03a3b", - "refs/tags/v0.0.3^{}": "c93e10f82d5d34341b2836202ebb6b0faa95fa71", - } - - t.Run("lazy-sync version via revspec", func(t *testing.T) { - // the v0.0.3 tag should be created on-demand through the revspec parameter - // For context, see https://github.com/sourcegraph/sourcegraph/pull/38811 - _, err := s.Fetch(ctx, "", dir, "v0.0.3^0") - require.ErrorContains(t, err, "401 unauthorized") // v0.0.1 is still erroring - require.Equal(t, s.svc.(*fakeDepsService).upsertedDeps, []dependencies.MinimalPackageRepoRef{{ - Scheme: fakeVersionedPackage{}.Scheme(), - Name: "foo", - Versions: []dependencies.MinimalPackageRepoRefVersion{{Version: "0.0.3"}}, - }}) - s.assertRefs(t, dir, bothV2andV3Refs) - // We triggered a single download for v0.0.3 since it was lazily requested. - // We triggered a v0.0.1 download since it's still erroring. - s.assertDownloadCounts(t, depsSource, map[string]int{"foo@0.0.1": 3, "foo@0.0.2": 2, "foo@0.0.3": 1}) - }) - depsSource.download["foo@0.0.4"] = errors.New("0.0.4 not found") s.svc.(*fakeDepsService).upsertedDeps = []dependencies.MinimalPackageRepoRef{} - t.Run("lazy-sync error version via revspec", func(t *testing.T) { - // the v0.0.4 tag cannot be created on-demand because it returns a "0.0.4 not found" error - _, err := s.Fetch(ctx, "", dir, "v0.0.4^0") - require.Nil(t, err) - // // the 0.0.4 error is silently ignored, we only return the error for v0.0.1. - // require.Equal(t, fmt.Sprint(err.Error()), "error pushing dependency {\"foo\" \"0.0.1\"}: 401 unauthorized") - // the 0.0.4 dependency was not stored in the database because the download failed. - require.Equal(t, s.svc.(*fakeDepsService).upsertedDeps, []dependencies.MinimalPackageRepoRef{}) - // git tags are unchanged, v0.0.2 and v0.0.3 are cached. - s.assertRefs(t, dir, bothV2andV3Refs) - // We triggered downloads only for v0.0.4. - // No new downloads were triggered for cached or other errored versions. - s.assertDownloadCounts(t, depsSource, map[string]int{"foo@0.0.1": 3, "foo@0.0.2": 2, "foo@0.0.3": 1, "foo@0.0.4": 1}) - }) - depsSource.download["org.springframework.boot:spring-boot:3.0"] = notFoundError{errors.New("Please contact Josh Long")} t.Run("trying to download non-existent Maven dependency", func(t *testing.T) { diff --git a/cmd/gitserver/internal/vcssyncer/perforce.go b/cmd/gitserver/internal/vcssyncer/perforce.go index c464ace9ebd..ead93e4c294 100644 --- a/cmd/gitserver/internal/vcssyncer/perforce.go +++ b/cmd/gitserver/internal/vcssyncer/perforce.go @@ -208,7 +208,7 @@ func (s *perforceDepotSyncer) buildP4FusionCmd(ctx context.Context, depot, usern } // Fetch tries to fetch updates of a Perforce depot as a Git repository. -func (s *perforceDepotSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir, _ string) ([]byte, error) { +func (s *perforceDepotSyncer) Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir) ([]byte, error) { source, err := s.getRemoteURLSource(ctx, repoName) if err != nil { return nil, errors.Wrap(err, "getting remote URL source") diff --git a/cmd/gitserver/internal/vcssyncer/syncer.go b/cmd/gitserver/internal/vcssyncer/syncer.go index cb699d1fd73..08e8a2ce1d6 100644 --- a/cmd/gitserver/internal/vcssyncer/syncer.go +++ b/cmd/gitserver/internal/vcssyncer/syncer.go @@ -58,17 +58,11 @@ type VCSSyncer interface { // with both LF and CR being valid line terminators. Clone(ctx context.Context, repo api.RepoName, targetDir common.GitDir, tmpPath string, progressWriter io.Writer) error // Fetch tries to fetch updates from the remote to given directory. - // The revspec parameter is optional and specifies that the client is specifically - // interested in fetching the provided revspec (example "v2.3.4^0"). - // For package hosts (vcsPackagesSyncer, npm/pypi/crates.io), the revspec is used - // to lazily fetch package versions. More details at - // https://github.com/sourcegraph/sourcegraph/issues/37921#issuecomment-1184301885 - // Beware that the revspec parameter can be any random user-provided string. // 🚨 SECURITY: // Output returned from this function should NEVER contain sensitive information. // The VCSSyncer implementation is responsible of redacting potentially // sensitive data like secrets. - Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir, revspec string) ([]byte, error) + Fetch(ctx context.Context, repoName api.RepoName, dir common.GitDir) ([]byte, error) } type NewVCSSyncerOpts struct {