Merge pull request #875 from rd4398/fix-lint-req

fix(lint-requirements): allow duplicates in requirements.txt

Author: mergify[bot]

e2e/test_lint_requirements.sh

@@ -36,10 +36,10 @@ if fromager lint-requirements --resolve-requirements "$SCRIPTDIR/validate_inputs
     pass=false
 fi;
 
-# Test to demonstrate that command reports error for duplicate entries in files
+# Test to demonstrate that command accepts duplicate entries in requirements files
 
-if fromager lint-requirements --resolve-requirements "$SCRIPTDIR/validate_inputs/constraints.txt" "$SCRIPTDIR/validate_inputs/duplicate-requirements.txt"; then
-    echo "duplicate entries in files should have been recognized by the command" 1>&2
+if ! fromager lint-requirements --resolve-requirements "$SCRIPTDIR/validate_inputs/constraints.txt" "$SCRIPTDIR/validate_inputs/duplicate-requirements.txt"; then
+    echo "duplicate entries in requirements files should have been recognized as valid" 1>&2
     pass=false
 fi;
 

e2e/validate_inputs/duplicate-requirements.txt

@@ -1,5 +1,5 @@
-# This requirements file contains duplicates entries on purpose 
+# This requirements file contains duplicates entries on purpose
 # to test the lint-requirements. Do not attempt to fix this file
 stevedore==5.2.0
 kfp==2.11.0
-stevedore==5.2.1
+stevedore==5.3.0

src/fromager/commands/lint_requirements.py

@@ -65,12 +65,12 @@ def lint_requirements(
                 marker_key = str(requirement.marker) if requirement.marker else ""
                 unique_key = (requirement.name, marker_key)
 
-                if unique_key in unique_entries:
-                    raise InvalidRequirement(
-                        f"Duplicate entry, first found: {unique_entries[unique_key]}"
-                    )
-                unique_entries[unique_key] = requirement
                 if is_constraints:
+                    if unique_key in unique_entries:
+                        raise InvalidRequirement(
+                            f"Duplicate entry, first found: {unique_entries[unique_key]}"
+                        )
+                    unique_entries[unique_key] = requirement
                     if requirement.extras:
                         raise InvalidRequirement(
                             f"{requirement.name}: Constraints files cannot contain extra dependencies"

tests/test_lint_requirements.py

@@ -0,0 +1,176 @@
+import pathlib
+
+from click.testing import CliRunner
+
+from fromager.__main__ import main as fromager
+
+
+def test_requirements_allows_duplicates(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that requirements.txt files allow duplicate package names with different versions."""
+    requirements_file = tmp_path / "requirements.txt"
+    requirements_file.write_text("requests==2.28.0\nrequests==2.29.0\nnumpy==1.24.0\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(requirements_file)],
+    )
+
+    assert result.exit_code == 0, result.stdout
+    assert "Successfully validated 1 file(s)" in result.stdout
+
+
+def test_requirements_allows_duplicates_with_markers(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that requirements.txt files allow duplicate package names with different markers."""
+    requirements_file = tmp_path / "requirements.txt"
+    requirements_file.write_text(
+        'requests==2.28.0; python_version < "3.10"\n'
+        'requests==2.29.0; python_version >= "3.10"\n'
+    )
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(requirements_file)],
+    )
+
+    assert result.exit_code == 0, result.stdout
+    assert "Successfully validated 1 file(s)" in result.stdout
+
+
+def test_requirements_allows_same_package_multiple_times(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that requirements.txt files allow the same package multiple times (for multi-version builds)."""
+    requirements_file = tmp_path / "requirements.txt"
+    requirements_file.write_text("numpy==1.24.0\nnumpy==1.25.0\nnumpy==1.26.0\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(requirements_file)],
+    )
+
+    assert result.exit_code == 0, result.stdout
+    assert "Successfully validated 1 file(s)" in result.stdout
+
+
+def test_constraints_rejects_duplicates(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that constraints.txt files reject duplicate package names."""
+    constraints_file = tmp_path / "constraints.txt"
+    constraints_file.write_text("requests==2.28.0\nrequests==2.29.0\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(constraints_file)],
+    )
+
+    assert result.exit_code == 1
+    assert "Duplicate entry" in result.output
+    assert "requests==2.28.0" in result.output
+
+
+def test_constraints_rejects_duplicates_with_same_marker(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that constraints.txt files reject duplicate package names with the same marker."""
+    constraints_file = tmp_path / "constraints.txt"
+    constraints_file.write_text(
+        'requests==2.28.0; python_version < "3.10"\n'
+        'requests==2.29.0; python_version < "3.10"\n'
+    )
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(constraints_file)],
+    )
+
+    assert result.exit_code == 1
+    assert "Duplicate entry" in result.output
+
+
+def test_constraints_allows_different_markers(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that constraints.txt files allow the same package with different markers."""
+    constraints_file = tmp_path / "constraints.txt"
+    constraints_file.write_text(
+        'requests==2.28.0; python_version < "3.10"\n'
+        'requests==2.29.0; python_version >= "3.10"\n'
+    )
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(constraints_file)],
+    )
+
+    assert result.exit_code == 0, result.stdout
+    assert "Successfully validated 1 file(s)" in result.stdout
+
+
+def test_global_constraints_enforces_uniqueness(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that files ending with 'constraints.txt' (like global-constraints.txt) enforce uniqueness."""
+    constraints_file = tmp_path / "global-constraints.txt"
+    constraints_file.write_text("numpy==1.24.0\nnumpy==1.25.0\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(constraints_file)],
+    )
+
+    assert result.exit_code == 1
+    assert "Duplicate entry" in result.output
+
+
+def test_mixed_files_validation(tmp_path: pathlib.Path, cli_runner: CliRunner) -> None:
+    """Test validating both requirements.txt and constraints.txt files together."""
+    requirements_file = tmp_path / "requirements.txt"
+    requirements_file.write_text("requests==2.28.0\nrequests==2.29.0\n")
+
+    constraints_file = tmp_path / "constraints.txt"
+    constraints_file.write_text("numpy==1.24.0\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(requirements_file), str(constraints_file)],
+    )
+
+    assert result.exit_code == 0, result.stdout
+    assert "Successfully validated 2 file(s)" in result.stdout
+
+
+def test_constraints_rejects_extras(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that constraints.txt files reject packages with extras."""
+    constraints_file = tmp_path / "constraints.txt"
+    constraints_file.write_text("requests[security]==2.28.0\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(constraints_file)],
+    )
+
+    assert result.exit_code == 1
+    assert "Constraints files cannot contain extra dependencies" in result.output
+
+
+def test_constraints_requires_version_specifier(
+    tmp_path: pathlib.Path, cli_runner: CliRunner
+) -> None:
+    """Test that constraints.txt files require version specifiers."""
+    constraints_file = tmp_path / "constraints.txt"
+    constraints_file.write_text("requests\n")
+
+    result = cli_runner.invoke(
+        fromager,
+        ["lint-requirements", str(constraints_file)],
+    )
+
+    assert result.exit_code == 1
+    assert "Constraints must have a version specifier" in result.output