feat: Git manifest sources can now specify a reference to pin to

This was in the documentation but not implemented (as discovered in #342). `#<tag>` was supported for Git packages, so I've factored that code out and reused it. Added an integration test for this, along with a couple of new test helper functions that should make these kind of tests simpler in the future.
cashapp · Nov 13, 2022 · 14c868b · 14c868b
1 parent b678983
commit 14c868b
Show file tree

Hide file tree

Showing 17 changed files with 222 additions and 83 deletions.
diff --git a/cache/git.go b/cache/git.go
@@ -23,27 +23,15 @@ func (s *gitSource) OpenLocal(c *Cache, checksum string) (*os.File, error) {
 func (s *gitSource) Download(b *ui.Task, cache *Cache, checksum string) (string, string, string, error) {
 	base := BasePath(checksum, s.URL)
 	checkoutDir := filepath.Join(cache.root, base)
-	repo, tag := parseGitURL(s.URL)
-	args := []string{"git", "clone", "--depth=1", repo, checkoutDir}
-	if tag != "" {
-		args = append(args, "--branch="+tag)
-	}
-	err := util.RunInDir(b, cache.root, args...)
-	if err != nil {
-		return "", "", "", errors.WithStack(err)
-	}
-
-	bts, err := util.CaptureInDir(b, checkoutDir, "git", "rev-parse", "HEAD")
+	etag, err := util.GitClone(b, &util.RealCommandRunner{}, s.URL, checkoutDir)
 	if err != nil {
-		return "", "", "", errors.WithStack(err)
+		return "", "", "", errors.Wrap(err, s.URL)
 	}
-	etag := strings.Trim(string(bts), "\n")
-
 	return filepath.Join(cache.root, base), etag, "", nil
 }
 
 func (s *gitSource) ETag(b *ui.Task) (etag string, err error) {
-	repo, tag := parseGitURL(s.URL)
+	repo, tag := util.ParseGitURL(s.URL)
 	if tag == "" {
 		tag = "HEAD"
 	}
@@ -61,7 +49,7 @@ func (s *gitSource) ETag(b *ui.Task) (etag string, err error) {
 }
 
 func (s *gitSource) Validate() error {
-	repo, tag := parseGitURL(s.URL)
+	repo, tag := util.ParseGitURL(s.URL)
 	if tag == "" {
 		tag = "HEAD"
 	}
@@ -72,12 +60,3 @@ func (s *gitSource) Validate() error {
 	}
 	return nil
 }
-
-func parseGitURL(source string) (repo, tag string) {
-	parts := strings.SplitN(source, "#", 2)
-	repo = parts[0]
-	if len(parts) > 1 {
-		tag = parts[1]
-	}
-	return
-}
diff --git a/docs/content/packaging/schema/channel.md b/docs/content/packaging/schema/channel.md
@@ -36,7 +36,8 @@ Used by: [&lt;manifest>](../manifest#blocks)
 | `root` | `string?` | Override root for package. |
 | `runtime-dependencies` | `[string]?` | Packages used internally by this package, but not installed to the target environment |
 | `sha256` | `string?` | SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence. |
-| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;tag&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
+| `sha256-source` | `string?` | URL for SHA256 checksum file for source package. |
+| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;ref&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
 | `strip` | `number?` | Number of path prefix elements to strip. |
 | `test` | `string?` | Command that will test the package is operational. |
 | `update` | `string` | Update frequency for this channel. |

diff --git a/docs/content/packaging/schema/darwin.md b/docs/content/packaging/schema/darwin.md
@@ -36,7 +36,8 @@ Used by: [channel](../channel#blocks) [linux](../linux#blocks) [&lt;manifest>](.
 | `root` | `string?` | Override root for package. |
 | `runtime-dependencies` | `[string]?` | Packages used internally by this package, but not installed to the target environment |
 | `sha256` | `string?` | SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence. |
-| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;tag&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
+| `sha256-source` | `string?` | URL for SHA256 checksum file for source package. |
+| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;ref&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
 | `strip` | `number?` | Number of path prefix elements to strip. |
 | `test` | `string?` | Command that will test the package is operational. |
 | `vars` | `{string: string}?` | Set local variables used during manifest evaluation. |
diff --git a/docs/content/packaging/schema/linux.md b/docs/content/packaging/schema/linux.md
@@ -36,7 +36,8 @@ Used by: [channel](../channel#blocks) [darwin](../darwin#blocks) [&lt;manifest>]
 | `root` | `string?` | Override root for package. |
 | `runtime-dependencies` | `[string]?` | Packages used internally by this package, but not installed to the target environment |
 | `sha256` | `string?` | SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence. |
-| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;tag&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
+| `sha256-source` | `string?` | URL for SHA256 checksum file for source package. |
+| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;ref&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
 | `strip` | `number?` | Number of path prefix elements to strip. |
 | `test` | `string?` | Command that will test the package is operational. |
 | `vars` | `{string: string}?` | Set local variables used during manifest evaluation. |
diff --git a/docs/content/packaging/schema/manifest.md b/docs/content/packaging/schema/manifest.md
@@ -40,8 +40,9 @@ Each Hermit package manifest is a nested structure containing OS/architecture-sp
 | `root` | `string?` | Override root for package. |
 | `runtime-dependencies` | `[string]?` | Packages used internally by this package, but not installed to the target environment |
 | `sha256` | `string?` | SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence. |
+| `sha256-source` | `string?` | URL for SHA256 checksum file for source package. |
 | `sha256sums` | `{string: string}?` | SHA256 checksums of source packages for verification. |
-| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;tag&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
+| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;ref&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
 | `strip` | `number?` | Number of path prefix elements to strip. |
 | `test` | `string?` | Command that will test the package is operational. |
 | `vars` | `{string: string}?` | Set local variables used during manifest evaluation. |
diff --git a/docs/content/packaging/schema/platform.md b/docs/content/packaging/schema/platform.md
@@ -36,7 +36,8 @@ Used by: [channel](../channel#blocks) [darwin](../darwin#blocks) [linux](../linu
 | `root` | `string?` | Override root for package. |
 | `runtime-dependencies` | `[string]?` | Packages used internally by this package, but not installed to the target environment |
 | `sha256` | `string?` | SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence. |
-| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;tag&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
+| `sha256-source` | `string?` | URL for SHA256 checksum file for source package. |
+| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;ref&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
 | `strip` | `number?` | Number of path prefix elements to strip. |
 | `test` | `string?` | Command that will test the package is operational. |
 | `vars` | `{string: string}?` | Set local variables used during manifest evaluation. |
diff --git a/docs/content/packaging/schema/version.md b/docs/content/packaging/schema/version.md
@@ -37,7 +37,8 @@ Used by: [&lt;manifest>](../manifest#blocks)
 | `root` | `string?` | Override root for package. |
 | `runtime-dependencies` | `[string]?` | Packages used internally by this package, but not installed to the target environment |
 | `sha256` | `string?` | SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence. |
-| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;tag&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
+| `sha256-source` | `string?` | URL for SHA256 checksum file for source package. |
+| `source` | `string?` | URL for source package. Valid URLs are Git repositories (using .git[#&lt;ref&gt;] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix) |
 | `strip` | `number?` | Number of path prefix elements to strip. |
 | `test` | `string?` | Command that will test the package is operational. |
 | `vars` | `{string: string}?` | Set local variables used during manifest evaluation. |
diff --git a/docs/content/usage/config.md b/docs/content/usage/config.md
@@ -18,7 +18,7 @@ customise that Hermit environment.
 
 Hermit supports three different manifest sources:
 
-1. Git repositories; any cloneable URI ending with `.git`, eg.<br/>`https://github.com/cashapp/hermit-packages.git`. An optional `#<tag>` suffix can be added to checkout a specific tag.
+1. Git repositories; any cloneable URI ending with `.git`, eg.<br/>`https://github.com/cashapp/hermit-packages.git`. An optional `#<ref>` suffix can be added to checkout a specific reference.
 2. Local filesystem, eg. `file:///home/user/my-packages`.<br/>This is mostly only useful for local development and testing.
 3. Environment relative, eg. `env:///my-packages`.<br/>This will search for package manifests in the directory `${HERMIT_ENV}/my-packages`. Useful for local overrides.
 

diff --git a/integration/integration_test.go b/integration/integration_test.go
@@ -1,4 +1,4 @@
-// Package integration provides integration tests for Hermit.
+// Package integration_test provides integration tests for Hermit.
 //
 // Each test is run against the supported shells, in a temporary directory, with
 // a version of Hermit built from the current source.
@@ -13,11 +13,13 @@
 package integration_test
 
 import (
+	"archive/zip"
 	"bufio"
 	"fmt"
 	"io"
 	"io/fs"
 	"io/ioutil"
+	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -31,48 +33,6 @@ import (
 	"github.com/creack/pty"
 )
 
-var shells = [][]string{
-	{"bash", "--norc", "--noprofile"},
-	{"zsh", "--no-rcs", "--no-globalrcs"},
-}
-
-// Functions that test scripts can use to communicate back to the test framework.
-const preamble = `
-set -euo pipefail
-
-hermit-send() {
-  echo "$@" 1>&3
-}
-
-assert() {
-  if ! "$@"; then
-    hermit-send "error: assertion failed: $@"
-    exit 1
-  fi
-}
-
-# Run a shell command and emulate what the Hermit shell hooks would do.
-#
-# usage: with-prompt-hooks <cmd>
-#
-# Normally this is done by shell hooks, but because we're not running interactively this is not possible.
-with-prompt-hooks() {
-  "$@"
-  res=$?
-  # We need to reset the change timestamp, as file timestamps are at second resolution.
-  # Some IT updates could be lost without this
-  export HERMIT_BIN_CHANGE=0
-
-  if test -n "${PROMPT_COMMAND+_}"; then
-    eval "$PROMPT_COMMAND"
-  elif [ -n "${ZSH_VERSION-}" ]; then
-    update_hermit_env
-  fi
-
-  return $res
-}
-`
-
 func TestIntegration(t *testing.T) {
 	tests := []struct {
 		name         string
@@ -238,6 +198,13 @@ func TestIntegration(t *testing.T) {
 			hermit manifest add-digests packages/testbin1.hcl
 			assert grep d4f8989a4a6bf56ccc768c094448aa5f42be3b9f0287adc2f4dfd2241f80d2c0 packages/testbin1.hcl 
 			`},
+		// Test that git sources with a specific reference are handled correctly.
+		{name: "SourceWithRef",
+			preparations: prep{unzip("git-source.zip", "source.git"), serveDir("source.git")},
+			script: `
+			hermit init --sources="file://$PWD/source.git#c0551672d8d179b93615e9612deaa2c3cc4fe0b5"
+			assert fail ./bin/hermit info testbin1-1.0.1
+			`},
 	}
 
 	checkForShells(t)
@@ -354,6 +321,54 @@ func TestIntegration(t *testing.T) {
 	}
 }
 
+var shells = [][]string{
+	{"bash", "--norc", "--noprofile"},
+	{"zsh", "--no-rcs", "--no-globalrcs"},
+}
+
+// Functions that test scripts can use to communicate back to the test framework.
+const preamble = `
+set -euo pipefail
+
+hermit-send() {
+  echo "$@" 1>&3
+}
+
+assert() {
+  if [ "${1:-}" = "fail" ]; then
+    shift
+    if "$@"; then
+      hermit-send "error: assertion should have failed: $@"
+      exit 1
+    fi
+  elif ! "$@"; then
+    hermit-send "error: assertion failed: $@"
+    exit 1
+  fi
+}
+
+# Run a shell command and emulate what the Hermit shell hooks would do.
+#
+# usage: with-prompt-hooks <cmd>
+#
+# Normally this is done by shell hooks, but because we're not running interactively this is not possible.
+with-prompt-hooks() {
+  "$@"
+  res=$?
+  # We need to reset the change timestamp, as file timestamps are at second resolution.
+  # Some IT updates could be lost without this
+  export HERMIT_BIN_CHANGE=0
+
+  if test -n "${PROMPT_COMMAND+_}"; then
+    eval "$PROMPT_COMMAND"
+  elif [ -n "${ZSH_VERSION-}" ]; then
+    update_hermit_env
+  fi
+
+  return $res
+}
+`
+
 // Build Hermit from source.
 func buildAndInjectHermit(t *testing.T, environ []string) (outenviron []string) {
 	t.Helper()
@@ -527,3 +542,80 @@ func outputContains(text string) expectation {
 		assert.Contains(t, output, text, "%s", output)
 	}
 }
+
+// Unzip zipFile relative to testdata, into the test directory at dest.
+func unzip(zipFile, dest string) preparation {
+	return func(t *testing.T, dir string) string {
+		t.Helper()
+		r, err := zip.OpenReader(filepath.Join("testdata", zipFile))
+		assert.NoError(t, err)
+		defer r.Close()
+		for _, f := range r.File {
+			path := filepath.Join(dir, dest, f.Name)
+			if f.FileInfo().IsDir() {
+				err = os.MkdirAll(path, f.Mode())
+				continue
+			}
+
+			rc, err := f.Open()
+			assert.NoError(t, err)
+			err = os.MkdirAll(filepath.Dir(path), 0700)
+			assert.NoError(t, err)
+			w, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, f.Mode())
+			assert.NoError(t, err)
+			_, err = io.Copy(w, rc)
+			err = w.Close()
+			assert.NoError(t, err)
+			err = rc.Close()
+			assert.NoError(t, err)
+		}
+		return ""
+	}
+}
+
+// Serve a directory on 127.0.0.1:8999
+func serveDir(httpRoot string) preparation {
+	return func(t *testing.T, dir string) string {
+		srv := &http.Server{
+			Addr:    "127.0.0.1:8999",
+			Handler: http.FileServer(http.Dir(filepath.Join(dir, httpRoot))),
+		}
+		go func() { _ = srv.ListenAndServe() }()
+		t.Cleanup(func() { _ = srv.Close() })
+		return ""
+	}
+}
+
+// Serve the given zip file on 127.0.0.1:8999
+func serveZip(zipFile string) preparation {
+	return func(t *testing.T, dir string) string {
+		t.Helper()
+		zr, err := zip.OpenReader(zipFile)
+		assert.NoError(t, err)
+		files := map[string]*zip.File{}
+		for _, file := range zr.File {
+			files[file.Name] = file
+		}
+		srv := &http.Server{
+			Addr: "127.0.0.1:8999",
+			Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				path := strings.TrimPrefix(r.URL.Path, "/")
+				file, ok := files[path]
+				if !ok {
+					http.NotFound(w, r)
+					return
+				}
+				w.Header().Add("Content-Type", "application/octet-stream")
+				w.Header().Add("Content-Size", fmt.Sprintf("%d", file.UncompressedSize64))
+				rc, err := file.Open()
+				assert.NoError(t, err)
+				defer rc.Close()
+				_, err = io.Copy(w, rc)
+				assert.NoError(t, err)
+			}),
+		}
+		go func() { _ = srv.ListenAndServe() }()
+		t.Cleanup(func() { _ = srv.Close() })
+		return ""
+	}
+}
diff --git a/integration/testdata/git-source.zip b/integration/testdata/git-source.zip
diff --git a/manifest/config.go b/manifest/config.go
@@ -38,7 +38,7 @@ type Layer struct {
 	Test         *string           `hcl:"test,optional" help:"Command that will test the package is operational."`
 	Env          envars.Envars     `hcl:"env,optional" help:"Environment variables to export."`
 	Vars         map[string]string `hcl:"vars,optional" help:"Set local variables used during manifest evaluation."`
-	Source       string            `hcl:"source,optional" help:"URL for source package. Valid URLs are Git repositories (using .git[#<tag>] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix)"`
+	Source       string            `hcl:"source,optional" help:"URL for source package. Valid URLs are Git repositories (using .git[#<ref>] suffix), Local Files (using file:// prefix), and Remote Files (using http:// or https:// prefix)"`
 	DontExtract  bool              `hcl:"dont-extract,optional" help:"Don't extract the package source, just copy it into the installation directory."`
 	Mirrors      []string          `hcl:"mirrors,optional" help:"Mirrors to use if the primary source is unavailable."`
 	SHA256       string            `hcl:"sha256,optional" help:"SHA256 of source package for verification. When in conflict with SHA256 in sha256sums, this value takes precedence."`

diff --git a/sources/git.go b/sources/git.go
@@ -95,14 +95,14 @@ func syncGit(b *ui.Task, dir, source, finalDest string, runner util.CommandRunne
 		return errors.WithStack(err)
 	}
 	defer os.RemoveAll(dest)
-	if err = runner.RunInDir(b, dest, "git", "clone", "--depth=1", source, dest); err != nil {
+	_, err = util.GitClone(b, runner, source, dest)
+	if err != nil {
 		return errors.WithStack(err)
 	}
 	_ = os.RemoveAll(finalDest)
 	// And finally, rename it into place.
 	if err = os.Rename(dest, finalDest); err != nil && !os.IsExist(err) { // Prevent races.
 		return errors.WithStack(err)
 	}
-
 	return nil
 }
diff --git a/sources/git_test.go b/sources/git_test.go
@@ -8,12 +8,19 @@ import (
 	"github.com/cashapp/hermit/errors"
 	"github.com/cashapp/hermit/sources"
 	"github.com/cashapp/hermit/ui"
+	"github.com/cashapp/hermit/util"
 )
 
 type FailingGit struct {
 	err error
 }
 
+var _ util.CommandRunner = &FailingGit{}
+
+func (f *FailingGit) CaptureInDir(log ui.Logger, dir string, args ...string) ([]byte, error) {
+	return nil, f.err
+}
+
 func (f *FailingGit) RunInDir(_ *ui.Task, _ string, _ ...string) error {
 	return f.err
 }