Page MenuHomeFreeBSD

sysutils/uefi-edk2-bhyve: update to be same as uefi-edk2-bhyve-devel and delete -devel port
ClosedPublic

Authored by bcran on Feb 3 2020, 3:07 AM.

Details

Summary

As indicated via email, araujo@ is no longer interested in mainintaing the
sysutils/uefi-edk2-bhyve port. So take over maintenance, update it to be
the same as sysutils/uefi-edk2-bhyve-devel
and delete the latter port.

Fix a couple of issues portlint flagged about the version number and the last two lines of Makefile.

Test Plan

Built in poudriere: https://poudriere.bsdio.com:8080/build.html?mastername=current-amd64-default-bsdio&build=2020-02-05_23h22m16s

uefi-edk2-bhyve-g20190424,2	sysutils/uefi-edk2-bhyve	success	00:23:20

Ran portlint.

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 29161
Build 27094: arc lint + arc unit

Event Timeline

  • Update MOVED to show that uefi-edk2-bhyve-devel is now uefi-edk2-bhyve
sysutils/uefi-edk2-bhyve/Makefile
15

bash is not in the base system, so it will not be found in PATH unless shells/bash is installed.

34–39

These should go before the options block. See Chapter 15. Order of Variables in Port Makefiles.

If you delete sysutils/uefi-edk2-bhyve-devel, don't forget to remove it from sysutils/Makefile

  • Update port Makefile to be similar in style to uefi-edk2-qemu
  • Remove sysutils/uefi-edk2-bhyve-devel from sysutils/Makefile
  • Move DEBUG_VARS, DEBUG_VARS_OFF and HTTP_BOOT_VARS to before OPTIONS
bcran marked an inline comment as done.Feb 3 2020, 5:04 PM
bcran added inline comments.
sysutils/uefi-edk2-bhyve/Makefile
15

I thought this line caused it to be installed? Could you check the changes I've made to see if they're better/worse please?

@mat I've updated the Makefile to be similar to sysutils/uefi-edk2-qemu - which involved removing BASH_CMD (replacing it with just 'bash'), removing USE_GCC and some other changes. I'd appreciate feedback about whether these changes are better or worse!

I thought this line caused it to be installed? Could you check the changes I've made to see if they're better/worse please?

It is a semantic difference.
Having a full path means you need that file to be installed, for reasons, maybe it's a header file that you need to build, or something.
Having only the name of the command means that you need to run the command, so PATH will be searched for it, and it will be installed if not found. The only case where you put the full path to the command is when there is a different version in the base system that is in the PATH.

sysutils/uefi-edk2-bhyve/Makefile
4–5

The tag says ...stable201903 which looks a lot like "march 2019", is there a reason the version up there says april 2019 and not march?

48–59

All those are out of order. See Chapter 15. Order of Variables in Port Makefiles.

Order is :

  1. USES, USE_* and related variables
  2. Standard framework variables, like MAKE_ARGS
  3. Options, and then options helpers
  4. Variables local to the Makefile
bcran marked 2 inline comments as done.
  • MAKE_ARGS should be earlier in the Makefile
sysutils/uefi-edk2-bhyve/Makefile
4–5

I know, it _should_ be 20190307, the date the tag was added - but I guess it's dated for when the port was updated. I'd prefer not to change it now, but instead wait until I update it to the next edk2-stable201911 tag.

sysutils/uefi-edk2-bhyve/Makefile
29

Are there slave ports using this one as a master port ?

If not, don't use +=.

40–58

This is in the USES/USE_* block, should be just after USES.

45–46

This is a standard framework variable, goes before options, with MAKE_ARGS.

60

-j4 is so wrong, a port should not force or decide which number of build jobs it is going to use. This is set by the user, globally, and is available through MAKE_JOBS_NUMBER.

You should use -j${MAKE_JOBS_NUMBER}.

Also, technically, you should not be using MAKE_CMD directly, but DO_MAKE_BUILD, probably something along the lines of:

${DO_MAKE_BUILD} -C ${WRKSRC}/BaseTools
70–74

I have not checked, but it feels like _GCC5 is strange, we don't have gcc5 in the tree, and the default is 8 or 9 depending on the arch.

bcran marked 3 inline comments as done.
  • Fix some more issues from the review feedback.
sysutils/uefi-edk2-bhyve/Makefile
60

It looks like I can't use ${DO_MAKE_BUILD} because it uses its own environment and won't work with the Makefile that EDK2 uses: for example EDK2 invokes "make" within the Makefiles.

70–74

That's the EDK2 toolchain, which supports everything from GCC 5 and newer with the same "GCC5" toolchain definition.

bcran marked 2 inline comments as done.Feb 4 2020, 4:18 PM

Thanks for the feedback, and sorry about all the mistakes!

This revision is now accepted and ready to land.Feb 5 2020, 10:37 PM
In D23476#516323, @mat wrote:

Fine by me.

Thanks. May I commit it with an "Approved By: mat" line, or would you prefer somebody else approve it?

Did you test it using poudriere?

sysutils/uefi-edk2-bhyve/Makefile
5

Why did you bump PORTEPOCH?

Did you test it using poudriere?

I'm running poudriere now. I had been focused on getting the uefi-edk2-qemu port fix committed.

sysutils/uefi-edk2-bhyve/Makefile
5

Because otherwise portlint said the version was going backwards. That is, that "0.2" is a higher version number than "g20190424".
It said I should either bump PORTEPOCH or there was another variable I could use (PORTREVISION?).

OK, as soon as the poudriere get the results, share it here.

sysutils/uefi-edk2-bhyve/Makefile
5

OK, PORTEPOCH is ok then.

OK, as soon as the poudriere get the results, share it here.

The build finished. I've posted and linked the results.

LGTM, Add:

Approve by: araujo

And have a happy uefi-edk2 maintainership time :)

bcran marked 3 inline comments as done.Feb 7 2020, 6:23 AM