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 #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KOLANICH
Copy link
Contributor

Added pyproject.toml.
Version is now fetched from the latest git tag on the current branch.
.gitignored bytecode cache files.

Added pyproject.toml.
Version is now fetched from the latest git tag on the current branch.
.gitignored bytecode cache files.
@Gallaecio
Copy link
Member

Why remove bumpversion?

@KOLANICH
Copy link
Contributor Author

Why remove bumpversion?

I guess I should revert deletion of that file. I'd need some discussion.

I am not very familiar to bumpversion, so I assummed it can only update versions in source code, so deleted it, since it was proposed to use git tags as a primary source of a version. It has some drawbacks though, so it may be useful to keep the versions as they are.

So I wanna know which workflow is preferred for you and if they can be implemented:

  • the primary location of version is source code, the secondary on are git tag and setup.cfg. version is updated using bumpversion. Pros:
    1. package can be built without git and setuptools_scm.
    2. you can use bump-version
      Cons:
    3. 3 locations to store tags
    4. no git commit hash info in version
  • the primary location is a git tag, no hardcode in source and setup.cfg. Pros:
    1. DRY principle is not violated, only a single place with version
    2. git commit hash in version for non-release versions
      Cons:
    3. --depth when fetching git repos should be enough to catch the version tag
    4. dependence on setuptools_scmandgit`
    5. cannot be built from archives without .git dir with all the needed info (but if you distribute the app on GitHub, it is not a big issue, we can expect users of GH to use git and on pypi wheels are put (I see no reason to put there sdists, since this is a pure python lib), which are results of build process, so the metadata and files should be there)
    6. IDK if it is possible to use bumpversion in this workflow to read the version from git tags and create a new tag with a new version.

@Gallaecio
Copy link
Member

I don’t see much value in the pros of dropping bumpversion: (1) DRY is maintained in that version only needs to be changed manually in one place, and (2) while nice-to-have, I don’t see much value on adding a commit hash to the locally-built version in this specific project.

Dropping bumpversion, on the other hand, makes the release process slightly more involved. I’m not sure having a Git tag in local use cases is worth that.

@Gallaecio
Copy link
Member

Before you work on this further, though, I think we need to discuss whether or not it makes sense to apply this change as a whole.
Why should cssselect switch to .toml? Since there are no plans to deprecate setup.py, I see no reason for this project to switch. I see no significant gain, while there is potential for issues.

@KOLANICH
Copy link
Contributor Author

Why should cssselect switch to .toml?

It is not yet a switch to toml. PEP 621 is not yet implemented in setuptools for that and noone knows when it will be implemented. It's just a switch to cfg for now.

This pyproject.toml currently just implements PEP 517 for building with build + for getting rid of setup.py + storing settings for setuptools_scm.

Since there are no plans to deprecate setup.py, I see no reason for this project to switch. I see no significant gain, while there is potential for issues.

setup.py is written in a Turing-complete language. It causes severe issues.

  • It cannot be reliably parsed into the data contained in it. It can be only executed. So malware can be inserted (and was really inserted in the wild, though I cannot Google exactly the link I had in mind (dated to 200x years), there are other links) into it. So one has to read it and try to fully understand its code in order to find any obfuscated backdoors.
  • It cannot be reliably edited. It is the consequence of the previous point. If one cannot understand the semantics of each AST node and the AST as whole (since one can redefine this semantics, i.e. by creating a wrapper around setuptools.setup called setup but using i.e. version for name and name for version, and such stuff will likely confuse even a human, not only a dumb algo), one cannot meaningfully edit it.

Declarative formats for metadata solve the both issues:

  • they are not Turing-complete;
  • they cannot redefine names, there is a bijection name <-> semantics.

@Gallaecio
Copy link
Member

Gallaecio commented Feb 21, 2021

None of those two issues seem relevant for cssselect. Issue 1 cannot be exploitable thought us (and, if it could, I don’t think this change would solve that), 2 is not a concern for us at the moment (we do not need automated complex edition of setup.py).

I would understand it if the Python community eventually deprecates setup.py to solve problem 1. But until that happens, I don’t see how this change, in this project, helps anyone.

@KOLANICH
Copy link
Contributor Author

  1. we do not need automated complex edition of setup.py

This feature is not for your direct consumption. It is more for transforming the ecosystem. Without parseable and editable metadata the features like automatic creation of github-wide and maybe all-code-hostings-wide dependency graph is unnecessary complicated and unreliable. Look, there is still no dependabot support for setup.py, instead it relies on requirements.txt. And so on and so on.

  1. It is very unlikely that python community would deprecate setup.py. Deprecation is just a word. It makes sense only if one
    • can enforce it;
    • will enforce it;
    • has a hard deadline after which the decision will be enforced.

But the case of python 2 has showed that PSF has no means to enforce own decisions. Even after they have officially dropped py 2 after the multiple times delayed "last chinese warnings", there are still py2-only projects that don't accept PRs to upgrade them to py 3.

So, if setuptools and pip just dropped setup.py, these projects would just ignore the issue (it would take years to deliver a version of setuptools and pip to distros).

Also setup.py has legit use case. Though again in fact not (if properly implemented, the content of setup.py should be a separate setuptools plugin, and the package building should be just a configuring the plugin), it is impractical to write a plugin for each setup.py (though in fact it is impractical to use custom setup.py instead of reusing setuptools plugins that exist).
Anyway, it is obvious, that if PyPA decided to drop setup.py, this initiative would be sabotaged. So, setup.py cannot really be deprecated.

But what you can do, is to deprecate it for you. In this sense it is already deprecated for quite some people on the basis that setup.cfg is just "better" than setup.py. Little direct value for the maintainer currently (before having setup.py is not considered as an antifeature)? Yes. But it is "better", so we just migrate.

@Gallaecio
Copy link
Member

Gallaecio commented Feb 21, 2021

But what you can do, is to deprecate it for you.

My point is, until doing so gives us some benefit, I personally see no point.

For example, you mention dependabot. That could be a benefit. If dependabot was not able to parse setup.py, and dependabot was useful for cssselect, I think that would be enough to justify this change. However, dependabot seem to support setup.py to some extent (enough to support cssselect’s, most likely). More importantly, cssselect has no dependencies; and even if it had, I think dependabot is meant to be used in end projects, not in reusable libraries which aim to support wide ranges of dependencies.

Now, while I see no direct benefit in making this change at the moment, provided that it does not cause any issue (e.g. in Conda), I won’t block this change. I’m +0 here. If any other maintainer feels we should do this now, let’s do it.

I simply don’t want you to put too much time into this without first getting the backing of at least one maintainer.

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.

2 participants