Bug#1088314: RFS: lsm/1.0.21-2 -- Link connectivity monitor tool - transitional package

[Lucas Castro]

On 05/12/2024 12:54, Jeroen Ploemen wrote:

Control: reopen -1
Control: tags -1 moreinfo

Hi Lucas,

thanks for fixing most of the issues raised in my review. I'll
address the remaining ones below.

* changelog: revision 1.0.21-1.1 is missing, please restore.

This is still missing. If an upload makes it into the archive and you
agree with it being done, you normally want to include it even though
it wasn't prepared by you. See the developers reference, 5.11.4 [2]:
"To acknowledge an NMU, include its changes and changelog entry in
your next maintainer upload".

And it doesn't make sense not merge, btw.

Done.

I was aware about the the NMU for the source only but I wasn't aware about this requirement,

    + why the (undocumented!) bump of the versioning of the
      conflicts/replaces?

Fixed.

As a rule of thumb, every packagin change should be documented in
d/changelog - including things that may seem trivial or obvious.

As for the version bump itself, the question of 'why?' remains. The
versioning on the conflicts/replaces gets bumped to "<< 1.0.21-2", but
there are no packaging changes between 1.0.21-1 and 1.0.21-2 that
require doing so. The changelog entry tries to come up with an
explanation (good!), but unfortunately it does not make much sense.

That's why the bump it's required.

I still wait for comment about this, thanks.

I'm thinking about just mention about that when the packaging reach the stable version it'll be the package last version only that it'll be touching the upgrade/migration and not one or older before,

After all, the incompatibility is between the package designs from
before and after the renaming of the binary package from "lsm" to
"foolsm"; the versioning on the conflicts/replaces should reflect
that. And that also implies it should remain at "<< 1.0.21-1",
unless/until a future package revision introduces some other
incompatibility that demands a conflicts/replaces in its own right.

As said,  when it reached the Debian stable version, it'll not exist any '1.0.21-1' version over there. The conflicts/replace expression it'll still be true, but does it made sense?

* init script:
    + exits nonzero if the executable isn't installed anymore,
[...]

I really don't know if I understood this one, but maintscript
handle the upgrade on the transition.

My comment was about the part of the script that tests for the
presence of the program and then stops with a non-zero exit status:
---
if [ ! -x $DAEMON ]; then
   [...]
   log_end_msg 1
fi
---
The script should instead set its exit status to zero here, because
this situation (init script installed but $DAEMON missing) is
expected to happen whenever a user uninstalls the package. See Debian
policy, section 9.3.2 [1]; in particular the paragraph that starts
with "These scripts should not fail obscurely when the configuration
files remain but the package has been removed".

Indeed, true.
I had just fixed, but I'll get a little deep about sysv init script.

* rules: doesn't respect 'nodoc' build profile. See [2] for
examples to make the documentation build conditional.

nodoc had been append to build-depends field but not used as
expected,

That's because nodoc isn't the default build profile, so it wouldn't
be used unless explicitly ordered. The call to 'docbook-to-man' in
d/rules should be conditional on the build profile, like so:

But I'm going to added to d/rules.

What I meant it was that I hadn't implemented the code as you had attached below.

---
override_dh_auto_build:
ifeq (,$(filter nodoc,$(DEB_BUILD_OPTIONS)))
        docbook-to-man debian/foolsm.sgml > debian/foolsm.1
endif
         dh_auto_build -- PREFIX=${PREFIX}
---
Otherwise, building with the nodoc build profile would not install
the build-dependency on docbook-to-man, but still try to run it.


Then, one freshly introduced issue:
* there's an obvious typo ("$ start") in the init script restart code.

Well noticed.

And one that yours truly overlooked last time:
* please switch the upstream homepage in d/control to https as well.

Yep, I could get this done automatically after the d/watch.

[1]https://www.debian.org/doc/debian-policy/ch-opersys.html#writing-the-scripts
[2]https://www.debian.org/doc/manuals/developers-reference/pkgs.html#non-maintainer-uploads-nmus

I'm not going to rebuild and upload right now,




Thanks,

Lucas Castro

I'll wait for another reply because there's some topics opened, about the replace/conflicts.

OpenPGP_0x42F79A5E0A4D5598.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature