Skip to content

Commit fa508ab

Browse files
dmitshurgopherbot
authored andcommitted
internal/task: dynamically find nested modules to tidy during tagging
Fetch the git tree where go.mod is being updated, walk over its content to find nested modules to tidy. This way even if new nested modules are added, they'll be handled without needing additional attention here. The existing test cases continue to pass (i.e., the x/tools/gopls module is still being tidied). It required adding a minimal HTTP(S) git server to FakeGerrit, to act like the real Gerrit server, or rather just enough for gitfs to be able to successfully clone from it. For golang/go#68873. Change-Id: Ic571f2edc6345d25558591e531d58ecce70f1851 Reviewed-on: https://go-review.googlesource.com/c/build/+/617778 Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]>
1 parent 40c0547 commit fa508ab

File tree

3 files changed

+130
-32
lines changed

3 files changed

+130
-32
lines changed

internal/task/fakes.go

Lines changed: 90 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -349,24 +349,105 @@ func (g *FakeGerrit) GerritURL() string {
349349
return g.serverURL
350350
}
351351

352-
func (g *FakeGerrit) serveHTTP(w http.ResponseWriter, r *http.Request) {
353-
parts := strings.Split(r.URL.Path, "/")
354-
if len(parts) != 4 {
352+
func (g *FakeGerrit) serveHTTP(w http.ResponseWriter, req *http.Request) {
353+
switch url := req.URL.String(); {
354+
// Serve a revision tarball (.tar.gz) like Gerrit does.
355+
case strings.HasSuffix(url, ".tar.gz"):
356+
parts := strings.Split(req.URL.Path, "/")
357+
if len(parts) != 4 {
358+
w.WriteHeader(http.StatusNotFound)
359+
return
360+
}
361+
repo, err := g.repo(parts[1])
362+
if err != nil {
363+
w.WriteHeader(http.StatusNotFound)
364+
return
365+
}
366+
rev := strings.TrimSuffix(parts[3], ".tar.gz")
367+
archive, err := repo.dir.RunCommand(req.Context(), "archive", "--format=tgz", rev)
368+
if err != nil {
369+
w.WriteHeader(http.StatusInternalServerError)
370+
return
371+
}
372+
http.ServeContent(w, req, parts[3], time.Now(), bytes.NewReader(archive))
373+
return
374+
375+
// Serve a git repository over HTTP like Gerrit does.
376+
case strings.HasSuffix(url, "/info/refs?service=git-upload-pack"):
377+
parts := strings.Split(url[:len(url)-len("/info/refs?service=git-upload-pack")], "/")
378+
if len(parts) != 2 {
379+
w.WriteHeader(http.StatusNotFound)
380+
return
381+
}
382+
repo, err := g.repo(parts[1])
383+
if err != nil {
384+
w.WriteHeader(http.StatusNotFound)
385+
return
386+
}
387+
g.serveGitInfoRefsUploadPack(w, req, repo)
388+
return
389+
case strings.HasSuffix(url, "/git-upload-pack"):
390+
parts := strings.Split(url[:len(url)-len("/git-upload-pack")], "/")
391+
if len(parts) != 2 {
392+
w.WriteHeader(http.StatusNotFound)
393+
return
394+
}
395+
repo, err := g.repo(parts[1])
396+
if err != nil {
397+
w.WriteHeader(http.StatusNotFound)
398+
return
399+
}
400+
g.serveGitUploadPack(w, req, repo)
401+
return
402+
403+
default:
355404
w.WriteHeader(http.StatusNotFound)
356405
return
357406
}
358-
repo, err := g.repo(parts[1])
407+
}
408+
func (*FakeGerrit) serveGitInfoRefsUploadPack(w http.ResponseWriter, req *http.Request, repo *FakeRepo) {
409+
if req.Method != http.MethodGet {
410+
w.Header().Set("Allow", http.MethodGet)
411+
http.Error(w, "method should be GET", http.StatusMethodNotAllowed)
412+
return
413+
}
414+
cmd := exec.CommandContext(req.Context(), "git", "upload-pack", "--strict", "--advertise-refs", ".")
415+
cmd.Dir = filepath.Join(repo.dir.dir, ".git")
416+
cmd.Env = append(os.Environ(), "GIT_PROTOCOL="+req.Header.Get("Git-Protocol"))
417+
var buf bytes.Buffer
418+
cmd.Stdout = &buf
419+
err := cmd.Run()
359420
if err != nil {
360-
w.WriteHeader(http.StatusNotFound)
421+
http.Error(w, err.Error(), http.StatusInternalServerError)
422+
return
423+
}
424+
w.Header().Set("Content-Type", "application/x-git-upload-pack-advertisement")
425+
io.WriteString(w, "001e# service=git-upload-pack\n0000")
426+
io.Copy(w, &buf)
427+
}
428+
func (*FakeGerrit) serveGitUploadPack(w http.ResponseWriter, req *http.Request, repo *FakeRepo) {
429+
if req.Method != http.MethodPost {
430+
w.Header().Set("Allow", http.MethodPost)
431+
http.Error(w, "method should be POST", http.StatusMethodNotAllowed)
432+
return
433+
}
434+
if req.Header.Get("Content-Type") != "application/x-git-upload-pack-request" {
435+
http.Error(w, "unexpected Content-Type", http.StatusBadRequest)
361436
return
362437
}
363-
rev := strings.TrimSuffix(parts[3], ".tar.gz")
364-
archive, err := repo.dir.RunCommand(r.Context(), "archive", "--format=tgz", rev)
438+
cmd := exec.CommandContext(req.Context(), "git", "upload-pack", "--strict", "--stateless-rpc", ".")
439+
cmd.Dir = filepath.Join(repo.dir.dir, ".git")
440+
cmd.Env = append(os.Environ(), "GIT_PROTOCOL="+req.Header.Get("Git-Protocol"))
441+
cmd.Stdin = req.Body
442+
var buf bytes.Buffer
443+
cmd.Stdout = &buf
444+
err := cmd.Run()
365445
if err != nil {
366-
w.WriteHeader(http.StatusInternalServerError)
446+
http.Error(w, err.Error(), http.StatusInternalServerError)
367447
return
368448
}
369-
http.ServeContent(w, r, parts[3], time.Now(), bytes.NewReader(archive))
449+
w.Header().Set("Content-Type", "application/x-git-upload-pack-result")
450+
io.Copy(w, &buf)
370451
}
371452

372453
func (*FakeGerrit) QueryChanges(_ context.Context, query string) ([]*gerrit.ChangeInfo, error) {

internal/task/gerrit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type GerritClient interface {
5858
}
5959

6060
type RealGerritClient struct {
61-
Gitiles string
61+
Gitiles string // Gitiles server URL, without trailing slash. For example, "https://go.googlesource.com".
6262
Client *gerrit.Client
6363
}
6464

internal/task/tagx.go

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"io/fs"
1313
"net/url"
14+
pathpkg "path"
1415
"reflect"
1516
"regexp"
1617
"strconv"
@@ -19,6 +20,7 @@ import (
1920

2021
buildbucketpb "go.chromium.org/luci/buildbucket/proto"
2122
"golang.org/x/build/gerrit"
23+
"golang.org/x/build/internal/gitfs"
2224
"golang.org/x/build/internal/releasetargets"
2325
"golang.org/x/build/internal/relui/groups"
2426
wf "golang.org/x/build/internal/workflow"
@@ -389,32 +391,47 @@ func (x *TagXReposTasks) UpdateGoMod(ctx *wf.TaskContext, repo TagRepo, deps []T
389391
}
390392
script.WriteString("\n")
391393

392-
// Tidy the root module.
393-
// Also tidy nested modules with a replace directive.
394-
// TODO(go.dev/issue/68873): Dynamically detect and handle nested modules, instead of the fixed list below.
395-
dirs := []string{"."}
396-
switch repo.Name {
397-
case "exp":
398-
dirs = append(dirs, "slog/benchmarks/zap_benchmarks") // A local replace directive as of 2023-09-05.
399-
dirs = append(dirs, "slog/benchmarks/zerolog_benchmarks") // A local replace directive as of 2023-09-05.
400-
case "oscar":
401-
dirs = append(dirs, "internal/devtools", "internal/gaby", "internal/gcp", "internal/syncdb") // Using a checked-in go.work as of 2024-10-03.
402-
case "telemetry":
403-
dirs = append(dirs, "godev") // A local replace directive as of 2023-09-05.
404-
case "tools":
405-
dirs = append(dirs, "gopls") // A local replace directive as of 2023-09-05.
394+
// Tidy the root module and nested modules.
395+
// Look for the nested modules dynamically. See go.dev/issue/68873.
396+
gitRepo, err := gitfs.NewRepo(x.Gerrit.GitilesURL() + "/" + repo.Name)
397+
if err != nil {
398+
return nil, err
399+
}
400+
head, err := gitRepo.Resolve("refs/heads/" + branch)
401+
if err != nil {
402+
return nil, err
403+
}
404+
ctx.Printf("Using commit %q as the branch %q head.", head, branch)
405+
rootFS, err := gitRepo.CloneHash(head)
406+
if err != nil {
407+
return nil, err
406408
}
407409
var outputs []string
408-
for _, dir := range dirs {
409-
dropToolchain := ""
410-
if !repo.HasToolchain {
411-
// Don't introduce a toolchain directive if it wasn't already there.
412-
// TODO(go.dev/issue/68873): Read the nested module's go.mod. For now, re-use decision from the top-level module.
413-
dropToolchain = " && go get toolchain@none"
410+
if err := fs.WalkDir(rootFS, ".", func(path string, d fs.DirEntry, err error) error {
411+
if err != nil {
412+
return err
413+
}
414+
if path != "." && d.IsDir() && (strings.HasPrefix(d.Name(), ".") || strings.HasPrefix(d.Name(), "_") || d.Name() == "testdata") {
415+
// Skip directories that begin with ".", "_", or are named "testdata".
416+
return fs.SkipDir
417+
}
418+
if d.Name() == "go.mod" && !d.IsDir() { // A go.mod file.
419+
dir := pathpkg.Dir(path)
420+
dropToolchain := ""
421+
if !repo.HasToolchain {
422+
// Don't introduce a toolchain directive if it wasn't already there.
423+
// TODO(go.dev/issue/68873): Read the nested module's go.mod. For now, re-use decision from the top-level module.
424+
dropToolchain = " && go get toolchain@none"
425+
}
426+
script.WriteString(fmt.Sprintf("(cd %v && touch go.sum && go mod tidy%s)\n", dir, dropToolchain))
427+
outputs = append(outputs, dir+"/go.mod", dir+"/go.sum")
414428
}
415-
script.WriteString(fmt.Sprintf("(cd %v && touch go.sum && go mod tidy%s)\n", dir, dropToolchain))
416-
outputs = append(outputs, dir+"/go.mod", dir+"/go.sum")
429+
return nil
430+
}); err != nil {
431+
return nil, err
417432
}
433+
434+
// Execute the script to generate updated go.mod/go.sum files.
418435
build, err := x.CloudBuild.RunScript(ctx, script.String(), repo.Name, outputs)
419436
if err != nil {
420437
return nil, err

0 commit comments

Comments
 (0)