James Carlson wrote: > April Chin writes: > > In particular, we're looking for code reviewers who can take a look at > > the non-Makefile files in > > http://cr.grommit.com/~chin/ksh93-webrev-jul9/jul9-nonattfiles > > Some notes: > > - The usual way for handling non-working lint targets is to make > sure that the directory in question is not linted during a > 'nightly' build (which is what $SRC/Makefile.lint is about). > Having non-working lint targets (as in $SRC/cmd/ksh/Makefile.com) > doesn't seem too helpful to me. > > The current scheme works, but could be better.
The current scheme tries to avoid that people run into trouble when they manually walk through the directories and do a "make lint" (see "perl" build for a precedent) and then start to cleanup the lint warnings (see earlier comments about a "lint [warning cleanup] party" ... > - It'd be nice (though not required) to have helper scripts to do > the grunt work required during the build process. In particular, > $SRC/cmd/kes/Makefile.testshell is pretty hard to read, Why ? It's only one extra layer of esacping rules (for example Imakefiles have at least two layers) ... :-) > with a > bunch of escapes that are complicated by the fact that it's coded > in the middle of a makefile. Two or three lines are ok, but 58 > lines seems like a lot. Erm... the problem is that it needs _lots_ of variables from the main Makefile and an external script would still require access to all those makefiles, e.g. you won't save much by replacing the current "in-Makefile" script with a giant list of variables passed to a script. And the script has 58 lines but around ~~18 extra lines are needed to squish the script into the 80-column format... ;-( ... but: If we switch SHELL=/usr/bin/ksh over to SHELL=/usr/bin/ksh93 at least ~~8-12 lines can be removed (e.g. all the "egrep" usage, the weired "while read ; print $REPLY ..."-loop etc.) Anyway... for now I would prefer the current implementation (= without a seperate script) because it is IMO easier (at least for me (and we already put the rules into a seperate Makefile include to avoid that normal engieers die from looking at it by accident... =:-) )) ... > - $SRC/cmd/sgs/libelf/Makefile.targ: why is a .WAIT needed before > $(ROOTDEMODIRS)? It looks like it should go with the rest of the > directory targets. (Probably just a nit.) Erm... the ".WAIT" rule should make sure that the directories really exist before creating files into it, e.g. first create dirs, then populate it. > - The private libc buried in libast (unlinted, no less) We're working on the lint part right now (and I still own David an email with feedback and I have to file a bug against Sun Studio 11's "lint" for reporting bogus warnings in C99 mode... ;-( ) ... > is still > frightening. Not an issue to hold up integration, but I hope that > something better will be done in the future. Supporting more than > one libc in ON seems like a hazard. Well, there are at least four items why this part is there: - Solaris libc functions not conforming to POSIX/XPG standards (this is mainly been killed by forcing the C99/XPG6 flag in our build, otherwise this issue would be MUCH MUCH worse) and updating them is tricky because backwards-compatibilty to old libc bugs needs to be provided - Historical limitations of the libc implementation (for example the Solaris libc stdio implementation where is no way to integrate the AST stdio functionality without breaking backwards compatibilty (which is a _pain_ because the AST stdio implementation is much more flexible and _FASTER_). In theory there is the option to provide a build-time switch and build all new applications with an all-new stdio implementation... but I don't even dare to imagine how the matching ARC case would look like (war ? bloodbath ?) ...) - libast's functions have lots of extensions, for example the regular expression functions support more pattern matching modes (e.g. perl+shell expressions are currently supported and more may be added later). In theory this could be moved over to libc (and likely very welcomed by the community) but ARC'ing will be tricky because of some problematic restrictions in the libc code (which could be avoided by leaving the old code alone and only enable the extensions in >= XPG6 mode... but I am not sure whether the standards people at Sun would be happy to have "extensions" in the "XPG6 standard mode") - Someone has to maintain the code. libast is maintained by David, Glenn and the AST community, including further development. If we put the functions into Solaris libc - who would do the syncing with upstream and/or doing further enhancements ? > - No changes were made to these files: > $SRC/lib/libcmd/amd64/Makefile > $SRC/lib/libcmd/i386/Makefile > $SRC/lib/libcmd/sparc/Makefile > $SRC/lib/libcmd/sparcv9/Makefile > Don't deliver unchanged files. What do you mean ? > - $SRC/pkgdefs/Makefile: looks like a missed merge. (Not a code > review issue, but you'll want to clean it up before going for > RTI.) Erm... what do you mean ? ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 7950090 (;O/ \/ \O;)
