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

ci: Use uv pip for faster build #2038

Merged
merged 1 commit into from
Mar 23, 2024
Merged

ci: Use uv pip for faster build #2038

merged 1 commit into from
Mar 23, 2024

Conversation

rht
Copy link
Contributor

@rht rht commented Feb 16, 2024

uv is a fast drop-in pip, pip-compile, virtualenv etc replacement created by the creator of Ruff.

@Corvince
Copy link
Contributor

Thank you for mentioning uv, the news haven't reached me yet. I think this is quite exciting and I will start using uv today, but I strongly discourage its use for mesa just yet. The first public release is less than 24 hours ago. This potentially decreases our workflows by a few seconds, but I don't think that justifies the possible hurdles that are attached to bleeding edge software.

@rht
Copy link
Contributor Author

rht commented Feb 16, 2024

I have yet to measure by how much this shaves off the build time, because the CI still fails.

According to the blog post:

uv is designed as a drop-in replacement for pip and pip-tools, and is ready for production use today in projects built around those workflows.

If it builds and runs the code just fine, I see no reason not to use it. Besides, Ruff is considered bleeding edge, yet many huge projects use it in their CI. Not all "bleeding edge" public-released tools are the same in terms of reliability.

@rht
Copy link
Contributor Author

rht commented Feb 16, 2024

I have overcome astral-sh/uv#1374. Waiting for astral-sh/uv#1355 to be fixed, which should happen really soon.

@Corvince
Copy link
Contributor

Not all "bleeding edge" public-released tools are the same in terms of reliability.

Yes but there have already been 2 bugfix releases in the first 12 hours of its existence and you already linked two issues that you had to overcome/waiting for a fix. And these are just the obvious bugs, I am more worried about the subtle bugs, where it appears to be working, but some details are broken. This is inevitable for such large projects. We don't know if it will affect us, but why risk it?

@rht
Copy link
Contributor Author

rht commented Feb 16, 2024

Didn't you want to know how much this will speed up the macOS and Windows build? My sense is that it will be a lot, and to have a CI duration that is < 1min.

@quaquel
Copy link
Contributor

quaquel commented Feb 16, 2024

I agree with @Corvince. Let's give UV a few weeks to sort out the initial problems before we consider shifting.

Also, what is the problem with a CI that takes a bit over a minute?

@rht
Copy link
Contributor Author

rht commented Feb 16, 2024

Let's give UV a few weeks to sort out the initial problems before we consider shifting.

That sounds like a good timeline for Ruff developers. Ruff used to have a daily (if not more frequent) release cycle. And during this high frequency releases, it already had adopters by huge projects. Faster CI does matter, to the point they have been willing to adopt an alpha-release linter.

Also, what is the problem with a CI that takes a bit over a minute?

#2039 (comment)

@EwoutH
Copy link
Contributor

EwoutH commented Feb 17, 2024

but I strongly discourage its use for mesa just yet. The first public release is less than 24 hours ago.

Agreed. Let's wait a few week until uv stabilizes a bit more.

@EwoutH EwoutH added the ci Release notes label label Feb 17, 2024
- name: Install dependencies
# Only if the cache misses
# Based on https://github.com/pypa/pip/issues/8049#issuecomment-633845028
# read_requirements.py should be removed once
# https://github.com/pypa/pip/issues/11440 is resolved.
if: steps.cache.outputs.cache-hit != 'true'
run: |
pip install toml
rm -fr ${Python_ROOT_DIR}/lib/python${{ matrix.python-version}}/site-packages/pip-23.0.1.dist-info
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this line needed? Seems like it would break easily when the pip version changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uv breaks if there are multiple pip installed. There isn't a bug report for this yet, though if I were to open one, would be resolved within days because of the use case in GH Actions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about this one? Specifically I don't see how why multiple pips should be installed at all and why uv should interact with them at all. Also I haven't seen this in other workflow files that already use uv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because they use uv venv, e.g. astral-sh/uv#1386 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking of waiting until the non-venv solution has stabilized, but uv venv is blazing fast, so it shouldn't matter to create the venv.

Copy link
Contributor Author

@rht rht Feb 20, 2024

Choose a reason for hiding this comment

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

With uv venv, pytest failed https://github.com/projectmesa/mesa/actions/runs/7980718647/job/21790983916?pr=2038 because Solara has a hardcoded assumption on the virtualenv information.

.github/workflows/build_lint.yml Outdated Show resolved Hide resolved
@Corvince
Copy link
Contributor

I think its funny that pip takes 2 seconds to install just uv and then uv continues to install the dependencies in a few milliseconds. Curious how it will perform with outdated caches.

@rht
Copy link
Contributor Author

rht commented Feb 20, 2024

Everything builds except for Windows, which is a matter of using rm -r -fo instead of rm -fr.
The macOS build is sped up 2x (which should be sped up further after catching). The examples build is sped up from 27s in #2044 to 17s.

I should be able to revert my advanced (but complex) caching back to setup-python pip cache, and it would be simpler and even faster overall.

@rht rht force-pushed the uv branch 13 times, most recently from cef4928 to 6f76bf5 Compare February 23, 2024 05:32
@rht
Copy link
Contributor Author

rht commented Feb 23, 2024

Tests all passed.

@rht
Copy link
Contributor Author

rht commented Mar 3, 2024

It's now even more concise with uv pip install --system ....

@EwoutH
Copy link
Contributor

EwoutH commented Mar 3, 2024

Nice work, I like the reduced complexity of our CI configuration. I think we can try it.

@rht
Copy link
Contributor Author

rht commented Mar 4, 2024

There has been ~100 PRs opened that contain uv pip install, including from the astral-sh repo itself. How about we wait until at least ~300? https://github.com/search?q=%22uv+pip+install%22&type=pullrequests.

Simplify build_lint.yml caching

temp

Use uv venv

uv: Use --system flag

ci: Remove -e from pip install
Copy link
Contributor

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

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

Run uv pip install --system .[dev]
Resolved 110 packages in 903ms
Downloaded 108 packages in 1.76s
Installed 110 packages in 267ms

This is quite awesome. Thanks!

@EwoutH EwoutH merged commit 3563854 into projectmesa:main Mar 23, 2024
12 checks passed
@rht rht deleted the uv branch March 23, 2024 11:28
vitorfrois pushed a commit to vitorfrois/mesa that referenced this pull request Jul 15, 2024
uv is a fast drop-in pip, pip-compile, virtualenv etc replacement created by the creator of Ruff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Release notes label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants