svn commit: r483758 - head/devel/libpru
Mathieu Arnold
mat at FreeBSD.org
Fri Nov 2 10:20:55 UTC 2018
On Fri, Nov 02, 2018 at 10:56:00AM +0100, Jan Beich wrote:
> Mathieu Arnold <mat at FreeBSD.org> writes:
>
> > On Fri, Nov 02, 2018 at 01:35:52AM +0000, Mark Linimon wrote:
> >
> >> Author: linimon
> >> Date: Fri Nov 2 01:35:51 2018
> >> New Revision: 483758
> >> URL: https://svnweb.freebsd.org/changeset/ports/483758
> >>
> >> Log:
> >> Fix build with GCC-based architectures.
> >>
> >> PR: 232851
> >> Submitted by: Piotr Kubaj
> >>
> >> Modified:
> >> head/devel/libpru/Makefile
> >>
> >> Modified: head/devel/libpru/Makefile
> >> ==============================================================================
> >> --- head/devel/libpru/Makefile Fri Nov 2 01:34:15 2018 (r483757)
> >> +++ head/devel/libpru/Makefile Fri Nov 2 01:35:51 2018 (r483758)
> >> @@ -11,13 +11,15 @@ COMMENT= Library to interface with PRUs
> >> LICENSE= BSD2CLAUSE
> >>
> >> IGNORE_DragonFly= only supported on FreeBSD
> >> -BROKEN_mips= Does not build: unrecognized command line option -Weverything
> >> -BROKEN_mips64= Does not build: unrecognized command line option -Weverything
> >> -BROKEN_powerpc64= Does not build: unrecognized command line option -Weverything
> >> -BROKEN_sparc64= Does not build: unrecognized command line option -Weverything
> >> -
> >> USES= cmake
> >>
> >> WRKSRC= ${WRKDIR}/rpaulo-libpru-5a74157b82b8
> >>
> >> -.include <bsd.port.mk>
> >> +.include <bsd.port.pre.mk>
> >> +
> >> +post-patch:
> >> +.if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} == sparc64
> >> + ${REINPLACE_CMD} -e 's/ -Weverything//' ${WRKSRC}/CMakeLists.txt
> >> +.endif
> >> +
> >> +.include <bsd.port.post.mk>
> >
> > Could you try and put the whole target definition inside the if so that
> > it does not needlessly create an empty target, like this:
> >
> >
> > .if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} == sparc64
> > post-patch:
> > ${REINPLACE_CMD} -e 's/ -Weverything//' ${WRKSRC}/CMakeLists.txt
> > .endif
>
> I disagree. If a target is defined conditionally then adding unconditional
> one with the same name becomes error-prone in that the conflict would only
> occur if the condition is true. For example:
>
> post-patch:
> @${REINPLACE_CMD} -e 's/ -Werror//' ${WRKSRC}/CMakeLists.txt
>
> .include <bsd.port.pre.mk>
>
> post-patch:
> .if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} == sparc64
> ${REINPLACE_CMD} -e 's/ -Weverything//' ${WRKSRC}/CMakeLists.txt
> .endif
>
> .include <bsd.port.post.mk>
>
> where the conflict is hidden on Tier1 archs while Tier2 ones are undertested.
This example is obviously very badly written. It should either be:
post-patch:
@${REINPLACE_CMD} -e 's/ -Werror//' ${WRKSRC}/CMakeLists.txt
.if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} == sparc64
${REINPLACE_CMD} -e 's/ -Weverything//' ${WRKSRC}/CMakeLists.txt
.endif
or:
post-patch:
.if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} == sparc64
${REINPLACE_CMD} -e 's/ -Weverything//' ${WRKSRC}/CMakeLists.txt
.else
@${REINPLACE_CMD} -e 's/ -Werror//' ${WRKSRC}/CMakeLists.txt
.endif
depending on what you want to do.
But to the point, it has nothing to do with what I was saying.
If the target you add is only required when the conditions are met, then
only define it inside the .if.
It makes everything a wee bit faster by not having to compute order for
one more target.
--
Mathieu Arnold
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 963 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-ports-head/attachments/20181102/668b887e/attachment.sig>
More information about the svn-ports-head
mailing list