Bug 258999 - comms/spandsp: Update to 3.0.0
Summary: comms/spandsp: Update to 3.0.0
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Fernando Apesteguía
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2021-10-08 04:22 UTC by Dustin Marquess
Modified: 2021-11-10 06:22 UTC (History)
4 users (show)

See Also:


Attachments
spandsp update patch (6.03 KB, patch)
2021-10-08 04:22 UTC, Dustin Marquess
no flags Details | Diff
Fixed patch (6.67 KB, patch)
2021-10-13 21:43 UTC, Dustin Marquess
no flags Details | Diff
Proposed patch (6.10 KB, patch)
2021-10-14 11:54 UTC, Fernando Apesteguía
no flags Details | Diff
Proposed patch (6.19 KB, patch)
2021-10-14 13:11 UTC, Fernando Apesteguía
no flags Details | Diff
Updated patch (6.82 KB, patch)
2021-10-27 08:30 UTC, Dustin Marquess
no flags Details | Diff
Updated patch to spandsp + fixes to opal, asterisk16, & asterisk18 (20.16 KB, patch)
2021-11-06 04:46 UTC, Dustin Marquess
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dustin Marquess 2021-10-08 04:22:37 UTC
Created attachment 228509 [details]
spandsp update patch

FreeSWITCH now requires a newer version of SpanDSP, one that they themselves seem to be updating in their GitHub account.


Sadly, they haven't tagged any released yet nor actually released any, so I'm having to go by commit tag  for now.
Comment 1 Fernando Apesteguía freebsd_committer 2021-10-08 06:05:15 UTC
^Triage: If there is a changelog or release notes URL available for this version, please add it to the URL field.

Q/A:  Makefile: [8]: USE_* seen before USES.  According to the porters-handbook, USES must appear first.
 Makefile: [10]: contiguous blank lines (> 1 lines) found.
 Makefile: extra item "GITHUB_COMMIT" placed in the PORTNAME section.
 Makefile: extra item "WRKSRC" placed in the PORTNAME section.
 Makefile: extra item "USE_GITHUB" placed in the PORTNAME section.
 Makefile: extra item "GH_ACCOUNT" placed in the PORTNAME section.
 Makefile: extra item "GH_TAGNAME" placed in the PORTNAME section.
 /data/fernape_data/FreeBSD-repos/ports/comms/spandsp/files/patch-Makefile.am: patch was not generated using ``make makepatch''.  It is recommended to use ``make makepatch'' when you need to [re-]generate a patch to ensure proper patch format.

^Triage: Please confirm this change passes QA (portlint, poudriere at least).
--
https://www.freebsd.org/doc/en/books/porters-handbook/testing.html


Thanks!
Comment 2 Dustin Marquess 2021-10-13 21:43:08 UTC
Created attachment 228681 [details]
Fixed patch

Fixes up portlint complaintd
Comment 3 Dustin Marquess 2021-10-13 21:44:30 UTC
Updated the patch to fix all portlint complaints, sorry for missing those!

Sadly, the only real "changelog" I can find are the actual commit log in GitHub. There IS a ChangeLog file, but they haven't updated that since FreeSWITCH took over maintenance of spandsp, it appears :(.
Comment 4 Daniel Engberg freebsd_committer 2021-10-14 09:31:51 UTC
Make sure to set sane optimization and that the configure script doesn't make assumptions based on the buildbox.
Comment 5 Daniel Engberg freebsd_committer 2021-10-14 10:39:50 UTC
..also upon further review PORTVERSION is also incorrect (bogus) as far as I can tell?

https://docs.freebsd.org/en/books/porters-handbook/book/#makefile-master_sites-github-description
"Example 19. Using USE_GITHUB to Access a Commit Between Two Versions"

Should be more suitable?
Comment 6 Fernando Apesteguía freebsd_committer 2021-10-14 11:54:22 UTC
Created attachment 228693 [details]
Proposed patch

Reorder variables
Add USES=jpeg
Remove WRKSRC
Comment 7 Fernando Apesteguía freebsd_committer 2021-10-14 11:56:51 UTC
(In reply to Daniel Engberg from comment #5)
The project upstream does not provide any tags either so we can't use git describe:

~/tmp/spandsp$ git describe --tags 284fe91
fatal: No names found, cannot describe anything.

PORTVERSION here follows the .so major number. I don't know of a better way to do this.

About the configure script, can you clarify?

Thanks!
Comment 8 Daniel Engberg freebsd_committer 2021-10-14 12:56:00 UTC
(In reply to Fernando Apesteguía from comment #7)

Ahh, my bad... In that case I'd suggest 3.0.0.gCOMMITDATE such as 3.0.0.g20211010 (see net/sems) as we're not really packaging 3.0.0?

As for the optimization I noticed this https://github.com/freeswitch/spandsp/pull/20 and the configure script. Upon further inspection it seems like it's all disconnected / not complete so we don't need to worry about it at all. :-)

It would be nice if we could git rid of the static library too but in that case you'd also need to adjust  misc/libsupertone to not build a static library as far as I can tell.

Don't forgot to check users =)
Comment 9 Fernando Apesteguía freebsd_committer 2021-10-14 13:11:47 UTC
Created attachment 228696 [details]
Proposed patch

Switch to DISTVERSION and add commit date.

Dustin, since you are going to take care of the port, what do you think?

For the record, comms/spandsp, net/freeswitch**, multimedia/gstreamer1-plugins  and net/sems build fine in 12.2 and 13.0.

** net/freeswitch fails to package even with current comms/spandsp 0.0.6. But it builds with the new library version.

Tests in current are running now...
Comment 10 Fernando Apesteguía freebsd_committer 2021-10-14 13:12:31 UTC
(In reply to Daniel Engberg from comment #8)
Thanks for the link, I wasn't aware of that bug.
Comment 11 Dustin Marquess 2021-10-15 03:18:09 UTC
(In reply to Fernando Apesteguía from comment #9)

LGTM!

I'd love to remove the static lib too!

I've never used libsupertone personally, but I can look into it.

This whole thing started because I wanted to update to the latest FreeSWITCH (and Kamailio), as as you found out, the newer versions of FS just won't compile with 0.0.6 anymore.
Comment 12 Fernando Apesteguía freebsd_committer 2021-10-15 10:29:56 UTC
(In reply to Dustin Marquess from comment #11)
Hi Dustin,

The .so library provided by the port will change from .2 to .3 so a PORTREVISION bump would be required for the dependent ports.

Would you add that to the most current patch?

Thanks!
Comment 13 Dustin Marquess 2021-10-27 08:30:30 UTC
Created attachment 229059 [details]
Updated patch

Updated patch that adds PORTREVISION
Comment 14 Dustin Marquess 2021-10-27 08:34:19 UTC
Sorry Fernando, I was out on holiday.

I was under the mistaken belief that since DISTVERSION/PORTVERSION got bumped that PORTREVISION didn't need to get bumped. I've bumped it now.

Is there anything else that needs to be done?  It looks like I need to update net/sofia-sip next now :(.
Comment 15 Fernando Apesteguía freebsd_committer 2021-10-31 16:51:19 UTC
(In reply to Dustin Marquess from comment #14)
Hi Dustin,

The PORTREVISION bump is for the ports that depend on comms/spandsp.

$ git grep comms/spandsp | grep -Ev 'MOVED|UPDATING' | cut -f1 -d: | sort -u
audio/baresip/Makefile.depends
misc/libsupertone/Makefile
multimedia/gstreamer1-plugins/Makefile.common
net/asterisk16/Makefile
net/asterisk18/Makefile
net/mediastreamer/Makefile
net/opal/Makefile
net/sems/Makefile
net/wireshark/Makefile
net/yate/Makefile

But I can do it at commit time if you want.
Comment 16 Fernando Apesteguía freebsd_committer 2021-10-31 16:52:16 UTC
Comment on attachment 229059 [details]
Updated patch

PORTREVISION should be BUMPED in ports which depend on comms/spandsp
Comment 17 Fernando Apesteguía freebsd_committer 2021-10-31 22:58:48 UTC
Unfortunately, this update breaks:

net/asterisk16
net/asterisk18
net/opal
Comment 18 Dustin Marquess 2021-11-01 02:10:27 UTC
Well that’s unfortunate. I’ll see if I can patch those tomorrow to work. If not, I guess I’ll have to submit this as a new spandsp3 package.
Comment 19 Daniel Engberg freebsd_committer 2021-11-01 11:08:15 UTC
opal is very outdated, I'd suggest marking the FAX option as broken unless you want to take on the task of updating to a recent version.
Comment 20 Dustin Marquess 2021-11-05 04:33:13 UTC
Okay, I got opal working with the new spandsp, and I'm about to work on the 2 Asterisk versions (I'm guessing those will be easier since I can see what changes they made in the newer Asterisk versions and just backport them).

What's the proper procedure? I'm assuming I file 3 new bugs, one for each broken port, with patches in them and refer back to this bug?

Sorry for the newbie question :).
Comment 21 Fernando Apesteguía freebsd_committer 2021-11-05 09:04:12 UTC
(In reply to Dustin Marquess from comment #20)
I would do everything in the same patch and once it is ready, I would add maintainers for net/opal and net/asterisk* here.

Another possibility would be to open a review in Phabricator.
Comment 22 Dustin Marquess 2021-11-06 04:46:55 UTC
Created attachment 229312 [details]
Updated patch to spandsp + fixes to opal, asterisk16, & asterisk18

Here's the jumbo patch as suggested.
Comment 23 Dustin Marquess 2021-11-06 04:57:07 UTC
I should note that I didn't attempt to add of the new features (color FAX, etc) to the asterisk code, just made the existing functionality work again.

If the maintainer(s) prefer I work on adding the new features, I can, but I figured that would best be done upstream.
Comment 24 Fernando Apesteguía freebsd_committer 2021-11-06 17:54:33 UTC
(In reply to Dustin Marquess from comment #23)
Thanks a lot for the new patch.

net/opal and net/asterisk1* manintainers in copy in case they want to do further testing.
Comment 25 Guido Falsi freebsd_committer 2021-11-06 22:53:57 UTC
Hi,

I tested the patch, it builds fine and looks fine.

Asterisk runs when compiled with this patch, but I'm unable to properly test the spandsp code at runtime, since it is used to emulate a fax machine, and I've not used such things since...the 90s I think, so I don't know how to really perform such a test.

Anyway this patch looks fine, but I'd ask for a PORTREVISION bump to the asterisk ports when committing.

Regarding the patches I have a question and a request:

- are you planning on submitting these to the asterisk developers?

If not, I'd rather try to upstream these, but then I think some work will be required to get them ready for the asterisk repo, so my request:

- is it possible to add some #ifdef to allow the code to work with both the new and the old spandsp? I think upstream would require that.
Comment 26 Dustin Marquess 2021-11-06 23:30:34 UTC
Indeed, I plan on submitting a "proper" patch, once Digium's lawyers approve my Contributor License Agreement.
Comment 27 Fernando Apesteguía freebsd_committer 2021-11-08 17:35:27 UTC
Thanks Guido for the input.

PORTREVISION will be bumped for all consumers since there is a major shared library change from 2 to 3.

If nothing shows up, I plan to commit this soon.
Comment 28 Dustin Marquess 2021-11-09 00:45:20 UTC
Cleaner patch (retains backwards compatibility) pushed to upstream Asterisk Gerrit:

https://gerrit.asterisk.org/c/asterisk/+/17332
Comment 29 commit-hook freebsd_committer 2021-11-10 06:22:07 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=a55c182ab0a7408eda8401a60a99d3b43ed49ce8

commit a55c182ab0a7408eda8401a60a99d3b43ed49ce8
Author:     Fernando Apesteguía <fernape@FreeBSD.org>
AuthorDate: 2021-11-06 17:46:50 +0000
Commit:     Fernando Apesteguía <fernape@FreeBSD.org>
CommitDate: 2021-11-10 06:20:12 +0000

    comms/spandsp: Update to 3.0.0

    Patch consumers and bump PORTREVISION where appropriate.

    PR:     258999
    Reported by:    jailbird@fdf.net (maintainer)
    Reviewed by:    madpilot@

 audio/baresip/Makefile                             |   2 +-
 comms/spandsp/Makefile                             |  18 ++-
 comms/spandsp/distinfo                             |   5 +-
 comms/spandsp/files/patch-Makefile.am              |   6 +-
 comms/spandsp/files/patch-configure.ac (gone)      |  37 -----
 comms/spandsp/pkg-plist                            |  18 ++-
 misc/libsupertone/Makefile                         |   2 +-
 net/asterisk16/Makefile                            |   1 +
 .../files/patch-res_res__fax__spandsp.c (new)      |  62 +++++++++
 net/asterisk18/Makefile                            |   1 +
 .../files/patch-res_res__fax__spandsp.c (new)      |  62 +++++++++
 net/mediastreamer/Makefile                         |   2 +-
 net/opal/Makefile                                  |   2 +-
 ...plugins_fax_fax__spandsp_spandsp__fax.cpp (new) | 153 +++++++++++++++++++++
 net/sems/Makefile                                  |   1 +
 net/wireshark/Makefile                             |   1 +
 net/yate/Makefile                                  |   1 +
 17 files changed, 321 insertions(+), 53 deletions(-)
Comment 30 Fernando Apesteguía freebsd_committer 2021-11-10 06:22:47 UTC
Committed,

Thank you all.