Skip to content

Commit bbd7650

Browse files
authored
Refactor: separate execv into its own package (#3720)
## Changes Move process replacement functionality from libs/exec to libs/execv to separate concerns. The exec package now focuses on command execution, while execv handles process replacement via syscalls. Shell preparation code is extracted to prepare.go. ShellExecv is moved to the execv package and renamed to Shell for clarity. The next step is to also rename `exec` to something that implies executing scripts through a shell. ## Why I looked into the package while working on #3659 and found this could be improved.
1 parent c1fc21c commit bbd7650

File tree

11 files changed

+172
-116
lines changed

11 files changed

+172
-116
lines changed

cmd/bundle/run.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"github.com/databricks/cli/libs/auth"
2020
"github.com/databricks/cli/libs/cmdctx"
2121
"github.com/databricks/cli/libs/cmdio"
22-
"github.com/databricks/cli/libs/exec"
22+
"github.com/databricks/cli/libs/execv"
2323
"github.com/databricks/cli/libs/flags"
2424
"github.com/databricks/cli/libs/logdiag"
2525
"github.com/spf13/cobra"
@@ -278,7 +278,7 @@ func scriptEnv(cmd *cobra.Command, b *bundle.Bundle) []string {
278278
}
279279

280280
func executeScript(content string, cmd *cobra.Command, b *bundle.Bundle) error {
281-
return exec.ShellExecv(content, b.BundleRootPath, scriptEnv(cmd, b))
281+
return execv.Shell(content, b.BundleRootPath, scriptEnv(cmd, b))
282282
}
283283

284284
func executeInline(cmd *cobra.Command, args []string, b *bundle.Bundle) error {
@@ -287,7 +287,7 @@ func executeInline(cmd *cobra.Command, args []string, b *bundle.Bundle) error {
287287
return err
288288
}
289289

290-
return exec.Execv(exec.ExecvOptions{
290+
return execv.Execv(execv.Options{
291291
Args: args,
292292
Env: scriptEnv(cmd, b),
293293
Dir: dir,

libs/exec/execv.go

Lines changed: 0 additions & 34 deletions
This file was deleted.

libs/exec/prepare.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package exec
2+
3+
// ShellPreparation contains everything needed to execute a shell command.
4+
type ShellPreparation struct {
5+
// Executable is the path to the shell executable.
6+
Executable string
7+
// Args contains all arguments including the executable as the first element.
8+
Args []string
9+
// CleanupFn should be called to clean up any temporary resources.
10+
// It may be nil if no cleanup is needed.
11+
CleanupFn func()
12+
}
13+
14+
// PrepareShellCommand prepares a command string for execution through a shell.
15+
// It finds an available shell (bash, sh, or cmd.exe) and returns the prepared
16+
// command with executable, arguments, and an optional cleanup function.
17+
func PrepareShellCommand(command string) (*ShellPreparation, error) {
18+
shell, err := findShell()
19+
if err != nil {
20+
return nil, err
21+
}
22+
23+
ec, err := shell.prepare(command)
24+
if err != nil {
25+
return nil, err
26+
}
27+
28+
args := []string{ec.executable}
29+
args = append(args, ec.args...)
30+
31+
var cleanupFn func()
32+
if ec.scriptFile != "" {
33+
cleanupFn = func() {
34+
ec.cleanup()
35+
}
36+
}
37+
38+
return &ShellPreparation{
39+
Executable: ec.executable,
40+
Args: args,
41+
CleanupFn: cleanupFn,
42+
}, nil
43+
}

libs/exec/prepare_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package exec
2+
3+
import (
4+
"os/exec"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestPrepareShellCommand(t *testing.T) {
12+
prep, err := PrepareShellCommand("echo hello")
13+
require.NoError(t, err)
14+
15+
bashPath, err := exec.LookPath("bash")
16+
require.NoError(t, err)
17+
assert.Equal(t, bashPath, prep.Executable)
18+
assert.Equal(t, bashPath, prep.Args[0])
19+
assert.Equal(t, "-ec", prep.Args[1])
20+
assert.Equal(t, "echo hello", prep.Args[2])
21+
assert.Nil(t, prep.CleanupFn)
22+
}

libs/exec/shell_execv.go

Lines changed: 0 additions & 45 deletions
This file was deleted.

libs/execv/execv.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package execv
2+
3+
import "os"
4+
5+
// Options specifies the configuration for process replacement.
6+
type Options struct {
7+
// Args is the name of the command to run and its arguments.
8+
// Eg: ["echo", "hello"] for "echo hello"
9+
Args []string
10+
11+
// Env specifies the environment variables to set in the child process.
12+
Env []string
13+
14+
// Dir specifies the working directory of the command.
15+
// If Dir is an empty string, Execv runs the command in the
16+
// calling process's current directory.
17+
Dir string
18+
19+
// cleanup is called before process replacement (Windows only).
20+
// Use this to clean up temporary files created during command preparation.
21+
// On Unix systems, cleanup cannot be performed after Execv because
22+
// the process is replaced.
23+
cleanup func()
24+
25+
// windowsExit is a callback to exit the current process on Windows.
26+
// This field is used for testing and should not be set by callers.
27+
windowsExit func(status int)
28+
}
29+
30+
// Execv replaces the current process with the specified command.
31+
//
32+
// On Unix systems (Linux, macOS), this uses the execve syscall which
33+
// replaces the current process image with a new process image.
34+
// Control never returns to the caller on success.
35+
//
36+
// On Windows, which doesn't have an equivalent syscall, this function
37+
// creates a child process, waits for it to complete, and exits with
38+
// the child's exit code to emulate Unix behavior.
39+
func Execv(opts Options) error {
40+
if opts.windowsExit == nil {
41+
opts.windowsExit = os.Exit
42+
}
43+
return execv(opts)
44+
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//go:build linux || darwin
22

3-
package exec
3+
package execv
44

55
import (
66
"fmt"
@@ -9,7 +9,7 @@ import (
99
"syscall"
1010
)
1111

12-
func execv(opts ExecvOptions) error {
12+
func execv(opts Options) error {
1313
if opts.Dir != "" {
1414
err := os.Chdir(opts.Dir)
1515
if err != nil {
Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//go:build windows
22

3-
package exec
3+
package execv
44

55
import (
66
"fmt"
@@ -9,22 +9,23 @@ import (
99
"strings"
1010
)
1111

12-
// Note: Windows does not support an execv syscall that replaces the current process.
13-
// To emulate this, we create a child process, pass the stdin, stdout and stderr file descriptors,
14-
// and return the exit code.
12+
// execv emulates Unix execv behavior on Windows.
13+
//
14+
// Windows does not support an execv syscall that replaces the current process.
15+
// To emulate this, we create a child process, pass the stdin, stdout and stderr
16+
// file descriptors, wait for it to complete, and exit with its exit code.
17+
//
1518
// ref: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/execv-wexecv?view=msvc-170
16-
func execv(opts ExecvOptions) error {
19+
func execv(opts Options) error {
1720
if opts.cleanup != nil {
1821
defer opts.cleanup()
1922
}
2023

2124
windowsExit := func(status int) {
22-
// First clean up the temporary script if it exists.
25+
// Clean up before exiting (defer will not run).
2326
if opts.cleanup != nil {
2427
opts.cleanup()
2528
}
26-
27-
// Then exit the process.
2829
opts.windowsExit(status)
2930
}
3031

@@ -43,7 +44,7 @@ func execv(opts ExecvOptions) error {
4344

4445
err = cmd.Start()
4546
if err != nil {
46-
return fmt.Errorf(" %s failed: %w", strings.Join(opts.Args, " "), err)
47+
return fmt.Errorf("%s failed: %w", strings.Join(opts.Args, " "), err)
4748
}
4849

4950
err = cmd.Wait()

libs/execv/shell.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package execv
2+
3+
import "github.com/databricks/cli/libs/exec"
4+
5+
// Shell executes a shell script by replacing the current process.
6+
//
7+
// The script content is executed through the system's default shell:
8+
// - On Unix systems: bash (preferred) or sh
9+
// - On Windows: cmd.exe
10+
//
11+
// This function calls the execve syscall on Unix systems, which replaces
12+
// the current process. On Windows, it creates a child process and exits
13+
// with the child's exit code.
14+
//
15+
// Note: For cmd.exe, the script is written to a temporary file which is
16+
// automatically cleaned up. On Unix systems, temporary files in /tmp are
17+
// periodically cleaned up by the system.
18+
//
19+
// This function does not return on success.
20+
func Shell(content, dir string, env []string) error {
21+
prep, err := exec.PrepareShellCommand(content)
22+
if err != nil {
23+
return err
24+
}
25+
26+
return Execv(Options{
27+
Args: prep.Args,
28+
Env: env,
29+
Dir: dir,
30+
cleanup: prep.CleanupFn,
31+
})
32+
}
Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,24 @@
1-
package exec
1+
package execv
22

33
import (
44
"os"
5-
"os/exec"
5+
osexec "os/exec"
66
"path/filepath"
77
"runtime"
88
"testing"
99

1010
"github.com/databricks/cli/internal/testutil"
11+
"github.com/databricks/cli/libs/exec"
1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314
)
1415

15-
func TestShellExecvOpts(t *testing.T) {
16-
opts, err := shellExecvOpts("echo hello", "/a/b/c", []string{"key1=value1", "key2=value2"})
17-
require.NoError(t, err)
18-
19-
assert.Equal(t, []string{"key1=value1", "key2=value2"}, opts.Env)
20-
assert.Equal(t, "/a/b/c", opts.Dir)
21-
22-
bashPath, err := exec.LookPath("bash")
23-
require.NoError(t, err)
24-
assert.Equal(t, bashPath, opts.Args[0])
25-
assert.Equal(t, "-ec", opts.Args[1])
26-
assert.Equal(t, "echo hello", opts.Args[2])
27-
}
28-
29-
func TestShellExecv_Windows(t *testing.T) {
16+
func TestShell_Windows(t *testing.T) {
3017
if runtime.GOOS != "windows" {
3118
t.Skip("skipping windows test")
3219
}
3320

34-
cmdExePath, err := exec.LookPath("cmd.exe")
21+
cmdExePath, err := osexec.LookPath("cmd.exe")
3522
require.NoError(t, err)
3623

3724
// Cleanup environment so that other shells like bash and sh are not used.
@@ -56,7 +43,7 @@ func TestShellExecv_Windows(t *testing.T) {
5643
dir := t.TempDir()
5744
t.Setenv("TMP", dir)
5845

59-
opts, err := shellExecvOpts(test.content, dir, []string{})
46+
prep, err := exec.PrepareShellCommand(test.content)
6047
require.NoError(t, err)
6148

6249
// Verify that the temporary file is created.
@@ -66,8 +53,14 @@ func TestShellExecv_Windows(t *testing.T) {
6653
assert.Regexp(t, "cli-exec.*\\.cmd", files[0].Name())
6754

6855
exitCode := -1
69-
opts.windowsExit = func(status int) {
70-
exitCode = status
56+
opts := Options{
57+
Args: prep.Args,
58+
Env: []string{},
59+
Dir: dir,
60+
cleanup: prep.CleanupFn,
61+
windowsExit: func(status int) {
62+
exitCode = status
63+
},
7164
}
7265

7366
// Execute the script.

0 commit comments

Comments
 (0)