Created attachment 231393 [details] Patch networks.conf location Virtualbox recently introduced a new configuration file called networks.conf, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259399#c16 for details. Right now this is expected to live in /etc/vbox, which goes against hier(7). The attached patch corrects this to /usr/local/etc/vbox (it's not looking at PREFIX, as the vbox installer script is also hardcoded to /usr/local - this way things are at least consistent).
Can't hardcode /usr/local, its a variable user-defined path. Patch hardcoded value using REINPLACE_CMD to substitute in ${LOCALBASE}. Thanks! Also, is quarterly affected?
(In reply to Kubilay Kocak from comment #1) > Can't hardcode /usr/local, its a variable user-defined path. > Patch hardcoded value using REINPLACE_CMD to substitute in ${LOCALBASE}. > Thanks! Hi, I know the dance, but I explained why I didn't do that in the bug description, the existing patch hardcoding LOCALBASE to be /usr/local is [0]. Either it's corrected there as well, or we hardcode it in my patch (having parts in /usr/local/etc/vbox and parts in $LOCALBASE/etc/vbox doesn't make things better). As I'm not the maintainer I went the route that was less work and aligned with how things are done in the port at the moment. I *could* of course do that extra work, but before I do it I would need to know it will be looked at by maintainers (who seem to overlap with portmgr). > Also, is quarterly affected? Yes [0] https://cgit.freebsd.org/ports/tree/emulators/virtualbox-ose/files/patch-src-VBox-Installer-freebsd-VBox.sh?h=2022Q1&id=6348afa6983147beb3d46f9afdd72c9fbc46ca49#n20
(In reply to Kubilay Kocak from comment #1) Okay, ignore what I wrote, I just realized that /usr/local is actually replaced in Makefile without using something like %%LOCALBASE%% in the patch. The advantage of doing it this way is that it plays nicely with `make makepatch`, the downside is, that it causes warnings ("REINPLACE_CMD ran, but did not modify file contents") in case LOCALBASE == "/usr/local". So I just added the patched file to the same REINPLACE_CMD. While there, I changed the patch so that vbox actually looks for networks.conf in two places - first in $LOCALBASE/etc/vbox and if it's not in there, then in /etc/vbox. This will make sure people's existing setups won't break on upgrade. I also wrapped REINPLACE_CMDs that are noops in case of default LOCALBASE/PREFIX values in if-statements to make stage-qa pass without warnings. Patch will follow shortly.
Created attachment 231490 [details] Patch as described in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=261522#c3
@vbox ping?
Maintainer timeout?
Hi, as a member of vbox@ I'll say that I disagree with the conditionals in the makefile around the REINPLACE_CMD. The warning message is not beatiful but does no harm, OTOH the Makefile is already overly complex as is. Regarding the source file patch it looks reasonable, but why are you adding: #define VBOX_GLOBAL_NETWORK_CONFIG_PATH2 "/etc/vbox/networks.conf" AFAIK the only place where vbox looks for all it's files is /usr/local/etc/vbox. Does other virtualbox source code parts look in /etc/vbox in addition to local?
Created attachment 232367 [details] Updated patch to move networks.conf to LOCALBASE @madpilot Please see updated the patch based on our private conversation.
(In reply to Michael Gmelin from comment #8) Thanks for the revised patch. It looks fine. I'd like to test these locally before committing it, so please allow me some time for testing. Hope to be able to be done in a day or two, unluckily compiling a lot of ports on my build machine and it's quite busy.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=66635b2061daf5993d8bbf80457ee59946d91da4 commit 66635b2061daf5993d8bbf80457ee59946d91da4 Author: Michael Gmelin <grembo@FreeBSD.org> AuthorDate: 2022-03-11 21:44:02 +0000 Commit: Guido Falsi <madpilot@FreeBSD.org> CommitDate: 2022-03-11 21:51:31 +0000 emulators/virtualbox-ose: Put vbox/networks.conf in the right place Since virtualbox-ose 6.1.26 a new networks.conf file has been introduced and the software looks for it in /etc/vbox. This commit introduces a patch to make virtualbox look for it in PREFIX/etc/vbox. To help users with configurations already using this file in /etc/vbox notes to UPDATING, pkg-message and to the main virtualbox executable script have been added. Also adding some comments to the Makefile to note that QA warning due to reinplace being run but not having changed any file are expected due to the design of this port Makefile. PR: 261522 UPDATING | 8 ++++++++ emulators/virtualbox-ose/Makefile | 6 ++++++ .../files/patch-src-VBox-Installer-freebsd-VBox.sh | 13 ++++++++++--- ...ch-src_VBox_HostDrivers_adpctl_VBoxNetAdpCtl.cpp (new) | 11 +++++++++++ .../virtualbox-ose/{pkg-message => files/pkg-message.in} | 15 +++++++++++++++ 5 files changed, 50 insertions(+), 3 deletions(-)
Patch committed. Thanks for your work!
(In reply to Guido Falsi from comment #11) @Guido Thanks for review, testing, and committing.
(In reply to Michael Gmelin from comment #12) +1 More broadly, thanks to _everyone_ in Bugzilla, and elsewhere, who helps to move things forward for FreeBSD. (There's no 'thank you' or 'thumbs-up' button/feature in Bugzilla; visualise it being clicked, clicked, clicked, clicked, clicked …)