[illumos-Developer] Review 693

[Gordon Ross]

On Wed, Jul 13, 2011 at 7:12 AM, Igor Kozhukhov <ikozhukhov at gmail.com> wrote:
> Hello All,
>
> Please review : integration Sun Devpro Math Library to illumos tree (693)
>
> http://igork.vlds.ru/nexenta/cr/illumos-693-webrev/

usr/src/Makefile.lint
This appears to be alphabetical, so please keep it that way.

usr/src/cmd/lms/Makefile
This part of the include path should come from $(ENVCPPFLAGS1)
I you use CPPFLAGS += ... this should come in automatically.

usr/src/lib/libsqlite/Makefile.com
  ditto CPPFLAGS += ...

usr/src/cmd/perl/skel/Makefile
  Ditto ENVCPPFLAGS1
  If you must set INC=... here, you should use the appropriate
  variables from usr/src/Makefile.master, i.e. INC=$(CPPFLAGS)

usr/src/lib/Makefile
  The first insert should be two lines later.
  The second and third should be located to
  keep those sections alphabetical.

usr/src/lib/libm/Makefile
  This is an unusual Makefile, doing the install work here,
  instead of in the subdir (per-arch) Makefiles.  Why?

usr/src/lib/libm/amd64/Makefile
  Wow!  That's one LONG Makefile.
  This could be shortened using pattern expansions...

So, popping "up a level" for a second, where did all these
come from?  Did you write them?  Or do you have diffs
against some previous version of this code?  Were there
sparc assembler files to match usr/src/lib/libm/src/i386?


Also, as Alex Eremin commented, I'd suggest getting rid of the
"src" directory level for new files matching these patterns:
  usr/src/lib/libm/src/mvec/*
  usr/src/lib/libm/src/C/*
  usr/src/lib/libm/src/LD/*
  usr/src/lib/libm/src/Q/*
  usr/src/lib/libm/src/R/*
  usr/src/lib/libm/src/complex/*
  usr/src/lib/libm/src/m9x/*

and for the following subdirs, I suggest moving:  (from, to)
  usr/src/lib/libm/src/i386/amd64/       usr/src/lib/libm/ml/amd64/
  usr/src/lib/libm/src/i386/common/    usr/src/lib/libm/ml/common/
("ml" for machine language, like under usr/src/uts)

Are there packaging changes still to do?
( usr/src/pkg/manifest/* )

Thanks,
Gordon

> What have been done:
> 1. integrated LIBM to illumos build for intel i386 and amd64
> 2. fixed compilation issues in packages for using headers from integrated
> LIBM
> Package system/library/math/header-math
>  with headers have been removed from system and build was using integrated
> LIBM headers
>
> This LIBM have been tested on OI-151 and NCP4.
>
> This version of LIBM don't have libmtsk*
>