Skip to content

Commit 00d07eb

Browse files
committed
fix(lint-requirements): allow duplicates in requirements.txt
The lint-requirements command was incorrectly enforcing uniqueness rules on requirements.txt files. The uniqueness constraint (one entry per package name and marker combination) should only apply to constraints.txt files, not requirements.txt files. This was preventing users from specifying multiple versions of the same package in requirements.txt files, which is needed to support building multiple versions of a package. Changes: - Move uniqueness validation inside the is_constraints check so it only applies to files ending with constraints.txt - requirements.txt files can now contain duplicate package names with different versions or markers - Add comprehensive test coverage for both requirements.txt and constraints.txt validation scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Fixes #874 Signed-off-by: Rohan Devasthale <rdevasth@redhat.com> # Update the e2e test to expect duplicates in requirements.txt files to # pass validation instead of fail, aligning with the fix that allows # duplicates in requirements.txt. # Changes: # - Invert test expectation at lines 39-44 to expect success when # duplicates are present in requirements.txt # - Update duplicate-requirements.txt to use stevedore==5.3.0 instead # of the non-existent 5.2.1 version # - Fix trailing whitespace and add newline at end of file
1 parent a8d19ce commit 00d07eb

4 files changed

Lines changed: 186 additions & 10 deletions

File tree

e2e/test_lint_requirements.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ if fromager lint-requirements --resolve-requirements "$SCRIPTDIR/validate_inputs
3636
pass=false
3737
fi;
3838

39-
# Test to demonstrate that command reports error for duplicate entries in files
39+
# Test to demonstrate that command accepts duplicate entries in requirements files
4040

41-
if fromager lint-requirements --resolve-requirements "$SCRIPTDIR/validate_inputs/constraints.txt" "$SCRIPTDIR/validate_inputs/duplicate-requirements.txt"; then
42-
echo "duplicate entries in files should have been recognized by the command" 1>&2
41+
if ! fromager lint-requirements --resolve-requirements "$SCRIPTDIR/validate_inputs/constraints.txt" "$SCRIPTDIR/validate_inputs/duplicate-requirements.txt"; then
42+
echo "duplicate entries in requirements files should have been recognized as valid" 1>&2
4343
pass=false
4444
fi;
4545

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
# This requirements file contains duplicates entries on purpose
1+
# This requirements file contains duplicates entries on purpose
22
# to test the lint-requirements. Do not attempt to fix this file
33
stevedore==5.2.0
44
kfp==2.11.0
5-
stevedore==5.2.1
5+
stevedore==5.3.0

src/fromager/commands/lint_requirements.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,12 @@ def lint_requirements(
6565
marker_key = str(requirement.marker) if requirement.marker else ""
6666
unique_key = (requirement.name, marker_key)
6767

68-
if unique_key in unique_entries:
69-
raise InvalidRequirement(
70-
f"Duplicate entry, first found: {unique_entries[unique_key]}"
71-
)
72-
unique_entries[unique_key] = requirement
7368
if is_constraints:
69+
if unique_key in unique_entries:
70+
raise InvalidRequirement(
71+
f"Duplicate entry, first found: {unique_entries[unique_key]}"
72+
)
73+
unique_entries[unique_key] = requirement
7474
if requirement.extras:
7575
raise InvalidRequirement(
7676
f"{requirement.name}: Constraints files cannot contain extra dependencies"

tests/test_lint_requirements.py

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import pathlib
2+
3+
from click.testing import CliRunner
4+
5+
from fromager.__main__ import main as fromager
6+
7+
8+
def test_requirements_allows_duplicates(
9+
tmp_path: pathlib.Path, cli_runner: CliRunner
10+
) -> None:
11+
"""Test that requirements.txt files allow duplicate package names with different versions."""
12+
requirements_file = tmp_path / "requirements.txt"
13+
requirements_file.write_text("requests==2.28.0\nrequests==2.29.0\nnumpy==1.24.0\n")
14+
15+
result = cli_runner.invoke(
16+
fromager,
17+
["lint-requirements", str(requirements_file)],
18+
)
19+
20+
assert result.exit_code == 0, result.stdout
21+
assert "Successfully validated 1 file(s)" in result.stdout
22+
23+
24+
def test_requirements_allows_duplicates_with_markers(
25+
tmp_path: pathlib.Path, cli_runner: CliRunner
26+
) -> None:
27+
"""Test that requirements.txt files allow duplicate package names with different markers."""
28+
requirements_file = tmp_path / "requirements.txt"
29+
requirements_file.write_text(
30+
'requests==2.28.0; python_version < "3.10"\n'
31+
'requests==2.29.0; python_version >= "3.10"\n'
32+
)
33+
34+
result = cli_runner.invoke(
35+
fromager,
36+
["lint-requirements", str(requirements_file)],
37+
)
38+
39+
assert result.exit_code == 0, result.stdout
40+
assert "Successfully validated 1 file(s)" in result.stdout
41+
42+
43+
def test_requirements_allows_same_package_multiple_times(
44+
tmp_path: pathlib.Path, cli_runner: CliRunner
45+
) -> None:
46+
"""Test that requirements.txt files allow the same package multiple times (for multi-version builds)."""
47+
requirements_file = tmp_path / "requirements.txt"
48+
requirements_file.write_text("numpy==1.24.0\nnumpy==1.25.0\nnumpy==1.26.0\n")
49+
50+
result = cli_runner.invoke(
51+
fromager,
52+
["lint-requirements", str(requirements_file)],
53+
)
54+
55+
assert result.exit_code == 0, result.stdout
56+
assert "Successfully validated 1 file(s)" in result.stdout
57+
58+
59+
def test_constraints_rejects_duplicates(
60+
tmp_path: pathlib.Path, cli_runner: CliRunner
61+
) -> None:
62+
"""Test that constraints.txt files reject duplicate package names."""
63+
constraints_file = tmp_path / "constraints.txt"
64+
constraints_file.write_text("requests==2.28.0\nrequests==2.29.0\n")
65+
66+
result = cli_runner.invoke(
67+
fromager,
68+
["lint-requirements", str(constraints_file)],
69+
)
70+
71+
assert result.exit_code == 1
72+
assert "Duplicate entry" in result.output
73+
assert "requests==2.28.0" in result.output
74+
75+
76+
def test_constraints_rejects_duplicates_with_same_marker(
77+
tmp_path: pathlib.Path, cli_runner: CliRunner
78+
) -> None:
79+
"""Test that constraints.txt files reject duplicate package names with the same marker."""
80+
constraints_file = tmp_path / "constraints.txt"
81+
constraints_file.write_text(
82+
'requests==2.28.0; python_version < "3.10"\n'
83+
'requests==2.29.0; python_version < "3.10"\n'
84+
)
85+
86+
result = cli_runner.invoke(
87+
fromager,
88+
["lint-requirements", str(constraints_file)],
89+
)
90+
91+
assert result.exit_code == 1
92+
assert "Duplicate entry" in result.output
93+
94+
95+
def test_constraints_allows_different_markers(
96+
tmp_path: pathlib.Path, cli_runner: CliRunner
97+
) -> None:
98+
"""Test that constraints.txt files allow the same package with different markers."""
99+
constraints_file = tmp_path / "constraints.txt"
100+
constraints_file.write_text(
101+
'requests==2.28.0; python_version < "3.10"\n'
102+
'requests==2.29.0; python_version >= "3.10"\n'
103+
)
104+
105+
result = cli_runner.invoke(
106+
fromager,
107+
["lint-requirements", str(constraints_file)],
108+
)
109+
110+
assert result.exit_code == 0, result.stdout
111+
assert "Successfully validated 1 file(s)" in result.stdout
112+
113+
114+
def test_global_constraints_enforces_uniqueness(
115+
tmp_path: pathlib.Path, cli_runner: CliRunner
116+
) -> None:
117+
"""Test that files ending with 'constraints.txt' (like global-constraints.txt) enforce uniqueness."""
118+
constraints_file = tmp_path / "global-constraints.txt"
119+
constraints_file.write_text("numpy==1.24.0\nnumpy==1.25.0\n")
120+
121+
result = cli_runner.invoke(
122+
fromager,
123+
["lint-requirements", str(constraints_file)],
124+
)
125+
126+
assert result.exit_code == 1
127+
assert "Duplicate entry" in result.output
128+
129+
130+
def test_mixed_files_validation(tmp_path: pathlib.Path, cli_runner: CliRunner) -> None:
131+
"""Test validating both requirements.txt and constraints.txt files together."""
132+
requirements_file = tmp_path / "requirements.txt"
133+
requirements_file.write_text("requests==2.28.0\nrequests==2.29.0\n")
134+
135+
constraints_file = tmp_path / "constraints.txt"
136+
constraints_file.write_text("numpy==1.24.0\n")
137+
138+
result = cli_runner.invoke(
139+
fromager,
140+
["lint-requirements", str(requirements_file), str(constraints_file)],
141+
)
142+
143+
assert result.exit_code == 0, result.stdout
144+
assert "Successfully validated 2 file(s)" in result.stdout
145+
146+
147+
def test_constraints_rejects_extras(
148+
tmp_path: pathlib.Path, cli_runner: CliRunner
149+
) -> None:
150+
"""Test that constraints.txt files reject packages with extras."""
151+
constraints_file = tmp_path / "constraints.txt"
152+
constraints_file.write_text("requests[security]==2.28.0\n")
153+
154+
result = cli_runner.invoke(
155+
fromager,
156+
["lint-requirements", str(constraints_file)],
157+
)
158+
159+
assert result.exit_code == 1
160+
assert "Constraints files cannot contain extra dependencies" in result.output
161+
162+
163+
def test_constraints_requires_version_specifier(
164+
tmp_path: pathlib.Path, cli_runner: CliRunner
165+
) -> None:
166+
"""Test that constraints.txt files require version specifiers."""
167+
constraints_file = tmp_path / "constraints.txt"
168+
constraints_file.write_text("requests\n")
169+
170+
result = cli_runner.invoke(
171+
fromager,
172+
["lint-requirements", str(constraints_file)],
173+
)
174+
175+
assert result.exit_code == 1
176+
assert "Constraints must have a version specifier" in result.output

0 commit comments

Comments
 (0)