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

fix: properly normalize Windows paths, add windows test runner #242

Merged
merged 5 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* text eol=lf
Copy link
Collaborator Author

@kzantow kzantow May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems logical, since go fmt produces only LF endings and if the .go files get checked out on Windows with CRLF, you get "every file changed" when running go fmt. This also solves the package verification code test issues, because the line endings don't change there, either.

18 changes: 16 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ name: build
on: [push, pull_request]
jobs:
tests:
runs-on: ubuntu-20.04
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@main
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: '1.18'
Expand All @@ -16,3 +16,17 @@ jobs:
uses: shogo82148/actions-goveralls@v1
with:
path-to-profile: profile.cov

tests-latest:
strategy:
matrix:
go-version: ['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: ${{ matrix.go-version }}
- name: Run tests
run: make test
28 changes: 6 additions & 22 deletions builder/build_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down
11 changes: 7 additions & 4 deletions utils/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,21 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"strings"
)

// 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 {
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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:]
Expand Down