From c7e8e1d44f67a02b54f9bce8460abe25c602509d Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 30 Sep 2025 08:22:08 -0600 Subject: [PATCH 1/3] Add Windows ConPTY support This adds full Windows support using ConPTY (Windows Pseudo Console API). Key changes include: - New Pty and Tty interfaces to abstract Unix *os.File and Windows handles - Windows-specific implementation using ConPTY API - Updated API to return interfaces instead of concrete *os.File types - Cross-platform window size management - Process lifecycle management for Windows - Updated golangci-lint configuration to version 2 format - Modernized Go version support (1.21.5) Co-Authored-By: Nathan Rijksen Co-Authored-By: photostorm Co-Authored-By: Larry Clapp Co-Authored-By: Guillaume J. Charmes --- .golangci.yml | 469 +++++++++++++--------------------------- README.md | 4 +- cmd_windows.go | 277 ++++++++++++++++++++++++ doc.go | 44 +++- fd_helper_other_test.go | 5 +- go.mod | 4 +- go.sum | 2 + helpers_test.go | 5 +- io_test.go | 17 +- pty_unsupported.go | 4 +- pty_windows.go | 186 ++++++++++++++++ run.go | 45 +--- run_unix.go | 69 ++++++ run_windows.go | 237 ++++++++++++++++++++ start.go | 25 --- start_windows.go | 19 -- test_crosscompile.sh | 4 +- winsize.go | 17 +- winsize_unix.go | 16 +- winsize_unsupported.go | 23 -- winsize_windows.go | 86 ++++++++ 21 files changed, 1087 insertions(+), 471 deletions(-) create mode 100644 cmd_windows.go create mode 100644 go.sum create mode 100644 pty_windows.go create mode 100644 run_unix.go create mode 100644 run_windows.go delete mode 100644 start.go delete mode 100644 start_windows.go delete mode 100644 winsize_unsupported.go create mode 100644 winsize_windows.go diff --git a/.golangci.yml b/.golangci.yml index f023e0f..18ad7ce 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,324 +1,153 @@ ---- -# Reference: https://golangci-lint.run/usage/configuration/ +version: "2" run: - timeout: 5m - # modules-download-mode: vendor - - # Include test files. tests: true - - skip-dirs: [] - - skip-files: [] - -output: - # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number". - format: colored-line-number - print-issued-lines: true - print-linter-name: true - -# Linter specific settings. See below in the `linter.enable` section for details on what each linter is doing. -linters-settings: - dogsled: - # Checks assignments with too many blank identifiers. Default is 2. - max-blank-identifiers: 2 - - dupl: - # Tokens count to trigger issue. - threshold: 150 - - errcheck: - # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. - # Enabled as this is often overlooked by developers. - check-type-assertions: true - # Report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`. - # Disabled as we consider that if the developer did type `_`, it was on purpose. - # Note that while this isn't enforced by the linter, each and every case of ignored error should - # be accompanied with a comment explaining why that error is being discarded. - check-blank: false - - exhaustive: - # Indicates that switch statements are to be considered exhaustive if a - # 'default' case is present, even if all enum members aren't listed in the - # switch. - default-signifies-exhaustive: false - - funlen: - # funlen checks the number of lines/statements in a function. - # While is is always best to keep functions short for readability, maintainability and testing, - # the default are a bit too strict (60 lines / 40 statements), increase it to be more flexible. - lines: 160 - statements: 70 - - # NOTE: We don't set `gci` for import order as it supports only one prefix. Use `goimports.local-prefixes` instead. - - gocognit: - # Minimal code complexity to report, defaults to 30 in gocognit, defaults 10 in golangci. - # Use 15 as it allows for some flexibility while preventing too much complexity. - # NOTE: Similar to gocyclo. - min-complexity: 35 - - nestif: - # Minimal complexity of if statements to report. - min-complexity: 8 - - goconst: - # Minimal length of string constant. - min-len: 4 - # Minimal occurrences count to trigger. - # Increase the default from 3 to 5 as small number of const usage can reduce readability instead of improving it. - min-occurrences: 5 - - gocritic: - # Which checks should be disabled; can't be combined with 'enabled-checks'. - # See https://go-critic.github.io/overview#checks-overview - # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` - disabled-checks: - - hugeParam # Very strict check on the size of variables being copied. Too strict for most developer. - # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. - # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". - enabled-tags: - - diagnostic - - style - - opinionated - - performance - settings: - rangeValCopy: - sizeThreshold: 1024 # Increase the allowed copied bytes in range. - - cyclop: - max-complexity: 35 - - gocyclo: - # Similar check as gocognit. - # NOTE: We might be able to remove this linter as it is redundant with gocyclo. It is in golangci-lint, so we keep it for now. - min-complexity: 35 - - godot: - # Check all top-level comments, not only declarations. - check-all: true - - gofmt: - # simplify code: gofmt with `-s` option. - simplify: true - - # NOTE: the goheader settings are set per-project. - - goimports: - # Put imports beginning with prefix after 3rd-party packages. - # It's a comma-separated list of prefixes. - local-prefixes: "github.com/creack/pty" - - golint: - # Minimal confidence for issues, default is 0.8. - min-confidence: 0.8 - - gosimple: - # Select the Go version to target. The default is '1.13'. - go: "1.18" - # https://staticcheck.io/docs/options#checks - checks: ["all"] - - gosec: - - govet: - # Enable all available checks from go vet. - enable-all: false - # Report about shadowed variables. - check-shadowing: true - - # NOTE: depguard is disabled as it is very slow and made redundant by gomodguard. - - lll: - # Make sure everyone is on the same level, fix the tab width to go's default. - tab-width: 8 - # Increase the default max line length to give more flexibility. Forcing newlines can reduce readability instead of improving it. - line-length: 180 - - misspell: - locale: US - ignore-words: - - nakedret: - # Make an issue if func has more lines of code than this setting and it has naked returns; default is 30. - # NOTE: Consider setting this to 1 to prevent naked returns. - max-func-lines: 30 - - nolintlint: - # Prevent ununsed directive to avoid stale comments. - allow-unused: false - # Require an explanation of nonzero length after each nolint directive. - require-explanation: true - # Exclude following linters from requiring an explanation. - # NOTE: It is strongly discouraged to put anything in there. - allow-no-explanation: [] - # Enable to require nolint directives to mention the specific linter being suppressed. This ensurce the developer understand the reason being the error. - require-specific: true - - prealloc: - # NOTE: For most programs usage of prealloc will be a premature optimization. - # Keep thing simple, pre-alloc what is obvious and profile the program for more complex scenarios. - # - simple: true # Checkonly on simple loops that have no returns/breaks/continues/gotos in them. - range-loops: true # Check range loops, true by default - for-loops: false # Check suggestions on for loops, false by default - - rowserrcheck: - packages: [] - - staticcheck: - # Select the Go version to target. The default is '1.13'. - go: "1.18" - # https://staticcheck.io/docs/options#checks - checks: ["all"] - - stylecheck: - # Select the Go version to target. The default is '1.13'. - go: "1.18" - # https://staticcheck.io/docs/options#checks - checks: ["all"] # "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"] - - tagliatelle: - # Check the struck tag name case. - case: - # Use the struct field name to check the name of the struct tag. - use-field-name: false - rules: - # Any struct tag type can be used. - # support string case: `camel`, `pascal`, `kebab`, `snake`, `goCamel`, `goPascal`, `goKebab`, `goSnake`, `upper`, `lower` - json: snake - firestore: camel - yaml: camel - xml: camel - bson: camel - avro: snake - mapstructure: kebab - envconfig: upper - - unparam: - # Don't create an error if an exported code have static params being used. It is often expected in libraries. - # NOTE: It would be nice if this linter would differentiate between a main package and a lib. - check-exported: true - - unused: {} - - whitespace: - multi-if: false # Enforces newlines (or comments) after every multi-line if statement - multi-func: false # Enforces newlines (or comments) after every multi-line function signature - -# Run `golangci-lint help linters` to get the full list of linter with their description. linters: - disable-all: true - # NOTE: enable-all is deprecated because too many people don't pin versions... - # We still require explicit documentation on why some linters are disabled. - # disable: - # - depguard # Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false] - # - exhaustivestruct # Checks if all struct's fields are initialized [fast: true, auto-fix: false] - # - forbidigo # Forbids identifiers [fast: true, auto-fix: false] - # - gci # Gci control golang package import order and make it always deterministic. [fast: true, auto-fix: true] - # - godox # Tool for detection of FIXME, TODO and other comment keywords [fast: true, auto-fix: false] - # - goerr113 # Golang linter to check the errors handling expressions [fast: true, auto-fix: false] - # - golint # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: false, auto-fix: false] - # - gomnd # An analyzer to detect magic numbers. [fast: true, auto-fix: false] - # - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. [fast: true, auto-fix: false] - # - interfacer # Linter that suggests narrower interface types [fast: false, auto-fix: false] - # - maligned # Tool to detect Go structs that would take less memory if their fields were sorted [fast: false, auto-fix: false] - # - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity [fast: true, auto-fix: false] - # - scopelint # Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false] - # - wrapcheck # Checks that errors returned from external packages are wrapped [fast: false, auto-fix: false] - # - wsl # Whitespace Linter - Forces you to use empty lines! [fast: true, auto-fix: false] - - # disable-reasons: - # - depguard # Checks whitelisted/blacklisted import path, but runs way too slow. Not that useful. - # - exhaustivestruct # Good concept, but not mature enough (errors on not assignable fields like locks) and too noisy when using AWS SDK as most fields are unused. - # - forbidigo # Great idea, but too strict out of the box. Probably will re-enable soon. - # - gci # Conflicts with goimports/gofumpt. - # - godox # Don't fail when finding TODO, FIXME, etc. - # - goerr113 # Too many false positives. - # - golint # Deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner. Replaced by revive. - # - gomnd # Checks for magic numbers. Disabled due to too many false positives not configurable (03/01/2020 v1.23.7). - # - gomoddirectives # Doesn't support //nolint to whitelist. - # - interfacer # Deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner. - # - maligned # Deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner. Replaced by govet 'fieldalignment'. - # - nlreturn # Actually reduces readability in most cases. - # - scopelint # Deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner. Replaced by exportloopref. - # - wrapcheck # Good concept, but always warns for http coded errors. Need to re-enable and whitelist our error package. - # - wsl # Forces to add newlines around blocks. Lots of false positives, not that useful. - + default: none enable: - - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers [fast: true, auto-fix: false] - - bodyclose # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false] - - cyclop # checks function and package cyclomatic complexity [fast: false, auto-fix: false] - - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false] - - dupl # Tool for code clone detection [fast: true, auto-fix: false] - - durationcheck # check for two durations multiplied together [fast: false, auto-fix: false] - - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: false, auto-fix: false] - - errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. [fast: false, auto-fix: false] - - errorlint # go-errorlint is a source code linter for Go software that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. [fast: false, auto-fix: false] - - exhaustive # check exhaustiveness of enum switch statements [fast: false, auto-fix: false] - - exportloopref # checks for pointers to enclosing loop variables [fast: false, auto-fix: false] - - forcetypeassert # finds forced type assertions [fast: true, auto-fix: false] - - funlen # Tool for detection of long functions [fast: true, auto-fix: false] - - gochecknoglobals # check that no global variables exist [fast: true, auto-fix: false] - - gochecknoinits # Checks that no init functions are present in Go code [fast: true, auto-fix: false] - - gocognit # Computes and checks the cognitive complexity of functions [fast: true, auto-fix: false] - - goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false] - - gocritic # Provides many diagnostics that check for bugs, performance and style issues. [fast: false, auto-fix: false] - - gocyclo # Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false] - - godot # Check if comments end in a period [fast: true, auto-fix: true] - - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true] - - gofumpt # Gofumpt checks whether code was gofumpt-ed. [fast: true, auto-fix: true] - - goheader # Checks is file header matches to pattern [fast: true, auto-fix: false] - - goimports # Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true] - - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. [fast: true, auto-fix: false] - - goprintffuncname # Checks that printf-like functions are named with `f` at the end [fast: true, auto-fix: false] - - gosec # (gas): Inspects source code for security problems [fast: false, auto-fix: false] - - gosimple # (megacheck): Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false] - - govet # (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false] - - importas # Enforces consistent import aliases [fast: false, auto-fix: false] - - ineffassign # Detects when assignments to existing variables are not used [fast: true, auto-fix: false] - - lll # Reports long lines [fast: true, auto-fix: false] - - makezero # Finds slice declarations with non-zero initial length [fast: false, auto-fix: false] - - misspell # Finds commonly misspelled English words in comments [fast: true, auto-fix: true] - - nakedret # Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false] - - nestif # Reports deeply nested if statements [fast: true, auto-fix: false] - - nilerr # Finds the code that returns nil even if it checks that the error is not nil. [fast: false, auto-fix: false] - - noctx # noctx finds sending http request without context.Context [fast: false, auto-fix: false] - - nolintlint # Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: false] - - paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test [fast: true, auto-fix: false] - - prealloc # Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false] - - predeclared # find code that shadows one of Go's predeclared identifiers [fast: true, auto-fix: false] - - promlinter # Check Prometheus metrics naming via promlint [fast: true, auto-fix: false] - - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. [fast: false, auto-fix: false] - # Disabled due to generic. Work in progress upstream. - # - rowserrcheck # checks whether Err of rows is checked successfully [fast: false, auto-fix: false] - # Disabled due to generic. Work in progress upstream. - # - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. [fast: false, auto-fix: false] - - staticcheck # (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false] - - stylecheck # Stylecheck is a replacement for golint [fast: false, auto-fix: false] - # Disabled due to generic. Work in progress upstream. - # - tagliatelle # Checks the struct tags. [fast: true, auto-fix: false] - # - testpackage # linter that makes you use a separate _test package [fast: true, auto-fix: false] - - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers [fast: false, auto-fix: false] - - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes [fast: false, auto-fix: false] - - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false] - - unconvert # Remove unnecessary type conversions [fast: false, auto-fix: false] - - unparam # Reports unused function parameters [fast: false, auto-fix: false] - # Disabled due to way too many false positive in go1.20. - # - unused # (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false] - # Disabled due to generic. Work in progress upstream. - # - wastedassign # wastedassign finds wasted assignment statements. [fast: false, auto-fix: false] - - whitespace # Tool for detection of leading and trailing whitespace [fast: true, auto-fix: true] - -issues: - exclude: - # Allow shadowing of 'err'. - - 'shadow: declaration of "err" shadows declaration' - # Allow shadowing of `ctx`. - - 'shadow: declaration of "ctx" shadows declaration' - # Maximum issues count per one linter. Set to 0 to disable. Default is 50. - max-per-linter: 10 - # Disable default excludes. Always be explicit on what we exclude. - exclude-use-default: false - # Exclude some linters from running on tests files. - exclude-rules: [] + - asciicheck + - bodyclose + - copyloopvar + - cyclop + - dogsled + - dupl + - durationcheck + - errcheck + - errname + - errorlint + - exhaustive + - forcetypeassert + - funlen + - gochecknoglobals + - gochecknoinits + - gocognit + - goconst + - gocritic + - gocyclo + - godot + - goheader + - gomodguard + - goprintffuncname + - gosec + - govet + - importas + - ineffassign + - intrange + - lll + - makezero + - misspell + - nakedret + - nestif + - nilerr + - noctx + - nolintlint + - paralleltest + - prealloc + - predeclared + - promlinter + - staticcheck + - thelper + - tparallel + - unconvert + - unparam + - wastedassign + - whitespace + settings: + cyclop: + max-complexity: 35 + dogsled: + max-blank-identifiers: 2 + dupl: + threshold: 150 + errcheck: + check-type-assertions: true + check-blank: false + exhaustive: + default-signifies-exhaustive: false + funlen: + lines: 160 + statements: 70 + gocognit: + min-complexity: 35 + goconst: + min-len: 4 + min-occurrences: 5 + gocritic: + disabled-checks: + - hugeParam + enabled-tags: + - diagnostic + - style + - opinionated + - performance + settings: + rangeValCopy: + sizeThreshold: 1024 + gocyclo: + min-complexity: 35 + govet: + enable-all: false + lll: + line-length: 180 + tab-width: 8 + misspell: + locale: US + nakedret: + max-func-lines: 30 + nestif: + min-complexity: 8 + nolintlint: + require-explanation: true + require-specific: true + allow-unused: false + prealloc: + simple: true + range-loops: true + for-loops: false + staticcheck: + checks: + - all + tagliatelle: + case: + rules: + avro: snake + bson: camel + envconfig: upper + firestore: camel + json: snake + mapstructure: kebab + xml: camel + yaml: camel + use-field-name: false + unparam: + check-exported: true + whitespace: + multi-if: false + multi-func: false + exclusions: + generated: lax + rules: + - path: (.+)\.go$ + text: 'shadow: declaration of "err" shadows declaration' + - path: (.+)\.go$ + text: 'shadow: declaration of "ctx" shadows declaration' + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - gofumpt + - goimports + settings: + gofmt: + simplify: true + goimports: + local-prefixes: + - github.com/creack/pty + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/README.md b/README.md index b6a1cf5..0d0ab1e 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # pty -Pty is a Go package for using unix pseudo-terminals. +Pty is a Go package for using unix pseudo-terminals and windows ConPty. ## Install @@ -12,6 +12,8 @@ go get github.com/creack/pty Note that those examples are for demonstration purpose only, to showcase how to use the library. They are not meant to be used in any kind of production environment. If you want to **set deadlines to work** and `Close()` **interrupting** `Read()` on the returned `*os.File`, you will need to call `syscall.SetNonblock` manually. +__NOTE:__ This package requires `ConPty` support on windows platform, please make sure your windows system meet [these requirements](https://docs.microsoft.com/en-us/windows/console/createpseudoconsole#requirements) + ### Command ```go diff --git a/cmd_windows.go b/cmd_windows.go new file mode 100644 index 0000000..5e628f8 --- /dev/null +++ b/cmd_windows.go @@ -0,0 +1,277 @@ +//go:build windows +// +build windows + +package pty + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "unicode/utf16" + "unsafe" + + "golang.org/x/sys/windows" +) + +// copied from os/exec.Cmd for platform compatibility +// we need to use startupInfoEx for pty support, but os/exec.Cmd only have +// support for startupInfo on windows, so we have to rewrite some internal +// logic for windows while keep its behavior compatible with other platforms. + +// windowExecCmd represents an external command being prepared or run. +// +// A cmd cannot be reused after calling its Run, Output or CombinedOutput +// methods. +type windowExecCmd struct { + cmd *exec.Cmd + waitCalled bool + conPty *WindowsPty + conTty *WindowsTty + attrList *windows.ProcThreadAttributeListContainer +} + +func (c *windowExecCmd) close() error { + c.attrList.Delete() + _ = c.conPty.Close() + _ = c.conTty.Close() + + return nil +} + +func (c *windowExecCmd) argv() []string { + if len(c.cmd.Args) > 0 { + return c.cmd.Args + } + + return []string{c.cmd.Path} +} + +// +// Helpers for working with Windows. These are exact copies of the same utilities found in the go stdlib. +// + +func lookExtensions(path, dir string) (string, error) { + if filepath.Base(path) == path { + path = filepath.Join(".", path) + } + + if dir == "" { + return exec.LookPath(path) + } + + if filepath.VolumeName(path) != "" { + return exec.LookPath(path) + } + + if len(path) > 1 && os.IsPathSeparator(path[0]) { + return exec.LookPath(path) + } + + dirandpath := filepath.Join(dir, path) + + // We assume that LookPath will only add file extension. + lp, err := exec.LookPath(dirandpath) + if err != nil { + return "", err + } + + ext := strings.TrimPrefix(lp, dirandpath) + + return path + ext, nil +} + +func dedupEnvCase(caseInsensitive bool, env []string) []string { + // Construct the output in reverse order, to preserve the + // last occurrence of each key. + out := make([]string, 0, len(env)) + saw := make(map[string]bool, len(env)) + for n := len(env); n > 0; n-- { + kv := env[n-1] + + i := strings.Index(kv, "=") + if i == 0 { + // We observe in practice keys with a single leading "=" on Windows. + // TODO(#49886): Should we consume only the first leading "=" as part + // of the key, or parse through arbitrarily many of them until a non-"="? + i = strings.Index(kv[1:], "=") + 1 + } + if i < 0 { + if kv != "" { + // The entry is not of the form "key=value" (as it is required to be). + // Leave it as-is for now. + // TODO(#52436): should we strip or reject these bogus entries? + out = append(out, kv) + } + continue + } + k := kv[:i] + if caseInsensitive { + k = strings.ToLower(k) + } + if saw[k] { + continue + } + + saw[k] = true + out = append(out, kv) + } + + // Now reverse the slice to restore the original order. + for i := 0; i < len(out)/2; i++ { + j := len(out) - i - 1 + out[i], out[j] = out[j], out[i] + } + + return out +} + +func addCriticalEnv(env []string) []string { + for _, kv := range env { + eq := strings.Index(kv, "=") + if eq < 0 { + continue + } + k := kv[:eq] + if strings.EqualFold(k, "SYSTEMROOT") { + // We already have it. + return env + } + } + + return append(env, "SYSTEMROOT="+os.Getenv("SYSTEMROOT")) +} + +func execEnvDefault(sys *syscall.SysProcAttr) (env []string, err error) { + if sys == nil || sys.Token == 0 { + return syscall.Environ(), nil + } + + var block *uint16 + err = windows.CreateEnvironmentBlock(&block, windows.Token(sys.Token), false) + if err != nil { + return nil, err + } + + defer windows.DestroyEnvironmentBlock(block) + blockp := uintptr(unsafe.Pointer(block)) + + for { + + // find NUL terminator + end := unsafe.Pointer(blockp) + for *(*uint16)(end) != 0 { + end = unsafe.Pointer(uintptr(end) + 2) + } + + n := (uintptr(end) - uintptr(unsafe.Pointer(blockp))) / 2 + if n == 0 { + // environment block ends with empty string + break + } + + entry := (*[(1 << 30) - 1]uint16)(unsafe.Pointer(blockp))[:n:n] + env = append(env, string(utf16.Decode(entry))) + blockp += 2 * (uintptr(len(entry)) + 1) + } + return +} + +func createEnvBlock(envv []string) *uint16 { + if len(envv) == 0 { + return &utf16.Encode([]rune("\x00\x00"))[0] + } + length := 0 + for _, s := range envv { + length += len(s) + 1 + } + length += 1 + + b := make([]byte, length) + i := 0 + for _, s := range envv { + l := len(s) + copy(b[i:i+l], []byte(s)) + copy(b[i+l:i+l+1], []byte{0}) + i = i + l + 1 + } + copy(b[i:i+1], []byte{0}) + + return &utf16.Encode([]rune(string(b)))[0] +} + +func makeCmdLine(args []string) string { + var s string + for _, v := range args { + if s != "" { + s += " " + } + s += windows.EscapeArg(v) + } + return s +} + +func isSlash(c uint8) bool { + return c == '\\' || c == '/' +} + +func normalizeDir(dir string) (name string, err error) { + ndir, err := syscall.FullPath(dir) + if err != nil { + return "", err + } + if len(ndir) > 2 && isSlash(ndir[0]) && isSlash(ndir[1]) { + // dir cannot have \\server\share\path form + return "", syscall.EINVAL + } + return ndir, nil +} + +func volToUpper(ch int) int { + if 'a' <= ch && ch <= 'z' { + ch += 'A' - 'a' + } + return ch +} + +func joinExeDirAndFName(dir, p string) (name string, err error) { + if len(p) == 0 { + return "", syscall.EINVAL + } + if len(p) > 2 && isSlash(p[0]) && isSlash(p[1]) { + // \\server\share\path form + return p, nil + } + if len(p) > 1 && p[1] == ':' { + // has drive letter + if len(p) == 2 { + return "", syscall.EINVAL + } + if isSlash(p[2]) { + return p, nil + } else { + d, err := normalizeDir(dir) + if err != nil { + return "", err + } + if volToUpper(int(p[0])) == volToUpper(int(d[0])) { + return syscall.FullPath(d + "\\" + p[2:]) + } else { + return syscall.FullPath(p) + } + } + } else { + // no drive letter + d, err := normalizeDir(dir) + if err != nil { + return "", err + } + + if isSlash(p[0]) { + return windows.FullPath(d[:2] + p) + } else { + return windows.FullPath(d + "\\" + p) + } + } +} diff --git a/doc.go b/doc.go index 3c8b324..989e286 100644 --- a/doc.go +++ b/doc.go @@ -3,7 +3,8 @@ package pty import ( "errors" - "os" + "io" + "time" ) // ErrUnsupported is returned if a function is not @@ -11,6 +12,45 @@ import ( var ErrUnsupported = errors.New("unsupported") // Open a pty and its corresponding tty. -func Open() (pty, tty *os.File, err error) { +func Open() (Pty, Tty, error) { return open() } + +// FdHolder surfaces the Fd() method of the underlying handle. +type FdHolder interface { + Fd() uintptr +} + +// DeadlineHolder surfaces the SetDeadline() method to sets the read and write deadlines. +type DeadlineHolder interface { + SetDeadline(t time.Time) error +} + +// Pty for terminal control in current process. +// +// - For Unix systems, the real type is *os.File. +// - For Windows, the real type is a *WindowsPty for ConPTY handle. +type Pty interface { + // FdHolder is intended to resize / control ioctls of the TTY of the child process in current process. + FdHolder + + Name() string + + // WriteString is only used to identify Pty and Tty. + WriteString(s string) (n int, err error) + + io.ReadWriteCloser +} + +// Tty for data I/O in child process. +// +// - For Unix systems, the real type is *os.File. +// - For Windows, the real type is a *WindowsTty, which is a combination of two pipe file. +type Tty interface { + // FdHolder Fd only intended for manual InheritSize from Pty. + FdHolder + + Name() string + + io.ReadWriteCloser +} diff --git a/fd_helper_other_test.go b/fd_helper_other_test.go index 9cb5152..b82fceb 100644 --- a/fd_helper_other_test.go +++ b/fd_helper_other_test.go @@ -4,11 +4,10 @@ package pty import ( - "os" "testing" ) -func getNonBlockingFile(t *testing.T, file *os.File, path string) *os.File { +func getNonBlockingFile(t *testing.T, ptmx Pty, path string) Pty { t.Helper() - return file + return ptmx } diff --git a/go.mod b/go.mod index b099e3f..a22acd4 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/creack/pty -go 1.18 +go 1.21.5 + +require golang.org/x/sys v0.13.0 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..d4673ec --- /dev/null +++ b/go.sum @@ -0,0 +1,2 @@ +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/helpers_test.go b/helpers_test.go index fbc98b7..d712955 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -4,12 +4,11 @@ import ( "bytes" "fmt" "io" - "os" "testing" ) -// openCloses opens a pty/tty pair and stages the closing as part of the cleanup. -func openClose(t *testing.T) (pty, tty *os.File) { +// openClose opens a pty/tty pair and stages the closing as part of the cleanup. +func openClose(t *testing.T) (Pty, Tty) { t.Helper() pty, tty, err := Open() diff --git a/io_test.go b/io_test.go index 922f9c1..41e9021 100644 --- a/io_test.go +++ b/io_test.go @@ -34,11 +34,18 @@ func TestReadDeadline(t *testing.T) { t.Fatalf("Error: set non block: %s", err) } - if err := ptmx.SetDeadline(time.Now().Add(timeout / 10)); err != nil { + var ok bool + var ptmxd DeadlineHolder + if ptmxd, ok = ptmx.(DeadlineHolder); !ok { + t.Fatalf("Unexpected ptmx type: missing deadline method (%T)", ptmx) + } + + err := ptmxd.SetDeadline(time.Now().Add(timeout / 10)) + if err != nil { if errors.Is(err, os.ErrNoDeadline) { - t.Skipf("Deadline is not supported on %s/%s.", runtime.GOOS, runtime.GOARCH) + t.Skipf("deadline is not supported on %s/%s", runtime.GOOS, runtime.GOARCH) } else { - t.Fatalf("Error: set deadline: %s.", err) + t.Fatalf("error: set deadline: %v\n", err) } } @@ -86,9 +93,7 @@ func TestReadClose(t *testing.T) { } // Open pty and setup watchdogs for graceful and not so graceful failure modes. -func prepare(t *testing.T) (ptmx *os.File, done func()) { - t.Helper() - +func prepare(t *testing.T) (ptmx Pty, done func()) { if runtime.GOOS == "darwin" { t.Log("creack/pty uses blocking i/o on darwin intentionally:") t.Log("> https://github.com/creack/pty/issues/52") diff --git a/pty_unsupported.go b/pty_unsupported.go index 0971dc7..e5c1a98 100644 --- a/pty_unsupported.go +++ b/pty_unsupported.go @@ -1,5 +1,5 @@ -//go:build !linux && !darwin && !freebsd && !dragonfly && !netbsd && !openbsd && !solaris && !zos -// +build !linux,!darwin,!freebsd,!dragonfly,!netbsd,!openbsd,!solaris,!zos +//go:build !linux && !darwin && !freebsd && !dragonfly && !netbsd && !openbsd && !solaris && !zos && !windows +// +build !linux,!darwin,!freebsd,!dragonfly,!netbsd,!openbsd,!solaris,!zos,!windows package pty diff --git a/pty_windows.go b/pty_windows.go new file mode 100644 index 0000000..7dbe189 --- /dev/null +++ b/pty_windows.go @@ -0,0 +1,186 @@ +//go:build windows +// +build windows + +package pty + +import ( + "fmt" + "os" + "syscall" + "time" + "unsafe" + + "golang.org/x/sys/windows" +) + +const ( + // Ref: https://pkg.go.dev/golang.org/x/sys/windows#pkg-constants + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE = 0x20016 +) + +type WindowsPty struct { + handle windows.Handle + r, w *os.File +} + +type WindowsTty struct { + handle windows.Handle + r, w *os.File +} + +var ( + // NOTE(security): As noted by the comment of syscall.NewLazyDLL and syscall.LoadDLL + // user need to call internal/syscall/windows/sysdll.Add("kernel32.dll") to make sure + // the kernel32.dll is loaded from windows system path. + // + // Ref: https://pkg.go.dev/syscall@go1.13?GOOS=windows#LoadDLL + kernel32DLL = windows.NewLazySystemDLL("kernel32.dll") + + // https://docs.microsoft.com/en-us/windows/console/createpseudoconsole + createPseudoConsole = kernel32DLL.NewProc("CreatePseudoConsole") + closePseudoConsole = kernel32DLL.NewProc("ClosePseudoConsole") + + resizePseudoConsole = kernel32DLL.NewProc("ResizePseudoConsole") + getConsoleScreenBufferInfo = kernel32DLL.NewProc("GetConsoleScreenBufferInfo") +) + +func open() (_ Pty, _ Tty, err error) { + pr, consoleW, err := os.Pipe() + if err != nil { + return nil, nil, err + } + + consoleR, pw, err := os.Pipe() + if err != nil { + // Closing everything. Best effort. + _ = consoleW.Close() + _ = pr.Close() + return nil, nil, err + } + + var consoleHandle windows.Handle + + // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. + if err := procCreatePseudoConsole( + windows.Handle(consoleR.Fd()), + windows.Handle(consoleW.Fd()), + 0, + &consoleHandle); err != nil { + // Closing everything. Best effort. + _ = consoleW.Close() + _ = pr.Close() + _ = pw.Close() + _ = consoleR.Close() + return nil, nil, err + } + + return &WindowsPty{ + handle: consoleHandle, + r: pr, + w: pw, + }, &WindowsTty{ + handle: consoleHandle, + r: consoleR, + w: consoleW, + }, nil +} + +func (p *WindowsPty) Name() string { + return p.r.Name() +} + +func (p *WindowsPty) Fd() uintptr { + return uintptr(p.handle) +} + +func (p *WindowsPty) Read(data []byte) (int, error) { + return p.r.Read(data) +} + +func (p *WindowsPty) Write(data []byte) (int, error) { + return p.w.Write(data) +} + +func (p *WindowsPty) WriteString(s string) (int, error) { + return p.w.WriteString(s) +} + +func (p *WindowsPty) UpdateProcThreadAttribute(attrList *windows.ProcThreadAttributeListContainer) error { + if err := attrList.Update( + PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + unsafe.Pointer(p.handle), + unsafe.Sizeof(p.handle), + ); err != nil { + return fmt.Errorf("failed to update proc thread attributes for pseudo console: %w", err) + } + + return nil +} + +func (p *WindowsPty) Close() error { + // Best effort. + _ = p.r.Close() + _ = p.w.Close() + + if err := closePseudoConsole.Find(); err != nil { + return err + } + + _, _, err := closePseudoConsole.Call(uintptr(p.handle)) + return err +} + +func (p *WindowsPty) SetDeadline(value time.Time) error { + return os.ErrNoDeadline +} + +func (t *WindowsTty) Name() string { + return t.r.Name() +} + +func (t *WindowsTty) Fd() uintptr { + return uintptr(t.handle) +} + +func (t *WindowsTty) Read(p []byte) (int, error) { + return t.r.Read(p) +} + +func (t *WindowsTty) Write(p []byte) (int, error) { + return t.w.Write(p) +} + +func (t *WindowsTty) Close() error { + _ = t.r.Close() // Best effort. + return t.w.Close() +} + +func (t *WindowsTty) SetDeadline(value time.Time) error { + return os.ErrNoDeadline +} + +func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFlags uint32, consoleHandle *windows.Handle) error { + if err := createPseudoConsole.Find(); err != nil { + return err + } + + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := createPseudoConsole.Call( + (windowsCoord{X: 80, Y: 30}).Pack(), // Size: default 80x30 window. + uintptr(hInput), // Console input. + uintptr(hOutput), // Console output. + uintptr(dwFlags), // Console flags, currently only PSEUDOCONSOLE_INHERIT_CURSOR supported. + uintptr(unsafe.Pointer(consoleHandle)), // Console handler value return. + ) + + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + + // S_OK: 0 + return syscall.Errno(r0) + } + + return nil +} diff --git a/run.go b/run.go index 4755366..0b97b94 100644 --- a/run.go +++ b/run.go @@ -1,9 +1,7 @@ package pty import ( - "os" "os/exec" - "syscall" ) // Start assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, @@ -11,47 +9,6 @@ import ( // corresponding pty. // // Starts the process in a new session and sets the controlling terminal. -func Start(cmd *exec.Cmd) (*os.File, error) { +func Start(cmd *exec.Cmd) (Pty, error) { return StartWithSize(cmd, nil) } - -// StartWithAttrs assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, -// and c.Stderr, calls c.Start, and returns the File of the tty's -// corresponding pty. -// -// This will resize the pty to the specified size before starting the command if a size is provided. -// The `attrs` parameter overrides the one set in c.SysProcAttr. -// -// This should generally not be needed. Used in some edge cases where it is needed to create a pty -// without a controlling terminal. -func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (*os.File, error) { - pty, tty, err := Open() - if err != nil { - return nil, err - } - defer func() { _ = tty.Close() }() // Best effort. - - if sz != nil { - if err := Setsize(pty, sz); err != nil { - _ = pty.Close() // Best effort. - return nil, err - } - } - if c.Stdout == nil { - c.Stdout = tty - } - if c.Stderr == nil { - c.Stderr = tty - } - if c.Stdin == nil { - c.Stdin = tty - } - - c.SysProcAttr = attrs - - if err := c.Start(); err != nil { - _ = pty.Close() // Best effort. - return nil, err - } - return pty, err -} diff --git a/run_unix.go b/run_unix.go new file mode 100644 index 0000000..6d43291 --- /dev/null +++ b/run_unix.go @@ -0,0 +1,69 @@ +//go:build !windows +// +build !windows + +package pty + +import ( + "os/exec" + "syscall" +) + +// StartWithSize assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command. +// Starts the process in a new session and sets the controlling terminal. +func StartWithSize(c *exec.Cmd, sz *Winsize) (Pty, error) { + if c.SysProcAttr == nil { + c.SysProcAttr = &syscall.SysProcAttr{} + } + c.SysProcAttr.Setsid = true + c.SysProcAttr.Setctty = true + return StartWithAttrs(c, sz, c.SysProcAttr) +} + +// StartWithAttrs assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command if a size is provided. +// The `attrs` parameter overrides the one set in c.SysProcAttr. +// +// This should generally not be needed. Used in some edge cases where it is needed to create a pty +// without a controlling terminal. +func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, error) { + pty, tty, err := open() + if err != nil { + return nil, err + } + defer func() { + // always close tty fds since it's being used in another process + // but pty is kept to resize tty + _ = tty.Close() + }() + + if sz != nil { + if err := Setsize(pty, sz); err != nil { + _ = pty.Close() + return nil, err + } + } + if c.Stdout == nil { + c.Stdout = tty + } + if c.Stderr == nil { + c.Stderr = tty + } + if c.Stdin == nil { + c.Stdin = tty + } + + c.SysProcAttr = attrs + + if err := c.Start(); err != nil { + _ = pty.Close() + return nil, err + } + return pty, err +} diff --git a/run_windows.go b/run_windows.go new file mode 100644 index 0000000..e933a87 --- /dev/null +++ b/run_windows.go @@ -0,0 +1,237 @@ +//go:build windows +// +build windows + +package pty + +import ( + "errors" + "fmt" + "os" + "os/exec" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +// StartWithSize assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command. +// Starts the process in a new session and sets the controlling terminal. +func StartWithSize(c *exec.Cmd, sz *Winsize) (Pty, error) { + return StartWithAttrs(c, sz, c.SysProcAttr) +} + +// StartWithAttrs assigns a pseudo-terminal Tty to c.Stdin, c.Stdout, +// and c.Stderr, calls c.Start, and returns the File of the tty's +// corresponding Pty. +// +// This will resize the Pty to the specified size before starting the command if a size is provided. +// The `attrs` parameter overrides the one set in c.SysProcAttr. +// +// This should generally not be needed. Used in some edge cases where it is needed to create a pty +// without a controlling terminal. +func StartWithAttrs(c *exec.Cmd, sz *Winsize, attrs *syscall.SysProcAttr) (Pty, error) { + pty, tty, err := open() + if err != nil { + return nil, err + } + + defer func() { + // Unlike unix command exec, do not close tty unless error happened. + if err != nil { + _ = pty.Close() // Best effort. + } + }() + + if sz != nil { + if err := Setsize(pty, sz); err != nil { + return nil, err + } + } + + // Unlike unix command exec, do not set stdin/stdout/stderr. + + c.SysProcAttr = attrs + + // Do not use os/exec.Start since we need to append console handler to startup info. + + w := windowExecCmd{ + cmd: c, + waitCalled: false, + conPty: pty.(*WindowsPty), + conTty: tty.(*WindowsTty), + } + + if err := w.Start(); err != nil { + return nil, err + } + + return pty, err +} + +// Start the specified command but does not wait for it to complete. +// +// If Start returns successfully, the c.Process field will be set. +// +// The Wait method will return the exit code and release associated resources +// once the command exits. +func (c *windowExecCmd) Start() error { + if c.cmd.Process != nil { + return errors.New("exec: already started") + } + + argv0 := c.cmd.Path + var argv0p *uint16 + var argvp *uint16 + var dirp *uint16 + var err error + + sys := c.cmd.SysProcAttr + if sys == nil { + sys = &syscall.SysProcAttr{} + } + + if c.cmd.Env == nil { + c.cmd.Env, err = execEnvDefault(sys) + if err != nil { + return err + } + } + + var lp string + + lp, err = lookExtensions(c.cmd.Path, c.cmd.Dir) + if err != nil { + return err + } + + c.cmd.Path = lp + + if len(c.cmd.Dir) != 0 { + // Windows CreateProcess looks for argv0 relative to the current + // directory, and, only once the new process is started, it does + // Chdir(attr.Dir). We are adjusting for that difference here by + // making argv0 absolute. + + argv0, err = joinExeDirAndFName(c.cmd.Dir, c.cmd.Path) + if err != nil { + return err + } + } + + argv0p, err = syscall.UTF16PtrFromString(argv0) + if err != nil { + return err + } + + var cmdline string + + // Windows CreateProcess takes the command line as a single string: + // use attr.CmdLine if set, else build the command line by escaping + // and joining each argument with spaces. + if sys.CmdLine != "" { + cmdline = sys.CmdLine + } else { + cmdline = makeCmdLine(c.argv()) + } + + if len(cmdline) != 0 { + argvp, err = windows.UTF16PtrFromString(cmdline) + if err != nil { + return err + } + } + + if len(c.cmd.Dir) != 0 { + dirp, err = windows.UTF16PtrFromString(c.cmd.Dir) + if err != nil { + return err + } + } + + // Acquire the fork lock so that no other threads + // create new fds that are not yet close-on-exec + // before we fork. + syscall.ForkLock.Lock() + defer syscall.ForkLock.Unlock() + + siEx := new(windows.StartupInfoEx) + siEx.Flags = windows.STARTF_USESTDHANDLES + + if sys.HideWindow { + siEx.Flags |= syscall.STARTF_USESHOWWINDOW + siEx.ShowWindow = syscall.SW_HIDE + } + + pi := new(windows.ProcessInformation) + + // Need EXTENDED_STARTUPINFO_PRESENT as we're making use of the attribute list field. + flags := sys.CreationFlags | uint32(windows.CREATE_UNICODE_ENVIRONMENT) | windows.EXTENDED_STARTUPINFO_PRESENT + + c.attrList, err = windows.NewProcThreadAttributeList(1) + if err != nil { + return fmt.Errorf("failed to initialize process thread attribute list: %w", err) + } + + if c.conPty != nil { + if err = c.conPty.UpdateProcThreadAttribute(c.attrList); err != nil { + return err + } + } + + siEx.ProcThreadAttributeList = c.attrList.List() + siEx.Cb = uint32(unsafe.Sizeof(*siEx)) + + if sys.Token != 0 { + err = windows.CreateProcessAsUser( + windows.Token(sys.Token), + argv0p, + argvp, + nil, + nil, + false, + flags, + createEnvBlock(addCriticalEnv(dedupEnvCase(true, c.cmd.Env))), + dirp, + &siEx.StartupInfo, + pi, + ) + } else { + err = windows.CreateProcess( + argv0p, + argvp, + nil, + nil, + false, + flags, + createEnvBlock(addCriticalEnv(dedupEnvCase(true, c.cmd.Env))), + dirp, + &siEx.StartupInfo, + pi, + ) + } + if err != nil { + return err + } + + defer func() { + _ = windows.CloseHandle(pi.Thread) + }() + + c.cmd.Process, err = os.FindProcess(int(pi.ProcessId)) + if err != nil { + return err + } + + go c.waitProcess(c.cmd.Process) + + return nil +} + +func (c *windowExecCmd) waitProcess(process *os.Process) { + _, _ = process.Wait() + _ = c.close() +} diff --git a/start.go b/start.go deleted file mode 100644 index 9b51635..0000000 --- a/start.go +++ /dev/null @@ -1,25 +0,0 @@ -//go:build !windows -// +build !windows - -package pty - -import ( - "os" - "os/exec" - "syscall" -) - -// StartWithSize assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, -// and c.Stderr, calls c.Start, and returns the File of the tty's -// corresponding pty. -// -// This will resize the pty to the specified size before starting the command. -// Starts the process in a new session and sets the controlling terminal. -func StartWithSize(cmd *exec.Cmd, ws *Winsize) (*os.File, error) { - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = &syscall.SysProcAttr{} - } - cmd.SysProcAttr.Setsid = true - cmd.SysProcAttr.Setctty = true - return StartWithAttrs(cmd, ws, cmd.SysProcAttr) -} diff --git a/start_windows.go b/start_windows.go deleted file mode 100644 index 7e9530b..0000000 --- a/start_windows.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build windows -// +build windows - -package pty - -import ( - "os" - "os/exec" -) - -// StartWithSize assigns a pseudo-terminal tty os.File to c.Stdin, c.Stdout, -// and c.Stderr, calls c.Start, and returns the File of the tty's -// corresponding pty. -// -// This will resize the pty to the specified size before starting the command. -// Starts the process in a new session and sets the controlling terminal. -func StartWithSize(cmd *exec.Cmd, ws *Winsize) (*os.File, error) { - return nil, ErrUnsupported -} diff --git a/test_crosscompile.sh b/test_crosscompile.sh index 40df89a..890ad45 100755 --- a/test_crosscompile.sh +++ b/test_crosscompile.sh @@ -32,9 +32,7 @@ cross netbsd amd64 386 arm arm64 cross openbsd amd64 386 arm arm64 cross dragonfly amd64 cross solaris amd64 - -# Not expected to work but should still compile. -cross windows amd64 386 arm +cross windows amd64 386 arm # TODO: Fix compilation error on openbsd/arm. # TODO: Merge the solaris PR. diff --git a/winsize.go b/winsize.go index cfa3e5f..470db87 100644 --- a/winsize.go +++ b/winsize.go @@ -1,11 +1,17 @@ package pty -import "os" +// Winsize describes the terminal size. +type Winsize struct { + Rows uint16 // ws_row: Number of rows (in cells). + Cols uint16 // ws_col: Number of columns (in cells). + X uint16 // ws_xpixel: Width in pixels. + Y uint16 // ws_ypixel: Height in pixels. +} // InheritSize applies the terminal size of pty to tty. This should be run // in a signal handler for syscall.SIGWINCH to automatically resize the tty when // the pty receives a window size change notification. -func InheritSize(pty, tty *os.File) error { +func InheritSize(pty Pty, tty Tty) error { size, err := GetsizeFull(pty) if err != nil { return err @@ -15,10 +21,7 @@ func InheritSize(pty, tty *os.File) error { // Getsize returns the number of rows (lines) and cols (positions // in each line) in terminal t. -func Getsize(t *os.File) (rows, cols int, err error) { +func Getsize(t FdHolder) (rows, cols int, err error) { ws, err := GetsizeFull(t) - if err != nil { - return 0, 0, err - } - return int(ws.Rows), int(ws.Cols), nil + return int(ws.Rows), int(ws.Cols), err } diff --git a/winsize_unix.go b/winsize_unix.go index 8dbbcda..883a823 100644 --- a/winsize_unix.go +++ b/winsize_unix.go @@ -9,26 +9,18 @@ import ( "unsafe" ) -// Winsize describes the terminal size. -type Winsize struct { - Rows uint16 // ws_row: Number of rows (in cells). - Cols uint16 // ws_col: Number of columns (in cells). - X uint16 // ws_xpixel: Width in pixels. - Y uint16 // ws_ypixel: Height in pixels. -} - // Setsize resizes t to s. -func Setsize(t *os.File, ws *Winsize) error { +func Setsize(t FdHolder, ws *Winsize) error { //nolint:gosec // Expected unsafe pointer for Syscall call. - return ioctl(t, syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) + return ioctl(t.(*os.File), syscall.TIOCSWINSZ, uintptr(unsafe.Pointer(ws))) } // GetsizeFull returns the full terminal size description. -func GetsizeFull(t *os.File) (size *Winsize, err error) { +func GetsizeFull(t FdHolder) (size *Winsize, err error) { var ws Winsize //nolint:gosec // Expected unsafe pointer for Syscall call. - if err := ioctl(t, syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&ws))); err != nil { + if err := ioctl(t.(*os.File), syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&ws))); err != nil { return nil, err } return &ws, nil diff --git a/winsize_unsupported.go b/winsize_unsupported.go deleted file mode 100644 index 0d21099..0000000 --- a/winsize_unsupported.go +++ /dev/null @@ -1,23 +0,0 @@ -//go:build windows -// +build windows - -package pty - -import ( - "os" -) - -// Winsize is a dummy struct to enable compilation on unsupported platforms. -type Winsize struct { - Rows, Cols, X, Y uint16 -} - -// Setsize resizes t to s. -func Setsize(*os.File, *Winsize) error { - return ErrUnsupported -} - -// GetsizeFull returns the full terminal size description. -func GetsizeFull(*os.File) (*Winsize, error) { - return nil, ErrUnsupported -} diff --git a/winsize_windows.go b/winsize_windows.go new file mode 100644 index 0000000..c9a3111 --- /dev/null +++ b/winsize_windows.go @@ -0,0 +1,86 @@ +//go:build windows +// +build windows + +package pty + +import ( + "syscall" + "unsafe" +) + +// Types from golang.org/x/sys/windows. + +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#Coord +type windowsCoord struct { + X int16 + Y int16 +} + +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#SmallRect +type windowsSmallRect struct { + Left int16 + Top int16 + Right int16 + Bottom int16 +} + +// Ref: https://pkg.go.dev/golang.org/x/sys/windows#ConsoleScreenBufferInfo +type windowsConsoleScreenBufferInfo struct { + Size windowsCoord + CursorPosition windowsCoord + Attributes uint16 + Window windowsSmallRect + MaximumWindowSize windowsCoord +} + +func (c windowsCoord) Pack() uintptr { + return uintptr((int32(c.Y) << 16) | int32(c.X)) +} + +// Setsize resizes t to ws. +func Setsize(t FdHolder, ws *Winsize) error { + if err := resizePseudoConsole.Find(); err != nil { + return err + } + + // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := resizePseudoConsole.Call( + t.Fd(), + (windowsCoord{X: int16(ws.Cols), Y: int16(ws.Rows)}).Pack(), + ) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + + // S_OK: 0 + return syscall.Errno(r0) + } + + return nil +} + +// GetsizeFull returns the full terminal size description. +func GetsizeFull(t FdHolder) (size *Winsize, err error) { + if err := getConsoleScreenBufferInfo.Find(); err != nil { + return nil, err + } + + var info windowsConsoleScreenBufferInfo + // TODO: Check if it is expected to ignore `err` here. + r0, _, _ := getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) + if int32(r0) < 0 { + if r0&0x1fff0000 == 0x00070000 { + r0 &= 0xffff + } + + // S_OK: 0 + return nil, syscall.Errno(r0) + } + + return &Winsize{ + Rows: uint16(info.Window.Bottom - info.Window.Top + 1), + Cols: uint16(info.Window.Right - info.Window.Left + 1), + }, nil +} From c6dcb88dcc43adb2d28657d6d5068e7fd15cfca5 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 30 Sep 2025 09:42:14 -0600 Subject: [PATCH 2/3] fix: broken handler and clean up code --- .golangci.yml | 469 +++++++++++++++++++++++++++++++-------------- go.mod | 4 +- go.sum | 2 + io_test.go | 1 + pty_windows.go | 2 +- winsize_windows.go | 23 ++- 6 files changed, 345 insertions(+), 156 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 18ad7ce..f023e0f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,153 +1,324 @@ -version: "2" +--- +# Reference: https://golangci-lint.run/usage/configuration/ run: + timeout: 5m + # modules-download-mode: vendor + + # Include test files. tests: true + + skip-dirs: [] + + skip-files: [] + +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number". + format: colored-line-number + print-issued-lines: true + print-linter-name: true + +# Linter specific settings. See below in the `linter.enable` section for details on what each linter is doing. +linters-settings: + dogsled: + # Checks assignments with too many blank identifiers. Default is 2. + max-blank-identifiers: 2 + + dupl: + # Tokens count to trigger issue. + threshold: 150 + + errcheck: + # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. + # Enabled as this is often overlooked by developers. + check-type-assertions: true + # Report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`. + # Disabled as we consider that if the developer did type `_`, it was on purpose. + # Note that while this isn't enforced by the linter, each and every case of ignored error should + # be accompanied with a comment explaining why that error is being discarded. + check-blank: false + + exhaustive: + # Indicates that switch statements are to be considered exhaustive if a + # 'default' case is present, even if all enum members aren't listed in the + # switch. + default-signifies-exhaustive: false + + funlen: + # funlen checks the number of lines/statements in a function. + # While is is always best to keep functions short for readability, maintainability and testing, + # the default are a bit too strict (60 lines / 40 statements), increase it to be more flexible. + lines: 160 + statements: 70 + + # NOTE: We don't set `gci` for import order as it supports only one prefix. Use `goimports.local-prefixes` instead. + + gocognit: + # Minimal code complexity to report, defaults to 30 in gocognit, defaults 10 in golangci. + # Use 15 as it allows for some flexibility while preventing too much complexity. + # NOTE: Similar to gocyclo. + min-complexity: 35 + + nestif: + # Minimal complexity of if statements to report. + min-complexity: 8 + + goconst: + # Minimal length of string constant. + min-len: 4 + # Minimal occurrences count to trigger. + # Increase the default from 3 to 5 as small number of const usage can reduce readability instead of improving it. + min-occurrences: 5 + + gocritic: + # Which checks should be disabled; can't be combined with 'enabled-checks'. + # See https://go-critic.github.io/overview#checks-overview + # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` + disabled-checks: + - hugeParam # Very strict check on the size of variables being copied. Too strict for most developer. + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + enabled-tags: + - diagnostic + - style + - opinionated + - performance + settings: + rangeValCopy: + sizeThreshold: 1024 # Increase the allowed copied bytes in range. + + cyclop: + max-complexity: 35 + + gocyclo: + # Similar check as gocognit. + # NOTE: We might be able to remove this linter as it is redundant with gocyclo. It is in golangci-lint, so we keep it for now. + min-complexity: 35 + + godot: + # Check all top-level comments, not only declarations. + check-all: true + + gofmt: + # simplify code: gofmt with `-s` option. + simplify: true + + # NOTE: the goheader settings are set per-project. + + goimports: + # Put imports beginning with prefix after 3rd-party packages. + # It's a comma-separated list of prefixes. + local-prefixes: "github.com/creack/pty" + + golint: + # Minimal confidence for issues, default is 0.8. + min-confidence: 0.8 + + gosimple: + # Select the Go version to target. The default is '1.13'. + go: "1.18" + # https://staticcheck.io/docs/options#checks + checks: ["all"] + + gosec: + + govet: + # Enable all available checks from go vet. + enable-all: false + # Report about shadowed variables. + check-shadowing: true + + # NOTE: depguard is disabled as it is very slow and made redundant by gomodguard. + + lll: + # Make sure everyone is on the same level, fix the tab width to go's default. + tab-width: 8 + # Increase the default max line length to give more flexibility. Forcing newlines can reduce readability instead of improving it. + line-length: 180 + + misspell: + locale: US + ignore-words: + + nakedret: + # Make an issue if func has more lines of code than this setting and it has naked returns; default is 30. + # NOTE: Consider setting this to 1 to prevent naked returns. + max-func-lines: 30 + + nolintlint: + # Prevent ununsed directive to avoid stale comments. + allow-unused: false + # Require an explanation of nonzero length after each nolint directive. + require-explanation: true + # Exclude following linters from requiring an explanation. + # NOTE: It is strongly discouraged to put anything in there. + allow-no-explanation: [] + # Enable to require nolint directives to mention the specific linter being suppressed. This ensurce the developer understand the reason being the error. + require-specific: true + + prealloc: + # NOTE: For most programs usage of prealloc will be a premature optimization. + # Keep thing simple, pre-alloc what is obvious and profile the program for more complex scenarios. + # + simple: true # Checkonly on simple loops that have no returns/breaks/continues/gotos in them. + range-loops: true # Check range loops, true by default + for-loops: false # Check suggestions on for loops, false by default + + rowserrcheck: + packages: [] + + staticcheck: + # Select the Go version to target. The default is '1.13'. + go: "1.18" + # https://staticcheck.io/docs/options#checks + checks: ["all"] + + stylecheck: + # Select the Go version to target. The default is '1.13'. + go: "1.18" + # https://staticcheck.io/docs/options#checks + checks: ["all"] # "-ST1000", "-ST1003", "-ST1016", "-ST1020", "-ST1021", "-ST1022"] + + tagliatelle: + # Check the struck tag name case. + case: + # Use the struct field name to check the name of the struct tag. + use-field-name: false + rules: + # Any struct tag type can be used. + # support string case: `camel`, `pascal`, `kebab`, `snake`, `goCamel`, `goPascal`, `goKebab`, `goSnake`, `upper`, `lower` + json: snake + firestore: camel + yaml: camel + xml: camel + bson: camel + avro: snake + mapstructure: kebab + envconfig: upper + + unparam: + # Don't create an error if an exported code have static params being used. It is often expected in libraries. + # NOTE: It would be nice if this linter would differentiate between a main package and a lib. + check-exported: true + + unused: {} + + whitespace: + multi-if: false # Enforces newlines (or comments) after every multi-line if statement + multi-func: false # Enforces newlines (or comments) after every multi-line function signature + +# Run `golangci-lint help linters` to get the full list of linter with their description. linters: - default: none + disable-all: true + # NOTE: enable-all is deprecated because too many people don't pin versions... + # We still require explicit documentation on why some linters are disabled. + # disable: + # - depguard # Go linter that checks if package imports are in a list of acceptable packages [fast: true, auto-fix: false] + # - exhaustivestruct # Checks if all struct's fields are initialized [fast: true, auto-fix: false] + # - forbidigo # Forbids identifiers [fast: true, auto-fix: false] + # - gci # Gci control golang package import order and make it always deterministic. [fast: true, auto-fix: true] + # - godox # Tool for detection of FIXME, TODO and other comment keywords [fast: true, auto-fix: false] + # - goerr113 # Golang linter to check the errors handling expressions [fast: true, auto-fix: false] + # - golint # Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: false, auto-fix: false] + # - gomnd # An analyzer to detect magic numbers. [fast: true, auto-fix: false] + # - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. [fast: true, auto-fix: false] + # - interfacer # Linter that suggests narrower interface types [fast: false, auto-fix: false] + # - maligned # Tool to detect Go structs that would take less memory if their fields were sorted [fast: false, auto-fix: false] + # - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity [fast: true, auto-fix: false] + # - scopelint # Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false] + # - wrapcheck # Checks that errors returned from external packages are wrapped [fast: false, auto-fix: false] + # - wsl # Whitespace Linter - Forces you to use empty lines! [fast: true, auto-fix: false] + + # disable-reasons: + # - depguard # Checks whitelisted/blacklisted import path, but runs way too slow. Not that useful. + # - exhaustivestruct # Good concept, but not mature enough (errors on not assignable fields like locks) and too noisy when using AWS SDK as most fields are unused. + # - forbidigo # Great idea, but too strict out of the box. Probably will re-enable soon. + # - gci # Conflicts with goimports/gofumpt. + # - godox # Don't fail when finding TODO, FIXME, etc. + # - goerr113 # Too many false positives. + # - golint # Deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner. Replaced by revive. + # - gomnd # Checks for magic numbers. Disabled due to too many false positives not configurable (03/01/2020 v1.23.7). + # - gomoddirectives # Doesn't support //nolint to whitelist. + # - interfacer # Deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner. + # - maligned # Deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner. Replaced by govet 'fieldalignment'. + # - nlreturn # Actually reduces readability in most cases. + # - scopelint # Deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner. Replaced by exportloopref. + # - wrapcheck # Good concept, but always warns for http coded errors. Need to re-enable and whitelist our error package. + # - wsl # Forces to add newlines around blocks. Lots of false positives, not that useful. + enable: - - asciicheck - - bodyclose - - copyloopvar - - cyclop - - dogsled - - dupl - - durationcheck - - errcheck - - errname - - errorlint - - exhaustive - - forcetypeassert - - funlen - - gochecknoglobals - - gochecknoinits - - gocognit - - goconst - - gocritic - - gocyclo - - godot - - goheader - - gomodguard - - goprintffuncname - - gosec - - govet - - importas - - ineffassign - - intrange - - lll - - makezero - - misspell - - nakedret - - nestif - - nilerr - - noctx - - nolintlint - - paralleltest - - prealloc - - predeclared - - promlinter - - staticcheck - - thelper - - tparallel - - unconvert - - unparam - - wastedassign - - whitespace - settings: - cyclop: - max-complexity: 35 - dogsled: - max-blank-identifiers: 2 - dupl: - threshold: 150 - errcheck: - check-type-assertions: true - check-blank: false - exhaustive: - default-signifies-exhaustive: false - funlen: - lines: 160 - statements: 70 - gocognit: - min-complexity: 35 - goconst: - min-len: 4 - min-occurrences: 5 - gocritic: - disabled-checks: - - hugeParam - enabled-tags: - - diagnostic - - style - - opinionated - - performance - settings: - rangeValCopy: - sizeThreshold: 1024 - gocyclo: - min-complexity: 35 - govet: - enable-all: false - lll: - line-length: 180 - tab-width: 8 - misspell: - locale: US - nakedret: - max-func-lines: 30 - nestif: - min-complexity: 8 - nolintlint: - require-explanation: true - require-specific: true - allow-unused: false - prealloc: - simple: true - range-loops: true - for-loops: false - staticcheck: - checks: - - all - tagliatelle: - case: - rules: - avro: snake - bson: camel - envconfig: upper - firestore: camel - json: snake - mapstructure: kebab - xml: camel - yaml: camel - use-field-name: false - unparam: - check-exported: true - whitespace: - multi-if: false - multi-func: false - exclusions: - generated: lax - rules: - - path: (.+)\.go$ - text: 'shadow: declaration of "err" shadows declaration' - - path: (.+)\.go$ - text: 'shadow: declaration of "ctx" shadows declaration' - paths: - - third_party$ - - builtin$ - - examples$ -formatters: - enable: - - gofmt - - gofumpt - - goimports - settings: - gofmt: - simplify: true - goimports: - local-prefixes: - - github.com/creack/pty - exclusions: - generated: lax - paths: - - third_party$ - - builtin$ - - examples$ + - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers [fast: true, auto-fix: false] + - bodyclose # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false] + - cyclop # checks function and package cyclomatic complexity [fast: false, auto-fix: false] + - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false] + - dupl # Tool for code clone detection [fast: true, auto-fix: false] + - durationcheck # check for two durations multiplied together [fast: false, auto-fix: false] + - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: false, auto-fix: false] + - errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. [fast: false, auto-fix: false] + - errorlint # go-errorlint is a source code linter for Go software that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. [fast: false, auto-fix: false] + - exhaustive # check exhaustiveness of enum switch statements [fast: false, auto-fix: false] + - exportloopref # checks for pointers to enclosing loop variables [fast: false, auto-fix: false] + - forcetypeassert # finds forced type assertions [fast: true, auto-fix: false] + - funlen # Tool for detection of long functions [fast: true, auto-fix: false] + - gochecknoglobals # check that no global variables exist [fast: true, auto-fix: false] + - gochecknoinits # Checks that no init functions are present in Go code [fast: true, auto-fix: false] + - gocognit # Computes and checks the cognitive complexity of functions [fast: true, auto-fix: false] + - goconst # Finds repeated strings that could be replaced by a constant [fast: true, auto-fix: false] + - gocritic # Provides many diagnostics that check for bugs, performance and style issues. [fast: false, auto-fix: false] + - gocyclo # Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false] + - godot # Check if comments end in a period [fast: true, auto-fix: true] + - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true] + - gofumpt # Gofumpt checks whether code was gofumpt-ed. [fast: true, auto-fix: true] + - goheader # Checks is file header matches to pattern [fast: true, auto-fix: false] + - goimports # Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true] + - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. [fast: true, auto-fix: false] + - goprintffuncname # Checks that printf-like functions are named with `f` at the end [fast: true, auto-fix: false] + - gosec # (gas): Inspects source code for security problems [fast: false, auto-fix: false] + - gosimple # (megacheck): Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false] + - govet # (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false] + - importas # Enforces consistent import aliases [fast: false, auto-fix: false] + - ineffassign # Detects when assignments to existing variables are not used [fast: true, auto-fix: false] + - lll # Reports long lines [fast: true, auto-fix: false] + - makezero # Finds slice declarations with non-zero initial length [fast: false, auto-fix: false] + - misspell # Finds commonly misspelled English words in comments [fast: true, auto-fix: true] + - nakedret # Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false] + - nestif # Reports deeply nested if statements [fast: true, auto-fix: false] + - nilerr # Finds the code that returns nil even if it checks that the error is not nil. [fast: false, auto-fix: false] + - noctx # noctx finds sending http request without context.Context [fast: false, auto-fix: false] + - nolintlint # Reports ill-formed or insufficient nolint directives [fast: true, auto-fix: false] + - paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test [fast: true, auto-fix: false] + - prealloc # Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false] + - predeclared # find code that shadows one of Go's predeclared identifiers [fast: true, auto-fix: false] + - promlinter # Check Prometheus metrics naming via promlint [fast: true, auto-fix: false] + - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. [fast: false, auto-fix: false] + # Disabled due to generic. Work in progress upstream. + # - rowserrcheck # checks whether Err of rows is checked successfully [fast: false, auto-fix: false] + # Disabled due to generic. Work in progress upstream. + # - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. [fast: false, auto-fix: false] + - staticcheck # (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false] + - stylecheck # Stylecheck is a replacement for golint [fast: false, auto-fix: false] + # Disabled due to generic. Work in progress upstream. + # - tagliatelle # Checks the struct tags. [fast: true, auto-fix: false] + # - testpackage # linter that makes you use a separate _test package [fast: true, auto-fix: false] + - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers [fast: false, auto-fix: false] + - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes [fast: false, auto-fix: false] + - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false] + - unconvert # Remove unnecessary type conversions [fast: false, auto-fix: false] + - unparam # Reports unused function parameters [fast: false, auto-fix: false] + # Disabled due to way too many false positive in go1.20. + # - unused # (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false] + # Disabled due to generic. Work in progress upstream. + # - wastedassign # wastedassign finds wasted assignment statements. [fast: false, auto-fix: false] + - whitespace # Tool for detection of leading and trailing whitespace [fast: true, auto-fix: true] + +issues: + exclude: + # Allow shadowing of 'err'. + - 'shadow: declaration of "err" shadows declaration' + # Allow shadowing of `ctx`. + - 'shadow: declaration of "ctx" shadows declaration' + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-per-linter: 10 + # Disable default excludes. Always be explicit on what we exclude. + exclude-use-default: false + # Exclude some linters from running on tests files. + exclude-rules: [] diff --git a/go.mod b/go.mod index a22acd4..9fa7dd2 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module github.com/creack/pty -go 1.21.5 +go 1.24.0 -require golang.org/x/sys v0.13.0 +require golang.org/x/sys v0.36.0 diff --git a/go.sum b/go.sum index d4673ec..255a0f7 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,4 @@ golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k= +golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= diff --git a/io_test.go b/io_test.go index 41e9021..47a74fe 100644 --- a/io_test.go +++ b/io_test.go @@ -94,6 +94,7 @@ func TestReadClose(t *testing.T) { // Open pty and setup watchdogs for graceful and not so graceful failure modes. func prepare(t *testing.T) (ptmx Pty, done func()) { + t.Helper() if runtime.GOOS == "darwin" { t.Log("creack/pty uses blocking i/o on darwin intentionally:") t.Log("> https://github.com/creack/pty/issues/52") diff --git a/pty_windows.go b/pty_windows.go index 7dbe189..94ca848 100644 --- a/pty_windows.go +++ b/pty_windows.go @@ -164,7 +164,7 @@ func procCreatePseudoConsole(hInput windows.Handle, hOutput windows.Handle, dwFl return err } - // TODO: Check if it is expected to ignore `err` here. + //nolint:errcheck // Windows syscall: actual error status is in r0 HRESULT, not err. r0, _, _ := createPseudoConsole.Call( (windowsCoord{X: 80, Y: 30}).Pack(), // Size: default 80x30 window. uintptr(hInput), // Console input. diff --git a/winsize_windows.go b/winsize_windows.go index c9a3111..6685ab8 100644 --- a/winsize_windows.go +++ b/winsize_windows.go @@ -43,8 +43,7 @@ func Setsize(t FdHolder, ws *Winsize) error { return err } - // TODO: As we removed the use of `.Fd()` on Unix (https://github.com/creack/pty/pull/168), we need to check if we should do the same here. - // TODO: Check if it is expected to ignore `err` here. + //nolint:errcheck // Windows syscall: actual error status is in r0 HRESULT, not err. r0, _, _ := resizePseudoConsole.Call( t.Fd(), (windowsCoord{X: int16(ws.Cols), Y: int16(ws.Rows)}).Pack(), @@ -63,13 +62,29 @@ func Setsize(t FdHolder, ws *Winsize) error { // GetsizeFull returns the full terminal size description. func GetsizeFull(t FdHolder) (size *Winsize, err error) { + // GetConsoleScreenBufferInfo requires a console buffer handle, not a pseudo console handle. + // For WindowsPty/WindowsTty, we need to get the actual console buffer handle. + var consoleHandle uintptr + + switch v := t.(type) { + case *WindowsPty: + // Use the write pipe's file descriptor as it's connected to the console output + consoleHandle = v.w.Fd() + case *WindowsTty: + // Use the write pipe's file descriptor as it's connected to the console output + consoleHandle = v.w.Fd() + default: + // Fallback to Fd() for other types + consoleHandle = t.Fd() + } + if err := getConsoleScreenBufferInfo.Find(); err != nil { return nil, err } var info windowsConsoleScreenBufferInfo - // TODO: Check if it is expected to ignore `err` here. - r0, _, _ := getConsoleScreenBufferInfo.Call(t.Fd(), uintptr(unsafe.Pointer(&info))) + //nolint:errcheck // Windows syscall: actual error status is in r0 HRESULT, not err. + r0, _, _ := getConsoleScreenBufferInfo.Call(consoleHandle, uintptr(unsafe.Pointer(&info))) if int32(r0) < 0 { if r0&0x1fff0000 == 0x00070000 { r0 &= 0xffff From 44f370a42ffa15deeec0f49d7997daabf948decd Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 30 Sep 2025 12:00:30 -0600 Subject: [PATCH 3/3] add github workflow --- .github/workflows/test-windows.yml | 44 ++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 .github/workflows/test-windows.yml diff --git a/.github/workflows/test-windows.yml b/.github/workflows/test-windows.yml new file mode 100644 index 0000000..7068dce --- /dev/null +++ b/.github/workflows/test-windows.yml @@ -0,0 +1,44 @@ +--- +name: Test Windows + +"on": + push: + branches: + - master + - windows-support + pull_request: + branches: + - master + +jobs: + test: + name: "Test go ${{ matrix.go_version }} on Windows" + runs-on: windows-latest + + strategy: + matrix: + go_version: + - stable + - oldstable + + steps: + - name: Set up Go ${{ matrix.go_version }} + uses: actions/setup-go@v4 + with: + go-version: ${{ matrix.go_version }} + check-latest: true + + - name: Checkout repo + uses: actions/checkout@v4 + + - name: Download dependencies + run: go mod download + + - name: Build + run: go build -v + + - name: Run tests + run: go test -v -timeout=5m + + - name: Run tests (count=10) + run: go test -count=10 -timeout=10m