Re: review for datasette/0.65.2-1

Mahangu Weerasinghe <[email protected]>
Newsgroups gmane.linux.debian.devel.python
Message-ID <CACe4y=9X=Uk0fXJdGKbAi=tqK9yJzd3ua9XJtYASGzyrsfHXgA@mail.gmail.com>
Hi Jeroen,
Thanks again for the thorough review! I believe I have addressed all your
feedback:

> upstream/0.65.2 tag still points to the old source import

Fixed. Tag now points to newest upstream commit (2c25896).

> vendored copy of pint gets installed into the binary pkg

Added "datasette/vendored/" to debian/clean as you suggested. Also added
execute_after_dh_auto_install to remove it from the installed package.

> control: unnecessary hardcoded python deps on binary pkg

Removed all except python3-markupsafe. Kept it explicit as you suggested
since it's not in setup.py install_requires (only a transitive dep via
Jinja2).

> control: build-deps on python3-platformdirs, -typing-extensions, -flex*
unused

Removed all four packages from Build-Depends.

> patches: remove-vendored-pint patch header contains boilerplate

Removed DEP3 template boilerplate from patch header.

> rules: '--with python3' is redundant

Removed. Using only "dh $@ --buildsystem=pybuild".

> rules: override_dh_auto_build -> execute_after_dh_auto_build

Changed to execute_after_dh_auto_build and removed redundant
dh_auto_build call.

> copyright: vendored copy of pint missing

Added Files stanza for datasette/vendored/pint/* with BSD-3-Clause license.

> lintian: unused-override application-in-library-section

Old override removed. New override added for image-file-in-usr-lib
(favicon.png is part of web UI).

> lintian: package-contains-documentation-outside-usr-share-doc,
> vcs-control-file, privacy-breach-generic

Removed docs directory from build and installed package. The docs were
copied for tests via PYBUILD_BEFORE_TEST and accidentally installed.
These are .rst source files, not built HTML - comprehensive docs are
available at docs.datasette.io.

> lintian: source-contains-prebuilt-javascript-object

Added overrides to debian/source/lintian-overrides for all five JS files,
with comment referencing debian/missing-sources/.

> Why extend-diff-ignore for setup.cfg and .eggs/?

Removed setup.cfg - confirmed it's not modified during build. Kept .eggs/
since setuptools creates it during build for pytest_runner.

> Why include-binaries for tests/spatialite.db?

Removed. I originally added it preemptively, but since spatialite.db is in
the orig tarball and not modified by any patch, it looks like we don't need
to do this. Thanks!

Also fixed a Salsa i386 build issue: the test suite was exhausting thread
limits
due to 32-bit address space constraints. Added ulimit -s 1024 for i386
builds
to reduce per-thread stack size.

Ready for re-review. Looking forward to your feedback!

Best,
Mahangu

On Mon, Dec 1, 2025 at 11:07 PM Jeroen Ploemen <[email protected]> wrote:

> Hi Mahangu,
>
> > Tested in clean chroot and Salsa CI also now passing. Ready for
> > re-review.
> >
> > Note: I made additional changes to get CI green (test configuration
> > via PYBUILD_TEST_ARGS/PYBUILD_BEFORE_TEST, pristine-tar,
> > Build-Depends adjustments). I'm still learning Debian packaging, so
> > I'd appreciate any feedback if these aren't the right approaches.
>
> thanks for all those fixes, much better now. Don't worry if there's
> a lot of feedback, you didn't pick the easiest package to start out
> with packaging from scratch.
>
> Some issues remaining:
>
> * repo: upstream/0.65.2 tag still points to the old source import,
>   should be moved to the newest one; see
>
> https://salsa.debian.org/python-team/packages/datasette/-/commits/upstream
>
> * vendored copy of pint gets installed into the binary pkg; you might
>   want to add an entry to d/clean (datasette/vendored/*) to ensure it
>   cannot be used or installed by accident.
>
> * control: there's still a load of unnecessary hardcoded python deps
>   on binary pkg; you probably only need to keep markupsafe (or patch
>   setup.py so it's picked up by dh_python/pybuild); everything else
>   looks like it would be provided via ${python3:Depends}.
>
> * control: build-deps on python3-platformdirs, -typing-extensions,
>   and -flex* were needed only for the vendored pint; with that out of
>   the picture, these deps are unused and should be removed.
>
> * patches: remove-vendored-pint patch header contains boilerplate
>   dep3 template.
>
> * rules: '--with python3' is redundant when build-depending on
>   dh-sequence-python3.
>
> * rules: override_dh_auto_build -> execute_after_dh_auto_build?
>
> * copyright: the vendored copy of pint is still missing here. It must
>   be included even if it's not used or installed, simply because it's
>   shippped in the orig tarball.
>
> * some lintian hits need attention, particularly: (from
>   https://salsa.debian.org/python-team/packages/datasette/-/jobs/8674686)
>   I: datasette: unused-override application-in-library-section python
> [usr/bin/datasette] [usr/share/lintian/overrides/datasette:4]
>     --> section already changed to 'database'
>   I: datasette: package-contains-documentation-outside-usr-share-doc
> [usr/lib/python3/dist-packages/docs/*]
>     --> these typically go in /usr/share/doc/<pkgname>
>   W: datasette: package-contains-vcs-control-file
> [usr/lib/python3/dist-packages/docs/.gitignore]
>   W: datasette: privacy-breach-generic [<script defer data-domain="
> docs.datasette.io" src="https://plausible.io/js/plausible.js">] (
> https://plausible.io/js/plausible.js) [...]
>     --> this appears to be an analytics script, simply patching it out
> should be fine
>
> * these lintian hits on the other hand deserve an override, given
>   that the non-minified versions are in d/missing-sources:
>   P: datasette source: source-contains-prebuilt-javascript-object
> [datasette/static/cm-resize-1.0.1.min.js]
>   P: datasette source: source-contains-prebuilt-javascript-object
> [datasette/static/codemirror-5.57.0-sql.min.js]
>   P: datasette source: source-contains-prebuilt-javascript-object
> [datasette/static/codemirror-5.57.0.min.js]
>   P: datasette source: source-contains-prebuilt-javascript-object
> [datasette/static/sql-formatter-2.3.3.min.js]
>   P: datasette source: source-contains-prebuilt-javascript-object
> [datasette/static/table.js]
>
> * why d/source/options extend-diff-ignore = "^setup\.cfg$" - does
>   this get modifed somehow? Same question for
>   d/source/include-binaries tests/spatialite.db?
>
>
> As always, once the above comments have been addressed, simply re-add
> the package to the IRC channel topic and/or ping me by e-mail.
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.