-
Notifications
You must be signed in to change notification settings - Fork 39
Update build process to current standards #84
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
base: develop
Are you sure you want to change the base?
Update build process to current standards #84
Conversation
4e5fd32
to
2cd94b3
Compare
* setup.py → pyproject.toml * use setuptools_scm to retrieve version from Git tags at build time * use importlib.metadata.version to retrieve version at run time * setuptools_scm pulls all files under version control, adapt MANIFEST.in * update years of copyright under doc
2cd94b3
to
8b80418
Compare
The CI error is:
It's as if |
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.
Many thanks for your effort. But I'm sorry, I'm not happy with the overall dircetion and inclined not to accept the change.
use setuptools_scm to retrieve version from Git tags at build time
Please note that I dropped setuptools_scm in favour of git-props in #75. I don't think, I will revert that decision.
use importlib.metadata.version to retrieve version at run time
Please see my comment on that change in doc/src/conf.py.
setuptools_scm pulls all files under version control, adapt MANIFEST.in
Yeah and that is plain silly and only one of several reasons to drop setuptools_scm. A source distribution is the collection of files needed to install the package from the sources. That is not to be confused with a snapshot of the source repository. Yes, I know that you fix that with the several exclude clauses in MANIFEST.in, but I consider it just stupid if I have to list the files that I don't want to include.
setup.py → pyproject.toml
Of course I know that the official docs recommend that. But to be honest, I see several practical disadvantages in that approach and fail to see any real benefit.
update years of copyright under doc
Obviuosly, this is sensible to do. It will certainly be done in time bevor the next release.
Homepage = "https://github.com/RKrahl/pytest-dependency" | ||
Documentation = "https://pytest-dependency.readthedocs.io" | ||
Source = "https://github.com/RKrahl/pytest-dependency.git" | ||
Download = "https://github.com/RKrahl/pytest-dependency/releases/" |
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.
The current setup.py
sets the Download
URL to "https://github.com/RKrahl/pytest-dependency/releases/%s/" % release
for a very good reason: as you can see see on the PyPI package page for release 0.6.0, it directly points to https://github.com/RKrahl/pytest-dependency/releases/0.6.0/, not to https://github.com/RKrahl/pytest-dependency/releases/ or https://github.com/RKrahl/pytest-dependency/releases/tag/latest/
This has the advantage that this link always matches the version you are looking at and remains valid also if I publish new releases.
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.
Yes, but it requires to execute code at installation time, which is to be avoided.
author = 'Rolf Krahl' | ||
|
||
# The full version, including alpha/beta/rc tags | ||
release = pytest_dependency.__version__ | ||
try: | ||
release = version(project) |
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.
This will discover the version of the currently installed version. Not the version of the package that the current document build is running from.
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.
We can write to pytest_dependency.__version__
, no problem, but need to make sure pytest_dependency.__version__
is written before building the documentation.
include tests/pytest.ini | ||
include tests/test_*.py | ||
exclude Makefile | ||
exclude python-pytest-dependency.spec |
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.
I believe it is simply weird if I need to list all the files that I don't want to include.
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.
Indeed, setuptools_scm
performs two tasks at the same time, without any clear way to disable one of them:
- retrieve version from Git tags,
- rewrite the list of files to be included in distributions based on the contents of version control system.
I'm not fond of that either.
@@ -0,0 +1,34 @@ | |||
[build-system] | |||
requires = ["setuptools>=64", "setuptools-scm"] |
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.
Installing a plain Python package is such a trivial task that I don't like to to add extra requirements just for this. setuptools (without version constraint) is an exception that I'm willing to take, but I prefer not to go beyond that.
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.
Requiring a (not so much) recent setuptools at build time is not a strong requirement. Besides, even at installation time, setuptools needs to be current. From the Python Packaging User Guide:
Ensure pip, setuptools, and wheel are up to date
While
pip
alone is sufficient to install from pre-built binary archives, up to date copies of thesetuptools
andwheel
projects are useful to ensure you can also install from source archives:
I can understand why you want to remove setuptools_scm
.
All Python packages are moving to recent versions of setuptools, that's the direction taken by the whole community. I don't understand arguments such as "I prefer not to".
"License :: OSI Approved :: Apache Software License", | ||
"Operating System :: OS Independent", | ||
"Programming Language :: Python", | ||
"Programming Language :: Python :: 3", |
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.
I prefer to explicitly list the Python version the respective release supports. It makes it easier when you browse PyPI and to answer questions like "what was the last release that did support Python X.Y?"
And, before you ask: yes before making any release I do test with all officially supported Python versions. I'm not relying on the GitHub workflow alone for that.
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.
That's what requires-python = ">=3.4"
is for. It does appear in PyPI. Why duplicate the information?
Note that in the current setup, the test suite imports |
Sure, but you cannot navigate against the direction taken by the whole community for ever. And perhaps the community as a whole knows better... For example, from a security point of view, an installation process based on an executable Python file instead of a declarative configuration file is not great. The declarative configuration file is not a silver bullet, of course, but it's a step in the right direction. |
By the way, current git-props is documented to require Python 3.6: In any case, I understand why you want to stick with git-props. That's an obstacle indeed, it might be hard to use from a declarative config file. |
Yes, this problem surfaces very often when updating the build system of a package. I must admit I don't know what the recommended / best / official ways to handle that are. I need more reading. Unless you have clues? |
Fixes #83.
I haven't updated the documentation yet, still based on
setup.py
. I'll do that once the current changes have been discussed/accepted.