diff --git a/internal/runtime/python/python.go b/internal/runtime/python/python.go index 8d5ad6aa..fb82e116 100644 --- a/internal/runtime/python/python.go +++ b/internal/runtime/python/python.go @@ -76,6 +76,45 @@ func getPythonExecutable() string { return "python3" } +// getVenvBinDir returns the platform-specific bin directory inside a virtual environment +func getVenvBinDir(venvPath string) string { + if runtime.GOOS == "windows" { + return filepath.Join(venvPath, "Scripts") + } + return filepath.Join(venvPath, "bin") +} + +// ActivateVenvIfPresent sets the process environment variables that a Python +// virtual environment's activate script would set, so that child processes +// (hook scripts) inherit the activated venv. This is a no-op when no .venv +// directory exists in projectDir. +// +// The three env vars (VIRTUAL_ENV, PATH, PYTHONHOME) are the stable contract +// defined by PEP 405 (Python 3.3, 2012). Other tools (Poetry, tox, pipx) use +// the same approach. Sourcing the shell-specific activate script (activate, +// activate.fish, Activate.ps1) would be higher maintenance because it varies +// by shell. +func ActivateVenvIfPresent(fs afero.Fs, osClient types.Os, projectDir string) (bool, error) { + venvPath := getVenvPath(projectDir) + if !venvExists(fs, venvPath) { + return false, nil + } + + binDir := getVenvBinDir(venvPath) + + if err := osClient.Setenv("VIRTUAL_ENV", venvPath); err != nil { + return false, err + } + if err := osClient.Setenv("PATH", binDir+string(filepath.ListSeparator)+osClient.Getenv("PATH")); err != nil { + return false, err + } + if err := osClient.Unsetenv("PYTHONHOME"); err != nil { + return false, err + } + + return true, nil +} + // getPipExecutable returns the path to the pip executable in the virtual environment func getPipExecutable(venvPath string) string { if runtime.GOOS == "windows" { diff --git a/internal/runtime/python/python_test.go b/internal/runtime/python/python_test.go index f16af53c..4f6355ff 100644 --- a/internal/runtime/python/python_test.go +++ b/internal/runtime/python/python_test.go @@ -18,6 +18,7 @@ import ( "encoding/json" "fmt" "path/filepath" + "runtime" "testing" "github.com/slackapi/slack-cli/internal/config" @@ -314,6 +315,66 @@ func Test_venvExists(t *testing.T) { } } +func Test_getVenvBinDir(t *testing.T) { + result := getVenvBinDir("/path/to/.venv") + if runtime.GOOS == "windows" { + require.Equal(t, filepath.Join("/path/to/.venv", "Scripts"), result) + } else { + require.Equal(t, filepath.Join("/path/to/.venv", "bin"), result) + } +} + +func Test_ActivateVenvIfPresent(t *testing.T) { + tests := map[string]struct { + createVenv bool + expectedActivated bool + }{ + "activates venv when it exists": { + createVenv: true, + expectedActivated: true, + }, + "no-op when venv does not exist": { + createVenv: false, + expectedActivated: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fs := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + projectDir := "/path/to/project" + venvPath := filepath.Join(projectDir, ".venv") + + originalPath := "/usr/bin:/bin" + osMock.On("Getenv", "PATH").Return(originalPath) + osMock.AddDefaultMocks() + + if tc.createVenv { + // Create the pip executable so venvExists returns true + pipPath := getPipExecutable(venvPath) + err := fs.MkdirAll(filepath.Dir(pipPath), 0755) + require.NoError(t, err) + err = afero.WriteFile(fs, pipPath, []byte(""), 0755) + require.NoError(t, err) + } + + activated, err := ActivateVenvIfPresent(fs, osMock, projectDir) + require.NoError(t, err) + require.Equal(t, tc.expectedActivated, activated) + + if tc.expectedActivated { + expectedBinDir := getVenvBinDir(venvPath) + osMock.AssertCalled(t, "Setenv", "VIRTUAL_ENV", venvPath) + osMock.AssertCalled(t, "Setenv", "PATH", expectedBinDir+string(filepath.ListSeparator)+originalPath) + osMock.AssertCalled(t, "Unsetenv", "PYTHONHOME") + } else { + osMock.AssertNotCalled(t, "Setenv", mock.Anything, mock.Anything) + osMock.AssertNotCalled(t, "Unsetenv", mock.Anything) + } + }) + } +} + func Test_Python_InstallProjectDependencies(t *testing.T) { tests := map[string]struct { existingFiles map[string]string diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 11d35492..e6098245 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -66,6 +66,14 @@ func New(runtimeName string) (Runtime, error) { return rt, nil } +// ActivatePythonVenvIfPresent activates a Python virtual environment if one +// exists in the given project directory. This sets VIRTUAL_ENV, prepends the +// venv bin directory to PATH, and unsets PYTHONHOME on the current process so +// that child processes (hook scripts) inherit the activated venv. +func ActivatePythonVenvIfPresent(fs afero.Fs, os types.Os, projectDir string) (bool, error) { + return python.ActivateVenvIfPresent(fs, os, projectDir) +} + // NewDetectProject returns a new Runtime based on the project type of dirPath func NewDetectProject(ctx context.Context, fs afero.Fs, dirPath string, sdkConfig hooks.SDKCLIConfig) (Runtime, error) { var rt Runtime diff --git a/internal/runtime/runtime_test.go b/internal/runtime/runtime_test.go index 932c1698..8a2c29a2 100644 --- a/internal/runtime/runtime_test.go +++ b/internal/runtime/runtime_test.go @@ -15,6 +15,7 @@ package runtime import ( + "path/filepath" "testing" "github.com/slackapi/slack-cli/internal/hooks" @@ -22,7 +23,9 @@ import ( "github.com/slackapi/slack-cli/internal/runtime/node" "github.com/slackapi/slack-cli/internal/runtime/python" "github.com/slackapi/slack-cli/internal/slackcontext" + "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/spf13/afero" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -57,6 +60,55 @@ func Test_Runtime_New(t *testing.T) { } } +func Test_ActivatePythonVenvIfPresent(t *testing.T) { + tests := map[string]struct { + createVenv bool + expectedActivated bool + }{ + "activates venv when it exists": { + createVenv: true, + expectedActivated: true, + }, + "no-op when venv does not exist": { + createVenv: false, + expectedActivated: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fs := slackdeps.NewFsMock() + osMock := slackdeps.NewOsMock() + projectDir := "/path/to/project" + venvPath := filepath.Join(projectDir, ".venv") + + osMock.On("Getenv", "PATH").Return("/usr/bin:/bin") + osMock.AddDefaultMocks() + + if tc.createVenv { + // Create the pip executable so venvExists returns true + pipPath := filepath.Join(venvPath, "bin", "pip") + err := fs.MkdirAll(filepath.Dir(pipPath), 0755) + require.NoError(t, err) + err = afero.WriteFile(fs, pipPath, []byte(""), 0755) + require.NoError(t, err) + } + + activated, err := ActivatePythonVenvIfPresent(fs, osMock, projectDir) + require.NoError(t, err) + require.Equal(t, tc.expectedActivated, activated) + + if tc.expectedActivated { + osMock.AssertCalled(t, "Setenv", "VIRTUAL_ENV", venvPath) + osMock.AssertCalled(t, "Setenv", "PATH", mock.Anything) + osMock.AssertCalled(t, "Unsetenv", "PYTHONHOME") + } else { + osMock.AssertNotCalled(t, "Setenv", mock.Anything, mock.Anything) + osMock.AssertNotCalled(t, "Unsetenv", mock.Anything) + } + }) + } +} + func Test_Runtime_NewDetectProject(t *testing.T) { tests := map[string]struct { sdkConfig hooks.SDKCLIConfig diff --git a/internal/shared/clients.go b/internal/shared/clients.go index ee51b049..55f2d3c9 100644 --- a/internal/shared/clients.go +++ b/internal/shared/clients.go @@ -224,6 +224,14 @@ func (c *ClientFactory) InitSDKConfig(ctx context.Context, dirPath string) error // Move upward one directory level dirPath = filepath.Dir(dirPath) } + // Activate Python virtual environment if present, so hook scripts + // can resolve the venv's Python and installed packages. + if activated, err := runtime.ActivatePythonVenvIfPresent(c.Fs, c.Os, dirPath); err != nil { + c.IO.PrintDebug(ctx, "failed to activate Python virtual environment: %s", err) + } else if activated { + c.IO.PrintDebug(ctx, "Activated Python virtual environment .venv") + } + configFileBytes, err := afero.ReadFile(c.Fs, hooksJSONFilePath) if err != nil { return err // Fixes regression: do not wrap this error, so that the caller can use `os.IsNotExists` diff --git a/internal/shared/types/slackdeps.go b/internal/shared/types/slackdeps.go index f2dd1ae0..858c5201 100644 --- a/internal/shared/types/slackdeps.go +++ b/internal/shared/types/slackdeps.go @@ -38,6 +38,9 @@ type Os interface { // Setenv defaults to `os.Setenv` and can be mocked to test Setenv(key string, value string) error + // Unsetenv defaults to `os.Unsetenv` and can be mocked to test + Unsetenv(key string) error + // Getwd defaults to `os.Getwd` and can be mocked to test Getwd() (dir string, err error) diff --git a/internal/slackdeps/os.go b/internal/slackdeps/os.go index d61c3f1b..4a353235 100644 --- a/internal/slackdeps/os.go +++ b/internal/slackdeps/os.go @@ -53,6 +53,11 @@ func (c *Os) Setenv(key string, value string) error { return os.Setenv(key, value) } +// Unsetenv defaults to `os.Unsetenv` and can be mocked to test +func (c *Os) Unsetenv(key string) error { + return os.Unsetenv(key) +} + // Getwd defaults to `os.Getwd` and can be mocked to test func (c *Os) Getwd() (dir string, err error) { return os.Getwd() diff --git a/internal/slackdeps/os_mock.go b/internal/slackdeps/os_mock.go index ec19f156..f8f1620f 100644 --- a/internal/slackdeps/os_mock.go +++ b/internal/slackdeps/os_mock.go @@ -46,6 +46,7 @@ func (m *OsMock) AddDefaultMocks() { m.On("LookPath", mock.Anything).Return("", nil) m.On("LookupEnv", mock.Anything).Return("", false) m.On("Setenv", mock.Anything, mock.Anything).Return(nil) + m.On("Unsetenv", mock.Anything).Return(nil) m.On("Getwd").Return(MockWorkingDirectory, nil) m.On("UserHomeDir").Return(MockHomeDirectory, nil) m.On("GetExecutionDir").Return(MockHomeDirectory, nil) @@ -83,6 +84,14 @@ func (m *OsMock) Setenv(key string, value string) error { return args.Error(0) } +// Unsetenv mocks the unsetting of an environment variable +func (m *OsMock) Unsetenv(key string) error { + m.On("Getenv", key).Return("") + m.On("LookupEnv", key).Return("", false) + args := m.Called(key) + return args.Error(0) +} + // Getwd mocks returning the working directory. func (m *OsMock) Getwd() (_dir string, _err error) { args := m.Called()