Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] Move Python venv folder location to ~/.velocitas/packages/ #336

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/features/PACKAGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ Other Python processes spawned from a Python-based program will **not** be autom
(Because the dependencies of that scripts will probably differ from those of the calling program's component.)

The venv of each component is created within the project cache directory in the folder `pyvenv`. For example the
component `Foo`s venv would be located in `/home/vscode/.velocitas/projects/<hash>/pyvenv/foo/`. This also contains
component `Foo`s venv would be located in `/home/vscode/.velocitas/packages/.pyvenvs/foo/`. This also contains
Copy link
Contributor

@erikbosch erikbosch Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should better do a minor update to the line 137 to use the right folder name

"package cache directory in the folder .pyvenvs."

the installed dependencies of that component.

### `id` - string
Expand Down
3 changes: 2 additions & 1 deletion src/modules/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { IPty, spawn } from 'node-pty';
import { exec } from 'node:child_process';
import { resolve } from 'node:path';
import { ExecSpec, ProgramSpec } from './component';
import { getPackageFolderPath } from './package';
import { ProjectCache } from './project-cache';
import { ProjectConfig } from './projectConfig/projectConfig';
import { stdOutParser } from './stdout-parser';
Expand Down Expand Up @@ -151,7 +152,7 @@ async function createPythonVenv(
cwd: string,
loggingOptions: { writeStdout?: boolean; verbose?: boolean },
) {
const venvDir = `${ProjectCache.getCacheDir()}/pyvenv/${componentId}`;
const venvDir = `${getPackageFolderPath()}/.pyvenvs/${componentId}`;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes just the Python venv issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue this is not a package and should not go in the package path.

My suggestion would be to move all project related data which is right now put into $VELOCITAS_HOME/projects/<HASH> directly into the project's workspace under a hidden folder .velocitas that way the user is able to freely rename or move his project and still keep all of his cache and project data intact.

This would even be an easy fix by introducing and environment variable called VELOCITAS_INLINE_PROJECT which, if set, redirects the output which would usually go to $VELOCITAS_HOME/projects/<HASH> to $VELOCITAS_WORKSPACE/.velocitas

let venvCreateCmd = command;
if (command === 'pip') {
venvCreateCmd = 'python';
Expand Down