Peter Memishian wrote:
> 
>  > >         * A number of places do something like this:
>  > >
>  > >             #
>  > >             # ksh is not lint-clean yet.  Fake up a target.
>  > >             #
>  > >             lint:
>  > >                     @ print "usr/src/cmd/ksh is not lint-clean: skipping"
>  > >                     @ $(TRUE)
>  > >
>  > >           The above doesn't seem consistent with the way we handle this
>  > >           elsewhere in ON.  Instead, the general rule is to have a normal
>  > >           lint target that spews warnings, and simply omit the directory
>  > >           from $(SRC)/Makefile.lint
>  >
>  > AFAIK (if I remeber this correctly) we tried that and it didn't work and
>  > then we've copied the solution of the "perl" built - see
>  > 
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/cmd/perl/Makefile#59
>  > The original "Incomplete Tourist Guide" covered this item - see
>  > 
> http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2006-June/000433.html
> 
> It mentions it, but I don't see a justification.  We generally only use
> that for things we know we will never make lint-clean -- and there are
> only two examples of that which I see in ON (fmli and perl).  Everything
> else works the way I suggested.

For now ksh93 is not lint clean (it may become "lint" clean with minor
exceptions (silenced via "-erroff=") in the future - if you follow the
evolution of ksh93s- to ksh93s+ you'll notice that _lots_ of cleanup
work (e.g. compiler warnings, lint warnings, issues reported by Rational
Purify and "dbx -check access", Klockwork.com did us a favour and ran
their tools over the sources and reported the results back to
David&&Glenn etc.) has been done, not only for ksh93 but for the whole
AST codebase (which rivals OS/Net in size)), therefore I followed the
"perl" example.

I've did a quick search through my mightly email archive... AFAIK the
original reason for doing it was to avoid that people do a "make lint ;
hit errors and then randomly start patching the sources at the next code
cleanup party" ...

>  > >         * A number of places use "COBJS" rather than "OBJECTS" directly 
> --
>  > >
>  > >            Unless there's some problem with OBJECTS, it should be used
>  > >            directly.  That way, macros like SRCS will work automatically.
>  >
>  > Erm, I've copied that from other Makefiles which used COBJS. It seems
>  > both styles are used within OS/Net - which one is the preferred ?
> 
> usr/src/lib/README.Makefiles recommends OBJECTS and never suggests COBJS.

Ok... done (see attached patch) ...

>  > >         * A number of places have stuff like:
>  > >
>  > >             # mapfile-vers does not live in common/ because this 
> directory
>  > >             # is for AST code only
>  > >             MAPFILES=       ../mapfile-vers
>  > >             MAPOPTS=        $(MAPFILES:%=-M %)
>  > >             DYNFLAGS +=     $(MAPOPTS)
>  > >
>  > >           It's not clear to me why common/ is "for AST code only"
>  >
>  > This was done to simplify updates (e.g. to avoid that an semi-automated
>  > update prodecure doesn't crush other sources to death).
> 
> But the trade-off is that it's different from almost every other library
> Makefile

Uhm... AFAIK there are plenty of other libraries who do that. Roger A.
Faulkner wrote in
http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2006-August/000782.html
-- snip --
I recommend, given the complexity of the proposed directory
hierarchy, to just put mapfile-vers in the top-level directory
(Option 2. above).  There are plenty of other libraries that
do this.
-- snip --

> But if you really believe it simpifies updates, I'm OK with it
> -- just make that clear in the comment

Any suggestions for the comment (I hoped the current one was clear
enougth) ? Would...
-- snip --
# mapfile-vers does not live in common/ because this directory
# is for AST code only (e.g. to make automated code updates
# easier)
-- snip --
... be Ok ?

> (and remove the MAPOPTS and
> DYNFLAGS stuff).

Already done (see attached patch) ...

>  > >         * A number of places have stuff like:
>  > >
>  > >             # Override this top level flag so the compiler builds in its
>  > >             # native C99 mode.  This has been enabled to support the math
>  > >             # stuff in ksh93.
>  > >             C99MODE= $(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1
>  > >
>  > >           I'm not sure what the two -D directives have to do with 
> enabling
>  > >           C99 mode, though.
>  >
>  > We compile ksh93 as C99/XPG6 application for a number of reasons,
> 
> I'm not objecting to that.  I'm pointing out that C99MODE is being abused
> to enable XPG6 as well.

Uhm... I don't think this is an abuse...

> Why aren't those #defines in CPPFLAGS?

... because these flags work&&act together. You can't enable C99 mode
without XPG6 (in this case) and you can't enable XPG6 without enabling
C99. That's why these flags are "together" (and to make sure these flags
don't get changed by accident since it my blow-up other bits in subtle
ways and then you spend hours with debugging (again)).

In theory we could add something like a "C99XPG6ENABLE" value (which
expands to "$(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1") for
"C99MODE" - would that be usefull (please read the comment in
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/cmd/getconf/Makefile
about XPG6MODE, this stuff is slightly tricky and has some holes with
poisoned spikes at the bottom... ;-( ) ?

>  > >         * I don't really understand what the "confusion with having too
>  > >           many object files in the toplevel pics/ directory" is.
>  >
>  > It's my desire to avoid having hundreds of object files in one subdir
>  > when it is possible to put the object files into subdirs which follow
>  > the subdir layout of the source code (e.g. see Mozilla codebase for
>  > another example). IMO it's cleaner and easier to navigate (at least for
>  > humans) with subdirs than using the DOS1.0-style and dump all objects
>  > into one flat directory (I have a similar patch for libc which should
>  > solve the madness in those Makefiles, too...).
> 
> Do you really often navigate into "pics"?  Why?  In any case, I am against
> special-purpose library-specific tweaks like this -- either the stuff
> should find its way into lib/Makefile.*, or be yanked out.

Well, I think seperate objdirs following the source layout is cleaner
and IMO it scales better than the "few hundred files in one flat
dir"-solution (e.g. 10 dirs with 10 subdirs containing 10 subdirs
containing 10 files each is AFAIK faster to access&&handle than all
files in one flat dir (however the filename cache will hide some of the
performance bottlenecks)). As I said I had a patch for libc (AFAIK
B37-based) and could write one to do the same tree-wide to get some
things cleaned-up - if such a change is Ok...

... I could remove the seperate objdir stuff for all libraries except
libast (which is very special in many cases and needs it to fetch the
autogenerated sources from $(TRANSMACH) (and no, the sources cannot be
moved easily... ;-/ )) if there is the consens that the "subdirs in
objdir" is the wrong approach...


I've attached the current changes as
"ksh93_integration_webrev20070514_update001.diff.txt" (still missing the
bits which are under discussion above) ...

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) [EMAIL PROTECTED]
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 7950090
 (;O/ \/ \O;)
Index: src/lib/libshell/Makefile.com
===================================================================
--- src/lib/libshell/Makefile.com       (revision 701)
+++ src/lib/libshell/Makefile.com       (working copy)
@@ -28,7 +28,7 @@
 LIBRARY=       libshell.a
 VERS=          .1
 
-COBJS= \
+OBJECTS= \
        bltins/alarm.o \
        bltins/cd_pwd.o \
        bltins/cflow.o \
@@ -90,19 +90,17 @@
        sh/waitevent.o \
        sh/xec.o
 
-OBJECTS=       $(COBJS) $(MOBJS)
-
 # We are storing the object files into subdirs avoid the
 # confusion with having too many object files in the toplevel pics/
 # directory (this matches the way how the original AST build system
 # deals with this "logistic" issue) - the rules below ensure that
 # the destination directory is available.
-COBJDIRS =  \
+OBJDIRS =  \
        bltins \
        data \
        edit \
        sh
-PICSDIRS= $(COBJDIRS:%=pics/%)
+PICSDIRS= $(OBJDIRS:%=pics/%)
 mkpicdirs:
        @mkdir -p $(PICSDIRS)
 
@@ -113,15 +111,12 @@
 # mapfile-vers does not live in common/ because this directory
 # is for AST code only
 MAPFILES=       ../mapfile-vers
-MAPOPTS=        $(MAPFILES:%=-M %)
-DYNFLAGS +=     $(MAPOPTS)
 
 # Override this top level flag so the compiler builds in its native
-# C99 mode.  This has been enabled to support the math stuff in ksh93.
+# C99+XPG6 mode.  This has been enabled to support the math stuff
+# and other features in ksh93.
 C99MODE= $(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1
 
-SRCS=          $(COBJS:%.o=../common/%.c)
-
 LIBS =         $(DYNLIB) $(LINTLIB)
 
 # load dll, socket, nsl and secdb libraries on demand
Index: src/lib/libcmd/Makefile.com
===================================================================
--- src/lib/libcmd/Makefile.com (revision 701)
+++ src/lib/libcmd/Makefile.com (working copy)
@@ -76,11 +76,10 @@
 # mapfile-vers does not live in common/ because this directory
 # is for AST code only
 MAPFILES=       ../mapfile-vers
-MAPOPTS=        $(MAPFILES:%=-M %)
-DYNFLAGS +=     $(MAPOPTS)
 
 # Override this top level flag so the compiler builds in its native
-# C99 mode.  This has been enabled to support the math stuff in ksh93.
+# C99+XPG6 mode.  This has been enabled to support the math stuff
+# and other features in ksh93.
 C99MODE= $(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1
 
 LIBS =         $(DYNLIB) $(LINTLIB)
Index: src/lib/libdll/Makefile.com
===================================================================
--- src/lib/libdll/Makefile.com (revision 701)
+++ src/lib/libdll/Makefile.com (working copy)
@@ -28,7 +28,7 @@
 LIBRARY=       libdll.a
 VERS=          .1
 
-COBJS= \
+OBJECTS= \
        dlfcn.o \
        dllfind.o \
        dlllook.o \
@@ -36,9 +36,6 @@
        dllplug.o \
        dllscan.o
 
-
-OBJECTS= $(COBJS)
-
 include ../../libast/Makefile.libastl10n
 
 include ../../Makefile.lib
@@ -46,15 +43,12 @@
 # mapfile-vers does not live in common/ because this directory
 # is for AST code only
 MAPFILES=       ../mapfile-vers
-MAPOPTS=        $(MAPFILES:%=-M %)
-DYNFLAGS +=     $(MAPOPTS)
 
 # Override this top level flag so the compiler builds in its native
-# C99 mode.  This has been enabled to support the math stuff in ksh93.
+# C99+XPG6 mode.  This has been enabled to support the math stuff
+# and other features in ksh93.
 C99MODE= $(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1
 
-SRCS=          $(COBJS:%.o=../common/%.c)
-
 LIBS =         $(DYNLIB) $(LINTLIB)
 LDLIBS +=      -last -lc
 $(LINTLIB) :=  SRCS = $(SRCDIR)/$(LINTSRC)
Index: src/lib/libast/sparcv9/Makefile
===================================================================
--- src/lib/libast/sparcv9/Makefile     (revision 698)
+++ src/lib/libast/sparcv9/Makefile     (working copy)
@@ -35,9 +35,9 @@
 ASTPLATFORMCPPFLAGS = '-DHOSTTYPE="sol$(GETRELEASEMINOR:sh).sun4"'
 
 # platform-specific sources
-COBJS = \
+OBJECTS = \
        sparcv9/src/lib/libast/conftab.o
-COBJDIRS = \
+OBJDIRS = \
        sparcv9/src/lib/libast
 
 include ../Makefile.com
Index: src/lib/libast/sparc/Makefile
===================================================================
--- src/lib/libast/sparc/Makefile       (revision 698)
+++ src/lib/libast/sparc/Makefile       (working copy)
@@ -35,9 +35,9 @@
 ASTPLATFORMCPPFLAGS = '-DHOSTTYPE="sol$(GETRELEASEMINOR:sh).sun4"'
 
 # platform-specific sources
-COBJS = \
+OBJECTS = \
        sparc/src/lib/libast/conftab.o
-COBJDIRS = \
+OBJDIRS = \
        sparc/src/lib/libast
 
 include ../Makefile.com
Index: src/lib/libast/i386/Makefile
===================================================================
--- src/lib/libast/i386/Makefile        (revision 698)
+++ src/lib/libast/i386/Makefile        (working copy)
@@ -35,9 +35,9 @@
 ASTPLATFORMCPPFLAGS = '-DHOSTTYPE="sol$(GETRELEASEMINOR:sh).i386"'
 
 # platform-specific sources
-COBJS = \
+OBJECTS = \
        i386/src/lib/libast/conftab.o
-COBJDIRS = \
+OBJDIRS = \
        i386/src/lib/libast
 
 include ../Makefile.com
Index: src/lib/libast/Makefile.com
===================================================================
--- src/lib/libast/Makefile.com (revision 701)
+++ src/lib/libast/Makefile.com (working copy)
@@ -29,7 +29,7 @@
 VERS=          .1
 
 # platform-independent sources are in common/
-COBJS += \
+OBJECTS += \
        common/cdt/dtclose.o \
        common/cdt/dtdisc.o \
        common/cdt/dtextract.o \
@@ -601,14 +601,12 @@
        common/vmalloc/vmtrace.o \
        common/vmalloc/vmwalk.o
 
-OBJECTS= $(COBJS) $(MOBJS)
-
 # We are storing the object files into subdirs avoid the
 # confusion with having 550+ object files in the toplevel pics/
 # directory (this matches the way how the original AST build system
 # deals with this "logistic" issue) - the rules below ensure that
 # the destination directory is available.
-COBJDIRS += \
+OBJDIRS += \
        common/cdt \
        common/comp \
        common/dir \
@@ -627,7 +625,7 @@
        common/uwin \
        common/vec \
        common/vmalloc
-PICSDIRS= $(COBJDIRS:%=pics/%)
+PICSDIRS= $(OBJDIRS:%=pics/%)
 mkpicdirs:
        @mkdir -p $(PICSDIRS)
 
@@ -645,14 +643,15 @@
 # mapfile-vers does not live in common/ because this directory
 # is for AST code only
 MAPFILES=       ../mapfile-vers
-MAPOPTS=        $(MAPFILES:%=-M %)
-DYNFLAGS +=     $(MAPOPTS)
 
 # Override this top level flag so the compiler builds in its native
-# C99 mode.  This has been enabled to support the math stuff in ksh93.
+# C99+XPG6 mode.  This has been enabled to support the math stuff
+# and other features in ksh93.
 C99MODE= $(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1
 
-SRCS=          $(COBJS:%.o=../%.c)
+# special rule because sources live both ../common (normal)
+# and $(TRANSMACH) (generated)
+SRCS=          $(OBJECTS:%.o=../%.c)
 
 LIBS =         $(DYNLIB) $(LINTLIB)
 
Index: src/lib/libast/amd64/Makefile
===================================================================
--- src/lib/libast/amd64/Makefile       (revision 698)
+++ src/lib/libast/amd64/Makefile       (working copy)
@@ -35,9 +35,9 @@
 ASTPLATFORMCPPFLAGS = '-DHOSTTYPE="sol$(GETRELEASEMINOR:sh).i386"'
 
 # platform-specific sources
-COBJS = \
+OBJECTS = \
        amd64/src/lib/libast/conftab.o
-COBJDIRS = \
+OBJDIRS = \
        amd64/src/lib/libast
 
 include ../Makefile.com
Index: src/lib/libpp/Makefile.com
===================================================================
--- src/lib/libpp/Makefile.com  (revision 701)
+++ src/lib/libpp/Makefile.com  (working copy)
@@ -28,7 +28,7 @@
 LIBRARY=       libpp.a
 VERS=          .1
 
-COBJS= \
+OBJECTS= \
        ppargs.o \
        ppbuiltin.o \
        ppcall.o \
@@ -54,8 +54,6 @@
        ppsearch.o \
        pptrace.o
 
-OBJECTS= $(COBJS)
-
 include ../../libast/Makefile.libastl10n
 
 include ../../Makefile.lib
@@ -63,15 +61,12 @@
 # mapfile-vers does not live in common/ because this directory
 # is for AST code only
 MAPFILES=       ../mapfile-vers
-MAPOPTS=        $(MAPFILES:%=-M %)
-DYNFLAGS +=     $(MAPOPTS)
 
 # Override this top level flag so the compiler builds in its native
-# C99 mode.  This has been enabled to support the math stuff in ksh93.
+# C99+XPG6 mode.  This has been enabled to support the math stuff
+# and other features in ksh93.
 C99MODE= $(C99_ENABLE) -D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1
 
-SRCS=          $(COBJS:%.o=../common/%.c)
-
 LIBS =         $(DYNLIB) $(LINTLIB)
 LDLIBS +=      -last -lc
 $(LINTLIB) :=  SRCS = $(SRCDIR)/$(LINTSRC)
_______________________________________________
opensolaris-code mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/opensolaris-code

Reply via email to