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

Moved the metadata into setup.cfg. #179

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KOLANICH
Copy link

Added pyproject.toml.
Version is now fetched from git tags.

@isidentical
Copy link
Collaborator

Just to confirm, what is the motivation?

@KOLANICH
Copy link
Author

  • Get rid of any package-supplied Turing-complete or escalatable to Turing-complete code at the stage of building of packages.
  • Allow fully automatic tools to parse and modify unbuilt package metadata.

@davidhalter
Copy link
Owner

While I think this is a great change, I think it's probably too early. There are probably still issues with the new build system and I'm going to wait until everyone has new pip versions and other major libraries are using this as well.

So keeping it open for the next few years until we can probably finally merge :)

@KOLANICH
Copy link
Author

KOLANICH commented Feb 27, 2021

I think it's probably too early.

setup.cfg syntax is available since Dec 2016.

I can create a simple setup.py and move setuptools_scm config there, so the requirements to setuptools and setuptools_scm will be lowered. pyproject.toml won't have to be removed, since the versions that don't support it can just ignore it.

@blueyed
Copy link
Contributor

blueyed commented Feb 27, 2021

setuptools_scm

Have you considered using https://github.com/codrsquad/setupmeta instead?
(I often found setuptools-scm fragile / rather bad at error handling.)

@KOLANICH
Copy link
Author

KOLANICH commented Feb 27, 2021

Have you considered using https://github.com/codrsquad/setupmeta instead?
(I often found setuptools-scm fragile / rather bad at error handling.)

What are the advantages of it? I have read its readme and only found out that

  • it has many features unneeded when declarative metadata is used
  • it supports a bit more versioning schemes, most of all look not very needed (at least for me) because I don't think it is a good idea to tie the least significant version component to count of commits.

Could you explain what fragility of setuptools_scm it handles better, it may make sense to port the code handling it to setuptools_scm rather than to switch to setupmeta.

@gousaiyang
Copy link
Collaborator

Just want to point out two potentially breaking changes here. These should be reviewed before moving all metadata to setup.cfg.

Version number retrieval

The Python Packaging User Guide from PyPA introduced 7 techniques to maintain a single version number in the source code, and probably people cannot agree on a single universally "correct" one. Currently parso is using Technique 6 (import parso from setup.py). Since parso does not have any dependency packages (install_requires), this technique works good for parso.

By using setuptools_scm or similar tools, this is switching to Technique 7, and it relies on the assumption that the code must reside in a version-controlled directory (i.e. you cannot build from an extracted tarball of source code), and the version tag is correctly set prior to the build.

Alternatively, we could use the improved way of Technique 1, which is to specify version = attr: parso.__version__ in setup.cfg. This requires setuptools 46.4.0 (published in May 2020).

Long description content concatenation

readme = open('README.rst').read() + '\n\n' + open('CHANGELOG.rst').read() in setup.py will be changed to long_description = file: README.rst, CHANGELOG.rst in setup.cfg. The documentation of setuptools says that multiple files are concatenated, but did not mention whether newlines are added between files. I'm not an expert of reStructuredText and not sure whether a missing newline between two paragraphs might break the reStructuredText syntax.

@KOLANICH
Copy link
Author

By using setuptools_scm or similar tools, this is switching to Technique 7, and it relies on the assumption that the code must reside in a version-controlled directory (i.e. you cannot build from an extracted tarball of source code), and the version tag is correctly set prior to the build.

It is a very valid concern. It disallows usage of fetches of depth 1. Though fetches of longer depth (I personally use 50, which was used on Travis especially of this issue) usually succeed, but it is not guaranteed.

In my other software, prebuilder (in fact fetchers lib, mainly used in it), I use a bit different approach - in workflows based on it I control the fetching process entirely, so I implemented a way to fetch the last tag on a needed branch from a remote, unfortunately it has caused me to maintain some service-specific modules for each major code hosting. A better alternative would be to modify git itself to expose this information over its smart protocol (probably in a form of a commit graph blob), but I am not going to do that because of the reasons that are offtop for this issue.

Anyway, the convention in the projects using git is to set tags to versions, and this project follows it.

The Python Packaging User Guide from PyPA

Thank you for the link, BTW. I feel I'd have to send a PR there too, I see multiple places in it that can be improved.

Alternatively, we could use the improved way of Technique 1, which is to specify version = attr: parso.version in setup.cfg. This requires setuptools 46.4.0 (published in May 2020).

It is a very bad idea to do this way. This method (and other methods relying on importing) assummes that the directory from which build can be triggered is the root dir of the repo. Also it executes code on stage of package building, the code of the package itself, it is even worse than just executing only setup.py, because setup.py is small and more easy for audit than the package that has enough space to hide a backdoor in, so it defeats the purpose of this PR.

I'm not an expert of reStructuredText and not sure whether a missing newline between two paragraphs might break the reStructuredText syntax.

This is a valid concern. But I saw pretty a lot of packages concatenating the files without separator. But given rst is the blessed format for docs in python community, I don't estimate the probability of breaking markup to be high. I guess we should just unpack the wheel and check if the rst in its metadata rendered correctly.

@KOLANICH KOLANICH force-pushed the setup.cfg branch 2 times, most recently from 319d2fa to 3d7e5f8 Compare September 8, 2021 10:03
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.

5 participants