From 99f2750431c7f58726b52240707e01e1e4dff110 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Fri, 17 May 2024 11:54:36 -0400 Subject: [PATCH 1/5] chore: add windows test runner Signed-off-by: Keith Zantow --- .gitattributes | 1 + .github/workflows/build.yml | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..fcadb2cf --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +* text eol=lf diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5e041aad..f11d3e94 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,17 +2,29 @@ name: build on: [push, pull_request] +env: + go-version: '1.18' jobs: tests: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@main + - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: '1.18' + go-version: ${{ env.go-version }} - name: Run tests run: make test - name: Send coverage report to coveralls uses: shogo82148/actions-goveralls@v1 with: path-to-profile: profile.cov + + windows-tests: + runs-on: windows-2022 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.go-version }} + - name: Run tests + run: make test From 61ccee802f88ba86080322cf6dbe88557be6ac45 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Fri, 17 May 2024 13:34:21 -0400 Subject: [PATCH 2/5] fix: properly handle windows file paths in utility functions Signed-off-by: Keith Zantow --- builder/build_package.go | 28 ++++++---------------------- utils/filesystem.go | 11 +++++++---- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/builder/build_package.go b/builder/build_package.go index c31c32de..100722f0 100644 --- a/builder/build_package.go +++ b/builder/build_package.go @@ -4,9 +4,6 @@ package builder import ( "fmt" - "path/filepath" - "regexp" - "runtime" "github.com/spdx/tools-golang/spdx" "github.com/spdx/tools-golang/spdx/v2/common" @@ -21,32 +18,19 @@ import ( func BuildPackageSection(packageName string, dirRoot string, pathsIgnore []string) (*spdx.Package, error) { // build the file section first, so we'll have it available // for calculating the package verification code - filepaths, err := utils.GetAllFilePaths(dirRoot, pathsIgnore) - osType := runtime.GOOS + relativePaths, err := utils.GetAllFilePaths(dirRoot, pathsIgnore) if err != nil { return nil, err } - re, ok := regexp.Compile("/+") - if ok != nil { - return nil, err - } - dirRootLen := 0 - if osType == "windows" { - dirRootLen = len(dirRoot) - } - files := []*spdx.File{} fileNumber := 0 - for _, fp := range filepaths { - newFilePatch := "" - if osType == "windows" { - newFilePatch = filepath.FromSlash("." + fp[dirRootLen:]) - } else { - newFilePatch = filepath.FromSlash("./" + fp) - } - newFile, err := BuildFileSection(re.ReplaceAllLiteralString(newFilePatch, string(filepath.Separator)), dirRoot, fileNumber) + for _, filePath := range relativePaths { + // SPDX spec says file names should generally start with ./ + // see: https://spdx.github.io/spdx-spec/v2.3/file-information/#81-file-name-field + relativePath := "." + filePath + newFile, err := BuildFileSection(relativePath, dirRoot, fileNumber) if err != nil { return nil, err } diff --git a/utils/filesystem.go b/utils/filesystem.go index bb8c53ee..bc8db172 100644 --- a/utils/filesystem.go +++ b/utils/filesystem.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "os" + "path" "path/filepath" "strings" ) @@ -17,12 +18,13 @@ import ( // GetAllFilePaths takes a path to a directory (including an optional slice of // path patterns to ignore), and returns a slice of relative paths to all files // in that directory and its subdirectories (excluding those that are ignored). +// These paths are always normalized to use URI-like forward-slashes but begin with / func GetAllFilePaths(dirRoot string, pathsIgnored []string) ([]string, error) { // paths is a _pointer_ to a slice -- not just a slice. // this is so that it can be appropriately modified by append // in the sub-function. paths := &[]string{} - prefix := strings.TrimSuffix(dirRoot, "/") + prefix := strings.TrimSuffix(filepath.ToSlash(dirRoot), "/") err := filepath.Walk(dirRoot, func(path string, fi os.FileInfo, err error) error { if err != nil { @@ -37,6 +39,7 @@ func GetAllFilePaths(dirRoot string, pathsIgnored []string) ([]string, error) { return nil } + path = filepath.ToSlash(path) shortPath := strings.TrimPrefix(path, prefix) // don't include path if it should be ignored @@ -55,7 +58,7 @@ func GetAllFilePaths(dirRoot string, pathsIgnored []string) ([]string, error) { // GetHashesForFilePath takes a path to a file on disk, and returns // SHA1, SHA256 and MD5 hashes for that file as strings. func GetHashesForFilePath(p string) (string, string, string, error) { - f, err := os.Open(p) + f, err := os.Open(filepath.FromSlash(p)) if err != nil { return "", "", "", err } @@ -82,11 +85,11 @@ func GetHashesForFilePath(p string) (string, string, string, error) { // and determines whether that file should be ignored because it matches // any of those patterns. func ShouldIgnore(fileName string, pathsIgnored []string) bool { - fDirs, fFile := filepath.Split(fileName) + fDirs, fFile := path.Split(fileName) for _, pattern := range pathsIgnored { // split into dir(s) and filename - patternDirs, patternFile := filepath.Split(pattern) + patternDirs, patternFile := path.Split(pattern) patternDirStars := strings.HasPrefix(patternDirs, "**") if patternDirStars { patternDirs = patternDirs[2:] From fdfed09dd04ad565c7a0a9bcfc6edc00d14fe258 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Sat, 18 May 2024 15:41:45 -0400 Subject: [PATCH 3/5] chore: use matrix on all available OSes and different go versions Signed-off-by: Keith Zantow --- .github/workflows/build.yml | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f11d3e94..364648ad 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,29 +2,22 @@ name: build on: [push, pull_request] -env: - go-version: '1.18' jobs: tests: - runs-on: ubuntu-20.04 + strategy: + matrix: + go-version: ['1.18', 'stable'] + os: [ubuntu-latest, windows-latest, macos-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: ${{ env.go-version }} + go-version: ${{ matrix.go-version }} - name: Run tests run: make test - name: Send coverage report to coveralls uses: shogo82148/actions-goveralls@v1 + if: ${{ matrix.os == 'ubuntu-latest' && matrix.go-version == 'stable' }} with: path-to-profile: profile.cov - - windows-tests: - runs-on: windows-2022 - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.go-version }} - - name: Run tests - run: make test From 457a9af6453969dc08e23da560f9a1657611490a Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Sat, 18 May 2024 15:56:08 -0400 Subject: [PATCH 4/5] chore: tests is a required check Signed-off-by: Keith Zantow --- .github/workflows/build.yml | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 364648ad..97144cfd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,9 +4,23 @@ name: build on: [push, pull_request] jobs: tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: '1.18' + - name: Run tests + run: make test + - name: Send coverage report to coveralls + uses: shogo82148/actions-goveralls@v1 + with: + path-to-profile: profile.cov + + tests-latest: strategy: matrix: - go-version: ['1.18', 'stable'] + go-version: ['stable'] os: [ubuntu-latest, windows-latest, macos-latest] runs-on: ${{ matrix.os }} steps: @@ -16,8 +30,3 @@ jobs: go-version: ${{ matrix.go-version }} - name: Run tests run: make test - - name: Send coverage report to coveralls - uses: shogo82148/actions-goveralls@v1 - if: ${{ matrix.os == 'ubuntu-latest' && matrix.go-version == 'stable' }} - with: - path-to-profile: profile.cov From 7509724ef87535ef057a9aec29edb4b2def01eaf Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Mon, 20 May 2024 10:44:34 -0400 Subject: [PATCH 5/5] chore: naming, gosimports Signed-off-by: Keith Zantow --- .github/workflows/build.yml | 4 ++-- builder/build_package.go | 8 ++++---- spdx/v2/v2_1/document.go | 2 +- spdx/v2/v2_2/json/empty_values_test.go | 6 ++++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 97144cfd..0dc3a9b9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -3,7 +3,7 @@ name: build on: [push, pull_request] jobs: - tests: + tests: # this is the only required check runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -17,7 +17,7 @@ jobs: with: path-to-profile: profile.cov - tests-latest: + test-platforms: strategy: matrix: go-version: ['stable'] diff --git a/builder/build_package.go b/builder/build_package.go index 100722f0..68ec353c 100644 --- a/builder/build_package.go +++ b/builder/build_package.go @@ -18,7 +18,7 @@ import ( func BuildPackageSection(packageName string, dirRoot string, pathsIgnore []string) (*spdx.Package, error) { // build the file section first, so we'll have it available // for calculating the package verification code - relativePaths, err := utils.GetAllFilePaths(dirRoot, pathsIgnore) + shortPaths, err := utils.GetAllFilePaths(dirRoot, pathsIgnore) if err != nil { return nil, err @@ -26,10 +26,10 @@ func BuildPackageSection(packageName string, dirRoot string, pathsIgnore []strin files := []*spdx.File{} fileNumber := 0 - for _, filePath := range relativePaths { - // SPDX spec says file names should generally start with ./ + for _, shortPath := range shortPaths { + // SPDX spec says file names should generally start with ./ and the shortPath already starts with / // see: https://spdx.github.io/spdx-spec/v2.3/file-information/#81-file-name-field - relativePath := "." + filePath + relativePath := "." + shortPath newFile, err := BuildFileSection(relativePath, dirRoot, fileNumber) if err != nil { return nil, err diff --git a/spdx/v2/v2_1/document.go b/spdx/v2/v2_1/document.go index 60a27c44..bd6b41d4 100644 --- a/spdx/v2/v2_1/document.go +++ b/spdx/v2/v2_1/document.go @@ -4,7 +4,7 @@ package v2_1 import ( - "github.com/anchore/go-struct-converter" + converter "github.com/anchore/go-struct-converter" "github.com/spdx/tools-golang/spdx/v2/common" ) diff --git a/spdx/v2/v2_2/json/empty_values_test.go b/spdx/v2/v2_2/json/empty_values_test.go index dc6e09ed..59abdc9d 100644 --- a/spdx/v2/v2_2/json/empty_values_test.go +++ b/spdx/v2/v2_2/json/empty_values_test.go @@ -2,10 +2,12 @@ package json import ( "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + "github.com/spdx/tools-golang/spdx/v2/common" spdx "github.com/spdx/tools-golang/spdx/v2/v2_2" - "github.com/stretchr/testify/require" - "testing" ) func Test_omitsAppropriateProperties(t *testing.T) {