Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 19 Nov 2015 14:02:29 -0800
From:      Bryan Drewery <bdrewery@FreeBSD.org>
To:        "Simon J. Gerraty" <sjg@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-vendor@freebsd.org
Subject:   Re: svn commit: r289720 - in vendor/NetBSD/bmake/dist: . mk unit-tests
Message-ID:  <564E46F5.9080505@FreeBSD.org>
In-Reply-To: <56280EE6.9050909@FreeBSD.org>
References:  <201510212214.t9LMENGq030089@repo.freebsd.org> <56280EE6.9050909@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 10/21/15 3:17 PM, Bryan Drewery wrote:
> On 10/21/2015 3:14 PM, Simon J. Gerraty wrote:
>> +2015-10-12  Simon J. Gerraty  <sjg@bad.crufty.net> + +	* var.c:
>> the conditional expressions used with ':?' can be +	expensive, if
>> already discarding do not evaluate or expand +	anything. +=20
>> +2015-10-10  Simon J. Gerraty  <sjg@bad.crufty.net> + +	*
>> Makefile (MAKE_VERSION): 20151010 +	  Merge with NetBSD make,
>> pick up +	  o Add Boolean wantit flag to Var_Subst and Var_Parse=20
>> +	    when FALSE we know we are discarding the result and can +
>> skip operations like Cmd_Exec.
>=20
> Thank you!
>=20
> I haven't yet had a chance to try these but I do expect it to be=20
> beneficial for ports.
>=20

By the way, I think the changes here to delay evaluating :? fixes or
avoids an obscure bug I just ran into as well.

> ~/git/freebsd # git diff
> targets/pseudo/userland/misc/Makefile.depend diff --git
> a/targets/pseudo/userland/misc/Makefile.depend
> b/targets/pseudo/userland/misc/Makefile.depend index
> 0c98392..7e88956 100644 ---
> a/targets/pseudo/userland/misc/Makefile.depend +++
> b/targets/pseudo/userland/misc/Makefile.depend @@ -2,43 +2,49 @@
>=20
> # This file is not autogenerated - take care!
>=20
> +.if !defined(MK_FORTH) +.include <src.opts.mk> +.endif + DIRDEPS =3D
> \ rescue/librescue \ rescue/rescue \ -       sys/boot/ficl \=20
> etc/sendmail \
>=20
> +DIRDEPS.${MK_FORTH}+=3D \ +       sys/boot/ficl \ +
> sys/boot/forth \

This creates either DIRDEPS.yes or DIRDEPS.no. FreeBSD uses this
pattern for SUBDIR now in the main build dirs (bin, sbin, usr.bin,
usr.sbin). Adding DIRDEPS support seems trivial enough...

> ~/git/freebsd # git diff share/mk/local.dirdeps.mk diff --git
> a/share/mk/local.dirdeps.mk b/share/mk/local.dirdeps.mk index
> ff4ee18..9b92273 100644 --- a/share/mk/local.dirdeps.mk +++
> b/share/mk/local.dirdeps.mk @@ -104,3 +104,8 @@
> CSU_DIR.${DEP_MACHINE_ARCH} ?=3D csu/${DEP_MACHINE_ARCH} CSU_DIR :=3D
> ${CSU_DIR.${DEP_MACHINE_ARCH}} BOOT_MACHINE_DIR:=3D
> ${BOOT_MACHINE_DIR.${DEP_MACHINE}} KERNEL_NAME:=3D
> ${KERNEL_NAME.${DEP_MACHINE}} + +# Support DIRDEPS.${MK_OPTION}=20
> +.if !empty(DIRDEPS) +DIRDEPS:=3D ${DIRDEPS} ${DIRDEPS.yes} +.endif

Now for the bug. Even though this works, it somehow leaves behind a
cached value of a non-expanded version that the __depdirs:=3D line uses
in dirdeps.mk (with a :?) when DIRDEPS.yes is empty (since MK_FORTH=3Dno
here sets DIRDEPS.no instead):

> ~/git/freebsd # MK_FORTH=3Dno make -f
> targets/pseudo/userland/misc/Makefile.depend -V DIRDEPS make:
> "/root/git/freebsd/share/mk/dirdeps.mk" line 495: warning: Missing
> closing parenthesis for exists() make: Bad conditional expression
> `exists(/root/git/freebsd/${DIRDEPS)' in
> exists(/root/git/freebsd/${DIRDEPS)?/root/git/freebsd/${DIRDEPS.yes}:
>
>=20
rescue/librescue  rescue/rescue  etc/sendmail sys/boot/i386/boot0
sys/boot/i386/boot0sio  sys/boot/i386/boot2  sys/boot/i386/btx/btx
sys/boot/i386/btx/btxldr  sys/boot/i386/btx/li
> b  sys/boot/i386/cdboot  sys/boot/i386/gptboot
> sys/boot/i386/gptzfsboot  sys/boot/i386/kgzldr
> sys/boot/i386/libfirewire  sys/boot/i386/libi386
> sys/boot/i386/loader  sys/boot/i386 /mbr  sys/boot/i386/pmbr
> sys/boot/i386/pxeldr  sys/boot/i386/zfsboot
> sys/boot/i386/zfsloader  sys/boot/efi/libefi
> sys/boot/userboot/ficl  sys/boot/userboot/libstand  sys/boot/use=20
> rboot/test  sys/boot/userboot/userboot  sys/boot/zfs

Note the exists() has a space in it and missing } and is all messed
up. The resulting DIRDEPS is proper though and lacks sys/boot/ficl and
sys/boot/forth

With MK_FORTH=3Dyes (note it does have sys/boot/ficl and sys/boot/forth
fine)

> ~/git/freebsd # MK_FORTH=3Dyes make -f
> targets/pseudo/userland/misc/Makefile.depend -V DIRDEPS=20
> rescue/librescue  rescue/rescue  etc/sendmail sys/boot/i386/boot0
> sys/boot/i386/boot0sio  sys/boot/i386/boot2  sys/boot/i386/btx/btx
> sys/boot/i386/btx/btxldr  sys/boot/i386/btx/li b
> sys/boot/i386/cdboot  sys/boot/i386/gptboot
> sys/boot/i386/gptzfsboot  sys/boot/i386/kgzldr
> sys/boot/i386/libfirewire  sys/boot/i386/libi386
> sys/boot/i386/loader  sys/boot/i386 /mbr  sys/boot/i386/pmbr
> sys/boot/i386/pxeldr  sys/boot/i386/zfsboot
> sys/boot/i386/zfsloader  sys/boot/efi/libefi
> sys/boot/userboot/ficl  sys/boot/userboot/libstand  sys/boot/use=20
> rboot/test  sys/boot/userboot/userboot  sys/boot/zfs sys/boot/ficl
> sys/boot/forth


Now with a newer bmake which has no errors:

> ~/git/freebsd # MK_FORTH=3Dno usr.bin/bmake/make -f
> targets/pseudo/userland/misc/Makefile.depend -V DIRDEPS=20
> rescue/librescue  rescue/rescue  etc/sendmail sys/boot/i386/boot0
> sys/boot/i386/boot0sio  sys/boot/i386/boot2  sys/boot/i386/btx/btx
> sys/boot/i386/btx/btxldr  sys/boot/i386/btx/li b
> sys/boot/i386/cdboot  sys/boot/i386/gptboot
> sys/boot/i386/gptzfsboot  sys/boot/i386/kgzldr
> sys/boot/i386/libfirewire  sys/boot/i386/libi386
> sys/boot/i386/loader  sys/boot/i386 /mbr  sys/boot/i386/pmbr
> sys/boot/i386/pxeldr  sys/boot/i386/zfsboot
> sys/boot/i386/zfsloader sys/boot/efi/libefi sys/boot/zfs

> ~/git/freebsd # MK_FORTH=3Dyes usr.bin/bmake/make -f
> targets/pseudo/userland/misc/Makefile.depend -V DIRDEPS=20
> rescue/librescue  rescue/rescue  etc/sendmail sys/boot/i386/boot0
> sys/boot/i386/boot0sio  sys/boot/i386/boot2  sys/boot/i386/btx/btx
> sys/boot/i386/btx/btxldr  sys/boot/i386/btx/li b
> sys/boot/i386/cdboot  sys/boot/i386/gptboot
> sys/boot/i386/gptzfsboot  sys/boot/i386/kgzldr
> sys/boot/i386/libfirewire  sys/boot/i386/libi386
> sys/boot/i386/loader  sys/boot/i386 /mbr  sys/boot/i386/pmbr
> sys/boot/i386/pxeldr  sys/boot/i386/zfsboot
> sys/boot/i386/zfsloader sys/boot/efi/libefi sys/boot/zfs
> sys/boot/ficl  sys/boot/forth






- --=20
Regards,
Bryan Drewery
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org

iQEcBAEBCgAGBQJWTkb1AAoJEDXXcbtuRpfPIYoH/2Gbu7/dTUEXpIueJuD73ZYH
PGMktZWsJzDUdQyKKQ+rGakBDmxDylEnXVo1G+RaCDlsLYzujacDE82JB86w005a
H2edxlTCI1A9ZZ/BZAt8XgESLQOMyAM2xnRt1Hgl31zGVacjjpwkz8URVK5ut+cx
JJkN34X5LOzX3PGawIkgTXks7ZS+9Y38GW26DJsOujS6rzGWFjm3t9A+vVVvuPt8
jXxD0i977xjuqyVpRD36UG901SOij2U0aTdx1tQ6o8R6CfT0PfKEC8RgsPs4vh5X
dHYsdlnTJWntMNpaJyO+NB+1ClOr79lrBpVt+Iiatsx2Ys8aH6wOk/8sczkpZxg=3D
=3Dz55G
-----END PGP SIGNATURE-----



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