From 2a58253d20ad6d48c55f138f0c92f62544fa7c28 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 10 Aug 2023 11:36:42 +0200 Subject: [PATCH] Consolidate functions in libs/git (#652) ## Changes The functions in `libs/git/git.go` assumed global state (e.g. working directory) and were no longer used. This change consolidates the functionality to turn an origin URL into an HTTPS URL. Closes #187. ## Tests Expanded existing unit test. --- libs/git/git.go | 80 ------------------------------------------ libs/git/git_test.go | 22 ------------ libs/git/utils.go | 24 ++++++------- libs/git/utils_test.go | 15 ++++++-- 4 files changed, 24 insertions(+), 117 deletions(-) delete mode 100644 libs/git/git.go delete mode 100644 libs/git/git_test.go diff --git a/libs/git/git.go b/libs/git/git.go deleted file mode 100644 index c5d09034..00000000 --- a/libs/git/git.go +++ /dev/null @@ -1,80 +0,0 @@ -package git - -import ( - "fmt" - "net/url" - "os" - "path" - "strings" - - "github.com/databricks/cli/folders" - giturls "github.com/whilp/git-urls" - "gopkg.in/ini.v1" -) - -func Root() (string, error) { - wd, err := os.Getwd() - if err != nil { - return "", err - } - return folders.FindDirWithLeaf(wd, ".git") -} - -// Origin finds the git repository the project is cloned from, so that -// we could automatically verify if this project is checked out in repos -// home folder of the user according to recommended best practices. Can -// also be used to determine a good enough default project name. -func Origin() (*url.URL, error) { - root, err := Root() - if err != nil { - return nil, err - } - file := fmt.Sprintf("%s/.git/config", root) - gitConfig, err := ini.Load(file) - if err != nil { - return nil, err - } - section := gitConfig.Section(`remote "origin"`) - if section == nil { - return nil, fmt.Errorf("remote `origin` is not defined in %s", file) - } - url := section.Key("url") - if url == nil { - return nil, fmt.Errorf("git origin url is not defined") - } - return giturls.Parse(url.Value()) -} - -// HttpsOrigin returns URL in the format expected by Databricks Repos -// platform functionality. Gradually expand implementation to work with -// other formats of git URLs. -func HttpsOrigin() (string, error) { - origin, err := Origin() - if err != nil { - return "", err - } - // if current repo is checked out with a SSH key - if origin.Scheme != "https" { - origin.Scheme = "https" - } - // `git@` is not required for HTTPS, as Databricks Repos are checked - // out using an API token instead of username. But does it hold true - // for all of the git implementations? - if origin.User != nil { - origin.User = nil - } - // Remove `.git` suffix, if present. - origin.Path = strings.TrimSuffix(origin.Path, ".git") - return origin.String(), nil -} - -// RepositoryName returns repository name as last path entry from detected -// git repository up the tree or returns error if it fails to do so. -func RepositoryName() (string, error) { - origin, err := Origin() - if err != nil { - return "", err - } - base := path.Base(origin.Path) - return strings.TrimSuffix(base, ".git"), nil -} diff --git a/libs/git/git_test.go b/libs/git/git_test.go deleted file mode 100644 index 818ba842..00000000 --- a/libs/git/git_test.go +++ /dev/null @@ -1,22 +0,0 @@ -package git - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestGetGitOrigin(t *testing.T) { - this, err := RepositoryName() - assert.NoError(t, err) - assert.Equal(t, "cli", this) -} - -func TestHttpsOrigin(t *testing.T) { - url, err := HttpsOrigin() - assert.NoError(t, err) - // must pass on the upcoming forks - assert.True(t, strings.HasPrefix(url, "https://github.com"), url) - assert.True(t, strings.HasSuffix(url, "cli"), url) -} diff --git a/libs/git/utils.go b/libs/git/utils.go index 13ce2c9e..1d38da3a 100644 --- a/libs/git/utils.go +++ b/libs/git/utils.go @@ -6,23 +6,23 @@ import ( giturls "github.com/whilp/git-urls" ) +// Return an origin URL as an HTTPS URL. +// The transformations in this function are not guaranteed to work for all +// Git providers. They are only guaranteed to work for GitHub. func ToHttpsUrl(url string) (string, error) { - originUrl, err := giturls.Parse(url) + origin, err := giturls.Parse(url) if err != nil { return "", err } - if originUrl.Scheme == "https" { - return originUrl.String(), nil + // If this repository is checked out over SSH + if origin.Scheme != "https" { + origin.Scheme = "https" } - // if current repo is checked out with a SSH key - if originUrl.Scheme != "https" { - originUrl.Scheme = "https" - } - // `git@` is not required for HTTPS - if originUrl.User != nil { - originUrl.User = nil + // Basic auth is not applicable for an HTTPS URL. + if origin.User != nil { + origin.User = nil } // Remove `.git` suffix, if present. - originUrl.Path = strings.TrimSuffix(originUrl.Path, ".git") - return originUrl.String(), nil + origin.Path = strings.TrimSuffix(origin.Path, ".git") + return origin.String(), nil } diff --git a/libs/git/utils_test.go b/libs/git/utils_test.go index 52a912da..2a77cae1 100644 --- a/libs/git/utils_test.go +++ b/libs/git/utils_test.go @@ -7,7 +7,16 @@ import ( ) func TestToHttpsUrlForSsh(t *testing.T) { - url, err := ToHttpsUrl("user@foo.com:org/repo-name.git") - assert.NoError(t, err) - assert.Equal(t, "https://foo.com/org/repo-name", url) + for _, e := range []struct { + url string + expected string + }{ + {"user@foo.com:org/repo-name.git", "https://foo.com/org/repo-name"}, + {"git@github.com:databricks/cli.git", "https://github.com/databricks/cli"}, + {"https://github.com/databricks/cli.git", "https://github.com/databricks/cli"}, + } { + url, err := ToHttpsUrl(e.url) + assert.NoError(t, err) + assert.Equal(t, e.expected, url) + } }