Skip to content
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
39 changes: 26 additions & 13 deletions env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,35 +265,48 @@ func parseReader(r io.Reader) (Vars, []Problem, error) {
// alongside the error whenever a file was opened at all. Only [Result.Env] is
// nil, since a rejected file declares nothing.
func Load(paths ...string) (Result, error) {
file, path, err := openFirst(paths)
if err != nil {
return Result{}, err
}
return loadFile(file, path)
}

// openFirst opens the first candidate that exists, and reports which one that
// was: with a search path, the caller cannot otherwise tell.
//
// The file is returned open, and closing it is the caller's job from here.
func openFirst(paths []string) (*os.File, string, error) {
if len(paths) == 0 {
paths = []string{DefaultPath}
}

var (
file *os.File
path string
lastErr error
)
var lastErr error
for _, candidate := range paths {
f, err := os.Open(candidate)
if err == nil {
file, path = f, candidate
break
return f, candidate, nil
}
// Only a missing file is a miss. A candidate that exists but cannot be
// read — no permission, a directory — is a problem to report rather than
// a reason to look further: silently falling through to the next
// candidate would hide it.
if !errors.Is(err, iofs.ErrNotExist) {
return Result{}, fmt.Errorf("reading %s: %w", candidate, err)
return nil, "", fmt.Errorf("reading %s: %w", candidate, err)
}
lastErr = err
}
if file == nil {
return Result{}, fmt.Errorf("reading %s: %w", strings.Join(paths, ", "), lastErr)
}
return nil, "", fmt.Errorf("reading %s: %w", strings.Join(paths, ", "), lastErr)
}

v, problems, err := parseReader(file)
// loadFile reads an opened environment file, closes it, and reports what it
// declared. path names the file, for the Result and any [ParseError] to carry.
//
// It takes an interface where its only caller holds an *os.File, because the
// close it has to report on is the one thing a real file will not do: a
// descriptor opened read-only has nothing left to fail at.
func loadFile(rc io.ReadCloser, path string) (Result, error) {
v, problems, err := parseReader(rc)
// Closed here rather than deferred: the command hands over with syscall.Exec,
// which runs no deferred function, so this package must not leave the close
// to one either.
Expand All @@ -302,7 +315,7 @@ func Load(paths ...string) (Result, error) {
// give has already been read, so refusing to run the command over it would
// withhold a working environment for a problem that no longer affects it.
var notes []Note
if cErr := file.Close(); cErr != nil {
if cErr := rc.Close(); cErr != nil {
notes = append(notes, CloseError{Err: cErr})
}
// Path and Notes survive a failure, and only Env does not: a file that was
Expand Down
79 changes: 79 additions & 0 deletions env/load_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// This file is in package env, where every other test file is in env_test,
// because the property it covers is unreachable from outside: Load opens the
// file itself, read-only, and a descriptor with no write-back has nothing left
// to fail at on Close. loadFile takes a reader precisely so a fake can fail.
package env

import (
"errors"
"io"
"io/fs"
"strings"
"testing"
)

// failingCloser is an opened environment file whose Close reports err.
type failingCloser struct {
io.Reader
err error
}

func (f failingCloser) Close() error { return f.err }

// TestLoadFileCloseError covers what a failed close must not cost the caller:
// everything the file had to give has already been read, so the close failure
// travels as a Note beside the result rather than replacing it.
func TestLoadFileCloseError(t *testing.T) {
const path = "x.env"
cause := &fs.PathError{Op: "close", Path: path, Err: fs.ErrPermission}

t.Run("a usable file", func(t *testing.T) {
rc := failingCloser{Reader: strings.NewReader("NAME=value\n"), err: cause}

res, err := loadFile(rc, path)
if err != nil {
t.Fatalf("error = %v, expected none: a close failure must not withhold a working environment", err)
}
if res.Env["NAME"] != "value" {
t.Errorf("Env = %v, expected it to hold NAME=value", res.Env)
}
assertCloseNote(t, res, path, cause)
})

t.Run("a rejected file", func(t *testing.T) {
rc := failingCloser{Reader: strings.NewReader("9LEADING=x\n"), err: cause}

res, err := loadFile(rc, path)
perr, ok := errors.AsType[*ParseError](err)
if !ok {
t.Fatalf("error %v is not a *ParseError, so a caller cannot act on it", err)
}
if perr.Path != path {
t.Errorf("ParseError.Path = %q, expected %q", perr.Path, path)
}
if res.Env != nil {
t.Errorf("expected no environment beside an error, got %v", res.Env)
}
// The property this case exists for: the parse failure does not bury the
// unrelated finding the caller could still act on.
assertCloseNote(t, res, path, cause)
})
}

func assertCloseNote(t *testing.T, res Result, path string, cause error) {
t.Helper()

if res.Path != path {
t.Errorf("Result.Path = %q, expected %q", res.Path, path)
}
if len(res.Notes) != 1 {
t.Fatalf("Notes = %v, expected exactly the close failure", res.Notes)
}
note, ok := res.Notes[0].(CloseError)
if !ok {
t.Fatalf("note %v is not a CloseError, so its cause is unreachable", res.Notes[0])
}
if !errors.Is(note, cause) {
t.Errorf("errors.Is(note, cause) is false for %v", note)
}
}
51 changes: 51 additions & 0 deletions renovate-config-example.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
{
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"extends": ["config:recommended"],
"repositories": [
"fgm/envrun",
"fgm/untilMongod",
"fgm/container",
"fgm/drupal_redis_stats",
"fgm/izidic",
"fgm/pflagheaders"
],
"enabledManagers": ["gomod", "github-actions", "npm"],
"packageRules": [
{
"matchManagers": ["gomod"],
"groupName": "Go dependencies",
"groupSlug": "go-deps",
"updateTypes": ["minor", "patch"]
},
{
"matchManagers": ["github-actions"],
"groupName": "GitHub Actions",
"groupSlug": "github-actions"
},
{
"matchManagers": ["npm"],
"matchPaths": ["examples/**"],
"groupName": "npm dependencies",
"groupSlug": "npm-deps"
}
],
"repositoryRules": [
{
"matchRepositoryNames": ["fgm/pflagheaders"],
"schedule": ["before 3am on Monday"]
},
{
"matchRepositoryNames": ["fgm/envrun", "fgm/container"],
"schedule": ["before 3am on the first Saturday"]
},
{
"matchRepositoryNames": ["fgm/untilMongod"],
"schedule": ["before 3am on the 1st of the month"]
},
{
"matchRepositoryNames": ["fgm/drupal_redis_stats", "fgm/izidic"],
"schedule": ["before 3am on Monday"],
"enabledManagers": ["gomod"]
}
]
}
6 changes: 6 additions & 0 deletions renovate.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"extends": [
"local>fgm/renovate-config"
]
}