Roland Mainz wrote:
Hi!

----

I created a couple of webrevs in various flavours based on today's
ksh93-integration sources. This isn't the "reliminary review request"
(yet; the official rquest should come on ~~ monday or tuesday), just an
attempt to provide an updated webrev.


I'm yet to look in any detail at the webrevs, but most of the comments I had from looking at diffs prior to this are covered in your notes, so I'll comment on them.

** Notes:
- All AST sources are in usr/src/lib/lib(ast|pp|dll|cmd|shell)/common/
and usr/src/cmd/ast/. All files are 100% identical with the upstream
(AT&T) versions except those listed in "usr/src/lib/libshell/misc/ksh93_solaris_builtin_patch.diff".
####----> All changes to the upstream sources ====> MUST BE <==== stored
in *.diff files (and a README per *.diff) in usr/src/lib/libshell/misc/
to make sure they can be backed-out and re-applied when the AST sources
in the OS/Net tree get updated (e.g. the usual update procedure works
like this: diff old and new upstream sources, backout the diffs from
usr/src/lib/libshell/misc/*.diff from the OS/Net tree, apply diffs from
upstream, reapply diffs from usr/src/lib/libshell/misc/*.diff, refresh
files in usr/src/lib/lib(ast|pp|dll|cmd|shell)/(${MACH32}|${MACH64})/
from a native AST build).

I don't understand why this is necessary, the SCM will provide you with diffs and their associated comments.

Anyone who is patching files in
usr/src/lib/lib(ast|pp|dll|cmd|shell)/common/ and usr/src/cmd/ast/
without storing diffs+READMEs in usr/src/lib/libshell/misc/*.diff will
be <insert-some-horrible-torture-here (gatekeepers should decide how the
punishment should look like... I'd suggest the usage of deep pits filled
with komodo dragons, hot iron, boilling acid etc. (in random order))>.

Has this diff dance actually been agreed upon by anyone? I'd like to see an explanation of why you can't just use the SCM to fulfill your needs. (I can accept one may exist, I just don't see what it is...)

- The includes delivered to /usr/include/ast/ are a merge of 32bit and
64bit includes from
usr/src/lib/lib(ast|pp|dll|cmd|shell)/(${MACH32}|${MACH64})/include/ast/
created using "/usr/bin/diff -D". This allows the usage of one unified
set of includes for 32bit and 64bit binaries instead of shipping two
different copies.

I think someone else commented on this so I'll keep out pending that explanation...

- usr/src/lib/lib(ast|pp|dll|cmd|shell)/Makefile.com list "-erroff="
flags per object file, not globally, resulting in lists which look
"huge" because we list each appeance of a suppressed warning explicitly
including a comment (and "yes", we're working on reducing the list, step
by step).

I don't see why this can't be done prior to putback, beyond your aversion to deviating *at all* from the upstream sources. I'd far prefer to see these lists empty prior to putback, even if upstream haven't accepted patches (yet).

-- Rich

_______________________________________________
opensolaris-code mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/opensolaris-code

Reply via email to