Skip to content

Commit 3521090

Browse files
jskeetjskeet
authored
feat(internal/librarian): add -branch flag for cloning and PRs (#1893)
The same flag is used for two purposes (because you'd almost always want them to be the same): - The remote branch to clone (ignored when not using a remote repo) - The remote branch to use as the base for a PR (i.e. the branch you want to merge into) The flag is used for "generate" and "release init". Fixes #812.
1 parent 8d75c78 commit 3521090

File tree

12 files changed

+60
-25
lines changed

12 files changed

+60
-25
lines changed

‎internal/config/config.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ type Config struct {
9393
// APISource is specified with the -api-source flag.
9494
APISource string
9595

96+
// Branch is the remote branch of the language repository to use.
97+
// This is the branch which is cloned when Repo is a URL, and also used
98+
// as the base reference for any pull requests created by the command.
99+
// By default, the branch "main" is used.
100+
Branch string
101+
96102
// Build determines whether to build the generated library, and is only
97103
// used in the generate command.
98104
//

‎internal/github/github.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (c *Client) GetRawContent(ctx context.Context, path, ref string) ([]byte, e
117117
// CreatePullRequest creates a pull request in the remote repo.
118118
// At the moment this requires a single remote to be configured,
119119
// which must have a GitHub HTTPS URL. We assume a base branch of "main".
120-
func (c *Client) CreatePullRequest(ctx context.Context, repo *Repository, remoteBranch, title, body string) (*PullRequestMetadata, error) {
120+
func (c *Client) CreatePullRequest(ctx context.Context, repo *Repository, remoteBranch, baseBranch, title, body string) (*PullRequestMetadata, error) {
121121
if body == "" {
122122
slog.Warn("Provided PR body is empty, setting default.")
123123
body = "Regenerated all changed APIs. See individual commits for details."
@@ -126,7 +126,7 @@ func (c *Client) CreatePullRequest(ctx context.Context, repo *Repository, remote
126126
newPR := &github.NewPullRequest{
127127
Title: &title,
128128
Head: &remoteBranch,
129-
Base: github.Ptr("main"),
129+
Base: &baseBranch,
130130
Body: github.Ptr(body),
131131
MaintainerCanModify: github.Ptr(true),
132132
}

‎internal/github/github_test.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ func TestCreatePullRequest(t *testing.T) {
303303
for _, test := range []struct {
304304
name string
305305
remoteBranch string
306+
remoteBase string
306307
title string
307308
body string
308309
handler http.HandlerFunc
@@ -313,6 +314,7 @@ func TestCreatePullRequest(t *testing.T) {
313314
{
314315
name: "Success with provided body",
315316
remoteBranch: "feature-branch",
317+
remoteBase: "base-branch",
316318
title: "New Feature",
317319
body: "This is a new feature.",
318320
handler: func(w http.ResponseWriter, r *http.Request) {
@@ -335,8 +337,8 @@ func TestCreatePullRequest(t *testing.T) {
335337
if *newPR.Head != "feature-branch" {
336338
t.Errorf("unexpected head: got %q, want %q", *newPR.Head, "feature-branch")
337339
}
338-
if *newPR.Base != "main" {
339-
t.Errorf("unexpected base: got %q, want %q", *newPR.Base, "main")
340+
if *newPR.Base != "base-branch" {
341+
t.Errorf("unexpected base: got %q, want %q", *newPR.Base, "base-branch")
340342
}
341343
fmt.Fprint(w, `{"number": 1, "html_url": "https://github.com/owner/repo/pull/1"}`)
342344
},
@@ -345,6 +347,7 @@ func TestCreatePullRequest(t *testing.T) {
345347
{
346348
name: "Success with empty body",
347349
remoteBranch: "another-branch",
350+
remoteBase: "main",
348351
title: "Another PR",
349352
body: "",
350353
handler: func(w http.ResponseWriter, r *http.Request) {
@@ -363,6 +366,7 @@ func TestCreatePullRequest(t *testing.T) {
363366
{
364367
name: "GitHub API error",
365368
remoteBranch: "error-branch",
369+
remoteBase: "main",
366370
title: "Error PR",
367371
body: "This will fail.",
368372
handler: func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) },
@@ -382,7 +386,7 @@ func TestCreatePullRequest(t *testing.T) {
382386
}
383387
client.BaseURL, _ = url.Parse(server.URL + "/")
384388

385-
metadata, err := client.CreatePullRequest(context.Background(), repo, test.remoteBranch, test.title, test.body)
389+
metadata, err := client.CreatePullRequest(context.Background(), repo, test.remoteBranch, test.remoteBase, test.title, test.body)
386390

387391
if test.wantErr {
388392
if err == nil {

‎internal/gitrepo/gitrepo.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,12 @@ type RepositoryOptions struct {
6262
// Dir is the directory where the repository will reside locally. Required.
6363
Dir string
6464
// MaybeClone will try to clone the repository if it does not exist locally.
65-
// If set to true, RemoteURL must also be set. Optional.
65+
// If set to true, RemoteURL and RemoteBranch must also be set. Optional.
6666
MaybeClone bool
67-
// RemoteURL is the URL of the remote repository to clone from. Required if Clone is not CloneOptionNone.
67+
// RemoteURL is the URL of the remote repository to clone from. Required if MaybeClone is set to true.
6868
RemoteURL string
69+
// RemoteBranch is the remote branch to clone. Required if MaybeClone is set to true.
70+
RemoteBranch string
6971
// CI is the type of Continuous Integration (CI) environment in which
7072
// the tool is executing.
7173
CI string
@@ -105,8 +107,11 @@ func newRepositoryWithoutUser(opts *RepositoryOptions) (*LocalRepository, error)
105107
if opts.RemoteURL == "" {
106108
return nil, fmt.Errorf("gitrepo: remote URL is required when cloning")
107109
}
110+
if opts.RemoteBranch == "" {
111+
return nil, fmt.Errorf("gitrepo: remote branch is required when cloning")
112+
}
108113
slog.Info("Repository not found, executing clone")
109-
return clone(opts.Dir, opts.RemoteURL, opts.CI)
114+
return clone(opts.Dir, opts.RemoteURL, opts.RemoteBranch, opts.CI)
110115
}
111116
return nil, fmt.Errorf("failed to check for repository at %q: %w", opts.Dir, err)
112117
}
@@ -124,11 +129,11 @@ func open(dir string) (*LocalRepository, error) {
124129
}, nil
125130
}
126131

127-
func clone(dir, url, ci string) (*LocalRepository, error) {
132+
func clone(dir, url, branch, ci string) (*LocalRepository, error) {
128133
slog.Info("Cloning repository", "url", url, "dir", dir)
129134
options := &git.CloneOptions{
130135
URL: url,
131-
ReferenceName: plumbing.HEAD,
136+
ReferenceName: plumbing.NewBranchReferenceName(branch),
132137
SingleBranch: true,
133138
Tags: git.AllTags,
134139
// .NET uses submodules for conformance tests.

‎internal/gitrepo/gitrepo_test.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,10 @@ func TestNewRepository(t *testing.T) {
9090
{
9191
name: "clone maybe",
9292
opts: &RepositoryOptions{
93-
Dir: filepath.Join(tmpDir, "clone-maybe"),
94-
MaybeClone: true,
95-
RemoteURL: remoteDir,
93+
Dir: filepath.Join(tmpDir, "clone-maybe"),
94+
MaybeClone: true,
95+
RemoteURL: remoteDir,
96+
RemoteBranch: "master",
9697
},
9798
wantDir: filepath.Join(tmpDir, "clone-maybe"),
9899
},
@@ -107,9 +108,19 @@ func TestNewRepository(t *testing.T) {
107108
},
108109
{
109110
name: "clone maybe no remote url",
111+
opts: &RepositoryOptions{
112+
Dir: filepath.Join(tmpDir, "clone-maybe-no-remote"),
113+
MaybeClone: true,
114+
RemoteBranch: "main",
115+
},
116+
wantErr: true,
117+
},
118+
{
119+
name: "clone maybe no remote branch",
110120
opts: &RepositoryOptions{
111121
Dir: filepath.Join(tmpDir, "clone-maybe-no-remote"),
112122
MaybeClone: true,
123+
RemoteURL: remoteDir,
113124
},
114125
wantErr: true,
115126
},

‎internal/librarian/command.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,22 @@ type commandRunner struct {
5050
image string
5151
}
5252

53+
const defaultAPISourceBranch = "master"
54+
5355
func newCommandRunner(cfg *config.Config) (*commandRunner, error) {
5456
if cfg.APISource == "" {
5557
cfg.APISource = "https://github.com/googleapis/googleapis"
5658
}
5759

58-
languageRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.Repo, cfg.CI, cfg.GitHubToken)
60+
languageRepo, err := cloneOrOpenRepo(cfg.WorkRoot, cfg.Repo, cfg.Branch, cfg.CI, cfg.GitHubToken)
5961
if err != nil {
6062
return nil, err
6163
}
6264

6365
var sourceRepo gitrepo.Repository
6466
var sourceRepoDir string
6567
if cfg.CommandName == generateCmdName {
66-
sourceRepo, err = cloneOrOpenRepo(cfg.WorkRoot, cfg.APISource, cfg.CI, cfg.GitHubToken)
68+
sourceRepo, err = cloneOrOpenRepo(cfg.WorkRoot, cfg.APISource, defaultAPISourceBranch, cfg.CI, cfg.GitHubToken)
6769
if err != nil {
6870
return nil, err
6971
}
@@ -115,7 +117,7 @@ func newCommandRunner(cfg *config.Config) (*commandRunner, error) {
115117
}, nil
116118
}
117119

118-
func cloneOrOpenRepo(workRoot, repo, ci string, gitPassword string) (*gitrepo.LocalRepository, error) {
120+
func cloneOrOpenRepo(workRoot, repo, branch, ci string, gitPassword string) (*gitrepo.LocalRepository, error) {
119121
if repo == "" {
120122
return nil, errors.New("repo must be specified")
121123
}
@@ -127,11 +129,12 @@ func cloneOrOpenRepo(workRoot, repo, ci string, gitPassword string) (*gitrepo.Lo
127129
repoName := path.Base(strings.TrimSuffix(repo, "/"))
128130
repoPath := filepath.Join(workRoot, repoName)
129131
return gitrepo.NewRepository(&gitrepo.RepositoryOptions{
130-
Dir: repoPath,
131-
MaybeClone: true,
132-
RemoteURL: repo,
133-
CI: ci,
134-
GitPassword: gitPassword,
132+
Dir: repoPath,
133+
MaybeClone: true,
134+
RemoteURL: repo,
135+
RemoteBranch: branch,
136+
CI: ci,
137+
GitPassword: gitPassword,
135138
})
136139
}
137140
// repo is a directory
@@ -364,7 +367,7 @@ func commitAndPush(ctx context.Context, cfg *config.Config, repo gitrepo.Reposit
364367
titlePrefix := "Librarian pull request"
365368
title := fmt.Sprintf("%s: %s", titlePrefix, datetimeNow)
366369
slog.Info("Creating pull request", slog.String("branch", branch), slog.String("title", title))
367-
if _, err = ghClient.CreatePullRequest(ctx, gitHubRepo, branch, title, commitMessage); err != nil {
370+
if _, err = ghClient.CreatePullRequest(ctx, gitHubRepo, branch, cfg.Branch, title, commitMessage); err != nil {
368371
return fmt.Errorf("failed to create pull request: %w", err)
369372
}
370373
return nil

‎internal/librarian/command_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func TestCloneOrOpenLanguageRepo(t *testing.T) {
256256
}
257257
}()
258258

259-
repo, err := cloneOrOpenRepo(workRoot, test.repo, test.ci, "")
259+
repo, err := cloneOrOpenRepo(workRoot, test.repo, test.ci, "main", "")
260260
if test.wantErr {
261261
if err == nil {
262262
t.Error("cloneOrOpenLanguageRepo() expected an error but got nil")

‎internal/librarian/flags.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ func addFlagRepo(fs *flag.FlagSet, cfg *config.Config) {
7171
directory is configured as a language repository.`)
7272
}
7373

74+
func addFlagBranch(fs *flag.FlagSet, cfg *config.Config) {
75+
fs.StringVar(&cfg.Branch, "branch", "main", "remote branch to use with the code repository for cloning and pull requests.")
76+
}
77+
7478
func addFlagWorkRoot(fs *flag.FlagSet, cfg *config.Config) {
7579
fs.StringVar(&cfg.WorkRoot, "output", "", "Working directory root. When this is not specified, a working directory will be created in /tmp.")
7680
}

‎internal/librarian/generate.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ func init() {
8282
addFlagImage(fs, cfg)
8383
addFlagLibrary(fs, cfg)
8484
addFlagRepo(fs, cfg)
85+
addFlagBranch(fs, cfg)
8586
addFlagWorkRoot(fs, cfg)
8687
addFlagPush(fs, cfg)
8788
}

‎internal/librarian/librarian.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func lookupCommand(cmd *cli.Command, args []string) (*cli.Command, []string, err
9696
// GitHubClient is an abstraction over the GitHub client.
9797
type GitHubClient interface {
9898
GetRawContent(ctx context.Context, path, ref string) ([]byte, error)
99-
CreatePullRequest(ctx context.Context, repo *github.Repository, remoteBranch, title, body string) (*github.PullRequestMetadata, error)
99+
CreatePullRequest(ctx context.Context, repo *github.Repository, remoteBranch, remoteBase, title, body string) (*github.PullRequestMetadata, error)
100100
AddLabelsToIssue(ctx context.Context, repo *github.Repository, number int, labels []string) error
101101
GetLabels(ctx context.Context, number int) ([]string, error)
102102
ReplaceLabels(ctx context.Context, number int, labels []string) error

0 commit comments

Comments
 (0)