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

uv + uv.lock #200

Merged
merged 5 commits into from
Dec 9, 2024
Merged

uv + uv.lock #200

merged 5 commits into from
Dec 9, 2024

Conversation

fabi1cazenave
Copy link
Collaborator

uv makes it so much easier to handle dependencies and venv. Amazing stuff.

@gagath any idea of what should be done to make it suitable for you as a Debian maintainer?

@gagath
Copy link
Contributor

gagath commented Nov 26, 2024

@fabi1cazenave We currently run the tests in autopkgtests by calling make test PYTHON3=$v for each supported python3 version in Debian to ease transitions: https://salsa.debian.org/python-team/packages/kalamine/-/blob/debian/latest/debian/tests/pytest?ref_type=heads

This change breaks make test by requiring uv, which we do not want to introduce as a dependency just to run tests. It also breaks the PYTHON3 variable. We could copy/paste the test lines into the autopkgtest script, but it is kind of a regression as we would need to watch for changes to the test target in the Makefile on every release.

From a packaging perspective, this PR is a regression.

@fabi1cazenave
Copy link
Collaborator Author

Hi @gagath, thanks for the quick reply.

I don’t expect Debian maintainers to depend on uv for Python packages, but I was wondering if there was an acceptable way to make it work for both workflows. Would it be okay if uv is used when available on the host, but not required ? T’hat’s what I think I’ve done in my latest commit.

I certainly don’t want to make it worse for packaging, but uv does make it easier for quick development tasks. Please let me know if the current proposal is okay. :-)

@@ -2,35 +2,59 @@

PYTHON3?=python3

UV := $(shell command -v uv 2> /dev/null)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suddenly wondering whether this works on Windows or not.
@Geobert can you test this, please?

Copy link
Collaborator

Choose a reason for hiding this comment

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

will do this evening :)

@Geobert
Copy link
Collaborator

Geobert commented Dec 6, 2024

I don’t know much about uv so I rely on @gagath’s expertise ^^

@gagath
Copy link
Contributor

gagath commented Dec 6, 2024

Debian package and tests run OK with this uv branch. However new warnings are introduced because of the corpus thing. I will create a separate issue.

@gagath
Copy link
Contributor

gagath commented Dec 6, 2024

Here are the new issues related to corpus/ with the Debian packaging: #203

Again, not related to this uv PR that builds and tests fine with the debian packaging so good to merge for me.

@fabi1cazenave fabi1cazenave merged commit 40b0907 into main Dec 9, 2024
16 checks passed
@fabi1cazenave fabi1cazenave deleted the uv branch December 9, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants