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/README.md b/README.md index 6255b5d81..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 @@ -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/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 364784f9f..e21416b9f 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -150,28 +150,33 @@ type PullRequestFile struct { type ReviewRequests struct { Nodes []struct { - RequestedReviewer struct { - TypeName string `json:"__typename"` - Login string `json:"login"` - Name string `json:"name"` - Slug string `json:"slug"` - Organization struct { - Login string `json:"login"` - } - } + 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 { - if a.RequestedReviewer.TypeName == teamTypeName { - logins[i] = fmt.Sprintf("%s/%s", a.RequestedReviewer.Organization.Login, a.RequestedReviewer.Slug) - } else { - logins[i] = a.RequestedReviewer.Login - } + for i, r := range r.Nodes { + logins[i] = r.RequestedReviewer.LoginOrSlug() } return logins } @@ -391,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()) @@ -526,67 +531,23 @@ 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", + "requiresStrictStatusChecks", "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") } - 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/query_builder.go b/api/query_builder.go index 3dc97b826..c9ab62d13 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -216,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..7ee00b061 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -21,6 +21,7 @@ import ( "github.com/cli/cli/internal/run" "github.com/cli/cli/internal/update" "github.com/cli/cli/pkg/cmd/alias/expand" + "github.com/cli/cli/pkg/cmd/extensions" "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" @@ -92,14 +93,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,15 +133,27 @@ 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 := extensions.NewManager() + 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 + } } } @@ -264,7 +269,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..080876810 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ 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/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go.sum b/go.sum index 7f89f8dd8..aad4401e7 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/internal/config/config_file.go b/internal/config/config_file.go index bcf475f64..304ea3327 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -16,7 +16,10 @@ import ( const ( 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 @@ -38,27 +41,118 @@ func ConfigDir() string { } // If the path does not exist try migrating config from default paths - if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - autoMigrateConfigDir(path) + if !dirExists(path) { + _ = 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) { +func autoMigrateConfigDir(newPath string) error { path, err := os.UserHomeDir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateConfigDir(oldPath, newPath) - return + return migrateDir(oldPath, newPath) } path, err = homedir.Dir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateConfigDir(oldPath, newPath) + 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 { @@ -66,13 +160,9 @@ func dirExists(path string) bool { return err == nil && f.IsDir() } -var migrateConfigDir = func(oldPath, newPath string) { - if oldPath == newPath { - return - } - - _ = os.MkdirAll(filepath.Dir(newPath), 0755) - _ = os.Rename(oldPath, newPath) +func fileExists(path string) bool { + f, err := os.Stat(path) + return err == nil && !f.IsDir() } func ConfigFile() string { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 188f10ff0..4c35f24f9 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -152,6 +152,8 @@ func Test_parseConfigFile(t *testing.T) { } func Test_ConfigDir(t *testing.T) { + tempDir := t.TempDir() + tests := []struct { name string onlyWindows bool @@ -159,63 +161,63 @@ func Test_ConfigDir(t *testing.T) { output string }{ { - name: "no envVars", + name: "HOME/USERPROFILE specified", env: map[string]string{ "GH_CONFIG_DIR": "", "XDG_CONFIG_HOME": "", "AppData": "", - "USERPROFILE": "", - "HOME": "", + "USERPROFILE": tempDir, + "HOME": tempDir, }, - output: ".config/gh", + output: filepath.Join(tempDir, ".config", "gh"), }, { name: "GH_CONFIG_DIR specified", env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "XDG_CONFIG_HOME specified", env: map[string]string{ - "XDG_CONFIG_HOME": "/tmp", + "XDG_CONFIG_HOME": tempDir, }, - output: "/tmp/gh", + output: filepath.Join(tempDir, "gh"), }, { name: "GH_CONFIG_DIR and XDG_CONFIG_HOME specified", env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", - "XDG_CONFIG_HOME": "/tmp", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "XDG_CONFIG_HOME": tempDir, }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "AppData specified", onlyWindows: true, env: map[string]string{ - "AppData": "/tmp/", + "AppData": tempDir, }, - output: "/tmp/GitHub CLI", + output: filepath.Join(tempDir, "GitHub CLI"), }, { name: "GH_CONFIG_DIR and AppData specified", onlyWindows: true, env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", - "AppData": "/tmp", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "AppData": tempDir, }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "XDG_CONFIG_HOME and AppData specified", onlyWindows: true, env: map[string]string{ - "XDG_CONFIG_HOME": "/tmp", - "AppData": "/tmp", + "XDG_CONFIG_HOME": tempDir, + "AppData": tempDir, }, - output: "/tmp/gh", + output: filepath.Join(tempDir, "gh"), }, } @@ -227,22 +229,25 @@ func Test_ConfigDir(t *testing.T) { if tt.env != nil { for k, v := range tt.env { old := os.Getenv(k) - os.Setenv(k, filepath.FromSlash(v)) + os.Setenv(k, v) defer os.Setenv(k, old) } } - defer stubMigrateConfigDir()() - assert.Equal(t, filepath.FromSlash(tt.output), 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) - defer stubMigrateConfigDir()() cfg := NewFromString(`pager: less`) err := cfg.Write() @@ -264,7 +269,8 @@ func Test_configFile_Write_toDisk(t *testing.T) { } } -func Test_autoMigrateConfigDir_noMigration(t *testing.T) { +func Test_autoMigrateConfigDir_noMigration_notExist(t *testing.T) { + homeDir := t.TempDir() migrateDir := t.TempDir() homeEnvVar := "HOME" @@ -272,10 +278,11 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, "/nonexistent-dir") + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateConfigDir(migrateDir) + err := autoMigrateConfigDir(migrateDir) + assert.Equal(t, errNotExist, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -283,17 +290,21 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { } func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { - migrateDir := t.TempDir() + 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, migrateDir) + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateConfigDir(migrateDir) + err = autoMigrateConfigDir(migrateDir) + assert.Equal(t, errSamePath, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -301,31 +312,240 @@ func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { } func Test_autoMigrateConfigDir_migration(t *testing.T) { - defaultDir := t.TempDir() - dd := filepath.Join(defaultDir, ".config", "gh") + homeDir := t.TempDir() migrateDir := t.TempDir() - md := filepath.Join(migrateDir, ".config", "gh") + 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, defaultDir) + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - err := os.MkdirAll(dd, 0777) + err := os.MkdirAll(homeConfigDir, 0755) assert.NoError(t, err) - f, err := ioutil.TempFile(dd, "") + f, err := ioutil.TempFile(homeConfigDir, "") assert.NoError(t, err) f.Close() - autoMigrateConfigDir(md) + err = autoMigrateConfigDir(migrateConfigDir) + assert.NoError(t, err) - _, err = ioutil.ReadDir(dd) + _, err = ioutil.ReadDir(homeConfigDir) assert.True(t, os.IsNotExist(err)) - files, err := ioutil.ReadDir(md) + 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/testing.go b/internal/config/testing.go index 1908ac30e..31a5fb2a8 100644 --- a/internal/config/testing.go +++ b/internal/config/testing.go @@ -62,11 +62,3 @@ func stubConfig(main, hosts string) func() { ReadConfigFile = orig } } - -func stubMigrateConfigDir() func() { - orig := migrateConfigDir - migrateConfigDir = func(_, _ string) {} - return func() { - migrateConfigDir = orig - } -} 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/extensions/command.go b/pkg/cmd/extensions/command.go new file mode 100644 index 000000000..298787dcb --- /dev/null +++ b/pkg/cmd/extensions/command.go @@ -0,0 +1,77 @@ +package extensions + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +func NewCmdExtensions(io *iostreams.IOStreams) *cobra.Command { + m := NewManager() + + extCmd := cobra.Command{ + Use: "extensions", + Short: "Manage gh extensions", + } + + 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") + } + for _, c := range cmds { + name := filepath.Base(c) + parts := strings.SplitN(name, "-", 2) + fmt.Fprintf(io.Out, "%s %s\n", parts[0], parts[1]) + } + return nil + }, + }, + &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 !strings.HasPrefix(repo.RepoName(), "gh-") { + return errors.New("the repository name must start with `gh-`") + } + protocol := "https" // TODO: respect user's preferred protocol + return m.Install(ghrepo.FormatRemoteURL(repo, protocol), io.Out, io.ErrOut) + }, + }, + &cobra.Command{ + Use: "upgrade", + Short: "Upgrade installed extensions", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return m.Upgrade(io.Out, io.ErrOut) + }, + }, + ) + + extCmd.Hidden = true + return &extCmd +} diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go new file mode 100644 index 000000000..c3988b483 --- /dev/null +++ b/pkg/cmd/extensions/manager.go @@ -0,0 +1,121 @@ +package extensions + +import ( + "errors" + "fmt" + "io" + "io/ioutil" + "os" + "os/exec" + "path" + "path/filepath" + "strings" + + "github.com/cli/cli/internal/config" + "github.com/cli/safeexec" +) + +type Manager struct { + dataDir func() string + lookPath func(string) (string, error) +} + +func NewManager() *Manager { + return &Manager{ + dataDir: config.ConfigDir, + lookPath: safeexec.LookPath, + } +} + +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 := "gh-" + args[0] + forwardArgs := args[1:] + + for _, e := range m.List() { + if filepath.Base(e) == extName { + exe = e + break + } + } + if exe == "" { + return false, nil + } + + // TODO: parse the shebang on Windows and invoke the correct interpreter instead of invoking directly + externalCmd := exec.Command(exe, forwardArgs...) + externalCmd.Stdin = stdin + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + return true, externalCmd.Run() +} + +func (m *Manager) List() []string { + dir := m.installDir() + entries, err := ioutil.ReadDir(dir) + if err != nil { + return nil + } + + var results []string + for _, f := range entries { + if !strings.HasPrefix(f.Name(), "gh-") || !(f.IsDir() || f.Mode()&os.ModeSymlink != 0) { + continue + } + results = append(results, filepath.Join(dir, f.Name(), f.Name())) + } + 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 := exec.Command(exe, "clone", cloneURL, targetDir) + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + return externalCmd.Run() +} + +func (m *Manager) Upgrade(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") + } + + for _, f := range exts { + fmt.Fprintf(stdout, "[%s]: ", filepath.Base(f)) + dir := filepath.Dir(f) + externalCmd := exec.Command(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 + } + } + return err +} + +func (m *Manager) installDir() string { + return filepath.Join(m.dataDir(), "extensions") +} diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index e9023c852..eec60bc56 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -6,6 +6,8 @@ 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" @@ -14,11 +16,93 @@ import ( ) 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 + } + 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), nil + } +} + +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 +114,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..1bf8702c8 --- /dev/null +++ b/pkg/cmd/factory/default_test.go @@ -0,0 +1,391 @@ +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") + if tt.config != nil { + f.Config = func() (config.Config, error) { + 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") + if tt.config != nil { + f.Config = func() (config.Config, error) { + 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..23534a0d3 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -8,7 +8,6 @@ import ( "time" "github.com/cli/cli/api" - "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/iostreams" ) @@ -53,8 +52,12 @@ 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 { var opts []api.ClientOption if verbose := os.Getenv("DEBUG"); verbose != "" { logTraffic := strings.Contains(verbose, "api") @@ -64,7 +67,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,13 +88,10 @@ 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 + 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 }), @@ -100,3 +100,10 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string return api.NewHTTPClient(opts...) } + +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..6172d846c --- /dev/null +++ b/pkg/cmd/factory/http_test.go @@ -0,0 +1,174 @@ +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", + }, + 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", + }, + 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", + }, + wantStderr: heredoc.Doc(` + * Request at