I looked at the makefiles for Roland's changes, using
svn diff -r 178:287
http://svn.genunix.org/repos/on/branches/ksh93/gisburn/prototype002/m1_ast_ast_imported
I've got a bunch of comments and things I'd like to see changed, but
before I get to that, I want to thank Roland for (a) doing a huge pile
of work, and (b) being patient with me (I know I haven't been responding
as quickly as anyone would like).
Also, I should mention that I'll be away from email starting sometime
tomorrow (2006-06-23) through sometime in the middle of next week.
Anyway...
Overall, I think it's in pretty good shape. Some specific issues:
* cmd/ksh/Makefile.com
- why is GROUP set to root? The current Solaris ksh88 is installed as
group bin.
* usr/src/cmd/ksh/Makefile.ksh93switch
- for putback, BUILD_KSH93_AS_BINKSH will need to be 0.
* usr/src/cmd/ksh/Makefile.com
usr/src/lib/libshell/Makefile.com
- CPPFLAGS: it would better to have a single makefile fragment for
-D/-U that is included by these two makefiles
* usr/src/cmd/ksh/Makefile.com
- install target: why add the "set -x"?
* usr/src/cmd/ksh/Makefile
- where are we with i18n? will we have a real ksh93.po prior to
putback?
* usr/src/lib/libshell/common/Makefile
usr/src/lib/libdll/common/Makefile
usr/src/lib/libast/common/Makefile
- I think that having these files in the ON tree will cause confusion.
I'd like either to remove them or explain in README's that this file
is not used for ON. (Maybe use
lib/<library>/common/README.opensolaris?)
* usr/src/lib/libshell/common/Makefile.com
usr/src/lib/libcmd/Makefile.com
usr/src/cmd/ksh/Makefile.com
usr/src/lib/libdll/Makefile.com
usr/src/lib/libast/Makefile.com
- CPPFLAGS: what breaks when you put /usr/include in front of the
include list? Several other ON components deal with this by using
CPPFLAGS = <stuff> $(CPPFLAGS.master)
If that would work here, I think we should do it--it should help
maintainability of the ON code as a whole.
* usr/src/lib/libshell/Makefile.com
usr/src/lib/libcmd/Makefile.com
usr/src/lib/libast/Makefile.com
- I see that the workaround hack for the PAGESIZE redefinition is
still in place. This will need to be changed prior to putback. (At
least I assume that the "CERRWARN += -erroff=E_MACRO_REDEFINED" in
libast/Makefile.com is for PAGESIZE and nothing else.)
* usr/src/lib/libshell/Makefile
usr/src/lib/libdll/Makefile
usr/src/lib/libast/Makefile
- these still have the comment "64bit disabled for now due lack of
AMD64 build machine". I thought the issue was that the 64-bit
support is broken, and that it's being fixed upstream.
* usr/src/lib/Makefile
- you shouldn't need the new .WAIT's (for libast et al), since you
have the explicit dependencies listed further down in the makefile
* usr/src/lib/libdll/sparc/Makefile
usr/src/lib/libdll/i386/Makefile
usr/src/lib/libast/sparc/Makefile
usr/src/lib/libast/i386/Makefile
- HOSTTYPE still has the OS release number hardwired in. If this
won't actually cause problems for future releases (when uname -r
moves to 5.12), then we can treat this as a cleanup nit, I think.
* usr/src/lib/libast/i386/Makefile
usr/src/lib/libast/sparc/Makefile
usr/src/lib/libast/Makefile.com
- these all have code to enable PIC; does that really need to be
in all 3 places?
Okay, that's it.
cheers,
mike
