Repository: compose-spec/compose-go
Pull request #342 (2023-01-12 08:36:28)

provide diagnostic message as we fail to parse some env file
tweak original godotenv library so that we track line number and can offer relevant error message.
Also include file path we failed to parse on error


Patch:
diff --git a/Makefile b/Makefile
index 1a66ee2..e25e8f4 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,3 @@ check-license: build-validate-image
 setup: ## Setup the precommit hook
 	@which pre-commit > /dev/null 2>&1 || (echo "pre-commit not installed see README." && false)
 	@pre-commit install
-
-.PHONY: sync
-sync:
-	curl -fSL https://raw.githubusercontent.com/compose-spec/compose-spec/master/schema/compose-spec.json -o schema/compose-spec.json
\ No newline at end of file
diff --git a/cli/options.go b/cli/options.go
index 304993b..567ac5a 100644
--- a/cli/options.go
+++ b/cli/options.go
@@ -238,7 +238,7 @@ func GetEnvFromFile(currentEnv map[string]string, workingDir string, filename st
 
 	file, err := os.Open(dotEnvFile)
 	if err != nil {
-		return envMap, err
+		return envMap, errors.Wrapf(err, "failed to read %s", dotEnvFile)
 	}
 	defer file.Close()
 
@@ -250,7 +250,7 @@ func GetEnvFromFile(currentEnv map[string]string, workingDir string, filename st
 		return v, true
 	})
 	if err != nil {
-		return envMap, err
+		return envMap, errors.Wrapf(err, "failed to read %s", dotEnvFile)
 	}
 	for k, v := range env {
 		envMap[k] = v
diff --git a/dotenv/fixtures/invalid1.env b/dotenv/fixtures/invalid1.env
index 38f7e0e..18ed705 100644
--- a/dotenv/fixtures/invalid1.env
+++ b/dotenv/fixtures/invalid1.env
@@ -1,2 +1,8 @@
+# some comments
+foo="
+  a
+  multine
+  value
+"
 INVALID LINE
-foo=bar
+zot=qix
\ No newline at end of file
diff --git a/dotenv/godotenv.go b/dotenv/godotenv.go
index c1c12ea..ae34ffa 100644
--- a/dotenv/godotenv.go
+++ b/dotenv/godotenv.go
@@ -15,13 +15,9 @@ package dotenv
 
 import (
 	"bytes"
-	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"regexp"
-	"sort"
-	"strconv"
 	"strings"
 
 	"github.com/compose-spec/compose-go/template"
@@ -72,21 +68,6 @@ func Load(filenames ...string) error {
 	return load(false, filenames...)
 }
 
-// Overload will read your env file(s) and load them into ENV for this process.
-//
-// Call this function as close as possible to the start of your program (ideally in main).
-//
-// If you call Overload without any args it will default to loading .env in the current path.
-//
-// You can otherwise tell it which files to load (there can be more than one) like:
-//
-//	godotenv.Overload("fileone", "filetwo")
-//
-// It's important to note this WILL OVERRIDE an env variable that already exists - consider the .env file to forcefilly set all vars.
-func Overload(filenames ...string) error {
-	return load(true, filenames...)
-}
-
 func load(overload bool, filenames ...string) error {
 	filenames = filenamesOrDefault(filenames)
 	for _, filename := range filenames {
@@ -128,82 +109,13 @@ func Read(filenames ...string) (map[string]string, error) {
 	return ReadWithLookup(nil, filenames...)
 }
 
-// Unmarshal reads an env file from a string, returning a map of keys and values.
-func Unmarshal(str string) (map[string]string, error) {
-	return UnmarshalBytes([]byte(str))
-}
-
-// UnmarshalBytes parses env file from byte slice of chars, returning a map of keys and values.
-func UnmarshalBytes(src []byte) (map[string]string, error) {
-	return UnmarshalBytesWithLookup(src, nil)
-}
-
 // UnmarshalBytesWithLookup parses env file from byte slice of chars, returning a map of keys and values.
 func UnmarshalBytesWithLookup(src []byte, lookupFn LookupFn) (map[string]string, error) {
 	out := make(map[string]string)
-	err := parseBytes(src, out, lookupFn)
+	err := newParser().parseBytes(src, out, lookupFn)
 	return out, err
 }
 
-// Exec loads env vars from the specified filenames (empty map falls back to default)
-// then executes the cmd specified.
-//
-// Simply hooks up os.Stdin/err/out to the command and calls Run()
-//
-// If you want more fine grained control over your command it's recommended
-// that you use `Load()` or `Read()` and the `os/exec` package yourself.
-//
-// Deprecated: Use the `os/exec` package directly.
-func Exec(filenames []string, cmd string, cmdArgs []string) error {
-	if err := Load(filenames...); err != nil {
-		return err
-	}
-
-	command := exec.Command(cmd, cmdArgs...)
-	command.Stdin = os.Stdin
-	command.Stdout = os.Stdout
-	command.Stderr = os.Stderr
-	return command.Run()
-}
-
-// Write serializes the given environment and writes it to a file
-//
-// Deprecated: The serialization functions are untested and unmaintained.
-func Write(envMap map[string]string, filename string) error {
-	//goland:noinspection GoDeprecation
-	content, err := Marshal(envMap)
-	if err != nil {
-		return err
-	}
-	file, err := os.Create(filename)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-	_, err = file.WriteString(content + "\n")
-	if err != nil {
-		return err
-	}
-	return file.Sync()
-}
-
-// Marshal outputs the given environment as a dotenv-formatted environment file.
-// Each line is in the format: KEY="VALUE" where VALUE is backslash-escaped.
-//
-// Deprecated: The serialization functions are untested and unmaintained.
-func Marshal(envMap map[string]string) (string, error) {
-	lines := make([]string, 0, len(envMap))
-	for k, v := range envMap {
-		if d, err := strconv.Atoi(v); err == nil {
-			lines = append(lines, fmt.Sprintf(`%s=%d`, k, d))
-		} else {
-			lines = append(lines, fmt.Sprintf(`%s="%s"`, k, doubleQuoteEscape(v))) // nolint // Cannot use %q here
-		}
-	}
-	sort.Strings(lines)
-	return strings.Join(lines, "\n"), nil
-}
-
 func filenamesOrDefault(filenames []string) []string {
 	if len(filenames) == 0 {
 		return []string{".env"}
@@ -255,19 +167,3 @@ func expandVariables(value string, envMap map[string]string, lookupFn LookupFn)
 	}
 	return retVal, nil
 }
-
-// Deprecated: only used by unsupported/untested code for Marshal/Write.
-func doubleQuoteEscape(line string) string {
-	const doubleQuoteSpecialChars = "\\\n\r\"!$`"
-	for _, c := range doubleQuoteSpecialChars {
-		toReplace := "\\" + string(c)
-		if c == '\n' {
-			toReplace = `\n`
-		}
-		if c == '\r' {
-			toReplace = `\r`
-		}
-		line = strings.ReplaceAll(line, string(c), toReplace)
-	}
-	return line
-}
diff --git a/dotenv/parser.go b/dotenv/parser.go
index 84347e3..e046619 100644
--- a/dotenv/parser.go
+++ b/dotenv/parser.go
@@ -21,24 +21,34 @@ var (
 	exportRegex    = regexp.MustCompile(`^export\s+`)
 )
 
-func parseBytes(src []byte, out map[string]string, lookupFn LookupFn) error {
+type parser struct {
+	line int
+}
+
+func newParser() *parser {
+	return &parser{
+		line: 1,
+	}
+}
+
+func (p *parser) parseBytes(src []byte, out map[string]string, lookupFn LookupFn) error {
 	cutset := src
 	if lookupFn == nil {
 		lookupFn = noLookupFn
 	}
 	for {
-		cutset = getStatementStart(cutset)
+		cutset = p.getStatementStart(cutset)
 		if cutset == nil {
 			// reached end of file
 			break
 		}
 
-		key, left, inherited, err := locateKeyName(cutset)
+		key, left, inherited, err := p.locateKeyName(cutset)
 		if err != nil {
 			return err
 		}
 		if strings.Contains(key, " ") {
-			return errors.New("key cannot contain a space")
+			return fmt.Errorf("line %d: key cannot contain a space", p.line)
 		}
 
 		if inherited {
@@ -50,7 +60,7 @@ func parseBytes(src []byte, out map[string]string, lookupFn LookupFn) error {
 			continue
 		}
 
-		value, left, err := extractVarValue(left, out, lookupFn)
+		value, left, err := p.extractVarValue(left, out, lookupFn)
 		if err != nil {
 			return err
 		}
@@ -65,8 +75,8 @@ func parseBytes(src []byte, out map[string]string, lookupFn LookupFn) error {
 // getStatementPosition returns position of statement begin.
 //
 // It skips any comment line or non-whitespace character.
-func getStatementStart(src []byte) []byte {
-	pos := indexOfNonSpaceChar(src)
+func (p *parser) getStatementStart(src []byte) []byte {
+	pos := p.indexOfNonSpaceChar(src)
 	if pos == -1 {
 		return nil
 	}
@@ -81,12 +91,11 @@ func getStatementStart(src []byte) []byte {
 	if pos == -1 {
 		return nil
 	}
-
-	return getStatementStart(src[pos:])
+	return p.getStatementStart(src[pos:])
 }
 
 // locateKeyName locates and parses key name and returns rest of slice
-func locateKeyName(src []byte) (string, []byte, bool, error) {
+func (p *parser) locateKeyName(src []byte) (string, []byte, bool, error) {
 	var key string
 	var inherited bool
 	// trim "export" and space at beginning
@@ -116,8 +125,8 @@ loop:
 			}
 
 			return "", nil, inherited, fmt.Errorf(
-				`unexpected character %q in variable name near %q`,
-				string(char), string(src))
+				`line %d: unexpected character %q in variable name`,
+				p.line, string(char))
 		}
 	}
 
@@ -132,11 +141,12 @@ loop:
 }
 
 // extractVarValue extracts variable value and returns rest of slice
-func extractVarValue(src []byte, envMap map[string]string, lookupFn LookupFn) (string, []byte, error) {
+func (p *parser) extractVarValue(src []byte, envMap map[string]string, lookupFn LookupFn) (string, []byte, error) {
 	quote, isQuoted := hasQuotePrefix(src)
 	if !isQuoted {
 		// unquoted value - read until new line
 		value, rest, _ := bytes.Cut(src, []byte("\n"))
+		p.line++
 
 		// Remove inline comments on unquoted lines
 		value, _, _ = bytes.Cut(value, []byte(" #"))
@@ -147,6 +157,9 @@ func extractVarValue(src []byte, envMap map[string]string, lookupFn LookupFn) (s
 
 	// lookup quoted string terminator
 	for i := 1; i < len(src); i++ {
+		if src[i] == '\n' {
+			p.line++
+		}
 		if char := src[i]; char != quote {
 			continue
 		}
@@ -177,7 +190,7 @@ func extractVarValue(src []byte, envMap map[string]string, lookupFn LookupFn) (s
 		valEndIndex = len(src)
 	}
 
-	return "", nil, fmt.Errorf("unterminated quoted value %s", src[:valEndIndex])
+	return "", nil, fmt.Errorf("line %d: unterminated quoted value %s", p.line, src[:valEndIndex])
 }
 
 func expandEscapes(str string) string {
@@ -212,8 +225,11 @@ func expandEscapes(str string) string {
 	return out
 }
 
-func indexOfNonSpaceChar(src []byte) int {
+func (p *parser) indexOfNonSpaceChar(src []byte) int {
 	return bytes.IndexFunc(src, func(r rune) bool {
+		if r == '\n' {
+			p.line++
+		}
 		return !unicode.IsSpace(r)
 	})
 }
diff --git a/loader/loader.go b/loader/loader.go
index 64b691b..6afa79f 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -657,7 +657,7 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, l
 
 			fileVars, err := dotenv.ParseWithLookup(bytes.NewBuffer(b), resolve)
 			if err != nil {
-				return err
+				return errors.Wrapf(err, "Failed to load %s", filePath)
 			}
 			env := types.MappingWithEquals{}
 			for k, v := range fileVars {
diff --git a/schema/compose-spec.json b/schema/compose-spec.json
index 6f82797..b7b90c3 100644
--- a/schema/compose-spec.json
+++ b/schema/compose-spec.json
@@ -141,7 +141,6 @@
         },
         "cap_add": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
         "cap_drop": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
-        "cgroup": {"type": "string", "enum": ["host", "private"]},
         "cgroup_parent": {"type": "string"},
         "command": {
           "oneOf": [
@@ -582,12 +581,12 @@
       "items": {
         "type": "object",
         "properties": {
-            "capabilities": {"$ref": "#/definitions/list_of_strings"},
-            "count": {"type": ["string", "integer"]},
-            "device_ids": {"$ref": "#/definitions/list_of_strings"},
-            "driver":{"type": "string"},
-            "options":{"$ref": "#/definitions/list_or_dict"}
-          },
+          "capabilities": {"$ref": "#/definitions/list_of_strings"},
+          "count": {"type": ["string", "integer"]},
+          "device_ids": {"$ref": "#/definitions/list_of_strings"},
+          "driver":{"type": "string"},
+          "options":{"$ref": "#/definitions/list_or_dict"}
+        },
         "additionalProperties": false,
         "patternProperties": {"^x-": {}}
       }


Test patch:
diff --git a/dotenv/godotenv_test.go b/dotenv/godotenv_test.go
index 163941d..cee78c6 100644
--- a/dotenv/godotenv_test.go
+++ b/dotenv/godotenv_test.go
@@ -55,14 +55,6 @@ func TestLoadWithNoArgsLoadsDotEnv(t *testing.T) {
 	}
 }
 
-func TestOverloadWithNoArgsOverloadsDotEnv(t *testing.T) {
-	err := Overload()
-	pathError := err.(*os.PathError)
-	if pathError == nil || pathError.Op != "open" || pathError.Path != ".env" {
-		t.Errorf("Didn't try and open .env by default")
-	}
-}
-
 func TestLoadFileNotFound(t *testing.T) {
 	err := Load("somefilethatwillneverexistever.env")
 	if err == nil {
@@ -70,13 +62,6 @@ func TestLoadFileNotFound(t *testing.T) {
 	}
 }
 
-func TestOverloadFileNotFound(t *testing.T) {
-	err := Overload("somefilethatwillneverexistever.env")
-	if err == nil {
-		t.Error("File wasn't found but Overload didn't return an error")
-	}
-}
-
 func TestReadPlainEnv(t *testing.T) {
 	envFileName := "fixtures/plain.env"
 	expectedValues := map[string]string{
@@ -139,20 +124,6 @@ func TestLoadDoesNotOverride(t *testing.T) {
 	loadEnvAndCompareValues(t, Load, envFileName, expectedValues, presets)
 }
 
-func TestOverloadDoesOverride(t *testing.T) {
-	envFileName := "fixtures/plain.env"
-
-	// ensure NO overload
-	presets := map[string]string{
-		"OPTION_A": "do_not_override",
-	}
-
-	expectedValues := map[string]string{
-		"OPTION_A": "1",
-	}
-	loadEnvAndCompareValues(t, Overload, envFileName, expectedValues, presets)
-}
-
 func TestLoadPlainEnv(t *testing.T) {
 	envFileName := "fixtures/plain.env"
 	expectedValues := map[string]string{
@@ -494,7 +465,7 @@ func TestLinesToIgnore(t *testing.T) {
 
 	for n, c := range cases {
 		t.Run(n, func(t *testing.T) {
-			got := string(getStatementStart([]byte(c.input)))
+			got := string(newParser().getStatementStart([]byte(c.input)))
 			if got != c.want {
 				t.Errorf("Expected:\t %q\nGot:\t %q", c.want, got)
 			}
@@ -513,10 +484,8 @@ func TestErrorReadDirectory(t *testing.T) {
 
 func TestErrorParsing(t *testing.T) {
 	envFileName := "fixtures/invalid1.env"
-	envMap, err := Read(envFileName)
-	if err == nil {
-		t.Errorf("Expected error, got %v", envMap)
-	}
+	_, err := Read(envFileName)
+	assert.ErrorContains(t, err, "line 7: key cannot contain a space")
 }
 
 func TestInheritedEnvVariableSameSize(t *testing.T) {