net/opensips31: Open Source SIP Server
ClosedPublic
Actions

Authored by diizzy on Sep 1 2021, 9:32 AM.

Details

Reviewers

tcberner
arrowd
lbartoletti

Commits

R11:4e429139a01d: net/opensips31: New port: Open SIP Server

Summary

New port, removed deprecated entries in Makefile and connected it in net/Makefile otherwise it's the same as the submission

Deprecated entries:

# Created by: Euan Thoms <euan@potensol.com>
# $FreeBSD$

PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253844

Test Plan

Compile tested on FreeBSD 13.0-STABLE #0 stable/13-n246086-a20c597c6d7 (amd64) (make, make check-plist) with all options
Poudriere testport OK 12.2-RELEASE (amd64) (default options)
Poudriere testport OK 11.4-RELEASE (amd64) (default options)

Diff Detail

Repository

R11 FreeBSD ports repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

diizzy requested review of this revision.Sep 1 2021, 9:32 AM

diizzy created this revision.

Great job shepherding the original submitter through the process!

net/opensips31/Makefile
2	Why not just `opensips`?
35	Are these subs really needed? They do not contain varying parts, so their contents can be put directly to `pkg-plist`.
52	This seems to be a no-op: `pkg-message` doesn't contain any substitutions.
77	There and below OPT_VARS [1] helpers can be used. [1]: https://docs.freebsd.org/en/books/porters-handbook/makefiles/#options-vars

Remove PLIST_SUB and hardcode paths in pkg-plist
Remove SUB_FILES for pkg-message (not needed)
Convert if statments for options to OPT_VARS
Use framework variables for cp and mv instead of hardcoding

diizzy added inline comments.Sep 1 2021, 7:53 PM

net/opensips31/Makefile
2	Upstream maintains several LTS branches
35	You're right, fixed
52	Corrected
77	Section reworked

arrowd added inline comments.Sep 1 2021, 8:45 PM

net/opensips31/Makefile
102	These install targets look strange to me. When the option is OFF, these don't get added to `TMPPLIST`, but they are still present in the `STAGEDIR`, so poudriere will report them as orphaned. These directories should be removed from the staging dir when corresponding options are OFF. Or am I missing something?
119	This should be `${MV}`, I think. `@sample` keyword will copy `dictionary.opensips.sample` file to `dictionary.opensips` during installation. Also, it seems that this file is installed even with `RADIUS` turned to off.

diizzy added inline comments.Sep 3 2021, 9:55 AM

net/opensips31/Makefile
102	These seem to work as expected, at least testport in Poudriere doesn't complain

arrowd added inline comments.Sep 3 2021, 12:57 PM

net/opensips31/Makefile
102	These seem to work as expected, at least testport in Poudriere doesn't complain Ah, now I get it. These targets are used only to generate `pkg-plist` items dynamically. Instead, these files can be put into `pkg-plist` with `%%PGSQL%%` prefix. While I'm not a fan of dynamic plist generation, I won't insist if you leave it as is.