Merge pull request #32502 from tonistiigi/git-allow-pr-number

Allow specifying any remote ref in git checkout URLs

docs/reference/commandline/build.md

@@ -74,12 +74,12 @@ pre-packaged tarball contexts and plain text files.
 ### Git repositories
 
 When the `URL` parameter points to the location of a Git repository, the
-repository acts as the build context. The system recursively clones the
-repository and its submodules using a `git clone --depth 1 --recursive`
-command. This command runs in a temporary directory on your local host. After
-the command succeeds, the directory is sent to the Docker daemon as the
-context. Local clones give you the ability to access private repositories using
-local user credentials, VPN's, and so forth.
+repository acts as the build context. The system recursively fetches the
+repository and its submodules. The commit history is not preserved. A
+repository is first pulled into a temporary directory on your local host. After
+the that succeeds, the directory is sent to the Docker daemon as the context.
+Local copy gives you the ability to access private repositories using local
+user credentials, VPN's, and so forth.
 
 > **Note:**
 > If the `URL` parameter contains a fragment the system will recursively clone
@@ -87,8 +87,9 @@ local user credentials, VPN's, and so forth.
 
 Git URLs accept context configuration in their fragment section, separated by a
 colon `:`.  The first part represents the reference that Git will check out,
-this can be either a branch, a tag, or a commit SHA. The second part represents
-a subdirectory inside the repository that will be used as a build context.
+this can be either a branch, a tag, or a remote reference. The second part
+represents a subdirectory inside the repository that will be used as a build
+context.
 
 For example, run this command to use a directory called `docker` in the branch
 `container`:
@@ -105,12 +106,11 @@ Build Syntax Suffix             | Commit Used           | Build Context Used
 `myrepo.git`                    | `refs/heads/master`   | `/`
 `myrepo.git#mytag`              | `refs/tags/mytag`     | `/`
 `myrepo.git#mybranch`           | `refs/heads/mybranch` | `/`
-`myrepo.git#abcdef`             | `sha1 = abcdef`       | `/`
+`myrepo.git#pull/42/head`       | `refs/pull/42/head`   | `/`
 `myrepo.git#:myfolder`          | `refs/heads/master`   | `/myfolder`
 `myrepo.git#master:myfolder`    | `refs/heads/master`   | `/myfolder`
 `myrepo.git#mytag:myfolder`     | `refs/tags/mytag`     | `/myfolder`
 `myrepo.git#mybranch:myfolder`  | `refs/heads/mybranch` | `/myfolder`
-`myrepo.git#abcdef:myfolder`    | `sha1 = abcdef`       | `/myfolder`
 
 
 ### Tarball contexts

pkg/gitutils/gitutils.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/urlutil"
+	"github.com/pkg/errors"
 )
 
 // Clone clones a repository into a newly created directory which
@@ -30,21 +31,45 @@ func Clone(remoteURL string) (string, error) {
 		return "", err
 	}
 
-	fragment := u.Fragment
-	clone := cloneArgs(u, root)
+	if out, err := gitWithinDir(root, "init"); err != nil {
+		return "", errors.Wrapf(err, "failed to init repo at %s: %s", root, out)
+	}
+
+	ref, subdir := getRefAndSubdir(u.Fragment)
+	fetch := fetchArgs(u, ref)
+
+	u.Fragment = ""
+
+	// Add origin remote for compatibility with previous implementation that
+	// used "git clone" and also to make sure local refs are created for branches
+	if out, err := gitWithinDir(root, "remote", "add", "origin", u.String()); err != nil {
+		return "", errors.Wrapf(err, "failed add origin repo at %s: %s", u.String(), out)
+	}
 
-	if output, err := git(clone...); err != nil {
-		return "", fmt.Errorf("Error trying to use git: %s (%s)", err, output)
+	if output, err := gitWithinDir(root, fetch...); err != nil {
+		return "", errors.Wrapf(err, "error fetching: %s", output)
 	}
 
-	return checkoutGit(fragment, root)
+	return checkoutGit(root, ref, subdir)
+}
+
+func getRefAndSubdir(fragment string) (ref string, subdir string) {
+	refAndDir := strings.SplitN(fragment, ":", 2)
+	ref = "master"
+	if len(refAndDir[0]) != 0 {
+		ref = refAndDir[0]
+	}
+	if len(refAndDir) > 1 && len(refAndDir[1]) != 0 {
+		subdir = refAndDir[1]
+	}
+	return
 }
 
-func cloneArgs(remoteURL *url.URL, root string) []string {
-	args := []string{"clone", "--recursive"}
-	shallow := len(remoteURL.Fragment) == 0
+func fetchArgs(remoteURL *url.URL, ref string) []string {
+	args := []string{"fetch", "--recurse-submodules=yes"}
+	shallow := true
 
-	if shallow && strings.HasPrefix(remoteURL.Scheme, "http") {
+	if strings.HasPrefix(remoteURL.Scheme, "http") {
 		res, err := http.Head(fmt.Sprintf("%s/info/refs?service=git-upload-pack", remoteURL))
 		if err != nil || res.Header.Get("Content-Type") != "application/x-git-upload-pack-advertisement" {
 			shallow = false
@@ -55,26 +80,23 @@ func cloneArgs(remoteURL *url.URL, root string) []string {
 		args = append(args, "--depth", "1")
 	}
 
-	if remoteURL.Fragment != "" {
-		remoteURL.Fragment = ""
-	}
-
-	return append(args, remoteURL.String(), root)
+	return append(args, "origin", ref)
 }
 
-func checkoutGit(fragment, root string) (string, error) {
-	refAndDir := strings.SplitN(fragment, ":", 2)
-
-	if len(refAndDir[0]) != 0 {
-		if output, err := gitWithinDir(root, "checkout", refAndDir[0]); err != nil {
-			return "", fmt.Errorf("Error trying to use git: %s (%s)", err, output)
+func checkoutGit(root, ref, subdir string) (string, error) {
+	// Try checking out by ref name first. This will work on branches and sets
+	// .git/HEAD to the current branch name
+	if output, err := gitWithinDir(root, "checkout", ref); err != nil {
+		// If checking out by branch name fails check out the last fetched ref
+		if _, err2 := gitWithinDir(root, "checkout", "FETCH_HEAD"); err2 != nil {
+			return "", errors.Wrapf(err, "error checking out %s: %s", ref, output)
 		}
 	}
 
-	if len(refAndDir) > 1 && len(refAndDir[1]) != 0 {
-		newCtx, err := symlink.FollowSymlinkInScope(filepath.Join(root, refAndDir[1]), root)
+	if subdir != "" {
+		newCtx, err := symlink.FollowSymlinkInScope(filepath.Join(root, subdir), root)
 		if err != nil {
-			return "", fmt.Errorf("Error setting git context, %q not within git root: %s", refAndDir[1], err)
+			return "", errors.Wrapf(err, "error setting git context, %q not within git root", subdir)
 		}
 
 		fi, err := os.Stat(newCtx)
@@ -82,7 +104,7 @@ func checkoutGit(fragment, root string) (string, error) {
 			return "", err
 		}
 		if !fi.IsDir() {
-			return "", fmt.Errorf("Error setting git context, not a directory: %s", newCtx)
+			return "", errors.Errorf("error setting git context, not a directory: %s", newCtx)
 		}
 		root = newCtx
 	}

pkg/gitutils/gitutils_test.go

@@ -20,15 +20,14 @@ func TestCloneArgsSmartHttp(t *testing.T) {
 	serverURL, _ := url.Parse(server.URL)
 
 	serverURL.Path = "/repo.git"
-	gitURL := serverURL.String()
 
 	mux.HandleFunc("/repo.git/info/refs", func(w http.ResponseWriter, r *http.Request) {
 		q := r.URL.Query().Get("service")
 		w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-advertisement", q))
 	})
 
-	args := cloneArgs(serverURL, "/tmp")
-	exp := []string{"clone", "--recursive", "--depth", "1", gitURL, "/tmp"}
+	args := fetchArgs(serverURL, "master")
+	exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"}
 	if !reflect.DeepEqual(args, exp) {
 		t.Fatalf("Expected %v, got %v", exp, args)
 	}
@@ -40,14 +39,13 @@ func TestCloneArgsDumbHttp(t *testing.T) {
 	serverURL, _ := url.Parse(server.URL)
 
 	serverURL.Path = "/repo.git"
-	gitURL := serverURL.String()
 
 	mux.HandleFunc("/repo.git/info/refs", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/plain")
 	})
 
-	args := cloneArgs(serverURL, "/tmp")
-	exp := []string{"clone", "--recursive", gitURL, "/tmp"}
+	args := fetchArgs(serverURL, "master")
+	exp := []string{"fetch", "--recurse-submodules=yes", "origin", "master"}
 	if !reflect.DeepEqual(args, exp) {
 		t.Fatalf("Expected %v, got %v", exp, args)
 	}
@@ -55,17 +53,8 @@ func TestCloneArgsDumbHttp(t *testing.T) {
 
 func TestCloneArgsGit(t *testing.T) {
 	u, _ := url.Parse("git://github.com/docker/docker")
-	args := cloneArgs(u, "/tmp")
-	exp := []string{"clone", "--recursive", "--depth", "1", "git://github.com/docker/docker", "/tmp"}
-	if !reflect.DeepEqual(args, exp) {
-		t.Fatalf("Expected %v, got %v", exp, args)
-	}
-}
-
-func TestCloneArgsStripFragment(t *testing.T) {
-	u, _ := url.Parse("git://github.com/docker/docker#test")
-	args := cloneArgs(u, "/tmp")
-	exp := []string{"clone", "--recursive", "git://github.com/docker/docker", "/tmp"}
+	args := fetchArgs(u, "master")
+	exp := []string{"fetch", "--recurse-submodules=yes", "--depth", "1", "origin", "master"}
 	if !reflect.DeepEqual(args, exp) {
 		t.Fatalf("Expected %v, got %v", exp, args)
 	}
@@ -198,7 +187,8 @@ func TestCheckoutGit(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		r, err := checkoutGit(c.frag, gitDir)
+		ref, subdir := getRefAndSubdir(c.frag)
+		r, err := checkoutGit(gitDir, ref, subdir)
 
 		fail := err != nil
 		if fail != c.fail {