Bug 241619 - net-im/prosody: update to 0.11.3 + lua 5.2
Summary: net-im/prosody: update to 0.11.3 + lua 5.2
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: David Thiel
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-31 17:29 UTC by Thomas Morper
Modified: 2019-11-20 23:38 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (lx)


Attachments
svn diff to update net-im/prosody to 0.11.3 + lua 5.2 (6.67 KB, patch)
2019-10-31 17:29 UTC, Thomas Morper
no flags Details | Diff
Poudriere build log (25.68 KB, text/plain)
2019-11-01 06:37 UTC, David Thiel
no flags Details
svn diff, corrected version (8.62 KB, patch)
2019-11-04 02:01 UTC, Thomas Morper
no flags Details | Diff
svn diff, including new pid file location (9.10 KB, patch)
2019-11-20 23:38 UTC, Thomas Morper
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Morper 2019-10-31 17:29:49 UTC
Created attachment 208743 [details]
svn diff to update net-im/prosody to 0.11.3 + lua 5.2

I'd like to propose some changes to the prosody port, all contained in the attached patch:

1) Update Prosody to 0.11.3.

2) Drop LuaJIT and switch to Lua 5.2. LuaJIT is effectively dead, and though still working with Lua 5.1 Prosody's official-as-documented requirement is Lua 5.2.

3) Use arc4random.

4) Do not install the example certs because absolutely noone should use them.

5) Store user data in /var/db/prosody rather than /usr/local/var/lib/prosody. This requires moving an existing folder and I'm not sure about the right way to do this... is there a reliable way to do this automatically or would a pkg-install message be sufficient?

6) Install prosody-migrator (PR#235189). Do not add luadbi to RUN_DEPENDS though because it would pull in about 150MB of dependencies for this optional feature.

7) Update the configure-script switches. Explicitly state all required settings and do not use the "ostype" switch because some settings will be wrong then.

The patched port will build without warnings in poudriere and the resulting package has been working without any problems on my little Prosody server.
Comment 1 David Thiel freebsd_committer 2019-11-01 06:36:15 UTC
Thanks! Couple notes:

1) What's the reasoning behind arc4random change?

2) Is there a reason to use /var/db/ for the pid instead of /var/run?

3) There are some errors when building via poudriere. Log attached.
Comment 2 David Thiel freebsd_committer 2019-11-01 06:37:04 UTC
Created attachment 208756 [details]
Poudriere build log
Comment 3 Thomas Morper 2019-11-04 01:59:56 UTC
1) I had no intention for switching to arc4random, but I discovered this option while looking for something else, and it seems to be a sensible choice. arc4random is supposed to give us secure, performant, non-blocking randomness, and since we're on BSD we get it for free - not even a linker flag needed. So I guess there's mostly no reason for not using arc4random.

2) The default location for the pid file is the data directory which got moved from /usr/local/var/lib/prosody to the more fitting /var/db/prosody meaning the pid file ended up there as well. I agree that /var/run would be a better location for the pid file, but this needs more changes and checks and I didn't want to adress this issue in this round.

3) Sorry, I forgot to "svn add" a file and messed up the diff. Corrected version attached. Thanks for checking!
Comment 4 Thomas Morper 2019-11-04 02:01:07 UTC
Created attachment 208837 [details]
svn diff, corrected version
Comment 5 David Thiel freebsd_committer 2019-11-12 06:16:23 UTC
Ok, looks like that works. For the pid, I'm fine with it in either location, but I think it's best we decide now, otherwise people might have to change their configs twice. For whatever reason, in my production server it's specified in the config file.

I'm proposing the upgrade steps as:

1) service prosody stop
2) pkg upgrade prosody (or cd /usr/ports/net-im/prosody && make deinstall && make install)
3) mv /usr/local/var/lib/prosody/* /var/db/prosody/
4) If "pidfile" is specified in prosody.cfg.lua, change its new location to /var/db/prosody/prosody.pid (or remove the line)
5) service prosody start


We could technically do step 3 automatically on install, but I'm worried about yanking a db out from under a running process.
Comment 6 Thomas Morper 2019-11-20 23:37:18 UTC
Finally I had a look at the pid file...

By default Prosody doesn't write a pid file. However, a pid file needs to be defined via the "pidfile" config option or prosodyctl won't work. Without a leading path the pid file will be placed in the data directory (aka /var/db/prosody), otherwise in the defined location which needs write permissions for the prosody user.

So, I've updated the patch to create /var/run/prosody with the required permissions and use /var/run/prosody/prosody.pid as the default location in the rc script. This should match the users' expectations.

While I was at it I simplified the rc script. Grepping for an explicit "daemonize=false" in the config, then daemonizing via "daemon" rather than "prosodyctl" just isn't right, and without the "-p" option can't possibly work correctly, either. So this is gone. We stick to prosodyctl. It's up to the user to choose a different startup method when required (e.g. debugging).

With these changes #4 in your proposed upgrade steps would be

4) Set 'pidfile = "/var/run/prosody/prosody.pid"' in prosody.cfg.lua

What's the best way to communicate the upgrade steps to the user? Should the 5-step-guide be placed in pkg-message or is there a better way?
Comment 7 Thomas Morper 2019-11-20 23:38:25 UTC
Created attachment 209298 [details]
svn diff, including new pid file location