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

fix: long-description on Windows #2168

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented Nov 7, 2022

Summary

  • OS: Windows
  • Bug fix: yes
  • Type: wheels
  • Fixes:

Description

Not sure why the Popen communication is failing on GHA but it inserts a bunch of newlines that shouldn't be there.
This PR uses a temporary file rather than communicating through Popen.

Signed-off-by: mayeut <mayeut@users.noreply.github.com>
Signed-off-by: mayeut <mayeut@users.noreply.github.com>
@mayeut
Copy link
Contributor Author

mayeut commented Nov 7, 2022

@giampaolo, it's probably not as clean is I'd want but thought I'd share this anyway as it's getting late and I probably won't be able to work again on this before the week-end.

@mayeut mayeut changed the title Twine check fix: long-description on Windows Nov 7, 2022
Comment on lines +47 to +49
if len(sys.argv) > 2:
with open(sys.argv[2], "wb") as f:
f.write(data.encode("utf8"))
Copy link
Owner

Choose a reason for hiding this comment

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

Couldn't you simply do data.encode("utf8") from setup.py and avoid editing this file?

Choose a reason for hiding this comment

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

@mayeut this one is missing import sys

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't missing in my commit (since sys was already used to read the first argument) so I don't know what went wrong on master.

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

That's my fault actually. I manually edited @mayeut's PR to get rid of some parts, then merged it.

@giampaolo giampaolo merged commit 90c2eec into giampaolo:master Nov 10, 2022
@giampaolo
Copy link
Owner

giampaolo commented Nov 10, 2022

Merging as-is. I will try to fix the encoding stuff in a separate commit.

ddelange added a commit to ddelange/psutil that referenced this pull request Nov 11, 2022
* 'master' of https://github.com/giampaolo/psutil:
  fix: long-description on Windows (giampaolo#2168)
  GH actions: update actions' deps to latest version
  use argparse module in 3 scripts/internal scripts
  setup.py: provide better err msg if can't compile
  giampaolo#2164: fix compilation failures on linux < 2.6.27 / CentOS 5 (giampaolo#2171)
giampaolo added a commit that referenced this pull request Nov 11, 2022
@giampaolo
Copy link
Owner

Ok, hopefully c0aadb1 should fix this once and for all.

@mayeut mayeut deleted the twine-check branch November 11, 2022 15:45
ddelange added a commit to ddelange/psutil that referenced this pull request Nov 11, 2022
* 'master' of https://github.com/giampaolo/psutil:
  fix long_description on Windows (see: giampaolo#2168)
  make.bat: use default 'python' exe; get rid of hard-coded python path
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