Repository: tensorchord/envd (language: python)

Issue:
bug(run): Failed to run ["cd xx", "ls"]
## Description

## Reproduction

## Additional Info

<!-- It will be very helpful if you can provide the version info by running the command `envd version`. -->

---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.



Discussion / hints:
Is it for build or runtime?
Build.

Fix patch:
diff --git a/examples/ianvs/build.envd b/examples/ianvs/build.envd
new file mode 100644
index 000000000..253b3480b
--- /dev/null
+++ b/examples/ianvs/build.envd
@@ -0,0 +1,11 @@
+def build():
+    base(os="ubuntu20.04", language="python3.6")
+    shell("zsh")
+    install.system_packages(name=["git", "libgl1-mesa-glx", "zip"])
+    run(commands=[
+      "git clone https://github.com/kubeedge/ianvs.git",
+      "cd ./ianvs",
+      "pip install -r requirements.txt",
+      "pip install ./examples/resources/third_party/*",
+      "python setup.py install"
+    ])
diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go
index 7012436f9..46a05cfaf 100644
--- a/pkg/builder/builder.go
+++ b/pkg/builder/builder.go
@@ -176,7 +176,8 @@ func (b generalBuilder) Interpret() error {
 }
 
 func (b generalBuilder) compile(ctx context.Context) (*llb.Definition, error) {
-	def, err := ir.Compile(ctx, filepath.Base(b.BuildContextDir), b.PubKeyPath)
+	envName := filepath.Base(b.BuildContextDir)
+	def, err := ir.Compile(ctx, envName, b.PubKeyPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to compile build.envd")
 	}
diff --git a/pkg/lang/frontend/starlark/install/install.go b/pkg/lang/frontend/starlark/install/install.go
index b6616b34a..727b99570 100644
--- a/pkg/lang/frontend/starlark/install/install.go
+++ b/pkg/lang/frontend/starlark/install/install.go
@@ -59,19 +59,12 @@ func ruleFuncPyPIPackage(thread *starlark.Thread, _ *starlark.Builtin,
 		}
 	}
 
-	var path *string = nil
 	requirementsFileStr := requirementsFile.GoString()
-	if requirementsFileStr != "" {
-		buildContextDir := starlark.Universe[builtin.BuildContextDir]
-		buildContextDirStr := buildContextDir.(starlark.String).GoString()
-		buf := filepath.Join(buildContextDirStr, requirementsFileStr)
-		path = &buf
-	}
 
 	logger.Debugf("rule `%s` is invoked, name=%v, requirements=%s",
 		rulePyPIPackage, nameList, requirementsFileStr)
 
-	err := ir.PyPIPackage(nameList, path)
+	err := ir.PyPIPackage(nameList, requirementsFileStr)
 	return starlark.None, err
 }
 
diff --git a/pkg/lang/ir/cache.go b/pkg/lang/ir/cache.go
index b8e33d501..5848978dd 100644
--- a/pkg/lang/ir/cache.go
+++ b/pkg/lang/ir/cache.go
@@ -24,9 +24,9 @@ func (g Graph) CacheID(filename string) string {
 	gpu := g.CUDA != nil || g.CUDNN != nil
 	var cacheID string
 	if gpu {
-		cacheID = fmt.Sprintf("%s/%s-gpu", filename, g.CachePrefix)
+		cacheID = fmt.Sprintf("%s/%s-gpu", filename, g.EnvironmentName)
 	} else {
-		cacheID = fmt.Sprintf("%s/%s-cpu", filename, g.CachePrefix)
+		cacheID = fmt.Sprintf("%s/%s-cpu", filename, g.EnvironmentName)
 	}
 	logrus.Debugf("apt/pypi calculated cacheID: %s", cacheID)
 	return cacheID
diff --git a/pkg/lang/ir/compile.go b/pkg/lang/ir/compile.go
index d5238b100..f48850e0e 100644
--- a/pkg/lang/ir/compile.go
+++ b/pkg/lang/ir/compile.go
@@ -39,10 +39,12 @@ func NewGraph() *Graph {
 		RuntimeCommands: make(map[string]string),
 		RuntimeEnviron:  make(map[string]string),
 	}
+	langVersion := languageVersionDefault
 	return &Graph{
 		OS: osDefault,
 		Language: Language{
-			Name: languageDefault,
+			Name:    languageDefault,
+			Version: &langVersion,
 		},
 		CUDA:    nil,
 		CUDNN:   nil,
@@ -68,13 +70,13 @@ func NumGPUs() int {
 	return DefaultGraph.NumGPUs
 }
 
-func Compile(ctx context.Context, cachePrefix string, pub string) (*llb.Definition, error) {
+func Compile(ctx context.Context, envName string, pub string) (*llb.Definition, error) {
 	w, err := compileui.New(ctx, os.Stdout, "auto")
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create compileui")
 	}
 	DefaultGraph.Writer = w
-	DefaultGraph.CachePrefix = cachePrefix
+	DefaultGraph.EnvironmentName = envName
 	DefaultGraph.PublicKeyPath = pub
 
 	uid, gid, err := getUIDGID()
@@ -86,7 +88,11 @@ func Compile(ctx context.Context, cachePrefix string, pub string) (*llb.Definiti
 		return nil, errors.Wrap(err, "failed to compile")
 	}
 	// TODO(gaocegege): Support multi platform.
-	return state.Marshal(ctx, llb.LinuxAmd64)
+	def, err := state.Marshal(ctx, llb.LinuxAmd64)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to marshal the llb definition")
+	}
+	return def, nil
 }
 
 func Labels() (map[string]string, error) {
diff --git a/pkg/lang/ir/consts.go b/pkg/lang/ir/consts.go
index fe765c62d..30c51630f 100644
--- a/pkg/lang/ir/consts.go
+++ b/pkg/lang/ir/consts.go
@@ -15,9 +15,10 @@
 package ir
 
 const (
-	osDefault         = "ubuntu20.04"
-	languageDefault   = "python3"
-	pypiIndexModeAuto = "auto"
+	osDefault              = "ubuntu20.04"
+	languageDefault        = "python"
+	languageVersionDefault = "3"
+	pypiIndexModeAuto      = "auto"
 
 	// used inside the container
 	defaultConfigDir   = "/home/envd/.config"
diff --git a/pkg/lang/ir/interface.go b/pkg/lang/ir/interface.go
index 6eb261345..0e05dbd9d 100644
--- a/pkg/lang/ir/interface.go
+++ b/pkg/lang/ir/interface.go
@@ -37,16 +37,13 @@ func Base(os, language, image string) error {
 	return nil
 }
 
-func PyPIPackage(deps []string, requirementsFile *string) error {
+func PyPIPackage(deps []string, requirementsFile string) error {
 	DefaultGraph.PyPIPackages = append(DefaultGraph.PyPIPackages, deps...)
 
-	if requirementsFile != nil {
-		parsed, err := parser.ParsePythonRequirements(*requirementsFile)
-		if err != nil {
-			return err
-		}
-		DefaultGraph.PyPIPackages = append(DefaultGraph.PyPIPackages, parsed...)
+	if requirementsFile != "" {
+		DefaultGraph.RequirementsFile = &requirementsFile
 	}
+
 	return nil
 }
 
diff --git a/pkg/lang/ir/python.go b/pkg/lang/ir/python.go
index 4ab5a24c2..3d474f6e8 100644
--- a/pkg/lang/ir/python.go
+++ b/pkg/lang/ir/python.go
@@ -22,6 +22,8 @@ import (
 	"github.com/cockroachdb/errors"
 	"github.com/moby/buildkit/client/llb"
 	"github.com/sirupsen/logrus"
+
+	"github.com/tensorchord/envd/pkg/flag"
 )
 
 const (
@@ -113,7 +115,7 @@ func (g Graph) compileAlternative(root llb.State) llb.State {
 }
 
 func (g Graph) compilePyPIPackages(root llb.State) llb.State {
-	if len(g.PyPIPackages) == 0 {
+	if len(g.PyPIPackages) == 0 && g.RequirementsFile == nil {
 		return root
 	}
 
@@ -121,26 +123,50 @@ func (g Graph) compilePyPIPackages(root llb.State) llb.State {
 	// Create the cache directory to the container. see issue #582
 	root = g.CompileCacheDir(root, cacheDir)
 
-	// Compose the package install command.
-	var sb strings.Builder
-	// Always use the conda's pip.
-	sb.WriteString("/opt/conda/envs/envd/bin/python -m pip install")
-	for _, pkg := range g.PyPIPackages {
-		sb.WriteString(fmt.Sprintf(" %s", pkg))
-	}
-
-	cmd := sb.String()
-	logrus.Debugf("pip command: %s", cmd)
-	root = llb.User("envd")(root)
-	// Refer to https://github.com/moby/buildkit/blob/31054718bf775bf32d1376fe1f3611985f837584/frontend/dockerfile/dockerfile2llb/convert_runmount.go#L46
 	cache := root.File(llb.Mkdir("/cache",
 		0755, llb.WithParents(true), llb.WithUIDGID(g.uid, g.gid)), llb.WithCustomName("[internal] setting pip cache mount permissions"))
-	run := root.
-		Run(llb.Shlex(cmd), llb.WithCustomNamef("pip install %s",
-			strings.Join(g.PyPIPackages, " ")))
-	run.AddMount(cacheDir, cache,
-		llb.AsPersistentCacheDir(g.CacheID(cacheDir), llb.CacheMountShared), llb.SourcePath("/cache"))
-	return run.Root()
+
+	if len(g.PyPIPackages) != 0 {
+		// Compose the package install command.
+		var sb strings.Builder
+		// Always use the conda's pip.
+		sb.WriteString("/opt/conda/envs/envd/bin/python -m pip install")
+		for _, pkg := range g.PyPIPackages {
+			sb.WriteString(fmt.Sprintf(" %s", pkg))
+		}
+
+		cmd := sb.String()
+		logrus.WithField("command", cmd).
+			Debug("Configure pip install statements")
+		root = llb.User("envd")(root)
+		run := root.
+			Run(llb.Shlex(sb.String()), llb.WithCustomNamef("pip install %s",
+				strings.Join(g.PyPIPackages, " ")))
+		// Refer to https://github.com/moby/buildkit/blob/31054718bf775bf32d1376fe1f3611985f837584/frontend/dockerfile/dockerfile2llb/convert_runmount.go#L46
+		run.AddMount(cacheDir, cache,
+			llb.AsPersistentCacheDir(g.CacheID(cacheDir), llb.CacheMountShared), llb.SourcePath("/cache"))
+		root = run.Root()
+	}
+
+	if g.RequirementsFile != nil {
+		// Compose the package install command.
+		var sb strings.Builder
+		sb.WriteString("/opt/conda/envs/envd/bin/python -m pip install -r ")
+		sb.WriteString(*g.RequirementsFile)
+		cmd := sb.String()
+		logrus.WithField("command", cmd).
+			Debug("Configure pip install requirements statements")
+		root = root.Dir(g.getWorkingDir())
+		run := root.
+			Run(llb.Shlex(cmd), llb.WithCustomNamef("pip install %s",
+				strings.Join(g.PyPIPackages, " ")))
+		run.AddMount(cacheDir, cache,
+			llb.AsPersistentCacheDir(g.CacheID(cacheDir), llb.CacheMountShared), llb.SourcePath("/cache"))
+		run.AddMount(g.getWorkingDir(),
+			llb.Local(flag.FlagBuildContext), llb.Readonly)
+		root = run.Root()
+	}
+	return root
 }
 
 func (g Graph) compilePyPIIndex(root llb.State) llb.State {
diff --git a/pkg/lang/ir/system.go b/pkg/lang/ir/system.go
index d8d7c8d1d..4fc1f4f2f 100644
--- a/pkg/lang/ir/system.go
+++ b/pkg/lang/ir/system.go
@@ -55,10 +55,23 @@ func (g Graph) compileRun(root llb.State) llb.State {
 		return root.Run(llb.Shlex(fmt.Sprintf("bash -c \"%s\"", g.Exec[0]))).Root()
 	}
 
-	run := root.Run(llb.Shlex(fmt.Sprintf("bash -c \"%s\"", g.Exec[0])))
-	for _, c := range g.Exec[1:] {
-		run = run.Run(llb.Shlex(fmt.Sprintf("bash -c \"%s\"", c)))
+	var sb strings.Builder
+	sb.WriteString("set -euo pipefail\n")
+	for _, c := range g.Exec {
+		sb.WriteString(c + "\n")
 	}
+
+	cmdStr := fmt.Sprintf("bash -c '%s'", sb.String())
+	logrus.WithField("command", cmdStr).Debug("compile run command")
+	workingDir := g.getWorkingDir()
+	run := root.Dir(workingDir).
+		Run(llb.Shlex(cmdStr))
+	// Mount the build context into the build process.
+	// TODO(gaocegege): Maybe we should make it readonly,
+	// but these cases then cannot be supported:
+	// run(commands=["git clone xx.git"])
+	run.AddMount(workingDir, llb.Local(flag.FlagBuildContext))
+
 	return run.Root()
 }
 
diff --git a/pkg/lang/ir/types.go b/pkg/lang/ir/types.go
index 59c859a8d..3214ddd0d 100644
--- a/pkg/lang/ir/types.go
+++ b/pkg/lang/ir/types.go
@@ -43,10 +43,11 @@ type Graph struct {
 
 	PublicKeyPath string
 
-	PyPIPackages   []string
-	RPackages      []string
-	JuliaPackages  []string
-	SystemPackages []string
+	PyPIPackages     []string
+	RequirementsFile *string
+	RPackages        []string
+	JuliaPackages    []string
+	SystemPackages   []string
 
 	VSCodePlugins []vscode.Plugin
 
@@ -60,8 +61,11 @@ type Graph struct {
 	*CondaConfig
 	*RStudioServerConfig
 
-	Writer      compileui.Writer
-	CachePrefix string
+	Writer compileui.Writer
+	// EnvironmentName is the base name of the environment.
+	// It is the BaseDir(BuildContextDir)
+	// e.g. mnist, streamlit-mnist
+	EnvironmentName string
 
 	RuntimeGraph
 }
diff --git a/pkg/lang/ir/util.go b/pkg/lang/ir/util.go
index 3a2809b63..dc67a7851 100644
--- a/pkg/lang/ir/util.go
+++ b/pkg/lang/ir/util.go
@@ -16,12 +16,17 @@ package ir
 
 import (
 	"os/user"
+	"path/filepath"
 	"regexp"
 	"strconv"
 
 	"github.com/cockroachdb/errors"
 )
 
+func (g Graph) getWorkingDir() string {
+	return filepath.Join("/home/envd", g.EnvironmentName)
+}
+
 func parseLanguage(l string) (string, *string, error) {
 	var language, version string
 	if l == "" {


Test patch:
diff --git a/e2e/python_test.go b/e2e/python_test.go
new file mode 100644
index 000000000..2134820dc
--- /dev/null
+++ b/e2e/python_test.go
@@ -0,0 +1,49 @@
+// Copyright 2022 The envd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package e2e
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+)
+
+var _ = Describe("python", Ordered, func() {
+	It("Should build packages successfully", func() {
+		exampleName := "python/packages"
+		testcase := "e2e"
+		e := NewExample(exampleName, testcase)
+		e.BuildImage(true)()
+		e.RunContainer()()
+		e.DestroyContainer()()
+		e.RemoveImage()()
+	})
+	It("Should build requirements successfully", func() {
+		exampleName := "python/requirements"
+		testcase := "e2e"
+		e := NewExample(exampleName, testcase)
+		e.BuildImage(true)()
+		e.RunContainer()()
+		e.DestroyContainer()()
+		e.RemoveImage()()
+	})
+	It("Should build hybrid successfully", func() {
+		exampleName := "python/hybrid"
+		testcase := "e2e"
+		e := NewExample(exampleName, testcase)
+		e.BuildImage(true)()
+		e.RunContainer()()
+		e.DestroyContainer()()
+		e.RemoveImage()()
+	})
+})
diff --git a/e2e/testdata/python/hybrid/build.envd b/e2e/testdata/python/hybrid/build.envd
new file mode 100644
index 000000000..99ddcec97
--- /dev/null
+++ b/e2e/testdata/python/hybrid/build.envd
@@ -0,0 +1,4 @@
+def build():
+    install.python_packages(name=[
+        "via"
+    ], requirements="requirements.txt")
diff --git a/e2e/testdata/python/hybrid/requirements.txt b/e2e/testdata/python/hybrid/requirements.txt
new file mode 100644
index 000000000..682070c44
--- /dev/null
+++ b/e2e/testdata/python/hybrid/requirements.txt
@@ -0,0 +1,1 @@
+via
diff --git a/e2e/testdata/python/packages/build.envd b/e2e/testdata/python/packages/build.envd
new file mode 100644
index 000000000..10338b4f0
--- /dev/null
+++ b/e2e/testdata/python/packages/build.envd
@@ -0,0 +1,4 @@
+def build():
+    install.python_packages(name = [
+        "via"
+    ])
diff --git a/e2e/testdata/python/requirements/build.envd b/e2e/testdata/python/requirements/build.envd
new file mode 100644
index 000000000..1a8b7799e
--- /dev/null
+++ b/e2e/testdata/python/requirements/build.envd
@@ -0,0 +1,2 @@
+def build():
+    install.python_packages(requirements="./requirements.txt")
diff --git a/e2e/testdata/python/requirements/requirements.txt b/e2e/testdata/python/requirements/requirements.txt
new file mode 100644
index 000000000..682070c44
--- /dev/null
+++ b/e2e/testdata/python/requirements/requirements.txt
@@ -0,0 +1,1 @@
+via
diff --git a/e2e/testdata/run/build.envd b/e2e/testdata/run/build.envd
new file mode 100644
index 000000000..f544e6e36
--- /dev/null
+++ b/e2e/testdata/run/build.envd
@@ -0,0 +1,9 @@
+def build():
+    base(os="ubuntu20.04", language="python3")
+    shell("zsh")
+    run(commands=[
+        "mkdir test",
+        "cd test",
+        "ls",
+        "pwd",
+    ])