From 4a7f2e57b0c58fd2c0af5ad101d54189eb3ce805 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 13:57:05 +0100 Subject: [PATCH 1/8] Support bare repo creation --- pkg/cmd/repo/create/create.go | 8 ++- pkg/cmd/repo/create/create_test.go | 95 ++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 2fc127aba..684ba78ba 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -748,10 +748,12 @@ func isLocalRepo(gitClient *git.Client) (bool, error) { return false, projectDirErr } } - if projectDir != ".git" { - return false, nil + + if projectDir == ".git" || projectDir == "." { + return true, nil } - return true, nil + + return false, nil } // clone the checkout branch to specified path diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index cc0ec602a..cef86db9c 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -443,6 +443,68 @@ func Test_createRun(t *testing.T) { }, wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", }, + { + name: "interactive with existing bare repository public", + opts: &CreateOptions{Interactive: true}, + tty: true, + promptStubs: func(p *prompter.PrompterMock) { + p.ConfirmFunc = func(message string, defaultValue bool) (bool, error) { + switch message { + case "Add a remote?": + return false, nil + default: + return false, fmt.Errorf("unexpected confirm prompt: %s", message) + } + } + p.InputFunc = func(message, defaultValue string) (string, error) { + switch message { + case "Path to local repository": + return defaultValue, nil + case "Repository name": + return "REPO", nil + case "Description": + return "my new repo", nil + default: + return "", fmt.Errorf("unexpected input prompt: %s", message) + } + } + p.SelectFunc = func(message, defaultValue string, options []string) (int, error) { + switch message { + case "What would you like to do?": + return prompter.IndexFor(options, "Push an existing local repository to GitHub") + case "Visibility": + return prompter.IndexFor(options, "Private") + default: + return 0, fmt.Errorf("unexpected select prompt: %s", message) + } + } + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"someuser","organizations":{"nodes": []}}}}`)) + reg.Register( + httpmock.GraphQL(`mutation RepositoryCreate\b`), + httpmock.StringResponse(` + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } + } + } + }`)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C . rev-parse --git-dir`, 0, ".") + cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") + }, + wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", + }, { name: "interactive with existing repository public add remote and push", opts: &CreateOptions{Interactive: true}, @@ -696,6 +758,39 @@ func Test_createRun(t *testing.T) { }, wantStdout: "https://github.com/OWNER/REPO\n", }, + { + name: "noninteractive create bare from source", + opts: &CreateOptions{ + Interactive: false, + Source: ".", + Name: "REPO", + Visibility: "PRIVATE", + }, + tty: false, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation RepositoryCreate\b`), + httpmock.StringResponse(` + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } + } + } + }`)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C . rev-parse --git-dir`, 0, ".") + cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") + cs.Register(`git -C . remote add origin https://github.com/OWNER/REPO`, 0, "") + }, + wantStdout: "https://github.com/OWNER/REPO\n", + }, { name: "noninteractive clone from scratch", opts: &CreateOptions{ From bc85e11d05ec2af5e322d2e324405e6d412d1e6a Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 14:11:16 +0100 Subject: [PATCH 2/8] Backfill repo creation failure tests --- pkg/cmd/repo/create/create_test.go | 36 +++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index cef86db9c..490a976ba 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -791,6 +791,36 @@ func Test_createRun(t *testing.T) { }, wantStdout: "https://github.com/OWNER/REPO\n", }, + { + name: "noninteractive create from cwd that isn't a git repo", + opts: &CreateOptions{ + Interactive: false, + Source: ".", + Name: "REPO", + Visibility: "PRIVATE", + }, + tty: false, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C . rev-parse --git-dir`, 128, "") + }, + wantErr: true, + errMsg: "current directory is not a git repository. Run `git init` to initialize it", + }, + { + name: "noninteractive create from cwd that isn't a git repo", + opts: &CreateOptions{ + Interactive: false, + Source: "some-dir", + Name: "REPO", + Visibility: "PRIVATE", + }, + tty: false, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C some-dir rev-parse --git-dir`, 128, "") + }, + wantErr: true, + errMsg: "some-dir is not a git repository. Run `git -C \"some-dir\" init` to initialize it", + }, { name: "noninteractive clone from scratch", opts: &CreateOptions{ @@ -951,11 +981,11 @@ func Test_createRun(t *testing.T) { defer reg.Verify(t) err := createRun(tt.opts) if tt.wantErr { - assert.Error(t, err) - assert.Equal(t, tt.errMsg, err.Error()) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) return } - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, tt.wantStdout, stdout.String()) assert.Equal(t, "", stderr.String()) }) From f515e9c1e7cc0472e3c15e73c55e6e110a204e71 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 14:12:25 +0100 Subject: [PATCH 3/8] Use errWithExitCode interface in repo create isLocalRepo --- pkg/cmd/repo/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 684ba78ba..c3a16956d 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -740,7 +740,7 @@ func hasCommits(gitClient *git.Client) (bool, error) { func isLocalRepo(gitClient *git.Client) (bool, error) { projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { - var execError *exec.ExitError + var execError errWithExitCode if errors.As(projectDirErr, &execError) { if exitCode := int(execError.ExitCode()); exitCode == 128 { return false, nil From 2efb9935db2ba3429559c21a60d19c9504a1751b Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 14:18:26 +0100 Subject: [PATCH 4/8] Doc isLocalRepo and git.Client IsLocalRepo differences --- pkg/cmd/repo/create/create.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index c3a16956d..77961c987 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -736,7 +736,11 @@ func hasCommits(gitClient *git.Client) (bool, error) { return false, nil } -// check if path is the top level directory of a git repo +// Check if path is the top level directory of a git repo +// This is subtly different from the git.Client IsLocalRepo method, +// which returns true if we are _anywhere_ inside of a git repository. +// I'm not sure whether this distinction really matters for repo create, +// but I'm not confident enough right now to make that change. func isLocalRepo(gitClient *git.Client) (bool, error) { projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { From 6a97dbfadf0322cd024091efc8fc96a2a71ce23f Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 15:34:43 +0100 Subject: [PATCH 5/8] Add acceptance test for bare repo create --- .../testdata/repo/repo-create-bare.txtar | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 acceptance/testdata/repo/repo-create-bare.txtar diff --git a/acceptance/testdata/repo/repo-create-bare.txtar b/acceptance/testdata/repo/repo-create-bare.txtar new file mode 100644 index 000000000..b835c420b --- /dev/null +++ b/acceptance/testdata/repo/repo-create-bare.txtar @@ -0,0 +1,35 @@ +# It's unclear what we want to do with these acceptance tests beyond our GHEC discovery, so skip new ones by default +skip + +# Set up env var +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Initialise a local repository with two branches +# We expect a bare repo to have all refs pushed with --mirror +mkdir ${REPO} +cd ${REPO} +exec git init +exec git checkout -b feature-1 +exec git commit --allow-empty -m 'Empty Commit 1' + +exec git checkout -b feature-2 +exec git commit --allow-empty -m 'Empty Commit 2' + +# Clone a bare repo from that local repo +cd .. +exec git clone --bare ${REPO} ${REPO}-bare +cd ${REPO}-bare + +# Create a GitHub repository from that bare repo +exec gh repo create ${ORG}/${REPO} --private --source . --push --remote bare + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Check the remote repo has both branches +exec gh api /repos/${ORG}/${REPO}/branches +stdout 'feature-1' +stdout 'feature-2' From e3665955a5143325f497cdf10c2076d5e76b69c0 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 16:08:50 +0100 Subject: [PATCH 6/8] Push --mirror on bare repo create --- pkg/cmd/repo/create/create.go | 52 +++++++++++++++++++++--------- pkg/cmd/repo/create/create_test.go | 36 +++++++++++++-------- 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 77961c987..ac4ef39c9 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -556,11 +556,11 @@ func createFromLocal(opts *CreateOptions) error { return err } - isRepo, err := isLocalRepo(opts.GitClient) + repoType, err := localRepoType(opts.GitClient) if err != nil { return err } - if !isRepo { + if repoType == unknown { if repoPath == "." { return fmt.Errorf("current directory is not a git repository. Run `git init` to initialize it") } @@ -659,15 +659,31 @@ func createFromLocal(opts *CreateOptions) error { } } - if opts.Push { + if opts.Push && repoType == working { err := opts.GitClient.Push(context.Background(), baseRemote, "HEAD") if err != nil { return err } + if isTTY { fmt.Fprintf(stdout, "%s Pushed commits to %s\n", cs.SuccessIcon(), remoteURL) } } + + if opts.Push && repoType == bare { + cmd, err := opts.GitClient.AuthenticatedCommand(context.Background(), "push", baseRemote, "--mirror") + if err != nil { + return err + } + if err = cmd.Run(); err != nil { + return err + } + + if isTTY { + fmt.Fprintf(stdout, "%s Mirrored all refs to %s\n", cs.SuccessIcon(), remoteURL) + } + } + return nil } @@ -736,28 +752,34 @@ func hasCommits(gitClient *git.Client) (bool, error) { return false, nil } -// Check if path is the top level directory of a git repo -// This is subtly different from the git.Client IsLocalRepo method, -// which returns true if we are _anywhere_ inside of a git repository. -// I'm not sure whether this distinction really matters for repo create, -// but I'm not confident enough right now to make that change. -func isLocalRepo(gitClient *git.Client) (bool, error) { +type repoType int + +const ( + unknown repoType = iota + working + bare +) + +func localRepoType(gitClient *git.Client) (repoType, error) { projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { var execError errWithExitCode if errors.As(projectDirErr, &execError) { if exitCode := int(execError.ExitCode()); exitCode == 128 { - return false, nil + return unknown, nil } - return false, projectDirErr + return unknown, projectDirErr } } - if projectDir == ".git" || projectDir == "." { - return true, nil + switch projectDir { + case ".": + return bare, nil + case ".git": + return working, nil + default: + return unknown, nil } - - return false, nil } // clone the checkout branch to specified path diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 490a976ba..a0f9ac207 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -444,14 +444,16 @@ func Test_createRun(t *testing.T) { wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", }, { - name: "interactive with existing bare repository public", + name: "interactive with existing bare repository public and push", opts: &CreateOptions{Interactive: true}, tty: true, promptStubs: func(p *prompter.PrompterMock) { p.ConfirmFunc = func(message string, defaultValue bool) (bool, error) { switch message { case "Add a remote?": - return false, nil + return true, nil + case `Would you like to push commits from the current branch to "origin"?`: + return true, nil default: return false, fmt.Errorf("unexpected confirm prompt: %s", message) } @@ -464,6 +466,8 @@ func Test_createRun(t *testing.T) { return "REPO", nil case "Description": return "my new repo", nil + case "What should the new remote be called?": + return defaultValue, nil default: return "", fmt.Errorf("unexpected input prompt: %s", message) } @@ -502,8 +506,10 @@ func Test_createRun(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git -C . rev-parse --git-dir`, 0, ".") cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") + cs.Register(`git -C . remote add origin https://github.com/OWNER/REPO`, 0, "") + cs.Register(`git -C . push origin --mirror`, 0, "") }, - wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", + wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n✓ Added remote https://github.com/OWNER/REPO.git\n✓ Mirrored all refs to https://github.com/OWNER/REPO.git\n", }, { name: "interactive with existing repository public add remote and push", @@ -759,10 +765,11 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO\n", }, { - name: "noninteractive create bare from source", + name: "noninteractive create bare from source and push", opts: &CreateOptions{ Interactive: false, Source: ".", + Push: true, Name: "REPO", Visibility: "PRIVATE", }, @@ -771,23 +778,24 @@ func Test_createRun(t *testing.T) { reg.Register( httpmock.GraphQL(`mutation RepositoryCreate\b`), httpmock.StringResponse(` - { - "data": { - "createRepository": { - "repository": { - "id": "REPOID", - "name": "REPO", - "owner": {"login":"OWNER"}, - "url": "https://github.com/OWNER/REPO" + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } } } - } - }`)) + }`)) }, execStubs: func(cs *run.CommandStubber) { cs.Register(`git -C . rev-parse --git-dir`, 0, ".") cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") cs.Register(`git -C . remote add origin https://github.com/OWNER/REPO`, 0, "") + cs.Register(`git -C . push origin --mirror`, 0, "") }, wantStdout: "https://github.com/OWNER/REPO\n", }, From 8e63268aba2c643771c6427c31abce21ad4fe95a Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 16:10:32 +0100 Subject: [PATCH 7/8] Doc push behaviour for bare repo create --- pkg/cmd/repo/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index ac4ef39c9..de99c4bc3 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -94,7 +94,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co To create a remote repository from an existing local repository, specify the source directory with %[1]s--source%[1]s. By default, the remote repository name will be the name of the source directory. - Pass %[1]s--push%[1]s to push any local commits to the new repository. + Pass %[1]s--push%[1]s to push any local commits to the new repository. If the repo is bare, this will mirror all refs. For language or platform .gitignore templates to use with %[1]s--gitignore%[1]s, . From 7bcb0633914e635692fe7a1e31d96d713bf48392 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 16:17:06 +0100 Subject: [PATCH 8/8] Modify push prompt on repo create when bare --- pkg/cmd/repo/create/create.go | 7 ++++++- pkg/cmd/repo/create/create_test.go | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index de99c4bc3..79c349aa4 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -652,8 +652,13 @@ func createFromLocal(opts *CreateOptions) error { // don't prompt for push if there are no commits if opts.Interactive && committed { + msg := fmt.Sprintf("Would you like to push commits from the current branch to %q?", baseRemote) + if repoType == bare { + msg = fmt.Sprintf("Would you like to mirror all refs to %q?", baseRemote) + } + var err error - opts.Push, err = opts.Prompter.Confirm(fmt.Sprintf("Would you like to push commits from the current branch to %q?", baseRemote), true) + opts.Push, err = opts.Prompter.Confirm(msg, true) if err != nil { return err } diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index a0f9ac207..c33cfdad6 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -452,7 +452,7 @@ func Test_createRun(t *testing.T) { switch message { case "Add a remote?": return true, nil - case `Would you like to push commits from the current branch to "origin"?`: + case `Would you like to mirror all refs to "origin"?`: return true, nil default: return false, fmt.Errorf("unexpected confirm prompt: %s", message)