From f4b09c811ec291eb111d70c018803f89394ae16e Mon Sep 17 00:00:00 2001 From: Benoit Gagnon Date: Wed, 23 Oct 2019 10:52:39 -0400 Subject: [PATCH 1/6] replace ETag SHA1 assumption with an explicit call to git ls-remote Not using an undocumented ETag header from the GitHub archive API is probably for the best. This is slightly slower due to extra round-trip, but it is still much faster than cloning the repository to resolve the ref. Signed-off-by: Benoit Gagnon --- pkg/git.go | 56 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/pkg/git.go b/pkg/git.go index 25907e4..e08bb85 100644 --- a/pkg/git.go +++ b/pkg/git.go @@ -45,47 +45,33 @@ func NewGitPackage(source *spec.GitSource) Interface { } } -func downloadGitHubArchive(filepath string, url string) (string, error) { +func downloadGitHubArchive(filepath string, url string) error { // Get the data resp, err := http.Get(url) if err != nil { - return "", err + return err } color.Cyan("GET %s %d", url, resp.StatusCode) if resp.StatusCode != 200 { - return "", errors.New(fmt.Sprintf("unexpected status code %d", resp.StatusCode)) + return errors.New(fmt.Sprintf("unexpected status code %d", resp.StatusCode)) } - // GitHub conveniently uses the commit SHA1 at the ETag - // signature for the archive. This is needed when doing `jb update` - // to resolve a ref (ie. "master") to a commit SHA1 for the lock file - etagValue := resp.Header.Get(http.CanonicalHeaderKey("ETag")) - if etagValue == "" { - return "", errors.New("ETag header is missing from response") - } - - commitShaPattern, _ := regexp.Compile("^\"([0-9a-f]{40})\"$") - m := commitShaPattern.FindStringSubmatch(etagValue) - if len(m) < 2 { - return "", errors.New(fmt.Sprintf("etag value \"%s\" does not look like a SHA1", etagValue)) - } - commitSha := m[1] defer resp.Body.Close() // Create the file out, err := os.Create(filepath) if err != nil { - return "", err + return err } defer out.Close() // Write the body to file _, err = io.Copy(out, resp.Body) if err != nil { - return "", err + return err } - return commitSha, nil + return nil } func gzipUntar(dst string, r io.Reader, subDir string) error { @@ -157,6 +143,21 @@ func gzipUntar(dst string, r io.Reader, subDir string) error { } } +func remoteResolveRef(ctx context.Context, remote string, ref string) (string, error) { + b := bytes.NewBuffer(nil) + cmd := exec.CommandContext(ctx, "git", "ls-remote", "--heads", "--tags", "--refs", "--quiet", remote, ref) + cmd.Stdin = os.Stdin + cmd.Stdout = b + cmd.Stderr = os.Stderr + err := cmd.Run() + if err != nil { + return "", err + } + commitShaPattern, _ := regexp.Compile("^([0-9a-f]{40})\\b") + commitSha := commitShaPattern.FindString(b.String()) + return commitSha, nil +} + func (p *GitPackage) Install(ctx context.Context, name, dir, version string) (string, error) { destPath := path.Join(dir, name) @@ -171,11 +172,22 @@ func (p *GitPackage) Install(ctx context.Context, name, dir, version string) (st // the ETag header included in the response. isGitHubRemote, err := regexp.MatchString(`^(https|ssh)://github\.com/.+$`, p.Source.Remote) if isGitHubRemote { - archiveUrl := fmt.Sprintf("%s/archive/%s.tar.gz", p.Source.Remote, version) + // Let git ls-remote decide if "version" is a ref or a SHA1 in the unlikely + // but possible event that a ref is exactly 40 hex characters + commitSha, err := remoteResolveRef(ctx, p.Source.Remote, version) + + // If the ref resolution failed and "version" looks like a SHA1, assume it is one + // and proceed. + commitShaPattern, _ := regexp.Compile("^([0-9a-f]{40})$") + if commitSha == "" && commitShaPattern.MatchString(version) { + commitSha = version + } + + archiveUrl := fmt.Sprintf("%s/archive/%s.tar.gz", p.Source.Remote, commitSha) archiveFilepath := fmt.Sprintf("%s.tar.gz", tmpDir) defer os.Remove(archiveFilepath) - commitSha, err := downloadGitHubArchive(archiveFilepath, archiveUrl) + err = downloadGitHubArchive(archiveFilepath, archiveUrl) if err == nil { r, err := os.Open(archiveFilepath) defer r.Close() From a44fae09a0d64e5b51f46a2076ba8f5026a6bafb Mon Sep 17 00:00:00 2001 From: Benoit Gagnon Date: Thu, 24 Oct 2019 10:46:51 -0400 Subject: [PATCH 2/6] declare Buffer instead of using NewBuffer Co-Authored-By: David Genest --- pkg/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/git.go b/pkg/git.go index e08bb85..375a5b8 100644 --- a/pkg/git.go +++ b/pkg/git.go @@ -144,7 +144,7 @@ func gzipUntar(dst string, r io.Reader, subDir string) error { } func remoteResolveRef(ctx context.Context, remote string, ref string) (string, error) { - b := bytes.NewBuffer(nil) + b := &bytes.Buffer{} cmd := exec.CommandContext(ctx, "git", "ls-remote", "--heads", "--tags", "--refs", "--quiet", remote, ref) cmd.Stdin = os.Stdin cmd.Stdout = b From f09b41c7da79d6045ac16eda3fe745b54e23c371 Mon Sep 17 00:00:00 2001 From: Benoit Gagnon Date: Thu, 24 Oct 2019 10:47:35 -0400 Subject: [PATCH 3/6] use regexp.MustCompile Co-Authored-By: David Genest --- pkg/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/git.go b/pkg/git.go index 375a5b8..00ac845 100644 --- a/pkg/git.go +++ b/pkg/git.go @@ -178,7 +178,7 @@ func (p *GitPackage) Install(ctx context.Context, name, dir, version string) (st // If the ref resolution failed and "version" looks like a SHA1, assume it is one // and proceed. - commitShaPattern, _ := regexp.Compile("^([0-9a-f]{40})$") + commitShaPattern := regexp.MustCompile("^([0-9a-f]{40})$") if commitSha == "" && commitShaPattern.MatchString(version) { commitSha = version } From 2bf42f11cde4463c6fc4b6a343e89ebb6ae47901 Mon Sep 17 00:00:00 2001 From: Benoit Gagnon Date: Thu, 24 Oct 2019 11:05:54 -0400 Subject: [PATCH 4/6] use fmt.Errorf(...) instead of errors.new(fmt.Sprintf(...)) Signed-off-by: Benoit Gagnon --- pkg/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/git.go b/pkg/git.go index 00ac845..54d74e5 100644 --- a/pkg/git.go +++ b/pkg/git.go @@ -53,7 +53,7 @@ func downloadGitHubArchive(filepath string, url string) error { } color.Cyan("GET %s %d", url, resp.StatusCode) if resp.StatusCode != 200 { - return errors.New(fmt.Sprintf("unexpected status code %d", resp.StatusCode)) + return fmt.Errorf("unexpected status code %d", resp.StatusCode) } defer resp.Body.Close() From 1915ef519a5a87cde3d108f6e4f0fb0493ea2c8e Mon Sep 17 00:00:00 2001 From: Benoit Gagnon Date: Thu, 24 Oct 2019 11:06:36 -0400 Subject: [PATCH 5/6] relax the SHA1 regex to cover SHA1, SHA256 and any other length of SHA Signed-off-by: Benoit Gagnon --- pkg/git.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/git.go b/pkg/git.go index 54d74e5..3d4859a 100644 --- a/pkg/git.go +++ b/pkg/git.go @@ -153,7 +153,7 @@ func remoteResolveRef(ctx context.Context, remote string, ref string) (string, e if err != nil { return "", err } - commitShaPattern, _ := regexp.Compile("^([0-9a-f]{40})\\b") + commitShaPattern := regexp.MustCompile("^([0-9a-f]{40,})\\b") commitSha := commitShaPattern.FindString(b.String()) return commitSha, nil } @@ -176,9 +176,9 @@ func (p *GitPackage) Install(ctx context.Context, name, dir, version string) (st // but possible event that a ref is exactly 40 hex characters commitSha, err := remoteResolveRef(ctx, p.Source.Remote, version) - // If the ref resolution failed and "version" looks like a SHA1, assume it is one - // and proceed. - commitShaPattern := regexp.MustCompile("^([0-9a-f]{40})$") + // If the ref resolution failed and "version" looks like a SHA, + // assume it is one and proceed. + commitShaPattern := regexp.MustCompile("^([0-9a-f]{40,})$") if commitSha == "" && commitShaPattern.MatchString(version) { commitSha = version } From ed7d84f846786d8100b96eb596c4e399f17801af Mon Sep 17 00:00:00 2001 From: Benoit Gagnon Date: Thu, 24 Oct 2019 11:08:24 -0400 Subject: [PATCH 6/6] update comments to reflect git install changes Signed-off-by: Benoit Gagnon --- pkg/git.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/git.go b/pkg/git.go index 3d4859a..46e505d 100644 --- a/pkg/git.go +++ b/pkg/git.go @@ -168,12 +168,11 @@ func (p *GitPackage) Install(ctx context.Context, name, dir, version string) (st defer os.RemoveAll(tmpDir) // Optimization for GitHub sources: download a tarball archive of the requested - // version instead of cloning the entire repository. The SHA1 is discovered through - // the ETag header included in the response. + // version instead of cloning the entire repository. isGitHubRemote, err := regexp.MatchString(`^(https|ssh)://github\.com/.+$`, p.Source.Remote) if isGitHubRemote { - // Let git ls-remote decide if "version" is a ref or a SHA1 in the unlikely - // but possible event that a ref is exactly 40 hex characters + // Let git ls-remote decide if "version" is a ref or a commit SHA in the unlikely + // but possible event that a ref is comprised of 40 or more hex characters commitSha, err := remoteResolveRef(ctx, p.Source.Remote, version) // If the ref resolution failed and "version" looks like a SHA,