Bug 267079 - emulators/virtualbox-ose-additions*: code to support VirtualBox shared folders: SAVENAME-related build failures on CURRENT ≥ 1400068
Summary: emulators/virtualbox-ose-additions*: code to support VirtualBox shared folder...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Graham Perrin
URL: https://github.com/freebsd/freebsd-po...
Keywords:
Depends on:
Blocks:
 
Reported: 2022-10-15 13:27 UTC by Graham Perrin
Modified: 2022-12-19 22:50 UTC (History)
5 users (show)

See Also:
mjg: maintainer-feedback?
mjg: merge-quarterly?


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Graham Perrin freebsd_committer freebsd_triage 2022-10-15 13:27:23 UTC
Via the most recent report at <https://portsfallout.com/fallout?port=emulators%2Fvirtualbox-ose-additions>, from <https://lists.freebsd.org/archives/freebsd-pkg-fallout/2022-October/280835.html>: 


----
…
/wrkdirs/usr/ports/emulators/virtualbox-ose-additions-nox11-legacy/work/VirtualBox-5.2.44/src/VBox/Additions/freebsd/vboxvfs/vboxvfs_vnops.c:1304:22: error: use of undeclared identifier 'SAVENAME'; did you mean 'RENAME'?
                                cnp->cn_flags |= SAVENAME;
                                                 ^~~~~~~~
                                                 RENAME
/usr/src/sys/sys/namei.h:44:40: note: 'RENAME' declared here
enum nameiop { LOOKUP, CREATE, DELETE, RENAME };
                                       ^
3 warnings and 1 error generated.
…
----


5b5b7e2ca2fa9a2418dd51749f4ef6f881ae7179 (2022-09-17) was: 

> vfs: always retain path buffer after lookup
> 
> This removes some of the complexity needed to maintain HASBUF and
> allows for removing injecting SAVENAME by filesystems.

<https://reviews.freebsd.org/D36542> additionally summarises: 

> … Ultimately both flags get retired.
Comment 1 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 02:20:39 UTC
💭

If the likelihood of a quick fix for this bug is low, then (a thought): 

a) add an option to build with support for shared folders

b) for now, make the default to build _without_ this support

   – and maybe mention this in UPDATING.

Please note, this is not to chase a quick fix. 

It's partly to recognise that: 

* a relatively large percentage of users of this port will treat 
  the port itself as essential

* a relatively small subset will treat shared folders as essential. 


<https://www.freshports.org/emulators/virtualbox-ose-additions/#config>


Retrospective
=============

<https://github.com/freebsd/freebsd-ports/blob/7f06d5b68d3929faa7a2a94f12585a9f4a45f159/emulators/virtualbox-ose/files/patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vnops.c#L1417-L1436>

Context (2017-07-18): 

> emulators/virtualbox-ose-additions: Add support for VirtualBox SharedFolder
> 
> This is an experimental implementation of VirtualBox SharedFolder subsystem 
> support for FreeBSD.
> 
> The implementation is based on github.com/lwhsu/freebsd-vboxfs.
> 
> It's tested and worked as expected.
> 
> The locking mechanism may need enhancements.
> 
> Reviewed by:	         jkim (vbox), mat (mentor)
> Approved by:	         jkim (vbox), mat (mentor)
> Sponsored by:          Netzkommune GmbH
> Differential Revision: https://reviews.freebsd.org/D11602
Comment 2 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 02:31:40 UTC
Re: comment #0

mjg@ kib@ please, any pointers on how this might be fixed? 

Thank you
Comment 3 Mateusz Guzik freebsd_committer freebsd_triage 2022-10-17 09:13:35 UTC
It should to the same thing zfs is doing:

#if __FreeBSD_version < 1400068
                                cnp->cn_flags |= SAVENAME;
#endif

basically all uses of SAVENAME hidden behind the above
Comment 4 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 09:46:27 UTC
mjg@: thank you. 

Anyone: I'd like to attempt a patch, but this type of thing is way beyond me (sorry) …
Comment 5 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 10:20:05 UTC
Summary line begins: 

    emulators/virtualbox-ose-additions*

– to signify that this bug also affects

    emulators/virtualbox-ose-additions-legacy
    emulators/virtualbox-ose-additions-nox11
    emulators/virtualbox-ose-additions-nox11-legacy

… although (side note) I suspect that the FreshPorts 'Find bugs' feature at pages such as <https://www.freshports.org/emulators/virtualbox-ose-additions-legacy/> does not work with wildcards in this way.
Comment 6 Mateusz Guzik freebsd_committer freebsd_triage 2022-10-17 10:51:08 UTC
https://people.freebsd.org/~mjg/vbox-savename.diff

I can't be arsed to go through hoops to submit it upstream. According to their policy it is fine to denote the patch as MIT licensed, which I do.
Comment 7 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 11:39:48 UTC
(In reply to Mateusz Guzik from comment #6)

1. assigning to myself, I'll prepare a diff for the four ports and open a 
   review in Phabricator

2. you are my favourite person.
Comment 8 Graham Perrin freebsd_committer freebsd_triage 2022-10-17 16:19:13 UTC
(In reply to Mateusz Guzik from comment #6)

> https://people.freebsd.org/~mjg/vbox-savename.diff
> 
> I can't be arsed to go through hoops to submit it upstream. According to 
> their policy it is fine to denote the patch as MIT licensed, which I do.

Comparing what's above with what's below …


Anyone, please: before I proceed to a diff for Phabricator, does 
<https://github.com/freebsd/freebsd-ports/compare/main...grahamperrin:freebsd-ports:bug-267079.diff> look about right? 

Have I got the wrong end of the stick, poking at those two files? 

Or the right end? 

Cheers
Comment 9 Mateusz Guzik freebsd_committer freebsd_triage 2022-10-19 13:46:52 UTC
Looks fine.
Comment 10 Graham Perrin freebsd_committer freebsd_triage 2022-10-19 23:58:08 UTC
(In reply to Mateusz Guzik from comment #9)

Thanks, I'll progress this … probably tomorrow night.
Comment 11 Graham Perrin freebsd_committer freebsd_triage 2022-10-20 18:58:50 UTC
<https://reviews.freebsd.org/D37074> ready for review.
Comment 12 commit-hook freebsd_committer freebsd_triage 2022-12-19 21:34:51 UTC
A commit in branch main references this bug:

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

commit c35a710801728ab8ed3b2639a9a89f890f6acc29
Author:     Mateusz Guzik <mjg@FreeBSD.org>
AuthorDate: 2022-12-19 21:30:08 +0000
Commit:     Graham Perrin <grahamperrin@FreeBSD.org>
CommitDate: 2022-12-19 21:30:08 +0000

    emulators/virtualbox-ose: fix builds on CURRENT

    SAVENAME was retired by D36542 https://reviews.freebsd.org/D36542

    Bug 267079 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=267079
    involves failures to build four ports, for VirtualBox guest additions,
    on FreeBSD-CURRENT:

        emulators/virtualbox-ose-additions
        emulators/virtualbox-ose-additions-legacy
        emulators/virtualbox-ose-additions-nox11
        emulators/virtualbox-ose-additions-nox11-legacy

    Fix bug 267079 for CURRENT 1400068 and greater by hiding the use of
    SAVENAME in patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vnops.c
    for:

        emulators/virtualbox-ose
        emulators/virtualbox-ose-legacy

    PR:                  : 267079
    Author:              : mjg
    Approved by:         : ports-committers (lwhsu), khng
    Differential revision: https://reviews.freebsd.org/D37074

 .../files/patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vnops.c     | 2 ++
 .../files/patch-src_VBox_Additions_freebsd_vboxvfs_vboxvfs__vnops.c     | 2 ++
 2 files changed, 4 insertions(+)