Page MenuHomeFreeBSD

emulators/virtualbox-ose: fix builds on CURRENT
ClosedPublic

Authored by grahamperrin on Oct 20 2022, 6:57 PM.
Tags
None
Referenced Files
F102843921: D37074.diff
Sun, Nov 17, 9:53 PM
Unknown Object (File)
Mon, Nov 11, 5:05 AM
Unknown Object (File)
Tue, Nov 5, 3:45 PM
Unknown Object (File)
Tue, Oct 22, 5:08 PM
Unknown Object (File)
Oct 17 2024, 6:10 AM
Unknown Object (File)
Oct 15 2024, 2:39 AM
Unknown Object (File)
Oct 14 2024, 4:29 AM
Unknown Object (File)
Oct 12 2024, 12:26 PM
Subscribers

Details

Summary

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
Test Plan

https://github.com/freebsd/freebsd-ports/compare/main...grahamperrin:freebsd-ports:bug-267079.diff was OK'd by @mjg under https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=267079#c8.

Given the OK there, is a test plan required here?

To me, the code change appears non-intrusive (not likely to break something whilst fixing something else), however I'm not a coder.


I'll delete my bug-267079 branch after a commit to main.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

grahamperrin created this revision.

Please, is the summary reasonable?

(Drafting the summary was quite mind-bending for me. Build failures affecting four ports (for guest additions), to be fixed in code for two other ports …)

I don't have a ports commit bit, so – if approved – this will require a commit by someone else.

Note that whilst the diff here is from me, we should treat @mjg as the true author (of the diff that was offered at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=267079#c6).

Thanks

I don't have a ports commit bit, so – if approved – this will require a commit by someone else.

This is a wrong assumption, cross-repository commit is encouraged, anyone is welcomed to do so with approval from a committer in that repository. So please don't limit yourself and do stretch your contribution.

Note that whilst the diff here is from me, we should treat @mjg as the true author (of the diff that was offered at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=267079#c6).

you can use git commit --author

@lwhsu thanks for the correction. I'll attempt the commit, subject to approval of the review.

I'll add mentors as reviewers.

This revision is now accepted and ready to land.Nov 8 2022, 12:35 AM

Looks fine to me, however please let it be further reviewed by and obtain approvals from ports committers.

This revision now requires review to proceed.Nov 8 2022, 5:09 AM

Thank you,

In D37074#847529, @khng wrote:

… please let it be further reviewed by and obtain approvals from ports committers.

Please, can a ports committer approve, or review this?

I was bumped twice last month (13th and 20th November).

this is a trivial change and it fixes a build breakage, i don't think waiting any further is warranted

I can approve you to commit this by my ports bit. While I'm not in vbox team and they are the maintainer of these ports, I think this still is fine to commit as this should be in the blanket of breakage fix. BTW, If you're going to do cross-repository commit, you should do as a ports committer does, please commit it after you have verified this patch actually works.

This revision is now accepted and ready to land.Dec 3 2022, 6:36 PM

I can approve you to commit this by my ports bit. While I'm not in vbox team and they are the maintainer of these ports, I think this still is fine to commit as this should be in the blanket pf breakage fix. BTW, If you're going to do cross-repository commit, you should do as a ports committer does, please commit it after you have verified this patch actually works.

More precisely, in this case since @mjg and @khng are fine with this, for me (as a ports committer) I'll verify that this patch applies and the package builds, as the final sanity check before commit.

People, thanks for your patience. I had to learn a few more things before attempting the commit.

I'll delete my bug-267079 branch after a commit to main.

Done.

In D37074#854782, @lwhsu wrote (3rd December):

… I'll verify that this patch applies and the package builds, as the final sanity check before commit.

Please: did I misinterpret that – did I commit, and close this, too soon?

We have bug 268603 for the package not building on FreeBSD:14:amd64.

In D37074#854782, @lwhsu wrote (3rd December):

… I'll verify that this patch applies and the package builds, as the final sanity check before commit.

Please: did I misinterpret that – did I commit, and close this, too soon?

We have bug 268603 for the package not building on FreeBSD:14:amd64.

Maybe. My two comments should be read together. What I mean is if you want to do a commit in the ports tree, you have to do the responsibility as a ports committer -- verify the sanity of what you commit. And I try to explain the sanity check in ports more: verify the patch applies and the package builds.

More clearly: my approval is under a condition: you have done the minimal check for a ports commit, as the correctness of the fix has already checked by two other developers.

It looks that you didn't do these test, or did it wrong before going ahead to commit.

Thanks for clarifying.

… It looks that you didn't do these test, …

True.