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.

Sign up for GitHub

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

Jump to bottom

Add version field to pyproject.toml for pyluwen (v0.4.8) #11

Closed
wants to merge 1 commit into from
Closed

Add version field to pyproject.toml for pyluwen (v0.4.8) #11

wants to merge 1 commit into from

Conversation

dongjin-na
Copy link

Changes

  • Added version = "0.4.8" to crates/pyluwen/pyproject.toml.

Why this is needed

  • This change resolves build failures occurring during the installation of pyluwen via pip.
  • This issue also affects the installation of the tt-smi package, as pyluwen is a dependency.

Installation Failure Log (Before Adding Version)

pip install ./crates/pyluwen
Processing ./crates/pyluwen
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [5 lines of output]
      💥 maturin failed
        Caused by: `project.version` field is required in pyproject.toml unless it is present in the `project.dynamic` list
      Error running maturin: Command '['maturin', 'pep517', 'write-dist-info', '--metadata-directory', '/tmp/pip-modern-metadata-7v6sai1t', '--interpreter', '/home/dongjin/anaconda3/bin/python']' returned non-zero exit status 1.
      Checking for Rust toolchain....
      Running `maturin pep517 write-dist-info --metadata-directory /tmp/pip-modern-metadata-7v6sai1t --interpreter /home/dongjin/anaconda3/bin/python`
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

After adding version in crates/pyluwen/pyproject.toml.

 pip install ./crates/pyluwen
Processing ./crates/pyluwen
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: pyluwen
  Building wheel for pyluwen (pyproject.toml) ... done
  Created wheel for pyluwen: filename=pyluwen-0.4.8-cp312-cp312-linux_x86_64.whl size=616338 sha256=064eb4d1355f76df2520b5f27a270a9dcf75b9e7a2d401a521d7ce798e236b3d
  Stored in directory: /tmp/pip-ephem-wheel-cache-gt9sgh38/wheels/19/55/1c/9c3984010e91ebb2f6adb1c87d9ba112a1172c13d4c5426861
Successfully built pyluwen
Installing collected packages: pyluwen
  Attempting uninstall: pyluwen
    Found existing installation: pyluwen 0.4.8
    Uninstalling pyluwen-0.4.8:
      Successfully uninstalled pyluwen-0.4.8
Successfully installed pyluwen-0.4.8

@windwardly
Copy link

This also impacts the tt-flash/pyproject.toml and I believe just this fix won't fix as the line in tt-flash will pick a version before your fix:
"pyluwen @ git+https://github.com/tenstorrent/luwen.git@v0.4.6#subdirectory=crates/pyluwen",

Also likely will not work with tt-smi because I believe this will roll back to before this fix unless the tag / version are updated to include your work.
'pyluwen @ git+https://github.com/tenstorrent/luwen.git@v0.4.8#subdirectory=crates/pyluwen',

If a new 0.4.6 and new 0.4.8 are made, then only this repo needs to be fixed (kind of hack because there would be 2 different versions of the same version). If new version is made, then the other two will need to refer the new version (a cleaner way as each version is its own).

@milank94 milank94 added the bug Something isn't working label Dec 29, 2024
@warthog9
Copy link
Member

What version of pip and python is this happening on as I can't reproduce this

@warthog9
Copy link
Member

Apparently there's an unexpected Christmas day release of Maturin that's causing issues:

PyO3/maturin#2390

and

PyO3/maturin#2391

This should be flipped to:

[project]
dynamic = ["version"]

https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#version

and double checked before it can be merged. An expanded commit message and a sign-off would also be appreciated.

@windwardly
Copy link

The fix you proposed works for me (as a commenter, not PR author)
Currently: tt-smi uses 0.4.8, tt-flash uses 0.4.6; and tt-topology uses 0.3.8 version of luwen.
I built each with tip + your change and they built and ran; no testing beyond that.
Thanks.

warthog9 added a commit to warthog9/luwen that referenced this pull request Dec 30, 2024
@warthog9
Pyluwen: Add "version" to pyproject.toml
6f622ea 
Maturin made a change to come inline more specifically with
pyproject.toml spec, unfortunately it's not a "backwards compatible"
change and was done with little fanfare or warning, and immediately
caused breakage outside distro's more plodding versions.

Maturin Issue: PyO3/maturin#2390
pyproject.toml spec: https://packaging.python.org/en/latest/specifications/pyproject-toml/#dynamic

change in Maturin committed here:

PyO3/maturin#2391

This commit superceeds the luwen PR here:

tenstorrent#11

as that sets an explicit version, and we don't want that as it adds more
sources of truth on the version (seriously there should be 1, and only
1).

Signed-off-by: John 'Warthog9' Hawley <jhawley@tenstorrent.com>
@warthog9 warthog9 mentioned this pull request Dec 30, 2024
Merged
@warthog9
Copy link
Member

@dongjin-na @windwardly Happy to add you to #12 as "reported-by" and "tested-by" - just give me a ping if you want to do that before we queue up the PR

@windwardly
Copy link

@dongjin-na @windwardly Happy to add you to #12 as "reported-by" and "tested-by" - just give me a ping if you want to do that before we queue up the PR

No thank you. Just happy to have it fixed and propagated. Thank for your work!

@dongjin-na
Copy link
Author

@dongjin-na @windwardly Happy to add you to #12 as "reported-by" and "tested-by" - just give me a ping if you want to do that before we queue up the PR

Thanks for the offer. Please add me as "reported-by." I appreciate it.

warthog9 added a commit to warthog9/luwen that referenced this pull request Dec 30, 2024
@warthog9
Pyluwen: Add "version" to pyproject.toml
754b8b6 
Maturin made a change to come inline more specifically with
pyproject.toml spec, unfortunately it's not a "backwards compatible"
change and was done with little fanfare or warning, and immediately
caused breakage outside distro's more plodding versions.

Maturin Issue: PyO3/maturin#2390
pyproject.toml spec: https://packaging.python.org/en/latest/specifications/pyproject-toml/#dynamic

change in Maturin committed here:

PyO3/maturin#2391

This commit superceeds the luwen PR here:

tenstorrent#11

as that sets an explicit version, and we don't want that as it adds more
sources of truth on the version (seriously there should be 1, and only
1).

Reported-by: Dongjin Na <dongjin.na@moreh.io>
Signed-off-by: John 'Warthog9' Hawley <jhawley@tenstorrent.com>
@warthog9
Copy link
Member

Closing as #12 covers this better

@warthog9 warthog9 closed this Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dongjin-na @windwardly @warthog9 @milank94