Bug 266778

Summary: emulators/virtualbox-ose[-legacy]: VM VirtualBox Manager: Help menu: unable to open external browser
Product: Ports & Packages Reporter: martin <martin>
Component: Individual Port(s)Assignee: Vladimir Druzenko <vvd>
Status: Closed FIXED    
Severity: Affects Some People CC: grahamperrin, vvd
Priority: --- Flags: bugzilla: maintainer-feedback? (vbox)
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D40119
Bug Depends on:    
Bug Blocks: 266992    
Attachments:
Description Flags
patch to fix PATH
none
patch for virtualbox-ose and virtualbox-ose legacy
none
patch to fix PATH with better order of path locations
none
Add /usr/local/bin and /usr/local/sbin to PATH for both virtualbox-ose and virtualbox-ose-legacy none

Description martin 2022-10-02 23:31:45 UTC 
Created attachment 237030 [details]
patch to fix PATH

In VirtualBox Manager, QT based GUI, there's a menu Help that has several help items. E.g. "VirtualBox Forums" which should navigate user to a proper webpage. Under FreeBSD user is presented with error dialogue blaming desktop environment (DE). Error occurs even if DE is fully capable of doing so (opening browser,etc.).

Short description of the problem: /usr/local/bin/virtualbox shell wrapper is setting PATH variable to a very generic one excluding /usr/local/bin. Due to this browsers outside this PATH are not found.

Handling URL is done by QT's QDesktopServices API. During the resolving  detectWebBrowser() is called:

https://codebrowser.dev/qt5/qtbase/src/platformsupport/services/genericunix/qgenericunixservices.cpp.html#_ZL16detectWebBrowserRK10QByteArraybP7QString

which calls findExecutable()

https://codebrowser.dev/qt5/qtbase/src/corelib/io/qstandardpaths.cpp.html#_ZN14QStandardPaths14findExecutableERK7QStringRK11QStringList

Here qgetenv("PATH") is called which is used as PATH to search executables.

It makes sense to expand the PATH to include FreeBSD's standard /usr/local/bin as that's native path for ports programs. 

I've added the patch towards the virtualbox-ose-6.1.36.
Comment 1 Graham Perrin freebsd_committer freebsd_triage 2022-10-13 22:33:54 UTC 
Thanks, 

(In reply to martin from comment #0)

Can the patch be broadened, to include emulators/virtualbox-ose-legacy, without difficulty?
Comment 2 martin 2022-10-14 18:27:21 UTC 
Created attachment 237306 [details]
patch for virtualbox-ose and virtualbox-ose legacy
Comment 3 martin 2022-10-14 18:27:48 UTC 
My patch is fixing src/VBox/Installer/freebsd/VBox.sh. As I was testing if I can use this in legacy I've noticed that actually VBox.sh exists because of a patch files/patch-src-VBox-Installer-freebsd-VBox.sh for given package. I.e. it's patch that creates this file for FreeBSD. 

So instead of patching file that as created as a result of a patch it makes sense to patch that patch. While files are different diff produces the same patch as only very small portion of the script has to be modified and that part is the same.
Comment 4 Graham Perrin freebsd_committer freebsd_triage 2022-10-15 08:49:21 UTC 
(In reply to martin from comment #3)

Nice find! Thank you. 

Whilst here, a nit: is it worth making the order more like what's normally seen? 

So, 

PATH="/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin"


----

That is, based partly on the result of a fairly recent installation of 13.1. 

root@fuji:~ # uname -aKU
FreeBSD fuji 13.1-RELEASE-p2 FreeBSD 13.1-RELEASE-p2 GENERIC amd64 1301000 1301000
root@fuji:~ # whoami ; echo $PATH
root
/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin
root@fuji:~ #
Comment 5 martin 2022-10-15 15:45:49 UTC 
Personally I like it the way it is in patch. There's no reason for VirtualBox to expand path outside of standard path (e.g. ~/bin; note not /root/bin as most likely vbox is not launched as root).
Comment 6 Graham Perrin freebsd_committer freebsd_triage 2022-10-15 16:09:43 UTC 
(In reply to martin from comment #5)

Thanks, and please note that the suggested path included neither ~/bin nor /root/bin …

… the (full) output from    echo $PATH    was only to demonstrate the order of the subset.
Comment 7 martin 2022-10-15 19:16:26 UTC 
Created attachment 237357 [details]
patch to fix PATH with better order of path locations
Comment 8 martin 2022-10-15 19:17:40 UTC 
Ok, my bad.

Well, it does make sense to look in "bin" first as it's most likely location of user programs. So following that logic it makes sense to shuffle them probably even more so that those path locations are searched first.
Comment 9 martin 2023-01-01 20:13:44 UTC 
What is the reason of state change? Is there something else required for the patch to be ready?
Comment 10 Graham Perrin freebsd_committer freebsd_triage 2023-01-02 20:34:05 UTC 
(In reply to martin from comment #9)

> What is the reason of state change?

Thanks for asking. 

Removal of keywords is due to their deprecation. Related: 

* the new descriptions (click 'Keywords') – these were outdated around the 
  time of my first change

* <https://bugs.freebsd.org/bugzilla/> ▶ Latest FreeBSD Bugzilla Changes

* <https://wiki.freebsd.org/Bugzilla#Resources> ▶ 
  <https://wiki.freebsd.org/Bugzilla/Keywords#Process_based_keywords>

* <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=227147>.

----

> Is there something else required for the patch to be ready?

I do not know, but we have vbox@ flagged for maintainer feedback.
Comment 11 Vladimir Druzenko freebsd_committer freebsd_triage 2023-05-16 17:33:58 UTC 
Created attachment 242218 [details]
Add /usr/local/bin and /usr/local/sbin to PATH for both virtualbox-ose and virtualbox-ose-legacy

(In reply to Graham Perrin from comment #10)
> I do not know, but we have vbox@ flagged for maintainer feedback.
vbox@ are half-dead (as far as I see last years).
Comment 12 Vladimir Druzenko freebsd_committer freebsd_triage 2023-05-16 19:29:21 UTC 
I hope vbox@ allow me resole several PRs with emulators/virtualbox-ose*.
Comment 13 commit-hook freebsd_committer freebsd_triage 2023-05-18 19:49:21 UTC 
A commit in branch main references this bug:

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

commit 8b9f42573ed3ebcd6a84f7ccd1ba2ef000357332
Author:     Vladimir Druzenko <vvd@FreeBSD.org>
AuthorDate: 2023-05-18 19:41:30 +0000
Commit:     Vladimir Druzenko <vvd@FreeBSD.org>
CommitDate: 2023-05-18 19:48:47 +0000

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

    While here sort out LOCALBASE, PREFIX, VBOX_DIR, VBOX_ETC

    PR:                     266778
    Approved by:            arrowd (mentor)
    Differential Revision:  https://reviews.freebsd.org/D40119

 emulators/virtualbox-ose-legacy/Makefile                     |  6 ++++--
 .../files/patch-src-VBox-Installer-freebsd-VBox.sh           | 10 +++++-----
 emulators/virtualbox-ose/Makefile                            | 10 ++++++----
 .../files/patch-src-VBox-Installer-freebsd-VBox.sh           | 12 ++++++------
 .../patch-src_VBox_HostDrivers_adpctl_VBoxNetAdpCtl.cpp      |  2 +-
 emulators/virtualbox-ose/files/pkg-message.in                |  6 +++---
 6 files changed, 25 insertions(+), 21 deletions(-)