Mike Kupfer wrote: [snip] > >> * 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 > > Roland> I thought about that - but the usr/src/cmd/ksh/Makefile.com > Roland> AFAIK differes in the -I statements used since it does not have > Roland> to reference libshell's internal sources. Your choice: Should I > Roland> add a common "Makefile.libshellflags" for these options or leave > Roland> it in it's current status ? > > I'd like to see a makefile fragment that just has the stuff that needs > to be the same. Maybe it could be called Makefile.libshellcommon? > Define one or more variables that have the common settings in that > makefile. Then have the ksh and libshell Makefile.com's include it and > build up using the defined variable. Skeleton example: > > Makefile.libshellcommon: > > COMMONDEFINES = -DKSHELL -DSHOPT_APPEND ... > > ksh/Makefile.com: > > include $(SRC)/lib/libshell/Makefile.libshellcommon > [...] > CPPFLAGS = $(DTEXTDOM) ... $(COMMONDEFINES) ... > > (At least I hope something that simple will work.)
Yes, but is it reallly a justification to create such a file because there are only two common locations in Makefiles ? There are like other locations in other Makefiles for which the same "rules" would apply then... On the othe hand there is "shcomp" (the shell script to ksh bytecode compiler) which would need the same set of flags (unfortunately I am late and it's far to late to get it added to the prototype (and therefore will require an extra fight with ARC&co to get this "in" ... ;-( )), making this three locations... the question is whether we do it "now" or when "shcomp" hits OS/Net ... > >> * usr/src/cmd/ksh/Makefile.com > >> - install target: why add the "set -x"? > > Roland> From ksh(1): > Roland> -- snip -- > Roland> -x Prints commands and their arguments as they are > Roland> executed. > Roland> -- snip -- > Roland> The basic idea is to see which exact command failed when a > Roland> problem occurs. > > Okay, I thought it might be something like that. I'm concerned about > extra noise in the build and having to add something to nightly.sh to > filter it out. But I'm okay with leaving it in for now and seeing if > it's really a problem or just my paranoia. ;-) Erm... why should this extra output be bad ? More detailed build logs are not evil... :-) > >> * usr/src/cmd/ksh/Makefile > >> - where are we with i18n? will we have a real ksh93.po prior to > >> putback? > > Roland> I think you're asking here about l10n (=localisation), right ? > > Well, i18n and l10n both. I hadn't realized that the i18n issues had > been addressed, so I assumed that the l10n stuff was turned off because > we were waiting for the i18n fixes. Both i18n and l10n support is compiled in the ksh93 binaries. It's just that we miss the files which contain the translated text... > Roland> Localisation (AFAIK the *.po stuff is for l10n) is working in > Roland> the ksh93 binaries - but I simply didn't investigate the details > Roland> including: > Roland> - Where are the localisation files used for the different > Roland> locales stored > > The way it works is each consolidation delivers a single messages > package to the globalization (g10n) group. The g10n group is > responsible for the translations. I assume that either the g10n group > or Release Engineering creates the l10n files/packages that get > installed. The package that ON delivers to g10n is SUNW0on (it's > defined in usr/src/pkgdefs, though I don't think it's part of the > standard build). Umpf... we can't stuff the l10n files for ksh93 into the OS/Net tree ? A seperate, non-public repository for such content isn't very usefull (for non-Sun people...) ;-( > Roland> how can we keep track of changes (syncronisation issue) ? > > I'm not entirely sure how this is managed currently. Entries in the bug > database do have a radio button "does this change affect localization > yes/no", but I don't know how that information gets back to the g10n > group, or if they even use it. This is probably a good question for > Ienup or for the i18n-discuss list. Ok... adding Ienup Sung <Ienup.Sung at Sun.COM> to the CC:-list... :-) [snip] > >> * 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?) > > Roland> Added to my ToDo list (killing them is an option, but the next > Roland> diff between ksh93 and Solairs sources then list these files as > Roland> "missing" and the person who does the update then has figure out > Roland> the "why ?". > > Hopefully this would be less of a problem once we have everything under > Mercurial. Erm... how should Mercurial be able to help in this case ? > Roland> And these Makefiles provide some interesting (well, > Roland> not essential) information, too - so I vote to not delete them > > Okay. Your proposal for the READMEs looks good. Ok... this is know under construction... I'll report back once this is done. > >> * 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. > > Roland> Using $(CPPFLAGS.master) is tricky, too - it MUST come as the > Roland> last element but future changes in -D may then cause silent > Roland> breakage in the AST sources because the last -D specified > Roland> overrides previous -D options so I prefer the current way to > Roland> expliclity list each single flag. > [...] > Roland> What should I do here now - leave it as it implemented currently > Roland> or try to "squish" $(CPPFLAGS.master) somewhere into the chain > Roland> in the hope that this won't bite back in the future... ? > > I guess I'd leave it as-is. It would be good to have a comment > explaining why we're not using $(CPPFLAGS.master) (basically giving the > reasoning that you gave). Ok... I've commited the comment change as http://polaris.blastwave.org/changeset/319 > >> I see that the workaround hack for the PAGESIZE redefinition is still > >> in place. This will need to be changed prior to putback. > > Roland> Yes... the problem here is that the PAGESIZE stuff sits in the > Roland> automatically generated AST headers (there are two other > Roland> locations in the source which have a similar problem and can > Roland> easily be "silenced" using the same solution (however until now > Roland> I didn't apply _ANY_ changes to the original AST > Roland> sources)). Adding a workaround (e.g. |#undef PAGESIZE|) is > Roland> possible but the next update of these headers will kill it - and > Roland> then we have a build failure which needs to be investigated, > Roland> e.g. making this a pain regardless of which way we choose. > > I'd much rather have a visible build failure than a silent failure due > to an undetected macro redefinition. I'll try to fix that after the next ksh93r+_alpha*-update - is that Ok for you ? > >> * 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. > > Roland> Originally I disabled the 64bit support until I had time to > Roland> seize a AMD64 box to work on that problem. During that work I > Roland> hit the problem that the public includes of libast and libshell > Roland> differ between 32bit and 64bit builds. The same problem exists > Roland> on SPARC64, too. There are two solutions: > Roland> 1. Ship two sets of includes, one for 32bit and one for 64bit > Roland> (for example stored at /usr/include/64/), however I am not > Roland> sure whether this is worth the trouble > [...] > Roland> 2. Wait until AST upstream fixed the problem > > Roland> I like to wait for [2] (and commited a comment change as > Roland> http://polaris.blastwave.org/changeset/296) unless you or April > Roland> think it's neccesary to do [1] ... > > I'm fine with waiting for upstream to fix the problem. But if it's not > fixed prior to putback, I'd really like to see the 64-bit (non-)support > done as cleanly as possible (e.g., don't install any 64-bit symlinks). Ok... I have to track-down the parts which install 64bit links at "make install" time and disable them, otherwise the removal of this links in the package definitions will make "makebfu" wail and scream (I originally added them to avoid that the gatekeeper complains) ... ;-( [snip] > By the way, thanks for the fine-grain checkins and the pointers to the > changeset web pages. That makes it much easier to review specific > changes. Yes... I wish we could move these changes and their comments into OS/Net without loosing the whole history... any ideas whether this is possible somehow ? ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 7950090 (;O/ \/ \O;)
