[illumos-Developer] Review 693

Richard Lowe richlowe at richlowe.net
Wed Jul 13 08:18:37 PDT 2011


> At this moment it is first integration and we can change what we need later
> - maybe update to new versions, maybe restructure if needed.
>

No, we need to get it right the first time.  90% of the really
terrible parts of our source tree are from the integration of formally
separate projects to be "fixed up later", it almost always goes
horribly wrong (look at usr/src/man and the lp sources, if you want
examples)

- I'm generally concerned by the need to add -I$(ROOT)/usr/include and
variations to Makefiles.  Were all these components previously broken,
and not using the proto includes?  In general, if something does
"CPPFLAGS =" the bug is that they're not appending to CPPFLAGS, rather
than anything specific about the contents.

  Using just $(ROOT)/... means that these components will ignore
ENVCPPFLAGS even after you're done.

  In a couple of cases you seem to be adding $(ROOT)/usr/include even
though CPPFLAGS _is_ being appended to.  What am I missing?

- Remove the SCCS keywords throughout (the #pragma ident, #ident, etc, lines)

usr/src/lib/Makefile:

  I'd far prefer that rather than using .WAIT after libm, that you
call out the dependencies explicitly (I'm assuming there are few
enough of them that this is worthwhile, if there aren't, do let me
know).  The use of .WAIT in there is almost entirely historical and
buggy (libc is a special case, obviously).

usr/src/lib/libm/amd64/Makefile:

 The presence of any mention of sparc in here is deeply suspicious.
Should a bunch of this be deleted?  In a Makefile.com (it certainly
looks like a bunch of it should be in a Makefile.com)

  34:
    Don't use SUNW_ISA (and an :sh) rule, just use $(MACH64) or
$(MACH) as appropriate.

  40,42:  Remove commented code

  70,75: All of this should be removed, and Makefile.master allowed to
do the same thing.

  138: Almost certainly shouldn't be there.

  145: I can't immediately tell what the libm folks were doing there,
but I'd vastly prefer that they didn't.

  154,155: Remove commented code

  169: Should be available from the make structure described in lib/README*

  441: Remove commented objects

  925: Remove commented rule

  943,1033: More libm crazyness.  Do we actually make use of these rules?

usr/src/lib/libm/amd64/mapfiles/*-sparc, v9a, v9b, etc.

  Why are there sparc mapfiles in this amd64 directory?

usr/src/lib/libm/i386/Integration.log:  Delete

usr/src/lib/libm/i386/Makefile:

  A bunch of (most) of this is duplicated with amd64/Makefile.  Put
the common pieces in usr/src/lib/libm/Makefile.com and include them.
(otherwise, all my comments from the amd64 Makefile apply here too).

usr/src/lib/libm/i386/llib-lm:  This looks the same as the amd64 copy,
if it is, it should be common, not duplicated.

usr/src/lib/libm/i386/mapfiles/*:  Same comments as for amd64, but if
these are all duplicated, make them common, don't have two copies.

I can't tell if the weird libm source organization is too well baked
in to change to be conventional (common sources in common/ platform
specifics in the i386/ and sparc/ top-level directories, etc.), if it
turns out after fixing the Makefile comments above that this is easy,
it'd be great if you did it.

I haven' looked at any of the libm sources, just the build
infrastructure.  I'll read through your diffs as compared to the libm
tarball in a while (or, hopefully, someone else will beat me to it)

Thanks for doing this,

-- Rich



More information about the Developer mailing list