-
Notifications
You must be signed in to change notification settings - Fork 7k
[deps] adding include_setuptools flag for depset config #58580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -319,6 +319,36 @@ def test_append_uv_flags_with_space_in_flag(self): | |
| output_text = output_file.read_text() | ||
| assert "--python-version 3.10" in output_text | ||
|
|
||
| def test_include_setuptools(self): | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| copy_data_to_tmpdir(tmpdir) | ||
| manager = _create_test_manager(tmpdir) | ||
| manager.compile( | ||
| constraints=[], | ||
| requirements=["requirements_test.txt"], | ||
| name="general_depset", | ||
| output="requirements_compiled_general.txt", | ||
| include_setuptools=True, | ||
| ) | ||
| output_file = Path(tmpdir) / "requirements_compiled_general.txt" | ||
| output_text = output_file.read_text() | ||
| assert "--unsafe-package setuptools" not in output_text | ||
|
|
||
| def test_ignore_setuptools(self): | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| copy_data_to_tmpdir(tmpdir) | ||
| manager = _create_test_manager(tmpdir) | ||
| manager.compile( | ||
| constraints=[], | ||
| requirements=["requirements_test.txt"], | ||
| name="general_depset", | ||
| output="requirements_compiled_general.txt", | ||
| include_setuptools=False, | ||
| ) | ||
| output_file = Path(tmpdir) / "requirements_compiled_general.txt" | ||
| output_text = output_file.read_text() | ||
| assert "--unsafe-package setuptools" in output_text | ||
|
|
||
|
Comment on lines
+322
to
+351
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests You'll need to ensure @pytest.mark.parametrize(
"include_setuptools, expect_unsafe_flag",
[
(True, False),
(False, True),
],
)
def test_setuptools_handling(self, include_setuptools, expect_unsafe_flag):
with tempfile.TemporaryDirectory() as tmpdir:
copy_data_to_tmpdir(tmpdir)
manager = _create_test_manager(tmpdir)
manager.compile(
constraints=[],
requirements=["requirements_test.txt"],
name="general_depset",
output="requirements_compiled_general.txt",
include_setuptools=include_setuptools,
)
output_file = Path(tmpdir) / "requirements_compiled_general.txt"
output_text = output_file.read_text()
has_unsafe_flag = "--unsafe-package setuptools" in output_text
assert has_unsafe_flag is expect_unsafe_flag |
||
| def test_override_uv_flag_single_flag(self): | ||
| expected_flags = DEFAULT_UV_FLAGS.copy() | ||
| expected_flags.remove("--index-strategy") | ||
|
|
@@ -334,12 +364,12 @@ def test_override_uv_flag_single_flag(self): | |
|
|
||
| def test_override_uv_flag_multiple_flags(self): | ||
| expected_flags = DEFAULT_UV_FLAGS.copy() | ||
| expected_flags.remove("--unsafe-package") | ||
| expected_flags.remove("setuptools") | ||
| expected_flags.extend(["--unsafe-package", "dummy"]) | ||
| expected_flags.remove("--index-url") | ||
| expected_flags.remove("https://pypi.org/simple") | ||
| expected_flags.extend(["--index-url", "https://dummyurl.com"]) | ||
| assert ( | ||
| _override_uv_flags( | ||
| ["--unsafe-package dummy"], | ||
| ["--index-url https://dummyurl.com"], | ||
| DEFAULT_UV_FLAGS.copy(), | ||
| ) | ||
| == expected_flags | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| # This file was autogenerated by uv via the following command: | ||
| # uv pip compile --generate-hashes --unsafe-package setuptools --index-url https://pypi.org/simple --index-strategy unsafe-best-match --no-strip-markers --emit-index-url --emit-find-links --unsafe-package ray --python-version=3.10 --python-platform=linux -c release/ray_release/byod/ray_base_extra_testdeps_py3.10.lock docker/base-deps/requirements.in docker/base-extra/requirements.in -o python/deplocks/base_extra/ray_base_extra_py3.10.lock | ||
| # uv pip compile --generate-hashes --index-url https://pypi.org/simple --index-strategy unsafe-best-match --no-strip-markers --emit-index-url --emit-find-links --unsafe-package setuptools --unsafe-package ray --python-version=3.10 --python-platform=linux -c release/ray_release/byod/ray_base_extra_testdeps_py3.10.lock docker/base-deps/requirements.in docker/base-extra/requirements.in -o python/deplocks/base_extra/ray_base_extra_py3.10.lock | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm... I thought these flags are sorted already?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no i reverted the flag sorting. Order matters and didnt want to sort lexigraphically. Will handle that with the flag interface |
||
| --index-url https://pypi.org/simple | ||
|
|
||
| adlfs==2023.8.0 \ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Override Logic Ignores Setuptools Flag
The
include_setuptoolsflag is applied beforeoverride_flags, causing it to be ignored when override flags contain--unsafe-package. The_override_uv_flagsfunction removes all instances of flag names that appear in the override list, so if override flags include--unsafe-package ray, it removes the previously added--unsafe-package setuptools. The setuptools flag should be added after applying override flags to ensure theinclude_setuptoolssetting is respected.