Fix the suffix bug in Windows#16
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to fix a Windows-specific executable suffix issue when compiling/running exercises, and also updates the module’s Go/tooling dependencies.
Changes:
- Update exercise compilation to return a temp executable path (instead of an open file handle) and attempt to append
.exeon Windows. - Add additional error checks around CLI output printing.
- Bump
go.modGo version and upgrade several dependencies (with correspondinggo.sumupdates).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cmd/gopherlings/main.go | Adjusts temp executable creation/execution flow and adds Windows-specific naming logic. |
| go.mod | Updates Go version directive and bumps direct/indirect dependencies. |
| go.sum | Updates checksums to match the dependency upgrades. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _, err = red.Fprintln(os.Stderr, err) | ||
| if err != nil { | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
main no longer exits with a non-zero status when rootCmd.Execute() returns an error. After printing the error, the program falls through and returns exit code 0 unless printing fails, which will mask failures for callers/CI. Ensure the error path still calls os.Exit(1) (or returns an error to main).
| _, err = red.Fprintln(os.Stderr, err) | |
| if err != nil { | |
| os.Exit(1) | |
| } | |
| _, printErr := red.Fprintln(os.Stderr, err) | |
| if printErr != nil { | |
| os.Exit(1) | |
| } | |
| os.Exit(1) |
| if runtime.GOOS == "windows" { | ||
| name += ".exe" | ||
| } | ||
| f, err := os.CreateTemp("", name) |
There was a problem hiding this comment.
On Windows, appending .exe to the pattern passed to os.CreateTemp doesn't guarantee the resulting filename ends with .exe (CreateTemp appends random characters after the pattern unless you include *). This can still produce a non-executable path like foo.exe12345, so the Windows suffix issue likely persists. Use a pattern like name+"*.exe" (or create a temp dir and join name+".exe") so the final file actually has the .exe suffix.
| if runtime.GOOS == "windows" { | |
| name += ".exe" | |
| } | |
| f, err := os.CreateTemp("", name) | |
| pattern := name | |
| if runtime.GOOS == "windows" { | |
| pattern = name + "*.exe" | |
| } | |
| f, err := os.CreateTemp("", pattern) |
| return "", err | ||
| } | ||
| tmpPath := f.Name() | ||
| f.Close() |
There was a problem hiding this comment.
The result of f.Close() is ignored. Since the file must be closed on Windows before it can be executed/overwritten reliably, handle the close error and clean up the temp file on failure.
| f.Close() | |
| if err := f.Close(); err != nil { | |
| // Best-effort cleanup if we fail to close the temp file. | |
| _ = os.Remove(tmpPath) | |
| return "", err | |
| } |
| go 1.26 | ||
|
|
||
| require ( | ||
| github.com/fatih/color v1.13.0 | ||
| github.com/fsnotify/fsnotify v1.5.4 | ||
| github.com/spf13/cobra v1.5.0 | ||
| github.com/fatih/color v1.19.0 | ||
| github.com/fsnotify/fsnotify v1.9.0 | ||
| github.com/spf13/cobra v1.10.2 |
There was a problem hiding this comment.
The PR description is about a Windows suffix bug, but this change also bumps the module go version from 1.18 to 1.26 and updates multiple dependencies. Raising the go directive is a compatibility-breaking change for users/build environments pinned to older Go versions, so it should be justified in the PR (or reverted if not required for the fix).
|
What is the windows suffix bug? @Qxy0happy |
|
During the use of this tool, the following error was encountered:
exec: "D:\Cache\Temp\User\30625\001-hello633839467": executable file not found in %PATH% Subsequent analysis indicates that this issue arises from the suffix automatically appended by the Windows operating system. Consequently, the program is unable to locate the file corresponding to f.Name(). |
This PR fixes the suffix bug on Windows.