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

Groups > linux.debian.maint.python > #14681 > unrolled thread

request for review: python-pyglfw

Back to article view | Back to linux.debian.maint.python

Contents

  request for review: python-pyglfw Étienne Mollier <emollier@emlwks999.eu> - 2022-12-04 12:50 +0100
    Re: request for review: python-pyglfw Timo Röhling <roehling@debian.org> - 2022-12-04 14:30 +0100
      Re: request for review: python-pyglfw Étienne Mollier <emollier@emlwks999.eu> - 2022-12-04 15:50 +0100

#14681 — request for review: python-pyglfw

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

Hi,

I'm a DD, but since this is my first DPT package, I wouldn't be
against having a second pair of eyeballs having a look at the
python-pyglfw package I produced this morning[1].  The packaging
in itself was super smooth, I just wanted to make sure I didn't
miss team specific bits; I had the policy and guide under the
eyes while packaging, but one never knows.

[1]: https://salsa.debian.org/python-team/packages/python-pyglfw

Have a nice day,  :)
-- 
Étienne Mollier <emollier@emlwks999.eu>
Fingerprint:  8f91 b227 c7d6 f2b1 948c  8236 793c f67e 8f0d 11da
Sent from /dev/pts/2, please excuse my verbosity.
On air: The Tangent - The World We Drive Through

[toc] | [next] | [standalone]

#14682

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

Hi Étienne,

* Étienne Mollier <emollier@emlwks999.eu> [2022-12-04 12:46]:
>I'm a DD, but since this is my first DPT package, I wouldn't be
>against having a second pair of eyeballs having a look at the
>python-pyglfw package I produced this morning[1].  The packaging
>in itself was super smooth, I just wanted to make sure I didn't
>miss team specific bits; I had the policy and guide under the
>eyes while packaging, but one never knows.
>
>[1]: https://salsa.debian.org/python-team/packages/python-pyglfw
- You forgot to push the upstream and pristine-tar branches, the
   upstream/2.5.5+dfsg tag, and you should set
   "debian-branch = debian/master" in d/gbp.conf

- I think the package can be arch:all, as the package will be
   identical an all architectures, with all architecture-specific
   bits hidden behind the ctypes indirection.

- The "Build-Depends: libglfw3 <!nocheck>" seems unnecessary, because
   AFAICT, there is no test suite in the package at all.

- There is no "Testsuite: autopkgtest-pkg-python" in d/control. I'm
   really not sure if this is an issue, because I usually have more
   intrusive tests and seldomly rely on the default one. Besides, you
   did add a config in d/tests, which may also suffice? I really
   don't know, but wanted to mention it just in case.

- I've never set LC_ALL in d/rules. Is there a particular reason
   why it is necessary?

- Personally, I prefer having dh-sequence-python3 in Build-Depends,
   so I don't have to add --with python3 in d/rules.

Everything else looks good to me, with the caveat that I did not
actually test-build the package, because of the missing pushes.

Oh, and welcome to the team, nice to have you here!


Cheers
Timo


-- 
⢀⣴⠾⠻⢶⣦⠀   ╭────────────────────────────────────────────────────╮
⣾⠁⢠⠒⠀⣿⡁   │ Timo Röhling                                       │
⢿⡄⠘⠷⠚⠋⠀   │ 9B03 EBB9 8300 DF97 C2B1  23BF CC8C 6BDD 1403 F4CA │
⠈⠳⣄⠀⠀⠀⠀   ╰────────────────────────────────────────────────────╯

[toc] | [prev] | [next] | [standalone]

#14683

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

Hi Timo,

Timo Röhling, on 2022-12-04:
> * Étienne Mollier <emollier@emlwks999.eu> [2022-12-04 12:46]:
> > I'm a DD, but since this is my first DPT package, I wouldn't be
> > against having a second pair of eyeballs having a look at the
> > python-pyglfw package I produced this morning[1].  The packaging
> > in itself was super smooth, I just wanted to make sure I didn't
> > miss team specific bits; I had the policy and guide under the
> > eyes while packaging, but one never knows.
> > 
> > [1]: https://salsa.debian.org/python-team/packages/python-pyglfw
> - You forgot to push the upstream and pristine-tar branches, the
>   upstream/2.5.5+dfsg tag, and you should set
>   "debian-branch = debian/master" in d/gbp.conf

Ack, these look like an oversight of mine.  These should be
fixed now.  Sorry for the mess.

> - I think the package can be arch:all, as the package will be
>   identical an all architectures, with all architecture-specific
>   bits hidden behind the ctypes indirection.

Ack, I did some further testing, and it seems to be fine to move
to arch all, unless I missed a bit.

> - The "Build-Depends: libglfw3 <!nocheck>" seems unnecessary, because
>   AFAICT, there is no test suite in the package at all.

If I remove it, the test scanner seems to catch something which
in turn fails to load the module:

	I: pybuild base:240: cd /<<PKGBUILDDIR>>/.pybuild/cpython3_3.11/build; python3.11 -m unittest discover -v 
	glfw (unittest.loader._FailedTest.glfw) ... ERROR
	
	======================================================================
	ERROR: glfw (unittest.loader._FailedTest.glfw)
	----------------------------------------------------------------------
	ImportError: Failed to import test module: glfw
	Traceback (most recent call last):
	  File "/usr/lib/python3.11/unittest/loader.py", line 440, in _find_test_path
	    package = self._get_module_from_name(name)
	              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	  File "/usr/lib/python3.11/unittest/loader.py", line 350, in _get_module_from_name
	    __import__(name)
	  File "/<<PKGBUILDDIR>>/.pybuild/cpython3_3.11/build/glfw/__init__.py", line 43, in <module>
	    raise ImportError("Failed to load GLFW3 shared library.")
	ImportError: Failed to load GLFW3 shared library.

In doubt, I'll keep the test dependency for now.  For some
reason, I still witness this error in spite of the library
installed on riscv64, but arm64 build went alright; there might
be something wrong with the dependency.

> - There is no "Testsuite: autopkgtest-pkg-python" in d/control. I'm
>   really not sure if this is an issue, because I usually have more
>   intrusive tests and seldomly rely on the default one. Besides, you
>   did add a config in d/tests, which may also suffice? I really
>   don't know, but wanted to mention it just in case.

I forcefully run it when running sbuild, so I tend to forget to
append it.  I have a more in-depth test in preparation, but it's
commented out due to #1025312.  Anyway, I added the necessary
statement to d/control, shouldn't hurt.

> - I've never set LC_ALL in d/rules. Is there a particular reason
>   why it is necessary?

That's a safety precaution for reproducible builds, when some
artifacts are locale dependent.  This may not be 100% necessary
in the present case, but I tend to keep it around just in case.

> - Personally, I prefer having dh-sequence-python3 in Build-Depends,
>   so I don't have to add --with python3 in d/rules.

I was not aware of that (or just overheard it exists), I give it
a try.  Thanks!

> Everything else looks good to me, with the caveat that I did not
> actually test-build the package, because of the missing pushes.
> 
> Oh, and welcome to the team, nice to have you here!

Thanks for your time and the warm welcome!  :)

Have a nice day,  :)
-- 
Étienne Mollier <emollier@emlwks999.eu>
Fingerprint:  8f91 b227 c7d6 f2b1 948c  8236 793c f67e 8f0d 11da
Sent from /dev/pts/2, please excuse my verbosity.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.maint.python

csiph-web