RPM spec file cleanup

[davidcoutadeur]

This issue is to discuss the RPM spec file cleanup first proposed by @xavierba

Original proposed PR:

• Spec file clean up #634
• RPM spec file cleanup #817

[davidcoutadeur]

Current PR, rebased on master is #847

[davidcoutadeur]

I have reviewed the PR #847. It seems fine now.

Here is the list of breaking change in the #847 PR, which must be brought to .deb package, and notified in the changelog / in the release notes / documentation:

• the dependencies are now explicitly listed in the RPM, including the bundled ones.
• the configuration files are now in /etc/self-service-password directory
• the previous configuration files present in /usr/share/self-service-password/conf (all .php files) are migrated to /etc/self-service-password/ during the upgrade process. config.inc.php is migrated as a config.inc.php.bak file, all other php file names are preserved.
• the very old configuration files, present in /usr/share/self-service-password/ are NOT migrated during the upgrade process, and must be upgraded manually. These files have been deprecated since version 0.9, release in 2015 of October. The release note must define how to migrate these configuration files
• a default config.inc.local.php is now provided as a configuration file by the package
• smarty is now a required package See smarty4: Update to smarty4 #850
• some bundled dependencies have been updated. See smarty4: Update to smarty4 #850, bootstrap 5.3: Update bootstrap library #834, and these dependency list: Update bundled php/javascript libs versions #849
• unit tests are now run by the build process, only for fedora distribution and debian packaging
• a minimum version of PHP is now required by the package: >=7.3
• new basic dependency is now required by the package: sed
• hidden files (.gitignore,...) from bundled dependencies are now removed from the package
• now, the cache is being cleaned-up during self-service-password upgrade / install

[coudot]

We should discuss a little about providing local config file, I don't understand the need.

[davidcoutadeur]

I am not completely aligned with providing this new local config file.

There are two benefits IMO:

• self-explain to the users that the good way to configure ssp is to use the local file
• show a set of minimal values (those in local file) that must be configured before starting

However it may be a little overkill to provide this local file, which is somehow a duplicate from config.inc.php

[coudot]

Yes, it can be provided as doc, but users can choose to modify the main config file which is managed as config by the package, or create a local config to avoid comparing the changes on main config during the upgrade

[xavierba]

The idea with providing a local config file is to help users toward configuring the app way the project is instructing them to do it, that is, let the default config file unchanged and override variables in the local config file. It also helps to have a tiny number of variable to look at and possibly change, rather than a 430 lines file.

Actually, I would advocate the default config.inc.php file is not really a config file but rather the application's default and as such should not even be writable and moved out of the conf directory. Only the real config file, that is config.inc.local.php, should be in the config directory. However, this is a modification of the application's logic, not a packaging choice to be made.

[coudot]

In this case the change would be indeed to have a default value php file that will be loaded before the configuration, this indeed a change in the software.

I keep thinking it is interesting to have all configuration parameters in config.inc.php.

And I would prefer not putting config.inc.local.php in packages.

[xavierba]

Do you mean, put the config.inc.local.php file as a file explicitly added by the package itself ? In which case, I agree, this should be shipped in the git repo itself, if shipped at all.

[coudot]

I mean this file should not be shipped at all, or only in /usr/share/doc as an example

[davidcoutadeur]

This feature should be ready. Only thing missing is updating the requirement of smarty4. It can be done in #850