Hi Ahmad,
On Mon, 2025-01-13 at 18:24 +0000, Ahmad Khalifa wrote:
> I recently started packaging 'ntfy' and its dependency 'when'.
[...]
> I would really appreciate a review of the packaging for both ntfy/when
> and looking for a sponsor as well.
I can't help with uploading, but I have reviewed the packaging.
> Both packages are on salsa and mentors:
For future reference, you can send an email directly to the debian-go
mailing list if you need something within the Go Team reviewed and
uploaded without filing an RFS. Since you sent this email to the ML,
you've got our attention :)
> when - ITP: #1089834
> --------------------
> https://salsa.debian.org/go-team/packages/golang-github-olebedev-when
===== d/copyright =====
- You don't need every single contributor in the copyright file, but
only when an explicit copyright statement has been placed. You can
change that list out for just "2016 Oleg Lebedev <ole6edev@gmail.com>"
(email taken from [1]).
===== d/control =====
- Add the "<!nocheck>" build profile[2] constraint after the golang-
github-stretchr-testify-dev build dependency, as it is only used during
tests, and remove from Depends.
> ntfy - ITP: #1078571
> --------------------
> https://salsa.debian.org/go-team/packages/golang-github-binwiederhier-ntfy
===== d/control =====
- Since this is a program rather than a library, the source package
should be named "nfty" rather than "golang-github-binwiederhier-ntfy".
This will also make its name match the name in the ITP. I've renamed the
repo on Salsa to "nfty".
- The "nfty" binary package stanza is missing the Static-Built-Using
field. Please add "Static-Built-Using: ${misc:Static-Built-Using}" to
align with proposed changes to the Debian Policy (see [3] and the
proposed wording at [4] for more information).
- The "nfty" binary package stanza needs its own "Section" field, since
it's not a Golang library but a program intended for end-users. Please
select a section from [5].
- This is not a library that others can depend on. The "heckel-ntfy-v2-
dev" (which isn't named correctly BTW) stanza should be removed
entirely, as well as any related files like its install file.
===== d/manpages =====
- This is your choice, but maintaining manpages on your own can be a
burden. If you want, you can forward the issue and your manual page
upstream and remove it from the Debian packaging so you don't have to
worry about it.
===== d/README.Debian =====
- This is supposed to contain information for end-users, but currently
it describes stuff in the source package. It would be better suited as
d/README.source.
===== d/copyright =====
- You should clarify with upstream whether its "Apache-2.0 or GPL-2" or
"Apache-2.0 and GPL-2" since they haven't made it clear, and this is
enough grounds for a rejection from ftp-masters.
- Please license debian/* under the same license as upstream instead of
just "GPL-2" so patches etc. can be forwarded to upstream without issue.
===== d/rules =====
- Never silence commands with "@", as its useful for logging and
debugging. Please remove it.
- Instead of setting DH_GOLANG_INSTALL_ALL = 1 and then specifying
DH_GOLANG_EXCLUDES, use DH_GOLANG_INSTALL_EXTRA to specify only the
additional things required for the build to proceed.
- Maybe you could use the SOURCE_DATE_EPOCH variable
from /usr/share/dpkg/pkg-info.mk to get the date of the latest changelog
entry and use that? See [6] for an example.
- $(DH_BUILD_OPTS) is not required.
- Move the .service files out of d/systemd and directly into debian/ so
dh_installsystemd picks them up. Replace
"execute_after_dh_installsystemd" with the following to prevent starting
or enabling them.
override_dh_installsystemd
dh_installsystemd --no-start --no-enable
See the dh_installsystemd(1) manual page, accessible by web at [7], for
more information.
- You do not need to manually invoke dh_installsysusers. It is run
automatically as part of the debhelper sequence.
- override_dh_golang contradicts the "--with=golang" argument passed to
dh, and is a big no-no. Static-Built-Using must be generated as I've
already outlined before.
- The "notdebian" tag is misleading, and isn't necessary to patch out
files from the build. Use "//go:build ignore" in your patches instead.
===== d/*.service =====
- There is an opportunity to harden the service units and better secure
them. See [8] for an example, and check out the systemd.service(5)
manual page[9] for more information.
===== d/patches =====
- "TODO: forward patch upstream after further testing" is not needed as
"Forwarded: no" already implies this.
===== d/ntfy.install (and the config files overall) =====
The configuration files are automatically covered by conffiles[10], but
you may want to consider manually merging the upstream config template
with users' configuration manually in a postinst script since conffiles
will prompt the user every time the package is upgraded. See [11] for an
example on how to do this with ufw.
The configuration files won't be owned by "_ntfy" but rather by "root".
If you need to set special permissions or have the files owned by
"_ntfy", you'll need to implement your own postinst script to create the
users (replace d/ntfy.sysusers) and chmod/chown the configuration files.
--
Make sure to remove unused patches and commented-out lines once you're
done with them to ensure the package is ready for upload.
> PS, I can't access project settings on go-team, so if someone is on
> their way to salsa and feels like setting the project avatar for me,
> that would be great:
> https://salsa.debian.org/go-team/packages/golang-github-binwiederhier-ntfy/-/blob/debian/latest/docs/static/img/ntfy.png
Done.
> Also if someone fixes the default branch on when to be debian instead of
> upstream would be great2:
> https://salsa.debian.org/go-team/packages/golang-github-olebedev-when
Done.
--
Maytham Alsudany
Debian Maintainer
maytham @ OFTC
maytha8 @ Libera
[1]: https://salsa.debian.org/go-team/packages/golang-github-olebedev-when/-/blob/debian/latest/LICENSE#L189
[2]: https://wiki.debian.org/BuildProfileSpec
[3]: https://bugs.debian.org/1069256
[4]: https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=1069256;filename=0001-Require-use-of-Static-Built-Using-to-declare-statica.patch;msg=95
[5]: https://www.debian.org/doc/debian-policy/ch-archive.html#sections
[6]: https://salsa.debian.org/go-team/packages/miniflux/-/blob/debian/sid/debian/rules#L14
[7]: https://manpages.debian.org/unstable/debhelper/dh_installsystemd.1.en.html
[8]: https://salsa.debian.org/go-team/packages/miniflux/-/blob/debian/sid/debian/miniflux.service#L15-32
[9]: https://manpages.debian.org/unstable/systemd/systemd.service.5.en.html
[10]: https://www.debian.org/doc/manuals/maint-guide/dother.en.html#conffiles
[11]: https://salsa.debian.org/go-team/packages/miniflux/-/blob/debian/sid/debian/miniflux.postinst
(This example also makes use of debconf to prompt the user for
configuration details during install. It depends on your use case whether
you should make use of this to populate required fields in the
configuration files, or if the program can run without any config
out-of-the-box.)
Attachment:
signature.asc
Description: This is a digitally signed message part