From 8f50437ba09ae7af62a89e0d683d0a52585d409e Mon Sep 17 00:00:00 2001 From: DiptoChakrabarty Date: Tue, 21 Jun 2022 21:10:37 +0530 Subject: [PATCH 1/6] replace error handling method in pkg/license --- pkg/license/catalog.go | 11 ++++++----- pkg/license/download.go | 30 ++++++++++++++-------------- pkg/license/implementation.go | 26 ++++++++++++------------ pkg/license/license.go | 37 ++++++++++++++++++----------------- pkg/license/license_test.go | 2 +- 5 files changed, 54 insertions(+), 52 deletions(-) diff --git a/pkg/license/catalog.go b/pkg/license/catalog.go index 5dc8b529..fa622d51 100644 --- a/pkg/license/catalog.go +++ b/pkg/license/catalog.go @@ -17,6 +17,7 @@ limitations under the License. package license import ( + "fmt" "os" "path/filepath" "sync" @@ -43,7 +44,7 @@ func NewCatalogWithOptions(opts *CatalogOptions) (catalog *Catalog, err error) { doptions.CacheDir = opts.CacheDir downloader, err := NewDownloaderWithOptions(doptions) if err != nil { - return nil, errors.Wrap(err, "creating downloader") + return nil, fmt.Errorf("creating downloader: %w", err) } catalog = &Catalog{ Downloader: downloader, @@ -63,7 +64,7 @@ func (catalog *Catalog) LoadLicenses() error { logrus.Info("Loading license data from downloader") licenses, err := catalog.Downloader.GetLicenses() if err != nil { - return errors.Wrap(err, "getting licenses from downloader") + return fmt.Errorf("getting licenses from downloader: %w", err) } catalog.List = licenses logrus.Infof("Got %d licenses from downloader", len(licenses.Licenses)) @@ -85,7 +86,7 @@ func (catalog *Catalog) WriteLicensesAsText(targetDir string) error { } if !util.Exists(targetDir) { if err := os.MkdirAll(targetDir, os.FileMode(0o755)); err != nil { - return errors.Wrap(err, "creating license data dir") + return fmt.Errorf("creating license data dir: %w", err) } } wg := sync.WaitGroup{} @@ -101,13 +102,13 @@ func (catalog *Catalog) WriteLicensesAsText(targetDir string) error { if err == nil { err = lerr } else { - err = errors.Wrap(err, lerr.Error()) + err = fmt.Errorf(lerr.Error(), err) } } }(l) } wg.Wait() - return errors.Wrap(err, "caught errors while writing license files") + return fmt.Errorf("caught errors while writing license files: %w", err) } // GetLicense returns a license struct from its SPDX ID label diff --git a/pkg/license/download.go b/pkg/license/download.go index ef899944..7c33e135 100644 --- a/pkg/license/download.go +++ b/pkg/license/download.go @@ -25,13 +25,13 @@ package license import ( "crypto/sha1" "encoding/json" + "errors" "fmt" "os" "path/filepath" "strings" "github.com/nozzle/throttler" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "sigs.k8s.io/release-utils/http" @@ -52,7 +52,7 @@ func NewDownloader() (*Downloader, error) { // NewDownloaderWithOptions returns a downloader with specific options func NewDownloaderWithOptions(opts *DownloaderOptions) (*Downloader, error) { if err := opts.Validate(); err != nil { - return nil, errors.Wrap(err, "validating downloader options") + return nil, fmt.Errorf("validating downloader options: %w", err) } impl := DefaultDownloaderImpl{} impl.SetOptions(opts) @@ -80,11 +80,11 @@ func (do *DownloaderOptions) Validate() error { dir, err := os.MkdirTemp(os.TempDir(), "license-cache-") do.CacheDir = dir if err != nil { - return errors.Wrap(err, "creating temporary directory") + return fmt.Errorf("creating temporary directory: %w", err) } } else if !util.Exists(do.CacheDir) { if err := os.MkdirAll(do.CacheDir, os.FileMode(0o755)); err != nil { - return errors.Wrap(err, "creating license downloader cache") + return fmt.Errorf("creating license downloader cache: %w", err) } } @@ -145,12 +145,12 @@ func (ddi *DefaultDownloaderImpl) GetLicenses() (licenses *List, err error) { // Get the list of licenses licensesJSON, err := http.NewAgent().Get(LicenseDataURL + LicenseListFilename) if err != nil { - return nil, errors.Wrap(err, "fetching licenses list") + return nil, fmt.Errorf("fetching licenses list: %w", err) } licenseList := &List{} if err := json.Unmarshal(licensesJSON, licenseList); err != nil { - return nil, errors.Wrap(err, "parsing SPDX licence list") + return nil, fmt.Errorf("parsing SPDX licence list: %w", err) } logrus.Infof("Read data for %d licenses. Downloading.", len(licenseList.LicenseData)) @@ -200,10 +200,10 @@ func (ddi *DefaultDownloaderImpl) cacheData(url string, data []byte) error { _, err := os.Stat(filepath.Dir(cacheFileName)) if err != nil && os.IsNotExist(err) { if err := os.MkdirAll(filepath.Dir(cacheFileName), os.FileMode(0o755)); err != nil { - return errors.Wrap(err, "creating cache directory") + return fmt.Errorf("creating cache directory: %w", err) } } - return errors.Wrap(os.WriteFile(cacheFileName, data, os.FileMode(0o644)), "writing cache file") + return fmt.Errorf("writing cache file: %w", os.WriteFile(cacheFileName, data, os.FileMode(0o644))) } // getCachedData returns cached data for an URL if we have it @@ -211,7 +211,7 @@ func (ddi *DefaultDownloaderImpl) getCachedData(url string) ([]byte, error) { cacheFileName := ddi.cacheFileName(url) finfo, err := os.Stat(cacheFileName) if err != nil && !os.IsNotExist(err) { - return nil, errors.Wrap(err, "checking if cached data exists") + return nil, fmt.Errorf("checking if cached data exists: %w", err) } if err != nil { @@ -221,11 +221,11 @@ func (ddi *DefaultDownloaderImpl) getCachedData(url string) ([]byte, error) { if finfo.Size() == 0 { logrus.Warn("Cached file is empty, removing") - return nil, errors.Wrap(os.Remove(cacheFileName), "removing corrupt cached file") + return nil, fmt.Errorf("removing corrupt cached file: %w", os.Remove(cacheFileName)) } licensesJSON, err := os.ReadFile(cacheFileName) if err != nil { - return nil, errors.Wrap(err, "reading cached data file") + return nil, fmt.Errorf("reading cached data file: %w", err) } return licensesJSON, nil } @@ -237,7 +237,7 @@ func (ddi *DefaultDownloaderImpl) getLicenseFromURL(url string) (license *Licens if ddi.Options.EnableCache { licenseJSON, err = ddi.getCachedData(url) if err != nil { - return nil, errors.Wrap(err, "checking download cache") + return nil, fmt.Errorf("checking download cache: %w", err) } if len(licenseJSON) > 0 { logrus.Debugf("Data for %s is already cached", url) @@ -249,14 +249,14 @@ func (ddi *DefaultDownloaderImpl) getLicenseFromURL(url string) (license *Licens logrus.Infof("Downloading license data from %s", url) licenseJSON, err = http.NewAgent().Get(url) if err != nil { - return nil, errors.Wrapf(err, "getting %s", url) + return nil, fmt.Errorf("getting %s: %w", url, err) } logrus.Infof("Downloaded %d bytes from %s", len(licenseJSON), url) if ddi.Options.EnableCache { if err := ddi.cacheData(url, licenseJSON); err != nil { - return nil, errors.Wrap(err, "caching url data") + return nil, fmt.Errorf("caching url data: %w", err) } } } @@ -264,7 +264,7 @@ func (ddi *DefaultDownloaderImpl) getLicenseFromURL(url string) (license *Licens // Parse the SPDX license from the JSON data l, err := ParseLicense(licenseJSON) if err != nil { - return nil, errors.Wrap(err, "parsing license json data") + return nil, fmt.Errorf("parsing license json data: %w", err) } return l, err } diff --git a/pkg/license/implementation.go b/pkg/license/implementation.go index 07ea78bc..55c40baa 100644 --- a/pkg/license/implementation.go +++ b/pkg/license/implementation.go @@ -17,12 +17,12 @@ limitations under the License. package license import ( + "fmt" "os" "path/filepath" "regexp" licenseclassifier "github.com/google/licenseclassifier/v2" - "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -37,7 +37,7 @@ type ReaderDefaultImpl struct { func (d *ReaderDefaultImpl) ClassifyFile(path string) (licenseTag string, moreTags []string, err error) { file, err := os.Open(path) if err != nil { - return licenseTag, nil, errors.Wrap(err, "opening file for analysis") + return licenseTag, nil, fmt.Errorf("opening file for analysis: %w", err) } defer file.Close() @@ -67,7 +67,7 @@ func (d *ReaderDefaultImpl) ClassifyLicenseFiles(paths []string) ( for _, f := range paths { label, _, err := d.ClassifyFile(f) if err != nil { - return nil, unrecognizedPaths, errors.Wrap(err, "classifying file") + return nil, unrecognizedPaths, fmt.Errorf("classifying file: %w", err) } if label == "" { unrecognizedPaths = append(unrecognizedPaths, f) @@ -77,11 +77,11 @@ func (d *ReaderDefaultImpl) ClassifyLicenseFiles(paths []string) ( license := d.catalog.GetLicense(label) if license == nil { return nil, unrecognizedPaths, - errors.Errorf("ID does not correspond to a valid license: '%s'", label) + fmt.Errorf("ID does not correspond to a valid license: '%s'", label) } licenseText, err := os.ReadFile(f) if err != nil { - return nil, nil, errors.Wrap(err, "reading license text") + return nil, nil, fmt.Errorf("reading license text: %w", err) } // Apend to the return results licenseList = append(licenseList, &ClassifyResult{f, string(licenseText), license}) @@ -105,7 +105,7 @@ func (d *ReaderDefaultImpl) LicenseFromFile(path string) (license *License, err // Run the files through the clasifier label, _, err := d.ClassifyFile(path) if err != nil { - return nil, errors.Wrap(err, "classifying file") + return nil, fmt.Errorf("classifying file: %w", err) } if label == "" { @@ -116,7 +116,7 @@ func (d *ReaderDefaultImpl) LicenseFromFile(path string) (license *License, err // Get the license corresponding to the ID label license = d.catalog.GetLicense(label) if license == nil { - return nil, errors.Errorf("ID does not correspond to a valid license: %s", label) + return nil, fmt.Errorf("ID does not correspond to a valid license: %s", label) } return license, nil @@ -148,7 +148,7 @@ func (d *ReaderDefaultImpl) FindLicenseFiles(path string) ([]string, error) { } return nil }); err != nil { - return nil, errors.Wrap(err, "scanning the directory for license files") + return nil, fmt.Errorf("scanning the directory for license files: %w", err) } logrus.Infof("%d license files found in directory", len(licenseList)) return licenseList, nil @@ -158,7 +158,7 @@ func (d *ReaderDefaultImpl) FindLicenseFiles(path string) ([]string, error) { func (d *ReaderDefaultImpl) Initialize(opts *ReaderOptions) error { // Validate our options before startin if err := opts.Validate(); err != nil { - return errors.Wrap(err, "validating the license reader options") + return fmt.Errorf("validating the license reader options: %w", err) } // Create the implementation's SPDX object @@ -166,24 +166,24 @@ func (d *ReaderDefaultImpl) Initialize(opts *ReaderOptions) error { catalogOpts.CacheDir = opts.CachePath() catalog, err := NewCatalogWithOptions(catalogOpts) if err != nil { - return errors.Wrap(err, "creating SPDX object") + return fmt.Errorf("creating SPDX object: %w", err) } d.catalog = catalog if err := d.catalog.LoadLicenses(); err != nil { - return errors.Wrap(err, "loading licenses") + return fmt.Errorf("loading licenses: %w", err) } logrus.Infof("Writing license data to %s", opts.CachePath()) // Write the licenses to disk as th classifier will need them if err := catalog.WriteLicensesAsText(opts.LicensesPath()); err != nil { - return errors.Wrap(err, "writing license data to disk") + return fmt.Errorf("writing license data to disk: %w", err) } // Create the implementation's classifier d.lc = licenseclassifier.NewClassifier(opts.ConfidenceThreshold) - return errors.Wrap(d.lc.LoadLicenses(opts.LicensesPath()), "loading licenses at init") + return fmt.Errorf("loading licenses at init: %w", d.lc.LoadLicenses(opts.LicensesPath())) } // Classifier returns the license classifier diff --git a/pkg/license/license.go b/pkg/license/license.go index 346a3149..e85d0d68 100644 --- a/pkg/license/license.go +++ b/pkg/license/license.go @@ -20,13 +20,13 @@ package license import ( "bufio" + "fmt" "encoding/json" "os" "path/filepath" "strings" "sync" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "sigs.k8s.io/release-utils/util" @@ -72,9 +72,9 @@ type Reader struct { // SetImplementation sets the implementation that the license reader will use func (r *Reader) SetImplementation(i ReaderImplementation) error { r.impl = i - return errors.Wrap( - r.impl.Initialize(r.Options), - "initializing the reader implementation", + return fmt.Errorf( + "initializing the reader implementation: %w", + r.impl.Initialize(r.Options) ) } @@ -86,14 +86,14 @@ func NewReader() (*Reader, error) { // NewReaderWithOptions returns a new license reader with the specified options func NewReaderWithOptions(opts *ReaderOptions) (r *Reader, err error) { if err := opts.Validate(); err != nil { - return nil, errors.Wrap(err, "validating reader options") + return nil, fmt.Errorf("validating reader options: %w", err) } r = &Reader{ Options: opts, } if err := r.SetImplementation(&ReaderDefaultImpl{}); err != nil { - return nil, errors.Wrap(err, "setting the reader implementation") + return nil, fmt.Errorf("setting the reader implementation: %w", err) } return r, nil @@ -113,12 +113,12 @@ func (ro *ReaderOptions) Validate() error { if ro.WorkDir == "" { dir, err := os.MkdirTemp("", "license-reader-") if err != nil { - return errors.Wrap(err, "creating working dir") + return fmt.Errorf("creating working dir: %w", err) } ro.WorkDir = dir // Otherwise, check it exists } else if _, err := os.Stat(ro.WorkDir); err != nil { - return errors.Wrap(err, "checking working directory") + return fmt.Errorf("checking working directory: %w", err) } // TODO check dirs @@ -157,7 +157,7 @@ func (r *Reader) LicenseFromLabel(label string) (license *License) { func (r *Reader) LicenseFromFile(filePath string) (license *License, err error) { license, err = r.impl.LicenseFromFile(filePath) if err != nil { - return nil, errors.Wrap(err, "classifying file to determine license") + return nil, fmt.Errorf("classifying file to determine license: %w", err) } return license, err } @@ -177,7 +177,7 @@ func (r *Reader) ReadTopLicense(path string) (*ClassifyResult, error) { if licenseFilePath != "" { result, _, err := r.impl.ClassifyLicenseFiles([]string{licenseFilePath}) if err != nil { - return nil, errors.Wrap(err, "scanning topmost license file") + return nil, fmt.Errorf("scanning topmost license file: %w", err) } if len(result) != 0 { logrus.Infof("Concluded license %s from %s", result[0].License.LicenseID, licenseFilePath) @@ -191,7 +191,7 @@ func (r *Reader) ReadTopLicense(path string) (*ClassifyResult, error) { bestPartsN := 0 licenseFiles, err := r.impl.FindLicenseFiles(path) if err != nil { - return nil, errors.Wrap(err, "finding license files in path") + return nil, fmt.Errorf("finding license files in path: %w", err) } for _, fileName := range licenseFiles { try := false @@ -213,7 +213,7 @@ func (r *Reader) ReadTopLicense(path string) (*ClassifyResult, error) { result, _, err := r.impl.ClassifyLicenseFiles([]string{fileName}) if err != nil { - return nil, errors.Wrap(err, "scanning topmost license file") + return nil, fmt.Errorf("scanning topmost license file: %w", err) } // If the file is a license, use it @@ -237,12 +237,12 @@ func (r *Reader) ReadLicenses(path string) ( ) { licenseFiles, err := r.impl.FindLicenseFiles(path) if err != nil { - return nil, nil, errors.Wrap(err, "searching for license files") + return nil, nil, fmt.Errorf("searching for license files: %w", err) } licenseList, unknownPaths, err = r.impl.ClassifyLicenseFiles(licenseFiles) if err != nil { - return nil, nil, errors.Wrap(err, "classifying found licenses") + return nil, nil, fmt.Errorf("classifying found licenses: %w", err) } return licenseList, unknownPaths, nil } @@ -272,7 +272,7 @@ func HasKubernetesBoilerPlate(filePath string) (bool, error) { // kubernetesBoilerPlate sut, err := os.Open(filePath) if err != nil { - return false, errors.Wrap(err, "opening file to check for k8s boilerplate") + return false, fmt.Errorf("opening file to check for k8s boilerplate: %w", err) } defer sut.Close() @@ -332,10 +332,11 @@ type License struct { // WriteText writes the SPDX license text to a text file func (license *License) WriteText(filePath string) error { - return errors.Wrap( + return fmt.Errorf( + "while writing license to text file: %w", os.WriteFile( filePath, []byte(license.LicenseText), os.FileMode(0o644), - ), "while writing license to text file", + ) ) } @@ -355,7 +356,7 @@ type ListEntry struct { func ParseLicense(licenseJSON []byte) (license *License, err error) { license = &License{} if err := json.Unmarshal(licenseJSON, license); err != nil { - return nil, errors.Wrap(err, "parsing SPDX licence") + return nil, fmt.Errorf("parsing SPDX licence: %w", err) } return license, nil } diff --git a/pkg/license/license_test.go b/pkg/license/license_test.go index 1c293060..2fde2d7b 100644 --- a/pkg/license/license_test.go +++ b/pkg/license/license_test.go @@ -17,11 +17,11 @@ limitations under the License. package license_test import ( + "errors" "os" "path/filepath" "testing" - "github.com/pkg/errors" "github.com/stretchr/testify/require" "sigs.k8s.io/bom/pkg/license" From 0ba14011def3a6334ba6710907f604863c943bd8 Mon Sep 17 00:00:00 2001 From: DiptoChakrabarty Date: Tue, 21 Jun 2022 21:28:50 +0530 Subject: [PATCH 2/6] fix newline error --- pkg/license/license.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/license/license.go b/pkg/license/license.go index e85d0d68..fcc8f924 100644 --- a/pkg/license/license.go +++ b/pkg/license/license.go @@ -20,8 +20,8 @@ package license import ( "bufio" - "fmt" "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -74,7 +74,7 @@ func (r *Reader) SetImplementation(i ReaderImplementation) error { r.impl = i return fmt.Errorf( "initializing the reader implementation: %w", - r.impl.Initialize(r.Options) + r.impl.Initialize(r.Options), ) } @@ -336,7 +336,7 @@ func (license *License) WriteText(filePath string) error { "while writing license to text file: %w", os.WriteFile( filePath, []byte(license.LicenseText), os.FileMode(0o644), - ) + ), ) } From 9785ff4ba4ff88b23b4935477f993cecfb73852b Mon Sep 17 00:00:00 2001 From: DiptoChakrabarty Date: Tue, 21 Jun 2022 21:43:50 +0530 Subject: [PATCH 3/6] check error before return --- pkg/license/license.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/license/license.go b/pkg/license/license.go index fcc8f924..aa372e29 100644 --- a/pkg/license/license.go +++ b/pkg/license/license.go @@ -72,10 +72,10 @@ type Reader struct { // SetImplementation sets the implementation that the license reader will use func (r *Reader) SetImplementation(i ReaderImplementation) error { r.impl = i - return fmt.Errorf( - "initializing the reader implementation: %w", - r.impl.Initialize(r.Options), - ) + if err := r.impl.Initialize(r.Options); err != nil { + return fmt.Errorf("initializing the reader implementation: %w", err) + } + return nil } // NewReader returns a license reader with the default options @@ -332,12 +332,10 @@ type License struct { // WriteText writes the SPDX license text to a text file func (license *License) WriteText(filePath string) error { - return fmt.Errorf( - "while writing license to text file: %w", - os.WriteFile( - filePath, []byte(license.LicenseText), os.FileMode(0o644), - ), - ) + if err := os.WriteFile(filePath, []byte(license.LicenseText), os.FileMode(0o644)); err != nil { + return fmt.Errorf("while writing license to text file: %w", err) + } + return nil } // ListEntry a license entry in the list From 0a0550d61c414a9f900a0a07dedbcc2e67b568c5 Mon Sep 17 00:00:00 2001 From: DiptoChakrabarty Date: Tue, 21 Jun 2022 21:48:11 +0530 Subject: [PATCH 4/6] check err before return --- pkg/license/download.go | 5 ++++- pkg/license/implementation.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/license/download.go b/pkg/license/download.go index 7c33e135..86c98c0c 100644 --- a/pkg/license/download.go +++ b/pkg/license/download.go @@ -203,7 +203,10 @@ func (ddi *DefaultDownloaderImpl) cacheData(url string, data []byte) error { return fmt.Errorf("creating cache directory: %w", err) } } - return fmt.Errorf("writing cache file: %w", os.WriteFile(cacheFileName, data, os.FileMode(0o644))) + if err = os.WriteFile(cacheFileName, data, os.FileMode(0o644)); err != nil { + return fmt.Errorf("writing cache file: %w", err) + } + return nil } // getCachedData returns cached data for an URL if we have it diff --git a/pkg/license/implementation.go b/pkg/license/implementation.go index 55c40baa..c843319e 100644 --- a/pkg/license/implementation.go +++ b/pkg/license/implementation.go @@ -183,7 +183,10 @@ func (d *ReaderDefaultImpl) Initialize(opts *ReaderOptions) error { // Create the implementation's classifier d.lc = licenseclassifier.NewClassifier(opts.ConfidenceThreshold) - return fmt.Errorf("loading licenses at init: %w", d.lc.LoadLicenses(opts.LicensesPath())) + if err := d.lc.LoadLicenses(opts.LicensesPath()); err != nil { + return fmt.Errorf("loading licenses at init: %w", err) + } + return nil } // Classifier returns the license classifier From 4ade5205836eb70ff323170873af196da5ceaba4 Mon Sep 17 00:00:00 2001 From: DiptoChakrabarty Date: Tue, 21 Jun 2022 21:52:03 +0530 Subject: [PATCH 5/6] check err in catalog writelicense --- pkg/license/catalog.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/license/catalog.go b/pkg/license/catalog.go index fa622d51..7c408871 100644 --- a/pkg/license/catalog.go +++ b/pkg/license/catalog.go @@ -108,7 +108,11 @@ func (catalog *Catalog) WriteLicensesAsText(targetDir string) error { }(l) } wg.Wait() - return fmt.Errorf("caught errors while writing license files: %w", err) + + if err != nil { + return fmt.Errorf("caught errors while writing license files: %w", err) + } + return nil } // GetLicense returns a license struct from its SPDX ID label From 73aa0d4d8464b00a0dfe8c55ff64161716b1781a Mon Sep 17 00:00:00 2001 From: DiptoChakrabarty Date: Wed, 22 Jun 2022 18:23:10 +0530 Subject: [PATCH 6/6] update error with type --- pkg/license/catalog.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/license/catalog.go b/pkg/license/catalog.go index 7c408871..8603908a 100644 --- a/pkg/license/catalog.go +++ b/pkg/license/catalog.go @@ -102,7 +102,7 @@ func (catalog *Catalog) WriteLicensesAsText(targetDir string) error { if err == nil { err = lerr } else { - err = fmt.Errorf(lerr.Error(), err) + err = fmt.Errorf("%v: %w", lerr, err) } } }(l)