Hi Paul, Thanks for your feedback.
Paul Cunningham wrote: > George, > > This mainly looks good to me, but see comments below ... > > George Vasick wrote: >> >> Please review my changes for the upgrade of autoconf from 2.61 to 2.63: >> >> http://cr.opensolaris.org/~gvasick/6838600/ > > 1. usr/src/cmd/autoconf/Makefile.sfw > Change 'env ' to 'env - ' on line ... > 51 cd $(VER); env SRC=$(SRC) \ > > Do these need to be defined ? ... > 53 BINDIR=$(ROOT)/usr/bin \ > 54 MANDIR=$(ROOT)/usr/share/man/man1 \ These seem to be required for the sunman script to execute. Without them, the script is not executed. > > Line ... > 72 ./configure --prefix=/usr) > could be changed so it uses the predefined '--prefix=' > from Makefile.master, eg. change to ... > $(SHELL) ./configure $(CONFIGURE_OPTIONS)) CONFIGURE_OPTIONS set both prefix and mandir. mandir is currently set in the install target. I think we only want to set prefix here. With respect to $(SHELL), the configure script specifies "#! /bin/sh" while SHELL is set to ksh93. This script happens to work with either shell, however I have seen other configure scripts fail if $(SHELL) is used. In grep'ing through the other Mafefile.sfw files, I see the usage is split between "./configure" and "$(SHELL) ./configure". I am not sure what the best practice on this should be. > > > Is the sunman-stability stuff getting added to the man pages? > ($(SRC)/tools/protofix --manscript ....) Yes. See previous comments. > > 2. usr/src/pkgdefs/SUNWaconf/copyright > Is that the correct 'Sun disclaimer' at the top, it looks > different/shorter to most others I've seen. The recent reviews I have seen all came back from legal review with this new, shorter disclaimer. > > You should probably add the source-owner copyright lines, > extracted from the tarball files, after the Sun disclaimer Good catch. Fixed. Thanks, George > > END
