Merge pull request #1149 from donny-dont/features/git-checkout-specified

Adding support for setting the workspace directly in the config

cmd/drone-build/main.go

@@ -5,9 +5,7 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"net/url"
 	"os"
-	"path/filepath"
 	"strings"
 
 	log "github.com/drone/drone/Godeps/_workspace/src/github.com/Sirupsen/logrus"
@@ -37,7 +35,6 @@ func main() {
 		os.Exit(1)
 		return
 	}
-	createClone(ctx)
 
 	// creates the Docker client, connecting to the
 	// linked Docker daemon
@@ -68,6 +65,7 @@ func main() {
 		os.Exit(1)
 		return
 	}
+	createClone(ctx)
 
 	var execs []execFunc
 	if *clone {
@@ -130,14 +128,16 @@ func createClone(c *Context) error {
 	// to the clone object for merge requests from bitbucket, gitlab, et al
 	// if len(c.Commit.PullRequest) != 0 {
 	// }
-
-	url_, err := url.Parse(c.Repo.Link)
-	if err != nil {
-		return err
-	}
-	c.Clone.Dir = filepath.Join("/drone/src", url_.Host, c.Repo.FullName)
+	pathv, ok := c.Conf.Clone.Config["path"]
+	if ok {
+		path, ok := pathv.(string)
+		if ok {
+			c.Clone.Dir = path
 			return nil
 		}
+	}
+	return fmt.Errorf("Workspace path not found")
+}
 
 func parseContext() (*Context, error) {
 	c := &Context{}

cmd/drone-build/run.go

@@ -1,8 +1,6 @@
 package main
 
 import (
-	"path/filepath"
-
 	"github.com/drone/drone/Godeps/_workspace/src/github.com/samalba/dockerclient"
 	common "github.com/drone/drone/pkg/types"
 	"github.com/drone/drone/pkg/yaml"
@@ -64,17 +62,6 @@ func setup(c *Context) error {
 		c.Conf.Build.Environment = append(c.Conf.Build.Environment, env)
 	}
 
-	// attempt to extract the clone path. i'm not a huge fan of
-	// this, by the way, but for now we'll keep it.
-	// TODO consider moving this to a transform?
-	pathv, ok := c.Conf.Clone.Config["path"]
-	if ok {
-		path, ok := pathv.(string)
-		if ok {
-			c.Clone.Dir = filepath.Join("/drone/src", path)
-		}
-	}
-
 	return nil
 }
 

pkg/runner/builtin/runner.go

@@ -2,14 +2,14 @@ package builtin
 
 import (
 	"bytes"
+	"crypto/tls"
+	"crypto/x509"
 	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"time"
-	"crypto/tls"
-	"crypto/x509"
 
 	"github.com/drone/drone/Godeps/_workspace/src/github.com/samalba/dockerclient"
 	"github.com/drone/drone/pkg/docker"

pkg/yaml/lint.go

@@ -13,7 +13,7 @@ import (
 // fails it should return an error message.
 type lintRule func(*common.Config) error
 
-var lintRules = [...]lintRule{
+var lintRules = []lintRule{
 	expectBuild,
 	expectImage,
 	expectCommand,
@@ -22,6 +22,7 @@ var lintRules = [...]lintRule{
 	expectTrustedPublish,
 	expectTrustedDeploy,
 	expectTrustedNotify,
+	expectCloneInWorkspace,
 	expectCacheInWorkspace,
 }
 
@@ -106,6 +107,33 @@ func expectTrustedNotify(c *common.Config) error {
 	return nil
 }
 
+// lint rule that fails if the clone directory is not contained
+// in the root workspace.
+func expectCloneInWorkspace(c *common.Config) error {
+	pathv, ok := c.Clone.Config["path"]
+	var path string
+
+	if ok {
+		path, _ = pathv.(string)
+	}
+	if len(path) == 0 {
+		// This should only happen if the transformer was not run
+		return fmt.Errorf("No workspace specified")
+	}
+
+  relative, relOk := filepath.Rel("/drone/src", path)
+	if relOk != nil {
+		return fmt.Errorf("Path is not relative to root")
+	}
+
+	cleaned := filepath.Clean(relative)
+	if strings.Index(cleaned, "../") != -1 {
+		return fmt.Errorf("Cannot clone above the root")
+	}
+
+	return nil
+}
+
 // lint rule that fails if the cache directories are not contained
 // in the workspace.
 func expectCacheInWorkspace(c *common.Config) error {

pkg/yaml/lint_test.go

@@ -79,6 +79,9 @@ func Test_Linter(t *testing.T) {
 			c.Build.Image = "golang"
 			c.Build.Config = map[string]interface{}{}
 			c.Build.Config["commands"] = []string{"go build", "go test"}
+			c.Clone = &common.Step{}
+			c.Clone.Config = map[string]interface{}{}
+			c.Clone.Config["path"] = "/drone/src/foo/bar"
 			c.Publish = map[string]*common.Step{}
 			c.Publish["docker"] = &common.Step{Image: "docker"}
 			c.Deploy = map[string]*common.Step{}
@@ -88,7 +91,29 @@ func Test_Linter(t *testing.T) {
 			g.Assert(Lint(c) == nil).IsTrue()
 		})
 
-		g.It("Should pass with path inside workspace", func() {
+		g.It("Should pass with clone path inside workspace", func() {
+			c := &common.Config{
+				Clone: &common.Step{
+					Config: map[string]interface{}{
+						"path": "/drone/src/foo/bar",
+					},
+				},
+			}
+			g.Assert(expectCloneInWorkspace(c) == nil).IsTrue()
+		})
+
+		g.It("Should fail with clone path outside workspace", func() {
+			c := &common.Config{
+				Clone: &common.Step{
+					Config: map[string]interface{}{
+						"path": "/foo/bar",
+					},
+				},
+			}
+			g.Assert(expectCloneInWorkspace(c) != nil).IsTrue()
+		})
+
+		g.It("Should pass with cache path inside workspace", func() {
 			c := &common.Config{
 				Build: &common.Step{
 					Cache: []string{".git", "/.git", "/.git/../.git/../.git"},
@@ -97,7 +122,7 @@ func Test_Linter(t *testing.T) {
 			g.Assert(expectCacheInWorkspace(c) == nil).IsTrue()
 		})
 
-		g.It("Should fail with path outside workspace", func() {
+		g.It("Should fail with cache path outside workspace", func() {
 			c := &common.Config{
 				Build: &common.Step{
 					Cache: []string{".git", "/.git", "../../.git"},
@@ -115,7 +140,7 @@ func Test_Linter(t *testing.T) {
 			g.Assert(expectCacheInWorkspace(c) != nil).IsTrue()
 		})
 
-		g.It("Should fail when : is in the path", func() {
+		g.It("Should fail when : is in the cache path", func() {
 			c := &common.Config{
 				Build: &common.Step{
 					Cache: []string{".git", ".git:/../"},

pkg/yaml/transform/transform.go

@@ -9,6 +9,12 @@ import (
 	common "github.com/drone/drone/pkg/types"
 )
 
+// buildRoot is the root build directory.
+//
+// If this changes then the matching value in lint.go needs
+// to be modified as well.
+const buildRoot = "/drone/src"
+
 // transformRule applies a check or transformation rule
 // to the build configuration.
 type transformRule func(*common.Config)
@@ -203,7 +209,17 @@ func rmNetwork(c *common.Config) {
 // directory to the configuration based on the repository
 // information.
 func transformWorkspace(c *common.Config, r *common.Repo) {
-	//c.Clone.Dir = workspaceRoot(r)
+	pathv, ok := c.Clone.Config["path"]
+	var path string
+
+	if ok {
+		path, _ = pathv.(string)
+	}
+	if len(path) == 0 {
+		path = repoPath(r)
+	}
+
+	c.Clone.Config["path"] = filepath.Join(buildRoot, path)
 }
 
 // transformCache is a transformer that adds volumes
@@ -218,14 +234,13 @@ func transformCache(c *common.Config, r *common.Repo) {
 	volumes := make([]string, cacheCount)
 
 	cache := cacheRoot(r)
-	workspace := workspaceRoot(r)
+	workspace := c.Clone.Config["path"].(string)
 
 	for i, dir := range c.Build.Cache {
 		cacheDir := filepath.Join(cache, dir)
 		workspaceDir := filepath.Join(workspace, dir)
 
 		volumes[i] = fmt.Sprintf("%s:%s", cacheDir, workspaceDir)
-		fmt.Printf("Volume %s", volumes[i])
 	}
 
 	c.Setup.Volumes = append(c.Setup.Volumes, volumes...)
@@ -272,7 +287,7 @@ func imageNameDefault(name, defaultName string) string {
 // workspaceRoot is a helper function that determines the
 // default workspace the build runs in.
 func workspaceRoot(r *common.Repo) string {
-  return filepath.Join("/drone/src", repoPath(r))
+	return filepath.Join(buildRoot, repoPath(r))
 }
 
 // cacheRoot is a helper function that deteremines the

pkg/yaml/transform/transform_test.go

@@ -1,6 +1,7 @@
 package transform
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/drone/drone/Godeps/_workspace/src/github.com/franela/goblin"
@@ -146,7 +147,9 @@ func Test_Transform(t *testing.T) {
 		g.It("Should have cached volumes", func() {
 			c := &common.Config{
 				Setup: &common.Step{},
-				Clone: &common.Step{},
+				Clone: &common.Step{
+					Config: map[string]interface{}{},
+				},
 				Build: &common.Step{
 					Cache: []string{".git", "foo", "bar"},
 				},
@@ -158,6 +161,7 @@ func Test_Transform(t *testing.T) {
 				Link:     "https://github.com/drone/drone",
 				FullName: "drone/drone",
 			}
+			transformWorkspace(c, r)
 			transformCache(c, r)
 
 			cacheCount := len(c.Build.Cache)
@@ -180,5 +184,37 @@ func Test_Transform(t *testing.T) {
 			testRange(c.Notify)
 			testRange(c.Compose)
 		})
+
+		g.It("Should have default workspace directory", func() {
+			c := &common.Config{
+				Clone: &common.Step{
+					Config: map[string]interface{}{},
+				},
+			}
+			r := &common.Repo{
+				Link:     "https://github.com/drone/drone",
+				FullName: "drone/drone",
+			}
+			transformWorkspace(c, r)
+
+			g.Assert(c.Clone.Config["path"]).Equal(workspaceRoot(r))
+		})
+
+		g.It("Should use path for working directory", func() {
+			c := &common.Config{
+				Clone: &common.Step{
+					Config: map[string]interface{}{
+						"path": "foo/bar",
+					},
+				},
+			}
+			r := &common.Repo{
+				Link:     "https://github.com/drone/drone",
+				FullName: "drone/drone",
+			}
+			transformWorkspace(c, r)
+
+			g.Assert(c.Clone.Config["path"]).Equal(filepath.Join(buildRoot, "foo/bar"))
+		})
 	})
 }