Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump min go version to 1.18 && read build info from embedded binary #2548

Merged
merged 6 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 48 additions & 95 deletions changeset/commit.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2018 The Knative Authors
Copyright 2022 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -17,113 +17,66 @@ limitations under the License.
package changeset

import (
"bufio"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"errors"
"regexp"
"strings"
"runtime/debug"
"strconv"
"sync"
)

const (
commitIDFile = "HEAD"
koDataPathEnvName = "KO_DATA_PATH"
// packedRefsFile is a file containing a list of refs, used to compact the
// list of refs instead of storing them on the filesystem directly.
// See https://git-scm.com/docs/git-pack-refs
packedRefsFile = "packed-refs"
)
const Unknown = "unknown"

var (
shaRegexp = regexp.MustCompile(`^[a-f0-9]{40,64}$`)
rev string
err error
once sync.Once

var commitIDRE = regexp.MustCompile(`^[a-f0-9]{40}$`)
// for testing
readBuildInfo = debug.ReadBuildInfo
)

// Get tries to fetch the first 7 digitals of GitHub commit ID from HEAD file in
// KO_DATA_PATH. If it fails, it returns the error it gets.
// Get returns the 'vcs.revision' property from the embedded build information
// This function will return an error if value is not a valid Git SHA
//
// The result will have a '-dirty' suffix if the workspace was not clean
func Get() (string, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a changeset.MustGet that panics if it can't detect the version? I'm updating some callsites to get this once at init-time, and they're all just log.Fataling if this returns an error. Or should this return Unknown if it can't read build info, and just never return an error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should this return Unknown if it can't read build info, and just never return an error?

I was thinking about this - let me do it in a follow up

data, err := readFileFromKoData(commitIDFile)
if err != nil {
return "", err
once.Do(func() {
rev, err = get()
})

return rev, err
}

func get() (string, error) {
info, ok := readBuildInfo()
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
return "", errors.New("unable to read build info")
}
commitID := strings.TrimSpace(string(data))
if rID := strings.TrimPrefix(commitID, "ref: "); rID != commitID {
// First try to read from the direct ref file - e.g. refs/heads/main
data, err := readFileFromKoData(rID)
if err != nil {
if !os.IsNotExist(err) {
return "", err
}

// Ref file didn't exist - it might be contained in the packed-refs
// file.
var pferr error
data, pferr = findPackedRef(rID)
// Only return the sub-error if the packed-refs file exists, otherwise
// just let the original error return (e.g. treat it as if we didn't
// even attempt the operation). This is primarily to keep the error
// messages clean.
if pferr != nil {
if os.IsNotExist(pferr) {
return "", err
}
return "", pferr
}

var revision string
var modified bool

for _, s := range info.Settings {
switch s.Key {
case "vcs.revision":
revision = s.Value
case "vcs.modified":
modified, _ = strconv.ParseBool(s.Value)
}
commitID = strings.TrimSpace(string(data))
}
if commitIDRE.MatchString(commitID) {
return commitID[:7], nil
}
return "", fmt.Errorf("%q is not a valid GitHub commit ID", commitID)
}

// readFileFromKoData tries to read data as string from the file with given name
// under KO_DATA_PATH then returns the content as string. The file is expected
// to be wrapped into the container from /kodata by ko. If it fails, returns
// the error it gets.
func readFileFromKoData(filename string) ([]byte, error) {
f, err := koDataFile(filename)
if err != nil {
return nil, err
if revision == "" {
return Unknown, nil
}
defer f.Close()
return ioutil.ReadAll(f)
}

// readFileFromKoData tries to open the file with given name under KO_DATA_PATH.
// The file is expected to be wrapped into the container from /kodata by ko.
// If it fails, returns the error it gets.
func koDataFile(filename string) (*os.File, error) {
koDataPath := os.Getenv(koDataPathEnvName)
if koDataPath == "" {
return nil, fmt.Errorf("%q does not exist or is empty", koDataPathEnvName)
if shaRegexp.MatchString(revision) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, we don't need this check 😎

(not necessarily advice to remove it, just a funny coincidence)

Actually, I wonder if this should just be if len(revision) >= 7, so folks using Subversion or whatever can still enjoy this goodness. I'm not sure there aren't other places we assume Git in and around here, but just in case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd leave it as - since I don't want to assume truncating will work for different VCS - ie svn is numeric so we'd be alternating the commit id

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A further improvement could be to do this only for vcs that use SHA's for revisions -ie git, hg etc.

revision = revision[:7]
}
return os.Open(filepath.Join(koDataPath, filename))
}

// findPackedRef searches the packed-ref file for ref values.
// This can happen if the # of refs in a repo grows too much - git will try
// and condense them into a file.
// See https://git-scm.com/docs/git-pack-refs
func findPackedRef(ref string) ([]byte, error) {
f, err := koDataFile(packedRefsFile)
if err != nil {
return nil, err
if modified {
revision += "-dirty"
}
defer f.Close()

scanner := bufio.NewScanner(f)
for scanner.Scan() {
// We only care about lines with `<commit> <ref>` pairs.
// Why this might happen:
// 1. Lines starting with ^ refer to unpeeled tag SHAs
// (e.g. the commits pointed to by annotated tags)
s := strings.Split(scanner.Text(), " ")
if len(s) != 2 {
continue
}
if ref == s[1] {
return []byte(s[0]), nil
}
}
return nil, fmt.Errorf("%q ref not found in packed-refs", ref)

return revision, nil
}
132 changes: 63 additions & 69 deletions changeset/commit_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2018 The Knative Authors
Copyright 2022 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -17,87 +17,81 @@ limitations under the License.
package changeset

import (
"errors"
"fmt"
"runtime/debug"
"sync"
"testing"
)

const (
nonCommittedHeadRef = "refs/heads/non_committed_branch"
testCommitID = "a2d1bdf"
"github.com/google/go-cmp/cmp"
)

func TestReadFile(t *testing.T) {
tests := []struct {
name string
koDataPath string
koDataPathEnvDoesNotExist bool
want string
wantErr bool
err error
func TestGet(t *testing.T) {

cases := []struct {
name string
info *debug.BuildInfo
ok bool
result string
wantErr string
}{{
name: "committed branch",
koDataPath: "testdata",
want: testCommitID,
}, {
name: "no refs link",
koDataPath: "testdata/noncommitted",
wantErr: true,
err: fmt.Errorf("open testdata/noncommitted/%s: no such file or directory", nonCommittedHeadRef),
}, {
name: "with refs link",
koDataPath: "testdata/with-refs",
want: testCommitID,
}, {
name: "with bad content",
koDataPath: "testdata/garbage",
wantErr: true,
err: fmt.Errorf(`"garbage contents" is not a valid GitHub commit ID`),
}, {
name: "KO_DATA_PATH is empty",
koDataPath: "",
wantErr: true,
err: fmt.Errorf("%q does not exist or is empty", koDataPathEnvName),
name: "info fails",
ok: false,
wantErr: "unable to read build info",
}, {
name: "KO_DATA_PATH does not exist",
koDataPath: "",
wantErr: true,
koDataPathEnvDoesNotExist: true,
err: fmt.Errorf("%q does not exist or is empty", koDataPathEnvName),
name: "missing revision",
ok: true,
info: &debug.BuildInfo{},
result: Unknown,
}, {
name: "HEAD file does not exist",
koDataPath: "testdata/nonexisting",
wantErr: true,
err: errors.New("open testdata/nonexisting/HEAD: no such file or directory"),
name: "SHA1 revision is truncated",
ok: true,
info: &debug.BuildInfo{
Settings: []debug.BuildSetting{{
Key: "vcs.revision", Value: "3666ce749d32abe7be0528380c8c05a4282cb733",
}},
},
result: "3666ce7",
}, {
name: "packed-ref",
koDataPath: "testdata/packed-refs",
want: testCommitID,
name: "SHA256 revision is truncated",
ok: true,
info: &debug.BuildInfo{
Settings: []debug.BuildSetting{{
Key: "vcs.revision", Value: "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824",
}},
},
result: "2cf24db",
}, {
name: "no refs with packed-ref",
koDataPath: "testdata/noncommited-packed-refs",
wantErr: true,
err: fmt.Errorf("%q ref not found in packed-refs", nonCommittedHeadRef),
name: "modified workspace results in -dirty suffix",
ok: true,
info: &debug.BuildInfo{
Settings: []debug.BuildSetting{{
Key: "vcs.revision", Value: "3666ce749d32abe7be0528380c8c05a4282cb733",
}, {
Key: "vcs.modified", Value: "true",
}},
},
result: "3666ce7-dirty",
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if !test.koDataPathEnvDoesNotExist {
t.Setenv(koDataPathEnvName, test.koDataPath)
}

got, err := Get()

if (err != nil) != test.wantErr {
t.Error("Get() =", err)
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
once = sync.Once{}
readBuildInfo = func() (info *debug.BuildInfo, ok bool) {
return c.info, c.ok
}
if !test.wantErr {
if test.want != got {
t.Errorf("wanted %q but got %q", test.want, got)
}
} else {
if err == nil || err.Error() != test.err.Error() {
t.Errorf("wanted error %v but got: %v", test.err, err)

val, err := Get()
if c.wantErr == "" && err != nil {
t.Fatal("unexpected error", err)
} else if c.wantErr != "" && err != nil {
if diff := cmp.Diff(c.wantErr, err.Error()); diff != "" {
t.Fatalf("error doesn't match expected: %s", diff)
}
} else if c.wantErr != "" && err == nil {
t.Fatalf("expected error %q but was nil", c.wantErr)
}

if diff := cmp.Diff(c.result, val); diff != "" {
t.Errorf("result doesn't match expected: %s", diff)
}
})
}
Expand Down
8 changes: 2 additions & 6 deletions changeset/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// Package changeset provides Knative utilities for fetching GitHub Commit ID
// from kodata directory. It requires GitHub HEAD file to be linked into
// Knative component source code via the following command:
// ln -s -r .git/HEAD ./cmd/<knative-component-name>/kodata/
// Then ko will build this file into $KO_DATA_PATH when building the container
// for a Knative component.
// Package changeset returns version control info that was embedded in the
// golang binary
package changeset
1 change: 0 additions & 1 deletion changeset/testdata/HEAD

This file was deleted.

1 change: 0 additions & 1 deletion changeset/testdata/garbage/HEAD

This file was deleted.

1 change: 0 additions & 1 deletion changeset/testdata/noncommited-packed-refs/HEAD

This file was deleted.

Empty file.
1 change: 0 additions & 1 deletion changeset/testdata/noncommitted/HEAD

This file was deleted.

1 change: 0 additions & 1 deletion changeset/testdata/packed-refs/HEAD

This file was deleted.

4 changes: 0 additions & 4 deletions changeset/testdata/packed-refs/packed-refs

This file was deleted.

1 change: 0 additions & 1 deletion changeset/testdata/with-refs/HEAD

This file was deleted.

1 change: 0 additions & 1 deletion changeset/testdata/with-refs/refs/heads/branch-name

This file was deleted.

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module knative.dev/pkg

go 1.17
go 1.18

require (
cloud.google.com/go v0.98.0
Expand Down
Loading