rS346323

cap_fileargs change committed in:

• rS346313
• rS346314
• rS346315 (main commit)
• rS346316 (update consumers)
• rS346318 (fix for GCC 4.2.1 archs)
• rS346321 (fix for fix for GCC 4.2.1 archs)

looks good, thanks

I'm fine with this as an immediate fix for the installer/VM boot issues.

FYI if you put just the D19507 reference (not the full URL) in a comment Phabricator will create a cross-reference automatically.

tested on eMAG and ThunderX2

In D19869#427634, @bcran wrote:

But, I think I'd like to have the new port committed before I finish that work, with ONLY_FOR_ARCHS=x64 remaining for the time being, since it's work I'd like to schedule and work on separately.

Tested eMAG on rS237234 + this change, serial console works as expected for initial boot messages until encountering AcpiExSystemMemorySpaceHandler panic same as reported in PR 237055. Will test on ThunderX2 soon.

The set -e issue has to be addressed one way or another.

I needed to add -O1 to stand/defs.mk else the loader was too big
to boot.

I think this is fine other than the set -e issue mentioned earlier.

Conceptually this LGTM, but I will defer to ports folks for implementation details related to MD/MI ports.

OR are you suggesting have:
uefi-edk2-qemu-x86_64
uefi-edk2-qemu-arm64

Some proposed style(9) fixups in https://github.com/emaste/freebsd/commit/8506f5949cb13d54a688414c5aae3f696e33db42

Approved

Approved

Approved.

Approved

Approved.

Could we (also) add assertions in pidfile_open for this case?

For reference,

commit 122d67f846cb0be2c9e1c3880db9eb9545bbe38c
Author: Rich Felker <dalias@aerifal.cx>
Date:   Thu Nov 8 15:00:02 2018 -0500

This LGTM as long as compilers or static analyzers don't try to "helpfully" warn that the string does not fit or will not be null terminated.

In D19788#424347, @rgrimes wrote:

Can we get a review to cause bhyve -b to emit a deprecation warning notice (gone_in(13)) if someone tries to use it?

In D19758#424232, @ngie wrote:

In D19758#424219, @emaste wrote:

I think it makes sense to svn cp first, then apply the changes to the contrib/ location; is that your plan?

Yup. Here's how I've been updating the code for reviews (I broke the merge history somehow when I didn't properly reintegrate the googletest branch, so I'm resorting to manual patching):

svn copy ^/vendor/google/capsicum-test/dist contrib/capsicum-test

I think it makes sense to svn cp first, then apply the changes to the contrib/ location; is that your plan?

In D13179#289885, @mat wrote:

Mmmm, I don't really know. The ports tree does not support patches from pull requests, only from our bugzilla, it would be much better to have the pull-request -> bugzilla gateway finally working.

Please reference the upstream pull request in the commit (perhaps mentioning that upstream has been slow on addressing PRs).

Can you generate a review that's just the local diffs, i.e., after the svn cp?

LGTM

approved

The downside of adding these is just the extra loops?

Approved

Approved. Please test make tinderbox though.