Page MenuHomeFreeBSD

sysutils/shim: update to 0.9
ClosedPublic

Authored by fernape on Sep 4 2018, 5:28 PM.

Details

Summary

Via PR 231029

  • Giving maintainership to egypcio@googlemail.com
  • Use of shebangfix
  • Reordering of some variables
  • Rework of do-install
Test Plan
  • portlint -AC OK
  • poudriere builds for {10.4,11.1}{amd64}

I can not test this program.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fernape created this revision.Sep 4 2018, 5:28 PM
tcberner added inline comments.Sep 4 2018, 7:46 PM
sysutils/shim/Makefile
27 ↗(On Diff #47647)

^ double "USE_GITHUB"

sysutils/shim/files/patch-Cryptlib-Makefile
4 ↗(On Diff #47647)

^ if its only for amd64, why the if?

sysutils/shim/files/patch-Makefile
15 ↗(On Diff #47647)

^dito

sysutils/shim/files/patch-lib-Makefile
4 ↗(On Diff #47647)

^dito

fernape updated this revision to Diff 47666.Sep 4 2018, 9:07 PM

Remove double USE_GITHUB

fernape marked an inline comment as done.Sep 4 2018, 9:10 PM
fernape added a reviewer: trasz.
mat added inline comments.Sep 5 2018, 3:06 PM
sysutils/shim/Makefile
14 ↗(On Diff #47666)

I know you only moved that line, but it would be nice to have a ONLY_FOR_ARCHS_REASON to say why.

fernape updated this revision to Diff 47711.Sep 5 2018, 9:02 PM

Add ONLY_FOR_ARCHS_REASON

fernape marked an inline comment as done.EditedSep 5 2018, 9:06 PM

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).

The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

sysutils/shim/Makefile
14 ↗(On Diff #47666)

Yep.

fernape marked an inline comment as done.Sep 5 2018, 9:06 PM
fernape updated this revision to Diff 47719.Sep 6 2018, 4:58 AM

Correcting ONLY_FOR_ARCHS_REASON message.

Harbormaster completed remote builds in B19431: Diff 47719.

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

Yes, what I meant is, $(ARCH)=amd64 should always be true when building the port, as it is only for ARCH=amd64. So you could simply just unconditionally override it with x86_64, no?

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

I got gnu-efi to build on my 12-i386 poudriere jail
Though have to play with plist because the names will change according to arch

Got rid of USE_GCC line as well

egypcio added a subscriber: egypcio.Sep 7 2018, 1:00 PM

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

I got gnu-efi to build on my 12-i386 poudriere jail
Though have to play with plist because the names will change according to arch
Got rid of USE_GCC line as well

should we try to change the port til this point, or create an equivalent version for i386?

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

Yes, what I meant is, $(ARCH)=amd64 should always be true when building the port, as it is only for ARCH=amd64. So you could simply just unconditionally override it with x86_64, no?

Sure, you're right.

Since there seems to be chance for this port to be available for i386 (if devel/gnu-efi can be made to work on i386 as it seems possible) I would leave the patch untouched though

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

I got gnu-efi to build on my 12-i386 poudriere jail
Though have to play with plist because the names will change according to arch
Got rid of USE_GCC line as well

I can assist in building/patching but I won't be able to run test it. Unfortunately some of the commits of these ports are a bit confusing. The initial commit in r377068 states the port is amd64 only despite the fact that the description says:

SDK for developing EFI applications for ARM-64, ARM-32, x86_64,
IA-64 (IPF), and IA-32 (x86) platforms...

In addition we should be careful with the sensitive software (sensitive in the sense that it is something that deals with private keys and such) and check carefully the other dependent port: sysutils/sbsigntool/

In D17019#363958, @egypcio_googlemail.com wrote:

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

I got gnu-efi to build on my 12-i386 poudriere jail
Though have to play with plist because the names will change according to arch
Got rid of USE_GCC line as well

should we try to change the port til this point, or create an equivalent version for i386?

I would, when ready, update devel/shim. If anyone is interested in making devel/gnu-efi work in archs other than amd64, I would open a different review.

In D17019#363958, @egypcio_googlemail.com wrote:

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

I got gnu-efi to build on my 12-i386 poudriere jail
Though have to play with plist because the names will change according to arch
Got rid of USE_GCC line as well

should we try to change the port til this point, or create an equivalent version for i386?

I would, when ready, update devel/shim. If anyone is interested in making devel/gnu-efi work in archs other than amd64, I would open a different review.

The build on my 12-i386 jail built fine. I haven’t tried mips or the ARMs yet. My issue is how to sub the archs in the plist without having to list each arch

The port is amd64 only because it depends on devel/gnu-efi that is 64 bit only. That's at least on FreeBSD because the upstream project does not say they don't support ia32 (in fact, according to the Makefile, they do).
The override ARCHpart is because $ARCH is "amd64" in FreeBSD but the Makefile expects to check against "x86_64" as it is often reported in Linux.

Yes, what I meant is, $(ARCH)=amd64 should always be true when building the port, as it is only for ARCH=amd64. So you could simply just unconditionally override it with x86_64, no?

Sure, you're right.
Since there seems to be chance for this port to be available for i386 (if devel/gnu-efi can be made to work on i386 as it seems possible) I would leave the patch untouched though

Sounds reasonable to me -- just looked a bit redundant at first :), so please keep it as is.

fernape marked 4 inline comments as done.Sep 8 2018, 12:27 PM

Hi there,

Maintainer gives an OK for the update. Are we good with this as it is?

trasz accepted this revision.Sep 20 2018, 8:04 PM
This revision is now accepted and ready to land.Sep 20 2018, 8:04 PM
tcberner accepted this revision.Sep 21 2018, 3:51 AM
mat added inline comments.Sep 21 2018, 8:25 AM
sysutils/shim/Makefile
14–15 ↗(On Diff #47719)

I don't think having this is a good idea. The day gnu-efi grows to support more than amd64, someone will have to sweep the tree for dependencies. (I know it was there before.) Poudriere will ignore it because it cannot build gnu-efi, and when building manually, it will fail very early when it tries to build gnu-efi.

fernape updated this revision to Diff 48315.Sep 21 2018, 3:39 PM

Removing ONLY_FOR_ARCHS* variables as suggested by mat@

This revision now requires review to proceed.Sep 21 2018, 3:39 PM
fernape marked an inline comment as done.Sep 21 2018, 3:39 PM

@mat, does it look good?

thank you for helping the original PR (231029) that created this diff rev.
very appreciated! looks good to me.

rene added a subscriber: rene.Nov 11 2018, 6:30 PM

What is the status of this review (and PR 231029) ?

In D17019#383351, @rene wrote:

What is the status of this review (and PR 231029) ?

I builds for me in amd64 but I can not provide run testing for this one, sorry.

If Vinicius is ok with it, he can take over PR 231029, I can provide build testing if necessary :-)

In D17019#383351, @rene wrote:

What is the status of this review (and PR 231029) ?

I builds for me in amd64 but I can not provide run testing for this one, sorry.
If Vinicius is ok with it, he can take over PR 231029, I can provide build testing if necessary :-)

I just took PR 231029. thank you, very appreciated!

rene added inline comments.Nov 13 2018, 7:51 PM
sysutils/shim/files/patch-Cryptlib-Makefile
4 ↗(On Diff #47647)

Because if devel/gnu-efi supports more than amd64, this port would be hard-coded to amd64 in a patch file which might be hard to find back. And this port itself does not enforce amd64.

rene added a comment.Nov 16 2018, 8:35 PM

Could you add NO_ARCH=yes to the port? poudriere hinted at this:

pkg-static: DEVELOPER_MODE: Notice: arch "FreeBSD:13:amd64" -- no architecture specific files found:

  • could this package use a wildcard architecture?
rene added a comment.Nov 16 2018, 8:38 PM

Hmm, or is the check confused because it thinks *.efi files are data files? Tthey are actually x86_64 files.

rene accepted this revision.Nov 16 2018, 8:46 PM
This revision is now accepted and ready to land.Nov 16 2018, 8:46 PM
This revision was automatically updated to reflect the committed changes.