Remove Sys V Init support from RPM distribution.

[JohanSaaw]

Fedora. RedHat and SUSE moved from Sys V Init to systemd in 2011 (Fedora
v. 15) and 2014 (RHEL7 and openSUSE/SLES 12) respectively.

The LMS RPM package has supported Sys V Init and systemd in parallel since
v. 8.2. The RPM package installs the systemd support by default if the OS
use systemd (users can of course change this after installation, but why
would they?).

I think it is now time to remove the support for Sys V Init. This mainly
means removing the Sys V Init script and some logic in the RPM spec file.

The file /etc/sysconfig/lyrionmusicserver was used to pass basic start-up
configuration to the Sys V Init script. This file has now been replaced
with a stub file pointing to a file explaining how to change the start-up
configuration using systemd drop in files for the systemd unit.

Further changes:

• a small fix to the logrotate configuration.
• added a dependency on systemd in the RPM spec file (%systemd_requires)

[JohanSaaw]

Hi,

I have a further question. The RPM sepc file has a require statement for PERL to be v 5.10 or higher. I have seen the change to remove PERL binaries for versions 5.20 and earlier.

Should I also bump the spec file require statement to require PERL version 5.22 or higher?

Regards, Johan

[michaelherger]

Thanks for this PR! As you know I'm no expert in this area...

@mavit - would you mind sharing your review thoughts of this suggested change?

[michaelherger]

typo in comment, change on purpose in the process name?

[JohanSaaw]

Hi,

Sorry for the typo. I'll fix that straight away.

The change in the process name is the main error that I fixed in that file. The pgrep command uses the string in the /proc/[pid]/stat file to find the PID. In that file only the first 15 characters of the command is used. If you pass longer strings to pgrep it will not find anything, unless you pass the string with the -f flag (and I have seen that this can lead to other unwanted side effects).

If you look at the logrotate file in the Debian package you will see the same there. They have also shortened squeezeboxserver.

But it is only good if someone else has a look at this too. Also maybe at the question about the required version of PERL in the RPM spec file.

Regards, Johan

[mavit]

Should I also bump the spec file require statement to require PERL version 5.22 or higher?

Seems sensible. We should probably also mention the maximum supported version, so:

Requires: (perl >= 5.22 and perl < 5.43)

For maximum accuracy, we could even:

Requires: (perl(:MODULE_COMPAT_5.22.0) or \
           perl(:MODULE_COMPAT_5.24.0) or \
           perl(:MODULE_COMPAT_5.26.0) or \
           perl(:MODULE_COMPAT_5.28.0) or \
           perl(:MODULE_COMPAT_5.30.0) or \
           perl(:MODULE_COMPAT_5.32.0) or \
           perl(:MODULE_COMPAT_5.34.0) or \
           perl(:MODULE_COMPAT_5.36.0) or \
           perl(:MODULE_COMPAT_5.38.0) or \
           perl(:MODULE_COMPAT_5.40.0) or \
           perl(:MODULE_COMPAT_5.42.0))

[mavit]

Now that we know that we have systemd, how about the following, to deliver the signal without guessing the process from its name?

Suggested change

	/bin/kill -USR1 `pgrep lyrionmusicser 2>&1` >/dev/null 2>&1 || true
	systemctl --kill-whom=main --signal=USR1 kill lyrionmusicserver.service

[JohanSaaw]

@mavit ,

Thanks for this suggestion. I was looking at this possibility too, but in the end decided against it, mostly because I did not want to change too much. But as you think this is better, then I'll add this. I would just like to do one small change and that is to use the full path to systemctl:

/usr/bin/systemctl --kill-whom=main --signal=USR1 kill lyrionmusicserver.service

Do you have any objections?

Regards, Johan

[mavit]

Looks good to me.

[JohanSaaw]

Done.

[mavit]

We need the following to ensure we have the %{_unitdir} macro:

BuildRequires: systemd-rpm-macros

https://docs.fedoraproject.org/en-US/packaging-guidelines/Systemd/#packaging_filesystem

[JohanSaaw]

@mavit,

OK, will do this. I guess I was lucky that it worked on my server without that macro. I will let you all know when I have implemented and tested this.

Regards, Johan

[mavit]

The idea of unconditionally enabling the service when the package is upgraded, even if the user has previously disabled it, makes me uneasy. Could we use presets and scriptlets instead?

https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_systemd

[JohanSaaw]

@mavit,

Yes I know this is strange, but it has always been like this, even before systemd support was introduced. The RPM scripts enabled and started the service. Please allow me some time to read up on the guide lines, and then I'll update the respective scripts.

Regards, Johan

[JohanSaaw]

Should I also bump the spec file require statement to require PERL version 5.22 or higher?

Seems sensible. We should probably also mention the maximum supported version, so:

Requires: (perl >= 5.22 and perl < 5.43)

For maximum accuracy, we could even:

Requires: (perl(:MODULE_COMPAT_5.22.0) or \
           perl(:MODULE_COMPAT_5.24.0) or \
           perl(:MODULE_COMPAT_5.26.0) or \
           perl(:MODULE_COMPAT_5.28.0) or \
           perl(:MODULE_COMPAT_5.30.0) or \
           perl(:MODULE_COMPAT_5.32.0) or \
           perl(:MODULE_COMPAT_5.34.0) or \
           perl(:MODULE_COMPAT_5.36.0) or \
           perl(:MODULE_COMPAT_5.38.0) or \
           perl(:MODULE_COMPAT_5.40.0) or \
           perl(:MODULE_COMPAT_5.42.0))

@mavit,

I had a look at the Require statement and I have done some tests. I ran into problems so I had to read up a bit before I got it working.

According to

https://rpm-software-management.github.io/rpm/manual/dependencies.html

"Requires" lines can't be continued over several lines (the short paragraph just above the "Conflicts" section). If your proposal with the MODULE_COMPAT_X.XX.X statements is rewritten into a single long line, then it works.

As such a long "Requires" line is not really readable, I would prefer to use your other proposal:

Requires: (perl >= 5.22 and perl < 5.43)

This format works on SUSE based distributions but not on Fedora/RedHat based distributions. Instead the format:

Requires: (perl(:VERSION) >= 5.22 and perl(:VERSION) < 5.43)

must be used.

There is, however, one more snag. Boolean conditions in Requires statements are only allowed in RPM 4.13 and above. CentOS 7, RHEL 7, Fedora 26 and older use older versions of RPM. In the SUSE world, openSUSE and SLES before 15.0 used older versions of RPM.

I don't know whether this is only a theoretical issue, i.e. if anyone runs LMS on such old versions of RedHat/SUSE distributions, or not. If it actually is a real issue, then it can be worked around like this in the spec file:

Requires: Perl >=5.22
Conflicts: Perl >=5.43

What do you think?

Regards, Johan

[JohanSaaw]

@mavit , @michaelherger ,

I have committed changes to my fork now that addresses the points raised by mavit.

• /etc/logrotate.d/lyrionmusicserver.conf
  Added the suggestion made by mavit with the small difference to use the full path for the systemctl command.

• Added BuildRequires: systemd-rpm-macros

• perl version requirements
  This was more difficult than I thought it would be. This is mostly due to the fact that Fedora/RedHat and SUSE handles version capabilities of perl differently.

The statement

(perl >= 5.22 and perl < 5.43)

Will only work on SUSE flavours

(perl(:VERSION) >= 5.22 and perl(:VERSION) < 5.43)

Will only work on Fedora/RedHat flavours.

This one
Requires: perl >= 5.22
Conflicts: perl >= 5.43

Does not work on Fedora/RedHat

If we really want to have a perl requirement for both larger or equal to 5.22 and less than 5.43 we need to use the long statement given by mavit here above. But it has to be re-written in a single line as "Requires:" statements can't be written on several lines with line continuation characters. So I ended up with:

Requires: (perl(:MODULE_COMPAT_5.22.0) or perl(:MODULE_COMPAT_5.24.0) or perl(:MODULE_COMPAT_5.26.0) or perl(:MODULE_COMPAT_5.28.0) or perl(:MODULE_COMPAT_5.30.0) or perl(:MODULE_COMPAT_5.32.0) or perl(:MODULE_COMPAT_5.34.0) or perl(:MODULE_COMPAT_5.36.0) or perl(:MODULE_COMPAT_5.38.0) or perl(:MODULE_COMPAT_5.40.0) or perl(:MODULE_COMPAT_5.42.0))

I should mention one more thing, it is only possible to use boolean logic in Requires statements on RPM 4.13 and newer. I think there is an overlap, at least in the RedHat world with version using Perl 5.22 and RPM versions lower than 4.13. I don't know how much of a problem this would be.

• Added systemd_*, macros in the respective RPM scriptlets.
• Added a systemd preset file to make sure that lyrionmusicserver is enabled at install.
• Added logic to start the lyrionmusicserver during install IF IT IS A FIRST TIME INSTALL

During upgrades the service will remain disabled if it was disabled before the upgrade started. Also if the service was stopped before the upgrade, it will remain stopped after the upgrade. And of course if the service was enabled and running before the upgrade it will remain that way after the upgrade.

I have tested this on openSUSE Leap, openSUSE tumbleweed, Rocky 10 and Fedora 43.

Please let me know if you have any questions or remarks.

Regards, Johan

[JohanSaaw]

I am really sorry, I have to retract what I said about the "Requires" statement for perl here above.

I did some more testing on older versions of SUSE and the "Requires" statement I chose does not work on openSUSE 15.6.

I am afraid I need to do more testing.

Regards, Johan

[JohanSaaw]

@mavit , @michaelherger

I have done some more testing and I can conclude that some of my testing was made more difficult than necessary because I am old. When I sit with a Linux command line in front of me, I instinctively write "rpm -ivh ..." when I want to install anything, rather than "zypper in ..." or "dnf install ..." respectively.

Another complication is that SUSE distros and RedHat based distros have chosen different methods to

Anyway, I have found that this Requires statement work on opensuse Leap 15.X, opensuse Leap 15, opensuse Tumbleweed, Rocky, v 8, 9, 10 and Fedora 43

Requires: ((perl >= 5.22 or perl(:VERSION) >= 5.22) with (perl < 5.43 or perl(:VERSION) < 5.43))

This works irrespectively of whether rpm or zypper/dnf is used to install the packages.
The disadvantage is that it will only work on OS versions using RPM v 4.14 and higher (the "with" operator was only introduced in RPM v. 4.14). This means that it will no longer be possible to install the package on Rocky, Alma and CentOS 7. Fedora started using RPM 4.14 with v. 27.

It will still work on older SUSE distributions (with earlier RPM versions) , but only when using zypper in that case.

Please also note that the current LMS 9.1 package can only be installed on Rocky 8 with dnf and not with RPM. Installing it with dnf will pull in 113 perl packages that are not really necessary.

I have committed the corresponding changes to my fork.

I am now finally confident that I found a working solution and would appreciate your feedback.

Regards, Johan

[JohanSaaw]

@mavit ,

Thanks, accepted that correction.

Regards, Johan

[mavit]

Please also note that the current LMS 9.1 package can only be installed on Rocky 8 with dnf and not with RPM. Installing it with dnf will pull in 113 perl packages that are not really necessary.

Ah, that's a good point. On Fedora and derivatives, by switching from requiring perl to perl(:VERSION), we effectively switch from pulling in the entire Perl core to just the Perl interpreter. Have you checked that we didn't need any of those core Perl modules?

If you BuildRequires: (perl-generators if perl(:VERSION)), does it magically work out all the module dependencies for us? It's supposed to do that, but I expect it will get horribly confused by our bundled CPAN modules.

It might be simplest to add an unversioned Requires: perl in addition to the versioned requires.

[mavit]

This means that it will no longer be possible to install the package on Rocky, Alma and CentOS 7

RHEL 7 came with Perl 5.16, so, given that we require Perl 5.22, making the package uninstallable for some other reason too seems fine.

[JohanSaaw]

Please also note that the current LMS 9.1 package can only be installed on Rocky 8 with dnf and not with RPM. Installing it with dnf will pull in 113 perl packages that are not really necessary.

Ah, that's a good point. On Fedora and derivatives, by switching from requiring perl to perl(:VERSION), we effectively switch from pulling in the entire Perl core to just the Perl interpreter. Have you checked that we didn't need any of those core Perl modules?

If you BuildRequires: (perl-generators if perl(:VERSION)), does it magically work out all the module dependencies for us? It's supposed to do that, but I expect it will get horribly confused by our bundled CPAN modules.

It might be simplest to add an unversioned Requires: perl in addition to the versioned requires.

I will create some clean Rocky/Alma installations and run tests again. I'll come back with my results.

Regards, Johan

[JohanSaaw]

@mavit,

You are right on Rocky/Alma 8 and 9 there are missing modules after an installation and the LMS did not start as it should.

On Rocky/Alma 10 and Fedora 40 and newer, they have added a meta-package "perl". The package description says: "This is a metapackage with all the Perl bits and core modules that can be found in the upstream tarball from perl.org."

So on Rocky/ALMA 10 and Fedora 40 and newer we don't have any issues.

I tested with BuildRequires: (perl-generators if perl(:VERSION)). On my SUSE installation it didn't do anything at all. On a Fedora installation it just generated an error and my build was interrupted.

I should probably explain that I don't have a proper build system. I just build using the buildme.pl script from the platforms repository (which of course just uses the rpmbuild command).

I then added a plain Requires: perl. This will solve the issues on Rocky/Alma 8 and 9, but it will require that the package is installed with dnf and not with rpm. On v. 8 113 additional Perl packages are installed and on v. 9 an additional 191 packages are installed.

I have committed a new version of the spec file.

I assume ideally we should actually have requires for the Perl Modules we actually need, rather than have this solution.

Regards, Johan

[mavit]

I assume ideally we should actually have requires for the Perl Modules we actually need, rather than have this solution.

That would be ideal from the point of view of shrinking the installation size. It has the minor drawback that someone might have to install additional dependencies later if they install a plugin that requires something from core that we didn’t expect.

If we can't use perl-generators to do it for us automatically, it also, of course, has the drawback of being work for someone to create and maintain. Given a choice, I think I’d rather see any effort in this area go into, e.g., LMS-Community/lms-community.github.io#126 (comment) (which I still hope to find time to have a proper look at).

[JohanSaaw]

@mavit ,

I made some further tests.

All RedHat flavours distros will need the

"Requires: perl"

statement in the RPM package. Installing the lyrion package with dnf will then pull in the perl core packages.

On SUSE it seems that the packages perl and perl-base are installed by default when a "normal" server installation is done. Otherwise they would be pulled in by the

Requires: ((perl >= 5.22 or perl(:VERSION) >= 5.22) with (perl < 5.43 or perl(:VERSION) < 5.43))

in the RPM.

I had a look at which perl modules from the OS are mandatory to get lyrion up and running. Adding the following in the RPM

Requires: perl(IO::Socket::SSL)
Requires: perl(strict)
Requires: perl(Config)
Requires: perl(Socket)
Requires: perl(FindBin)
Requires: perl(lib)
Requires: perl(Getopt::Long)
Requires: perl(File::Path)
Requires: perl(File::Copy)
Requires: perl(File::Find)
Requires: perl(POSIX)
Requires: perl(Time::HiRes)
Requires: perl(locale)
Requires: perl(DynaLoader)
Requires: perl(Sys::Hostname)
Requires: perl(Devel::Peek)
Requires: perl(I18N::LangTags)
Requires: perl(subs)
Requires: perl(Compress::Raw::Zlib)
Requires: perl(Digest::SHA)

will make the "Requires: perl" statement unnecessary for the RedHat flavours. Please note that some of the above like "Requires perl(Config)" will be present on the RedHat installations anyway, and are thus not strictly necessary in the RPM.

With the above requires statements the Lyrion server starts, I can play music from it. The music scan works, I can install plugins etc. But I can't guarantee that I have missed any necessary modules.

I think it would be bettwer to go for the combination of:

Requires: ((perl >= 5.22 or perl(:VERSION) >= 5.22) with (perl < 5.43 or perl(:VERSION) < 5.43))
"Requires: perl"

If we want to have a lower limit of Perl as well as a higher limit. Otherwise we can just revert to using a single

Requires: perl >= 5.22

What do you think.

Regards, Johan

[mavit]

I think we should defer to @michaelherger, here.

We have a question about how many other RPMs should be pulled in when installing the lyrionmusicserver package. Do you think all of the core Perl modules core should always be pulled in, so that plugins can always use any of these modules? Or do you think we should shrink the total installation size by only pulling in modules that we know we need?

On an empty Fedora container, the saving is 370MB of disk space (although in the real world, that will often be lower, since some of these modules may be pulled in by other things anyway).

[mavit]

Another option just occurred to me:

Requires: perl(Compress::Raw::Zlib)
Requires: perl(Config)
Requires: perl(Devel::Peek)
Requires: perl(Digest::SHA)
Requires: perl(DynaLoader)
Requires: perl(File::Copy)
Requires: perl(File::Find)
Requires: perl(File::Path)
Requires: perl(FindBin)
Requires: perl(Getopt::Long)
Requires: perl(I18N::LangTags)
Requires: perl(IO::Socket::SSL)
Requires: perl(POSIX)
Requires: perl(Socket)
Requires: perl(Sys::Hostname)
Requires: perl(Time::HiRes)
Requires: perl(lib)
Requires: perl(locale)
Requires: perl(strict)
Requires: perl(subs)
Recommends: perl

By Recommendsing perl, users get all the core modules by default, but can uninstall them if they prefer to save space.

[JohanSaaw]

Hi,

Yes that would probably be a good compromise. It would also safeguard against changes in the packaging on SUSE.

Regards, JOhan