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

downgrade yamlfmt to go 1.18 #105

Merged
merged 1 commit into from
Apr 1, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
go: [ '1.20' ]
go: [ '1.18', '1.19', '1.20' ]
steps:
- uses: actions/checkout@v3

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
if: startsWith(github.ref, 'refs/tags/')
strategy:
matrix:
version: ['1.20']
version: ['1.18']
env:
GORELEASER_CURRENT_TAG: ${{ github.ref_name }}
steps:
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
- id: yamlfmt
name: yamlfmt
description: This hook uses github.com/google/yamlfmt to format yaml files. Requires golang >1.20 to be installed.
description: This hook uses github.com/google/yamlfmt to format yaml files. Requires golang >1.18 to be installed.
entry: yamlfmt
language: golang
types: [yaml]
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ To download the `yamlfmt` command, you can download the desired binary from rele
```
go install github.com/google/yamlfmt/cmd/yamlfmt@latest
```
This currently requires Go version 1.20 or greater.
This currently requires Go version 1.18 or greater.

NOTE: Recommended setup if this is your first time installing Go would be in [this DigitalOcean blog post](https://www.digitalocean.com/community/tutorials/how-to-build-and-install-go-programs).

Expand Down
5 changes: 3 additions & 2 deletions cmd/yamlfmt/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/braydonk/yaml"
"github.com/google/yamlfmt"
"github.com/google/yamlfmt/command"
"github.com/google/yamlfmt/internal/collections"
"github.com/mitchellh/mapstructure"
)

Expand Down Expand Up @@ -227,7 +228,7 @@ func makeCommandConfigFromData(configData map[string]any) (*command.Config, erro

func parseFormatterConfigFlag(flagValues []string) (map[string]any, error) {
formatterValues := map[string]any{}
flagErrors := []error{}
flagErrors := collections.Errors{}

// Expected format: fieldname=value
for _, configField := range flagValues {
Expand Down Expand Up @@ -259,5 +260,5 @@ func parseFormatterConfigFlag(flagValues []string) (map[string]any, error) {
formatterValues[kv[0]] = kv[1]
}

return formatterValues, errors.Join(flagErrors...)
return formatterValues, flagErrors.Combine()
}
6 changes: 3 additions & 3 deletions engine.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package yamlfmt

import (
"errors"
"fmt"
"os"

"github.com/RageCage64/multilinediff"
"github.com/google/yamlfmt/internal/collections"
)

type Engine interface {
Expand Down Expand Up @@ -98,13 +98,13 @@ func (fds FileDiffs) StrOutputQuiet() string {
}

func (fds FileDiffs) ApplyAll() error {
applyErrs := make([]error, len(fds))
applyErrs := make(collections.Errors, len(fds))
i := 0
for _, diff := range fds {
applyErrs[i] = diff.Apply()
i++
}
return errors.Join(applyErrs...)
return applyErrs.Combine()
}

func (fds FileDiffs) ChangedCount() int {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/google/yamlfmt

go 1.20
go 1.18

require (
github.com/RageCage64/multilinediff v0.2.0
Expand Down
20 changes: 20 additions & 0 deletions internal/collections/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package collections

import "errors"

type Errors []error

func (errs Errors) Combine() error {
errMessage := ""

for _, err := range errs {
if err != nil {
errMessage += err.Error() + "\n"
}
}

if len(errMessage) == 0 {
return nil
}
return errors.New(errMessage)
}
45 changes: 45 additions & 0 deletions internal/collections/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package collections_test

import (
"errors"
"strings"
"testing"

"github.com/google/yamlfmt/internal/collections"
)

func TestErrorsCombine(t *testing.T) {
errs := collections.Errors{
errors.New("a"),
nil,
errors.New("c"),
}
err := errs.Combine()
if err == nil {
t.Fatal("expected combined err not to be nil")
}
for _, errEl := range errs {
if errEl == nil {
continue
}
if !strings.Contains(err.Error(), errEl.Error()) {
t.Fatalf("expected combined err to contain %v, got: %v", errEl, err)
}
}
}

func TestErrorsCombineEmpty(t *testing.T) {
errs := collections.Errors{}
err := errs.Combine()
if err != nil {
t.Fatalf("expected combined err to be nil, got: %v", err)
}
}

func TestErrorsCombineNilElements(t *testing.T) {
errs := collections.Errors{nil, nil, nil}
err := errs.Combine()
if err != nil {
t.Fatalf("expected combined err to be nil, got: %v", err)
}
}
9 changes: 8 additions & 1 deletion path_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package yamlfmt

import (
"io/fs"
"log"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -115,7 +116,13 @@ func (c *DoublestarCollector) CollectPaths() ([]string, error) {
}
excluded := false
for _, pattern := range c.Exclude {
match, err := doublestar.Match(filepath.Clean(pattern), path)
absPath, err := filepath.Abs(path)
if err != nil {
// I wonder how this could ever happen...
log.Printf("could not create absolute path for %s: %v", path, err)
Comment on lines +121 to +122
Copy link
Contributor

Choose a reason for hiding this comment

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

This literally never happens on a day-to-day basis, but an error can trigger under some improbable circumstances
Internally filepath.Abs calls the getwd syscall, which has quite a few possible errors : https://linux.die.net/man/3/getwd

Here is a small example triggering an error on purpose : https://go.dev/play/p/JoPt8x9bQJl

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah good to know, thank you for the information!

I think I am okay with how I'm handling this for now then; if it was able to collect that path in the first place up until this point, then failing getwd doesn't necessarily mean the program can't eventually read that file from the relative path it currently has. So logging the error and moving on will at least notify the user that if they expect that path to be excluded, something unexpected went wrong.

(I probably should have thrown a newline on that log though...)

continue
}
match, err := doublestar.PathMatch(filepath.Clean(pattern), absPath)
if err != nil {
return nil, err
}
Expand Down