refactor: enhance virtual environment handling for development and production modes in PythonProvider#1
refactor: enhance virtual environment handling for development and production modes in PythonProvider#1
Conversation
…oduction modes in PythonProvider - Updated GetVenvPath and related methods to return relative paths in development mode and absolute paths in production. - Consolidated installation commands for efficiency in development mode by combining multiple steps into single commands. - Ensured consistent handling of virtual environment paths across various installation methods and commands.
WalkthroughImplements context-aware Python virtualenv path resolution (host/install vs runtime) with dev-vs-prod branching, updates installers and build/deploy plan generation to use context-specific venv paths, and adjusts related start commands, PATH and VIRTUAL_ENV handling across install/deploy/dev flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Planner
participant EnvResolver as VenvResolver
participant Installer
participant Deployer
rect rgba(240,248,255,0.6)
note over Planner,EnvResolver: Resolve venv path per context
Planner->>EnvResolver: GetVenvPathForInstall(ctx)
EnvResolver-->>Planner: venvPath (".venv" if dev, "/app/.venv" if prod)
end
rect rgba(235,255,235,0.5)
note over Installer: Install-time actions
Installer->>EnvResolver: GetVenvPathForInstall(ctx)
EnvResolver-->>Installer: venvPath
alt Dev
Installer->>Installer: python -m venv venvPath && activate && install-tools && install-deps
else Prod
Installer->>Installer: python -m venv venvPath
Installer->>Installer: activate
Installer->>Installer: install-tools
Installer->>Installer: install-deps
end
end
rect rgba(255,250,240,0.5)
note over Deployer: Runtime setup and start
Deployer->>EnvResolver: GetVenvPath(ctx)
EnvResolver-->>Deployer: runtimeVenv (".venv" dev / "/app/.venv" prod)
Deployer->>Deployer: export PATH=runtimeVenv/bin:...
Deployer->>Deployer: set VIRTUAL_ENV=runtimeVenv
Deployer->>Deployer: run start command (host uses GetVenvPathForHost)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/providers/python/python.go (1)
488-501: Pipenv flow can create/overwrite the venv unexpectedlyYou pre-create the venv (python -m venv …), set VIRTUAL_ENV, and set PIPENV_IGNORE_VIRTUALENVS=1. Pipenv ignores the active venv and will attempt to create its own venv in-project (.venv). If .venv already exists (from pre-creation), this can lead to overwrite/upgrade behavior or failures depending on virtualenv/venv compatibility. Also, running pipenv from inside a venv is discouraged when you’re telling it to ignore that venv.
Recommend:
- Don’t pre-create the venv for Pipenv. Let Pipenv manage it with PIPENV_VENV_IN_PROJECT=1.
- Don’t set VIRTUAL_ENV for Pipenv.
- Use the pipenv installed via pipx (LOCAL_BIN_PATH) and pin the interpreter via “--python python” to use the mise-selected Python.
Apply this diff to simplify and harden Pipenv handling:
@@ install.AddEnvVars(p.GetPythonEnvVars(ctx)) install.AddEnvVars(map[string]string{ "PIPENV_CHECK_UPDATE": "false", "PIPENV_VENV_IN_PROJECT": "1", - "PIPENV_IGNORE_VIRTUALENVS": "1", - "VIRTUAL_ENV": venvPath, + // Let pipenv manage the venv; do not force an existing VIRTUAL_ENV. + // Use in-project venv for deterministic location. + "PIPENV_IGNORE_VIRTUALENVS": "0", }) @@ install.AddCommands([]plan.Command{ plan.NewPathCommand(LOCAL_BIN_PATH), - plan.NewPathCommand(venvPath + "/bin"), }) @@ - if ctx.App.HasMatch("Pipfile.lock") { - if ctx.Dev { - // In dev mode, concatenate commands for efficiency - install.AddCommands([]plan.Command{ - plan.NewCopyCommand("Pipfile"), - plan.NewCopyCommand("Pipfile.lock"), - // Create virtual environment, install pipenv, and install dependencies in one command - plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install pipenv && %s/bin/pipenv install --deploy --ignore-pipfile", venvPath, venvPath, venvPath)), - }) - } else { - // In production mode, use separate commands - install.AddCommands([]plan.Command{ - plan.NewCopyCommand("Pipfile"), - plan.NewCopyCommand("Pipfile.lock"), - // Create virtual environment - plan.NewExecCommand(fmt.Sprintf("python -m venv %s", venvPath)), - // Install pipenv in the virtual environment - plan.NewExecCommand(fmt.Sprintf("%s/bin/pip install pipenv", venvPath)), - // Install dependencies with pipenv - plan.NewExecCommand(fmt.Sprintf("%s/bin/pipenv install --deploy --ignore-pipfile", venvPath)), - }) - } - } else { - if ctx.Dev { - // In dev mode, concatenate commands for efficiency - install.AddCommands([]plan.Command{ - plan.NewCopyCommand("Pipfile"), - // Create virtual environment, install pipenv, and install dependencies in one command - plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install pipenv && %s/bin/pipenv install --skip-lock", venvPath, venvPath, venvPath)), - }) - } else { - // In production mode, use separate commands - install.AddCommands([]plan.Command{ - plan.NewCopyCommand("Pipfile"), - // Create virtual environment - plan.NewExecCommand(fmt.Sprintf("python -m venv %s", venvPath)), - // Install pipenv in the virtual environment - plan.NewExecCommand(fmt.Sprintf("%s/bin/pip install pipenv", venvPath)), - // Install dependencies with pipenv - plan.NewExecCommand(fmt.Sprintf("%s/bin/pipenv install --skip-lock", venvPath)), - }) - } - } + if ctx.App.HasMatch("Pipfile.lock") { + // Use in-project venv managed by pipenv + install.AddCommands([]plan.Command{ + plan.NewCopyCommand("Pipfile"), + plan.NewCopyCommand("Pipfile.lock"), + plan.NewExecCommand("pipenv --python python install --deploy --ignore-pipfile"), + // Ensure downstream steps find the venv bin on PATH + plan.NewPathCommand(venvPath + "/bin"), + }) + } else { + install.AddCommands([]plan.Command{ + plan.NewCopyCommand("Pipfile"), + plan.NewExecCommand("pipenv --python python install --skip-lock"), + plan.NewPathCommand(venvPath + "/bin"), + }) + }Also applies to: 503-511, 512-523, 525-532, 533-543
🧹 Nitpick comments (3)
core/providers/python/python.go (3)
112-114: Comment nit: the path is relative only in devThe code correctly uses GetVenvPath(ctx)+"/bin" (relative in dev, absolute in prod). The inline comment “Use relative path for deploy step” isn’t accurate for prod. Consider updating it for clarity.
- // Use relative path for deploy step + // Use .venv/bin in dev (relative), /app/.venv/bin in production (absolute)
592-595: Poetry: drop POETRY_VIRTUALENVS_PATH when using in-project venvWith POETRY_VIRTUALENVS_IN_PROJECT="true", POETRY_VIRTUALENVS_PATH is ignored and setting it to the venv path can be misleading. Prefer removing it to avoid confusion.
install.AddEnvVars(map[string]string{ - "VIRTUAL_ENV": venvPath, - "POETRY_VIRTUALENVS_PATH": venvPath, + "VIRTUAL_ENV": venvPath, "POETRY_VIRTUALENVS_IN_PROJECT": "true", })Also applies to: 599-607, 608-619
631-634: Consider upgrading pip before installsUpgrading pip reduces failures on newer manylinux wheels and TLS backends.
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install -r requirements.txt", venvPath, venvPath)), + plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install --upgrade pip && %s/bin/pip install -r requirements.txt", venvPath, venvPath, venvPath)), @@ - plan.NewExecCommand(fmt.Sprintf("%s/bin/pip install -r requirements.txt", venvPath)), + plan.NewExecCommand(fmt.Sprintf("%s/bin/pip install --upgrade pip && %s/bin/pip install -r requirements.txt", venvPath, venvPath)),Also applies to: 639-645, 646-652
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/providers/python/python.go(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
core/providers/python/python.go (1)
core/plan/command.go (4)
NewExecCommand(55-61)Command(10-13)NewPathCommand(78-81)NewCopyCommand(83-91)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: test (python-pip)
- GitHub Check: test (node-bun)
- GitHub Check: test (python-pip.disabled)
- GitHub Check: test (node-corepack)
- GitHub Check: test (python-uv-workspace)
- GitHub Check: test (node-yarn-3)
- GitHub Check: test (ruby-sinatra)
- GitHub Check: test (python-pipfile)
- GitHub Check: test (java-maven)
- GitHub Check: test (rust-custom-version)
- GitHub Check: test (rust-multiple-bins)
- GitHub Check: test (python-pdm)
- GitHub Check: test (node-remix)
- GitHub Check: test (java-gradle)
- GitHub Check: test (rust-ring)
- GitHub Check: test (node-yarn-1)
- GitHub Check: test (python-poetry)
- GitHub Check: test (ruby-vanilla)
- GitHub Check: test (ruby-execjs)
- GitHub Check: test (elixir-phoenix)
🔇 Additional comments (5)
core/providers/python/python.go (5)
25-30: Dev vs prod venv path split looks goodConsistent relative path in dev and absolute in prod is a clear improvement.
Also applies to: 40-45
84-85: Ensure uv cache/env are available in build phaseYou invoke uv in the build step using the install-context venv path, which is fine. Please also ensure uv-related env vars (e.g., UV_CACHE_DIR) are set on the build step to preserve caching/perf.
You can add something like this near where the build step is created:
build.AddEnvVars(map[string]string{ "UV_COMPILE_BYTECODE": "1", "UV_LINK_MODE": "copy", "UV_CACHE_DIR": UV_CACHE_DIR, "UV_PYTHON_DOWNLOADS": "never", })
135-136: Excluding the venv path from the app layer is correctPrevents vendoring the venv into the runtime layer. Good.
450-451: uv install flow: LGTM
- VIRTUAL_ENV is aligned with the chosen venv path.
- Dev mode combining steps is a good speedup; prod keeps steps split for cacheability.
Also applies to: 456-465, 466-480
552-557: PDM: approach is reasonablePre-creating a venv and setting VIRTUAL_ENV works with pdm. Dev/prod split is clear.
Also applies to: 561-569, 570-581
… tests - Modified snapshot files to use the absolute path for the virtual environment in build plans. - Adjusted test cases to ensure consistency in virtual environment command paths across different Python frameworks.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
core/providers/python/python.go (2)
260-337: Reduce duplication: derive host dev command from a single generator.GetDevStartCommandHost duplicates GetDevStartCommand logic. Implement Host as a thin wrapper.
-func (p *PythonProvider) GetDevStartCommandHost(ctx *generate.GenerateContext) string { - // Check if this is a Poetry project - hasPoetry := p.hasPoetry(ctx) - ... - return "" -} +func (p *PythonProvider) GetDevStartCommandHost(ctx *generate.GenerateContext) string { + // Currently identical to GetDevStartCommand; keep a separate method for API clarity. + return p.GetDevStartCommand(ctx) +}
112-114: Optional: Only append venv/bin to Deploy PATH in prod.In dev, StartCmdHost runs on host and StartCmd uses relative .venv; PATH injection in Deploy is mostly relevant to prod container. Consider gating on !ctx.Dev to keep Deploy minimal in dev.
- // Use relative path for deploy step - ctx.Deploy.Paths = append(ctx.Deploy.Paths, p.GetVenvPath(ctx)+"/bin") + if !ctx.Dev { + ctx.Deploy.Paths = append(ctx.Deploy.Paths, p.GetVenvPath(ctx)+"/bin") + }Ensure tests don’t depend on PATH in dev flows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
core/__snapshots__/TestGenerateBuildPlanForExamples_node-angular_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-django_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-fasthtml_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-flask_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-pdm_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-pip.disabled_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-pip_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-pipfile_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-poetry_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-system-deps.disabled_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-uv-packaged_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-uv-workspace_1.snap.json(1 hunks)core/__snapshots__/TestGenerateBuildPlanForExamples_python-uv_1.snap.json(1 hunks)core/providers/python/python.go(17 hunks)core/providers/python/python_dev_test.go(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- core/snapshots/TestGenerateBuildPlanForExamples_node-angular_1.snap.json
- core/snapshots/TestGenerateBuildPlanForExamples_python-uv_1.snap.json
🧰 Additional context used
🧬 Code graph analysis (2)
core/providers/python/python.go (1)
core/plan/command.go (4)
NewExecCommand(55-61)Command(10-13)NewPathCommand(78-81)NewCopyCommand(83-91)
core/providers/python/python_dev_test.go (1)
core/plan/plan.go (1)
Deploy(15-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: test (ruby-rails-postgres)
- GitHub Check: test (python-uv-packaged)
- GitHub Check: test (node-yarn-2)
- GitHub Check: test (config-file)
- GitHub Check: test (java-maven)
- GitHub Check: test (node-bun-no-deps)
- GitHub Check: test (python-system-deps.disabled)
- GitHub Check: test (node-vite-react-router-ssr)
- GitHub Check: test (ruby-2)
- GitHub Check: test (ruby-3)
- GitHub Check: test (ruby-vanilla)
- GitHub Check: test (ruby-local-deps)
- GitHub Check: test (ruby-execjs)
- GitHub Check: test (rust-cargo-workspaces-glob)
- GitHub Check: test (elixir-phoenix)
🔇 Additional comments (14)
core/__snapshots__/TestGenerateBuildPlanForExamples_python-uv-packaged_1.snap.json (1)
34-34: LGTM: consistent absolute venv references for packaged build.The change to "/app/.venv" matches PATH/startCommand usage and the updated provider logic.
Also applies to: 40-40
core/__snapshots__/TestGenerateBuildPlanForExamples_python-pip.disabled_1.snap.json (1)
33-33: LGTM: exclude/include now target the correct in-container venv.Matches deploy PATH and start command expectations.
Also applies to: 39-39
core/providers/python/python_dev_test.go (2)
27-27: Good: Dev StartCmd equals StartCmdHost.Asserting equality guarantees consistent host/container dev commands.
Also applies to: 49-49
62-62: Good: Dev start commands now use relative .venv paths.The expectations reflect the provider’s new dev/host venv handling.
Also applies to: 67-67
core/providers/python/python.go (2)
83-85: LGTM: Using GetVenvPathForInstall for uv sync matches dev/prod semantics.This ensures install-time commands align with the new path strategy.
589-595: Disable Poetry venv creation when pre-creating a venv
Setting bothPOETRY_VIRTUALENVS_PATHandPOETRY_VIRTUALENVS_IN_PROJECTalongside a manually createdVIRTUAL_ENVis brittle. To force Poetry to install into your pre-created venv, disable its venv creation:install.AddEnvVars(map[string]string{ "VIRTUAL_ENV": venvPath, - "POETRY_VIRTUALENVS_PATH": venvPath, - "POETRY_VIRTUALENVS_IN_PROJECT": "true", + "POETRY_VIRTUALENVS_CREATE": "false", // install into the pre-created venv + // Optional: retain in-project layout if you drop pre-create + // "POETRY_VIRTUALENVS_IN_PROJECT": "true", })Wrap chained commands in
sh -cto ensure proper shell evaluation:- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install poetry && %s/bin/poetry install --no-interaction --no-ansi --only main --no-root", venvPath, venvPath, venvPath)), + plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install poetry && %s/bin/poetry install --no-interaction --no-ansi --only main --no-root'", venvPath, venvPath, venvPath)),Alternatively, if you’d rather let Poetry manage the venv entirely:
- drop the manual venv creation and
VIRTUAL_ENV- set
POETRY_VIRTUALENVS_IN_PROJECT=true(and optionallyPOETRY_VIRTUALENVS_PATH)- switch to
poetry run …for all commandsWhich approach do you prefer?
core/__snapshots__/TestGenerateBuildPlanForExamples_python-uv-workspace_1.snap.json (1)
34-34: LGTM: absolute venv path aligns with prod/runtime semantics.
No stale relative “.venv” entries found in snapshots.core/__snapshots__/TestGenerateBuildPlanForExamples_python-system-deps.disabled_1.snap.json (1)
34-41: Absolute venv path aligns with install commandThe snapshot now mirrors the install step’s
python -m venv /app/.venv, so using the absolute path here keeps deploy inputs consistent.core/__snapshots__/TestGenerateBuildPlanForExamples_python-pipfile_1.snap.json (1)
29-36: Switch to/app/.venvkeeps deploy inputs in syncThis matches the install flow’s absolute venv location and keeps the include/exclude filters coherent. Looks good.
core/__snapshots__/TestGenerateBuildPlanForExamples_python-fasthtml_1.snap.json (1)
33-40: Consistent absolute venv path for FastHTML planThe adjusted include/exclude entries reflect the
/app/.venvcreation path, so the snapshot stays aligned with the install commands.core/__snapshots__/TestGenerateBuildPlanForExamples_python-poetry_1.snap.json (1)
29-36: Poetry snapshot matches/app/.venvlayoutThe absolute path now mirrors the virtualenv location established in the install step—no concerns.
core/__snapshots__/TestGenerateBuildPlanForExamples_python-flask_1.snap.json (1)
33-40: Flask deploy filters track the new venv pathWith the venv created at
/app/.venv, updating the include/exclude lists keeps the build plan consistent.core/__snapshots__/TestGenerateBuildPlanForExamples_python-pdm_1.snap.json (1)
29-36: PDM snapshot aligned with install-time venv pathIncluding and excluding
/app/.venvmatches the install step and maintains predictable artifact composition.core/__snapshots__/TestGenerateBuildPlanForExamples_python-pip_1.snap.json (1)
33-40: Pip example now references absolute venv pathThe deploy inputs reflect the same
/app/.venvpath produced during install, so the plan remains self-consistent.
| "exclude": [ | ||
| ".venv" | ||
| "/app/.venv" | ||
| ], |
There was a problem hiding this comment.
Keep excluding the host .venv directory
The inputs block with "local": true still uploads files from the developer’s machine, where the virtualenv lives at .venv. Changing the exclusion to /app/.venv no longer matches the host path, so we’ll end up shipping the entire local venv into the build artifact—ballooning upload size and causing non-deterministic deploys. Please continue emitting the relative .venv exclusion (or include both patterns) while leaving the install-stage include set to /app/.venv.
- "exclude": [
- "/app/.venv"
+ "exclude": [
+ ".venv"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "exclude": [ | |
| ".venv" | |
| "/app/.venv" | |
| ], | |
| "exclude": [ | |
| ".venv" | |
| ], |
🤖 Prompt for AI Agents
In core/__snapshots__/TestGenerateBuildPlanForExamples_python-django_1.snap.json
around lines 39 to 41, the snapshot now only excludes "/app/.venv" which fails
to prevent uploading the developer's local ".venv"; add the relative ".venv"
exclusion (or include both ".venv" and "/app/.venv") back into the "exclude"
array in the snapshot so the inputs block will not upload the host venv while
leaving the install-stage include pattern as "/app/.venv".
| plan.NewPathCommand(LOCAL_BIN_PATH), | ||
| plan.NewPathCommand(venvPath + "/bin"), | ||
| // Create virtual environment, install uv, and sync dependencies in one command | ||
| plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install uv && %s/bin/uv sync --locked --no-dev --no-install-project", venvPath, venvPath, venvPath)), | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Bug: Chained commands need a shell. Wrap with sh -c to make '&&' work.
ExecCommand doesn’t implicitly run via a shell. Without sh -c, '&&' will not execute as intended.
Apply:
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install uv && %s/bin/uv sync --locked --no-dev --no-install-project", venvPath, venvPath, venvPath)),
+ plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install uv && %s/bin/uv sync --locked --no-dev --no-install-project'", venvPath, venvPath, venvPath)),
@@
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install pipenv && %s/bin/pipenv install --deploy --ignore-pipfile", venvPath, venvPath, venvPath)),
+ plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install pipenv && %s/bin/pipenv install --deploy --ignore-pipfile'", venvPath, venvPath, venvPath)),
@@
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install pipenv && %s/bin/pipenv install --skip-lock", venvPath, venvPath, venvPath)),
+ plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install pipenv && %s/bin/pipenv install --skip-lock'", venvPath, venvPath, venvPath)),
@@
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install pdm && %s/bin/pdm install --check --prod --no-editable", venvPath, venvPath, venvPath)),
+ plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install pdm && %s/bin/pdm install --check --prod --no-editable'", venvPath, venvPath, venvPath)),
@@
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install poetry && %s/bin/poetry install --no-interaction --no-ansi --only main --no-root", venvPath, venvPath, venvPath)),
+ plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install poetry && %s/bin/poetry install --no-interaction --no-ansi --only main --no-root'", venvPath, venvPath, venvPath)),
@@
- install.AddCommands([]plan.Command{
- plan.NewExecCommand(fmt.Sprintf("python -m venv %s && %s/bin/pip install -r requirements.txt", venvPath, venvPath)),
- plan.NewPathCommand(venvPath + "/bin"),
- })
+ install.AddCommands([]plan.Command{
+ plan.NewExecCommand(fmt.Sprintf("sh -c 'python -m venv %s && %s/bin/pip install -r requirements.txt'", venvPath, venvPath)),
+ plan.NewPathCommand(venvPath + "/bin"),
+ })Search for remaining unwrapped chained ExecCommands:
Also applies to: 509-510, 530-531, 567-568, 605-606, 642-644
🏁 Script executed:
#!/bin/bash
rg -nP "NewExecCommand\\(fmt\\.Sprintf\\([^)]*&&" --type=goLength of output: 1185
Wrap chained commands in sh -c for ExecCommand
All calls to plan.NewExecCommand(fmt.Sprintf("… && …")) must be wrapped as sh -c '… && …' so the shell handles &&.
Remaining occurrences in core/providers/python/python.go at lines 463, 509, 530, 567, 605, 642.
🤖 Prompt for AI Agents
In core/providers/python/python.go around lines 460-464 (and the other reported
occurrences at ~463, 509, 530, 567, 605, 642), the ExecCommand is given a
chained command with && which requires a shell to interpret; update each
plan.NewExecCommand(...) call that passes a "cmd1 && cmd2" formatted string so
it instead invokes the shell: wrap the entire chained command in sh -c
'<original chained command>' (i.e. call NewExecCommand with a string that runs
sh -c and the original "a && b && c" as the shell argument), taking care to use
proper quoting/escaping so inner single quotes are preserved inside the Go
string. Ensure every listed occurrence is changed consistently.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor