Moved the metadata into setup.cfg #253

KOLANICH · 2020-07-09T10:56:32Z

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Fixes issue #:

Description of the changes being introduced by the pull request:

setup.cfg is a declarative format for metadata requiring no remote code execution in order to be parsed.
setuptools_scm fetches versions from git tags, which decreises new release burden. For non-tag commits it adds pat of a commit hash to a version, which helps bug reporting if anything goes wrong.
pyproject.toml is a newer declarative format for configuration. In some distant future setup.cfg contents will be moved there. Tools recently introduced declarative configuration, like setuptools_scm, use it. It also allows to build packages in an unified way: python3 -m pep517.build -b

Please verify and check that the pull request fulfils the following
requirements:

The code follows the Code Style Guidelines
Tests have been added for the bug fix or new feature
Docs have been added for the bug fix or new feature

coveralls · 2020-07-09T10:58:58Z

Coverage remained the same at 98.531% when pulling 62fb130 on prebuilder:setup.cfg into 0a8bde5 on secure-systems-lab:master.

sechkova

I support shifting towards newer packaging systems but can you elaborate (maybe update the doc section) what is the intended way of archive generation after this set of changes (see inline comments)?

sechkova · 2021-01-20T11:11:01Z

setup.py

@@ -1,4 +1,4 @@
-#! /usr/bin/env python
+#! /usr/bin/env python3


Unfortunately, I don't think python2 ca be dropped out of the equation yet. I let a maintainer confirm.
Is python2 fully incompatible with this set of changes or the update to py3 comes for a convenience?

I don't remember. I remember that the version of setuptools released in January 2020 has dropped python 2. I don't use python 2 since 2013.

Not everyone is that lucky ...

sechkova · 2021-01-20T11:28:08Z

setup.py

-
-setup(
-  name = 'securesystemslib',
-  version = '0.18.0',


I tested locally with python 3.7 generating a source dist as it is still listed in the documentation:
$ python setup.py sdist
The resulting archive is missing a version number: securesystemslib-0.0.0. I assume this may be some incompatibility with setuptools_scm?

In case I was not supposed to use setup.py to generate the sdist, what is the intended way of archive generation after this set of changes? Using the pep517 module as shown in the PR description did the job but the package documentation marks its high-level functionality as deprecated.
Seems like the recommended way is by using build.

Try to upgrade setuptools. You need a recent (at least January 2019) setuptools to understand pyproject.toml, and if it understands it, it would upgrade itself to the version having the needed hooks setuptools_scm that uses pyproject.toml for config depends on.

Seems like the recommended way is by using build.

Yes, it's true. The PR was originally created when there was no build.

lukpueh

Thanks for the persistence and sorry for the late review, @KOLANICH!

Let me know if you feel like switching to hatchling in this PR. I'm also fine if we do that in a follow-up PR. I wouldn't add setuptools_scm, though, as we didn't use it before and won't use in the future.

lukpueh · 2022-11-17T11:14:52Z

pyproject.toml

+dependencies = [
+    "six>=1.11.0",
+    'subprocess32; python_version < "3"',
+    "python-dateutil>=2.8.0",
+]


Suggested change

dependencies = [

"six>=1.11.0",

'subprocess32; python_version < "3"',

"python-dateutil>=2.8.0",

]

We don't have any of these dependencies anymore.

lukpueh · 2022-11-17T11:19:24Z

pyproject.toml

+[project.optional-dependencies]
+crypto = ["cryptography>=37.0.0"]
+pynacl = ["pynacl>1.2.0"]
+testing = ['mock; python_version < "3.3"']


Suggested change

testing = ['mock; python_version < "3.3"']

Don't need this one anymore either.

lukpueh · 2022-11-17T11:40:11Z

pyproject.toml

@@ -0,0 +1,63 @@
+[build-system]
+requires = ["setuptools>=61.2", "setuptools_scm[toml]>=3.4.3"]
+build-backend = "setuptools.build_meta"


I don't think we need the features of setuptools_scm, and I'd generally like us to switch to hatchling as we did in python-tuf and in-toto (theupdateframework/python-tuf#1896, in-toto/in-toto#490).

Current PR does use setuptools_scm magic to build the package version number (it's not anywhere in source right now). Otherwise I think you are correct.

Oh right, I forgot that we don't have the version in __init__.py. I'd rather add it there and not use git to infer it. But I'm fine with doing that in a follow up PR.

lukpueh · 2022-11-17T11:41:56Z

pyproject.toml

+PySPX = ["PySPX==0.5.0"]
+
+[tool.setuptools]
+include-package-data = false


How does this relate to MANIFEST.in? I think we want to include some non-*.py files in source distributions.

KOLANICH · 2022-11-17T22:42:10Z

Let me know if you feel like switching to hatchling in this PR. I'm also fine if we do that in a follow-up PR.

I'd prefer it to happen in a separate PR/commit. Switching to PEP 621 without changing the build backend is one change. Switching to hatchling is another one. (Though former is a prereq of the latter).

I wouldn't add setuptools_scm, though, as we didn't use it before and won't use in the future.

As jku has noticed, it is used to eliminate the need to populate the version manually into multiple places (git history, source code file, metadata file). Also it places git commit hash into version number of non-releaseversions, which is helpful for debugging if anything goes wrong. Hatchling has a similar plugin hatch-vcs using setuptools_scm under the hood.

I'd rather add it there and not use git to infer it. But I'm fine with doing that in a follow up PR.

It may be possible to make setuptools to infer it from the source code. Not sure about hatchling though, I have too little experience with it.

How does this relate to MANIFEST.in? I think we want to include some non-*.py files in source distributions.

When it is true it uses it to place the files into the wheel. It is not documented, but the docs make an impression this setting is not about sdists.

lukpueh · 2022-11-18T09:04:21Z

Let me know if you feel like switching to hatchling in this PR. I'm also fine if we do that in a follow-up PR.

I'd prefer it to happen in a separate PR/commit. Switching to PEP 621 without changing the build backend is one change. Switching to hatchling is another one. (Though former is a prereq of the latter).

Agreed. Let's switch to hatchling in a follow-up PR.

I wouldn't add setuptools_scm, though, as we didn't use it before and won't use in the future.

As jku has noticed, it is used to eliminate the need to populate the version manually into multiple places (git history, source code file, metadata file). Also it places git commit hash into version number of non-releaseversions, which is helpful for debugging if anything goes wrong. Hatchling has a similar plugin hatch-vcs using setuptools_scm under the hood.

Okay, let's use setuptools_scm then.

I'd rather add it there and not use git to infer it. But I'm fine with doing that in a follow up PR.

It may be possible to make setuptools to infer it from the source code. Not sure about hatchling though, I have too little experience with it.

Don't know how to do it in setuptools, but you can tell hatchling, where to find the version in source.

How does this relate to MANIFEST.in? I think we want to include some non-*.py files in source distributions.

When it is true it uses it to place the files into the wheel. It is not documented, but the docs make an impression this setting is not about sdists.

Thanks for the docs link. I suggest to either set it to true then or replicate the includes/excludes from MANIFEST.in here and remove MANIFEST.in.

lukpueh · 2022-11-22T09:57:20Z

I see you force-pushed but did not address my review comments. Are you interested in finalizing this PR, @KOLANICH?

…ct.toml`.

lukpueh

LGTM! I tried this and it works as expected.

Also checked that it includes everything we want it to include in an sdist via MANIFEST.in. Interestingly, it also includes at least one of the files we ignore via the manifest.

No need to fix this here, as we need to change the configuration when switching to hatchling anyways.

Thanks again for the contribution, @KOLANICH!

KOLANICH force-pushed the setup.cfg branch 2 times, most recently from f15865c to aa1bca1 Compare July 13, 2020 16:38

KOLANICH force-pushed the setup.cfg branch 2 times, most recently from 6a25aa4 to 0caff24 Compare August 4, 2020 23:11

KOLANICH force-pushed the setup.cfg branch from 0caff24 to ab23f17 Compare September 21, 2020 10:55

KOLANICH force-pushed the setup.cfg branch from ab23f17 to 6f72a86 Compare October 21, 2020 09:57

KOLANICH force-pushed the setup.cfg branch from 6f72a86 to 8fd8024 Compare December 23, 2020 13:16

KOLANICH force-pushed the setup.cfg branch from 8fd8024 to 62fb130 Compare January 12, 2021 16:35

sechkova reviewed Jan 20, 2021

View reviewed changes

KOLANICH force-pushed the setup.cfg branch 2 times, most recently from d1b7625 to feccb33 Compare October 20, 2022 16:25

KOLANICH force-pushed the setup.cfg branch from feccb33 to 07f94fd Compare October 31, 2022 09:51

KOLANICH force-pushed the setup.cfg branch from 07f94fd to 5106119 Compare November 7, 2022 18:23

lukpueh requested changes Nov 17, 2022

View reviewed changes

KOLANICH force-pushed the setup.cfg branch from 5106119 to 450cc84 Compare November 21, 2022 17:16

KOLANICH added 2 commits November 22, 2022 21:46

Moved the metadata into setup.cfg

b194a61

Moved the metadata from setup.cfg into PEP 621-compliant `pyproje…

a9e6b4a

…ct.toml`.

KOLANICH force-pushed the setup.cfg branch from 450cc84 to a9e6b4a Compare November 22, 2022 18:48

KOLANICH requested a review from lukpueh November 22, 2022 22:14

lukpueh approved these changes Nov 23, 2022

View reviewed changes

lukpueh merged commit fccaee8 into secure-systems-lab:master Nov 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved the metadata into setup.cfg #253

Moved the metadata into setup.cfg #253

KOLANICH commented Jul 9, 2020

coveralls commented Jul 9, 2020 •

edited

Loading

sechkova left a comment

sechkova Jan 20, 2021

KOLANICH Jan 20, 2021 •

edited

Loading

sechkova Jan 21, 2021

sechkova Jan 20, 2021

KOLANICH Jan 20, 2021

lukpueh left a comment

lukpueh Nov 17, 2022

lukpueh Nov 17, 2022

lukpueh Nov 17, 2022

jku Nov 17, 2022

lukpueh Nov 17, 2022

lukpueh Nov 17, 2022

KOLANICH commented Nov 17, 2022

lukpueh commented Nov 18, 2022

lukpueh commented Nov 22, 2022

lukpueh left a comment

		@@ -1,4 +1,4 @@
		#! /usr/bin/env python
		#! /usr/bin/env python3

Moved the metadata into setup.cfg #253

Moved the metadata into setup.cfg #253

Conversation

KOLANICH commented Jul 9, 2020

coveralls commented Jul 9, 2020 • edited Loading

sechkova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KOLANICH Jan 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KOLANICH commented Nov 17, 2022

lukpueh commented Nov 18, 2022

lukpueh commented Nov 22, 2022

lukpueh left a comment

Choose a reason for hiding this comment

coveralls commented Jul 9, 2020 •

edited

Loading

KOLANICH Jan 20, 2021 •

edited

Loading