-
Notifications
You must be signed in to change notification settings - Fork 140
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
Support [project] table in pyproject.toml (PEP 621) #393
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Executing ./bootstrap_dev.py
causes an error in common.py
:
INFO:flit.install:Extras to install for deps 'all': {'.none'}
INFO:flit.install:Installing requirements
Requirement already satisfied: toml in /usr/lib/python3.9/site-packages (from -r /tmp/tmpmf694e1xrequirements.txt (line 1)) (0.10.2)
INFO:flit.install:Symlinking /home/user/Dokumente/git/flit/flit_core/flit_core -> /home/user/.local/lib/python3.9/site-packages/flit_core
Traceback (most recent call last):
File "/home/user/Dokumente/git/flit/./bootstrap_dev.py", line 39, in <module>
Installer(
File "/home/user/Dokumente/git/flit/flit/install.py", line 413, in install
self.install_directly()
File "/home/user/Dokumente/git/flit/flit/install.py", line 331, in install_directly
self.write_dist_info(dirs['purelib'])
File "/home/user/Dokumente/git/flit/flit/install.py", line 357, in write_dist_info
metadata = common.make_metadata(self.module, self.ini_info)
File "flit_core/flit_core/common.py", line 388, in make_metadata
return Metadata(md_dict)
File "flit_core/flit_core/common.py", line 321, in __init__
self.version = data.pop('version')
KeyError: 'version'
Metadata always contains the line "License: UNKNOWN" regardless of the project.license
table.
flit_core/flit_core/config.py
Outdated
if 'email' in person: | ||
email = person['email'] | ||
if 'name' in person: | ||
email = '{} <{}>'.format(person["name"], email) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users can include weird stuff in this field and break the formatting. Not sure if this is an issue.
{ name = "Jane\n\n\nDoe <[email protected]>", email="[email protected]" }
One could use formataddr
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I didn't know about that function. It somewhat garbles non-ASCII text, though:
In [10]: email.utils.formataddr(('Zoë', '[email protected]>'))
Out[10]: '=?utf-8?q?Zo=C3=AB?= <[email protected]>>'
I don't know if that's what should happen. The original version metadata spec 1.0 (PEP 241) says the format is based on email headers, and presumably that's what email headers do. But that was almost 20 years ago, and it's possible that tools expect something a bit more Unicode native now. PEP 621 explicitly suggests "the format {name} <{email}>
".
I'll open a discussion on the Python discourse about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing it - the issue with |
(wrong button) |
I tested the PR a bit more and found a few issues with the
This is the example project I used to test flit: pythagoras.zip |
I can’t seem to make either |
@uranusjr Have you tried uninstalling flit before installing the version from this PR with |
I installed this branch into a fresh virtual environment. Maybe I messed up something, will check when I got more time. |
@uranusjr if you installed from this branch in some obvious ways, you may have only installed |
There's a wrinkle in trying out If I use |
I'm thinking about where to put the importable module name if it differs from the distribution name. I'm currently thinking of something like this: [project]
name = "pynsist"
#...
[tool.flit.module]
name = "nsist" I.e. you would Does that seem reasonable? Is there a clearer or more useful way of writing this? Part of my thinking is that the |
Yeah that feels right by me. Is specifying metadata with [tool.flit]
name = "x"
dist-name = "y"
module = { name = "z" } |
My thinking is that people will use either the older style Eventually I'd expect to remove support for |
I'm thinking of releasing this as an experimental feature, and encouraging people to specify the build-system requirement as |
Ah, and since it’s experimental, you don’t yet use it in the readme or docs, I assume. |
Yup precisely. It's mentioned in the release notes, and I'd encourage early adopters to try using it, and then hopefully it can be documented for version 3.4. One question I'd like to resolve is what it should do if there are extra fields in |
Adding support for PEP 621 (closes #377).
You should either use a new-style
[project]
table, or the existing[tool.flit.metadata]
format: Flit won't allow you to mix them. Version and description may be given statically in the[project]
table, or listed in thedynamic=
field to use Flit to get them from the module as before.Todo:
Maybe later:
flit init
flit.tomlify
?(I'm thinking I might release it and let enthusiastic users try it out before encouraging everyone to use it)