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

DRAFT: Feature/uv #112

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
*.pyc
.vscode
69 changes: 69 additions & 0 deletions catkin_virtualenv/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
## How UV Works

### Use Case

The main case for including a `UV` option in `catkin_virtualenv` is for hosts where a large number of virtual environments are created and those virtual environments share a sizeable number of identical packages.

`UV` benefits this use case by deduplicating packages installed into each virtual environment

### How it works
The deduplication feature of UV works by creating a single shared hardlink farm, we will call this the `cache`. Each time we attempt to install a pip package into a `venv` the following is performed

1. `UV` checks if the package is available in the `cache`
2. If the package is not available then the package is downloaded and placed in the `cache`
3. The package is available in the `cache` at this point.
4. Instead of copying the package from the `cache` into the `venv` the package is hardlinked into the `venv`
5. Because packages are hardlinked there a no issues one might find with symlinks. They are identical to files created by installing directly into the `venv`

##### Hardlinking

```
Every file on the Linux filesystem starts with a single hard link. The link is between the filename and the actual data stored on the filesystem. Creating an additional hard link to a file means a few different things.

When changes are made to one filename, the other reflects those changes. The permissions, link count, ownership, timestamps, and file content are the exact same. If the original file is deleted, the data still exists under the secondary hard link. The data is only removed from your drive when all links to the data have been removed. If you find two files with identical properties but are unsure if they are hard-linked, use the ls -i command to view the inode number. Files that are hard-linked together share the same inode number.

```

Ref : [Red Hat Hardlinks](https://www.redhat.com/sysadmin/linking-linux-explained)

### How do I use it

Use of `UV` is currently opt-in.
To use it modify your `catkin_generate_virtualenv` to resemble the following

```
catkin_generate_virtualenv(
INPUT_REQUIREMENTS requirements.in
USE_UV TRUE
UV_CACHE /tmp/testing/uv/cache
)

Choose a reason for hiding this comment

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

I know its just a draft, but I assume this will be the default? It would just be nice to enable this transparently without having to change any packages, if that is at all possible.

Also, as far as the cache location, what is the consequence if different packages set different cache locations? They just won't be able to share the venv I assume?

Copy link
Author

Choose a reason for hiding this comment

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

We wouldn't want this in /tmp because it would not survive a reboot. It should probably land in /var/lib/uv or whatever the current standard for persistent data is on Linux.


```

#### Configuration

#TODO Document OPTIONS

`UV` aims to be identical in usage to`Pip`, however there are configration differences. We do not allow mixing of `PIP` arguments with `UV` arguments.

**Allowed**
```
catkin_generate_virtualenv(
INPUT_REQUIREMENTS requirements.in
USE_UV TRUE
UV_CACHE /tmp/testing/uv/cache
EXTRA_UV_ARGS ....
)

```

**Not Allowed**
```
catkin_generate_virtualenv(
INPUT_REQUIREMENTS requirements.in
USE_UV TRUE
UV_CACHE /tmp/testing/uv/cache
EXTRA_PIP_ARGS ....
)

Choose a reason for hiding this comment

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

My only concern here is if we have packages that use the extra pip args, and if they do for what reason and would this be supported with UV.

Copy link
Author

Choose a reason for hiding this comment

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

The issue with EXTRA_PIP_ARGS is they get passed in as string to uv some pip args aren't supported.
I thought it cleaner to say: use these args if using uv use these args if using pip.
Admittedly some of the args will overlap.


```
94 changes: 77 additions & 17 deletions catkin_virtualenv/cmake/catkin_generate_virtualenv.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
function(catkin_generate_virtualenv)
set(oneValueArgs PYTHON_VERSION PYTHON_INTERPRETER USE_SYSTEM_PACKAGES ISOLATE_REQUIREMENTS INPUT_REQUIREMENTS CHECK_VENV)
set(multiValueArgs EXTRA_PIP_ARGS)
set(oneValueArgs PYTHON_VERSION PYTHON_INTERPRETER USE_SYSTEM_PACKAGES ISOLATE_REQUIREMENTS INPUT_REQUIREMENTS CHECK_VENV USE_UV UV_CACHE)
set(multiValueArgs EXTRA_PIP_ARGS EXTRA_UV_ARGS)
cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN} )

### Handle argument defaults and deprecations
Expand All @@ -32,19 +32,64 @@ function(catkin_generate_virtualenv)
set(ARG_PYTHON_INTERPRETER "python3")
endif()

if(NOT DEFINED ARG_USE_SYSTEM_PACKAGES OR ARG_USE_SYSTEM_PACKAGES)
message(STATUS "Using system site packages")
set(venv_args "--use-system-packages")


if(DEFINED ARG_USE_UV)
message(STATUS "USING UV")
set(use_uv "--use-uv")

if( ARG_ISOLATE_REQUIREMENTS )
message( FATAL_ERROR "You can use ISOLATE_REQUIREMENTS in conjunction with USE_UV" )

Choose a reason for hiding this comment

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

Did you mean You can't use ISOLATE_REQUIREMENTS...

Copy link
Author

Choose a reason for hiding this comment

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

Depends on what that means. Do you not want to use uv at all, do you want your own cache (why?)
I'll have to look at what ISOLATE_REQUIREMENTS means a bit more, for some reason I thought it had to do with venv chaining and not necessarily using system-packages

Copy link
Contributor

@paulbovbel paulbovbel Jul 16, 2024

Choose a reason for hiding this comment

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

ISOLATE_REQUIREMENTS does indeed cause a package not to inherit its dependencies requirements.txt. Interestingly enough we don't seem to use it anywhere anymore, and I can't remember why we added it. However, does beg the questions why it's not supported with uv

endif()

if(NOT DEFINED ARG_USE_UV_CACHE OR ARG_USE_UV_CACHE)
message(STATUS "Using uv cache")
set(use_uv_cache "--uv-cache")
endif()

if(NOT DEFINED ARG_UV_CACHE )
message(STATUS "Setting uv cache location to default")
set(uv_cache "/tmp/testing/uv/cache")
else()
message(STATUS "Setting uv cache location")
set(uv_cache "${ARG_UV_CACHE}")
endif()

message(STATUS "Clearing Venv args")
set(venv_args "")

if (NOT DEFINED ARG_EXTRA_UV_ARGS)
message(STATUS "Setting Extra UV Args")
set(ARG_EXTRA_UV_ARGS "")
endif()

else()

message(STATUS "NOT USING UV")

if(NOT DEFINED ARG_USE_SYSTEM_PACKAGES OR ARG_USE_SYSTEM_PACKAGES)
message(STATUS "Using system site packages")
set(venv_args "--use-system-packages")
endif()

if (NOT DEFINED ARG_EXTRA_PIP_ARGS)
set(ARG_EXTRA_PIP_ARGS "-qq" "--retries 10" "--timeout 30")
endif()

message(STATUS "Clearing UV Args")
set(use_uv "")
set(use_uv_cache "")
set(uv_cache "")
endif()


if(ARG_ISOLATE_REQUIREMENTS)
message(STATUS "Only using requirements from this catkin package")
set(lock_args "${lock_args} --no-deps")
endif()

if (NOT DEFINED ARG_EXTRA_PIP_ARGS)
set(ARG_EXTRA_PIP_ARGS "-qq" "--retries 10" "--timeout 30")
endif()



# Convert CMake list to ' '-separated list
string(REPLACE ";" "\ " processed_pip_args "${ARG_EXTRA_PIP_ARGS}")
Expand Down Expand Up @@ -92,10 +137,13 @@ function(catkin_generate_virtualenv)
endif()
endforeach()




add_custom_command(COMMENT "Generate virtualenv in ${CMAKE_BINARY_DIR}/${venv_dir}"
OUTPUT ${CMAKE_BINARY_DIR}/${venv_dir}/bin/python
COMMAND ${CATKIN_ENV} rosrun catkin_virtualenv venv_init ${venv_dir}
--python ${ARG_PYTHON_INTERPRETER} ${venv_args} --extra-pip-args ${processed_pip_args}
--python ${ARG_PYTHON_INTERPRETER} ${venv_args} --extra-pip-args ${processed_pip_args} ${use_uv} ${use_uv_cache} ${uv_cache}
)

if(DEFINED ARG_INPUT_REQUIREMENTS AND NOT package_requirements STREQUAL "")
Expand All @@ -114,7 +162,7 @@ function(catkin_generate_virtualenv)
add_custom_command(COMMENT "Install requirements to ${CMAKE_BINARY_DIR}/${venv_dir}"
OUTPUT ${CMAKE_BINARY_DIR}/${venv_dir}/bin/activate
COMMAND ${CATKIN_ENV} rosrun catkin_virtualenv venv_install ${venv_dir}
--requirements ${requirements_list} --extra-pip-args ${processed_pip_args}
--requirements ${requirements_list} --extra-pip-args ${processed_pip_args} ${use_uv} ${use_uv_cache} ${uv_cache}
DEPENDS
${CMAKE_BINARY_DIR}/${venv_dir}/bin/python
${package_requirements}
Expand Down Expand Up @@ -154,13 +202,25 @@ function(catkin_generate_virtualenv)

if(CATKIN_ENABLE_TESTING AND NOT package_requirements STREQUAL "" AND (NOT DEFINED ARG_CHECK_VENV OR ARG_CHECK_VENV))
file(MAKE_DIRECTORY ${CATKIN_TEST_RESULTS_DIR}/${PROJECT_NAME})
catkin_run_tests_target("venv_check" "${PROJECT_NAME}-requirements" "venv_check-${PROJECT_NAME}-requirements.xml"
COMMAND "${CATKIN_ENV} rosrun catkin_virtualenv venv_check ${venv_dir} --requirements ${package_requirements} \
--extra-pip-args \"${processed_pip_args}\" \
--xunit-output ${CATKIN_TEST_RESULTS_DIR}/${PROJECT_NAME}/venv_check-${PROJECT_NAME}-requirements.xml"
DEPENDENCIES ${PROJECT_NAME}_generate_virtualenv
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)
if(NOT DEFINED ARG_USE_UV)
catkin_run_tests_target("venv_check" "${PROJECT_NAME}-requirements" "venv_check-${PROJECT_NAME}-requirements.xml"
COMMAND "${CATKIN_ENV} rosrun catkin_virtualenv venv_check ${venv_dir} --requirements ${package_requirements} \
--extra-pip-args \"${processed_pip_args}\" \
--xunit-output ${CATKIN_TEST_RESULTS_DIR}/${PROJECT_NAME}/venv_check-${PROJECT_NAME}-requirements.xml"
DEPENDENCIES ${PROJECT_NAME}_generate_virtualenv
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)
else()
message(STATUS "PackReq: ${package_requirements}")

catkin_run_tests_target("venv_check" "${PROJECT_NAME}-requirements" "venv_check-${PROJECT_NAME}-requirements.xml"
COMMAND "${CATKIN_ENV} rosrun catkin_virtualenv venv_check ${venv_dir} --requirements ${package_requirements} \
--use-uv \
Copy link
Contributor

@paulbovbel paulbovbel Jul 16, 2024

Choose a reason for hiding this comment

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

one easy pattern to reduce duplication here:

if(DEFINED ARG_USE_UV)
  set(extra_args "${extra_args} --use-uv")
endif()

catkin_run_tests_target(... COMMAND ... ${extra_args}

--xunit-output ${CATKIN_TEST_RESULTS_DIR}/${PROJECT_NAME}/venv_check-${PROJECT_NAME}-requirements.xml"
DEPENDENCIES ${PROJECT_NAME}_generate_virtualenv
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)
endif()
endif()

install(DIRECTORY ${CMAKE_BINARY_DIR}/install/${venv_dir}
Expand Down
2 changes: 2 additions & 0 deletions catkin_virtualenv/requirements.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
catkin-pkg
nose
mock
rospkg
setproctitle
uv
62 changes: 27 additions & 35 deletions catkin_virtualenv/scripts/venv_check
Original file line number Diff line number Diff line change
Expand Up @@ -21,56 +21,48 @@
import argparse
import inspect
import sys
import typing

import xml.etree.ElementTree as ET

from catkin_virtualenv import configure_logging
from catkin_virtualenv.venv import Virtualenv
from catkin_virtualenv.uvvenv import UVVirtualEnv
from catkin_virtualenv.cli import _parse_check_args, _build_sanitized_extra_args, diff_check

logger = configure_logging()

if __name__ == '__main__':
logger = configure_logging()

parser = argparse.ArgumentParser(description=Virtualenv.install.__doc__)
parser.add_argument(
'venv', help="Path of virtualenv to manage.")
parser.add_argument(
'--requirements', required=True, help="Requirements to check.")
parser.add_argument(
'--extra-pip-args', default='""', type=str, help="Extra pip args for install.")
parser.add_argument(
'--xunit-output', help="Destination where to write xunit output.")
def main(argv: typing.Union[typing.List[str], None] = None) -> None:

args = parser.parse_args()
args = _parse_check_args(argv)

extra_pip_args = args.extra_pip_args[1:-1]
extra_pip_args = _build_sanitized_extra_args(args, False)

venv = Virtualenv(args.venv)
diff = venv.check(
requirements=args.requirements,
extra_pip_args=[arg for arg in extra_pip_args.split(" ") if arg != ""],
)
diff = []
if args.use_uv:
diff = []
venv = UVVirtualEnv(args.venv)
diff = venv.check(requirements=args.requirements, extra_uv_args=args.extra_uv_args)
else:
venv = Virtualenv(args.venv)
diff = venv.check(
requirements=args.requirements,
extra_pip_args=extra_pip_args,
)
check_xunit(args, diff)

if args.xunit_output:
testsuite = ET.Element('testsuite', name="venv_check", tests="1", failures="1" if diff else "0", errors="0")
testcase = ET.SubElement(testsuite, 'testcase', name="check_locked", classname="catkin_virtualenv.Venv")
if diff:
failure = ET.SubElement(testcase, 'failure', message="{} is not fully locked".format(args.requirements))
message = inspect.cleandoc("""
Consider defining INPUT_REQUIREMENTS to have catkin_virtualenv generate a lock file for this package.
See https://github.com/locusrobotics/catkin_virtualenv/blob/master/README.md#locking-dependencies.
The following changes would fully lock {requirements}:
""".format(requirements=args.requirements))
message += '\n' + '\n'.join(diff)
failure.text = message

else:
success = ET.SubElement(testcase, 'success', message="{} is fully locked".format(args.requirements))
def check_xunit(args: argparse.Namespace, diff: typing.List[str]):

tree = ET.ElementTree(testsuite)
tree.write(args.xunit_output, encoding='utf-8', xml_declaration=True)
if args.xunit_output:
diff_check(args.xunit_output, args.requirements, diff)

else:
if diff:
logger.error("{} is not fully locked, see diff:\n{}".format(args.requirements, '\n'.join(diff)))
logger.error("{} is not fully locked, see diff:\n{}".format(args.requirements, "\n".join(diff)))
sys.exit(1)


if __name__ == "__main__":
main()
46 changes: 22 additions & 24 deletions catkin_virtualenv/scripts/venv_init
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,31 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import argparse

import typing
from catkin_virtualenv import configure_logging
from catkin_virtualenv.venv import Virtualenv
from catkin_virtualenv.cli import _parse_init_args, _build_sanitized_extra_args
from catkin_virtualenv.uvvenv import UVVirtualEnv, PythonVersion


if __name__ == '__main__':
def main(argv: typing.Union[typing.List[str], None] = None) -> None:
configure_logging()

parser = argparse.ArgumentParser(description=Virtualenv.initialize.__doc__)
parser.add_argument(
'venv', help="Path where to initialize the virtualenv")
parser.add_argument(
'--python', default="python3", help="Build the virtualenv with which python version.")
parser.add_argument(
'--use-system-packages', action="store_true", help="Use system site packages.")
parser.add_argument(
'--extra-pip-args', default='""', type=str, help="Extra pip args for install.")

args = parser.parse_args()

extra_pip_args = args.extra_pip_args[1:-1]

print(args.use_system_packages)
venv = Virtualenv(args.venv)
venv.initialize(
python=args.python,
use_system_packages=args.use_system_packages,
extra_pip_args=[arg for arg in extra_pip_args.split(" ") if arg != ""],
)
args = _parse_init_args(argv)

if not args.use_uv:
sanitized_extra_pip_args = _build_sanitized_extra_args(args, use_uv=False)
venv = Virtualenv(args.venv)
venv.initialize(
python=args.python,
use_system_packages=args.use_system_packages,
extra_pip_args=sanitized_extra_pip_args,
)
else:
sanitized_extra_uv_args = _build_sanitized_extra_args(args, use_uv=True)
uvvenv = UVVirtualEnv(args.venv, args.uv_cache)
uvvenv.initialize(python=PythonVersion(args.python), extra_pip_args=sanitized_extra_uv_args)


if __name__ == "__main__":
main()
Loading