Merge pull request #14025 from gabemontero/new-app-clone-pr-ref

Merged by openshift-bot

pkg/build/builder/common.go

@@ -41,6 +41,7 @@ type GitClient interface {
 	CloneWithOptions(dir string, url string, args ...string) error
 	Fetch(dir string, url string, ref string) error
 	Checkout(dir string, ref string) error
+	PotentialPRRetryAsFetch(dir string, url string, ref string, err error) error
 	SubmoduleUpdate(dir string, init, recursive bool) error
 	TimedListRemote(timeout time.Duration, url string, args ...string) (string, string, error)
 	GetInfo(location string) (*git.SourceInfo, []error)

pkg/build/builder/source.go

@@ -259,17 +259,10 @@ func extractGitSource(ctx context.Context, gitClient GitClient, gitSource *api.G
 		}
 
 		if err := gitClient.Checkout(dir, commit); err != nil {
-			glog.V(4).Infof("Checkout after clone failed for ref %s with error: %v, attempting fetch", commit, err)
-			err = gitClient.Fetch(dir, gitSource.URI, commit)
+			err = gitClient.PotentialPRRetryAsFetch(dir, gitSource.URI, commit, err)
 			if err != nil {
 				return true, err
 			}
-
-			err = gitClient.Checkout(dir, "FETCH_HEAD")
-			if err != nil {
-				return true, err
-			}
-			glog.V(4).Infof("Fetch  / checkout for %s successful", commit)
 		}
 
 		// Recursively update --init

pkg/cmd/cli/cmd/startbuild.go

@@ -581,7 +581,10 @@ func streamPathToBuild(repo git.Repository, in io.Reader, out io.Writer, client
 					return nil, err
 				}
 				if err := repo.Checkout(tempDirectory, commit); err != nil {
-					return nil, err
+					err = repo.PotentialPRRetryAsFetch(tempDirectory, path, commit, err)
+					if err != nil {
+						return nil, err
+					}
 				}
 
 				// We'll continue to use tar on the temp directory

pkg/generate/app/sourcelookup.go

@@ -590,7 +590,10 @@ func CloneAndCheckoutSources(repo git.Repository, remote, ref, localDir, context
 	}
 	if len(ref) > 0 {
 		if err := repo.Checkout(localDir, ref); err != nil {
-			return "", fmt.Errorf("unable to checkout ref %q in %q repository: %v", ref, remote, err)
+			err = repo.PotentialPRRetryAsFetch(localDir, remote, ref, err)
+			if err != nil {
+				return "", fmt.Errorf("unable to checkout ref %q in %q repository: %v", ref, remote, err)
+			}
 		}
 	}
 	if len(contextDir) > 0 {

pkg/generate/git/repository.go

@@ -31,6 +31,7 @@ type Repository interface {
 	CloneMirror(dir string, url string) error
 	Fetch(dir string, url string, ref string) error
 	Checkout(dir string, ref string) error
+	PotentialPRRetryAsFetch(dir string, url string, ref string, err error) error
 	SubmoduleUpdate(dir string, init, recursive bool) error
 	Archive(dir, ref, format string, w io.Writer) error
 	Init(dir string, bare bool) error
@@ -127,6 +128,23 @@ func IsBareRoot(path string) (bool, error) {
 	return true, nil
 }
 
+// PotentialPRRetryAsFetch is used on checkout errors after a clone where the possibility
+// that a fetch or a PR ref is needed between the clone and checkout operations
+func (r *repository) PotentialPRRetryAsFetch(dir, remote, ref string, err error) error {
+	glog.V(4).Infof("Checkout after clone failed for ref %s with error: %v, attempting fetch", ref, err)
+	err = r.Fetch(dir, remote, ref)
+	if err != nil {
+		return err
+	}
+
+	err = r.Checkout(dir, "FETCH_HEAD")
+	if err != nil {
+		return err
+	}
+	glog.V(4).Infof("Fetch  / checkout for %s successful", ref)
+	return nil
+}
+
 // GetRootDir obtains the directory root for a Git repository
 func (r *repository) GetRootDir(location string) (string, error) {
 	dir, _, err := r.git(location, "rev-parse", "--git-dir")

test/cmd/newapp.sh

@@ -303,6 +303,9 @@ os::cmd::expect_failure_and_text 'oc new-app --dry-run mysq' 'error: only a part
 os::cmd::expect_failure_and_text 'oc new-app --dry-run mysq' 'The argument "mysq" only partially matched'
 os::cmd::expect_failure_and_text 'oc new-app --dry-run mysq' "Image stream \"mysql\" \\(tag \"5.7\"\\) in project"
 
+# ensure new-app with pr ref does not fail
+os::cmd::expect_success 'oc new-app https://github.com/openshift/ruby-hello-world#refs/pull/58/head --dry-run'
+
 # verify image streams with no tags are reported correctly and that --allow-missing-imagestream-tags works
 # new-app
 os::cmd::expect_success 'printf "apiVersion: v1\nkind: ImageStream\nmetadata:\n  name: emptystream\n" | oc create -f -'