Skip to content
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

Support uv compiled requirements files #10040

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions python/helpers/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ plette==2.1.0
poetry==1.8.3
# TODO: Replace 3p package `toml` with 3.11's new stdlib `tomllib` once we drop support for Python 3.10.
toml==0.10.2
uv==0.4.9

# Some dependencies will only install if Cython is present
Cython==3.0.10
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,14 @@ def compile_new_requirement_files
def compile_file(filename)
# Shell out to pip-compile, generate a new set of requirements.
# This is slow, as pip-compile needs to do installs.
options = pip_compile_options(filename)

options, command = pip_compile_options(filename)
options_fingerprint = pip_compile_options_fingerprint(options)

name_part = "pyenv exec pip-compile " \
name_part = "#{command} " \
"#{options} -P " \
"#{dependency.name}"
fingerprint_name_part = "pyenv exec pip-compile " \
fingerprint_name_part = "#{command} " \
"#{options_fingerprint} -P " \
"<dependency_name>"

Expand Down Expand Up @@ -453,10 +454,16 @@ def pip_compile_options(filename)
options += pip_compile_index_options

if (requirements_file = compiled_file_for_filename(filename))
options += pip_compile_options_from_compiled_file(requirements_file)
if requirements_file.content.include?("uv pip compile")
options += uv_pip_compile_options_from_compiled_file(requirements_file)
command = "pyenv exec uv pip compile"
else
options += pip_compile_options_from_compiled_file(requirements_file)
command = "pyenv exec pip-compile"
end
end

options.join(" ")
[options.join(" "), command]
end

def pip_compile_options_from_compiled_file(requirements_file)
Expand All @@ -483,6 +490,32 @@ def pip_compile_options_from_compiled_file(requirements_file)
options
end

def uv_pip_compile_options_from_compiled_file(requirements_file)
options = ["--output-file=#{requirements_file.name}"]

options << "--no-emit-index-url" unless requirements_file.content.include?("index-url http")
Copy link

@RazerM RazerM Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip-tools defaults to --emit-index-url, but will skip the default index.

uv defaults to --no-emit-index-url so I think this should be

Suggested change
options << "--no-emit-index-url" unless requirements_file.content.include?("index-url http")
options << "--emit-index-url" if requirements_file.content.include?("index-url http")


options << "--generate-hashes" if requirements_file.content.include?("--hash=sha")

options << "--no-annotate" unless requirements_file.content.include?("# via ")

options << "--pre" if requirements_file.content.include?("--pre")

options << "--no-strip-extras" if requirements_file.content.include?("--no-strip-extras")

if requirements_file.content.include?("--no-binary") || requirements_file.content.include?("--only-binary")
options << "--emit-build-options"
end

if (resolver = RESOLVER_REGEX.match(requirements_file.content))
options << "--resolver=#{resolver}"
end
Comment on lines +510 to +512
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any reason to pass this to uv, since it's just there for better UX


options << "--universal" if requirements_file.content.include?("--universal")

options
avilaton marked this conversation as resolved.
Show resolved Hide resolved
end

def pip_compile_index_options
credentials
.select { |cred| cred["type"] == "python_index" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ def fetch_latest_resolvable_version_string(requirement:)
def compile_file(filename)
# Shell out to pip-compile.
# This is slow, as pip-compile needs to do installs.
options = pip_compile_options(filename)
options, command = pip_compile_options(filename)
options_fingerprint = pip_compile_options_fingerprint(options)

run_pip_compile_command(
"pyenv exec pip-compile -v #{options} -P #{dependency.name} #{filename}",
fingerprint: "pyenv exec pip-compile -v #{options_fingerprint} -P <dependency_name> <filename>"
"#{command} -v #{options} -P #{dependency.name} #{filename}",
fingerprint: "#{command} -v #{options_fingerprint} -P <dependency_name> <filename>"
)

return true if dependency.top_level?
Expand All @@ -110,8 +110,8 @@ def compile_file(filename)
# update_not_possible.
write_original_manifest_files
run_pip_compile_command(
"pyenv exec pip-compile #{options} #{filename}",
fingerprint: "pyenv exec pip-compile #{options_fingerprint} <filename>"
"#{command} #{options} #{filename}",
fingerprint: "#{command} #{options_fingerprint} <filename>"
)

true
Expand Down Expand Up @@ -201,12 +201,12 @@ def check_original_requirements_resolvable
write_temporary_dependency_files(update_requirement: false)

filenames_to_compile.each do |filename|
options = pip_compile_options(filename)
options, command = pip_compile_options(filename)
options_fingerprint = pip_compile_options_fingerprint(options)

run_pip_compile_command(
"pyenv exec pip-compile #{options} #{filename}",
fingerprint: "pyenv exec pip-compile #{options_fingerprint} <filename>"
"#{command} #{options} #{filename}",
fingerprint: "#{command} #{options_fingerprint} <filename>"
)
end

Expand Down Expand Up @@ -251,7 +251,13 @@ def pip_compile_options(filename)
options << "--output-file=#{requirements_file.name}"
end

options.join(" ")
command = "pyenv exec pip-compile"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, but if else to set the command seems cleaner (similar to the other place in the code where this check is done)

if (requirements_file = compiled_file_for_filename(filename)) &&
requirements_file.content.include?("autogenerated by uv")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks a different string than the other function to generate flags from an existing requirements file.

Would it be worth extracting the text we search for and making them both use the same?

avilaton marked this conversation as resolved.
Show resolved Hide resolved
command = "pyenv exec uv pip compile"
end

[options.join(" "), command]
end

def pip_compile_index_options
Expand Down
Loading
Loading