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

Performance Improvements #204

Merged
merged 4 commits into from
Aug 31, 2023
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
9 changes: 2 additions & 7 deletions check.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,13 @@ func checkMain(_ *cobra.Command, args []string) error {
found := false

for _, lib := range libs {
if lib.LicensePath == "" {
if lib.LicenseFile == "" {
Bobgy marked this conversation as resolved.
Show resolved Hide resolved
fmt.Fprintf(os.Stderr, "Did not find license for library '%v'.\n", lib)
found = true
continue
}

licenses, err := classifier.Identify(lib.LicensePath)
if err != nil {
return err
}

for _, license := range licenses {
for _, license := range lib.Licenses {
if hasLicenseNames && !isAllowedLicenseName(license.Name, allowedLicenseNames) {
fmt.Fprintf(os.Stderr, "Not allowed license '%s' found for library '%v'.\n", license.Name, lib)
found = true
Expand Down
15 changes: 1 addition & 14 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,19 @@ require (
go.opencensus.io v0.24.0
golang.org/x/mod v0.10.0
golang.org/x/net v0.9.0
golang.org/x/sync v0.1.0
golang.org/x/text v0.9.0
golang.org/x/tools v0.8.0
gopkg.in/src-d/go-git.v4 v4.13.1
k8s.io/klog/v2 v2.90.1
)

require (
github.com/Microsoft/go-winio v0.5.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/emirpasic/gods v1.12.0 // indirect
github.com/go-logr/logr v1.2.0 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/google/martian/v3 v3.3.2 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
github.com/kevinburke/ssh_config v0.0.0-20201106050909-4977a11b4351 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/sergi/go-diff v1.2.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/src-d/gcfg v1.4.0 // indirect
github.com/xanzy/ssh-agent v0.3.0 // indirect
golang.org/x/crypto v0.1.0 // indirect
golang.org/x/sys v0.7.0 // indirect
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/src-d/go-billy.v4 v4.3.2 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
)
66 changes: 0 additions & 66 deletions go.sum

Large diffs are not rendered by default.

7 changes: 1 addition & 6 deletions licenses/classifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package licenses

import (
"fmt"
"os"

licenseclassifier "github.com/google/licenseclassifier/v2"
Expand Down Expand Up @@ -83,9 +82,5 @@ func (c *googleClassifier) Identify(licensePath string) ([]License, error) {
})
}

if len(matches.Matches) > 0 {
return licenses, nil
} else {
return nil, fmt.Errorf("no license found")
}
return licenses, nil
}
55 changes: 21 additions & 34 deletions licenses/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package licenses

import (
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -27,69 +26,57 @@ var (
licenseRegexp = regexp.MustCompile(`^(?i)((UN)?LICEN(S|C)E|COPYING|README|NOTICE).*$`)
)

// Find returns the file path of the license for this package.
// FindCandidates returns the candidate file path of the license for this package.
inteon marked this conversation as resolved.
Show resolved Hide resolved
//
// dir is path of the directory where we want to find a license.
// rootDir is path of the module containing this package. Find will not search out of the
// rootDir.
func Find(dir string, rootDir string, classifier Classifier) (string, error) {
func FindCandidates(dir string, rootDir string) ([]string, error) {
dir, err := filepath.Abs(dir)
if err != nil {
return "", err
return nil, err
}

rootDir, err = filepath.Abs(rootDir)
if err != nil {
return "", err
return nil, err
}

if !strings.HasPrefix(dir, rootDir) {
return "", fmt.Errorf("licenses.Find: rootDir %s should contain dir %s", rootDir, dir)
}
found, err := findUpwards(dir, licenseRegexp, rootDir, func(path string) bool {
// TODO(RJPercival): Return license details
if _, err := classifier.Identify(path); err != nil {
return false
}
return true
})
if err != nil {
if errors.Is(err, errNotFound) {
return "", fmt.Errorf("cannot find a known open source license for %q whose name matches regexp %s and locates up until %q", dir, licenseRegexp, rootDir)
}
return "", fmt.Errorf("finding a known open source license: %w", err)
return nil, fmt.Errorf("licenses.Find: rootDir %s should contain dir %s", rootDir, dir)
}
return found, nil

return findAllUpwards(dir, licenseRegexp, rootDir)
}

var errNotFound = fmt.Errorf("file/directory matching predicate and regexp not found")
func findAllUpwards(dir string, r *regexp.Regexp, stopAt string) ([]string, error) {
var foundPaths []string

func findUpwards(dir string, r *regexp.Regexp, stopAt string, predicate func(path string) bool) (string, error) {
// Dir must be made absolute for reliable matching with stopAt regexps
dir, err := filepath.Abs(dir)
if err != nil {
return "", err
}
start := dir
// Stop once we go out of the stopAt dir.
for strings.HasPrefix(dir, stopAt) {
dirContents, err := os.ReadDir(dir)
if err != nil {
return "", err
return nil, err
}

for _, f := range dirContents {
if f.IsDir() {
continue
}

if r.MatchString(f.Name()) {
path := filepath.Join(dir, f.Name())
if predicate != nil && !predicate(path) {
continue
}
return path, nil
foundPaths = append(foundPaths, path)
}
}

parent := filepath.Dir(dir)
if parent == dir {
// Can't go any higher up the directory tree.
break
}
dir = parent
}
return "", fmt.Errorf("findUpwards(dir=%q, regexp=%q, stopAt=%q, predicate=func): %w", start, r, stopAt, errNotFound)

return foundPaths, nil
}
178 changes: 73 additions & 105 deletions licenses/find_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,152 +17,120 @@ package licenses
import (
"os"
"path/filepath"
"regexp"
"testing"

"github.com/google/go-cmp/cmp"
)

func TestFind(t *testing.T) {
func TestFindCandidates(t *testing.T) {
wd, err := os.Getwd()
if err != nil {
t.Fatalf("Cannot get working directory: %v", err)
}

classifier := classifierStub{
licenses: map[string][]License{
"testdata/LICENSE": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/MIT/LICENSE.MIT": {
{
Name: "MIT",
Type: Notice,
},
},
"testdata/licence/LICENCE": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/copying/COPYING": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/notice/NOTICE.txt": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/readme/README.md": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/lowercase/license": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/license-apache-2.0/LICENSE-APACHE-2.0.txt": {
{
Name: "foo",
Type: Notice,
},
},
"testdata/unlicense/UNLICENSE": {
{
Name: "unlicense",
Type: Unencumbered,
},
},
},
}

for _, test := range []struct {
desc string
dir string
rootDir string
wantLicensePath string
wantErr *regexp.Regexp
desc string
dir string
rootDir string
wantLicensePathCandidates []string
}{
{
desc: "licenSe",
dir: "testdata",
wantLicensePath: filepath.Join(wd, "testdata/LICENSE"),
desc: "licenSe",
dir: "testdata",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "licenCe",
dir: "testdata/licence",
wantLicensePath: filepath.Join(wd, "testdata/licence/LICENCE"),
desc: "licenCe",
dir: "testdata/licence",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/licence/LICENCE"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "LICENSE.MIT",
dir: "testdata/MIT",
wantLicensePath: filepath.Join(wd, "testdata/MIT/LICENSE.MIT"),
desc: "LICENSE.MIT",
dir: "testdata/MIT",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/MIT/LICENSE.MIT"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "COPYING",
dir: "testdata/copying",
wantLicensePath: filepath.Join(wd, "testdata/copying/COPYING"),
desc: "COPYING",
dir: "testdata/copying",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/copying/COPYING"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "NOTICE",
dir: "testdata/notice",
wantLicensePath: filepath.Join(wd, "testdata/notice/NOTICE.txt"),
desc: "NOTICE",
dir: "testdata/notice",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/notice/NOTICE.txt"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "README",
dir: "testdata/readme",
wantLicensePath: filepath.Join(wd, "testdata/readme/README.md"),
desc: "README",
dir: "testdata/readme",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/readme/README.md"),
filepath.Join(wd, "testdata/LICENSE")},
},
{
desc: "parent dir",
dir: "testdata/internal",
wantLicensePath: filepath.Join(wd, "testdata/LICENSE"),
desc: "parent dir",
dir: "testdata/internal",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "lowercase",
dir: "testdata/lowercase",
wantLicensePath: filepath.Join(wd, "testdata/lowercase/license"),
desc: "lowercase",
dir: "testdata/lowercase",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/lowercase/license"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "license-apache-2.0.txt",
dir: "testdata/license-apache-2.0",
wantLicensePath: filepath.Join(wd, "testdata/license-apache-2.0/LICENSE-APACHE-2.0.txt"),
desc: "license-apache-2.0.txt",
dir: "testdata/license-apache-2.0",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/license-apache-2.0/LICENSE-APACHE-2.0.txt"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
{
desc: "proprietary-license",
dir: "testdata/proprietary-license",
rootDir: "testdata/proprietary-license",
wantErr: regexp.MustCompile(`cannot find a known open source license for.*testdata/proprietary-license.*whose name matches regexp.*and locates up until.*testdata/proprietary-license`),
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/proprietary-license/LICENSE"),
},
},
{
desc: "UNLICENSE",
dir: "testdata/unlicense",
wantLicensePath: filepath.Join(wd, "testdata/unlicense/UNLICENSE"),
desc: "UNLICENSE",
dir: "testdata/unlicense",
wantLicensePathCandidates: []string{
filepath.Join(wd, "testdata/unlicense/UNLICENSE"),
filepath.Join(wd, "testdata/LICENSE"),
},
},
} {
t.Run(test.desc, func(t *testing.T) {
if test.rootDir == "" {
test.rootDir = "./testdata"
}
licensePath, err := Find(test.dir, test.rootDir, classifier)
if test.wantErr != nil {
if err == nil || !test.wantErr.Match([]byte(err.Error())) {
t.Fatalf("Find(%q) = %q, %q, want (%q, %q)", test.dir, licensePath, err, "", test.wantErr)
}
return
licensePathCandidates, err := FindCandidates(test.dir, test.rootDir)
if err != nil {
t.Fatalf("FindCandidates(%q) = (%#v, %q), want (%q, nil)", test.dir, licensePathCandidates, err, test.wantLicensePathCandidates)
}
if err != nil || licensePath != test.wantLicensePath {
t.Fatalf("Find(%q) = (%#v, %q), want (%q, nil)", test.dir, licensePath, err, test.wantLicensePath)

if diff := cmp.Diff(test.wantLicensePathCandidates, licensePathCandidates); diff != "" {
t.Fatalf("FindCandidates(%q) = %q, %q, want (%q, nil); diff (-want +got): %s", test.dir, licensePathCandidates, err, test.wantLicensePathCandidates, diff)
}
})
}
Expand Down
Loading