Peter Memishian wrote: > > > * Webrev only over files which match the substring "makefile": > > > http://www.nrubsig.org/people/gisburn/work/solaris/ksh93_integration/ksh93_integration_prototype005_webrev_20070514/makefile_files/webrev/ > > I haven't had time to go through everything yet, but here a few general > things that caught my eye: > > * 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 > * A number of places use "COBJS" rather than "OBJECTS" directly -- > e.g.: > > COBJS= \ > dlfcn.o \ > dllfind.o \ > dlllook.o \ > dllnext.o \ > dllplug.o \ > dllscan.o > > 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 ? > * 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). Another reason is the 3rdparty license stuff - AFAIK if we put non-AST sources in such a dir the legal phases seem to be different (erm, or maybe not... I am not good in such lawyer stuff... my approach for these legal issues is to hide under my desk until they go away and attack someone else... ;-( ). > -- but > at worst you should need to redefine MAPFILES, not the other two. I'll fix that once by build is done... > * 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, including that we aim at replacing /usr/xpg4/bin/sh in the future, standards conformance and simply for the little detail that the upstream sources only compile in C99 mode when "-D_XOPEN_SOURCE=600 -D__EXTENSIONS__=1" are set - otherwise the Solaris headers trip over an "#error" statement in /usr/include/sys/feature_tests.h where the comments say: -- snip -- /* * It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application * using c99. The same is true for POSIX.1-1990, POSIX.2-1992, POSIX.1b, * and POSIX.1c applications. Likewise, it is invalid to compile an XPG6 * or a POSIX.1-2001 application with anything other than a c99 or later * compiler. Therefore, we force an error in both cases. */ #if defined(_STDC_C99) && (defined(__XOPEN_OR_POSIX) && !defined(_XPG6)) #error "Compiler or options invalid for pre-UNIX 03 X/Open applications \ and pre-2001 POSIX applications" #elif !defined(_STDC_C99) && \ (defined(__XOPEN_OR_POSIX) && defined(_XPG6)) #error "Compiler or options invalid; UNIX 03 and POSIX.1-2001 applications \ require the use of c99" #endif -- snip -- This works in both directions. BTW: This has been discussed before, see http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-February/002295.html for another detailed answer about this design decision (e.g. XPG6 is used by design (and to avoid that we trigger all the workaroounds for non-POSIX code in the ksh93 sources (which only cost performance))). I've updated the comments look like this: -- snip -- # Override this top level flag so the compiler builds in its native # 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 -- snip -- Better ? > * 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...). > Please > explain why the special mkpcdirs handling is necessary. "mkpcdirs" was added during the previous review cycles since doing a plain "mkdir -p subdir_of_objfile" as part of the compiler rule wasn't favoured by the reviewers... ;-( ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 7950090 (;O/ \/ \O;)
