Page MenuHomeFreeBSD

Makefile.inc1: Avoid using release/Makefile for VERSION.
ClosedPublic

Authored by bdrewery on Dec 16 2020, 8:36 PM.

Details

Summary

release/Makefile.inc1 has git executions that were being ran for each of
these lookups. The results were not needed so just lookup what we want
directly instead.

With an '.info here' in release/Makefile.inc1 this can be seen.

# /usr/bin/time env TARGET=amd64 TARGET_ARCH=amd64 make -f Makefile.inc1 -V LIBCOMPAT_OBJTOP
make[1]: "/root/git/freebsd2/release/Makefile.inc1" line 26: here
make[1]: "/root/git/freebsd2/release/Makefile.inc1" line 26: here
/scratch/obj/root/git/freebsd2/amd64.amd64/obj-lib32
        7.19 real         6.57 user         0.63 sys
# /usr/bin/time git rev-list --count HEAD
243484
        3.47 real         3.27 user         0.20 sys

Sponsored by: Dell EMC

Test Plan
# /usr/bin/time env TARGET=amd64 TARGET_ARCH=amd64 make -f Makefile.inc1 -V _BRANCH -V _REVISION -V VERSION
CURRENT
13.0
FreeBSD 13.0-CURRENT amd64 1300132
        0.15 real         0.05 user         0.10 sys

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Makefile.inc1
544

Copied this from release/Makefile and added an extra _ prefix

I'm not an expert in the build, but it LGTM. I only wonder if there ought to be a common makefile for this and release/Makefile both to include.

This revision is now accepted and ready to land.Dec 16 2020, 9:49 PM

For reference, originally introduced in rS257079

I'm not an expert in the build, but it LGTM. I only wonder if there ought to be a common makefile for this and release/Makefile both to include.

<ranty fickle reply>
Not duplicating code is definitely my ingrained desire. I tend to over-generalize so I've tried to compromise occasionally rather than a bunch of small share/mk/ files.

Makefile.inc1:_${_V}!=  eval $$(awk '/^${_V}=/{print}' ${SRCTOP}/sys/conf/newvers.sh); echo $$${_V}
release/Makefile:${_V}!=        eval $$(awk '/^${_V}=/{print}' ${.CURDIR}/../sys/conf/newvers.sh); echo $$${_V}
release/Makefile.vm:${_V}!=     eval $$(awk '/^${_V}=/{print}' ${.CURDIR}/../sys/conf/newvers.sh); echo $$${_V}
share/mk/local.meta.sys.mk:FREEBSD_REVISION!= sed -n '/^REVISION=/{s,.*=,,;s,",,g;p; }' ${SRCTOP}/sys/conf/newvers.sh

I'm open to consolidating these into a share/mk/src.newvers.mk but then I start thinking what should it provide? Should we need to set things like WANT_BRANCH and WANT_REVISION thus increasing SLOC? See I'm overthinking it! Could just move all of this to release/Makefile.version but then I think it's an odd place for something like share/mk/local.meta.sys.mk to be including.

For reference, originally introduced in rS257079

The dependency yeah. The need for VERSION is oddly just for ctfmerge from rS179233. The ctfmerge usage came in right before it in rS179184

Please hold off on this change for at least a day or so. There are a few places that I am aware of that might not handle this well, as-is, so I would like some time to dig into this further.