git URL parsing rewrite

hack/test-stirunimage.sh

@@ -82,11 +82,6 @@ test_debug "s2i build with relative path without file://"
 s2i build cakephp-ex openshift/php-55-centos7 test --loglevel=5 &> "${WORK_DIR}/s2i-rel-noproto.log"
 check_result $? "${WORK_DIR}/s2i-rel-noproto.log"
 
-test_debug "s2i build with relative path with file://"
-
-s2i build file://./cakephp-ex openshift/php-55-centos7 test --loglevel=5 &> "${WORK_DIR}/s2i-rel-proto.log"
-check_result $? "${WORK_DIR}/s2i-rel-proto.log"
-
 test_debug "s2i build with volume options"
 s2i build cakephp-ex openshift/php-55-centos7 test --volume "${WORK_DIR}:/home/:z" --loglevel=5 &> "${WORK_DIR}/s2i-volume-correct.log"
 check_result $? "${WORK_DIR}/s2i-volume-correct.log"

pkg/api/describe/describer.go

@@ -26,9 +26,6 @@ func Config(client docker.Client, config *api.Config) string {
 		describeBuilderImage(client, config, config.BuilderImage, out)
 		describeRuntimeImage(config, out)
 		fmt.Fprintf(out, "Source:\t%s\n", config.Source)
-		if len(config.Ref) > 0 {
-			fmt.Fprintf(out, "Source Ref:\t%s\n", config.Ref)
-		}
 		if len(config.ContextDir) > 0 {
 			fmt.Fprintf(out, "Context Directory:\t%s\n", config.ContextDir)
 		}

pkg/api/types.go

@@ -113,10 +113,7 @@ type Config struct {
 	IgnoreSubmodules bool
 
 	// Source URL describing the location of sources used to build the result image.
-	Source string
-
-	// Ref is a tag/branch to be used for build.
-	Ref string
+	Source *git.URL
 
 	// Tag is a result image tag name.
 	Tag string

pkg/api/validation/validation_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/openshift/source-to-image/pkg/api"
+	"github.com/openshift/source-to-image/pkg/scm/git"
 )
 
 func TestValidation(t *testing.T) {
@@ -14,7 +15,7 @@ func TestValidation(t *testing.T) {
 	}{
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				BuilderPullPolicy: api.DefaultBuilderPullPolicy,
@@ -23,7 +24,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				DockerNetworkMode: "foobar",
@@ -33,7 +34,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				DockerNetworkMode: api.NewDockerNetworkModeContainer("8d873e496bc3e80a1cb22e67f7de7be5b0633e27916b1144978d1419c0abfcdb"),
@@ -43,7 +44,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "",
+				Source:            nil,
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				DockerNetworkMode: api.NewDockerNetworkModeContainer("8d873e496bc3e80a1cb22e67f7de7be5b0633e27916b1144978d1419c0abfcdb"),
@@ -53,7 +54,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				BuilderPullPolicy: api.DefaultBuilderPullPolicy,
@@ -63,7 +64,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				BuilderPullPolicy: api.DefaultBuilderPullPolicy,
@@ -73,7 +74,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				BuilderPullPolicy: api.DefaultBuilderPullPolicy,
@@ -83,7 +84,7 @@ func TestValidation(t *testing.T) {
 		},
 		{
 			&api.Config{
-				Source:            "http://github.com/openshift/source",
+				Source:            git.MustParse("http://github.com/openshift/source"),
 				BuilderImage:      "openshift/builder",
 				DockerConfig:      &api.DockerConfig{Endpoint: "/var/run/docker.socket"},
 				BuilderPullPolicy: api.DefaultBuilderPullPolicy,

pkg/build/config.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/openshift/source-to-image/pkg/api"
 	"github.com/openshift/source-to-image/pkg/docker"
+	"github.com/openshift/source-to-image/pkg/scm/git"
 )
 
 // GenerateConfigFromLabels generates the S2I Config struct from the Docker
@@ -41,13 +42,17 @@ func GenerateConfigFromLabels(config *api.Config, metadata *docker.PullResult) e
 	}
 
 	if repo, ok := labels[api.DefaultNamespace+"build.source-location"]; ok {
-		config.Source = repo
+		source, err := git.Parse(repo)
+		if err != nil {
+			return fmt.Errorf("couldn't parse label %q: %v", api.DefaultNamespace+"source-location", err)
+		}
+		config.Source = source
 	} else {
 		return fmt.Errorf("required label %q not found in image", api.DefaultNamespace+"source-location")
 	}
 
 	config.ContextDir = labels[api.DefaultNamespace+"build.source-context-dir"]
-	config.Ref = labels[api.DefaultNamespace+"build.commit.ref"]
+	config.Source.URL.Fragment = labels[api.DefaultNamespace+"build.commit.ref"]
 
 	return nil
 }

pkg/build/strategies/onbuild/onbuild.go

@@ -56,12 +56,10 @@ func New(client docker.Client, config *api.Config, fs fs.FileSystem, overrides b
 
 	downloader := overrides.Downloader
 	if downloader == nil {
-		d, sourceURL, err := scm.DownloaderForSource(builder.fs, config.Source, config.ForceCopy)
+		downloader, err = scm.DownloaderForSource(builder.fs, config.Source, config.ForceCopy)
 		if err != nil {
 			return nil, err
 		}
-		downloader = d
-		config.Source = sourceURL
 	}
 
 	builder.source = onBuildSourceHandler{

pkg/build/strategies/sti/sti.go

@@ -150,14 +150,11 @@ func New(client dockerpkg.Client, config *api.Config, fs fs.FileSystem, override
 	// which would lead to replacing this quick short circuit (so this change is tactical)
 	builder.source = overrides.Downloader
 	if builder.source == nil && !config.Usage {
-		var downloader build.Downloader
-		var sourceURL string
-		downloader, sourceURL, err = scm.DownloaderForSource(builder.fs, config.Source, config.ForceCopy)
+		downloader, err := scm.DownloaderForSource(builder.fs, config.Source, config.ForceCopy)
 		if err != nil {
 			return nil, err
 		}
 		builder.source = downloader
-		config.Source = sourceURL
 	}
 	builder.garbage = build.NewDefaultCleaner(builder.fs, builder.docker)
 
@@ -351,7 +348,7 @@ func (builder *STI) Prepare(config *api.Config) error {
 	}
 
 	// fetch sources, for their .s2i/bin might contain s2i scripts
-	if len(config.Source) > 0 {
+	if config.Source != nil {
 		if builder.sourceInfo, err = builder.source.Download(config); err != nil {
 			builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(
 				utilstatus.ReasonFetchSourceFailed,

pkg/build/strategies/sti/sti_test.go

@@ -147,7 +147,7 @@ func (f *FakeDockerBuild) Build(*api.Config) (*api.Result, error) {
 
 func TestDefaultSource(t *testing.T) {
 	config := &api.Config{
-		Source:       "file://.",
+		Source:       git.MustParse("."),
 		DockerConfig: &api.DockerConfig{Endpoint: "unix:///var/run/docker.sock"},
 	}
 	client, err := docker.NewEngineAPIClient(config.DockerConfig)
@@ -158,7 +158,7 @@ func TestDefaultSource(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if config.Source == "" {
+	if config.Source == nil {
 		t.Errorf("Config.Source not set: %v", config.Source)
 	}
 	if _, ok := sti.source.(*file.File); !ok || sti.source == nil {
@@ -168,7 +168,7 @@ func TestDefaultSource(t *testing.T) {
 
 func TestEmptySource(t *testing.T) {
 	config := &api.Config{
-		Source:       "",
+		Source:       nil,
 		DockerConfig: &api.DockerConfig{Endpoint: "unix:///var/run/docker.sock"},
 	}
 	client, err := docker.NewEngineAPIClient(config.DockerConfig)
@@ -179,7 +179,7 @@ func TestEmptySource(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if config.Source != "" {
+	if config.Source != nil {
 		t.Errorf("Config.Source unexpectantly changed: %v", config.Source)
 	}
 	if _, ok := sti.source.(*empty.Noop); !ok || sti.source == nil {
@@ -587,18 +587,17 @@ func TestFetchSource(t *testing.T) {
 		gh := bh.git.(*test.FakeGit)
 
 		bh.config.WorkingDir = "/working-dir"
-		gh.ValidCloneSpecResult = true
+		bh.config.Source = git.MustParse("a-repo-source")
 		if ft.refSpecified {
-			bh.config.Ref = "a-branch"
+			bh.config.Source.URL.Fragment = "a-branch"
 		}
-		bh.config.Source = "a-repo-source"
 
 		expectedTargetDir := "/working-dir/upload/src"
 		_, e := bh.source.Download(bh.config)
 		if e != nil {
 			t.Errorf("Unexpected error %v [%d]", e, testNum)
 		}
-		if gh.CloneSource != "a-repo-source" {
+		if gh.CloneSource.StringNoFragment() != "a-repo-source" {
 			t.Errorf("Clone was not called with the expected source. Got %s, expected %s [%d]", gh.CloneSource, "a-source-repo-source", testNum)
 		}
 		if filepath.ToSlash(gh.CloneTarget) != expectedTargetDir {

pkg/cmd/cli/cmd/build.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/openshift/source-to-image/pkg/scm/git"
 	utilglog "github.com/openshift/source-to-image/pkg/util/glog"
 	"github.com/spf13/cobra"
 
@@ -27,6 +28,7 @@ var glog = utilglog.StderrLog
 
 // NewCmdBuild implements the S2I cli build command.
 func NewCmdBuild(cfg *api.Config) *cobra.Command {
+	var ref string
 	useConfig := false
 	oldScriptsFlag := ""
 	oldDestination := ""
@@ -52,7 +54,12 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
 
 			// If user specifies the arguments, then we override the stored ones
 			if len(args) >= 2 {
-				cfg.Source = args[0]
+				source, err := git.Parse(args[0])
+				if err != nil {
+					fmt.Fprintf(os.Stderr, "ERROR: couldn't parse %q: %v\n", args[0], err)
+				}
+				cfg.Source = source
+				cfg.Source.URL.Fragment = ref
 				cfg.BuilderImage = args[1]
 				if len(args) >= 3 {
 					cfg.Tag = args[2]
@@ -162,7 +169,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
 	buildCmd.Flags().BoolVar(&(cfg.RunImage), "run", false, "Run resulting image as part of invocation of this command")
 	buildCmd.Flags().BoolVar(&(cfg.IgnoreSubmodules), "ignore-submodules", false, "Ignore all git submodules when cloning application repository")
 	buildCmd.Flags().VarP(&(cfg.Environment), "env", "e", "Specify an single environment variable in NAME=VALUE format")
-	buildCmd.Flags().StringVarP(&(cfg.Ref), "ref", "r", "", "Specify a ref to check-out")
+	buildCmd.Flags().StringVarP(&(ref), "ref", "r", "", "Specify a ref to check-out")
 	buildCmd.Flags().StringVarP(&(cfg.AssembleUser), "assemble-user", "", "", "Specify the user to run assemble with")
 	buildCmd.Flags().StringVarP(&(cfg.ContextDir), "context-dir", "", "", "Specify the sub-directory inside the repository with the application sources")
 	buildCmd.Flags().StringVarP(&(cfg.ExcludeRegExp), "exclude", "", tar.DefaultExclusionPattern.String(), "Regular expression for selecting files from the source tree to exclude from the build, where the default excludes the '.git' directory (see https://golang.org/pkg/regexp for syntax, but note that \"\" will be interpreted as allow all files and exclude no files)")

pkg/config/config.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/openshift/source-to-image/pkg/api"
+	"github.com/openshift/source-to-image/pkg/scm/git"
 	utilglog "github.com/openshift/source-to-image/pkg/util/glog"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -31,7 +32,7 @@ type Config struct {
 func Save(config *api.Config, cmd *cobra.Command) {
 	c := Config{
 		BuilderImage: config.BuilderImage,
-		Source:       config.Source,
+		Source:       config.Source.String(),
 		Tag:          config.Tag,
 		Flags:        make(map[string]string),
 	}
@@ -73,8 +74,14 @@ func Restore(config *api.Config, cmd *cobra.Command) {
 		return
 	}
 
+	source, err := git.Parse(c.Source)
+	if err != nil {
+		glog.V(1).Infof("Unable to parse %s: %v", c.Source, err)
+		return
+	}
+
 	config.BuilderImage = c.BuilderImage
-	config.Source = c.Source
+	config.Source = source
 	config.Tag = c.Tag
 
 	envOverride := false

pkg/scm/downloaders/file/download.go

@@ -2,7 +2,6 @@ package file
 
 import (
 	"path/filepath"
-	"strings"
 
 	"github.com/openshift/source-to-image/pkg/api"
 	"github.com/openshift/source-to-image/pkg/scm/git"
@@ -18,14 +17,14 @@ type File struct {
 	fs.FileSystem
 }
 
-// Download copies sources from a local directory into the working directory
+// Download copies sources from a local directory into the working directory.
+// Caller guarantees that config.Source.IsLocal() is true.
 func (f *File) Download(config *api.Config) (*git.SourceInfo, error) {
 	config.WorkingSourceDir = filepath.Join(config.WorkingDir, api.Source)
-	source := strings.TrimPrefix(config.Source, "file://")
 
-	copySrc := source
+	copySrc := config.Source.LocalPath()
 	if len(config.ContextDir) > 0 {
-		copySrc = filepath.Join(source, config.ContextDir)
+		copySrc = filepath.Join(copySrc, config.ContextDir)
 	}
 
 	glog.V(1).Infof("Copying sources from %q to %q", copySrc, config.WorkingSourceDir)
@@ -37,7 +36,7 @@ func (f *File) Download(config *api.Config) (*git.SourceInfo, error) {
 	}
 
 	return &git.SourceInfo{
-		Location:   source,
+		Location:   config.Source.LocalPath(),
 		ContextDir: config.ContextDir,
 	}, nil
 }

pkg/scm/downloaders/file/download_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/openshift/source-to-image/pkg/api"
+	"github.com/openshift/source-to-image/pkg/scm/git"
 	testfs "github.com/openshift/source-to-image/pkg/test/fs"
 )
 
@@ -13,7 +14,7 @@ func TestDownload(t *testing.T) {
 	f := &File{fs}
 
 	config := &api.Config{
-		Source: "file:///foo",
+		Source: git.MustParse("/foo"),
 	}
 	info, err := f.Download(config)
 	if err != nil {
@@ -22,7 +23,7 @@ func TestDownload(t *testing.T) {
 	if fs.CopySource != "/foo" {
 		t.Errorf("Unexpected fs.CopySource %s", fs.CopySource)
 	}
-	if info.Location != config.Source[7:] || info.ContextDir != config.ContextDir {
+	if info.Location != config.Source.URL.Path || info.ContextDir != config.ContextDir {
 		t.Errorf("Unexpected info")
 	}
 }
@@ -32,7 +33,7 @@ func TestDownloadWithContext(t *testing.T) {
 	f := &File{fs}
 
 	config := &api.Config{
-		Source:     "file:///foo",
+		Source:     git.MustParse("/foo"),
 		ContextDir: "bar",
 	}
 	info, err := f.Download(config)
@@ -42,7 +43,7 @@ func TestDownloadWithContext(t *testing.T) {
 	if filepath.ToSlash(fs.CopySource) != "/foo/bar" {
 		t.Errorf("Unexpected fs.CopySource %s", fs.CopySource)
 	}
-	if info.Location != config.Source[7:] || info.ContextDir != config.ContextDir {
+	if info.Location != config.Source.URL.Path || info.ContextDir != config.ContextDir {
 		t.Errorf("Unexpected info")
 	}
 }