Bug 234508 - sysutils/grub2: Update to recent snapshot, fix build with GCC 8
Summary: sysutils/grub2: Update to recent snapshot, fix build with GCC 8
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks: 232961
  Show dependency treegraph
 
Reported: 2018-12-30 17:42 UTC by bergerkos
Modified: 2019-01-06 10:24 UTC (History)
0 users

See Also:


Attachments
grub port shell archive (23.14 KB, text/plain)
2018-12-30 17:42 UTC, bergerkos
no flags Details
a unified diff with changes applied (19.39 KB, patch)
2018-12-30 21:02 UTC, bergerkos
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.