Merge pull request #18631 from tiborvass/cleanup-utils

Create `builder/dockerignore` and `pkg/gitutils` to clean up `utils`

api/client/build.go

@@ -16,10 +16,12 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/builder/dockerignore"
 	Cli "github.com/docker/docker/cli"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/fileutils"
+	"github.com/docker/docker/pkg/gitutils"
 	"github.com/docker/docker/pkg/httputils"
 	"github.com/docker/docker/pkg/jsonmessage"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -131,7 +133,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 
 	var excludes []string
 	if err == nil {
-		excludes, err = utils.ReadDockerIgnore(f)
+		excludes, err = dockerignore.ReadAll(f)
 		if err != nil {
 			return err
 		}
@@ -421,7 +423,7 @@ func getContextFromReader(r io.Reader, dockerfileName string) (absContextDir, re
 // path of the dockerfile in that context directory, and a non-nil error on
 // success.
 func getContextFromGitURL(gitURL, dockerfileName string) (absContextDir, relDockerfile string, err error) {
-	if absContextDir, err = utils.GitClone(gitURL); err != nil {
+	if absContextDir, err = gitutils.Clone(gitURL); err != nil {
 		return "", "", fmt.Errorf("unable to 'git clone' to temporary context directory: %v", err)
 	}
 

builder/dockerignore.go

@@ -3,8 +3,8 @@ package builder
 import (
 	"os"
 
+	"github.com/docker/docker/builder/dockerignore"
 	"github.com/docker/docker/pkg/fileutils"
-	"github.com/docker/docker/utils"
 )
 
 // DockerIgnoreContext wraps a ModifiableContext to add a method
@@ -27,7 +27,7 @@ type DockerIgnoreContext struct {
 // TODO: Don't require a ModifiableContext (use Context instead) and don't remove
 // files, instead handle a list of files to be excluded from the context.
 func (c DockerIgnoreContext) Process(filesToRemove []string) error {
-	dockerignore, err := c.Open(".dockerignore")
+	f, err := c.Open(".dockerignore")
 	// Note that a missing .dockerignore file isn't treated as an error
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -35,7 +35,7 @@ func (c DockerIgnoreContext) Process(filesToRemove []string) error {
 		}
 		return err
 	}
-	excludes, _ := utils.ReadDockerIgnore(dockerignore)
+	excludes, _ := dockerignore.ReadAll(f)
 	filesToRemove = append([]string{".dockerignore"}, filesToRemove...)
 	for _, fileToRemove := range filesToRemove {
 		rm, _ := fileutils.Matches(fileToRemove, excludes)

builder/dockerignore/dockerignore.go

@@ -0,0 +1,34 @@
+package dockerignore
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"path/filepath"
+	"strings"
+)
+
+// ReadAll reads a .dockerignore file and returns the list of file patterns
+// to ignore. Note this will trim whitespace from each line as well
+// as use GO's "clean" func to get the shortest/cleanest path for each.
+func ReadAll(reader io.ReadCloser) ([]string, error) {
+	if reader == nil {
+		return nil, nil
+	}
+	defer reader.Close()
+	scanner := bufio.NewScanner(reader)
+	var excludes []string
+
+	for scanner.Scan() {
+		pattern := strings.TrimSpace(scanner.Text())
+		if pattern == "" {
+			continue
+		}
+		pattern = filepath.Clean(pattern)
+		excludes = append(excludes, pattern)
+	}
+	if err := scanner.Err(); err != nil {
+		return nil, fmt.Errorf("Error reading .dockerignore: %v", err)
+	}
+	return excludes, nil
+}

builder/dockerignore/dockerignore_test.go

@@ -0,0 +1,55 @@
+package dockerignore
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestReadAll(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "dockerignore-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	di, err := ReadAll(nil)
+	if err != nil {
+		t.Fatalf("Expected not to have error, got %v", err)
+	}
+
+	if diLen := len(di); diLen != 0 {
+		t.Fatalf("Expected to have zero dockerignore entry, got %d", diLen)
+	}
+
+	diName := filepath.Join(tmpDir, ".dockerignore")
+	content := fmt.Sprintf("test1\n/test2\n/a/file/here\n\nlastfile")
+	err = ioutil.WriteFile(diName, []byte(content), 0777)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	diFd, err := os.Open(diName)
+	if err != nil {
+		t.Fatal(err)
+	}
+	di, err = ReadAll(diFd)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if di[0] != "test1" {
+		t.Fatalf("First element is not test1")
+	}
+	if di[1] != "/test2" {
+		t.Fatalf("Second element is not /test2")
+	}
+	if di[2] != "/a/file/here" {
+		t.Fatalf("Third element is not /a/file/here")
+	}
+	if di[3] != "lastfile" {
+		t.Fatalf("Fourth element is not lastfile")
+	}
+}

builder/git.go

@@ -4,12 +4,12 @@ import (
 	"os"
 
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/utils"
+	"github.com/docker/docker/pkg/gitutils"
 )
 
 // MakeGitContext returns a Context from gitURL that is cloned in a temporary directory.
 func MakeGitContext(gitURL string) (ModifiableContext, error) {
-	root, err := utils.GitClone(gitURL)
+	root, err := gitutils.Clone(gitURL)
 	if err != nil {
 		return nil, err
 	}

pkg/gitutils/gitutils.go

@@ -1,4 +1,4 @@
-package utils
+package gitutils
 
 import (
 	"fmt"
@@ -14,9 +14,9 @@ import (
 	"github.com/docker/docker/pkg/urlutil"
 )
 
-// GitClone clones a repository into a newly created directory which
+// Clone clones a repository into a newly created directory which
 // will be under "docker-build-git"
-func GitClone(remoteURL string) (string, error) {
+func Clone(remoteURL string) (string, error) {
 	if !urlutil.IsGitTransport(remoteURL) {
 		remoteURL = "https://" + remoteURL
 	}

pkg/gitutils/gitutils_test.go

@@ -1,4 +1,4 @@
-package utils
+package gitutils
 
 import (
 	"fmt"

utils/utils.go

@@ -1,7 +1,6 @@
 package utils
 
 import (
-	"bufio"
 	"crypto/sha1"
 	"encoding/hex"
 	"fmt"
@@ -244,31 +243,6 @@ func ValidateContextDirectory(srcPath string, excludes []string) error {
 	})
 }
 
-// ReadDockerIgnore reads a .dockerignore file and returns the list of file patterns
-// to ignore. Note this will trim whitespace from each line as well
-// as use GO's "clean" func to get the shortest/cleanest path for each.
-func ReadDockerIgnore(reader io.ReadCloser) ([]string, error) {
-	if reader == nil {
-		return nil, nil
-	}
-	defer reader.Close()
-	scanner := bufio.NewScanner(reader)
-	var excludes []string
-
-	for scanner.Scan() {
-		pattern := strings.TrimSpace(scanner.Text())
-		if pattern == "" {
-			continue
-		}
-		pattern = filepath.Clean(pattern)
-		excludes = append(excludes, pattern)
-	}
-	if err := scanner.Err(); err != nil {
-		return nil, fmt.Errorf("Error reading .dockerignore: %v", err)
-	}
-	return excludes, nil
-}
-
 // GetErrorMessage returns the human readable message associated with
 // the passed-in error. In some cases the default Error() func returns
 // something that is less than useful so based on its types this func

utils/utils_test.go

@@ -1,12 +1,6 @@
 package utils
 
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"testing"
-)
+import "testing"
 
 func TestReplaceAndAppendEnvVars(t *testing.T) {
 	var (
@@ -25,49 +19,3 @@ func TestReplaceAndAppendEnvVars(t *testing.T) {
 		t.Fatalf("expected TERM=xterm got '%s'", env[1])
 	}
 }
-
-func TestReadDockerIgnore(t *testing.T) {
-	tmpDir, err := ioutil.TempDir("", "dockerignore-test")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(tmpDir)
-
-	di, err := ReadDockerIgnore(nil)
-	if err != nil {
-		t.Fatalf("Expected not to have error, got %v", err)
-	}
-
-	if diLen := len(di); diLen != 0 {
-		t.Fatalf("Expected to have zero dockerignore entry, got %d", diLen)
-	}
-
-	diName := filepath.Join(tmpDir, ".dockerignore")
-	content := fmt.Sprintf("test1\n/test2\n/a/file/here\n\nlastfile")
-	err = ioutil.WriteFile(diName, []byte(content), 0777)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	diFd, err := os.Open(diName)
-	if err != nil {
-		t.Fatal(err)
-	}
-	di, err = ReadDockerIgnore(diFd)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if di[0] != "test1" {
-		t.Fatalf("First element is not test1")
-	}
-	if di[1] != "/test2" {
-		t.Fatalf("Second element is not /test2")
-	}
-	if di[2] != "/a/file/here" {
-		t.Fatalf("Third element is not /a/file/here")
-	}
-	if di[3] != "lastfile" {
-		t.Fatalf("Fourth element is not lastfile")
-	}
-}