emulators/qemu: Fix build after recent change in src main.
AbandonedPublic
Actions

Authored by jhb on Aug 26 2022, 6:02 PM.

Details

Reviewers

bofh
imp

Summary

After commit e66390736613, <machine/pmap.h> requires <sys/param.h> for
howmany().

Diff Detail

Repository

rP FreeBSD ports repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 47172
Build 44059: arc lint + arc unit

Event Timeline

jhb created this revision.Aug 26 2022, 6:02 PM

Herald added a subscriber: mat. · View Herald TranscriptAug 26 2022, 6:02 PM

jhb requested review of this revision.Aug 26 2022, 6:02 PM

Harbormaster completed remote builds in B47140: Diff 109880.Aug 26 2022, 6:02 PM

If this is the only place that breaks, then fixing the port is probably fine. If there is more widespread fallout from the src commit, then I might have to add a __howmany or the like to use instead.

We'll need to get this upstream as well. Any other architectures affected?

This revision is now accepted and ready to land.Aug 26 2022, 6:28 PM

Also, they are quite late in their 7.1 release cycle right now. I don't know if I'll be able to get this change in, which would break their CI at some random point in the future...

I asked, and the likelihood of getting this in for the 7.1 release is quite low...

vm/pmap.h has always included machine/pmap.h, so there is no need for both includes.

Macro makeitso:

In D36362#825861, @alc wrote:

vm/pmap.h has always included machine/pmap.h, so there is no need for both includes.

It is including <vm/pmap.h> and <machine/trap.h>. <machine/pmap.h> is pulled in via the nested include in <vm/pmap.h>.

In D36362#825830, @imp wrote:

We'll need to get this upstream as well. Any other architectures affected?

Pretty much every other architecture is affected as the src commit touched every <machine/pmap.h>. Does the port build all of the user architectures by default? If so, it may be that <vm/pmap.h> isn't needed by other architecture backends?

Via the power of grep, it seems i386 as a host is also affected and probably needs the same patch to host-signal.h.

Patch i386 as well.

This revision now requires review to proceed.Aug 29 2022, 4:53 PM

Harbormaster completed remote builds in B47172: Diff 109965.Aug 29 2022, 4:53 PM

In D36362#826368, @jhb wrote:

Patch i386 as well.

emulators/qemu is marked to ignore for i386. So I don't think we need it.

I have tried it on 12/13/head successfully on amd64. Unfortunately my Internet connection is not yet active in my new home and my rpi is not yet active so cannot test on other ARCHs.

In D36362#826368, @jhb wrote:

Patch i386 as well.

It would be nice if we patched all of them, port inactive or no, so I can upstream this once 7.1 is released. It's too late to get this fix in there, sadly, but the only saving grace is that upstream bsd-user is utterly broken.

Or maybe a pull request to https://github.com/qemu-bsd-user/qemu-bsd-user with all the architectures wouldn't be a bad idea...

From what I can tell, other host architectures aren't affected, only amd64 and i386?

In D36362#826437, @jhb wrote:

From what I can tell, other host architectures aren't affected, only amd64 and i386?

As I cannot test on other ARCHS now I will await for your further confirmation to commit.

Should I just merge this to get the port unbroken?

In D36362#828764, @jhb wrote:

Should I just merge this to get the port unbroken?

Can you hold on till the weekend? I am just prepping for 7.1.0 while creating qemu70 where these patches will also land. And in case there is emergency feel free to commit.

I have committed emulators/qemu70 with some minor changes to your code as I couldn't find any report of this error on non-FreeBSD OS(Did I just say this?). 3:)

Functional test for update completed. ow running the final build tests for x86 ARCHS only. Will commit maybe today or tomorrow.

Committed with changes.

jhb abandoned this revision.Sep 30 2022, 9:22 PM

jhb mentioned this in D36927: sys/_pv_entry.h: Include sys/param.h.Oct 11 2022, 3:53 PM