Skip to content

Conversation

mikeland73
Copy link
Contributor

@mikeland73 mikeland73 commented Sep 19, 2022

Summary

This is stacked on #105. Please review that one first.

Adds basic pip support.

  • Uses requirements.txt for detection.
  • Uses venv for shell
  • Uses venv + pex for build

Does not yet libraries with native extensions like pandas but there's a plan for that.

How was it tested?

  • devbox shell.
  • devbox build
  • Unit tests

Base automatically changed from landau/nginx-support to main September 19, 2022 22:22
Copy link
Collaborator

@savil savil left a comment

Choose a reason for hiding this comment

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

TIL about pex. Nice work!

lgtm, but will defer to others who are more familiar with python environments.

Comment on lines +17 to +19
// ImportError: libstdc++.so.6: cannot open shared object file: No such file or directory
// possible solution is to set $LD_LIBRARY_PATH
// https://nixos.wiki/wiki/Packaging/Quirks_and_Caveats
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try adding gcc or binutils nix package when building with pandas.

return nil
}

return usererr.New(
Copy link
Collaborator

Choose a reason for hiding this comment

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

must be case-insen....

Comment on lines 74 to 75
python -m venv %[1]s;
source %[2]s;`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just in case srcDir has spaces:

Suggested change
python -m venv %[1]s;
source %[2]s;`)
python -m venv "%[1]s";
source "%[2]s";`)

devbox_test.go Outdated
@@ -12,6 +12,7 @@ import (
)

func TestDevbox(t *testing.T) {
os.Setenv("TMPDIR", "/tmp")
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you use t.Setenv it'll automatically restore the original value and guard against changing the value in parallel tests.

It might also be worth checking that /tmp exists before using it so we can provide a helpful error.

@mikeland73 mikeland73 merged commit 9cea339 into main Sep 23, 2022
@mikeland73 mikeland73 deleted the landau/pip-support branch September 23, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants