Merged
Conversation
5f9dbf9 to
a80dcff
Compare
Member
Author
|
Had to remove some bits from the test, because |
thaJeztah
commented
Jun 24, 2025
| doc: "context timeout", | ||
| cmdAndArgs: []string{testReExec3}, | ||
| expOut: "Hello test-reexec3", | ||
| expError: "signal: killed", |
Member
Author
There was a problem hiding this comment.
Ah, derp, and of course, windows has a different error;
=== RUN TestCommandContext/context_timeout
reexec_test.go:190: got "exit status 1", want "signal: killed"
f99a5f7 to
42b1ba7
Compare
Member
Author
|
thaJeztah
commented
Jun 24, 2025
Adds a CommandContext command, to provide the equivalent of exec.CommandContext. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
42b1ba7 to
7aa8514
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a CommandContext wrapper mirroring exec.CommandContext in the reexec package, enabling context-based cancellation for re-executed commands. Key changes:
- Added
CommandContextAPI inreexec.goand platform-specific implementations inreexec_other.goandreexec_linux.go. - Updated imports and registrations to support context-aware execution.
- Added
TestCommandContextinreexec_test.goto validate context cancellation, timeouts, and argument handling.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| reexec/reexec.go | Added public CommandContext function and imported context. |
| reexec/reexec_other.go | Introduced generic commandContext implementation. |
| reexec/reexec_linux.go | Added Linux-specific commandContext with Pdeathsig support. |
| reexec/reexec_test.go | New TestCommandContext covering basename, full path, args, cancel, and timeout cases. |
Comments suppressed due to low confidence (2)
reexec/reexec_test.go:139
- Missing import of "path/filepath", which is required for
filepath.Join. Please add"path/filepath"to the import block.
cmdAndArgs: []string{filepath.Join("something", testReExec2)},
reexec/reexec_other.go:19
- This
commandContextis duplicated inreexec_linux.go, leading to a redeclaration error on Linux builds. Add a build tag like// +build !linuxat the top of this file to restrict it to non-Linux platforms.
func commandContext(ctx context.Context, args ...string) *exec.Cmd {
vvoland
approved these changes
Jun 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a CommandContext command, to provide the equivalent of exec.CommandContext.