Peter Memishian wrote: > Roland, apologies for the delay. No problem... :-)
> I think we're rapidly converging. Ok... :-) > > > [ Resolved issues removed. ] > > > > > > > > > http://cr.grommit.com/~chin/ksh93-webrev-jun29/jun29-makefiles/ > > > > I still feel uneasy about the idea to create one seperate file for the > > C99MODE+CERRWARN stuff because it's AFAIK not a "catch all" solution. > > Some items in the Makefiles seem to require a specific position (e.g. > > before or after "include */Makefile.lib" etc.) and somehow I feel we may > > end up with something like usr/src/Makefile.ast_pre_stuff, > > usr/src/Makefile.ast_main_stuff and usr/src/Makefile.ast_post_stuff > > (just a feeling) ... > > ... and the KSHCPPFLAGS idea sounds nice but I'd like to wait for > > "shcomp" (ksh93's shell script bytecode compiler) to land in the tree > > 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... :-) )) ? Problem is that usr/src/ gets spammed with more and more mini-Makefile include files (and reaching from usr/src/cmd/{ksh,shcomp,busybox}/ over to usr/src/lib/libshell/ is not allowed) ... [snip] > > > > > lib/libcmd/Makefile.com: > > > > > > > > > > * 104: "-Isrc/lib/libcmd" seems bizarre; why is this necessary? > > > > > > I'm not sure I understand this yet. > > > > Ok... lets describe this differently. The "src/lib/libcmd" part in > > "-Isrc/lib/libcmd" has nothing to do with the OS/Net source layout, it's > > the relative subpath in usr/src/lib/libcmd/$(TRANSMACH)/, e.g. > > usr/src/lib/libcmd/$(TRANSMACH)/src/lib/libcmd/ ... and the same applies > > to the other AST libraries, for example libast has > > usr/src/lib/libast/$(TRANSMACH)/src/lib/libast/. This is how the > > upstream directory layout looks like (see > > http://mail.opensolaris.org/pipermail/busybox-dev/2007-April/000033.html > > , note that the "src/lib/lib(ast|dll|pp|cmd|shell)/"-part cannot be > > changed without poking in the AST sources (see below (the comment about > > maintaince))). > > I get it now, thanks :-) Yes, a bit alien, but I understand why you've > chosen to do it that way. Well, it has the advantage that the person who does the update half asleep in the morning (= me) can't pick the wrong files by accident... and I can run something like $ gdiff -r -u <more-flags> arch/<ast-arch-dir>/src/lib/lib<name-of-library>/ usr/src/lib<name-of-library>/${TRANSMACH}/src/lib/lib<name-of-library> # to verify that all files have been copied over (we already had ~~half a dozend updates in the last year and that part worked without problems, with and without using lots of coffee (or short: This part should be more or less "foolproof", unless I (the fool) invent a new way to screw the tree up... =:-) )). > > > > > lib/libdll/{sparc,sparcv9,amd64,i386}/Makefile: > > > > > > > > > > * GETRELEASEMINOR seems the hard way around; seems simpler > to > > > > > update Makefile.master to have: > > > > > > > > > > RELEASE_MAJOR=5 > > > > > RELEASE_MINOR=11 > > > > > RELEASE=$(RELEASE_MAJOR).$(RELEASE_MINOR) > > > > > > > > > > ... then get rid of GETRELEASEMINOR. (But if for some > reason it > > > > > needs to be kept, then please move it somewhere more > common.) > > Yes, but I can "predict" the results of the changes of my changes to > > "Makefile.lib". Adding new variables with VERY generic names (BTW: I > > wish "RELEASE" would be renamed to "OSNET_RELEASE") in "Makefile.master" > > is IMO a different thing because I don't know whether this may trigger > > problems if someone (for example in closed_bins/ or elsewhere) uses the > > same variable names (and I don't have the CPU power or disk space to > > setup a couple of "playground" trees and run a few tests (remeber my > > machines chew two days on OS/Net and the Ultra5 was originally shipped > > with 9GB disks... ;-( )). > > I checked; no one in usr/closed is making use of RELEASE_MAJOR or > RELEASE_MINOR. I proposed generic names because they are generic > macros :-) Ok... but right now I am more sulking about Makefile variable names like "RELEASE", "VERSION" or "PATCHID" ... whoever had the idea to use single-word generic names as tree-global variables should IMHO be roasted on a small fire (slowly in Beurre ravigote) and then fed to komodo dragons... =:-) > > BTW: See > > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/cmd/ipf/Makefile.ipf#17 > > That's a shame. > > 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 ? 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 ? [snip] > > > > > * 56-61: Since we're well past build 64, can this issue be > > > > > removed? (If not, please reformat the comment to be > 80-column > > > > > friendly.) > > > > > > > > Erm... I don't have a build machine > B61 and I can't simply update my > > > > two build machines because I would either loose the abilty to test the > > > > |libc::wordexp()| code or have to update the whole tree to a newer > > > > Nevada build (which would trigger problems for April because she would > > > > have to re-do her whole SCCS tree...). > > > > > > But you're well aware of the issue, so the comment is not for you, right? > > > Others will be running on newer builds (since B61 is history), so I don't > > > see why the comment is needed. > > > > Erm, the comment is needed because the shell code below adds an > > exception for the matching test run. I'd like to explain _why_ the > > exception is currently there (and AFAIK there is no variable to test for > > the Nevada build version (e.g. B[0-9]*)) to make the exception > > conditional... ;-( > > But why do you need to putback the exception? Build 61 and earlier are > history, and our source should not discuss issues that no longer apply. 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) ? > > > > > * 113-143: Please reformat to be 80-column friendly. > > > > Well, there is a comment about rewriting the whole monstrosity in ksh93. > > At least all the "egrep" and "grep" things will disappear and the whole > > thing will become more compact and easier to maintain. A seperate script > > is slightly tricky since it depends on _lots_ of variables in the > > Makefiles which would need to be passed as parameters somehow - and the > > result won't be much prettier then the current solution... ;-/ > > ... anyway... I've re-arranged the script to fit within the 80-column > > margin. The comments have been re-arranged to fit within the 80-column > > margin except the URL (otherwise you can't click it in the source > > browser) and the output of the "options.sh" test failure... > > ... is that Ok ? > > Yes, that sounds reasonable to me. > > 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" ... ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 7950090 (;O/ \/ \O;)
