Only fetch archive of subdirectories in workspaces (#462)

* Only fetch archive of subdirectories in workspaces

* Fetch additional files

* Add a comment

* Make fake file mux more generic

* Copy additional files into workspaces before creating git repo

* Add TODOs

* Fix directory check on Windows

* Hash local file path

* Fix typo in docstring

* More debug output

* Try this

* Remove debug output

* Sort names before mounting them

* Remove left-over slice

* Remove duplication in copying of files

* Hopefully fix copying

* Download additional files on way up from workspace to root dir

* Add a unit test for additional files

* Fix on Windows

* Debug output

* Do not use os.FileSeparator for URLs

* Update changelog entry

* Disable download of only workspace by default, add flag

* Add a docstring in schema

Author: mrnugget

CHANGELOG.md

@@ -13,7 +13,7 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Added
 
-- Experimental: [`workspaces` in campaign specs](https://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#workspaces) is now available to allow users to define multiple workspaces in a single repository. [#442](https://github.com/sourcegraph/src-cli/pull/442)
+- Experimental (requires Sourcegraph 3.25 or later): [`workspaces` in campaign specs](https://docs.sourcegraph.com/campaigns/references/campaign_spec_yaml_reference#workspaces) is now available to allow users to define multiple workspaces in a single repository. [#442](https://github.com/sourcegraph/src-cli/pull/442) and [#462](https://github.com/sourcegraph/src-cli/pull/462).
 - The `changesetTemplate.published` field can now also be used to address a specific changeset in a repository by adding `@branch-of-changeset` at the end of the pattern. See [#461](https://github.com/sourcegraph/src-cli/pull/461) for an example and details.
 
 ### Changed

cmd/src/campaigns_common.go

@@ -237,7 +237,7 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
 
 	fetcher := svc.NewRepoFetcher(flags.cacheDir, flags.cleanArchives)
 	for _, task := range tasks {
-		task.Archive = fetcher.Checkout(task.Repository)
+		task.Archive = fetcher.Checkout(task.Repository, task.ArchivePathToFetch())
 	}
 
 	opts := campaigns.ExecutorOpts{

internal/campaigns/bind_workspace.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -21,12 +22,16 @@ type dockerBindWorkspaceCreator struct {
 
 var _ WorkspaceCreator = &dockerBindWorkspaceCreator{}
 
-func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, steps []Step, zip string) (Workspace, error) {
-	w, err := wc.unzipToWorkspace(ctx, repo, zip)
+func (wc *dockerBindWorkspaceCreator) Create(ctx context.Context, repo *graphql.Repository, steps []Step, archive RepoZip) (Workspace, error) {
+	w, err := wc.unzipToWorkspace(ctx, repo, archive.Path())
 	if err != nil {
 		return nil, errors.Wrap(err, "unzipping the repository")
 	}
 
+	if err := wc.copyToWorkspace(ctx, w, archive.AdditionalFilePaths()); err != nil {
+		return nil, errors.Wrap(err, "copying additional files into workspace")
+	}
+
 	return w, errors.Wrap(wc.prepareGitRepo(ctx, w), "preparing local git repo")
 }
 
@@ -56,6 +61,44 @@ func (wc *dockerBindWorkspaceCreator) unzipToWorkspace(ctx context.Context, repo
 	return &dockerBindWorkspace{dir: workspace}, nil
 }
 
+func (wc *dockerBindWorkspaceCreator) copyToWorkspace(ctx context.Context, w *dockerBindWorkspace, files map[string]string) error {
+	for name, src := range files {
+		srcStat, err := os.Stat(src)
+		if err != nil {
+			return err
+		}
+
+		if !srcStat.Mode().IsRegular() {
+			return fmt.Errorf("%s is not a regular file", src)
+		}
+
+		srcFile, err := os.Open(src)
+		if err != nil {
+			return err
+		}
+
+		destPath := path.Join(w.dir, name)
+
+		destFile, err := prepareCopyDestinationFile(srcStat, destPath)
+		if err != nil {
+			return err
+		}
+		_, err = io.Copy(destFile, srcFile)
+		if err != nil {
+			return err
+		}
+
+		if cerr := destFile.Close(); cerr != nil {
+			return errors.Wrap(cerr, "closing destination file failed")
+		}
+		if cerr := srcFile.Close(); cerr != nil {
+			return errors.Wrap(cerr, "closing source file failed")
+		}
+	}
+
+	return nil
+}
+
 type dockerBindWorkspace struct {
 	dir string
 }
@@ -151,29 +194,10 @@ func unzip(zipFile, dest string) error {
 			continue
 		}
 
-		if err := mkdirAll(dest, filepath.Dir(f.Name), 0777); err != nil {
-			return err
-		}
-
-		outFile, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
+		outFile, err := prepareCopyDestinationFile(f.FileInfo(), fpath)
 		if err != nil {
 			return err
 		}
-
-		// Since the container might not run as the same user, we need to ensure
-		// that the file is globally writable. If the execute bit is normally
-		// set on the zipped up file, let's ensure we propagate that to the
-		// group and other permission bits too.
-		if f.Mode()&0111 != 0 {
-			if err := os.Chmod(outFile.Name(), 0777); err != nil {
-				return err
-			}
-		} else {
-			if err := os.Chmod(outFile.Name(), 0666); err != nil {
-				return err
-			}
-		}
-
 		rc, err := f.Open()
 		if err != nil {
 			outFile.Close()
@@ -196,6 +220,37 @@ func unzip(zipFile, dest string) error {
 	return nil
 }
 
+type moder interface {
+	Mode() os.FileMode
+}
+
+func prepareCopyDestinationFile(sourceInfo moder, dest string) (*os.File, error) {
+	if err := mkdirAll(filepath.Dir(dest), "", 0777); err != nil {
+		return nil, err
+	}
+
+	outFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, sourceInfo.Mode())
+	if err != nil {
+		return nil, errors.Wrap(err, "opening destination file failed")
+	}
+
+	// Since the container might not run as the same user, we need to ensure
+	// that the file is globally writable. If the execute bit is normally
+	// set on the zipped up file, let's ensure we propagate that to the
+	// group and other permission bits too.
+	if sourceInfo.Mode()&0111 != 0 {
+		err = os.Chmod(outFile.Name(), 0777)
+	} else {
+		err = os.Chmod(outFile.Name(), 0666)
+	}
+	if err != nil {
+		outFile.Close()
+		return nil, err
+	}
+
+	return outFile, nil
+}
+
 // Technically, this is a misnomer, since it might be a socket or block special,
 // but errPathExistsAsNonDir is just ugly for an internal type.
 type errPathExistsAsFile string

internal/campaigns/bind_workspace_test.go

@@ -30,23 +30,22 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 		DefaultBranch: &graphql.Branch{Name: "main", Target: graphql.Target{OID: "d34db33f"}},
 	}
 
-	archive := mockRepoArchive{
-		repo: repo,
-		files: map[string]string{
-			"README.md": "# Welcome to the README\n",
-		},
+	filesInZip := map[string]string{
+		"README.md": "# Welcome to the README\n",
 	}
 
+	fakeFilesTmpDir := workspaceTmpDir(t)
+
 	// Create a zip file for all the other tests to use.
-	f, err := ioutil.TempFile(workspaceTmpDir(t), "repo-zip-*")
+	f, err := ioutil.TempFile(fakeFilesTmpDir, "repo-zip-*")
 	if err != nil {
 		t.Fatal(err)
 	}
 	archivePath := f.Name()
 	t.Cleanup(func() { os.Remove(archivePath) })
 
 	zw := zip.NewWriter(f)
-	for name, body := range archive.files {
+	for name, body := range filesInZip {
 		f, err := zw.Create(name)
 		if err != nil {
 			t.Fatal(err)
@@ -58,11 +57,34 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 	zw.Close()
 	f.Close()
 
+	// Create "additional files" for the tests to use
+	additionalFiles := map[string]string{
+		".gitignore":   "This is the gitignore\n",
+		"another-file": "This is another file",
+	}
+	additionalFilePaths := map[string]string{}
+	for name, content := range additionalFiles {
+		f, err := ioutil.TempFile(fakeFilesTmpDir, name+"-*")
+		if err != nil {
+			t.Fatal(err)
+		}
+		filePath := f.Name()
+		t.Cleanup(func() { os.Remove(filePath) })
+
+		if _, err := f.Write([]byte(content)); err != nil {
+			t.Fatal(err)
+		}
+
+		additionalFilePaths[name] = filePath
+		f.Close()
+	}
+
 	t.Run("success", func(t *testing.T) {
 		testTempDir := workspaceTmpDir(t)
 
+		archive := &fakeRepoArchive{mockPath: archivePath}
 		creator := &dockerBindWorkspaceCreator{dir: testTempDir}
-		workspace, err := creator.Create(context.Background(), repo, nil, archivePath)
+		workspace, err := creator.Create(context.Background(), repo, nil, archive)
 		if err != nil {
 			t.Fatalf("unexpected error: %s", err)
 		}
@@ -72,8 +94,8 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 			t.Fatalf("error walking workspace: %s", err)
 		}
 
-		if !cmp.Equal(archive.files, haveUnzippedFiles) {
-			t.Fatalf("wrong files in workspace:\n%s", cmp.Diff(archive.files, haveUnzippedFiles))
+		if !cmp.Equal(filesInZip, haveUnzippedFiles) {
+			t.Fatalf("wrong files in workspace:\n%s", cmp.Diff(filesInZip, haveUnzippedFiles))
 		}
 	})
 
@@ -89,11 +111,44 @@ func TestDockerBindWorkspaceCreator_Create(t *testing.T) {
 		t.Cleanup(func() { os.Remove(badZipFile) })
 		badZip.Close()
 
+		badArchive := &fakeRepoArchive{mockPath: badZipFile}
+
 		creator := &dockerBindWorkspaceCreator{dir: testTempDir}
-		if _, err := creator.Create(context.Background(), repo, nil, badZipFile); err == nil {
+		if _, err := creator.Create(context.Background(), repo, nil, badArchive); err == nil {
 			t.Error("unexpected nil error")
 		}
 	})
+
+	t.Run("additional files", func(t *testing.T) {
+		testTempDir := workspaceTmpDir(t)
+
+		archive := &fakeRepoArchive{
+			mockPath:                archivePath,
+			mockAdditionalFilePaths: additionalFilePaths,
+		}
+
+		creator := &dockerBindWorkspaceCreator{dir: testTempDir}
+		workspace, err := creator.Create(context.Background(), repo, nil, archive)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+
+		haveUnzippedFiles, err := readWorkspaceFiles(workspace)
+		if err != nil {
+			t.Fatalf("error walking workspace: %s", err)
+		}
+
+		wantFiles := map[string]string{}
+		for name, content := range filesInZip {
+			wantFiles[name] = content
+		}
+		for name, content := range additionalFiles {
+			wantFiles[name] = content
+		}
+		if !cmp.Equal(wantFiles, haveUnzippedFiles) {
+			t.Fatalf("wrong files in workspace:\n%s", cmp.Diff(wantFiles, haveUnzippedFiles))
+		}
+	})
 }
 
 func TestMkdirAll(t *testing.T) {
@@ -250,7 +305,7 @@ func readWorkspaceFiles(workspace Workspace) (map[string]string, error) {
 			return err
 		}
 
-		if strings.HasPrefix(rel, ".git") {
+		if rel == ".git" || strings.HasPrefix(rel, ".git"+string(os.PathSeparator)) {
 			return nil
 		}
 
@@ -275,3 +330,20 @@ func dirContains(dir, filename string) (bool, error) {
 
 	return false, nil
 }
+
+var _ RepoZip = &fakeRepoArchive{}
+
+type fakeRepoArchive struct {
+	mockPath                string
+	mockAdditionalFilePaths map[string]string
+}
+
+func (f *fakeRepoArchive) Fetch(context.Context) error { return nil }
+func (f *fakeRepoArchive) Close() error                { return nil }
+func (f *fakeRepoArchive) Path() string                { return f.mockPath }
+func (f *fakeRepoArchive) AdditionalFilePaths() map[string]string {
+	if f.mockAdditionalFilePaths != nil {
+		return f.mockAdditionalFilePaths
+	}
+	return map[string]string{}
+}

internal/campaigns/campaign_spec.go

@@ -64,8 +64,9 @@ type ImportChangeset struct {
 }
 
 type WorkspaceConfiguration struct {
-	RootAtLocationOf string `json:"rootAtLocationOf,omitempty" yaml:"rootAtLocationOf"`
-	In               string `json:"in,omitempty" yaml:"in"`
+	RootAtLocationOf   string `json:"rootAtLocationOf,omitempty" yaml:"rootAtLocationOf"`
+	In                 string `json:"in,omitempty" yaml:"in"`
+	OnlyFetchWorkspace bool   `json:"onlyFetchWorkspace,omitempty" yaml:"onlyFetchWorkspace"`
 
 	glob glob.Glob
 }

internal/campaigns/executor.go

@@ -56,7 +56,15 @@ type Executor interface {
 
 type Task struct {
 	Repository *graphql.Repository
-	Path       string
+
+	// Path is the folder relative to the repository's root in which the steps
+	// should be executed.
+	Path string
+	// OnlyFetchWorkspace determines whether the repository archive contains
+	// the complete repository or just the files in Path (and additional files,
+	// see RepoFetcher).
+	// If Path is "" then this setting has no effect.
+	OnlyFetchWorkspace bool
 
 	Steps   []Step
 	Outputs map[string]interface{}
@@ -67,6 +75,13 @@ type Task struct {
 	Archive RepoZip `json:"-"`
 }
 
+func (t *Task) ArchivePathToFetch() string {
+	if t.OnlyFetchWorkspace {
+		return t.Path
+	}
+	return ""
+}
+
 func (t *Task) cacheKey() ExecutionCacheKey {
 	return ExecutionCacheKey{t}
 }