Bug 234508

Summary: sysutils/grub2: Update to recent snapshot, fix build with GCC 8
Product: Ports & Packages Reporter: bergerkos
Component: Individual Port(s)Assignee: Rene Ladan <rene>
Status: Closed Feedback Timeout    
Severity: Affects Only Me CC: gerald, rene
Priority: --- Keywords: needs-patch, needs-qa
Version: Latest   
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238688
Attachments:
Description Flags
grub port shell archive
none
a unified diff with changes applied
none
grub2.diff
none
grub2.diff
none
grub2.diff none

Description bergerkos 2018-12-30 17:42:28 UTC
Created attachment 200628 [details]
grub port shell archive

This is suggested as a replacement the "deprecated, soon to be removed" unmaintained port sysutils/grub2.
It is based on a working snapshot, version 2-03. Changes have been made to Makefile to make sure it does not install lang/gcc8 along with itself.
Comment 1 Tobias Kortkamp freebsd_committer 2018-12-30 18:18:53 UTC
Please make this a unified diff against the existing port.  It's
harder to see what you changed otherwise.

It would also be good to base this on a recent revision (at least
the changes and fixes from ports r484145 and ports r488585 appear
to not be included here).

X		${LOCALBASE}/bin/strip:devel/binutils \
X		${LOCALBASE}/bin/gcc8:lang/gcc8

These changes are wrong and ignore GCC_DEFAULT and other settings.
Please use USE_GCC.  If it doesn't need gcc at runtime, add

RUN_DEPENDS:=	${RUN_DEPENDS:Ngcc*}

after .include <bsd.port.mk>

X		${LOCALBASE}/bin/python:lang/python

Depending on the lang/python is forbidden by policy.  People went
through great pains to remove it from all ports, use

BINARY_ALIAS=	python=${PYTHON_CMD}

if the build calls python directly.

XICONV_PREFIX=	${LOCALBASE}

ICONV_PREFIX is not a port settable variable and should only be set
by the framework.  It also already set by USES=iconv

XMAKE_CMD=	${LOCALBASE}/bin/gmake

Already set by USES=gmake

X# Created by: bergerkos@yahoo.co.uk

Please retain the original "Created by" line.  You did not create
this port, sem@ did.  You'll get full credit for the update in the
commit log.

Xpre-configure:
X	@cd ${WRKSRC} && ${SH} autogen.sh
X

Why does USES=autoreconf no longer suffice?
Comment 2 bergerkos 2018-12-30 19:22:45 UTC
(In reply to Tobias Kortkamp from comment #1)
Thanks for your feedback, I've tried the changes you suggest.


>It would also be good to base this on a recent revision (at least
>the changes and fixes from ports r484145 and ports r488585 appear
>to not be included here).
These ARE included, I would think: in this snapshot grub-mkconfig does not make problems prefixing "t" as reported in r488585. And as for r484145, I ran portlint and did what it suggested, so no it finds no mistakes in my Makefile. But maybe I have overlooked something?

>Xpre-configure:
>X	@cd ${WRKSRC} && ${SH} autogen.sh
>X
>
>Why does USES=autoreconf no longer suffice?
Because "USES=autoreconf" doesn't do all that the autogen.sh does. That scripts does some other things as well.
Comment 3 Tobias Kortkamp freebsd_committer 2018-12-30 20:14:05 UTC
(In reply to bergerkos from comment #2
> in this snapshot grub-mkconfig does not make problems prefixing "t"
> as reported in r488585.

It's good that they have fixed most of the non-portable sed -e 's/^/\t/'.
I still see some in at least util/grub.d/30_os-prober.in, so I do not
think that this is fixed completely.  Probably could be easily fixed
with a single patch instead of falling back to gsed now though.

> And as for r484145, I ran portlint and did what it suggested, so
> no it finds no mistakes in my Makefile.

r484145 also removed CONFLICTS, for example.

> Because "USES=autoreconf" doesn't do all that the autogen.sh does.
> That scripts does some other things as well.

Ok, it seems to do a lot.  Is there a need to run autoreconf twice?
Both autogen.sh and the framework do it.  Would USES=autoreconf:build
suffice?
Comment 4 bergerkos 2018-12-30 20:36:28 UTC
(In reply to Tobias Kortkamp from comment #3)

Right, CONFLICTS must be removed.

>Ok, it seems to do a lot.  Is there a need to run autoreconf twice?
>Both autogen.sh and the framework do it.  Would USES=autoreconf:build
>suffice?
No it doesn't. I tried to do a unified patch for what autogen.sh does (with autoreconf -vi removed in the end of it), and it makes about 60,000 lines of code. Quite a bit.
Comment 5 bergerkos 2018-12-30 20:44:15 UTC
(In reply to bergerkos from comment #4)
Sorry, I misunderstood. YES, USES=autoreconf:build seems to do it all right.
Comment 6 bergerkos 2018-12-30 21:02:23 UTC
Created attachment 200635 [details]
a unified diff with changes applied

Here is the unified patch including the changes made according to the comments here.
Comment 7 Tobias Kortkamp freebsd_committer 2019-01-06 10:23:30 UTC
(In reply to bergerkos from comment #6)
> Created attachment 200635 [details]
> a unified diff with changes applied
> 
> Here is the unified patch including the changes made according to the
> comments here.

Patch appears to be based on ports r484145 and does not apply cleanly to
the latest revision ports r488585.  Please rebase.
Comment 8 Rene Ladan freebsd_committer 2019-02-05 20:10:17 UTC
(In reply to Tobias Kortkamp from comment #7)

Also I wonder a bit about the MASTER_SITES, can we use SAVANNAH somehow instead (which expands to multiple mirrors)?
Comment 9 Rene Ladan freebsd_committer 2019-02-05 20:13:44 UTC
Is 2.03 even a version? The latest release tag on https://git.savannah.gnu.org/cgit/grub.git/ mentions 2.02 from 21 months ago.
Comment 10 Tobias Kortkamp freebsd_committer 2019-02-18 16:44:16 UTC
(In reply to Rene Ladan from comment #9)
> Is 2.03 even a version? The latest release tag on
> https://git.savannah.gnu.org/cgit/grub.git/ mentions 2.02 from 21 months ago.

I don't think that GRUB 2.02 will build with GCC 8, so this is just using
a snapshot fetched from their git repo.  2.03 isn't a version yet.
Probably DISTVERSION{,PREFIX,SUFFIX} should be set to something generated
via git describe --tags similarly how we do it with USE_GITHUB for things
in between versions to avoid confusion once there is a 2.03.
Comment 11 Tobias Kortkamp freebsd_committer 2019-02-20 08:50:34 UTC
Created attachment 202179 [details]
grub2.diff

Here's a rebased patch based on the last patch here.

- I've changed it to a DISTVERSION triple based on git describe --tags
  using the latest commit.
- I had to delete a couple of patches manually that no longer applied.
  The patch as submitted did not include this except as comments.
- It runs 'python' during the configure phase.
- OPTIONS_DEFINE was set twice, so NLS was never defined.
- I readded some of the %%%% option markers to pkg-plist.  This is
  probably incomplete and just a guess as I could not get it to build.
- I reset maintainer back to ports given lack of feedback for > 1
  month after comment #7.

I cannot get it to build on 13.0-CURRENT however and it fails with:

cpp8 -DHAVE_CONFIG_H -I.  -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./include -DGRUB_FILE=\"util/grub-fstest.c\" -I. -I. -I. -I. -I./include -I./include -I./grub-core/lib/libgcrypt-grub/src/ -I./grub-core/gnulib -I./grub-core/gnulib  -I/____CPPFLAGS_QA_MARKER____ -DLIBICONV_PLUG -D_FILE_OFFSET_BITS=64 \
  -D'GRUB_MOD_INIT(x)=@MARKER@x@' util/grub-fstest.c grub-core/kern/emu/hostfs.c grub-core/disk/host.c grub-core/osdep/init.c > grub_fstest.pp || (rm -f grub_fstest.pp; exit 1)
cpp8: fatal error: too many input files
cpp8: fatal error: too many input files
compilation terminated.
compilation terminated.

Maybe I deleted one too many patches.  I've also tried this with
the older snapshot.  Same problem.
Comment 12 bergerkos 2019-02-20 10:21:52 UTC
(In reply to Tobias Kortkamp from comment #7)
Sorry for my lack of reply. I had more free time then than I do now. Is there still need for this?
Comment 13 Tobias Kortkamp freebsd_committer 2019-02-20 17:05:45 UTC
(In reply to bergerkos from comment #12)
> Sorry for my lack of reply. I had more free time then than I do now.

Don't worry about it.

> Is there still need for this?

Yes. :)
Comment 14 Tobias Kortkamp freebsd_committer 2019-02-20 17:06:20 UTC
Created attachment 202194 [details]
grub2.diff

Add CONFIGURE_ENV=CPP="${CC} -E" like it was set in in the port
before which I accidentally removed.

It builds fine now, but pkg-plist still needs to be checked with
regards to options.  And somebody should test if this thing actually
still boots something.
Comment 15 Tobias Kortkamp freebsd_committer 2019-02-20 18:04:31 UTC
Created attachment 202197 [details]
grub2.diff

- Add missing USES=pkgconfig and fix build in Poudriere too
- Attempt to fix build on i386; the efiemu modules failed to build
  there, so explicitly pass --disable-efiemu to disable them
- Add PKGNAMESUFFIX
- Put back CPE info

The build on i386 still fails with
gcc8 -DHAVE_CONFIG_H -I. -I..  -Wall -W  -DGRUB_MACHINE_PCBIOS=1 -DGRUB_MACHINE=I386_PC -m32 -I../include -I../include -DGRUB_FILE=\"lib/i386/relocator64.S\" -I. -I. -I.. -I.. -I../include -I../include -I../grub-core/lib/libgcrypt-grub/src/    -DLIBICONV_PLUG -I/usr/local/include -D_FILE_OFFSET_BITS=64 -g  -m32 -msoft-float -DGRUB_FILE=\"lib/i386/relocator64.S\" -I. -I. -I.. -I.. -I../include -I../include -I../grub-core/lib/libgcrypt-grub/src/ -DASM_FILE=1    -O2 -pipe  -DLIBICONV_PLUG -Wl,-rpath=/usr/local/lib/gcc8 -fno-strict-aliasing  -MT lib/i386/relocator_module-relocator64.o -MD -MP -MF lib/i386/.deps-core/relocator_module-relocator64.Tpo -c -o lib/i386/relocator_module-relocator64.o `test -f 'lib/i386/relocator64.S' || echo './'`lib/i386/relocator64.S
lib/i386/relocator64.S: Assembler messages:
lib/i386/relocator64.S:66: Error: unknown pseudo-op: `.code64'
lib/i386/relocator64.S:74: Error: bad register name `%rax'
lib/i386/relocator64.S:98: Error: bad register name `%rax'
lib/i386/relocator64.S:132: Error: bad register name `%rip)'
gmake[4]: *** [Makefile:31027: lib/i386/relocator_module-relocator64.o] Error 1
Comment 16 Tobias Kortkamp freebsd_committer 2019-04-28 19:24:17 UTC
If nobody has time to test or fix/help with the outstanding issue with i386 then
I think it'd be fine to remove the port from the collection now.
Comment 17 Gerald Pfeifer freebsd_committer 2019-04-28 21:49:55 UTC
(In reply to Tobias Kortkamp from comment #16)
> If nobody has time to test or fix/help with the outstanding issue with
> i386 then I think it'd be fine to remove the port from the collection now.

Makes sense to me.  Perhaps send a note to ports@ in parallel to deprecation?
Comment 18 bergerkos 2019-04-29 18:45:47 UTC
From Google search it looks like that particular issue was reported and fixed (by july 2018). I'll check.
Comment 19 Rene Ladan freebsd_committer 2019-08-03 10:45:21 UTC
(In reply to bergerkos from comment #18)

Can you share that patch here, or is it time to remove this port?
Comment 20 Kubilay Kocak freebsd_committer freebsd_triage 2019-08-12 00:16:52 UTC
*** Bug 232961 has been marked as a duplicate of this bug. ***
Comment 21 Rene Ladan freebsd_committer 2019-08-17 20:39:03 UTC
Expired port removed.