build: Port from MANIFEST.in to setuptools_scm #662
Conversation
|
Rebased to fix a merge conflict on |
|
Other changes in this branch (apart from moving to My Python packaging knowledge dates from the The diff between the whl generated on $ diff -ur ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin ./kolibri_explore_plugin-6.20.1.dev*-py3-none-any/kolibri_explore_plugin
Only in ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin: apps
Only in ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin: assets
Only in ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin: buildConfig.js
diff -ur ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/__init__.py ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/__init__.py
--- ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/__init__.py 2023-06-23 12:34:40.000000000 +0100
+++ ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/__init__.py 2023-06-23 14:27:12.000000000 +0100
@@ -1,3 +1 @@
-__version__ = "6.20.0"
-
default_app_config = "kolibri_explore_plugin.apps_config.ExploreConfig"
diff -ur ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/static/build-info.json ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/static/build-info.json
--- ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/static/build-info.json 2023-06-23 12:35:22.000000000 +0100
+++ ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/static/build-info.json 2023-06-23 14:27:58.000000000 +0100
@@ -1,7 +1,14 @@
{
- "commit": "9c64d2a",
- "date": "Thu Jun 22 22:43:21 2023",
+ "commit": "a8db71e",
+ "date": "Fri Jun 23 14:27:13 2023",
"version_name": "Ladybird",
"last_release": "v6.20.0",
"log": [
+{ "commit": "a8db71e", "subject": "scripts: Fix a small typo in bump_version.py", "author": "Philip Withnall" },
+{ "commit": "c075848", "subject": "docs: Fix a small typo in README.md", "author": "Philip Withnall" },
+{ "commit": "b9f1b24", "subject": "gitignore: Allow collections to be a symlink", "author": "Philip Withnall" },
+{ "commit": "d38143a", "subject": "build: Port from setup.py to pyproject.toml", "author": "Philip Withnall" },
+{ "commit": "e6584ee", "subject": "build: Fix author e-mail address", "author": "Philip Withnall" },
+{ "commit": "f2d92ec", "subject": "build: Generate package version number automatically from git", "author": "Philip Withnall" },
+{ "commit": "a306c19", "subject": "build: Port from MANIFEST.in to setuptools_scm", "author": "Philip Withnall" },
{ "commit": "00e431e", "subject": "footer: Correctly hide store buttons in apps", "author": "Will Thompson" }]}
Only in ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin: test
diff -ur ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/urls.py ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/urls.py
--- ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/urls.py 2023-06-23 12:34:40.000000000 +0100
+++ ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/urls.py 2023-06-23 14:27:12.000000000 +0100
@@ -2,7 +2,7 @@
from django.conf.urls import url
-from . import __version__ as VERSION
+from ._version import __version__ as VERSION
from .views import AppFileView
from .views import AppMetadataView
from .views import ExploreView
diff -ur ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/_version.py ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/_version.py
--- ./kolibri_explore_plugin-6.20.0-py2.py3-none-any/kolibri_explore_plugin/_version.py 2023-06-23 12:33:40.000000000 +0100
+++ ./kolibri_explore_plugin-6.20.1.dev9+ga8db71e-py3-none-any/kolibri_explore_plugin/_version.py 2023-06-23 14:28:06.000000000 +0100
@@ -1,4 +1,4 @@
# file generated by setuptools_scm
# don't change, don't track in version control
-__version__ = version = '6.20.1.dev7+gfea7109.d20230623'
-__version_tuple__ = version_tuple = (6, 20, 1, 'dev7', 'gfea7109.d20230623')
+__version__ = version = '6.20.1.dev9+ga8db71e'
+__version_tuple__ = version_tuple = (6, 20, 1, 'dev9', 'ga8db71e') |
These files/directories are now included in the wheel, whereas they weren’t previously. It seems reasonable to include them, since they’re checked in to git. |
|
Well that set of CI results is a complete disaster. More work obviously needed. |
|
Looks like |
There was a problem hiding this comment.
To me this is all great; I just had a couple questions. I'd like @manuq to take a look since he's put together most if not all of the existing packaging.
I’ve pushed a new |
pwithnall
force-pushed
the
647-manifest
branch
2 times, most recently
from
a420b07 to
c1ceca5
Compare
|
I think I might have something close to working here, but I think I want to spend some time tomorrow working out how to rationalise the dependency lists for the project. There’s about 5 different partial dependency lists in different places, and that’s going to lead to confusion. |
|
OK, I’ve tidied up the commits a bit and added some additional commits to consolidate on |
Thanks for the review! @dbnicholson, do you have any other comments? Otherwise I’ll go ahead and merge and see what breaks |
|
@dbnicholson any chance you could take another look? @pwithnall said he'll resolve the conflict just before merging (because the conflict will just be re-introduced in the meantime if anyone makes another release, as we just did during the platform team call) |
Sorry, this fell off my radar. Reviewing now. And since @pwithnall is done for the week, I can fixup the merge if all looks good. |
|
|
||
| [packages] | ||
| nodeenv = "==1.3.3" | ||
| ipdb = "==0.13.2" | ||
| requests = "==2.31.0" |
There was a problem hiding this comment.
Technically, all of these are dev-packages since none of this is installed at runtime. Even the requests used at runtime really comes from kolibri's vendored version. Anything required at runtime needs to be specified in install_requires to setuptools. This Pipfile is really a convenience for developers, so in a sense packages is correct for everything since development is the only time you'd use it. That said, I won't get hung up on the split.
| if: steps.cache-pipenv.outputs.cache-hit != 'true' | ||
| run: | | ||
| pipenv install --dev | ||
|
|
||
| - name: Set up Node | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| # Should be same version as in scripts/bootstrap.sh | ||
| node-version: '16.14.0' |
There was a problem hiding this comment.
Since this workflow now requires pipenv, arguably we could drop this and change the above pipenv install --dev to just run ./scripts/bootstrap.sh. Then node, yarn and the node dependencies are installed consistently without any duplication here. Actually, all the workflows should probably be changed to run bootstrap.sh so that all dev & test environments are consistent. Maybe another day.
| pre-commit==1.15.1 | ||
| twine | ||
| bumpversion | ||
| wheel |
There was a problem hiding this comment.
I think the idea was that requirements.txt would be synthesized from Pipfile.lock whenever it was updated. That way someone can install the dependencies with pip rather than requiring use of pipenv. That's more useful for a project where the installation process is to clone a git repo or fetch a released archive. For a library installed from PyPI it's less useful as these requirements won't be used by pip and developers can be expected to use pipenv.
Using `setuptools_scm` allows us to reliably ensure that all files in git are in the sdist, which avoids problems caused by occasionally forgetting to update `MANIFEST.in` when adding a new directory. `MANIFEST.in` still needs to be kept (in a reduced form) because there are a few files needed by webpack and by kolibri-installer-android which are only created at dist time, and exist outside setuptools. References: - https://github.com/pypa/setuptools_scm#readme - https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/ - pypa/setuptools-scm#190 - https://packaging.python.org/en/latest/guides/using-manifest-in/ Signed-off-by: Philip Withnall <pwithnall@endlessos.org> #647
`setuptools_scm` can do this for us, so that makes things a little easier to maintain. It now generates `_version.py` for us automatically. The `__version__` symbol from that file still has to be imported into `kolibri_explore_plugin/__init__.py` so that it’s re-exported from the `kolibri_explore_plugin` module. Although none of the code in this repository needs it, the `webpack_json.py` code from kolibri.git automatically looks at the `__version__` symbol of all the plugin modules it loads, and errors if it’s not present. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This means that the project should now be dist-built using `python -m build` rather than `python setup.py sdist` (although actually since we’re using yarn, just continue to dist it using `yarn build-dist`). Moving from `setup.py` to `pyproject.toml` means the project metadata is now described declaratively rather than procedurally, which allows multiple tools to access it without parsing or running Python code. References: - https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html - https://pypa-build.readthedocs.io/en/stable/ - https://packaging.python.org/en/latest/tutorials/packaging-projects/ - https://packaging.python.org/en/latest/specifications/declaring-project-metadata/#declaring-project-metadata - https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html - https://setuptools.pypa.io/en/latest/userguide/datafiles.html Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This allows developers to symlink it to a local checkout of the endless-key-collections repository. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Because they are only needed for development, not at runtime. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
kolibri-tools needs to be able to load the `kolibri_explore_plugin` Python module at build time, as part of building the frontend. If the `_version.py` for it has not been generated by then, this will fail. Manually invoke `setuptools_scm` to generate `_version.py`. This requires having `setuptools_scm` installed first. Signed-off-by: Philip Withnall <pwithnall@endlessos.org> #647
Otherwise the version number for the project can’t be generated. This necessary also means running the build steps under pipenv, so the dependencies are available in the build environment. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
They’re listed in pyproject.toml, but not in the Pipfile. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
All the packages listed in `requirements.txt` were already in the Pipfile. The version for `pre-commit` didn’t match, but it appears from commit c2d030f that it was up to date in the `Pipfile` but not `requirements.txt`. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
It seems to be completely unused. If we add tests in future, the test dependencies can be added to `[dev-packages]` in the Pipfile. Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Using
setuptools_scmallows us to reliably ensure that all files ingit are in the sdist, which avoids problems caused by occasionally
forgetting to update
MANIFEST.inwhen adding a new directory.MANIFEST.instill needs to be kept (in a reduced form) because thereare a few files needed by webpack and by kolibri-installer-android which
are only created at dist time, and exist outside setuptools.
References:
Signed-off-by: Philip Withnall pwithnall@endlessos.org
Fixes #647