diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index d339a0685..d48b17ae2 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -36,7 +36,7 @@ Run the new binary as: Run tests with: `go test ./...` -See [project layout documentation](../project-layout.md) for information on where to find specific source files. +See [project layout documentation](../docs/project-layout.md) for information on where to find specific source files. ## Submitting a pull request diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index c5d8011d7..d1f8e69f2 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -23,7 +23,7 @@ jobs: - name: Run GoReleaser uses: goreleaser/goreleaser-action@v2 with: - version: latest + version: v0.172.1 # pinning to prevent breaking on latest args: release --release-notes=CHANGELOG.md env: GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} @@ -197,3 +197,18 @@ jobs: GIT_AUTHOR_NAME: cli automation GIT_COMMITTER_EMAIL: noreply@github.com GIT_AUTHOR_EMAIL: noreply@github.com + - name: Bump Winget manifest + shell: pwsh + env: + WINGETCREATE_VERSION: v0.2.0.29-preview + GITHUB_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} + run: | + $tagname = $env:GITHUB_REF.Replace("refs/tags/", "") + $version = $tagname.Replace("v", "") + $url = "https://github.com/cli/cli/releases/download/${tagname}/gh_${version}_windows_amd64.msi" + iwr https://github.com/microsoft/winget-create/releases/download/${env:WINGETCREATE_VERSION}/wingetcreate.exe -OutFile wingetcreate.exe + + .\wingetcreate.exe update GitHub.cli --url $url --version $version + if ($version -notmatch "-") { + .\wingetcreate.exe submit .\manifests\g\GitHub\cli\${version}\ --token $env:GITHUB_TOKEN + } diff --git a/.goreleaser.yml b/.goreleaser.yml index 2f6d16f32..137981363 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -64,5 +64,6 @@ nfpms: formats: - deb - rpm - files: - "./share/man/man1/gh*.1": "/usr/share/man/man1" + contents: + - src: "/share/man/man1/gh*.1" + dst: "/usr/share/man/man1" diff --git a/README.md b/README.md index f212d162f..49d11ff70 100644 --- a/README.md +++ b/README.md @@ -14,13 +14,12 @@ GitHub CLI is available for repositories hosted on GitHub.com and GitHub Enterpr If anything feels off, or if you feel that some functionality is missing, please check out the [contributing page][contributing]. There you will find instructions for sharing your feedback, building the tool locally, and submitting pull requests to the project. - ## Installation ### macOS -`gh` is available via [Homebrew][], [MacPorts][], and as a downloadable binary from the [releases page][]. +`gh` is available via [Homebrew][], [MacPorts][], [Conda][], and as a downloadable binary from the [releases page][]. #### Homebrew @@ -34,24 +33,29 @@ If anything feels off, or if you feel that some functionality is missing, please | ---------------------- | ---------------------------------------------- | | `sudo port install gh` | `sudo port selfupdate && sudo port upgrade gh` | +#### Conda + +| Install: | Upgrade: | +|------------------------------------------|-----------------------------------------| +| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | + +Additional Conda installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). + ### Linux -`gh` is available via [Homebrew](#homebrew), and as downloadable binaries from the [releases page][]. +`gh` is available via [Homebrew](#homebrew), [Conda](#Conda), and as downloadable binaries from the [releases page][]. For more information and distro-specific instructions, see the [Linux installation docs](./docs/install_linux.md). ### Windows -`gh` is available via [WinGet][], [scoop][], [Chocolatey][], and as downloadable MSI. - +`gh` is available via [WinGet][], [scoop][], [Chocolatey][], [Conda](#Conda), and as downloadable MSI. #### WinGet | Install: | Upgrade: | | ------------------- | --------------------| -| `winget install gh` | `winget install gh` | - -WinGet does not have a specialized `upgrade` command yet, but the `install` command should work for upgrading to a newer version of GitHub CLI. +| `winget install gh` | `winget upgrade gh` | #### scoop @@ -69,9 +73,9 @@ For more information and distro-specific instructions, see the [Linux installati MSI installers are available for download on the [releases page][]. -### Github Actions +### GitHub Actions -GitHub CLI comes pre-installed in all [Github-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). +GitHub CLI comes pre-installed in all [GitHub-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). ### Other platforms @@ -88,13 +92,13 @@ what an official GitHub CLI tool can look like with a fundamentally different de tools bring GitHub to the terminal, `hub` behaves as a proxy to `git`, and `gh` is a standalone tool. Check out our [more detailed explanation][gh-vs-hub] to learn more. - [manual]: https://cli.github.com/manual/ [Homebrew]: https://brew.sh [MacPorts]: https://www.macports.org [winget]: https://github.com/microsoft/winget-cli [scoop]: https://scoop.sh [Chocolatey]: https://chocolatey.org +[Conda]: https://docs.conda.io/en/latest/ [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub [contributing]: ./.github/CONTRIBUTING.md diff --git a/api/client.go b/api/client.go index 21cb3febc..0ff48032a 100644 --- a/api/client.go +++ b/api/client.go @@ -220,7 +220,6 @@ func (c Client) REST(hostname string, method string, p string, body io.Reader, d if err != nil { return err } - err = json.Unmarshal(b, &data) if err != nil { return err diff --git a/api/export_pr.go b/api/export_pr.go index 4bd0aabc7..29a5c4a63 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -80,10 +80,20 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { case "reviewRequests": requests := make([]interface{}, 0, len(pr.ReviewRequests.Nodes)) for _, req := range pr.ReviewRequests.Nodes { - if req.RequestedReviewer.TypeName == "" { - continue + r := req.RequestedReviewer + switch r.TypeName { + case "User": + requests = append(requests, map[string]string{ + "__typename": r.TypeName, + "login": r.Login, + }) + case "Team": + requests = append(requests, map[string]string{ + "__typename": r.TypeName, + "name": r.Name, + "slug": r.LoginOrSlug(), + }) } - requests = append(requests, req.RequestedReviewer) } data[f] = &requests default: diff --git a/api/queries_pr.go b/api/queries_pr.go index 8a1d0e421..799776882 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -150,18 +150,33 @@ type PullRequestFile struct { type ReviewRequests struct { Nodes []struct { - RequestedReviewer struct { - TypeName string `json:"__typename"` - Login string `json:"login"` - Name string `json:"name"` - } + RequestedReviewer RequestedReviewer } } +type RequestedReviewer struct { + TypeName string `json:"__typename"` + Login string `json:"login"` + Name string `json:"name"` + Slug string `json:"slug"` + Organization struct { + Login string `json:"login"` + } `json:"organization"` +} + +func (r RequestedReviewer) LoginOrSlug() string { + if r.TypeName == teamTypeName { + return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug) + } + return r.Login +} + +const teamTypeName = "Team" + func (r ReviewRequests) Logins() []string { logins := make([]string, len(r.Nodes)) - for i, a := range r.Nodes { - logins[i] = a.RequestedReviewer.Login + for i, r := range r.Nodes { + logins[i] = r.RequestedReviewer.LoginOrSlug() } return logins } @@ -381,7 +396,7 @@ func PullRequestStatus(client *Client, repo ghrepo.Interface, options StatusOpti // these are always necessary to find the PR for the current branch fields.AddValues([]string{"isCrossRepository", "headRepositoryOwner", "headRefName"}) gr := PullRequestGraphQL(fields.ToSlice()) - fragments = fmt.Sprintf("fragment pr on PullRequest{%[1]s}fragment prWithReviews on PullRequest{%[1]s}", gr) + fragments = fmt.Sprintf("fragment pr on PullRequest{%s}fragment prWithReviews on PullRequest{...pr}", gr) } else { var err error fragments, err = pullRequestFragment(client.http, repo.RepoHost()) @@ -393,8 +408,8 @@ func PullRequestStatus(client *Client, repo ghrepo.Interface, options StatusOpti queryPrefix := ` query PullRequestStatus($owner: String!, $repo: String!, $headRefName: String!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - defaultBranchRef { - name + defaultBranchRef { + name } pullRequests(headRefName: $headRefName, first: $per_page, orderBy: { field: CREATED_AT, direction: DESC }) { totalCount @@ -410,8 +425,8 @@ func PullRequestStatus(client *Client, repo ghrepo.Interface, options StatusOpti queryPrefix = ` query PullRequestStatus($owner: String!, $repo: String!, $number: Int!, $viewerQuery: String!, $reviewerQuery: String!, $per_page: Int = 10) { repository(owner: $owner, name: $repo) { - defaultBranchRef { - name + defaultBranchRef { + name } pullRequest(number: $number) { ...prWithReviews @@ -516,67 +531,26 @@ func pullRequestFragment(httpClient *http.Client, hostname string) (string, erro return "", err } - var reviewsFragment string - if prFeatures.HasReviewDecision { - reviewsFragment = "reviewDecision" + fields := []string{ + "number", "title", "state", "url", "isDraft", "isCrossRepository", + "headRefName", "headRepositoryOwner", "mergeStateStatus", } - - var statusesFragment string if prFeatures.HasStatusCheckRollup { - statusesFragment = ` - commits(last: 1) { - nodes { - commit { - statusCheckRollup { - contexts(last: 100) { - nodes { - ...on StatusContext { - state - } - ...on CheckRun { - conclusion - status - } - } - } - } - } - } - } - ` + fields = append(fields, "statusCheckRollup") + } + if prFeatures.HasBranchProtectionRule { + fields = append(fields, "requiresStrictStatusChecks") } - var requiresStrictStatusChecks string - if prFeatures.HasBranchProtectionRule { - requiresStrictStatusChecks = ` - baseRef { - branchProtectionRule { - requiresStrictStatusChecks - } - }` + var reviewFields []string + if prFeatures.HasReviewDecision { + reviewFields = append(reviewFields, "reviewDecision") } fragments := fmt.Sprintf(` - fragment pr on PullRequest { - number - title - state - url - headRefName - mergeStateStatus - headRepositoryOwner { - login - } - %s - isCrossRepository - isDraft - %s - } - fragment prWithReviews on PullRequest { - ...pr - %s - } - `, requiresStrictStatusChecks, statusesFragment, reviewsFragment) + fragment pr on PullRequest {%s} + fragment prWithReviews on PullRequest {...pr,%s} + `, PullRequestGraphQL(fields), PullRequestGraphQL(reviewFields)) return fragments, nil } diff --git a/api/queries_pr_test.go b/api/queries_pr_test.go index 886f16dd0..537c71268 100644 --- a/api/queries_pr_test.go +++ b/api/queries_pr_test.go @@ -1,12 +1,13 @@ package api import ( - "reflect" + "encoding/json" "testing" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/httpmock" + "github.com/stretchr/testify/assert" ) func TestBranchDeleteRemote(t *testing.T) { @@ -148,13 +149,94 @@ func Test_determinePullRequestFeatures(t *testing.T) { } gotPrFeatures, err := determinePullRequestFeatures(httpClient, tt.hostname) - if (err != nil) != tt.wantErr { - t.Errorf("determinePullRequestFeatures() error = %v, wantErr %v", err, tt.wantErr) + if tt.wantErr { + assert.Error(t, err) return + } else { + assert.NoError(t, err) } - if !reflect.DeepEqual(gotPrFeatures, tt.wantPrFeatures) { - t.Errorf("determinePullRequestFeatures() = %v, want %v", gotPrFeatures, tt.wantPrFeatures) - } + assert.Equal(t, tt.wantPrFeatures, gotPrFeatures) + }) + } +} + +func Test_Logins(t *testing.T) { + rr := ReviewRequests{} + var tests = []struct { + name string + requestedReviews string + want []string + }{ + { + name: "no requested reviewers", + requestedReviews: `{"nodes": []}`, + want: []string{}, + }, + { + name: "user", + requestedReviews: `{"nodes": [ + { + "requestedreviewer": { + "__typename": "User", "login": "testuser" + } + } + ]}`, + want: []string{"testuser"}, + }, + { + name: "team", + requestedReviews: `{"nodes": [ + { + "requestedreviewer": { + "__typename": "Team", + "name": "Test Team", + "slug": "test-team", + "organization": {"login": "myorg"} + } + } + ]}`, + want: []string{"myorg/test-team"}, + }, + { + name: "multiple users and teams", + requestedReviews: `{"nodes": [ + { + "requestedreviewer": { + "__typename": "User", "login": "user1" + } + }, + { + "requestedreviewer": { + "__typename": "User", "login": "user2" + } + }, + { + "requestedreviewer": { + "__typename": "Team", + "name": "Test Team", + "slug": "test-team", + "organization": {"login": "myorg"} + } + }, + { + "requestedreviewer": { + "__typename": "Team", + "name": "Dev Team", + "slug": "dev-team", + "organization": {"login": "myorg"} + } + } + ]}`, + want: []string{"user1", "user2", "myorg/test-team", "myorg/dev-team"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := json.Unmarshal([]byte(tt.requestedReviews), &rr) + assert.NoError(t, err, "Failed to unmarshal json string as ReviewRequests") + logins := rr.Logins() + assert.Equal(t, tt.want, logins) }) } } diff --git a/api/queries_repo.go b/api/queries_repo.go index 5f1056c26..2587fde1c 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "sort" "strings" @@ -189,6 +190,11 @@ type IssueLabel struct { Color string `json:"color"` } +type License struct { + Key string `json:"key"` + Name string `json:"name"` +} + // RepoOwner is the login name of the owner func (r Repository) RepoOwner() string { return r.Owner.Login @@ -434,14 +440,17 @@ func InitRepoHostname(repo *Repository, hostname string) *Repository { return repo } -// repositoryV3 is the repository result from GitHub API v3 +// RepositoryV3 is the repository result from GitHub API v3 type repositoryV3 struct { - NodeID string + NodeID string `json:"node_id"` Name string CreatedAt time.Time `json:"created_at"` Owner struct { Login string } + Private bool + HTMLUrl string `json:"html_url"` + Parent *repositoryV3 } // ForkRepo forks the repository on GitHub and returns the new repository @@ -1109,3 +1118,24 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s } return ProjectsToPaths(projects, projectNames) } + +func CreateRepoTransformToV4(apiClient *Client, hostname string, method string, path string, body io.Reader) (*Repository, error) { + var responsev3 repositoryV3 + err := apiClient.REST(hostname, method, path, body, &responsev3) + + if err != nil { + return nil, err + } + + return &Repository{ + Name: responsev3.Name, + CreatedAt: responsev3.CreatedAt, + Owner: RepositoryOwner{ + Login: responsev3.Owner.Login, + }, + ID: responsev3.NodeID, + hostname: hostname, + URL: responsev3.HTMLUrl, + IsPrivate: responsev3.Private, + }, nil +} diff --git a/api/query_builder.go b/api/query_builder.go index 084e97195..c9ab62d13 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -41,7 +41,11 @@ var prReviewRequests = shortenQuery(` requestedReviewer { __typename, ...on User{login}, - ...on Team{name} + ...on Team{ + organization{login} + name, + slug + } } } } @@ -212,6 +216,8 @@ func PullRequestGraphQL(fields []string) string { q = append(q, `commits(last:1){nodes{commit{oid}}}`) case "commitsCount": // pseudo-field q = append(q, `commits{totalCount}`) + case "requiresStrictStatusChecks": // pseudo-field + q = append(q, `baseRef{branchProtectionRule{requiresStrictStatusChecks}}`) case "statusCheckRollup": q = append(q, StatusCheckRollupGraphQL("")) default: diff --git a/cmd/gh/main.go b/cmd/gh/main.go index b9dc52858..9c5e58555 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -92,14 +92,6 @@ func mainRun() exitCode { return exitError } - if prompt, _ := cfg.Get("", "prompt"); prompt == "disabled" { - cmdFactory.IOStreams.SetNeverPrompt(true) - } - - if pager, _ := cfg.Get("", "pager"); pager != "" { - cmdFactory.IOStreams.SetPager(pager) - } - // TODO: remove after FromFullName has been revisited if host, err := cfg.DefaultHost(); err == nil { ghrepo.SetDefaultHost(host) @@ -140,25 +132,61 @@ func mainRun() exitCode { err = preparedCmd.Run() if err != nil { - if ee, ok := err.(*exec.ExitError); ok { - return exitCode(ee.ExitCode()) + var execError *exec.ExitError + if errors.As(err, &execError) { + return exitCode(execError.ExitCode()) } - fmt.Fprintf(stderr, "failed to run external command: %s", err) return exitError } return exitOK + } else if c, _, err := rootCmd.Traverse(expandedArgs); err == nil && c == rootCmd && len(expandedArgs) > 0 { + extensionManager := cmdFactory.ExtensionManager + if found, err := extensionManager.Dispatch(expandedArgs, os.Stdin, os.Stdout, os.Stderr); err != nil { + var execError *exec.ExitError + if errors.As(err, &execError) { + return exitCode(execError.ExitCode()) + } + fmt.Fprintf(stderr, "failed to run extension: %s", err) + return exitError + } else if found { + return exitOK + } } } + // provide completions for aliases and extensions + rootCmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + var results []string + if aliases, err := cfg.Aliases(); err == nil { + for aliasName := range aliases.All() { + if strings.HasPrefix(aliasName, toComplete) { + results = append(results, aliasName) + } + } + } + for _, ext := range cmdFactory.ExtensionManager.List() { + if strings.HasPrefix(ext.Name(), toComplete) { + results = append(results, ext.Name()) + } + } + return results, cobra.ShellCompDirectiveNoFileComp + } + cs := cmdFactory.IOStreams.ColorScheme() - if cmd != nil && cmdutil.IsAuthCheckEnabled(cmd) && !cmdutil.CheckAuth(cfg) { - fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) - fmt.Fprintln(stderr) - fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") - return exitAuth + authError := errors.New("authError") + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + // require that the user is authenticated before running most commands + if cmdutil.IsAuthCheckEnabled(cmd) && !cmdutil.CheckAuth(cfg) { + fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) + fmt.Fprintln(stderr) + fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + return authError + } + + return nil } rootCmd.SetArgs(expandedArgs) @@ -172,6 +200,8 @@ func mainRun() exitCode { fmt.Fprint(stderr, "\n") } return exitCancel + } else if errors.Is(err, authError) { + return exitAuth } printError(stderr, err, cmd, hasDebug) @@ -264,7 +294,7 @@ func checkForUpdate(currentVersion string) (*update.ReleaseInfo, error) { } repo := updaterEnabled - stateFilePath := filepath.Join(config.ConfigDir(), "state.yml") + stateFilePath := filepath.Join(config.StateDir(), "state.yml") return update.CheckForUpdate(client, stateFilePath, repo, currentVersion) } diff --git a/docs/project-layout.md b/docs/project-layout.md index cf594a2e7..cf9758b46 100644 --- a/docs/project-layout.md +++ b/docs/project-layout.md @@ -75,7 +75,7 @@ and talk through which code gets run in order. This task might be tricky. Typically, gh commands do things like look up information from the git repository in the current directory, query the GitHub API, scan the user's `~/.ssh/config` file, clone or fetch git repositories, etc. Naturally, none of these things should ever happen for real when running tests, unless -you are sure that any filesystem operations are stricly scoped to a location made for and maintained by the +you are sure that any filesystem operations are strictly scoped to a location made for and maintained by the test itself. To avoid actually running things like making real API requests or shelling out to `git` commands, we stub them. You should look at how that's done within some existing tests. diff --git a/go.mod b/go.mod index 80b4b523a..af8db4b26 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/cli/cli go 1.13 require ( - github.com/AlecAivazis/survey/v2 v2.2.9 + github.com/AlecAivazis/survey/v2 v2.2.14 github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.11.1 github.com/charmbracelet/glamour v0.3.0 @@ -11,15 +11,16 @@ require ( github.com/cli/oauth v0.8.0 github.com/cli/safeexec v1.0.0 github.com/cpuguy83/go-md2man/v2 v2.0.0 + github.com/creack/pty v1.1.13 github.com/gabriel-vasile/mimetype v1.1.2 - github.com/google/go-cmp v0.5.2 + github.com/google/go-cmp v0.5.4 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/hashicorp/go-version v1.2.1 github.com/henvic/httpretty v0.0.6 - github.com/itchyny/gojq v0.12.1 + github.com/itchyny/gojq v0.12.4 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 github.com/mattn/go-colorable v0.1.8 - github.com/mattn/go-isatty v0.0.12 + github.com/mattn/go-isatty v0.0.13 github.com/mattn/go-runewidth v0.0.10 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d github.com/mitchellh/go-homedir v1.1.0 @@ -32,8 +33,8 @@ require ( github.com/stretchr/testify v1.6.1 golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 golang.org/x/sync v0.0.0-20190423024810-112230192c58 - golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44 - golang.org/x/term v0.0.0-20210422114643-f5beecf764ed + golang.org/x/sys v0.0.0-20210601080250-7ecdf8ef093b + golang.org/x/term v0.0.0-20210503060354-a79de5458b56 golang.org/x/text v0.3.4 // indirect gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b ) diff --git a/go.sum b/go.sum index 7f89f8dd8..534c4bfc7 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ cloud.google.com/go/firestore v1.1.0/go.mod h1:ulACoGHTpvq5r8rxGJ4ddJZBZqakUQqCl cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I= cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -github.com/AlecAivazis/survey/v2 v2.2.9 h1:LWvJtUswz/W9/zVVXELrmlvdwWcKE60ZAw0FWV9vssk= -github.com/AlecAivazis/survey/v2 v2.2.9/go.mod h1:9DYvHgXtiXm6nCn+jXnOXLKbH+Yo9u8fAS/SduGdoPk= +github.com/AlecAivazis/survey/v2 v2.2.14 h1:aTYTaCh1KLd+YWilkeJ65Ph78g48NVQ3ay9xmaNIyhk= +github.com/AlecAivazis/survey/v2 v2.2.14/go.mod h1:TH2kPCDU3Kqq7pLbnCWwZXDBjnhZtmsCle5EiYDJ2fg= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= @@ -62,6 +62,8 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng4PGlyM= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= +github.com/creack/pty v1.1.13 h1:rTPnd/xocYRjutMfqide2zle1u96upp1gm6eUHKi7us= +github.com/creack/pty v1.1.13/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964 h1:y5HC9v93H5EPKqaS1UYVg1uYah5Xf51mBfIoWehClUQ= github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964/go.mod h1:Xd9hchkHSWYkEqJwUGisez3G1QY8Ryz0sdWrLPMGjLk= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -97,8 +99,8 @@ github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Z github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= -github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= -github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= +github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= github.com/google/pprof v0.0.0-20190515194954-54271f7e092f/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc= @@ -143,13 +145,11 @@ github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174 h1:WlZsjVhE8Af9IcZDG github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW1W22yXSB90986pcOyQ7r45iio1KN2ez1A= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= -github.com/itchyny/astgen-go v0.0.0-20210113000433-0da0671862a3 h1:l7vogWrq+zj8v5t/G69/eT13nAGs2H7cq+CI2nlnKdk= -github.com/itchyny/astgen-go v0.0.0-20210113000433-0da0671862a3/go.mod h1:296z3W7Xsrp2mlIY88ruDKscuvrkL6zXCNRtaYVshzw= github.com/itchyny/go-flags v1.5.0/go.mod h1:lenkYuCobuxLBAd/HGFE4LRoW8D3B6iXRQfWYJ+MNbA= -github.com/itchyny/gojq v0.12.1 h1:pQJrG8LXgEbZe9hvpfjKg7UlBfieQQydIw3YQq+7WIA= -github.com/itchyny/gojq v0.12.1/go.mod h1:Y5Lz0qoT54ii+ucY/K3yNDy19qzxZvWNBMBpKUDQR/4= -github.com/itchyny/timefmt-go v0.1.1 h1:rLpnm9xxb39PEEVzO0n4IRp0q6/RmBc7Dy/rE4HrA0U= -github.com/itchyny/timefmt-go v0.1.1/go.mod h1:0osSSCQSASBJMsIZnhAaF1C2fCBTJZXrnj37mG8/c+A= +github.com/itchyny/gojq v0.12.4 h1:8zgOZWMejEWCLjbF/1mWY7hY7QEARm7dtuhC6Bp4R8o= +github.com/itchyny/gojq v0.12.4/go.mod h1:EQUSKgW/YaOxmXpAwGiowFDO4i2Rmtk5+9dFyeiymAg= +github.com/itchyny/timefmt-go v0.1.3 h1:7M3LGVDsqcd0VZH2U+x393obrzZisp7C0uEe921iRkU= +github.com/itchyny/timefmt-go v0.1.3/go.mod h1:0osSSCQSASBJMsIZnhAaF1C2fCBTJZXrnj37mG8/c+A= github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= @@ -178,8 +178,9 @@ github.com/mattn/go-colorable v0.1.8 h1:c1ghPdyEDarC70ftn0y+A/Ee++9zz8ljHG1b13eJ github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= -github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= +github.com/mattn/go-isatty v0.0.13 h1:qdl+GuBjcsKKDco5BsxPJlId98mSWNKqYA+Co0SC1yA= +github.com/mattn/go-isatty v0.0.13/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= github.com/mattn/go-runewidth v0.0.10 h1:CoZ3S2P7pvtP45xOtBw+/mDL2z0RKI576gSkzRRpdGg= github.com/mattn/go-runewidth v0.0.10/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= @@ -344,7 +345,6 @@ golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20190530182044-ad28b68e88f1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190606165138-5da285871e9c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -353,13 +353,13 @@ golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200413165638-669c56c373c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210113181707-4bcb84eeeb78/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210319071255-635bc2c9138d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44 h1:Bli41pIlzTzf3KEY06n+xnzK/BESIg2ze4Pgfh/aI8c= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210601080250-7ecdf8ef093b h1:qh4f65QIVFjq9eBURLEYWqaEXmOyqdUyiBSgaXWccWk= +golang.org/x/sys v0.0.0-20210601080250-7ecdf8ef093b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210422114643-f5beecf764ed h1:Ei4bQjjpYUsS4efOUz+5Nz++IVkHk87n2zBA0NxBWc0= -golang.org/x/term v0.0.0-20210422114643-f5beecf764ed/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210503060354-a79de5458b56 h1:b8jxX3zqjpqb2LklXPzKSGJhzyxCOZSz8ncv8Nv+y7w= +golang.org/x/term v0.0.0-20210503060354-a79de5458b56/go.mod h1:tfny5GFUkzUvx4ps4ajbZsCe5lw1metzhBm9T3x7oIY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= diff --git a/internal/config/config_file.go b/internal/config/config_file.go index e3d7d2d52..972f94d2b 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "syscall" "github.com/mitchellh/go-homedir" @@ -13,16 +14,156 @@ import ( ) const ( - GH_CONFIG_DIR = "GH_CONFIG_DIR" + GH_CONFIG_DIR = "GH_CONFIG_DIR" + XDG_CONFIG_HOME = "XDG_CONFIG_HOME" + XDG_STATE_HOME = "XDG_STATE_HOME" + XDG_DATA_HOME = "XDG_DATA_HOME" + APP_DATA = "AppData" + LOCAL_APP_DATA = "LocalAppData" ) +// Config path precedence +// 1. GH_CONFIG_DIR +// 2. XDG_CONFIG_HOME +// 3. AppData (windows only) +// 4. HOME func ConfigDir() string { - if v := os.Getenv(GH_CONFIG_DIR); v != "" { - return v + var path string + if a := os.Getenv(GH_CONFIG_DIR); a != "" { + path = a + } else if b := os.Getenv(XDG_CONFIG_HOME); b != "" { + path = filepath.Join(b, "gh") + } else if c := os.Getenv(APP_DATA); runtime.GOOS == "windows" && c != "" { + path = filepath.Join(c, "GitHub CLI") + } else { + d, _ := os.UserHomeDir() + path = filepath.Join(d, ".config", "gh") } - homeDir, _ := homeDirAutoMigrate() - return homeDir + // If the path does not exist and the GH_CONFIG_DIR flag is not set try + // migrating config from default paths. + if !dirExists(path) && os.Getenv(GH_CONFIG_DIR) == "" { + _ = autoMigrateConfigDir(path) + } + + return path +} + +// State path precedence +// 1. XDG_CONFIG_HOME +// 2. LocalAppData (windows only) +// 3. HOME +func StateDir() string { + var path string + if a := os.Getenv(XDG_STATE_HOME); a != "" { + path = filepath.Join(a, "gh") + } else if b := os.Getenv(LOCAL_APP_DATA); runtime.GOOS == "windows" && b != "" { + path = filepath.Join(b, "GitHub CLI") + } else { + c, _ := os.UserHomeDir() + path = filepath.Join(c, ".local", "state", "gh") + } + + // If the path does not exist try migrating state from default paths + if !dirExists(path) { + _ = autoMigrateStateDir(path) + } + + return path +} + +// Data path precedence +// 1. XDG_DATA_HOME +// 2. LocalAppData (windows only) +// 3. HOME +func DataDir() string { + var path string + if a := os.Getenv(XDG_DATA_HOME); a != "" { + path = filepath.Join(a, "gh") + } else if b := os.Getenv(LOCAL_APP_DATA); runtime.GOOS == "windows" && b != "" { + path = filepath.Join(b, "GitHub CLI") + } else { + c, _ := os.UserHomeDir() + path = filepath.Join(c, ".local", "share", "gh") + } + + return path +} + +var errSamePath = errors.New("same path") +var errNotExist = errors.New("not exist") + +// Check default paths (os.UserHomeDir, and homedir.Dir) for existing configs +// If configs exist then move them to newPath +// TODO: Remove support for homedir.Dir location in v2 +func autoMigrateConfigDir(newPath string) error { + path, err := os.UserHomeDir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateDir(oldPath, newPath) + } + + path, err = homedir.Dir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateDir(oldPath, newPath) + } + + return errNotExist +} + +// Check default paths (os.UserHomeDir, and homedir.Dir) for existing state file (state.yml) +// If state file exist then move it to newPath +// TODO: Remove support for homedir.Dir location in v2 +func autoMigrateStateDir(newPath string) error { + path, err := os.UserHomeDir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateFile(oldPath, newPath, "state.yml") + } + + path, err = homedir.Dir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateFile(oldPath, newPath, "state.yml") + } + + return errNotExist +} + +func migrateFile(oldPath, newPath, file string) error { + if oldPath == newPath { + return errSamePath + } + + oldFile := filepath.Join(oldPath, file) + newFile := filepath.Join(newPath, file) + + if !fileExists(oldFile) { + return errNotExist + } + + _ = os.MkdirAll(filepath.Dir(newFile), 0755) + return os.Rename(oldFile, newFile) +} + +func migrateDir(oldPath, newPath string) error { + if oldPath == newPath { + return errSamePath + } + + if !dirExists(oldPath) { + return errNotExist + } + + _ = os.MkdirAll(filepath.Dir(newPath), 0755) + return os.Rename(oldPath, newPath) +} + +func dirExists(path string) bool { + f, err := os.Stat(path) + return err == nil && f.IsDir() +} + +func fileExists(path string) bool { + f, err := os.Stat(path) + return err == nil && !f.IsDir() } func ConfigFile() string { @@ -63,36 +204,6 @@ func HomeDirPath(subdir string) (string, error) { return newPath, nil } -// Looks up the `~/.config/gh` directory with backwards-compatibility with go-homedir and auto-migration -// when an old homedir location was found. -func homeDirAutoMigrate() (string, error) { - homeDir, err := os.UserHomeDir() - if err != nil { - // TODO: remove go-homedir fallback in GitHub CLI v2 - if legacyDir, err := homedir.Dir(); err == nil { - return filepath.Join(legacyDir, ".config", "gh"), nil - } - return "", err - } - - newPath := filepath.Join(homeDir, ".config", "gh") - _, newPathErr := os.Stat(newPath) - if newPathErr == nil || !os.IsNotExist(err) { - return newPath, newPathErr - } - - // TODO: remove go-homedir fallback in GitHub CLI v2 - if legacyDir, err := homedir.Dir(); err == nil { - legacyPath := filepath.Join(legacyDir, ".config", "gh") - if s, err := os.Stat(legacyPath); err == nil && s.IsDir() { - _ = os.MkdirAll(filepath.Dir(newPath), 0755) - return newPath, os.Rename(legacyPath, newPath) - } - } - - return newPath, nil -} - var ReadConfigFile = func(filename string) ([]byte, error) { f, err := os.Open(filename) if err != nil { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index c7343b7ea..4c35f24f9 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -151,27 +152,100 @@ func Test_parseConfigFile(t *testing.T) { } func Test_ConfigDir(t *testing.T) { + tempDir := t.TempDir() + tests := []struct { - envVar string - want string + name string + onlyWindows bool + env map[string]string + output string }{ - {"/tmp/gh", ".tmp.gh"}, - {"", ".config.gh"}, + { + name: "HOME/USERPROFILE specified", + env: map[string]string{ + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "AppData": "", + "USERPROFILE": tempDir, + "HOME": tempDir, + }, + output: filepath.Join(tempDir, ".config", "gh"), + }, + { + name: "GH_CONFIG_DIR specified", + env: map[string]string{ + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + }, + output: filepath.Join(tempDir, "gh_config_dir"), + }, + { + name: "XDG_CONFIG_HOME specified", + env: map[string]string{ + "XDG_CONFIG_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + { + name: "GH_CONFIG_DIR and XDG_CONFIG_HOME specified", + env: map[string]string{ + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "XDG_CONFIG_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh_config_dir"), + }, + { + name: "AppData specified", + onlyWindows: true, + env: map[string]string{ + "AppData": tempDir, + }, + output: filepath.Join(tempDir, "GitHub CLI"), + }, + { + name: "GH_CONFIG_DIR and AppData specified", + onlyWindows: true, + env: map[string]string{ + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "AppData": tempDir, + }, + output: filepath.Join(tempDir, "gh_config_dir"), + }, + { + name: "XDG_CONFIG_HOME and AppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_CONFIG_HOME": tempDir, + "AppData": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, } for _, tt := range tests { - t.Run(fmt.Sprintf("envVar: %q", tt.envVar), func(t *testing.T) { - if tt.envVar != "" { - os.Setenv(GH_CONFIG_DIR, tt.envVar) - defer os.Unsetenv(GH_CONFIG_DIR) + if tt.onlyWindows && runtime.GOOS != "windows" { + continue + } + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } } - assert.Regexp(t, tt.want, ConfigDir()) + + // Create directory to skip auto migration code + // which gets run when target directory does not exist + _ = os.MkdirAll(tt.output, 0755) + + assert.Equal(t, tt.output, ConfigDir()) }) } } func Test_configFile_Write_toDisk(t *testing.T) { configDir := filepath.Join(t.TempDir(), ".config", "gh") + _ = os.MkdirAll(configDir, 0755) os.Setenv(GH_CONFIG_DIR, configDir) defer os.Unsetenv(GH_CONFIG_DIR) @@ -194,3 +268,284 @@ func Test_configFile_Write_toDisk(t *testing.T) { t.Errorf("unexpected hosts.yml: %q", string(configBytes)) } } + +func Test_autoMigrateConfigDir_noMigration_notExist(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := autoMigrateConfigDir(migrateDir) + assert.Equal(t, errNotExist, err) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { + homeDir := t.TempDir() + migrateDir := filepath.Join(homeDir, ".config", "gh") + err := os.MkdirAll(migrateDir, 0755) + assert.NoError(t, err) + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err = autoMigrateConfigDir(migrateDir) + assert.Equal(t, errSamePath, err) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateConfigDir_migration(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + homeConfigDir := filepath.Join(homeDir, ".config", "gh") + migrateConfigDir := filepath.Join(migrateDir, ".config", "gh") + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := os.MkdirAll(homeConfigDir, 0755) + assert.NoError(t, err) + f, err := ioutil.TempFile(homeConfigDir, "") + assert.NoError(t, err) + f.Close() + + err = autoMigrateConfigDir(migrateConfigDir) + assert.NoError(t, err) + + _, err = ioutil.ReadDir(homeConfigDir) + assert.True(t, os.IsNotExist(err)) + + files, err := ioutil.ReadDir(migrateConfigDir) + assert.NoError(t, err) + assert.Equal(t, 1, len(files)) +} + +func Test_StateDir(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + onlyWindows bool + env map[string]string + output string + }{ + { + name: "HOME/USERPROFILE specified", + env: map[string]string{ + "XDG_STATE_HOME": "", + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "LocalAppData": "", + "USERPROFILE": tempDir, + "HOME": tempDir, + }, + output: filepath.Join(tempDir, ".local", "state", "gh"), + }, + { + name: "XDG_STATE_HOME specified", + env: map[string]string{ + "XDG_STATE_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + { + name: "LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "GitHub CLI"), + }, + { + name: "XDG_STATE_HOME and LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_STATE_HOME": tempDir, + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + } + + for _, tt := range tests { + if tt.onlyWindows && runtime.GOOS != "windows" { + continue + } + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } + } + + // Create directory to skip auto migration code + // which gets run when target directory does not exist + _ = os.MkdirAll(tt.output, 0755) + + assert.Equal(t, tt.output, StateDir()) + }) + } +} + +func Test_autoMigrateStateDir_noMigration_notExist(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := autoMigrateStateDir(migrateDir) + assert.Equal(t, errNotExist, err) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateStateDir_noMigration_samePath(t *testing.T) { + homeDir := t.TempDir() + migrateDir := filepath.Join(homeDir, ".config", "gh") + err := os.MkdirAll(migrateDir, 0755) + assert.NoError(t, err) + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err = autoMigrateStateDir(migrateDir) + assert.Equal(t, errSamePath, err) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateStateDir_migration(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + homeConfigDir := filepath.Join(homeDir, ".config", "gh") + migrateStateDir := filepath.Join(migrateDir, ".local", "state", "gh") + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := os.MkdirAll(homeConfigDir, 0755) + assert.NoError(t, err) + err = ioutil.WriteFile(filepath.Join(homeConfigDir, "state.yml"), nil, 0755) + assert.NoError(t, err) + + err = autoMigrateStateDir(migrateStateDir) + assert.NoError(t, err) + + files, err := ioutil.ReadDir(homeConfigDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) + + files, err = ioutil.ReadDir(migrateStateDir) + assert.NoError(t, err) + assert.Equal(t, 1, len(files)) + assert.Equal(t, "state.yml", files[0].Name()) +} + +func Test_DataDir(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + onlyWindows bool + env map[string]string + output string + }{ + { + name: "HOME/USERPROFILE specified", + env: map[string]string{ + "XDG_DATA_HOME": "", + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "LocalAppData": "", + "USERPROFILE": tempDir, + "HOME": tempDir, + }, + output: filepath.Join(tempDir, ".local", "share", "gh"), + }, + { + name: "XDG_DATA_HOME specified", + env: map[string]string{ + "XDG_DATA_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + { + name: "LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "GitHub CLI"), + }, + { + name: "XDG_DATA_HOME and LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_DATA_HOME": tempDir, + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + } + + for _, tt := range tests { + if tt.onlyWindows && runtime.GOOS != "windows" { + continue + } + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } + } + + assert.Equal(t, tt.output, DataDir()) + }) + } +} diff --git a/internal/config/config_map.go b/internal/config/config_map.go index f6b25fcb6..8afaf3a4c 100644 --- a/internal/config/config_map.go +++ b/internal/config/config_map.go @@ -68,13 +68,18 @@ func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) { ce = &ConfigEntry{} - topLevelKeys := cm.Root.Content - for i, v := range topLevelKeys { + // Content slice goes [key1, value1, key2, value2, ...] + topLevelPairs := cm.Root.Content + for i, v := range topLevelPairs { + // Skip every other slice item since we only want to check against keys + if i%2 != 0 { + continue + } if v.Value == key { ce.KeyNode = v ce.Index = i - if i+1 < len(topLevelKeys) { - ce.ValueNode = topLevelKeys[i+1] + if i+1 < len(topLevelPairs) { + ce.ValueNode = topLevelPairs[i+1] } return } diff --git a/internal/config/config_map_test.go b/internal/config/config_map_test.go new file mode 100644 index 000000000..c504e4cbc --- /dev/null +++ b/internal/config/config_map_test.go @@ -0,0 +1,65 @@ +package config + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" +) + +func TestFindEntry(t *testing.T) { + tests := []struct { + name string + key string + output string + wantErr bool + }{ + { + name: "find key", + key: "valid", + output: "present", + }, + { + name: "find key that is not present", + key: "invalid", + wantErr: true, + }, + { + name: "find key with blank value", + key: "blank", + output: "", + }, + { + name: "find key that has same content as a value", + key: "same", + output: "logical", + }, + } + + for _, tt := range tests { + cm := ConfigMap{Root: testYaml()} + t.Run(tt.name, func(t *testing.T) { + out, err := cm.FindEntry(tt.key) + if tt.wantErr { + assert.EqualError(t, err, "not found") + return + } + assert.NoError(t, err) + fmt.Println(out) + assert.Equal(t, tt.output, out.ValueNode.Value) + }) + } +} + +func testYaml() *yaml.Node { + var root yaml.Node + var data = ` +valid: present +erroneous: same +blank: +same: logical +` + _ = yaml.Unmarshal([]byte(data), &root) + return root.Content[0] +} diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 9098a1c08..3714698bd 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -50,6 +50,11 @@ var configOptions = []ConfigOption{ Description: "the terminal pager program to send standard output to", DefaultValue: "", }, + { + Key: "http_unix_socket", + Description: "the path to a unix socket through which to make HTTP connection", + DefaultValue: "", + }, } func ConfigOptions() []ConfigOption { @@ -179,6 +184,15 @@ func NewBlankRoot() *yaml.Node { }, }, }, + { + HeadComment: "The path to a unix socket through which send HTTP connections. If blank, HTTP traffic will be handled by net/http.DefaultTransport.", + Kind: yaml.ScalarNode, + Value: "http_unix_socket", + }, + { + Kind: yaml.ScalarNode, + Value: "", + }, }, }, }, diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index fca819f46..fe5e3f239 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -50,6 +50,8 @@ func Test_defaultConfig(t *testing.T) { # Aliases allow you to create nicknames for gh commands aliases: co: pr checkout + # The path to a unix socket through which send HTTP connections. If blank, HTTP traffic will be handled by net/http.DefaultTransport. + http_unix_socket: `) assert.Equal(t, expected, mainBuf.String()) assert.Equal(t, "", hostsBuf.String()) @@ -81,6 +83,9 @@ func Test_ValidateValue(t *testing.T) { err = ValidateValue("got", "123") assert.NoError(t, err) + + err = ValidateValue("http_unix_socket", "really_anything/is/allowed/and/net.Dial\\(...\\)/will/ultimately/validate") + assert.NoError(t, err) } func Test_ValidateKey(t *testing.T) { @@ -98,4 +103,7 @@ func Test_ValidateKey(t *testing.T) { err = ValidateKey("pager") assert.NoError(t, err) + + err = ValidateKey("http_unix_socket") + assert.NoError(t, err) } diff --git a/internal/config/from_env_test.go b/internal/config/from_env_test.go index a5e03edd3..a3ad8fee4 100644 --- a/internal/config/from_env_test.go +++ b/internal/config/from_env_test.go @@ -13,11 +13,13 @@ func TestInheritEnv(t *testing.T) { orig_GITHUB_ENTERPRISE_TOKEN := os.Getenv("GITHUB_ENTERPRISE_TOKEN") orig_GH_TOKEN := os.Getenv("GH_TOKEN") orig_GH_ENTERPRISE_TOKEN := os.Getenv("GH_ENTERPRISE_TOKEN") + orig_AppData := os.Getenv("AppData") t.Cleanup(func() { os.Setenv("GITHUB_TOKEN", orig_GITHUB_TOKEN) os.Setenv("GITHUB_ENTERPRISE_TOKEN", orig_GITHUB_ENTERPRISE_TOKEN) os.Setenv("GH_TOKEN", orig_GH_TOKEN) os.Setenv("GH_ENTERPRISE_TOKEN", orig_GH_ENTERPRISE_TOKEN) + os.Setenv("AppData", orig_AppData) }) type wants struct { @@ -264,6 +266,7 @@ func TestInheritEnv(t *testing.T) { os.Setenv("GITHUB_ENTERPRISE_TOKEN", tt.GITHUB_ENTERPRISE_TOKEN) os.Setenv("GH_TOKEN", tt.GH_TOKEN) os.Setenv("GH_ENTERPRISE_TOKEN", tt.GH_ENTERPRISE_TOKEN) + os.Setenv("AppData", "") baseCfg := NewFromString(tt.baseConfig) cfg := InheritEnv(baseCfg) diff --git a/internal/httpunix/transport.go b/internal/httpunix/transport.go new file mode 100644 index 000000000..2326a5f91 --- /dev/null +++ b/internal/httpunix/transport.go @@ -0,0 +1,21 @@ +// package httpunix provides an http.RoundTripper which dials a server via a unix socket. +package httpunix + +import ( + "net" + "net/http" +) + +// NewRoundTripper returns an http.RoundTripper which sends requests via a unix +// socket at socketPath. +func NewRoundTripper(socketPath string) http.RoundTripper { + dial := func(network, addr string) (net.Conn, error) { + return net.Dial("unix", socketPath) + } + + return &http.Transport{ + Dial: dial, + DialTLS: dial, + DisableKeepAlives: true, + } +} diff --git a/internal/update/update.go b/internal/update/update.go index 024fd2377..f525544e9 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -3,6 +3,8 @@ package update import ( "fmt" "io/ioutil" + "os" + "path/filepath" "regexp" "strconv" "strings" @@ -83,9 +85,14 @@ func setStateEntry(stateFilePath string, t time.Time, r ReleaseInfo) error { if err != nil { return err } - _ = ioutil.WriteFile(stateFilePath, content, 0600) - return nil + err = os.MkdirAll(filepath.Dir(stateFilePath), 0755) + if err != nil { + return err + } + + err = ioutil.WriteFile(stateFilePath, content, 0600) + return err } func versionGreaterThan(v, w string) bool { diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 356ca1581..7880a9a6c 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -9,39 +9,30 @@ import ( "github.com/spf13/cobra" ) -type ActionsOptions struct { - IO *iostreams.IOStreams -} - func NewCmdActions(f *cmdutil.Factory) *cobra.Command { - opts := ActionsOptions{ - IO: f.IOStreams, - } + cs := f.IOStreams.ColorScheme() cmd := &cobra.Command{ Use: "actions", Short: "Learn about working with GitHub actions", - Long: actionsExplainer(nil), + Long: actionsExplainer(cs), Run: func(cmd *cobra.Command, args []string) { - actionsRun(opts) + fmt.Fprintln(f.IOStreams.Out, actionsExplainer(cs)) }, Annotations: map[string]string{ "IsActions": "true", }, } + cmdutil.DisableAuthCheck(cmd) + return cmd } func actionsExplainer(cs *iostreams.ColorScheme) string { - header := "Welcome to GitHub Actions on the command line." - runHeader := "Interacting with workflow runs" - workflowHeader := "Interacting with workflow files" - if cs != nil { - header = cs.Bold(header) - runHeader = cs.Bold(runHeader) - workflowHeader = cs.Bold(workflowHeader) - } + header := cs.Bold("Welcome to GitHub Actions on the command line.") + runHeader := cs.Bold("Interacting with workflow runs") + workflowHeader := cs.Bold("Interacting with workflow files") return heredoc.Docf(` %s @@ -70,8 +61,3 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { `, header, runHeader, workflowHeader) } - -func actionsRun(opts ActionsOptions) { - cs := opts.IO.ColorScheme() - fmt.Fprintln(opts.IO.Out, actionsExplainer(cs)) -} diff --git a/pkg/cmd/alias/expand/expand.go b/pkg/cmd/alias/expand/expand.go index ec791df52..e48262298 100644 --- a/pkg/cmd/alias/expand/expand.go +++ b/pkg/cmd/alias/expand/expand.go @@ -3,14 +3,13 @@ package expand import ( "errors" "fmt" - "os" - "path/filepath" + "os/exec" "regexp" "runtime" "strings" "github.com/cli/cli/internal/config" - "github.com/cli/safeexec" + "github.com/cli/cli/pkg/findsh" "github.com/google/shlex" ) @@ -80,27 +79,15 @@ func ExpandAlias(cfg config.Config, args []string, findShFunc func() (string, er } func findSh() (string, error) { - shPath, err := safeexec.LookPath("sh") - if err == nil { - return shPath, nil - } - - if runtime.GOOS == "windows" { - winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") - // We can try and find a sh executable in a Git for Windows install - gitPath, err := safeexec.LookPath("git") - if err != nil { - return "", winNotFoundErr + shPath, err := findsh.Find() + if err != nil { + if errors.Is(err, exec.ErrNotFound) { + if runtime.GOOS == "windows" { + return "", errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") + } + return "", errors.New("unable to locate sh to execute shell alias with") } - - shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe") - _, err = os.Stat(shPath) - if err != nil { - return "", winNotFoundErr - } - - return shPath, nil + return "", err } - - return "", errors.New("unable to locate sh to execute shell alias with") + return shPath, nil } diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index e7c4ea6d5..9975b9173 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -20,7 +20,8 @@ type SetOptions struct { Name string Expansion string IsShell bool - RootCmd *cobra.Command + + validCommand func(string) bool } func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { @@ -33,56 +34,62 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command Use: "set ", Short: "Create a shortcut for a gh command", Long: heredoc.Doc(` - Declare a word as a command alias that will expand to the specified command(s). + Define a word that will expand to a full gh command when invoked. - The expansion may specify additional arguments and flags. If the expansion - includes positional placeholders such as '$1', '$2', etc., any extra arguments - that follow the invocation of an alias will be inserted appropriately. - Reads from STDIN if '-' is specified as the expansion parameter. This can be useful - for commands with mixed quotes or multiple lines. + The expansion may specify additional arguments and flags. If the expansion includes + positional placeholders such as "$1", extra arguments that follow the alias will be + inserted appropriately. Otherwise, extra arguments will be appended to the expanded + command. - If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you - to compose commands with "|" or redirect with ">". Note that extra arguments following the alias - will not be automatically passed to the expanded expression. To have a shell alias receive - arguments, you must explicitly accept them using "$1", "$2", etc., or "$@" to accept all of them. + Use "-" as expansion argument to read the expansion string from standard input. This + is useful to avoid quoting issues when defining expansions. - Platform note: on Windows, shell aliases are executed via "sh" as installed by Git For Windows. If - you have installed git on Windows in some other way, shell aliases may not work for you. - - Quotes must always be used when defining a command as in the examples unless you pass '-' - as the expansion parameter and pipe your command to 'gh alias set'. + If the expansion starts with "!" or if "--shell" was given, the expansion is a shell + expression that will be evaluated through the "sh" interpreter when the alias is + invoked. This allows for chaining multiple commands via piping and redirection. `), Example: heredoc.Doc(` + # note: Command Prompt on Windows requires using double quotes for arguments $ gh alias set pv 'pr view' - $ gh pv -w 123 - #=> gh pr view -w 123 - - $ gh alias set bugs 'issue list --label="bugs"' + $ gh pv -w 123 #=> gh pr view -w 123 + + $ gh alias set bugs 'issue list --label=bugs' $ gh bugs $ gh alias set homework 'issue list --assignee @me' $ gh homework $ gh alias set epicsBy 'issue list --author="$1" --label="epic"' - $ gh epicsBy vilmibm - #=> gh issue list --author="vilmibm" --label="epic" + $ gh epicsBy vilmibm #=> gh issue list --author="vilmibm" --label="epic" - $ gh alias set --shell igrep 'gh issue list --label="$1" | grep $2' - $ gh igrep epic foo - #=> gh issue list --label="epic" | grep "foo" - - # users.txt contains multiline 'api graphql -F name="$1" ...' with mixed quotes - $ gh alias set users - < users.txt - $ gh users octocat - #=> gh api graphql -F name="octocat" ... + $ gh alias set --shell igrep 'gh issue list --label="$1" | grep "$2"' + $ gh igrep epic foo #=> gh issue list --label="epic" | grep "foo" `), Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - opts.RootCmd = cmd.Root() - opts.Name = args[0] opts.Expansion = args[1] + opts.validCommand = func(args string) bool { + split, err := shlex.Split(args) + if err != nil { + return false + } + + rootCmd := cmd.Root() + cmd, _, err := rootCmd.Traverse(split) + if err == nil && cmd != rootCmd { + return true + } + + for _, ext := range f.ExtensionManager.List() { + if ext.Name() == split[0] { + return true + } + } + return false + } + if runF != nil { return runF(opts) } @@ -123,11 +130,11 @@ func setRun(opts *SetOptions) error { } isShell = strings.HasPrefix(expansion, "!") - if validCommand(opts.RootCmd, opts.Name) { + if opts.validCommand(opts.Name) { return fmt.Errorf("could not create alias: %q is already a gh command", opts.Name) } - if !isShell && !validCommand(opts.RootCmd, expansion) { + if !isShell && !opts.validCommand(expansion) { return fmt.Errorf("could not create alias: %s does not correspond to a gh command", expansion) } @@ -153,16 +160,6 @@ func setRun(opts *SetOptions) error { return nil } -func validCommand(rootCmd *cobra.Command, expansion string) bool { - split, err := shlex.Split(expansion) - if err != nil { - return false - } - - cmd, _, err := rootCmd.Traverse(split) - return err == nil && cmd != rootCmd -} - func getExpansion(opts *SetOptions) (string, error) { if opts.Expansion == "-" { stdin, err := ioutil.ReadAll(opts.IO.In) diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 0e95a3c46..eaa5a9390 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -8,6 +8,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/extensions" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" "github.com/google/shlex" @@ -28,6 +29,11 @@ func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.Cmd Config: func() (config.Config, error) { return cfg, nil }, + ExtensionManager: &extensions.ExtensionManagerMock{ + ListFunc: func() []extensions.Extension { + return []extensions.Extension{} + }, + }, } cmd := NewCmdSet(factory, nil) diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go new file mode 100644 index 000000000..514477efc --- /dev/null +++ b/pkg/cmd/browse/browse.go @@ -0,0 +1,184 @@ +package browse + +import ( + "fmt" + "net/http" + "strconv" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type browser interface { + Browse(string) error +} + +type BrowseOptions struct { + BaseRepo func() (ghrepo.Interface, error) + Browser browser + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + + SelectorArg string + + Branch string + ProjectsFlag bool + SettingsFlag bool + WikiFlag bool + NoBrowserFlag bool +} + +func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Command { + opts := &BrowseOptions{ + Browser: f.Browser, + HttpClient: f.HttpClient, + IO: f.IOStreams, + } + + cmd := &cobra.Command{ + Long: "Open the GitHub repository in the web browser.", + Short: "Open the repository in the browser", + Use: "browse [ | ]", + Args: cobra.MaximumNArgs(1), + Example: heredoc.Doc(` + $ gh browse + #=> Open the home page of the current repository + + $ gh browse 217 + #=> Open issue or pull request 217 + + $ gh browse --settings + #=> Open repository settings + + $ gh browse main.go:312 + #=> Open main.go at line 312 + + $ gh browse main.go --branch main + #=> Open main.go in the main branch + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": heredoc.Doc(` + A browser location can be specified using arguments in the following format: + - by number for issue or pull request, e.g. "123"; or + - by path for opening folders and files, e.g. "cmd/gh/main.go" + `), + "help:environment": heredoc.Doc(` + To configure a web browser other than the default, use the BROWSER environment variable. + `), + }, + RunE: func(cmd *cobra.Command, args []string) error { + opts.BaseRepo = f.BaseRepo + + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if err := cmdutil.MutuallyExclusive( + "specify only one of `--branch`, `--projects`, `--wiki`, or `--settings`", + opts.Branch != "", + opts.WikiFlag, + opts.SettingsFlag, + opts.ProjectsFlag, + ); err != nil { + return err + } + + if runF != nil { + return runF(opts) + } + return runBrowse(opts) + }, + } + + cmdutil.EnableRepoOverride(cmd, f) + cmd.Flags().BoolVarP(&opts.ProjectsFlag, "projects", "p", false, "Open repository projects") + cmd.Flags().BoolVarP(&opts.WikiFlag, "wiki", "w", false, "Open repository wiki") + cmd.Flags().BoolVarP(&opts.SettingsFlag, "settings", "s", false, "Open repository settings") + cmd.Flags().BoolVarP(&opts.NoBrowserFlag, "no-browser", "n", false, "Print destination URL instead of opening the browser") + cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Select another branch by passing in the branch name") + + return cmd +} + +func runBrowse(opts *BrowseOptions) error { + baseRepo, err := opts.BaseRepo() + if err != nil { + return fmt.Errorf("unable to determine base repository: %w", err) + } + + httpClient, err := opts.HttpClient() + if err != nil { + return fmt.Errorf("unable to create an http client: %w", err) + } + url := ghrepo.GenerateRepoURL(baseRepo, "") + + if opts.SelectorArg == "" { + if opts.ProjectsFlag { + url += "/projects" + } + if opts.SettingsFlag { + url += "/settings" + } + if opts.WikiFlag { + url += "/wiki" + } + if opts.Branch != "" { + url += "/tree/" + opts.Branch + "/" + } + } else { + if isNumber(opts.SelectorArg) { + url += "/issues/" + opts.SelectorArg + } else { + fileArg, err := parseFileArg(opts.SelectorArg) + if err != nil { + return err + } + if opts.Branch != "" { + url += "/tree/" + opts.Branch + "/" + } else { + apiClient := api.NewClientFromHTTP(httpClient) + branchName, err := api.RepoDefaultBranch(apiClient, baseRepo) + if err != nil { + return err + } + url += "/tree/" + branchName + "/" + } + url += fileArg + } + } + + if opts.NoBrowserFlag { + fmt.Fprintf(opts.IO.Out, "%s\n", url) + return nil + } else { + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "now opening %s in browser\n", url) + } + return opts.Browser.Browse(url) + } +} + +func parseFileArg(fileArg string) (string, error) { + arr := strings.Split(fileArg, ":") + if len(arr) > 2 { + return "", fmt.Errorf("invalid use of colon\nUse 'gh browse --help' for more information about browse\n") + } + if len(arr) > 1 { + if !isNumber(arr[1]) { + return "", fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n") + } + return arr[0] + "#L" + arr[1], nil + } + return arr[0], nil +} + +func isNumber(arg string) bool { + _, err := strconv.Atoi(arg) + return err == nil +} diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go new file mode 100644 index 000000000..d30a95024 --- /dev/null +++ b/pkg/cmd/browse/browse_test.go @@ -0,0 +1,336 @@ +package browse + +import ( + "fmt" + "net/http" + "testing" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdBrowse(t *testing.T) { + tests := []struct { + name string + cli string + factory func(*cmdutil.Factory) *cmdutil.Factory + wants BrowseOptions + wantsErr bool + }{ + { + name: "no arguments", + cli: "", + wantsErr: false, + }, + { + name: "settings flag", + cli: "--settings", + wants: BrowseOptions{ + SettingsFlag: true, + }, + wantsErr: false, + }, + { + name: "projects flag", + cli: "--projects", + wants: BrowseOptions{ + ProjectsFlag: true, + }, + wantsErr: false, + }, + { + name: "wiki flag", + cli: "--wiki", + wants: BrowseOptions{ + WikiFlag: true, + }, + wantsErr: false, + }, + { + name: "no browser flag", + cli: "--no-browser", + wants: BrowseOptions{ + NoBrowserFlag: true, + }, + wantsErr: false, + }, + { + name: "branch flag", + cli: "--branch main", + wants: BrowseOptions{ + Branch: "main", + }, + wantsErr: false, + }, + { + name: "branch flag without a branch name", + cli: "--branch", + wantsErr: true, + }, + { + name: "combination: settings projects", + cli: "--settings --projects", + wants: BrowseOptions{ + SettingsFlag: true, + ProjectsFlag: true, + }, + wantsErr: true, + }, + { + name: "combination: projects wiki", + cli: "--projects --wiki", + wants: BrowseOptions{ + ProjectsFlag: true, + WikiFlag: true, + }, + wantsErr: true, + }, + { + name: "passed argument", + cli: "main.go", + wants: BrowseOptions{ + SelectorArg: "main.go", + }, + wantsErr: false, + }, + { + name: "passed two arguments", + cli: "main.go main.go", + wantsErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := cmdutil.Factory{} + var opts *BrowseOptions + cmd := NewCmdBrowse(&f, func(o *BrowseOptions) error { + opts = o + return nil + }) + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + cmd.SetArgs(argv) + _, err = cmd.ExecuteC() + + if tt.wantsErr { + assert.Error(t, err) + return + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.wants.Branch, opts.Branch) + assert.Equal(t, tt.wants.SelectorArg, opts.SelectorArg) + assert.Equal(t, tt.wants.ProjectsFlag, opts.ProjectsFlag) + assert.Equal(t, tt.wants.WikiFlag, opts.WikiFlag) + assert.Equal(t, tt.wants.NoBrowserFlag, opts.NoBrowserFlag) + assert.Equal(t, tt.wants.SettingsFlag, opts.SettingsFlag) + }) + } +} + +func Test_runBrowse(t *testing.T) { + tests := []struct { + name string + opts BrowseOptions + baseRepo ghrepo.Interface + defaultBranch string + expectedURL string + wantsErr bool + }{ + { + name: "no arguments", + opts: BrowseOptions{ + SelectorArg: "", + }, + baseRepo: ghrepo.New("jlsestak", "cli"), + expectedURL: "https://github.com/jlsestak/cli", + }, + { + name: "settings flag", + opts: BrowseOptions{ + SettingsFlag: true, + }, + baseRepo: ghrepo.New("bchadwic", "ObscuredByClouds"), + expectedURL: "https://github.com/bchadwic/ObscuredByClouds/settings", + }, + { + name: "projects flag", + opts: BrowseOptions{ + ProjectsFlag: true, + }, + baseRepo: ghrepo.New("ttran112", "7ate9"), + expectedURL: "https://github.com/ttran112/7ate9/projects", + }, + { + name: "wiki flag", + opts: BrowseOptions{ + WikiFlag: true, + }, + baseRepo: ghrepo.New("ravocean", "ThreatLevelMidnight"), + expectedURL: "https://github.com/ravocean/ThreatLevelMidnight/wiki", + }, + { + name: "file argument", + opts: BrowseOptions{SelectorArg: "path/to/file.txt"}, + baseRepo: ghrepo.New("ken", "mrprofessor"), + defaultBranch: "main", + expectedURL: "https://github.com/ken/mrprofessor/tree/main/path/to/file.txt", + }, + { + name: "issue argument", + opts: BrowseOptions{ + SelectorArg: "217", + }, + baseRepo: ghrepo.New("kevin", "MinTy"), + expectedURL: "https://github.com/kevin/MinTy/issues/217", + }, + { + name: "branch flag", + opts: BrowseOptions{ + Branch: "trunk", + }, + baseRepo: ghrepo.New("jlsestak", "CouldNotThinkOfARepoName"), + expectedURL: "https://github.com/jlsestak/CouldNotThinkOfARepoName/tree/trunk/", + }, + { + name: "branch flag with file", + opts: BrowseOptions{ + Branch: "trunk", + SelectorArg: "main.go", + }, + baseRepo: ghrepo.New("bchadwic", "LedZeppelinIV"), + expectedURL: "https://github.com/bchadwic/LedZeppelinIV/tree/trunk/main.go", + }, + { + name: "file with line number", + opts: BrowseOptions{ + SelectorArg: "path/to/file.txt:32", + }, + baseRepo: ghrepo.New("ravocean", "angur"), + defaultBranch: "trunk", + expectedURL: "https://github.com/ravocean/angur/tree/trunk/path/to/file.txt#L32", + }, + { + name: "file with invalid line number", + opts: BrowseOptions{ + SelectorArg: "path/to/file.txt:32:32", + }, + baseRepo: ghrepo.New("ttran112", "ttrain211"), + wantsErr: true, + }, + { + name: "branch with issue number", + opts: BrowseOptions{ + SelectorArg: "217", + Branch: "trunk", + }, + baseRepo: ghrepo.New("ken", "grc"), + wantsErr: false, + expectedURL: "https://github.com/ken/grc/issues/217", + }, + { + name: "opening branch file with line number", + opts: BrowseOptions{ + Branch: "first-browse-pull", + SelectorArg: "browse.go:32", + }, + baseRepo: ghrepo.New("github", "ThankYouGitHub"), + wantsErr: false, + expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/browse.go#L32", + }, + { + name: "no browser with branch file and line number", + opts: BrowseOptions{ + Branch: "3-0-stable", + SelectorArg: "init.rb:6", + NoBrowserFlag: true, + }, + baseRepo: ghrepo.New("mislav", "will_paginate"), + wantsErr: false, + expectedURL: "https://github.com/mislav/will_paginate/tree/3-0-stable/init.rb#L6", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + browser := cmdutil.TestBrowser{} + + reg := httpmock.Registry{} + defer reg.Verify(t) + if tt.defaultBranch != "" { + reg.StubRepoInfoResponse(tt.baseRepo.RepoOwner(), tt.baseRepo.RepoName(), tt.defaultBranch) + } + + opts := tt.opts + opts.IO = io + opts.BaseRepo = func() (ghrepo.Interface, error) { + return tt.baseRepo, nil + } + opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: ®}, nil + } + opts.Browser = &browser + + err := runBrowse(&opts) + if tt.wantsErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + if opts.NoBrowserFlag { + assert.Equal(t, fmt.Sprintf("%s\n", tt.expectedURL), stdout.String()) + assert.Equal(t, "", stderr.String()) + browser.Verify(t, "") + } else { + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + browser.Verify(t, tt.expectedURL) + } + }) + } +} + +func Test_parseFileArg(t *testing.T) { + tests := []struct { + name string + arg string + errorExpected bool + expectedFileArg string + stderrExpected string + }{ + { + name: "non line number", + arg: "main.go", + errorExpected: false, + expectedFileArg: "main.go", + }, + { + name: "line number", + arg: "main.go:32", + errorExpected: false, + expectedFileArg: "main.go#L32", + }, + { + name: "non line number error", + arg: "ma:in.go", + errorExpected: true, + stderrExpected: "invalid line number after colon\nUse 'gh browse --help' for more information about browse\n", + }, + } + for _, tt := range tests { + fileArg, err := parseFileArg(tt.arg) + if tt.errorExpected { + assert.Equal(t, err.Error(), tt.stderrExpected) + } else { + assert.Equal(t, err, nil) + assert.Equal(t, tt.expectedFileArg, fileArg) + } + } +} diff --git a/pkg/cmd/extensions/command.go b/pkg/cmd/extensions/command.go new file mode 100644 index 000000000..c7fc031da --- /dev/null +++ b/pkg/cmd/extensions/command.go @@ -0,0 +1,163 @@ +package extensions + +import ( + "errors" + "fmt" + "os" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/extensions" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command { + m := f.ExtensionManager + io := f.IOStreams + + extCmd := cobra.Command{ + Use: "extensions", + Short: "Manage gh extensions", + Long: heredoc.Docf(` + GitHub CLI extensions are repositories that provide additional gh commands. + + The name of the extension repository must start with "gh-" and it must contain an + executable of the same name. All arguments passed to the %[1]sgh %[1]s invocation + will be forwarded to the %[1]sgh-%[1]s executable of the extension. + + An extension cannot override any of the core gh commands. + `, "`"), + } + + extCmd.AddCommand( + &cobra.Command{ + Use: "list", + Short: "List installed extension commands", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + cmds := m.List() + if len(cmds) == 0 { + return errors.New("no extensions installed") + } + // cs := io.ColorScheme() + t := utils.NewTablePrinter(io) + for _, c := range cmds { + var repo string + if u, err := git.ParseURL(c.URL()); err == nil { + if r, err := ghrepo.FromURL(u); err == nil { + repo = ghrepo.FullName(r) + } + } + + t.AddField(fmt.Sprintf("gh %s", c.Name()), nil, nil) + t.AddField(repo, nil, nil) + // TODO: add notice about available update + //t.AddField("Update available", nil, cs.Green) + t.EndRow() + } + return t.Render() + }, + }, + &cobra.Command{ + Use: "install ", + Short: "Install a gh extension from a repository", + Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), + RunE: func(cmd *cobra.Command, args []string) error { + if args[0] == "." { + wd, err := os.Getwd() + if err != nil { + return err + } + return m.InstallLocal(wd) + } + + repo, err := ghrepo.FromFullName(args[0]) + if err != nil { + return err + } + if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { + return err + } + + cfg, err := f.Config() + if err != nil { + return err + } + protocol, _ := cfg.Get(repo.RepoHost(), "git_protocol") + return m.Install(ghrepo.FormatRemoteURL(repo, protocol), io.Out, io.ErrOut) + }, + }, + func() *cobra.Command { + var flagAll bool + cmd := &cobra.Command{ + Use: "upgrade { | --all}", + Short: "Upgrade installed extensions", + Args: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 && !flagAll { + return &cmdutil.FlagError{Err: errors.New("must specify an extension to upgrade")} + } + if len(args) > 0 && flagAll { + return &cmdutil.FlagError{Err: errors.New("cannot use `--all` with extension name")} + } + if len(args) > 1 { + return &cmdutil.FlagError{Err: errors.New("too many arguments")} + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + var name string + if len(args) > 0 { + name = args[0] + } + return m.Upgrade(name, io.Out, io.ErrOut) + }, + } + cmd.Flags().BoolVar(&flagAll, "all", false, "Upgrade all extensions") + return cmd + }(), + &cobra.Command{ + Use: "remove", + Short: "Remove an installed extension", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + extName := args[0] + if err := m.Remove(extName); err != nil { + return err + } + if io.IsStdoutTTY() { + cs := io.ColorScheme() + fmt.Fprintf(io.Out, "%s Removed extension %s\n", cs.SuccessIcon(), extName) + } + return nil + }, + }, + ) + + extCmd.Hidden = true + return &extCmd +} + +func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager, extName string) error { + if !strings.HasPrefix(extName, "gh-") { + return errors.New("extension repository name must start with `gh-`") + } + + commandName := strings.TrimPrefix(extName, "gh-") + if c, _, err := rootCmd.Traverse([]string{commandName}); err != nil { + return err + } else if c != rootCmd { + return fmt.Errorf("%q matches the name of a built-in command", commandName) + } + + for _, ext := range m.List() { + if ext.Name() == commandName { + return fmt.Errorf("there is already an installed extension that provides the %q command", commandName) + } + } + + return nil +} diff --git a/pkg/cmd/extensions/command_test.go b/pkg/cmd/extensions/command_test.go new file mode 100644 index 000000000..f67664eaa --- /dev/null +++ b/pkg/cmd/extensions/command_test.go @@ -0,0 +1,271 @@ +package extensions + +import ( + "io" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/extensions" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdExtensions(t *testing.T) { + tempDir := t.TempDir() + oldWd, _ := os.Getwd() + assert.NoError(t, os.Chdir(tempDir)) + t.Cleanup(func() { _ = os.Chdir(oldWd) }) + + tests := []struct { + name string + args []string + managerStubs func(em *extensions.ExtensionManagerMock) func(*testing.T) + isTTY bool + wantErr bool + errMsg string + wantStdout string + wantStderr string + }{ + { + name: "install an extension", + args: []string{"install", "owner/gh-some-ext"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.ListFunc = func() []extensions.Extension { + return []extensions.Extension{} + } + em.InstallFunc = func(s string, out, errOut io.Writer) error { + return nil + } + return func(t *testing.T) { + installCalls := em.InstallCalls() + assert.Equal(t, 1, len(installCalls)) + assert.Equal(t, "https://github.com/owner/gh-some-ext.git", installCalls[0].URL) + listCalls := em.ListCalls() + assert.Equal(t, 1, len(listCalls)) + } + }, + }, + { + name: "install an extension with same name as existing extension", + args: []string{"install", "owner/gh-existing-ext"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.ListFunc = func() []extensions.Extension { + e := &Extension{path: "owner2/gh-existing-ext"} + return []extensions.Extension{e} + } + return func(t *testing.T) { + calls := em.ListCalls() + assert.Equal(t, 1, len(calls)) + } + }, + wantErr: true, + errMsg: "there is already an installed extension that provides the \"existing-ext\" command", + }, + { + name: "install local extension", + args: []string{"install", "."}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.InstallLocalFunc = func(dir string) error { + return nil + } + return func(t *testing.T) { + calls := em.InstallLocalCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, tempDir, normalizeDir(calls[0].Dir)) + } + }, + }, + { + name: "upgrade error", + args: []string{"upgrade"}, + wantErr: true, + errMsg: "must specify an extension to upgrade", + }, + { + name: "upgrade an extension", + args: []string{"upgrade", "hello"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, out, errOut io.Writer) error { + return nil + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + } + }, + }, + { + name: "upgrade all", + args: []string{"upgrade", "--all"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, out, errOut io.Writer) error { + return nil + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "", calls[0].Name) + } + }, + }, + { + name: "remove extension tty", + args: []string{"remove", "hello"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.RemoveFunc = func(name string) error { + return nil + } + return func(t *testing.T) { + calls := em.RemoveCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + } + }, + isTTY: true, + wantStdout: "✓ Removed extension hello\n", + }, + { + name: "remove extension nontty", + args: []string{"remove", "hello"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.RemoveFunc = func(name string) error { + return nil + } + return func(t *testing.T) { + calls := em.RemoveCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + } + }, + isTTY: false, + wantStdout: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + + var assertFunc func(*testing.T) + em := &extensions.ExtensionManagerMock{} + if tt.managerStubs != nil { + assertFunc = tt.managerStubs(em) + } + + f := cmdutil.Factory{ + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + IOStreams: ios, + ExtensionManager: em, + } + + cmd := NewCmdExtensions(&f) + cmd.SetArgs(tt.args) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err := cmd.ExecuteC() + if tt.wantErr { + assert.EqualError(t, err, tt.errMsg) + } else { + assert.NoError(t, err) + } + + if assertFunc != nil { + assertFunc(t) + } + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} + +func normalizeDir(d string) string { + return strings.TrimPrefix(d, "/private") +} + +func Test_checkValidExtension(t *testing.T) { + rootCmd := &cobra.Command{} + rootCmd.AddCommand(&cobra.Command{Use: "help"}) + rootCmd.AddCommand(&cobra.Command{Use: "auth"}) + + m := &extensions.ExtensionManagerMock{ + ListFunc: func() []extensions.Extension { + return []extensions.Extension{ + &extensions.ExtensionMock{ + NameFunc: func() string { return "screensaver" }, + }, + &extensions.ExtensionMock{ + NameFunc: func() string { return "triage" }, + }, + } + }, + } + + type args struct { + rootCmd *cobra.Command + manager extensions.ExtensionManager + extName string + } + tests := []struct { + name string + args args + wantError string + }{ + { + name: "valid extension", + args: args{ + rootCmd: rootCmd, + manager: m, + extName: "gh-hello", + }, + }, + { + name: "invalid extension name", + args: args{ + rootCmd: rootCmd, + manager: m, + extName: "gherkins", + }, + wantError: "extension repository name must start with `gh-`", + }, + { + name: "clashes with built-in command", + args: args{ + rootCmd: rootCmd, + manager: m, + extName: "gh-auth", + }, + wantError: "\"auth\" matches the name of a built-in command", + }, + { + name: "clashes with an installed extension", + args: args{ + rootCmd: rootCmd, + manager: m, + extName: "gh-triage", + }, + wantError: "there is already an installed extension that provides the \"triage\" command", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkValidExtension(tt.args.rootCmd, tt.args.manager, tt.args.extName) + if tt.wantError == "" { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, tt.wantError) + } + }) + } +} diff --git a/pkg/cmd/extensions/extension.go b/pkg/cmd/extensions/extension.go new file mode 100644 index 000000000..8fa07e7cf --- /dev/null +++ b/pkg/cmd/extensions/extension.go @@ -0,0 +1,42 @@ +package extensions + +import ( + "errors" + "os" + "path/filepath" + "strings" +) + +type Extension struct { + path string + url string +} + +func (e *Extension) Name() string { + return strings.TrimPrefix(filepath.Base(e.path), "gh-") +} + +func (e *Extension) Path() string { + return e.path +} + +func (e *Extension) URL() string { + return e.url +} + +func (e *Extension) IsLocal() bool { + dir := filepath.Dir(e.path) + fileInfo, err := os.Lstat(dir) + if err != nil { + return false + } + // Check if extension is a symlink + if fileInfo.Mode()&os.ModeSymlink != 0 { + return true + } + // Check if extension does not have a git directory + if _, err = os.Stat(filepath.Join(dir, ".git")); errors.Is(err, os.ErrNotExist) { + return true + } + return false +} diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go new file mode 100644 index 000000000..ff6452657 --- /dev/null +++ b/pkg/cmd/extensions/manager.go @@ -0,0 +1,194 @@ +package extensions + +import ( + "bytes" + "errors" + "fmt" + "io" + "io/ioutil" + "os" + "os/exec" + "path" + "path/filepath" + "runtime" + "strings" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/extensions" + "github.com/cli/cli/pkg/findsh" + "github.com/cli/safeexec" +) + +type Manager struct { + dataDir func() string + lookPath func(string) (string, error) + findSh func() (string, error) + newCommand func(string, ...string) *exec.Cmd +} + +func NewManager() *Manager { + return &Manager{ + dataDir: config.DataDir, + lookPath: safeexec.LookPath, + findSh: findsh.Find, + newCommand: exec.Command, + } +} + +func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) { + if len(args) == 0 { + return false, errors.New("too few arguments in list") + } + + var exe string + extName := args[0] + forwardArgs := args[1:] + + for _, e := range m.list(false) { + if e.Name() == extName { + exe = e.Path() + break + } + } + if exe == "" { + return false, nil + } + + var externalCmd *exec.Cmd + + if runtime.GOOS == "windows" { + // Dispatch all extension calls through the `sh` interpreter to support executable files with a + // shebang line on Windows. + shExe, err := m.findSh() + if err != nil { + if errors.Is(err, exec.ErrNotFound) { + return true, errors.New("the `sh.exe` interpreter is required. Please install Git for Windows and try again") + } + return true, err + } + forwardArgs = append([]string{"-c", `command "$@"`, "--", exe}, forwardArgs...) + externalCmd = m.newCommand(shExe, forwardArgs...) + } else { + externalCmd = m.newCommand(exe, forwardArgs...) + } + externalCmd.Stdin = stdin + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + return true, externalCmd.Run() +} + +func (m *Manager) List() []extensions.Extension { + return m.list(true) +} + +func (m *Manager) list(includeMetadata bool) []extensions.Extension { + dir := m.installDir() + entries, err := ioutil.ReadDir(dir) + if err != nil { + return nil + } + + var gitExe string + if includeMetadata { + gitExe, _ = m.lookPath("git") + } + + var results []extensions.Extension + for _, f := range entries { + if !strings.HasPrefix(f.Name(), "gh-") || !(f.IsDir() || f.Mode()&os.ModeSymlink != 0) { + continue + } + var remoteURL string + if gitExe != "" { + stdout := bytes.Buffer{} + cmd := m.newCommand(gitExe, "--git-dir="+filepath.Join(dir, f.Name(), ".git"), "config", "remote.origin.url") + cmd.Stdout = &stdout + if err := cmd.Run(); err == nil { + remoteURL = strings.TrimSpace(stdout.String()) + } + } + results = append(results, &Extension{ + path: filepath.Join(dir, f.Name(), f.Name()), + url: remoteURL, + }) + } + return results +} + +func (m *Manager) InstallLocal(dir string) error { + name := filepath.Base(dir) + targetDir := filepath.Join(m.installDir(), name) + return os.Symlink(dir, targetDir) +} + +func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error { + exe, err := m.lookPath("git") + if err != nil { + return err + } + + name := strings.TrimSuffix(path.Base(cloneURL), ".git") + targetDir := filepath.Join(m.installDir(), name) + + externalCmd := m.newCommand(exe, "clone", cloneURL, targetDir) + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + return externalCmd.Run() +} + +var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") + +func (m *Manager) Upgrade(name string, stdout, stderr io.Writer) error { + exe, err := m.lookPath("git") + if err != nil { + return err + } + + exts := m.List() + if len(exts) == 0 { + return errors.New("no extensions installed") + } + + someUpgraded := false + for _, f := range exts { + if name == "" { + fmt.Fprintf(stdout, "[%s]: ", f.Name()) + } else if f.Name() != name { + continue + } + + if f.IsLocal() { + if name == "" { + fmt.Fprintf(stdout, "%s\n", localExtensionUpgradeError) + } else { + err = localExtensionUpgradeError + } + continue + } + + dir := filepath.Dir(f.Path()) + externalCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + if e := externalCmd.Run(); e != nil { + err = e + } + someUpgraded = true + } + if err == nil && !someUpgraded { + err = fmt.Errorf("no extension matched %q", name) + } + return err +} + +func (m *Manager) Remove(name string) error { + targetDir := filepath.Join(m.installDir(), "gh-"+name) + if _, err := os.Stat(targetDir); os.IsNotExist(err) { + return fmt.Errorf("no extension found: %q", targetDir) + } + return os.RemoveAll(targetDir) +} + +func (m *Manager) installDir() string { + return filepath.Join(m.dataDir(), "extensions") +} diff --git a/pkg/cmd/extensions/manager_test.go b/pkg/cmd/extensions/manager_test.go new file mode 100644 index 000000000..693bfed89 --- /dev/null +++ b/pkg/cmd/extensions/manager_test.go @@ -0,0 +1,204 @@ +package extensions + +import ( + "bytes" + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/stretchr/testify/assert" +) + +func TestHelperProcess(t *testing.T) { + if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" { + return + } + if err := func(args []string) error { + fmt.Fprintf(os.Stdout, "%v\n", args) + return nil + }(os.Args[3:]); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) +} + +func newTestManager(dir string) *Manager { + return &Manager{ + dataDir: func() string { return dir }, + lookPath: func(exe string) (string, error) { return exe, nil }, + findSh: func() (string, error) { return "sh", nil }, + newCommand: func(exe string, args ...string) *exec.Cmd { + args = append([]string{os.Args[0], "-test.run=TestHelperProcess", "--", exe}, args...) + cmd := exec.Command(args[0], args[1:]...) + cmd.Env = []string{"GH_WANT_HELPER_PROCESS=1"} + return cmd + }, + } +} + +func TestManager_List(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + + m := newTestManager(tempDir) + exts := m.List() + assert.Equal(t, 2, len(exts)) + assert.Equal(t, "hello", exts[0].Name()) + assert.Equal(t, "two", exts[1].Name()) +} + +func TestManager_Dispatch(t *testing.T) { + tempDir := t.TempDir() + extPath := filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello") + assert.NoError(t, stubExtension(extPath)) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + found, err := m.Dispatch([]string{"hello", "one", "two"}, nil, stdout, stderr) + assert.NoError(t, err) + assert.True(t, found) + + if runtime.GOOS == "windows" { + assert.Equal(t, fmt.Sprintf("[sh -c command \"$@\" -- %s one two]\n", extPath), stdout.String()) + } else { + assert.Equal(t, fmt.Sprintf("[%s one two]\n", extPath), stdout.String()) + } + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Remove(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + + m := newTestManager(tempDir) + err := m.Remove("hello") + assert.NoError(t, err) + + items, err := ioutil.ReadDir(filepath.Join(tempDir, "extensions")) + assert.NoError(t, err) + assert.Equal(t, 1, len(items)) + assert.Equal(t, "gh-two", items[0].Name()) +} + +func TestManager_Upgrade_AllExtensions(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + assert.NoError(t, stubLocalExtension(filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("", stdout, stderr) + assert.NoError(t, err) + + assert.Equal(t, heredoc.Docf( + ` + [hello]: [git -C %s --git-dir=%s pull --ff-only] + [local]: local extensions can not be upgraded + [two]: [git -C %s --git-dir=%s pull --ff-only] + `, + filepath.Join(tempDir, "extensions", "gh-hello"), + filepath.Join(tempDir, "extensions", "gh-hello", ".git"), + filepath.Join(tempDir, "extensions", "gh-two"), + filepath.Join(tempDir, "extensions", "gh-two", ".git"), + ), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Upgrade_RemoteExtension(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("remote", stdout, stderr) + assert.NoError(t, err) + assert.Equal(t, heredoc.Docf( + ` + [git -C %s --git-dir=%s pull --ff-only] + `, + filepath.Join(tempDir, "extensions", "gh-remote"), + filepath.Join(tempDir, "extensions", "gh-remote", ".git"), + ), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Upgrade_LocalExtension(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubLocalExtension(filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("local", stdout, stderr) + assert.EqualError(t, err, "local extensions can not be upgraded") + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Upgrade_NoExtensions(t *testing.T) { + tempDir := t.TempDir() + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("", stdout, stderr) + assert.EqualError(t, err, "no extensions installed") + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Install(t *testing.T) { + tempDir := t.TempDir() + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Install("https://github.com/owner/gh-some-ext.git", stdout, stderr) + assert.NoError(t, err) + assert.Equal(t, fmt.Sprintf("[git clone https://github.com/owner/gh-some-ext.git %s]\n", filepath.Join(tempDir, "extensions", "gh-some-ext")), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func stubExtension(path string) error { + dir := filepath.Dir(path) + gitDir := filepath.Join(dir, ".git") + if err := os.MkdirAll(dir, 0755); err != nil { + return err + } + if err := os.Mkdir(gitDir, 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) + if err != nil { + return err + } + return f.Close() +} + +func stubLocalExtension(path string) error { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) + if err != nil { + return err + } + return f.Close() +} diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index e9023c852..32187689b 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -6,19 +6,106 @@ import ( "net/http" "os" + "github.com/cli/cli/api" + "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/extensions" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" ) func New(appVersion string) *cmdutil.Factory { - io := iostreams.System() + f := &cmdutil.Factory{ + Config: configFunc(), // No factory dependencies + Branch: branchFunc(), // No factory dependencies + Executable: executable(), // No factory dependencies + ExtensionManager: extensions.NewManager(), + } + + f.IOStreams = ioStreams(f) // Depends on Config + f.HttpClient = httpClientFunc(f, appVersion) // Depends on Config, IOStreams, and appVersion + f.Remotes = remotesFunc(f) // Depends on Config + f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes + f.Browser = browser(f) // Depends on IOStreams + + return f +} + +func BaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { + return func() (ghrepo.Interface, error) { + remotes, err := f.Remotes() + if err != nil { + return nil, err + } + return remotes[0], nil + } +} + +func SmartBaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { + return func() (ghrepo.Interface, error) { + httpClient, err := f.HttpClient() + if err != nil { + return nil, err + } + + apiClient := api.NewClientFromHTTP(httpClient) + + remotes, err := f.Remotes() + if err != nil { + return nil, err + } + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") + if err != nil { + return nil, err + } + baseRepo, err := repoContext.BaseRepo(f.IOStreams) + if err != nil { + return nil, err + } + + return baseRepo, nil + } +} + +func remotesFunc(f *cmdutil.Factory) func() (context.Remotes, error) { + rr := &remoteResolver{ + readRemotes: git.Remotes, + getConfig: f.Config, + } + return rr.Resolver() +} + +func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, error) { + return func() (*http.Client, error) { + io := f.IOStreams + cfg, err := f.Config() + if err != nil { + return nil, err + } + return NewHTTPClient(io, cfg, appVersion, true) + } +} + +func browser(f *cmdutil.Factory) cmdutil.Browser { + io := f.IOStreams + return cmdutil.NewBrowser(os.Getenv("BROWSER"), io.Out, io.ErrOut) +} + +func executable() string { + gh := "gh" + if exe, err := os.Executable(); err == nil { + gh = exe + } + return gh +} + +func configFunc() func() (config.Config, error) { var cachedConfig config.Config var configError error - configFunc := func() (config.Config, error) { + return func() (config.Config, error) { if cachedConfig != nil || configError != nil { return cachedConfig, configError } @@ -30,45 +117,38 @@ func New(appVersion string) *cmdutil.Factory { cachedConfig = config.InheritEnv(cachedConfig) return cachedConfig, configError } +} - rr := &remoteResolver{ - readRemotes: git.Remotes, - getConfig: configFunc, - } - remotesFunc := rr.Resolver() - - ghExecutable := "gh" - if exe, err := os.Executable(); err == nil { - ghExecutable = exe - } - - return &cmdutil.Factory{ - IOStreams: io, - Config: configFunc, - Remotes: remotesFunc, - HttpClient: func() (*http.Client, error) { - cfg, err := configFunc() - if err != nil { - return nil, err - } - - return NewHTTPClient(io, cfg, appVersion, true), nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - remotes, err := remotesFunc() - if err != nil { - return nil, err - } - return remotes[0], nil - }, - Branch: func() (string, error) { - currentBranch, err := git.CurrentBranch() - if err != nil { - return "", fmt.Errorf("could not determine current branch: %w", err) - } - return currentBranch, nil - }, - Executable: ghExecutable, - Browser: cmdutil.NewBrowser(os.Getenv("BROWSER"), io.Out, io.ErrOut), +func branchFunc() func() (string, error) { + return func() (string, error) { + currentBranch, err := git.CurrentBranch() + if err != nil { + return "", fmt.Errorf("could not determine current branch: %w", err) + } + return currentBranch, nil } } + +func ioStreams(f *cmdutil.Factory) *iostreams.IOStreams { + io := iostreams.System() + cfg, err := f.Config() + if err != nil { + return io + } + + if prompt, _ := cfg.Get("", "prompt"); prompt == "disabled" { + io.SetNeverPrompt(true) + } + + // Pager precedence + // 1. GH_PAGER + // 2. pager from config + // 3. PAGER + if ghPager, ghPagerExists := os.LookupEnv("GH_PAGER"); ghPagerExists { + io.SetPager(ghPager) + } else if pager, _ := cfg.Get("", "pager"); pager != "" { + io.SetPager(pager) + } + + return io +} diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go new file mode 100644 index 000000000..c7e641aa1 --- /dev/null +++ b/pkg/cmd/factory/default_test.go @@ -0,0 +1,395 @@ +package factory + +import ( + "net/url" + "os" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/stretchr/testify/assert" +) + +func Test_BaseRepo(t *testing.T) { + orig_GH_HOST := os.Getenv("GH_HOST") + t.Cleanup(func() { + os.Setenv("GH_HOST", orig_GH_HOST) + }) + + tests := []struct { + name string + remotes git.RemoteSet + config config.Config + override string + wantsErr bool + wantsName string + wantsOwner string + wantsHost string + }{ + { + name: "matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://nonsense.com/owner/repo.git"), + }, + config: defaultConfig(), + wantsName: "repo", + wantsOwner: "owner", + wantsHost: "nonsense.com", + }, + { + name: "no matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://test.com/owner/repo.git"), + }, + config: defaultConfig(), + wantsErr: true, + }, + { + name: "override with matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://test.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsName: "repo", + wantsOwner: "owner", + wantsHost: "test.com", + }, + { + name: "override with no matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://nonsense.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.override != "" { + os.Setenv("GH_HOST", tt.override) + } else { + os.Unsetenv("GH_HOST") + } + f := New("1") + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + return tt.remotes, nil + }, + getConfig: func() (config.Config, error) { + return tt.config, nil + }, + } + f.Remotes = rr.Resolver() + f.BaseRepo = BaseRepoFunc(f) + repo, err := f.BaseRepo() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantsName, repo.RepoName()) + assert.Equal(t, tt.wantsOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantsHost, repo.RepoHost()) + }) + } +} + +func Test_SmartBaseRepo(t *testing.T) { + pu, _ := url.Parse("https://test.com/newowner/newrepo.git") + orig_GH_HOST := os.Getenv("GH_HOST") + t.Cleanup(func() { + os.Setenv("GH_HOST", orig_GH_HOST) + }) + + tests := []struct { + name string + remotes git.RemoteSet + config config.Config + override string + wantsErr bool + wantsName string + wantsOwner string + wantsHost string + }{ + { + name: "override with matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://test.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsName: "repo", + wantsOwner: "owner", + wantsHost: "test.com", + }, + { + name: "override with matching remote and base resolution", + remotes: git.RemoteSet{ + &git.Remote{Name: "origin", + Resolved: "base", + FetchURL: pu, + PushURL: pu}, + }, + config: defaultConfig(), + override: "test.com", + wantsName: "newrepo", + wantsOwner: "newowner", + wantsHost: "test.com", + }, + { + name: "override with matching remote and nonbase resolution", + remotes: git.RemoteSet{ + &git.Remote{Name: "origin", + Resolved: "johnny/test", + FetchURL: pu, + PushURL: pu}, + }, + config: defaultConfig(), + override: "test.com", + wantsName: "test", + wantsOwner: "johnny", + wantsHost: "test.com", + }, + { + name: "override with no matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://example.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.override != "" { + os.Setenv("GH_HOST", tt.override) + } else { + os.Unsetenv("GH_HOST") + } + f := New("1") + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + return tt.remotes, nil + }, + getConfig: func() (config.Config, error) { + return tt.config, nil + }, + } + f.Remotes = rr.Resolver() + f.BaseRepo = SmartBaseRepoFunc(f) + repo, err := f.BaseRepo() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantsName, repo.RepoName()) + assert.Equal(t, tt.wantsOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantsHost, repo.RepoHost()) + }) + } +} + +// Defined in pkg/cmdutil/repo_override.go but test it along with other BaseRepo functions +func Test_OverrideBaseRepo(t *testing.T) { + orig_GH_HOST := os.Getenv("GH_REPO") + t.Cleanup(func() { + os.Setenv("GH_REPO", orig_GH_HOST) + }) + + tests := []struct { + name string + remotes git.RemoteSet + config config.Config + envOverride string + argOverride string + wantsErr bool + wantsName string + wantsOwner string + wantsHost string + }{ + { + name: "override from argument", + argOverride: "override/test", + wantsHost: "github.com", + wantsOwner: "override", + wantsName: "test", + }, + { + name: "override from environment", + envOverride: "somehost.com/override/test", + wantsHost: "somehost.com", + wantsOwner: "override", + wantsName: "test", + }, + { + name: "no override", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://nonsense.com/owner/repo.git"), + }, + config: defaultConfig(), + wantsHost: "nonsense.com", + wantsOwner: "owner", + wantsName: "repo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envOverride != "" { + os.Setenv("GH_REPO", tt.envOverride) + } else { + os.Unsetenv("GH_REPO") + } + f := New("1") + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + return tt.remotes, nil + }, + getConfig: func() (config.Config, error) { + return tt.config, nil + }, + } + f.Remotes = rr.Resolver() + f.BaseRepo = cmdutil.OverrideBaseRepoFunc(f, tt.argOverride) + repo, err := f.BaseRepo() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantsName, repo.RepoName()) + assert.Equal(t, tt.wantsOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantsHost, repo.RepoHost()) + }) + } +} + +func Test_ioStreams_pager(t *testing.T) { + tests := []struct { + name string + env map[string]string + config config.Config + wantPager string + }{ + { + name: "GH_PAGER and PAGER set", + env: map[string]string{ + "GH_PAGER": "GH_PAGER", + "PAGER": "PAGER", + }, + wantPager: "GH_PAGER", + }, + { + name: "GH_PAGER and config pager set", + env: map[string]string{ + "GH_PAGER": "GH_PAGER", + }, + config: pagerConfig(), + wantPager: "GH_PAGER", + }, + { + name: "config pager and PAGER set", + env: map[string]string{ + "PAGER": "PAGER", + }, + config: pagerConfig(), + wantPager: "CONFIG_PAGER", + }, + { + name: "only PAGER set", + env: map[string]string{ + "PAGER": "PAGER", + }, + wantPager: "PAGER", + }, + { + name: "GH_PAGER set to blank string", + env: map[string]string{ + "GH_PAGER": "", + "PAGER": "PAGER", + }, + wantPager: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + if k == "GH_PAGER" { + defer os.Unsetenv(k) + } else { + defer os.Setenv(k, old) + } + } + } + f := New("1") + f.Config = func() (config.Config, error) { + if tt.config == nil { + return config.NewBlankConfig(), nil + } else { + return tt.config, nil + } + } + io := ioStreams(f) + assert.Equal(t, tt.wantPager, io.GetPager()) + }) + } +} + +func Test_ioStreams_prompt(t *testing.T) { + tests := []struct { + name string + config config.Config + promptDisabled bool + }{ + { + name: "default config", + promptDisabled: false, + }, + { + name: "config with prompt disabled", + config: disablePromptConfig(), + promptDisabled: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := New("1") + f.Config = func() (config.Config, error) { + if tt.config == nil { + return config.NewBlankConfig(), nil + } else { + return tt.config, nil + } + } + io := ioStreams(f) + assert.Equal(t, tt.promptDisabled, io.GetNeverPrompt()) + }) + } +} + +func defaultConfig() config.Config { + return config.InheritEnv(config.NewFromString(heredoc.Doc(` + hosts: + nonsense.com: + oauth_token: BLAH + `))) +} + +func pagerConfig() config.Config { + return config.NewFromString("pager: CONFIG_PAGER") +} + +func disablePromptConfig() config.Config { + return config.NewFromString("prompt: disabled") +} diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 2e7619540..28dac1ed6 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -8,8 +8,8 @@ import ( "time" "github.com/cli/cli/api" - "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/internal/httpunix" "github.com/cli/cli/pkg/iostreams" ) @@ -53,9 +53,36 @@ var timezoneNames = map[int]string{ 50400: "Pacific/Kiritimati", } +type configGetter interface { + Get(string, string) (string, error) +} + // generic authenticated HTTP client for commands -func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, setAccept bool) *http.Client { +func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string, setAccept bool) (*http.Client, error) { var opts []api.ClientOption + + // We need to check and potentially add the unix socket roundtripper option + // before adding any other options, since if we are going to use the unix + // socket transport, it needs to form the base of the transport chain + // represented by invocations of opts... + // + // Another approach might be to change the signature of api.NewHTTPClient to + // take an explicit base http.RoundTripper as its first parameter (it + // currently defaults internally to http.DefaultTransport), or add another + // variant like api.NewHTTPClientWithBaseRoundTripper. But, the only caller + // which would use that non-default behavior is right here, and it doesn't + // seem worth the cognitive overhead everywhere else just to serve this one + // use case. + unixSocket, err := cfg.Get("", "http_unix_socket") + if err != nil { + return nil, err + } + if unixSocket != "" { + opts = append(opts, api.ClientOption(func(http.RoundTripper) http.RoundTripper { + return httpunix.NewRoundTripper(unixSocket) + })) + } + if verbose := os.Getenv("DEBUG"); verbose != "" { logTraffic := strings.Contains(verbose, "api") opts = append(opts, api.VerboseLog(io.ErrOut, logTraffic, io.IsStderrTTY())) @@ -64,7 +91,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", appVersion)), api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) { - hostname := ghinstance.NormalizeHostname(req.URL.Hostname()) + hostname := ghinstance.NormalizeHostname(getHost(req)) if token, err := cfg.Get(hostname, "oauth_token"); err == nil && token != "" { return fmt.Sprintf("token %s", token), nil } @@ -85,18 +112,23 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string if setAccept { opts = append(opts, api.AddHeaderFunc("Accept", func(req *http.Request) (string, error) { - // antiope-preview: Checks - accept := "application/vnd.github.antiope-preview+json" - // introduced for #2952: pr branch up to date status - accept += ", application/vnd.github.merge-info-preview+json" - if ghinstance.IsEnterprise(req.URL.Hostname()) { - // shadow-cat-preview: Draft pull requests - accept += ", application/vnd.github.shadow-cat-preview" + accept := "application/vnd.github.merge-info-preview+json" // PullRequest.mergeStateStatus + accept += ", application/vnd.github.nebula-preview" // visibility when RESTing repos into an org + if ghinstance.IsEnterprise(getHost(req)) { + accept += ", application/vnd.github.antiope-preview" // Commit.statusCheckRollup + accept += ", application/vnd.github.shadow-cat-preview" // PullRequest.isDraft } return accept, nil }), ) } - return api.NewHTTPClient(opts...) + return api.NewHTTPClient(opts...), nil +} + +func getHost(r *http.Request) string { + if r.Host != "" { + return r.Host + } + return r.URL.Hostname() } diff --git a/pkg/cmd/factory/http_test.go b/pkg/cmd/factory/http_test.go new file mode 100644 index 000000000..b02a914f8 --- /dev/null +++ b/pkg/cmd/factory/http_test.go @@ -0,0 +1,175 @@ +package factory + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "regexp" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewHTTPClient(t *testing.T) { + type args struct { + config configGetter + appVersion string + setAccept bool + } + tests := []struct { + name string + args args + envDebug string + host string + wantHeader map[string]string + wantStderr string + }{ + { + name: "github.com with Accept header", + args: args{ + config: tinyConfig{"github.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: true, + }, + host: "github.com", + wantHeader: map[string]string{ + "authorization": "token MYTOKEN", + "user-agent": "GitHub CLI v1.2.3", + "accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview", + }, + wantStderr: "", + }, + { + name: "github.com no Accept header", + args: args{ + config: tinyConfig{"github.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: false, + }, + host: "github.com", + wantHeader: map[string]string{ + "authorization": "token MYTOKEN", + "user-agent": "GitHub CLI v1.2.3", + "accept": "", + }, + wantStderr: "", + }, + { + name: "github.com no authentication token", + args: args{ + config: tinyConfig{"example.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: true, + }, + host: "github.com", + wantHeader: map[string]string{ + "authorization": "", + "user-agent": "GitHub CLI v1.2.3", + "accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview", + }, + wantStderr: "", + }, + { + name: "github.com in verbose mode", + args: args{ + config: tinyConfig{"github.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: true, + }, + host: "github.com", + envDebug: "api", + wantHeader: map[string]string{ + "authorization": "token MYTOKEN", + "user-agent": "GitHub CLI v1.2.3", + "accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview", + }, + wantStderr: heredoc.Doc(` + * Request at