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 needed items for sdist #755

Merged
merged 7 commits into from
Oct 10, 2024
Merged

Add needed items for sdist #755

merged 7 commits into from
Oct 10, 2024

Conversation

emlowe
Copy link
Contributor

@emlowe emlowe commented Oct 9, 2024

Add in some missing bits in wheel/Cargo.toml and some hacks around missing parts in the tar archive

Main items are:
needing to add in chia-client and chia-ssl to wheel/Cargo.toml
needing to remove the ../LICENSE from wheel/Cargo.toml - this resulted in the tar not having the LICENSE file but also pip expecting it, so I just removed it for now.
I noticed the tar was missing the src directory, so I had to manually add in the src directory to the tar archive.

This does make a tar.gz that does build on my riscv machine and also my mac.

@emlowe emlowe requested review from Rigidity and altendky October 9, 2024 22:07
@emlowe emlowe marked this pull request as ready for review October 9, 2024 22:32
Copy link

Pull Request Test Coverage Report for Build 11263879338

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.638%

Totals Coverage Status
Change from base Build 11262793680: 0.0%
Covered Lines: 12708
Relevant Lines: 15194

💛 - Coveralls

Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

It's not obvious why chia-client and chia-ssl are needed to build the wheel. Is maturin failing to bundle transitive dependencies? I wouldn't expect these to be part of the dependency tree at all though

@emlowe
Copy link
Contributor Author

emlowe commented Oct 10, 2024

When maturin builds the sdist, it puts in the tar the Cargo.toml from the root workspace and not from the wheel directory. They are listed in the root Cargo as deps and default features.

@arvidn
Copy link
Contributor

arvidn commented Oct 10, 2024

I see. thanks for the explanation.

Copy link
Contributor

@Rigidity Rigidity left a comment

Choose a reason for hiding this comment

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

lgtm for now

@arvidn arvidn merged commit ba9d26d into main Oct 10, 2024
60 checks passed
@arvidn arvidn deleted the EL.sdist-hacks branch October 10, 2024 23:07
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