Now that there's other people looking at the X makefiles, I figured
I'd try posting a code review of the big cleanup I'm working on.
It started when I went looking at the ON makefiles for an idea of
how to set a default in the master included makefile but override
it in just a few specific makefiles.
That led me to this bit of ON's Makefile.master:
#
# The declaration POUND_SIGN is always '#'. This is needed to get around the
# make feature that '#' is always a comment delimiter, even when escaped or
# quoted. The only way of generating this is the :sh macro mechanism. Note
# however that in general :sh macros should be avoided in makefiles that are
# widely included into other makefiles, as the resulting shell executions can
# cause a noticable slowdown in build times.
#
POUND_SIGN:sh= echo \\043
[...]
# If CLOSED_IS_PRESENT is not set, assume the closed tree is present.
CLOSED_BUILD_1= $(CLOSED_IS_PRESENT:yes=)
CLOSED_BUILD= $(CLOSED_BUILD_1:no=$(POUND_SIGN))
EXPORT_RELEASE_BUILD= $(POUND_SIGN)
$(CLOSED_BUILD)EXPORT_RELEASE_BUILD=
While I stole that idea, I also took note of the comment and went to look
at the many bits of the X Makefiles that do stuff like:
DEFAULT_sun4_ARCH32_FLAGS = -xarch=v8plus
DEFAULT_i86pc_ARCH32_FLAGS = -xpentium -xregs=no%frameptr
ARCH32_FLAGS:sh=arch | sed 's/^\(.*\)$/\$\(DEFAULT_\1_ARCH32_FLAGS\)/'
This is both expensive (causes sh, arch & sed to fork on every Makefile
parse) and hard to read. Looking more at the ON makefiles I found I
could rewrite this to be both less work and more readable as:
MACH:sh=uname -p
DEFAULT_ARCH32_FLAGS_sparc = -xarch=v8plus
DEFAULT_ARCH32_FLAGS_i386 = -xpentium -xregs=no%frameptr
ARCH32_FLAGS = $(DEFAULT_ARCH32_FLAGS_$(MACH))
By running uname once for $MACH and just using it's result everywhere,
it got rid of a lot of unneccessary forks - dropping about 5 minutes
off the 2.5 hour build time on the build machine I tested on.
So I went wild cleaning up this stuff across our open-src Makefiles,
as well as making the Makefiles more consistent (now all architecture
specific variables end in _sparc or _i386, not mixing in _sun4 or
_i86pc as some cases had before, nor putting the architecture part in the
middle of the name).
So for your heads up and code reviewing pleasure, I've posted a webrev at:
http://cr.grommit.com/~alanc/6551329/
While the files are listed in alphabetical order, open-src/common/Makefile.inc
is probably the one to start with to understand the rest better that refer to
its changes.
I've tested these by diffing buildit logs against those from the workspace
before the changes and not found any unexpected differences in the output.
Please let me know what you think - if everyone's satisfied by Monday, these
can go into nv_65 (our build for nv_65 closes that evening).
--
-Alan Coopersmith- alan.coopersmith at sun.com
Sun Microsystems, Inc. - X Window System Engineering
