reqtxt: record entry provenance, and close three pip-parity gaps

CHANGELOG.md

@@ -73,8 +73,43 @@ mistaken for a safe patch upgrade.
   `amd64` where linux uses `x86_64`, and macOS uses `arm64` where linux uses
   `aarch64`.
 
+- `reqtxt`: every `Entry` and `OptionEntry` carries a `Source` with the file and
+  line it came from, plus `SourceOf` and a `WithPath` parse option.
+
+  `Flatten` splices every include level into one flat `File` and consumes each
+  `IncludeEntry`, so afterwards nothing remained to attribute an entry to a file,
+  and no entry carried a line number at all. That matters most for whole-file
+  options, since `Pre()` and `IndexURL()` are any-wins across the flattened
+  result: a single `--pre` nested three includes deep, even inside a `-c`
+  subtree, silently changes what a caller collects. "Enabled by line 2 of
+  extra.txt" is a materially better diagnostic than "enabled", and on a machine
+  with no network it is the difference between diagnosable and not.
+
+  `Flatten` sets the path per included file automatically. `Parse` leaves it
+  empty unless `WithPath` is given, since it takes content rather than a
+  filename.
+
 ### Fixed
 
+- `reqtxt`: `--all-releases`, `--only-final` and `--use-feature` are recognized.
+  All three are in pip's `SUPPORTED_OPTIONS`, and the consequence of their
+  absence was worse than losing normalization: an unrecognized option is assumed
+  boolean, so its argument was dispatched as its own line and became a
+  **fabricated requirement**. `--use-feature 2020-resolver` produced a package
+  named `2020-resolver` from a valid pip file.
+
+  `--all-releases` and `--only-final` are pip's per-package replacements for
+  `--pre`, which pip refuses to combine with them.
+
+- `reqtxt`: a standalone `--hash` line is no longer an error. pip logs *"line %s
+  has --hash but no requirement, and will be ignored"* and continues, so the file
+  installs fine and rejecting it made this package stricter than pip. It is
+  surfaced as a file-level option rather than dropped, so a caller can reproduce
+  pip's warning.
+
+- `reqtxt`: `Flatten` with a nil `open` callback returns an error instead of
+  panicking.
+
 - `tags`: `riscv64` and `loongarch64` targets now claim the same manylinux
   series as every other non-x86 architecture — floored at glibc 2.17, with the
   `manylinux2014_<arch>` legacy alias — matching pypa/packaging.

reqtxt/flatten.go

@@ -41,6 +41,13 @@ var schemeRE = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`)
 // recursive Parse call, so e.g. WithEnv expansion is applied consistently
 // at every level of the include tree.
 func Flatten(root string, open func(path string) ([]byte, error), opts ...ParseOption) (*File, error) {
+	// open is mandatory and is called for the root before anything else, so a nil
+	// callback would panic on the first dereference. In a CLI a stack trace is a
+	// materially worse failure than an error string, and the caller cannot tell
+	// them apart from the outside.
+	if open == nil {
+		return nil, errors.Join(ErrInvalidRequirementsFile, errors.New("Flatten requires a non-nil open callback"))
+	}
 	entries, err := flattenWalk(root, false, map[string]bool{}, open, opts)
 	if err != nil {
 		return nil, err
@@ -70,7 +77,10 @@ func flattenWalk(p string, constraintCtx bool, visited map[string]bool, open fun
 
 	content := decodeUTF8(raw)
 
-	file, err := Parse(content, opts...)
+	// WithPath is appended rather than prepended so a caller's own WithPath
+	// cannot silently attribute every included file to one path. Each level
+	// records the file actually opened.
+	file, err := Parse(content, append(append([]ParseOption(nil), opts...), WithPath(p))...)
 	if err != nil {
 		return nil, err
 	}

reqtxt/parse.go

@@ -36,6 +36,15 @@ var knownOptions = map[string]knownOption{
 	"--no-binary":       {optNoBinary, 1},
 	"--only-binary":     {optOnlyBinary, 1},
 	"--prefer-binary":   {optPreferBinary, 0},
+	// These three are in pip's SUPPORTED_OPTIONS and were missing here, which
+	// mattered more than "not normalized": an unknown bare option is assumed
+	// boolean, so its argument was dispatched as a separate line and became a
+	// FABRICATED requirement. "--use-feature 2020-resolver" yielded a package
+	// named "2020-resolver" from a perfectly valid pip file. All three take a
+	// value (pip: type="str").
+	"--all-releases": {optAllReleases, 1},
+	"--only-final":   {optOnlyFinal, 1},
+	"--use-feature":  {optUseFeature, 1},
 }
 
 // perLineValueOptions are the per-requirement options (other than
@@ -71,12 +80,42 @@ func Parse(content string, opts ...ParseOption) (*File, error) {
 		if err != nil {
 			return nil, err
 		}
+		// Stamped here rather than at each construction site: every entry from a
+		// logical line shares that line, including the per-requirement options
+		// gathered from its continuations, and one pass cannot miss a site.
+		src := Source{Path: cfg.path, Line: ll.line}
+		for _, e := range entries {
+			stampSource(e, src)
+		}
 		file.Entries = append(file.Entries, entries...)
 	}
 
 	return file, nil
 }
 
+// stampSource records src on e and on any per-requirement options attached to
+// it. A continuation line's options report the line the logical line STARTED on,
+// which is the same anchor reqtxt already uses for errors raised on a joined
+// line.
+func stampSource(e Entry, src Source) {
+	switch n := e.(type) {
+	case *RequirementEntry:
+		n.Source = src
+		for i := range n.Options {
+			n.Options[i].Source = src
+		}
+	case *IncludeEntry:
+		n.Source = src
+	case *UnnamedEntry:
+		n.Source = src
+		for i := range n.Options {
+			n.Options[i].Source = src
+		}
+	case *OptionEntry:
+		n.Source = src
+	}
+}
+
 // dispatchLogicalLine routes one preprocessed logical line to the flag
 // path or the package path, deciding from the line's first
 // whitespace-delimited raw token - before any shlex tokenization happens.
@@ -198,7 +237,28 @@ func dispatchEditable(hasEq bool, eqValue string, rest []string, lineNum int) ([
 // only ever a per-requirement option (see attachReqOptions).
 func dispatchFileOption(name string, hasEq bool, eqValue string, rest []string, lineNum int) ([]Entry, error) {
 	if name == "--hash" {
-		return nil, lineError(lineNum, "--hash has no associated requirement")
+		// pip does NOT reject this. req_file.py logs "line %s has --hash but no
+		// requirement, and will be ignored" and carries on, so a file containing
+		// one installs fine and rejecting it made this package stricter than the
+		// thing it models.
+		//
+		// Emitted as a file-level OptionEntry rather than dropped, so the
+		// information survives for a caller that wants to reproduce pip's warning.
+		// There is nothing to attach it to -- "--hash" is only ever a
+		// per-requirement option (see attachReqOptions) -- and File's accessors
+		// ignore names they do not know, so this cannot be mistaken for a
+		// requirement's hash.
+		value, remaining, err := takeValue(name, hasEq, eqValue, rest, lineNum)
+		if err != nil {
+			// A bare "--hash" with no value at all: still not fatal for pip, which
+			// never sees a value it can use either way.
+			value, remaining = eqValue, rest
+		}
+		following, err := dispatchLine(remaining, lineNum)
+		if err != nil {
+			return nil, err
+		}
+		return append([]Entry{&OptionEntry{Name: name, Value: value}}, following...), nil
 	}
 
 	var (
@@ -228,6 +288,14 @@ func dispatchFileOption(name string, hasEq bool, eqValue string, rest []string,
 		// Unknown flag: "--opt=value" takes the value; a bare "--opt" is
 		// boolean and never consumes the next token — it is left to be
 		// dispatched as its own, separate entry.
+		//
+		// That assumption is a guess, and it is the RIGHT guess for a boolean
+		// followed by a requirement ("--frob foo" yields the option and foo). It is
+		// the wrong guess for an unknown arity-1 option, where the argument becomes
+		// a fabricated requirement ("--timeout 60" yields a package named "60").
+		// Both readings are guesses and neither is safe, so the fix is not to
+		// change the guess but to leave fewer options unknown: see knownOptions,
+		// which is audited against pip's SUPPORTED_OPTIONS.
 		value, remaining = eqValue, rest
 	}
 

reqtxt/parse_test.go

@@ -94,10 +94,10 @@ func TestParse_Include(t *testing.T) {
 		content string
 		want    IncludeEntry
 	}{
-		{name: "-r space form", content: "-r base.txt", want: IncludeEntry{Path: "base.txt"}},
-		{name: "-c space form is a constraint", content: "-c c.txt", want: IncludeEntry{Path: "c.txt", Constraint: true}},
-		{name: "--requirement= form", content: "--requirement=x.txt", want: IncludeEntry{Path: "x.txt"}},
-		{name: "--constraint= form is a constraint", content: "--constraint=x.txt", want: IncludeEntry{Path: "x.txt", Constraint: true}},
+		{name: "-r space form", content: "-r base.txt", want: IncludeEntry{Path: "base.txt", Source: Source{Line: 1}}},
+		{name: "-c space form is a constraint", content: "-c c.txt", want: IncludeEntry{Path: "c.txt", Constraint: true, Source: Source{Line: 1}}},
+		{name: "--requirement= form", content: "--requirement=x.txt", want: IncludeEntry{Path: "x.txt", Source: Source{Line: 1}}},
+		{name: "--constraint= form is a constraint", content: "--constraint=x.txt", want: IncludeEntry{Path: "x.txt", Constraint: true, Source: Source{Line: 1}}},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -144,27 +144,27 @@ func TestParse_FileOptions(t *testing.T) {
 		{
 			name:    "--index-url space form",
 			content: "--index-url https://i/simple",
-			want:    []OptionEntry{{Name: "--index-url", Value: "https://i/simple"}},
+			want:    []OptionEntry{{Name: "--index-url", Value: "https://i/simple", Source: Source{Line: 1}}},
 		},
 		{
 			name:    "-i short form normalizes to --index-url",
 			content: "-i https://i/simple",
-			want:    []OptionEntry{{Name: "--index-url", Value: "https://i/simple"}},
+			want:    []OptionEntry{{Name: "--index-url", Value: "https://i/simple", Source: Source{Line: 1}}},
 		},
 		{
 			name:    "--index-url= form",
 			content: "--index-url=https://i",
-			want:    []OptionEntry{{Name: "--index-url", Value: "https://i"}},
+			want:    []OptionEntry{{Name: "--index-url", Value: "https://i", Source: Source{Line: 1}}},
 		},
 		{
 			name:    "--no-index is boolean",
 			content: "--no-index",
-			want:    []OptionEntry{{Name: "--no-index", Value: ""}},
+			want:    []OptionEntry{{Name: "--no-index", Value: "", Source: Source{Line: 1}}},
 		},
 		{
 			name:    "--find-links repeated across lines",
 			content: "--find-links a\n--find-links b",
-			want:    []OptionEntry{{Name: "--find-links", Value: "a"}, {Name: "--find-links", Value: "b"}},
+			want:    []OptionEntry{{Name: "--find-links", Value: "a", Source: Source{Line: 1}}, {Name: "--find-links", Value: "b", Source: Source{Line: 2}}},
 		},
 	}
 	for _, c := range cases {
@@ -189,15 +189,15 @@ func TestParse_UnknownFlag(t *testing.T) {
 
 	oe, ok := f.Entries[0].(*OptionEntry)
 	require.True(t, ok, "want *OptionEntry, got %T", f.Entries[0])
-	assert.Equal(t, OptionEntry{Name: "--frobnicate", Value: "1"}, *oe)
+	assert.Equal(t, OptionEntry{Name: "--frobnicate", Value: "1", Source: Source{Line: 1}}, *oe)
 
 	f, err = Parse("--frob foo")
 	require.NoError(t, err)
 	require.Len(t, f.Entries, 2)
 
 	oe, ok = f.Entries[0].(*OptionEntry)
 	require.True(t, ok, "entry 0: want *OptionEntry, got %T", f.Entries[0])
-	assert.Equal(t, OptionEntry{Name: "--frob", Value: ""}, *oe)
+	assert.Equal(t, OptionEntry{Name: "--frob", Value: "", Source: Source{Line: 1}}, *oe)
 
 	re, ok := f.Entries[1].(*RequirementEntry)
 	require.True(t, ok, "entry 1: want *RequirementEntry, got %T", f.Entries[1])
@@ -219,9 +219,13 @@ func TestParse_Hashes(t *testing.T) {
 	require.Error(t, err)
 	assert.ErrorIs(t, err, ErrInvalidRequirementsFile)
 
-	_, err = Parse("--hash=sha256:x")
-	require.Error(t, err)
-	assert.ErrorIs(t, err, ErrInvalidRequirementsFile)
+	// A standalone --hash is NOT an error: pip logs "line %s has --hash but no
+	// requirement, and will be ignored" and carries on, so a file containing one
+	// installs fine. It surfaces as a file-level option instead; see
+	// TestStandaloneHash_IsNotAnError.
+	f, err = Parse("--hash=sha256:x")
+	require.NoError(t, err)
+	assert.Empty(t, f.Requirements())
 }
 
 func TestParse_ConfigSettings(t *testing.T) {
@@ -232,7 +236,7 @@ func TestParse_ConfigSettings(t *testing.T) {
 	re, ok := f.Entries[0].(*RequirementEntry)
 	require.True(t, ok, "want *RequirementEntry, got %T", f.Entries[0])
 	require.Len(t, re.Options, 1)
-	assert.Equal(t, OptionEntry{Name: "--config-settings", Value: "x=y"}, re.Options[0])
+	assert.Equal(t, OptionEntry{Name: "--config-settings", Value: "x=y", Source: Source{Line: 1}}, re.Options[0])
 }
 
 func TestParse_BareArchiveIsUnnamed(t *testing.T) {

reqtxt/pipparity_test.go

@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+package reqtxt
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestKnownOptions_NoFabricatedRequirements is the bug these table entries fix.
+// An option missing from knownOptions is assumed boolean, so its argument is
+// dispatched as its own line and becomes a requirement. Every one of these is a
+// real pip SUPPORTED_OPTIONS entry, so this fired on valid files -- and a
+// consumer that resolves the closure of every requirement then hunts for a
+// package that does not exist.
+func TestKnownOptions_NoFabricatedRequirements(t *testing.T) {
+	for _, tc := range []struct {
+		content   string
+		wantName  string
+		wantValue string
+	}{
+		{"--use-feature 2020-resolver", optUseFeature, "2020-resolver"},
+		{"--use-feature=2020-resolver", optUseFeature, "2020-resolver"},
+		{"--all-releases :all:", optAllReleases, ":all:"},
+		{"--all-releases mypkg,other", optAllReleases, "mypkg,other"},
+		{"--only-final :all:", optOnlyFinal, ":all:"},
+		{"--only-final=mypkg", optOnlyFinal, "mypkg"},
+	} {
+		t.Run(tc.content, func(t *testing.T) {
+			f, err := Parse(tc.content)
+			require.NoError(t, err)
+
+			require.Len(t, f.Entries, 1, "must not also produce a requirement")
+			oe, ok := f.Entries[0].(*OptionEntry)
+			require.True(t, ok, "want *OptionEntry, got %T", f.Entries[0])
+			assert.Equal(t, tc.wantName, oe.Name)
+			assert.Equal(t, tc.wantValue, oe.Value)
+
+			assert.Empty(t, f.Requirements(), "no requirement may be fabricated")
+		})
+	}
+}
+
+// TestKnownOptions_CoversPipSupportedOptions guards against the same class of gap
+// reappearing. Every long option in pip's SUPPORTED_OPTIONS must be handled
+// here, either in knownOptions or by a dedicated dispatch path.
+func TestKnownOptions_CoversPipSupportedOptions(t *testing.T) {
+	// pip's SUPPORTED_OPTIONS, by long spelling (pip 26.1, req_file.py).
+	pipSupported := []string{
+		"--index-url", "--extra-index-url", "--no-index",
+		"--constraint", "--requirement", "--editable",
+		"--find-links", "--no-binary", "--only-binary", "--prefer-binary",
+		"--require-hashes", "--pre", "--all-releases", "--only-final",
+		"--trusted-host", "--use-feature",
+	}
+	// These have their own dispatch and never reach knownOptions.
+	dispatchedElsewhere := map[string]bool{
+		"--constraint": true, "--requirement": true, "--editable": true,
+	}
+
+	for _, opt := range pipSupported {
+		if dispatchedElsewhere[opt] {
+			continue
+		}
+		_, known := knownOptions[opt]
+		assert.True(t, known, "%s is in pip's SUPPORTED_OPTIONS but not knownOptions", opt)
+	}
+}
+
+// TestStandaloneHash_IsNotAnError pins parity with pip, which logs "line %s has
+// --hash but no requirement, and will be ignored" (req_file.py) and carries on.
+// Rejecting the file made this package stricter than the thing it models.
+func TestStandaloneHash_IsNotAnError(t *testing.T) {
+	for _, content := range []string{
+		"--hash=sha256:abc",
+		"--hash sha256:abc",
+		"requests==2.0\n--hash=sha256:abc",
+	} {
+		t.Run(content, func(t *testing.T) {
+			f, err := Parse(content)
+			require.NoError(t, err, "pip accepts this file")
+
+			var found bool
+			for _, o := range f.Options() {
+				if o.Name == "--hash" {
+					found = true
+				}
+			}
+			assert.True(t, found,
+				"the line is surfaced rather than dropped, so a caller can warn as pip does")
+		})
+	}
+}
+
+// TestStandaloneHash_DoesNotAttachToAPrecedingRequirement guards the thing that
+// made this an error in the first place: a file-level --hash must not be mistaken
+// for a hash belonging to an earlier requirement.
+func TestStandaloneHash_DoesNotAttachToAPrecedingRequirement(t *testing.T) {
+	f, err := Parse("requests==2.0\n--hash=sha256:abc")
+	require.NoError(t, err)
+
+	reqs := f.Requirements()
+	require.Len(t, reqs, 1)
+	assert.Empty(t, reqs[0].Hashes, "the standalone --hash belongs to no requirement")
+}
+
+// TestFlatten_NilOpenIsAnErrorNotAPanic covers the guard: open is called for the
+// root before anything else, so a nil callback used to panic on first deref. In a
+// CLI a stack trace is a materially worse failure than an error string.
+func TestFlatten_NilOpenIsAnErrorNotAPanic(t *testing.T) {
+	require.NotPanics(t, func() {
+		_, err := Flatten("root.txt", nil)
+		assert.ErrorIs(t, err, ErrInvalidRequirementsFile)
+	})
+}
+
+// TestTrailingSemicolonStillRejected records a NON-change. A review claimed pip
+// accepts "requests;" where this package rejects it. Checked against packaging
+// 26.3: Requirement("requests;") raises InvalidRequirement ("Expected a marker
+// variable or quoted string"), so the rejection is faithful and stays.
+func TestTrailingSemicolonStillRejected(t *testing.T) {
+	for _, content := range []string{"requests;", "requests ;"} {
+		t.Run(content, func(t *testing.T) {
+			_, err := Parse(content)
+			assert.ErrorIs(t, err, ErrInvalidRequirementsFile,
+				"packaging raises InvalidRequirement here too")
+		})
+	}
+}