-
Notifications
You must be signed in to change notification settings - Fork 811
net/miniupnpd: Build improvements #402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
net/miniupnpd: Build improvements #402
Conversation
cc7fa90
to
6f8ffe3
Compare
d0420ae
to
8461391
Compare
3301535
to
a9eb574
Compare
|
||
post-patch: | ||
@${REINPLACE_CMD} -e 's|\(-lssl -lcrypto\)|$$(LDFLAGS) \1|g' \ | ||
-e 's|^INSTALLMANDIR.*|INSTALLMANDIR = ${PREFIX}/share/man|' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least you should keep this part of the post-patch, otherwise the manpage will be misplaced and the file considered as orphan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I had proposed all the necessary changes in miniupnp/miniupnp@0ef0b9b and miniupnp/miniupnp@b4d7803 upstream. Therefore, this patch should no longer be required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I have not yet managed to set up a FreeBSD build environment, so I cannot verify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may I recommend you to try one of our VM images you can run in your favorite environment and set up a poudriere in minutes. Combined with the ability to use pre-compiled binaries (see pkg-bulk -b option, the power is on your hands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry but the commit you point doesn't fix the issue.
I think the BSD makefile is supposed to work for the whole family Free/Net and Open which had differences in the file hierarchy .
But, I think you are right we can get rid of this post-patch script.
net/miniupnpd/Makefile
Outdated
@${REINPLACE_CMD} -e 's|\(-lssl -lcrypto\)|$$(LDFLAGS) \1|g' \ | ||
-e 's|^INSTALLMANDIR.*|INSTALLMANDIR = ${PREFIX}/share/man|' \ | ||
${WRKSRC}/Makefile.bsd | ||
UPNP_DISABLEPPPCONN_DESC= Disable legacy (IGDv1 only) WANPPPConnection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPNP_DISABLEPPPCONN_DESC goes before UPNP_IGDV2_DESC
net/miniupnpd/Makefile
Outdated
LEASEFILE_CONFIGURE_ON= --leasefile | ||
UPNP_IGDV2_CONFIGURE_ON= --igd2 | ||
UPNP_STRICT_CONFIGURE_ON= --strict | ||
UPNP_DISABLEPPPCONN_CONFIGURE_ON= --disable-pppconn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPNP_DISABLEPPPCONN_CONFIGURE_ON goes before UPNP_IGDV2_CONFIGURE_ON
And don't forget to apply portlint, portclippy and portfmt to ensure everything is fine |
a9eb574
to
9d7e45b
Compare
|
||
post-patch: | ||
@${REINPLACE_CMD} -e 's|\(-lssl -lcrypto\)|$$(LDFLAGS) \1|g' \ | ||
-e 's|^INSTALLMANDIR.*|INSTALLMANDIR = ${PREFIX}/share/man|' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry but the commit you point doesn't fix the issue.
I think the BSD makefile is supposed to work for the whole family Free/Net and Open which had differences in the file hierarchy .
But, I think you are right we can get rid of this post-patch script.
net/miniupnpd/Makefile
Outdated
CPE_VENDOR= miniupnp_project | ||
|
||
USE_RC_SUBR= ${PORTNAME} | ||
|
||
HAS_CONFIGURE= yes | ||
# unconditionally use pf, ipfw does not work on FreeBSD | ||
CONFIGURE_ARGS= --firewall=pf --libpfctl | ||
CONFIGURE_ARGS= --firewall=pf \ | ||
--libpfctl | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MAKE_ENV= MANPREFIX=${PREFIX}/share
This is requires to correct the misplaced manpage man vs share/man
9d7e45b
to
fb2368d
Compare
Applied
As suggested, the Makefile has now been adjusted. I was able to successfully build the FreeBSD package with |
--disable-pppconn
to remove legacy (IGDv1 only) workaround no longer included in other implementations since >15y--vendorcfg
to enable configuration of manufacturer info, e.g. router name (friendly name, +5) displayed in windows explorer--https
)