feat: absolute imports (#63)

* feat: go-like import style

jb now creates a directory structure inside of vendor/ that is similar to how go
does (github.com/grafana/jsonnet-libs). This is reflected in the final import
paths, which means they will be go-like

* refactor(spec/deps): named regexs

* feat: make goImportStyle configurable

Defaults to off, can be enabled in `jsonnetfile.json`

* fix: integration test

* doc: license headers

* fix(deps): remove GO_IMPORT_STYLE

not an option anymore, will always do so and symlink

* feat: symlink to legacy location

* feat: allow to disable legacy links

* fix(test): legacyImports in integration tests

* fix(spec): test

* fix: respect legacyName aliases

It was possible to alias packages by changing `name` previously.

While names are now absolute (and computed), legacy links should still respect
old aliases to avoid breaking code.

* fix(test): integration

* fix(init): keep legacyImports enabled for now

* feat: rewrite imports

adds a command to automatically rewrite imports from legacy to absolute style

* fix(tool): rewrite confused by prefixing packages

When a package was a prefix of another one, it broke.
Fixed that by using a proper regular expression. Added a test to make sure it
works as expected

* Update cmd/jb/init.go

* fix: exclude local packages from legacy linking

They actually still use the old style, which is fine. LegacyLinking
messed them up, but from now on it just ignores symlinks that match a localPackage.

cmd/jb/init.go

@@ -15,32 +15,37 @@
 package main
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"path/filepath"
 
 	kingpin "gopkg.in/alecthomas/kingpin.v2"
 
 	"github.com/jsonnet-bundler/jsonnet-bundler/pkg/jsonnetfile"
+	"github.com/jsonnet-bundler/jsonnet-bundler/spec"
 )
 
 func initCommand(dir string) int {
 	exists, err := jsonnetfile.Exists(jsonnetfile.File)
-	if err != nil {
-		kingpin.Errorf("Failed to check for jsonnetfile.json: %v", err)
-		return 1
-	}
+	kingpin.FatalIfError(err, "Failed to check for jsonnetfile.json")
 
 	if exists {
 		kingpin.Errorf("jsonnetfile.json already exists")
 		return 1
 	}
 
+	s := spec.New()
+	// TODO: disable them by default eventually
+	// s.LegacyImports = false
+
+	contents, err := json.MarshalIndent(s, "", "  ")
+	kingpin.FatalIfError(err, "formatting jsonnetfile contents as json")
+	contents = append(contents, []byte("\n")...)
+
 	filename := filepath.Join(dir, jsonnetfile.File)
 
-	if err := ioutil.WriteFile(filename, []byte("{}\n"), 0644); err != nil {
-		kingpin.Errorf("Failed to write new jsonnetfile.json: %v", err)
-		return 1
-	}
+	ioutil.WriteFile(filename, contents, 0644)
+	kingpin.FatalIfError(err, "Failed to write new jsonnetfile.json")
 
 	return 0
 }

cmd/jb/install.go

@@ -27,6 +27,7 @@ import (
 	"github.com/jsonnet-bundler/jsonnet-bundler/pkg"
 	"github.com/jsonnet-bundler/jsonnet-bundler/pkg/jsonnetfile"
 	"github.com/jsonnet-bundler/jsonnet-bundler/spec"
+	"github.com/jsonnet-bundler/jsonnet-bundler/spec/deps"
 )
 
 func installCommand(dir, jsonnetHome string, uris []string) int {
@@ -53,23 +54,25 @@ func installCommand(dir, jsonnetHome string, uris []string) int {
 		"creating vendor folder")
 
 	for _, u := range uris {
-		d := parseDependency(dir, u)
+		d := deps.Parse(dir, u)
 		if d == nil {
 			kingpin.Fatalf("Unable to parse package URI `%s`", u)
 		}
 
-		if !depEqual(jsonnetFile.Dependencies[d.Name], *d) {
+		if !depEqual(jsonnetFile.Dependencies[d.Name()], *d) {
 			// the dep passed on the cli is different from the jsonnetFile
-			jsonnetFile.Dependencies[d.Name] = *d
+			jsonnetFile.Dependencies[d.Name()] = *d
 
 			// we want to install the passed version (ignore the lock)
-			delete(lockFile.Dependencies, d.Name)
+			delete(lockFile.Dependencies, d.Name())
 		}
 	}
 
 	locked, err := pkg.Ensure(jsonnetFile, jsonnetHome, lockFile.Dependencies)
 	kingpin.FatalIfError(err, "failed to install packages")
 
+	pkg.CleanLegacyName(jsonnetFile.Dependencies)
+
 	kingpin.FatalIfError(
 		writeChangedJsonnetFile(jbfilebytes, &jsonnetFile, filepath.Join(dir, jsonnetfile.File)),
 		"updating jsonnetfile.json")
@@ -81,8 +84,8 @@ func installCommand(dir, jsonnetHome string, uris []string) int {
 	return 0
 }
 
-func depEqual(d1, d2 spec.Dependency) bool {
-	name := d1.Name == d2.Name
+func depEqual(d1, d2 deps.Dependency) bool {
+	name := d1.Name() == d2.Name()
 	version := d1.Version == d2.Version
 	source := reflect.DeepEqual(d1.Source, d2.Source)
 

cmd/jb/install_test.go

@@ -25,8 +25,12 @@ import (
 
 	"github.com/jsonnet-bundler/jsonnet-bundler/pkg/jsonnetfile"
 	"github.com/jsonnet-bundler/jsonnet-bundler/spec"
+	"github.com/jsonnet-bundler/jsonnet-bundler/spec/deps"
 )
 
+// TODO: Change legacyImports to false eventually
+const initContents = `{"dependencies": [], "legacyImports": true}`
+
 func TestInstallCommand(t *testing.T) {
 	testcases := []struct {
 		Name                    string
@@ -38,19 +42,21 @@ func TestInstallCommand(t *testing.T) {
 		{
 			Name:                "NoURLs",
 			ExpectedCode:        0,
-			ExpectedJsonnetFile: []byte(`{}`),
-		}, {
+			ExpectedJsonnetFile: []byte(initContents),
+		},
+		{
 			Name:                    "OneURL",
 			URIs:                    []string{"github.com/jsonnet-bundler/jsonnet-bundler@v0.1.0"},
 			ExpectedCode:            0,
-			ExpectedJsonnetFile:     []byte(`{"dependencies": [{"name": "jsonnet-bundler", "source": {"git": {"remote": "https://github.com/jsonnet-bundler/jsonnet-bundler", "subdir": ""}}, "version": "v0.1.0"}]}`),
-			ExpectedJsonnetLockFile: []byte(`{"dependencies": [{"name": "jsonnet-bundler", "source": {"git": {"remote": "https://github.com/jsonnet-bundler/jsonnet-bundler", "subdir": ""}}, "version": "080f157c7fb85ad0281ea78f6c641eaa570a582f", "sum": "W1uI550rQ66axRpPXA2EZDquyPg/5PHZlvUz1NEzefg="}]}`),
-		}, {
-			Name:                    "Relative",
+			ExpectedJsonnetFile:     []byte(`{"dependencies": [{"source": {"git": {"remote": "https://github.com/jsonnet-bundler/jsonnet-bundler", "subdir": ""}}, "version": "v0.1.0"}], "legacyImports": true}`),
+			ExpectedJsonnetLockFile: []byte(`{"dependencies": [{"source": {"git": {"remote": "https://github.com/jsonnet-bundler/jsonnet-bundler", "subdir": ""}}, "version": "080f157c7fb85ad0281ea78f6c641eaa570a582f", "sum": "W1uI550rQ66axRpPXA2EZDquyPg/5PHZlvUz1NEzefg="}], "legacyImports": false}`),
+		},
+		{
+			Name:                    "Local",
 			URIs:                    []string{"jsonnet/foobar"},
 			ExpectedCode:            0,
-			ExpectedJsonnetFile:     []byte(`{"dependencies": [{"name": "foobar", "source": {"local": {"directory": "jsonnet/foobar"}}, "version": ""}]}`),
-			ExpectedJsonnetLockFile: []byte(`{"dependencies": [{"name": "foobar", "source": {"local": {"directory": "jsonnet/foobar"}}, "version": ""}]}`),
+			ExpectedJsonnetFile:     []byte(`{"dependencies": [{"source": {"local": {"directory": "jsonnet/foobar"}}, "version": ""}], "legacyImports": true}`),
+			ExpectedJsonnetLockFile: []byte(`{"dependencies": [{"source": {"local": {"directory": "jsonnet/foobar"}}, "version": ""}], "legacyImports": false}`),
 		},
 	}
 
@@ -70,12 +76,12 @@ func TestInstallCommand(t *testing.T) {
 			err := os.MkdirAll(localDependency, os.ModePerm)
 			assert.NoError(t, err)
 
+			// init + check it works correctly (legacyImports true, empty dependencies)
 			initCommand("")
+			jsonnetFileContent(t, jsonnetfile.File, []byte(initContents))
 
-			jsonnetFileContent(t, jsonnetfile.File, []byte(`{}`))
-
+			// install something, check it writes only if required, etc.
 			installCommand("", "vendor", tc.URIs)
-
 			jsonnetFileContent(t, jsonnetfile.File, tc.ExpectedJsonnetFile)
 			if tc.ExpectedJsonnetLockFile != nil {
 				jsonnetFileContent(t, jsonnetfile.LockFile, tc.ExpectedJsonnetLockFile)
@@ -111,11 +117,11 @@ func TestWriteChangedJsonnetFile(t *testing.T) {
 		},
 		{
 			Name:             "NoDiffNotEmpty",
-			JsonnetFileBytes: []byte(`{"dependencies": [{"name": "foobar"}]}`),
+			JsonnetFileBytes: []byte(`{"dependencies": [{"version": "master"}]}`),
 			NewJsonnetFile: spec.JsonnetFile{
-				Dependencies: map[string]spec.Dependency{
-					"foobar": {
-						Name: "foobar",
+				Dependencies: map[string]deps.Dependency{
+					"": {
+						Version: "master",
 					},
 				},
 			},
@@ -123,11 +129,10 @@ func TestWriteChangedJsonnetFile(t *testing.T) {
 		},
 		{
 			Name:             "DiffVersion",
-			JsonnetFileBytes: []byte(`{"dependencies": [{"name": "foobar", "version": "1.0"}]}`),
+			JsonnetFileBytes: []byte(`{"dependencies": [{"version": "1.0"}]}`),
 			NewJsonnetFile: spec.JsonnetFile{
-				Dependencies: map[string]spec.Dependency{
-					"foobar": {
-						Name:    "foobar",
+				Dependencies: map[string]deps.Dependency{
+					"": {
 						Version: "2.0",
 					},
 				},
@@ -138,17 +143,18 @@ func TestWriteChangedJsonnetFile(t *testing.T) {
 			Name:             "Diff",
 			JsonnetFileBytes: []byte(`{}`),
 			NewJsonnetFile: spec.JsonnetFile{
-				Dependencies: map[string]spec.Dependency{
-					"foobar": {
-						Name: "foobar",
-						Source: spec.Source{
-							GitSource: &spec.GitSource{
-								Remote: "https://github.com/foobar/foobar",
+				Dependencies: map[string]deps.Dependency{
+					"github.com/foobar/foobar": {
+						Source: deps.Source{
+							GitSource: &deps.Git{
+								Scheme: deps.GitSchemeHTTPS,
+								Host:   "github.com",
+								User:   "foobar",
+								Repo:   "foobar",
 								Subdir: "",
 							},
 						},
-						Version:   "master",
-						DepSource: "",
+						Version: "master",
 					}},
 			},
 			ExpectWrite: true,

cmd/jb/main.go

@@ -17,32 +17,16 @@ package main
 import (
 	"fmt"
 	"os"
-	"path"
-	"path/filepath"
-	"regexp"
 
 	"github.com/fatih/color"
 	"github.com/pkg/errors"
-
-	"github.com/jsonnet-bundler/jsonnet-bundler/spec"
 )
 
 const (
 	installActionName = "install"
 	updateActionName  = "update"
 	initActionName    = "init"
-)
-
-var (
-	gitSSHRegex                   = regexp.MustCompile(`git\+ssh://git@([^:]+):([^/]+)/([^/]+).git`)
-	gitSSHWithVersionRegex        = regexp.MustCompile(`git\+ssh://git@([^:]+):([^/]+)/([^/]+).git@(.*)`)
-	gitSSHWithPathRegex           = regexp.MustCompile(`git\+ssh://git@([^:]+):([^/]+)/([^/]+).git/(.*)`)
-	gitSSHWithPathAndVersionRegex = regexp.MustCompile(`git\+ssh://git@([^:]+):([^/]+)/([^/]+).git/(.*)@(.*)`)
-
-	githubSlugRegex                   = regexp.MustCompile("github.com/([-_a-zA-Z0-9]+)/([-_a-zA-Z0-9]+)")
-	githubSlugWithVersionRegex        = regexp.MustCompile("github.com/([-_a-zA-Z0-9]+)/([-_a-zA-Z0-9]+)@(.*)")
-	githubSlugWithPathRegex           = regexp.MustCompile("github.com/([-_a-zA-Z0-9]+)/([-_a-zA-Z0-9]+)/(.*)")
-	githubSlugWithPathAndVersionRegex = regexp.MustCompile("github.com/([-_a-zA-Z0-9]+)/([-_a-zA-Z0-9]+)/(.*)@(.*)")
+	rewriteActionName = "rewrite"
 )
 
 func main() {
@@ -69,6 +53,8 @@ func Main() int {
 
 	updateCmd := a.Command(updateActionName, "Update all dependencies.")
 
+	rewriteCmd := a.Command(rewriteActionName, "Automatically rewrite legacy imports to absolute ones")
+
 	command, err := a.Parse(os.Args[1:])
 	if err != nil {
 		fmt.Fprintln(os.Stderr, errors.Wrapf(err, "Error parsing commandline arguments"))
@@ -88,148 +74,11 @@ func Main() int {
 		return installCommand(workdir, cfg.JsonnetHome, *installCmdURIs)
 	case updateCmd.FullCommand():
 		return updateCommand(workdir, cfg.JsonnetHome)
+	case rewriteCmd.FullCommand():
+		return rewriteCommand(workdir, cfg.JsonnetHome)
 	default:
 		installCommand(workdir, cfg.JsonnetHome, []string{})
 	}
 
 	return 0
 }
-
-func parseDependency(dir, uri string) *spec.Dependency {
-	if uri == "" {
-		return nil
-	}
-
-	if githubSlugRegex.MatchString(uri) {
-		return parseGithubDependency(uri)
-	}
-
-	if gitSSHRegex.MatchString(uri) {
-		return parseGitSSHDependency(uri)
-	}
-
-	return parseLocalDependency(dir, uri)
-}
-
-func parseGitSSHDependency(p string) *spec.Dependency {
-	subdir := ""
-	host := ""
-	org := ""
-	repo := ""
-	version := "master"
-
-	switch {
-	case gitSSHWithPathAndVersionRegex.MatchString(p):
-		matches := gitSSHWithPathAndVersionRegex.FindStringSubmatch(p)
-		host = matches[1]
-		org = matches[2]
-		repo = matches[3]
-		subdir = matches[4]
-		version = matches[5]
-	case gitSSHWithPathRegex.MatchString(p):
-		matches := gitSSHWithPathRegex.FindStringSubmatch(p)
-		host = matches[1]
-		org = matches[2]
-		repo = matches[3]
-		subdir = matches[4]
-	case gitSSHWithVersionRegex.MatchString(p):
-		matches := gitSSHWithVersionRegex.FindStringSubmatch(p)
-		host = matches[1]
-		org = matches[2]
-		repo = matches[3]
-		version = matches[4]
-	default:
-		matches := gitSSHRegex.FindStringSubmatch(p)
-		host = matches[1]
-		org = matches[2]
-		repo = matches[3]
-	}
-
-	return &spec.Dependency{
-		Name: repo,
-		Source: spec.Source{
-			GitSource: &spec.GitSource{
-				Remote: fmt.Sprintf("git@%s:%s/%s", host, org, repo),
-				Subdir: subdir,
-			},
-		},
-		Version: version,
-	}
-}
-
-func parseGithubDependency(p string) *spec.Dependency {
-	if !githubSlugRegex.MatchString(p) {
-		return nil
-	}
-
-	name := ""
-	user := ""
-	repo := ""
-	subdir := ""
-	version := "master"
-
-	if githubSlugWithPathRegex.MatchString(p) {
-		if githubSlugWithPathAndVersionRegex.MatchString(p) {
-			matches := githubSlugWithPathAndVersionRegex.FindStringSubmatch(p)
-			user = matches[1]
-			repo = matches[2]
-			subdir = matches[3]
-			version = matches[4]
-			name = path.Base(subdir)
-		} else {
-			matches := githubSlugWithPathRegex.FindStringSubmatch(p)
-			user = matches[1]
-			repo = matches[2]
-			subdir = matches[3]
-			name = path.Base(subdir)
-		}
-	} else {
-		if githubSlugWithVersionRegex.MatchString(p) {
-			matches := githubSlugWithVersionRegex.FindStringSubmatch(p)
-			user = matches[1]
-			repo = matches[2]
-			name = repo
-			version = matches[3]
-		} else {
-			matches := githubSlugRegex.FindStringSubmatch(p)
-			user = matches[1]
-			repo = matches[2]
-			name = repo
-		}
-	}
-
-	return &spec.Dependency{
-		Name: name,
-		Source: spec.Source{
-			GitSource: &spec.GitSource{
-				Remote: fmt.Sprintf("https://github.com/%s/%s", user, repo),
-				Subdir: subdir,
-			},
-		},
-		Version: version,
-	}
-}
-
-func parseLocalDependency(dir, p string) *spec.Dependency {
-	clean := filepath.Clean(p)
-	abs := filepath.Join(dir, clean)
-
-	info, err := os.Stat(abs)
-	if err != nil {
-		return nil
-	}
-
-	if !info.IsDir() {
-		return nil
-	}
-
-	return &spec.Dependency{
-		Name: info.Name(),
-		Source: spec.Source{
-			LocalSource: &spec.LocalSource{
-				Directory: clean,
-			},
-		},
-		Version: "",
-	}
-}

cmd/jb/main_test.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
-	"github.com/jsonnet-bundler/jsonnet-bundler/spec"
+	"github.com/jsonnet-bundler/jsonnet-bundler/spec/deps"
 )
 
 func TestParseDependency(t *testing.T) {
@@ -34,7 +34,7 @@ func TestParseDependency(t *testing.T) {
 	tests := []struct {
 		name string
 		path string
-		want *spec.Dependency
+		want *deps.Dependency
 	}{
 		{
 			name: "Empty",
@@ -49,11 +49,13 @@ func TestParseDependency(t *testing.T) {
 		{
 			name: "GitHub",
 			path: "github.com/jsonnet-bundler/jsonnet-bundler",
-			want: &spec.Dependency{
-				Name: "jsonnet-bundler",
-				Source: spec.Source{
-					GitSource: &spec.GitSource{
-						Remote: "https://github.com/jsonnet-bundler/jsonnet-bundler",
+			want: &deps.Dependency{
+				Source: deps.Source{
+					GitSource: &deps.Git{
+						Scheme: deps.GitSchemeHTTPS,
+						Host:   "github.com",
+						User:   "jsonnet-bundler",
+						Repo:   "jsonnet-bundler",
 						Subdir: "",
 					},
 				},
@@ -63,11 +65,13 @@ func TestParseDependency(t *testing.T) {
 		{
 			name: "SSH",
 			path: "git+ssh://git@github.com:jsonnet-bundler/jsonnet-bundler.git",
-			want: &spec.Dependency{
-				Name: "jsonnet-bundler",
-				Source: spec.Source{
-					GitSource: &spec.GitSource{
-						Remote: "git@github.com:jsonnet-bundler/jsonnet-bundler",
+			want: &deps.Dependency{
+				Source: deps.Source{
+					GitSource: &deps.Git{
+						Scheme: deps.GitSchemeSSH,
+						Host:   "github.com",
+						User:   "jsonnet-bundler",
+						Repo:   "jsonnet-bundler",
 						Subdir: "",
 					},
 				},
@@ -77,10 +81,9 @@ func TestParseDependency(t *testing.T) {
 		{
 			name: "local",
 			path: testFolder,
-			want: &spec.Dependency{
-				Name: "foobar",
-				Source: spec.Source{
-					LocalSource: &spec.LocalSource{
+			want: &deps.Dependency{
+				Source: deps.Source{
+					LocalSource: &deps.Local{
 						Directory: "test/jsonnet/foobar",
 					},
 				},
@@ -90,7 +93,7 @@ func TestParseDependency(t *testing.T) {
 	}
 	for _, tt := range tests {
 		_ = t.Run(tt.name, func(t *testing.T) {
-			dependency := parseDependency("", tt.path)
+			dependency := deps.Parse("", tt.path)
 
 			if tt.path == "" {
 				assert.Nil(t, dependency)

cmd/jb/rewrite.go

@@ -0,0 +1,36 @@
+// Copyright 2018 jsonnet-bundler authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package main
+
+import (
+	"path/filepath"
+
+	kingpin "gopkg.in/alecthomas/kingpin.v2"
+
+	"github.com/jsonnet-bundler/jsonnet-bundler/pkg/jsonnetfile"
+	"github.com/jsonnet-bundler/jsonnet-bundler/tool/rewrite"
+)
+
+func rewriteCommand(dir, vendorDir string) int {
+	locks, err := jsonnetfile.Load(filepath.Join(dir, jsonnetfile.LockFile))
+	if err != nil {
+		kingpin.Fatalf("Failed to load lockFile: %s.\nThe locks are required to compute the new import names. Make sure to run `jb install` first.", err)
+	}
+
+	if err := rewrite.Rewrite(dir, vendorDir, locks.Dependencies); err != nil {
+		kingpin.FatalIfError(err, "")
+	}
+
+	return 0
+}

cmd/jb/update.go

@@ -24,6 +24,7 @@ import (
 	"github.com/jsonnet-bundler/jsonnet-bundler/pkg"
 	"github.com/jsonnet-bundler/jsonnet-bundler/pkg/jsonnetfile"
 	"github.com/jsonnet-bundler/jsonnet-bundler/spec"
+	"github.com/jsonnet-bundler/jsonnet-bundler/spec/deps"
 )
 
 func updateCommand(dir, jsonnetHome string, urls ...*url.URL) int {
@@ -39,7 +40,7 @@ func updateCommand(dir, jsonnetHome string, urls ...*url.URL) int {
 		"creating vendor folder")
 
 	// When updating, locks are ignored.
-	locks := map[string]spec.Dependency{}
+	locks := map[string]deps.Dependency{}
 	locked, err := pkg.Ensure(jsonnetFile, jsonnetHome, locks)
 	kingpin.FatalIfError(err, "failed to install packages")