Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.maint.python > #17221

Re: review for datasette/0.65.2-1

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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 <jcfp@debian.org> 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.
>

Back to linux.debian.maint.python | Previous | Next — Previous in thread | Next in thread | Find similar

Thread

review for datasette/0.65.2-1 Jeroen Ploemen <jcfp@debian.org> - 2025-11-29 11:50 +0100
  Re: review for datasette/0.65.2-1 Mahangu Weerasinghe <mahangu@gmail.com> - 2025-11-30 17:10 +0100
    Re: review for datasette/0.65.2-1 Jeroen Ploemen <jcfp@debian.org> - 2025-12-01 18:40 +0100
      Re: review for datasette/0.65.2-1 Mahangu Weerasinghe <mahangu@gmail.com> - 2025-12-04 03:40 +0100
        Re: review for datasette/0.65.2-1 Jeroen Ploemen <jcfp@debian.org> - 2025-12-04 18:30 +0100
          Re: review for datasette/0.65.2-1 Mahangu Weerasinghe <mahangu@gmail.com> - 2025-12-05 08:10 +0100

csiph-web