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

Add uv as develop backend command #2015

Merged
merged 3 commits into from
Apr 7, 2024

Conversation

dmatos2012
Copy link
Contributor

Hi,
This should close #1959. However, I would like some feedback/help before I imagine this is ready to merge. I plan on adding more tests to see that it works accross crates etc, but got into a weird test error loop, that I hope someone can help me here.

Basically about making the develop_hello_world_uv test pass. If I activate my venv then test fails with no-such file or directory hello-world. If I dont activate it, then it passes. Sadly, if I deactivate then the other tests dont pass as I need cffi to have my tests pass, what gives?

Doing a /target/debug/maturin develop --uv on 3-4 crates from test-crates worked fine, thus not sure whats with the test harness that doenst make it happy.

If anyone else more experienced can help me out, it would be great. Maybe not the best place for long comment here but didnt know where else :). Thanks for your time

Copy link

netlify bot commented Mar 28, 2024

Deploy Preview for maturin-guide ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f27ae17
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/660db1b70379300008a34864
😎 Deploy Preview https://deploy-preview-2015--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dmatos2012 dmatos2012 force-pushed the add-uv-for-develop-cmd branch from 159be56 to 0c2c007 Compare March 28, 2024 22:59
@messense messense self-requested a review April 2, 2024 01:21
Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

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

Thanks for starting this!

src/develop.rs Outdated Show resolved Hide resolved
src/develop.rs Outdated Show resolved Hide resolved
src/develop.rs Outdated Show resolved Hide resolved
src/develop.rs Outdated Show resolved Hide resolved
src/develop.rs Outdated Show resolved Hide resolved
@konstin
Copy link
Member

konstin commented Apr 2, 2024

Basically about making the develop_hello_world_uv test pass. If I activate my venv then test fails with no-such file or directory hello-world. If I dont activate it, then it passes.

Can you check where this hello-world comes from with which hello-world? The tests should be independent of the containing venv not counting cffi.

@dmatos2012
Copy link
Contributor Author

dmatos2012 commented Apr 2, 2024

So,

# develop_hello_world_uv
#[test]
fn develop_hello_world_uv() {
    handle_result(develop::test_develop(
        "test-crates/hello-world",
        None,
        "develop-hello-world-uv",
        false,
        true,
    ));
}
> virtualenv venv
created virtual environment CPython3.8.10.final.0-64 in 102ms
  creator CPython3Posix(dest=/home/david/oss/maturin/venv, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/david/.local/share/virtualenv)
    added seed packages: pip==24.0, setuptools==69.1.1, wheel==0.42.0
> cargo test develop_hello_world_uv (no venv activated)
running 1 test
test develop_hello_world_uv ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 56 filtered out; finished in 3.48s
> source venv/bin/activate
> pip install cffi

cargo test develop_hello_world_uv
Errors with: 

---- develop_hello_world_uv stdout ----
🔗 Found bin bindings
📡 Using build options bindings from pyproject.toml
🎯 Found 2 Cargo targets in `Cargo.toml`: hello-world, foo
📦 Built wheel to /tmp/.tmpgHeVhr/hello_world-0.1.0-py3-none-linux_x86_64.whl
✏️  Setting installed package as editable
🛠 Installed hello-world-0.1.0
Cause: Check install fail: exit status: 1 
--- Stdout:

--- Stderr:
Traceback (most recent call last):
  File "test-crates/hello-world/check_installed/check_installed.py", line 16, in <module>
    main()
  File "test-crates/hello-world/check_installed/check_installed.py", line 5, in main
    output = check_output(["hello-world"]).decode("utf-8").strip()
  File "/usr/lib/python3.8/subprocess.py", line 415, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib/python3.8/subprocess.py", line 493, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.8/subprocess.py", line 1704, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'hello-world'

thread 'develop_hello_world_uv' panicked at tests/common/mod.rs:88:13:
Check install fail: exit status: 1 
--- Stdout:

--- Stderr:
Traceback (most recent call last):
  File "test-crates/hello-world/check_installed/check_installed.py", line 16, in <module>
    main()
  File "test-crates/hello-world/check_installed/check_installed.py", line 5, in main
    output = check_output(["hello-world"]).decode("utf-8").strip()
  File "/usr/lib/python3.8/subprocess.py", line 415, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib/python3.8/subprocess.py", line 493, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.8/subprocess.py", line 1704, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'hello-world'

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    develop_hello_world_uv

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 56 filtered out; finished in 7.91s

error: test failed, to rerun pass `--test run`

Then, with the venv activated and then after the test failure I do:

> pip list
Package     Version
----------- -------
cffi        1.16.0
hello-world 0.1.0
pip         24.0
pycparser   2.22
setuptools  69.1.1
wheel       0.42.0
> hello-world  
Hello, world!

> which hello-world
/home/david/oss/maturin/venv/bin/hello-world

But if I change test_develop_hello_world_uv to just use pip(thus basically test_develop_hello_world) , by setting false in last arg and recreate then the whole venv, and test it using pip,

#[test]
fn develop_hello_world_uv() {
    handle_result(develop::test_develop(
        "test-crates/hello-world",
        None,
        "develop-hello-world-uv",
        false,
        false,
    ));
}
> cargo test develop_hello_world_uv
running 1 test
test develop_hello_world_uv ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 56 filtered out; finished in 3.60s

then

> pip list
Package    Version
---------- -------
cffi       1.16.0
pip        24.0
pycparser  2.22
setuptools 69.1.1
wheel      0.42.0

Doesnt show hello-world installed
So it looks like when using uv, the hello-world is installed to the venv but when just using pip is not?

@dmatos2012
Copy link
Contributor Author

I have also updated the code to reflect the changes @konstin . Thanks for your review! :)

src/develop.rs Outdated Show resolved Hide resolved
tests/common/develop.rs Outdated Show resolved Hide resolved
@messense
Copy link
Member

messense commented Apr 3, 2024

If I activate my venv then test fails with no-such file or directory hello-world. If I dont activate it, then it passes. Sadly, if I deactivate then the other tests dont pass as I need cffi to have my tests pass, what gives?

This might be an existing issue, I do remember encountering similar issue in that past. Usually I just install cffi globally to fix it.

@dmatos2012
Copy link
Contributor Author

Yeah, addressed your comments @messense. Yeah, and I guess just installing cffi globally fixed all my issues and run the tests without a venv.

Finally, before I add the TestInstallBackend::Uv for all tests, would you folks be ok with adding rstest as a dev-dependency crate? That way we can add both backend cases without duplicating the test. Let me know, and then ill change/add accordingly to cover all the tests that TestInstallBackend::Pip covers

@messense
Copy link
Member

messense commented Apr 4, 2024

Sounds reasonable to me.

@konstin
Copy link
Member

konstin commented Apr 5, 2024

rstest is a great addition!

For the test suite performance, i'd keep only running the develop test with uv, maybe we can eventually even switch the other tests to uv.

Copy link
Member

@messense messense left a comment

Choose a reason for hiding this comment

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

LGTM, we should probably also support uv other installations not from pypi, but it doesn't need to block merging this.

@messense
Copy link
Member

messense commented Apr 7, 2024

I'm a bit confused why CI passes without uv installed.

@dmatos2012
Copy link
Contributor Author

Hi, I'm outside my PC for the day, but it's think it's because I added uv as a package to install, like cffi. Not sure the test file now but it's on the diff. Wasn't sure is that was ok or not

@messense
Copy link
Member

messense commented Apr 7, 2024

I missed that, thanks for pointing it out!

@messense messense merged commit 311d4ad into PyO3:main Apr 7, 2024
28 of 29 checks passed
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jun 6, 2024
https://build.opensuse.org/request/show/1178629
by user mia + anag+factory
- Update to 1.6.0
  * Add pypi name validation
    gh#PyO3/maturin#2007
  * Add JSON schema generation
    gh#PyO3/maturin#2005
  * Detect compiling from Linux gnu to Linux musl as cross compiling
    gh#PyO3/maturin#2010
  * Upgrade uniffi to 0.27.0
    gh#PyO3/maturin#2021
  * Add instrumentation support for develop
    gh#PyO3/maturin#2019
  * Make tracing-subscriber mandatory
    gh#PyO3/maturin#2022
  * Import hook upgrade
    gh#PyO3/maturin#2024
  * Add uv as develop backend command
    gh#PyO3/maturin#2015
  * Also try uv in PATH in develop --uv
    gh#PyO3/maturin#2026
  * docs: update pyo3 to match tutorial
    gh#PyO3/maturin#2029
  * Add support for AIX
    gh#PyO3/maturin#2030
  * Remove rust-cpython from project init/new template
    gh#PyO3/maturin#2034
  * Only run uv tests
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.

option to use uv
3 participants