Peter Memishian wrote:
>
> > > > > > > > http://cr.grommit.com/~chin/ksh93-webrev-jun29/jun29-makefiles/
> >
> > > As per your follow-up mail, I know you went ahead with this -- but did
> you
> > > also do the KSHCPPFLAGS bit?
> >
> > No... I didn't like the idea much because it creates another Makefile
> > include for one flag and two consumers...
> >
> > > That part's especially important to me
> > > because without it we may have drift between the two definitions, and I
> > > don't see the particular need to wait for "shcomp".
> >
> > Grumpf... ;-(
> > ... what about adding a usr/src/Makefile.libshell (usr/src/Makefile.ast
> > is IMO too generic and the flags are for libshell consumers, not ksh
> > consumers (Ok... nitpicking... :-) )) ?
>
> I was expecting you'd use the existing usr/src/Makefile.ast.
Grumble... ;-(
... Ok... :-)
Fixed (the flag is called "LIBSHELLCPPFLAGS").
> I think it's
> the best compromise; I certainly prefer it to manually keeping the two
> lists in-sync.
Mhhh... yes... but now we have library-specific (e.g. libshell) flags in
a "global" Makefile include (e.g. usr/src/Makefile.ast) ... ;-(
> > > > > > > lib/libdll/{sparc,sparcv9,amd64,i386}/Makefile:
> > >
> > > Anyway, like I said before, if you want to delay splitting apart RELEASE
> > > into RELEASE_MAJOR and RELEASE_MINOR, I'm OK with that. But a bug should
> > > be filed on the issue so that we can clean up the ksh93 Makefiles and
> > > other victims of the current Makefile.master logic in the future.
> >
> > Ok...
> > ... what about adding a RELEASE_BUILDNUMBER to reflect the Nevada build
> > version (e.g. B61), too ?
>
> I think that's problematic. First, someone (or something) would have to
> do an integration to update it every build.
It seems someone is already doing that anyway:
-- snip --
$ uname -v
snv_61
-- snip --
> Second (and more
> importantly), we don't want software depending on the build number (it's
> bad enough some software depends in the major/minor numbers).
Mhhh... but AFAIK OS/Net has similar things in the kernel code to handle
CPU erratas depending on build flags, OBP/BIOS stuff and other things...
> > And what about adding an "ON_"-prefix (e.g. "ON_RELEASE_MAJOR",
> > "ON_RELEASE_MINOR", "ON_RELEASE", "ON_RELEASE_BUILDNUMBER",
> > "ON_VERSION", e.g. add a "namespace" called "ON_" for such tree-global
> > flags ?
>
> Sounds reasonable for new variables, but RELEASE is pretty well-known --
> e.g., nightly(1) environment scripts commonly make use of it.
xx@@@!!!... ;-(
It is only "nightly" itself or are there any other scripts which depend
on "RELEASE", too ?
> I agree
> that the generic names problematic.
Is there any rule against global, single-word Makefile variables with
less then four letters in the Makefile style guide yet ?
> > Grumpf... problem is that when I run the test suite over all locales
> > then the test run will abort at the first locale iteration (note that
> > the test suite is called for each of the locales defined in
> > ON_KSH_TEST_LOCALES to make sure we catch any multibyte locale bugs
> > (since I want to cleanup once and for all with all the xxx@@@!!!
> > multibyte bugs in Solaris)) ... and there is no (easy) way to make the
> > list of exceptions variable (well, wrong shell (or better: Too old...
> > the idea would be to put the exceptions into an associative array and
> > let the script later check whether there is an entry with that name...))
> > ...
> > ... what about adding an env/Makefile variable
> > "ON_KSH_TEST_GETCONF_ERRATA_6548104" which is set to "0" by default
> > (meaning "off") in the Makefile which can be set to "1" (=on) on demand
> > from the calling shell environment (e.g. for build machines < B64) ?
>
> Yuck.
Yes, yes, I agree... it's ugly... but at least the variable name lives
in a seperate namespace and describes the variable... :-)
> Please, no workarounds in the gate for bugs that were fixed before
> the functionality needing the workaround was introduced.
Ok...
> How about just removing this workaround right before putback?
I just removed it and added a general "catch all errors and ignore
them"-flag called "ON_KSH_TEST_IGNORE_TESTFAILURE". Not really nice but
may be usefull to see all the test failures on demand (currently the
test run aborts on the first test failure and you have to fix them
sequentially...) ...
... does that count as "fixed" ?
> > > One final tiny request: would you be willing to rename
> Makefile.astinclude
> > > to Makefile.ast or Makefile.astlib? My concern is that the name is
> > > several characters wider than anything else in usr/src/lib and makes it
> > > harder to see the full set of files that are in the directory with ls(1).
> > > (And AFIACT the "include" part is not particularly meaningful.)
> >
> > Erm, "Makefile.astinclude" is exclusively for processing the AST include
> > files and the filename should reflect that... mhhhh...
> > ... I don't like the idea of renaming it to "Makefile.ast" because there
> > is already usr/src/Makefile.ast as "generic AST flag collection Makefile
> > include" while "Makefile.astinclude" specifically does library header
> > processing...
> > ... what about renaming it to "Makefile.asthdr" ("AST HeaDeR processing
> > makefile include" ... or "Makefile.astinc" ...) ? That would be exactly
> > as long as "Makefile.astmsg" ...
>
> Ah, I'd misunderstood. Yes, "Makefile.asthdr" seems good.
Fixed.
I've attached a new patch as "ksh93_integration_review_meem004.diff.txt"
... can I commit the patch to the tree if it's Ok ?
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 7950090
(;O/ \/ \O;)
-------------- next part --------------
Index: src/cmd/ksh/Makefile.testshell
===================================================================
--- src/cmd/ksh/Makefile.testshell (revision 739)
+++ src/cmd/ksh/Makefile.testshell (working copy)
@@ -53,12 +53,6 @@
# io.sh[81]: picked up file descriptor zero for opening script file
# -- snip --
#
-# - "sun_solaris_getconf.sh" is currently in the whitelist for tests which are
-# allowed to fail per explicit permission by Don Cragun
-# (see
http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-April/002499.html)
-# because Solaris 11 < B64 returns a wrong value for "SSIZE_MAX" (see
-# bug #6548104). Builds on >= B64 should no longer fail.
-#
# - These tests need a working system clock, otherwise they'll bite you.
#
# - The test frontend in this Makefile should be rewritten in ksh93
@@ -97,6 +91,9 @@
# $ export ON_KSH_TEST_LIST=<value> # before $ make install #
ON_KSH_TEST_LIST = $(TESTSRC)/*.sh
+# Flag to control whether we should make test failures non-fatal
+ON_KSH_TEST_IGNORE_TESTFAILURE=false
+
# We must wait for other things in this subdir to finish before running
# the test suite, otherwise we may run into trouble that this activity
# may disturb the test suite run (resulting in weird "heisenbug"-like
@@ -128,8 +125,8 @@
fi ; \
(for test_item in $(ON_KSH_TEST_LIST) ; do \
[[ "$${test_item}" = "$(TESTSRC)/builtins.sh" || \
- "$${test_item}" = "$(TESTSRC)/options.sh" || \
- "$${test_item}" = "$(TESTSRC)/sun_solaris_getconf.sh" ]] && \
+ "$${test_item}" = "$(TESTSRC)/options.sh" ]] || \
+ $(ON_KSH_TEST_IGNORE_TESTFAILURE) && \
set +o errexit ; \
printf \
"## Running %s test: LANG='%s' script='%s'\n" \
Index: src/cmd/ksh/Makefile.com
===================================================================
--- src/cmd/ksh/Makefile.com (revision 739)
+++ src/cmd/ksh/Makefile.com (working copy)
@@ -54,33 +54,7 @@
# way to expliclity list each single flag.
CPPFLAGS = \
$(DTEXTDOM) $(DTS_ERRNO) \
- -I$(ROOT)/usr/include/ast \
- -DKSHELL \
- -DSHOPT_BRACEPAT \
- -DSHOPT_CMDLIB_BLTIN=0 \
- '-DSH_CMDLIB_DIR="/usr/ast/bin"' \
- '-DSHOPT_CMDLIB_HDR="solaris_cmdlist.h"' \
- -DSHOPT_DYNAMIC \
- -DSHOPT_ESH \
- -DSHOPT_FILESCAN \
- -DSHOPT_HISTEXPAND \
- -DSHOPT_KIA \
- -DSHOPT_MULTIBYTE \
- -DSHOPT_NAMESPACE \
- -DSHOPT_OPTIMIZE \
- -DSHOPT_PFSH \
- -DSHOPT_RAWONLY \
- -DSHOPT_SUID_EXEC \
- -DSHOPT_SYSRC \
- -DSHOPT_VSH \
- -D_BLD_shell \
- -D_PACKAGE_ast \
- -DERROR_CONTEXT_T=Error_context_t \
- '-DUSAGE_LICENSE=\
- "[-author?David Korn <dgk at research.att.com>]"\
- "[-copyright?Copyright (c) 1982-2007 AT&T Knowledge Ventures]"\
- "[-license?http://www.opensource.org/licenses/cpl1.0.txt]"\
- "[--catalog?libshell]"'
+ $(LIBSHELLCPPFLAGS)
CFLAGS += \
$(CCVERBOSE) \
Index: src/Makefile.ast
===================================================================
--- src/Makefile.ast (revision 739)
+++ src/Makefile.ast (working copy)
@@ -33,3 +33,35 @@
# silence common AST&co. warnings...
# ... about |#pragma prototyped| ...
CERRWARN += -erroff=E_UNRECOGNIZED_PRAGMA_IGNORED
+
+# common CPP flags for libshell consumers (ksh, shcomp etc.)
+LIBSHELLCPPFLAGS = \
+ -I$(ROOT)/usr/include/ast \
+ -DKSHELL \
+ -DSHOPT_BRACEPAT \
+ -DSHOPT_CMDLIB_BLTIN=0 \
+ '-DSH_CMDLIB_DIR="/usr/ast/bin"' \
+ '-DSHOPT_CMDLIB_HDR="solaris_cmdlist.h"' \
+ -DSHOPT_DYNAMIC \
+ -DSHOPT_ESH \
+ -DSHOPT_FILESCAN \
+ -DSHOPT_HISTEXPAND \
+ -DSHOPT_KIA \
+ -DSHOPT_MULTIBYTE \
+ -DSHOPT_NAMESPACE \
+ -DSHOPT_OPTIMIZE \
+ -DSHOPT_PFSH \
+ -DSHOPT_RAWONLY \
+ -DSHOPT_SUID_EXEC \
+ -DSHOPT_SYSRC \
+ -DSHOPT_VSH \
+ -D_BLD_shell \
+ -D_PACKAGE_ast \
+ -DERROR_CONTEXT_T=Error_context_t \
+ '-DUSAGE_LICENSE=\
+ "[-author?David Korn <dgk at research.att.com>]"\
+ "[-copyright?Copyright (c) 1982-2007 AT&T Knowledge Ventures]"\
+ "[-license?http://www.opensource.org/licenses/cpl1.0.txt]"\
+ "[--catalog?libshell]"'
+
+
Index: src/lib/libshell/Makefile.com
===================================================================
--- src/lib/libshell/Makefile.com (revision 739)
+++ src/lib/libshell/Makefile.com (working copy)
@@ -142,34 +142,9 @@
$(DTEXTDOM) $(DTS_ERRNO) \
-Isrc/cmd/ksh93 \
-I../common/include \
- -I$(ROOT)/usr/include/ast \
- -DKSHELL \
- -DSHOPT_BRACEPAT \
- -DSHOPT_CMDLIB_BLTIN=0 \
- '-DSH_CMDLIB_DIR="/usr/ast/bin"' \
- '-DSHOPT_CMDLIB_HDR="solaris_cmdlist.h"' \
- -DSHOPT_DYNAMIC \
- -DSHOPT_ESH \
- -DSHOPT_FILESCAN \
- -DSHOPT_HISTEXPAND \
- -DSHOPT_KIA \
- -DSHOPT_MULTIBYTE \
- -DSHOPT_NAMESPACE \
- -DSHOPT_OPTIMIZE \
- -DSHOPT_PFSH \
- -DSHOPT_RAWONLY \
- -DSHOPT_SUID_EXEC \
- -DSHOPT_SYSRC \
- -DSHOPT_VSH \
- -D_BLD_shell \
- -D_PACKAGE_ast \
- -DERROR_CONTEXT_T=Error_context_t \
- '-DUSAGE_LICENSE=\
- "[-author?David Korn <dgk at research.att.com>]"\
- "[-copyright?Copyright (c) 1982-2007 AT&T Knowledge Ventures]"\
- "[-license?http://www.opensource.org/licenses/cpl1.0.txt]"\
- "[--catalog?libshell]"'
+ $(LIBSHELLCPPFLAGS)
+
CFLAGS += \
$(CCVERBOSE) \
-xstrconst
Index: src/lib/libshell/Makefile
===================================================================
--- src/lib/libshell/Makefile (revision 739)
+++ src/lib/libshell/Makefile (working copy)
@@ -52,7 +52,7 @@
HDRDIR32= $(MACH)/include/ast
HDRDIR64= $(MACH64)/include/ast
-include ../Makefile.astinclude
+include ../Makefile.asthdr
install_h: $(ROOTHDRS)
Index: src/lib/libcmd/Makefile
===================================================================
--- src/lib/libcmd/Makefile (revision 739)
+++ src/lib/libcmd/Makefile (working copy)
@@ -49,7 +49,7 @@
HDRDIR32= $(MACH)/include/ast
HDRDIR64= $(MACH64)/include/ast
-include ../Makefile.astinclude
+include ../Makefile.asthdr
install_h: $(ROOTHDRS)
Index: src/lib/Makefile.astinclude
===================================================================
--- src/lib/Makefile.astinclude (revision 694)
+++ src/lib/Makefile.astinclude (working copy)
@@ -1,87 +0,0 @@
-#
-# CDDL HEADER START
-#
-# The contents of this file are subject to the terms of the
-# Common Development and Distribution License (the "License").
-# You may not use this file except in compliance with the License.
-#
-# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
-# or http://www.opensolaris.org/os/licensing.
-# See the License for the specific language governing permissions
-# and limitations under the License.
-#
-# When distributing Covered Code, include this CDDL HEADER in each
-# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
-# If applicable, add the following below this CDDL HEADER, with the
-# fields enclosed by brackets "[]" replaced with your own identifying
-# information: Portions Copyright [yyyy] [name of copyright owner]
-#
-# CDDL HEADER END
-#
-#
-# Copyright 2007 Sun Microsystems, Inc. All rights reserved.
-# Use is subject to license terms.
-#
-# ident "%Z%%M% %I% %E% SMI"
-#
-
-# Note: libast headers are generated by the AST build system outside OS/Net
-# (and then copied here) and depend on the architecture (e.g. "i386", "amd64",
-# "sparc", "sparcv9" etc. ...), we later merge them into one unified file
-# (see below)
-
-ROOTHDRDIR= $(ROOT)/usr/include/ast
-
-# Define the symbol used to distinguish between 32bit and 64bit parts of the
-# include file. We cannot use |_LP64| here because not every compiler (like
-# Studio 10/11/12) sets it by default (this doesn't harm because the AST
-# includes are OS- and platform-specific anyway) and we can't rely on the
-# system includes like <sys/isa_defs.h> because "/usr/bin/diff -D<symbol>"
-# adds the "#ifdef <symbol>" before any other content and "injecting" an
-# "#include <sys/isa_defs.h>" will alter the behaviour of the AST code
-# in unpredictable ways (e.g. the resulting code will not longer work).
-# Sun-Bug #6524070 ("RFE: Please set |_LP64| for 64bit platforms by default
-# (like gcc does)") has been filed against the Sun Studio compiler as RFE
-# to set |_LP64| for 64bit targets.
-# (INTEL_BLD is '#' for a Sparc build and SPARC_BLD is '#' for an Intel build)
-$(SPARC_BLD)AST64BITCPPSYMBOL = __sparcv9
-$(INTEL_BLD)AST64BITCPPSYMBOL = __amd64
-
-# We use a custom install sequence here to unify 32bit and 64bit AST includes
-# since we can only ship one set of includes. Therefore we use
-# "/usr/bin/diff -D <64bit>" (and for some exceptions a manual path) to
-# generate an unified version of the include files (and add a boilerplate text
-# which explains the interface stability status).
-# ToDo: Rewrite this in ksh93 to simplify the boilerplate generation
-$(ROOTHDRDIR)/%: $(HDRDIR32)/% $(HDRDIR64)/%
- @mkdir -p tmpastinclude
- @boilerplate="" \
- boilerplate="$${boilerplate}/*\n" \
- boilerplate="$${boilerplate} * BEGIN OpenSolaris section\n" \
- boilerplate="$${boilerplate} * This is an unstable interface; changes
may be made\n" \
- boilerplate="$${boilerplate} * without notice.\n" \
- boilerplate="$${boilerplate} * END OpenSolaris section\n" \
- boilerplate="$${boilerplate} */\n" ; \
- if [ "$(@F)" = "ast_limits.h" -o \
- "$(@F)" = "ast_dirent.h" ] ; then \
- print "# Building (concatenation) $(@F)" ; \
- ( \
- print -n "$${boilerplate}" ; \
- print '#ifndef $(AST64BITCPPSYMBOL)' ; \
- cat "$(HDRDIR32)/$(@F)" ; \
- print '#else /* $(AST64BITCPPSYMBOL) */' ; \
- cat "$(HDRDIR64)/$(@F)" ; \
- print '#endif /* $(AST64BITCPPSYMBOL) */' ; \
- ) >"tmpastinclude/$(@F)" ; \
- else \
- print "# Building (diff) $(@F)" ; \
- ( \
- set +e ; \
- print -n "$${boilerplate}" ; \
- /usr/bin/diff -D $(AST64BITCPPSYMBOL) "$(HDRDIR32)/$(@F)"
"$(HDRDIR64)/$(@F)" ; true ;\
- ) >"tmpastinclude/$(@F)" ; \
- fi
- $(INS) -s -m $(FILEMODE) -f $(@D) "tmpastinclude/$(@F)"
-
-# Add temprary include files to the list of files to "clobber"
-CLOBBERFILES += tmpastinclude/*
Index: src/lib/libdll/Makefile
===================================================================
--- src/lib/libdll/Makefile (revision 739)
+++ src/lib/libdll/Makefile (working copy)
@@ -50,7 +50,7 @@
HDRDIR32= $(MACH)/src/lib/libdll
HDRDIR64= $(MACH64)/src/lib/libdll
-include ../Makefile.astinclude
+include ../Makefile.asthdr
install_h: $(ROOTHDRS)
Index: src/lib/libast/Makefile
===================================================================
--- src/lib/libast/Makefile (revision 739)
+++ src/lib/libast/Makefile (working copy)
@@ -143,7 +143,7 @@
HDRDIR32= $(MACH)/include/ast
HDRDIR64= $(MACH64)/include/ast
-include ../Makefile.astinclude
+include ../Makefile.asthdr
install_h: $(ROOTHDRS)
Index: src/lib/libpp/Makefile
===================================================================
--- src/lib/libpp/Makefile (revision 739)
+++ src/lib/libpp/Makefile (working copy)
@@ -51,7 +51,7 @@
HDRDIR32= common
HDRDIR64= common
-include ../Makefile.astinclude
+include ../Makefile.asthdr
install_h: $(ROOTHDRS)
