From 7b58557fba3101f06307d4b40417bd34971ecf1f Mon Sep 17 00:00:00 2001 From: Rohan Devasthale Date: Mon, 13 Apr 2026 13:43:22 -0400 Subject: [PATCH] fix(bootstrap): auto-disable constraints and extend e2e test coverage Two related fixes for --multiple-versions flag: 1. **Auto-disable constraints with --multiple-versions** - When --multiple-versions flag is enabled, automatically set skip_constraints=True because constraints.txt cannot handle multiple versions of the same package - Log informative message: "automatically disabling constraints generation (incompatible with --multiple-versions)" 2. **Extend e2e test to verify dependency chain handling** - Use generous constraint range (flit-core>=3.9,<3.12) instead of pinning to single version - Verify that multiple versions of flit-core are built (not just tomli), confirming --multiple-versions works for entire chain - Check for at least 2 flit-core versions (test found 3) - Remove `|| true` since constraints are now auto-disabled The e2e test now validates that --multiple-versions bootstraps multiple versions of both top-level packages AND their dependencies, providing better test coverage. Fixes: #1044 Fixes: #1045 Co-Authored-By: Claude Sonnet 4.5 Signed-off-by: Rohan Devasthale --- e2e/test_bootstrap_multiple_versions.sh | 32 +++++- src/fromager/commands/bootstrap.py | 8 ++ tests/test_bootstrap.py | 138 ++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 5 deletions(-) diff --git a/e2e/test_bootstrap_multiple_versions.sh b/e2e/test_bootstrap_multiple_versions.sh index 2d77c4b2..7588adb6 100755 --- a/e2e/test_bootstrap_multiple_versions.sh +++ b/e2e/test_bootstrap_multiple_versions.sh @@ -7,18 +7,21 @@ SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" source "$SCRIPTDIR/common.sh" -# Create constraints file to pin build dependencies (keeps CI fast) +# Create constraints file with generous ranges to test multiple versions +# of build dependencies (not just top-level packages) constraints_file=$(mktemp) trap "rm -f $constraints_file" EXIT cat > "$constraints_file" <=3.9,<3.12 EOF # Use tomli with a version range that matches exactly 3 versions (2.0.0, 2.0.1, 2.0.2) # tomli has no runtime dependencies, making it fast to bootstrap -# It uses flit-core as build backend (pinned above) +# It uses flit-core as build backend, and we allow multiple flit-core versions +# to test that --multiple-versions works for the entire dependency chain # Using <=2.0.2 instead of <2.1 to be deterministic (tomli 2.1.0 exists) -# Note: constraints file generation will fail (expected with multiple versions) +# Note: constraints file generation is automatically disabled with --multiple-versions fromager \ --log-file="$OUTDIR/bootstrap.log" \ --error-log-file="$OUTDIR/fromager-errors.log" \ @@ -28,7 +31,7 @@ fromager \ --constraints-file="$constraints_file" \ bootstrap \ --multiple-versions \ - 'tomli>=2.0,<=2.0.2' || true + 'tomli>=2.0,<=2.0.2' # Check that wheels were built echo "Checking for wheels..." @@ -60,3 +63,22 @@ fi echo "" echo "SUCCESS: All expected tomli versions (2.0.0, 2.0.1, 2.0.2) were bootstrapped" + +# Verify that multiple versions of flit-core were built (dependency of tomli) +# This confirms that --multiple-versions works for the entire dependency chain +echo "" +echo "Checking for flit-core versions (build dependency)..." +FLIT_CORE_COUNT=$(find "$OUTDIR/wheels-repo/downloads/" -name 'flit_core-3.*.whl' | wc -l) +echo "Found $FLIT_CORE_COUNT flit-core 3.x wheel(s)" + +if [ "$FLIT_CORE_COUNT" -lt 2 ]; then + echo "" + echo "ERROR: Expected at least 2 flit-core versions, found $FLIT_CORE_COUNT" + echo "The --multiple-versions flag should bootstrap multiple versions of dependencies too" + echo "" + echo "Found flit-core wheels:" + find "$OUTDIR/wheels-repo/downloads/" -name 'flit_core-*.whl' + exit 1 +fi + +echo "✓ Multiple versions of flit-core were bootstrapped (confirms dependency chain handling)" diff --git a/src/fromager/commands/bootstrap.py b/src/fromager/commands/bootstrap.py index 3abf0644..a6268e73 100644 --- a/src/fromager/commands/bootstrap.py +++ b/src/fromager/commands/bootstrap.py @@ -159,6 +159,14 @@ def bootstrap( logger.info( "multiple versions mode enabled: will bootstrap all matching versions" ) + # Automatically disable constraints when multiple versions mode is enabled + # because constraints.txt cannot handle multiple versions of the same package + if not skip_constraints: + logger.info( + "automatically disabling constraints generation " + "(incompatible with --multiple-versions)" + ) + skip_constraints = True pre_built = wkctx.settings.list_pre_built() if pre_built: diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 025c048e..fa990ec4 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -1,4 +1,5 @@ import io +import logging import pathlib import textwrap from unittest.mock import Mock, patch @@ -541,6 +542,143 @@ def test_skip_constraints_cli_option() -> None: assert "Skip generating constraints.txt file" in result.output +@patch("fromager.commands.bootstrap.bootstrapper.Bootstrapper") +@patch("fromager.commands.bootstrap.server.start_wheel_server") +@patch("fromager.commands.bootstrap.progress.progress_context") +@patch("fromager.commands.bootstrap.metrics.summarize") +def test_multiple_versions_auto_disables_constraints( + mock_metrics: Mock, + mock_progress: Mock, + mock_server: Mock, + mock_bootstrapper: Mock, + tmp_context: context.WorkContext, + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that --multiple-versions alone auto-disables constraints and logs message""" + # Setup mocks + mock_progress.return_value.__enter__.return_value = Mock() + mock_progress.return_value.__exit__.return_value = None + mock_bt_instance = Mock() + mock_bt_instance.resolve_and_add_top_level.return_value = ("url", Version("1.0")) + mock_bt_instance.finalize.return_value = 0 + mock_bootstrapper.return_value = mock_bt_instance + + runner = CliRunner() + with runner.isolated_filesystem(): + # Create a temporary requirements file + pathlib.Path("req.txt").write_text("setuptools>=60\n") + + # Invoke with --multiple-versions but NOT --skip-constraints + with caplog.at_level(logging.INFO): + result = runner.invoke( + bootstrap.bootstrap, + [ + "-r", + "req.txt", + "--multiple-versions", + ], + obj=tmp_context, + ) + + # Should succeed + assert result.exit_code == 0 + + # Should log that constraints are auto-disabled + assert "automatically disabling constraints generation" in caplog.text + assert "incompatible with --multiple-versions" in caplog.text + + +@patch("fromager.commands.bootstrap.bootstrapper.Bootstrapper") +@patch("fromager.commands.bootstrap.server.start_wheel_server") +@patch("fromager.commands.bootstrap.progress.progress_context") +@patch("fromager.commands.bootstrap.metrics.summarize") +def test_multiple_versions_with_skip_constraints_no_duplicate_log( + mock_metrics: Mock, + mock_progress: Mock, + mock_server: Mock, + mock_bootstrapper: Mock, + tmp_context: context.WorkContext, + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that --multiple-versions --skip-constraints together doesn't log auto-disable message""" + # Setup mocks + mock_progress.return_value.__enter__.return_value = Mock() + mock_progress.return_value.__exit__.return_value = None + mock_bt_instance = Mock() + mock_bt_instance.resolve_and_add_top_level.return_value = ("url", Version("1.0")) + mock_bt_instance.finalize.return_value = 0 + mock_bootstrapper.return_value = mock_bt_instance + + runner = CliRunner() + with runner.isolated_filesystem(): + # Create a temporary requirements file + pathlib.Path("req.txt").write_text("setuptools>=60\n") + + # Invoke with BOTH --multiple-versions AND --skip-constraints + with caplog.at_level(logging.INFO): + result = runner.invoke( + bootstrap.bootstrap, + [ + "-r", + "req.txt", + "--multiple-versions", + "--skip-constraints", + ], + obj=tmp_context, + ) + + # Should succeed + assert result.exit_code == 0 + + # Should NOT log the auto-disable message (already disabled by user) + assert "automatically disabling constraints generation" not in caplog.text + + +@patch("fromager.commands.bootstrap.bootstrapper.Bootstrapper") +@patch("fromager.commands.bootstrap.server.start_wheel_server") +@patch("fromager.commands.bootstrap.progress.progress_context") +@patch("fromager.commands.bootstrap.metrics.summarize") +@patch("fromager.commands.bootstrap.write_constraints_file") +def test_without_multiple_versions_constraints_not_disabled( + mock_write_constraints: Mock, + mock_metrics: Mock, + mock_progress: Mock, + mock_server: Mock, + mock_bootstrapper: Mock, + tmp_context: context.WorkContext, +) -> None: + """Test that without --multiple-versions, constraints are not auto-disabled""" + # Setup mocks + mock_progress.return_value.__enter__.return_value = Mock() + mock_progress.return_value.__exit__.return_value = None + mock_bt_instance = Mock() + mock_bt_instance.resolve_and_add_top_level.return_value = ("url", Version("1.0")) + mock_bt_instance.finalize.return_value = 0 + mock_bootstrapper.return_value = mock_bt_instance + mock_write_constraints.return_value = True + + runner = CliRunner() + with runner.isolated_filesystem(): + # Create a temporary requirements file + pathlib.Path("req.txt").write_text("setuptools>=60\n") + + # Invoke WITHOUT --multiple-versions + result = runner.invoke( + bootstrap.bootstrap, + [ + "-r", + "req.txt", + ], + obj=tmp_context, + ) + + # Should succeed + assert result.exit_code == 0 + + # write_constraints_file should have been called (constraints NOT disabled) + assert mock_write_constraints.called + + @patch("fromager.gitutils.git_clone") def test_resolve_version_from_git_url_with_submodules_enabled( mock_git_clone: Mock,