Merge pull request #1 from rstudio/sj/tests

tests: add tests and GHA workflow

.gitattributes

@@ -0,0 +1 @@
+testdata/ linguist-generated=true

.github/dependabot.yml

@@ -0,0 +1,17 @@
+# Please see the documentation for all configuration options:
+# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates
+
+version: 2
+updates:
+  - package-ecosystem: "github-actions"
+    directory: "/"
+    schedule:
+      interval: "monthly"
+    reviewers:
+      - "rstudio/ppm"
+  - package-ecosystem: "gomod"
+    directory: "/"
+    schedule:
+      interval: "monthly"
+    reviewers:
+      - "rstudio/ppm"

.github/workflows/go.yml

@@ -0,0 +1,43 @@
+name: Go CI
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - name: install go
+        uses: actions/setup-go@v4
+        with:
+          go-version: 'stable'
+      - name: install python
+        uses: actions/setup-python@v4
+        with:
+          python-version: '3.x'
+      # Rust is required to build some packages
+      - name: install rust
+        uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+      - name: install python dependencies
+        run: pip install twine build wheel
+      - name: install go dependencies
+        run: go install gotest.tools/gotestsum@latest
+      - name: run golangci-lint
+        uses: reviewdog/action-golangci-lint@v2
+      - name: build
+        run: go build
+      - name: test
+        run: gotestsum --jsonfile unit-tests.json
+      - name: annotate tests
+        if: always()
+        uses: guyarb/[email protected]
+        with:
+          test-results: unit-tests.json

.gitignore

@@ -0,0 +1,4 @@
+testdata/repositories/
+!testdata/repositories/.gitkeep
+.idea
+*.iml

README.md

@@ -113,4 +113,16 @@ And here is example output:
     "blake2_256_digest": "2a8c9ca4a072d43e00503f16d08db68d690644949ac3e4704ba6c2e7028e8402"
   }
 ]
-```
+```
+
+## Linting
+
+```bash
+golangci-lint run --fix ./...
+```
+
+## Testing
+
+```bash
+go test ./...
+```

go.mod

@@ -3,8 +3,17 @@ module github.com/rstudio/python-distribution-parser
 go 1.21
 
 require (
-	golang.org/x/crypto v0.13.0
+	github.com/google/go-cmp v0.6.0
+	github.com/samber/lo v1.38.1
+	github.com/stretchr/testify v1.8.4
+	golang.org/x/crypto v0.14.0
 	golang.org/x/text v0.13.0
 )
 
-require golang.org/x/sys v0.12.0 // indirect
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
+	golang.org/x/sys v0.13.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)

go.sum

@@ -1,6 +1,40 @@
+github.com/bradleyjkemp/cupaloy v2.3.0+incompatible h1:UafIjBvWQmS9i/xRg+CamMrnLTKNzo+bdmT/oH34c2Y=
+github.com/bradleyjkemp/cupaloy v2.3.0+incompatible/go.mod h1:Au1Xw1sgaJ5iSFktEhYsS0dbQiS1B0/XMXl+42y9Ilk=
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
+github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/samber/lo v1.38.1 h1:j2XEAqXKb09Am4ebOg31SpvzUTTs6EN3VfgeLUhPdXM=
+github.com/samber/lo v1.38.1/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
+github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
+github.com/stretchr/objx v0.5.1 h1:4VhoImhV/Bm0ToFkXFi8hXNXwpDRZ/ynw3amt82mzq0=
+github.com/stretchr/objx v0.5.1/go.mod h1:/iHQpkQwBD6DLUmQ4pE+s1TXdob1mORJ4/UFdrifcy0=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
+github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
+github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 golang.org/x/crypto v0.13.0 h1:mvySKfSWJ+UKUii46M40LOvyWfN0s2U+46/jDd0e6Ck=
 golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc=
+golang.org/x/crypto v0.14.0 h1:wBqGXzWJW6m1XrIKlAH0Hs1JJ7+9KBwnIO8v66Q9cHc=
+golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4=
+golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 h1:3MTrJm4PyNL9NBqvYDSj3DHl46qQakyfqfWo4jgfaEM=
+golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE=
+golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
+golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
 golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
 golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
+golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
 golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

archiver/archive_reader.go → internal/archiver/archive_reader.go

@@ -7,12 +7,13 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"strings"
 )
 
-// archiveReader is an interface to abstract the behavior of different archive types.
-type archiveReader interface {
+// ArchiveReader is an interface to abstract the behavior of different archive types.
+type ArchiveReader interface {
 	FileNames() ([]string, error)
 	ReadFile(name string) ([]byte, error)
 	Close() error
@@ -37,7 +38,12 @@ func (z *zipReader) ReadFile(name string) ([]byte, error) {
 			if err != nil {
 				return nil, err
 			}
-			defer rc.Close()
+			defer func(rc io.ReadCloser) {
+				err := rc.Close()
+				if err != nil {
+					log.Printf("error closing reader: %v", err)
+				}
+			}(rc)
 			return io.ReadAll(rc)
 		}
 	}
@@ -51,7 +57,10 @@ type tarReader struct {
 }
 
 func (t *tarReader) resetReader() error {
-	t.Close()
+	err := t.Close()
+	if err != nil {
+		return err
+	}
 
 	// Reopen the file
 	f, err := os.Open(t.filename)
@@ -62,7 +71,10 @@ func (t *tarReader) resetReader() error {
 	if strings.HasSuffix(t.filename, ".tar.gz") || strings.HasSuffix(t.filename, ".tgz") {
 		gzr, err := gzip.NewReader(f)
 		if err != nil {
-			f.Close()
+			cerr := f.Close()
+			if cerr != nil {
+				return cerr
+			}
 			return err
 		}
 		t.Reader = tar.NewReader(gzr) // Reset the tar reader with new gzip reader
@@ -72,7 +84,12 @@ func (t *tarReader) resetReader() error {
 }
 
 func (t *tarReader) FileNames() ([]string, error) {
-	defer t.resetReader()
+	defer func(t *tarReader) {
+		err := t.resetReader()
+		if err != nil {
+			log.Printf("error resetting reader: %v", err)
+		}
+	}(t)
 	var names []string
 	for {
 		hdr, err := t.Next()
@@ -106,7 +123,7 @@ func (t *tarReader) Close() error {
 	return t.closer.Close()
 }
 
-func NewArchiveReader(fqn string) (archiveReader, error) {
+func NewArchiveReader(fqn string) (ArchiveReader, error) {
 	_, err := os.Stat(fqn)
 	if errors.Is(err, os.ErrNotExist) {
 		return nil, fmt.Errorf("no such file: %s", fqn)
@@ -127,7 +144,10 @@ func NewArchiveReader(fqn string) (archiveReader, error) {
 	if strings.HasSuffix(fqn, ".tar.gz") || strings.HasSuffix(fqn, ".tgz") {
 		gzr, err := gzip.NewReader(f)
 		if err != nil {
-			f.Close()
+			err := f.Close()
+			if err != nil {
+				log.Printf("error closing file: %v", err)
+			}
 			return nil, err
 		}
 		r := tar.NewReader(gzr)
@@ -138,7 +158,10 @@ func NewArchiveReader(fqn string) (archiveReader, error) {
 		return &tarReader{fqn, r, f}, nil
 	}
 
-	f.Close()
+	err = f.Close()
+	if err != nil {
+		log.Printf("error closing file: %v", err)
+	}
 	return nil, fmt.Errorf("not a known archive format: %s", fqn)
 }
 

distributions/distribution.go → internal/distributions/distribution.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"net/mail"
 	"reflect"
+	"regexp"
 	"strings"
 	"unicode"
 
@@ -51,7 +52,7 @@ func mustDecode(value interface{}) (string, error) {
 }
 
 func collapseLeadingWS(header, txt string) string {
-	if strings.ToLower(header) == "description" { // preserve newlines
+	if strings.ToLower(header) == "description" || strings.ToLower(header) == "license" { // preserve newlines
 		lines := strings.Split(strings.TrimSpace(txt), "\n")
 		for i, line := range lines {
 			if strings.HasPrefix(line, "        ") { // 8 spaces
@@ -77,8 +78,8 @@ var HeaderAttrs1_0 = []HeaderAttr{ // PEP 241
 	{"Metadata-Version", "metadata_version", false},
 	{"Name", "name", false},
 	{"Version", "version", false},
-	{"Platform", "platforms", true},
-	{"Supported-Platform", "supported_platforms", true},
+	{"Platform", "platform", true},
+	{"Supported-Platform", "supported_platform", true},
 	{"Summary", "summary", false},
 	{"Description", "description", false},
 	{"Keywords", "keywords", false},
@@ -90,7 +91,7 @@ var HeaderAttrs1_0 = []HeaderAttr{ // PEP 241
 
 var HeaderAttrs1_1 = append(HeaderAttrs1_0, []HeaderAttr{ // PEP 314
 	{"Classifier", "classifiers", true},
-	{"Download-URL", "download_url", false},
+	{"Download-Url", "download_url", false},
 	{"Requires", "requires", true},
 	{"Provides", "provides", true},
 	{"Obsoletes", "obsoletes", true},
@@ -104,7 +105,7 @@ var HeaderAttrs1_2 = append(HeaderAttrs1_1, []HeaderAttr{ // PEP 345
 	{"Requires-Dist", "requires_dist", true},
 	{"Provides-Dist", "provides_dist", true},
 	{"Obsoletes-Dist", "obsoletes_dist", true},
-	{"Project-URL", "project_urls", true},
+	{"Project-Url", "project_urls", true},
 }...)
 
 var HeaderAttrs2_0 = HeaderAttrs1_2 //XXX PEP 426?
@@ -129,12 +130,12 @@ type Distribution interface {
 	ExtractMetadata() error
 	Parse(data []byte) error
 
-	// Helper method to get values
+	// GetName is a helper method to get values
 	GetName() string
 	GetVersion() string
 	GetPythonVersion() string
 
-	// This is used to return a map of all the metadata,
+	// MetadataMap is used to return a map of all the metadata,
 	// similar to how twine passes the metadata in a multipart
 	// form request.
 	MetadataMap() map[string][]string
@@ -145,8 +146,8 @@ type BaseDistribution struct {
 	// version 1.0
 	Name               string   `json:"name"`
 	Version            string   `json:"version"`
-	Platforms          []string `json:"platforms"`
-	SupportedPlatforms []string `json:"supported_platforms"`
+	Platforms          []string `json:"platform"`
+	SupportedPlatforms []string `json:"supported_platform"`
 	Summary            string   `json:"summary"`
 	Description        string   `json:"description"`
 	Keywords           string   `json:"keywords"`
@@ -207,11 +208,18 @@ func (bd *BaseDistribution) Parse(data []byte) error {
 		headerValues := getAllHeaderValues(msg, headerAttr.HeaderName)
 		if len(headerValues) != 0 {
 			if headerAttr.Multiple {
-				bd.setJSONValue(headerAttr.AttrName, headerValues)
+				err := bd.setJSONValue(headerAttr.AttrName, headerValues)
+				if err != nil {
+					return err
+				}
 			} else if headerValues[0] != "UNKNOWN" {
-				bd.setJSONValue(headerAttr.AttrName, headerValues[0])
+				err := bd.setJSONValue(headerAttr.AttrName, headerValues[0])
+				if err != nil {
+					return err
+				}
 			}
 		}
+
 	}
 
 	body, err := io.ReadAll(msg.Body)
@@ -220,20 +228,23 @@ func (bd *BaseDistribution) Parse(data []byte) error {
 
 	}
 	if body != nil {
-		bd.setJSONValue("description", string(body))
+		err := bd.setJSONValue("description", string(body))
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }
 
 func (bd *BaseDistribution) GetName() string {
 	return bd.Name
 }
+
 func (bd *BaseDistribution) GetVersion() string {
 	return bd.Version
 }
 
-// Remember to implement this for other distributions
-// if they need something more specific (e.g. Wheels)
+// TODO: remember to implement this for other distributions if they need something more specific (e.g. Wheels)
 func (bd *BaseDistribution) GetPythonVersion() string {
 	return ""
 }
@@ -303,3 +314,12 @@ func StructToMap(input interface{}) map[string][]string {
 
 	return result
 }
+
+// Convert an arbitrary string to a standard distribution name.
+// Any runs of non-alphanumeric/. characters are replaced with a single '-'.
+// Copied from pkg_resources.safe_name for compatibility with warehouse.
+// See https://github.com/pypa/twine/issues/743.
+func SafeName(name string) string {
+	reg := regexp.MustCompile("[^A-Za-z0-9.]+")
+	return reg.ReplaceAllString(name, "-")
+}