Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 2 Nov 2018 11:20:50 +0100
From:      Mathieu Arnold <mat@FreeBSD.org>
To:        Jan Beich <jbeich@FreeBSD.org>
Cc:        Mathieu Arnold <mat@FreeBSD.org>, Mark Linimon <linimon@FreeBSD.org>, svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org
Subject:   Re: svn commit: r483758 - head/devel/libpru
Message-ID:  <20181102102050.3enbe2bma6rq4ujw@atuin.in.mat.cc>
In-Reply-To: <4lcz-ajm7-wny@FreeBSD.org>
References:  <201811020135.wA21ZqbL037253@repo.freebsd.org> <20181102090744.4f7mkmgjuxvrd776@atuin.in.mat.cc> <4lcz-ajm7-wny@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--j7h2xhzzhzrjxzkl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Nov 02, 2018 at 10:56:00AM +0100, Jan Beich wrote:
> Mathieu Arnold <mat@FreeBSD.org> writes:
>=20
> > 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
> >>=20
> >> Log:
> >>   Fix build with GCC-based architectures.
> >>  =20
> >>   PR:		232851
> >>   Submitted by:	Piotr Kubaj
> >>=20
> >> Modified:
> >>   head/devel/libpru/Makefile
> >>=20
> >> Modified: head/devel/libpru/Makefile
> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
> >> --- 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=3D	Library to interface with PRUs
> >>  LICENSE=3D	BSD2CLAUSE
> >> =20
> >>  IGNORE_DragonFly=3D	only supported on FreeBSD
> >> -BROKEN_mips=3D	Does not build: unrecognized command line option -Weve=
rything
> >> -BROKEN_mips64=3D	Does not build: unrecognized command line option -We=
verything
> >> -BROKEN_powerpc64=3D	Does not build: unrecognized command line option =
-Weverything
> >> -BROKEN_sparc64=3D	Does not build: unrecognized command line option -W=
everything
> >> -
> >>  USES=3D		cmake
> >> =20
> >>  WRKSRC=3D		${WRKDIR}/rpaulo-libpru-5a74157b82b8
> >> =20
> >> -.include <bsd.port.mk>
> >> +.include <bsd.port.pre.mk>
> >> +
> >> +post-patch:
> >> +.if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} =3D=3D 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} =3D=3D sparc64
> > post-patch:
> > 	${REINPLACE_CMD} -e 's/ -Weverything//'  ${WRKSRC}/CMakeLists.txt
> > .endif
>=20
> 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:
>=20
>   post-patch:
>           @${REINPLACE_CMD} -e 's/ -Werror//' ${WRKSRC}/CMakeLists.txt
>=20
>   .include <bsd.port.pre.mk>
>=20
>   post-patch:
>   .if ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} =3D=3D sparc64
>           ${REINPLACE_CMD} -e 's/ -Weverything//'  ${WRKSRC}/CMakeLists.t=
xt
>   .endif
>=20
>   .include <bsd.port.post.mk>
>=20
> where the conflict is hidden on Tier1 archs while Tier2 ones are undertes=
ted.

This example is obviously very badly written.  It should either be:

post-patch:
        @${REINPLACE_CMD} -e 's/ -Werror//' ${WRKSRC}/CMakeLists.txt
=2Eif ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} =3D=3D sparc64
        ${REINPLACE_CMD} -e 's/ -Weverything//'  ${WRKSRC}/CMakeLists.txt
=2Eendif

or:

post-patch:
=2Eif ${ARCH:Mmips*} || ${ARCH:Mpowerpc*} || ${ARCH} =3D=3D sparc64
        ${REINPLACE_CMD} -e 's/ -Weverything//'  ${WRKSRC}/CMakeLists.txt
=2Eelse
        @${REINPLACE_CMD} -e 's/ -Werror//' ${WRKSRC}/CMakeLists.txt
=2Eendif

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.

--=20
Mathieu Arnold

--j7h2xhzzhzrjxzkl
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQKTBAABCgB9FiEEOraXidLtEhBkQLpbOkUW81GDzkgFAlvcJQJfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNB
QjY5Nzg5RDJFRDEyMTA2NDQwQkE1QjNBNDUxNkYzNTE4M0NFNDgACgkQOkUW81GD
zkiGdg/+MRJ7JuL20rQukXhji9lFhSRzs1Z62pOiDw5lLVIdORlsuO6iR4IUM63K
ciFDl/IEBj4qXq0i3ANxrMsDe1t5q8Y8Wnu8wlH8CP4ObIBx8NV/XgTzGMyPOGUk
iR3+lz+Gg8pqo4sWjGzLZzTvkR6PX1OH928BvjHYLoJBIbbDiJKueCDOCnj9hcpi
xqyDy4a8KO/BVuTAKyR6hHYqwX9q4iTWsNLtLoub9cQxjEU1eyuSWaWdco+8BBNa
wT/dZK1ddrKWupTiqjGX2jFj8qDd9o6sJPrrMtR2/8HmNGM4+bXlL+45Ri5kUF9e
JFF2n4kiZPn5IE/tgMHEOHCovdmWrQFrPlOiCWl0zZzSxDGqMDBnxWVz0Hh+VdQ2
hIZG2sD7b5Xb6QjmRwvyHuv0Tux/kw6x4tbK36ptw8d+NV2Q/2g1NurH803bjkdi
q3wBLVkDzG5omP1D1lkHEDeMD7/9mQ0Ck7o5HtdomZLQesZ/M/qSVxK9o+emiu+p
ZrCq/Fvgm6rKESkgkgidNl++aVZroRN7PfITGDiY4FfRetOej2NLS+kSbPYsAYpr
/0ISSULHimK0GXNbr+CNA0gEajRU5bn8cdbATh3RDYNFELdAcasNz6cP/9ag0Lrn
eeA60gjrU3Rkv6CjE9J7k1wp3CghDvY5M+wmMb2AlXCEVnGYNIA=
=Kygb
-----END PGP SIGNATURE-----

--j7h2xhzzhzrjxzkl--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20181102102050.3enbe2bma6rq4ujw>