Roland, apologies for the delay. I think we're rapidly converging. > > [ 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? 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". > Ok... > ... I've moved Makefile.ksh93switch to usr/src/ to avoid the reaching > over" to other "peer" directories... Cool. > > > > 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. > > > > 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 :-) > 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. > > > > 39-46 of cmd/ksh/i386/Makefile. > > > > > > This logic is needed to install isaexec links for all variants of ksh > > > (e.g. restricted shell, profile shell etc.) depending on the setting in > > > Makefile.ksh93switch and the values of USRKSH_ALIAS_LIST and PROG, > > > ... > > > > I think I understand what the logic is for. What I don't understand is > > why the *same* logic appears to exist in cmd/ksh/Makefile, > > cmd/ksh/i386/Makefile, and cmd/ksh/sparc/Makefile. > > The logic in usr/src/cmd/ksh/Makefile is for the isaexec hardlinks while > the logic in usr/src/cmd/ksh/$(TRANSMACH)/ is for the binary and it's > hardlinks itself (and they can't move to Makefile.com since there is a > difference between ROOTPROG32 and ROOTPROG64 (and the detail that the > Makefiles in usr/src/cmd/ do not define TRANSMACH (that's why I added > CMDTRANSMACH))). I see, thanks :-) > > > > * 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. > > > > * 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.) -- meem
