Skip to content

Commit

Permalink
Performance Improvements (#204)
Browse files Browse the repository at this point in the history 
* remove unused git logic

Signed-off-by: Tim Ramlot <[email protected]>

* only Identify once & optimise logic

Signed-off-by: Tim Ramlot <[email protected]>

* add comment explaining how Libraries functions

Signed-off-by: Tim Ramlot <[email protected]>

* improve error message in case the LicensePath is UNKNOWN

Signed-off-by: Tim Ramlot <[email protected]>

---------

Signed-off-by: Tim Ramlot <[email protected]>
  • Loading branch information
@inteon
inteon authored Aug 31, 2023
1 parent 150520f commit 9a41918
Show file tree
Hide file tree
Showing 13 changed files with 482 additions and 599 deletions.
9 changes: 2 additions & 7 deletions check.go
View file
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 == "" {
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
View file
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
View file

Large diffs are not rendered by default.

7 changes: 1 addition & 6 deletions licenses/classifier.go
View file
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
View file
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.
//
// 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
View file
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

0 comments on commit 9a41918

Please sign in to comment.