Page MenuHomeFreeBSD

emulators/virtualbox-ose[-legacy]: VM VirtualBox Manager: Help menu: unable to open external browser
ClosedPublic

Authored by vvd on May 16 2023, 7:25 PM.
Tags
Referenced Files
F102173546: D40119.id122066.diff
Fri, Nov 8, 12:23 PM
F102146010: D40119.id122066.diff
Fri, Nov 8, 4:44 AM
Unknown Object (File)
Thu, Nov 7, 10:47 AM
Unknown Object (File)
Wed, Nov 6, 11:19 AM
Unknown Object (File)
Tue, Nov 5, 2:51 PM
Unknown Object (File)
Mon, Oct 28, 7:30 PM
Unknown Object (File)
Mon, Oct 21, 7:05 AM
Unknown Object (File)
Fri, Oct 18, 9:30 AM
Subscribers
None

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

vvd requested review of this revision.May 16 2023, 7:25 PM
vvd created this revision.
emulators/virtualbox-ose-legacy/files/patch-src-VBox-Installer-freebsd-VBox.sh
19

I'm not sure that putting /usr/local/ before base is good practice. With this patch, a malicious /usr/local/bin/mount could be called instead of /sbin/mount. Would we be better off putting /usr/local/... at the end instead?

emulators/virtualbox-ose-legacy/files/patch-src-VBox-Installer-freebsd-VBox.sh
19

My 1st version before upload here was:
/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/sbin
But changed to current after read patch: https://bugs.freebsd.org/bugzilla/attachment.cgi?id=237357&action=diff

emulators/virtualbox-ose-legacy/files/patch-src-VBox-Installer-freebsd-VBox.sh
19

I always start with what FreeBSD itself uses (from /etc/login.conf, and I skip ~/bin of course):

$ awk '/path/ {print;exit;}' /etc/login.conf
        :path=/sbin /bin /usr/sbin /usr/bin /usr/local/sbin /usr/local/bin ~/bin:\

At the end of the day, you're the committer. Don't be afraid to modify patches from submitters; they're counting on you for that. Most submitters see changes in the final patch as a learning opportunity.

PATH changed to: /sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin

vvd marked 2 inline comments as done.May 17 2023, 5:04 PM

Ideally, we'd want to use %%LOCALBASE%% substitution there, but meh. I think this is fine as it is.

This revision is now accepted and ready to land.May 18 2023, 7:05 AM

Ideally, we'd want to use %%LOCALBASE%% substitution there, but meh. I think this is fine as it is.

We do, actually. https://github.com/freebsd/freebsd-ports/blob/main/emulators/virtualbox-ose-legacy/Makefile#L326

Replaced all "/usr/local" with %%LOCALBASE%% and "/usr/local/lib/virtualbox" with %%VBOX_DIR%%.

This revision now requires review to proceed.May 18 2023, 1:05 PM
emulators/virtualbox-ose-legacy/Makefile
327

%%LOCALBASE%% should be replaced with ${LOCALBASE}, obviously.

emulators/virtualbox-ose-legacy/files/patch-src-VBox-Installer-freebsd-VBox.sh
20

Here %%PREFIX should be used, I believe.

emulators/virtualbox-ose/files/patch-src_VBox_HostDrivers_adpctl_VBoxNetAdpCtl.cpp
8

%%PREFIX%%

After discussion in IRC did several improvements.

Removed double REINPLACE_CMD for src/VBox/HostDrivers/adpctl/VBoxNetAdpCtl.cpp - 1st is in patch.

This revision is now accepted and ready to land.May 18 2023, 6:31 PM