James Carlson wrote:
> Roland Mainz writes:
> > James Carlson wrote:
[snip]
> > [snip]
> > > > > cmd/ksh/amd64/Makefile
> > > > > cmd/ksh/i386/Makefile
> > > > >
> > > > > 39-48: use the $(INS.link) rule instead.
> > > >
> > > > Erm... that won't work AFAIK. $(USRKSH_ALIAS_LIST) is a variable list
> > > > and changes based on the switches defined by "Makefile.ksh93switch" (we
> > >
> > > Part of the (seemingly unnecessary) complexity here is that
> > > $(USRKSH_ALIAS_LIST) includes $(PROG), and this little script exists
> > > in part just to skip over that one entry.
> [...]
> > > install: $(ROOTLINKS64)
> > > $(ROOTLINKS64): $(ROOTPROG64)
> > > $(ROOTPROG64): all
> >
> > Erm, correct me if I am wrong... this would wallpaper hardlinks
> > unconditionally over the frontend binary which makes this depend on the
> > build order. If the wrong order is used you have lots of links but no
> > frontend binary, right (OkOk... we could enforce this via adding some
> > ".WAIT" statements... but see below...) ?
>
> That's how we handle other things, such as isaexec. Controlling build
> order and dependencies is pretty much the reason 'make' is there.
My question is: What is more evil - the ".WAIT" or the mini-scripts we
use right now ?
[snip]
> > > > Same problem as in usr/src/cmd/ksh/$(MACH)/Makefile:
> > > > $(USRKSH_ALIAS_LIST) is a variable list and the tree supports switching
> > > > the install mode defined by Makefile.ksh93switch between single builds
> > > > for testing (otherwise a rebuild from scratch may be required).
> > >
> > > As above, I think the complexity here is in having $(PROG) appear
> > > inside $(USRKSH_ALIAS_LIST), not the way in which the switch is made.
> >
> > The problem is that $(PROG) changes and that we don't know whether the
> > frontend will be stored at /usr/bin/ksh93 (making /usr/bin/ksh a
> > hardlink) or /usr/bin/ksh (making /usr/bin/ksh93 a hardlink) in the
> > final post-migration world (Ok, it's a long way down the river of tears
> > util we reach that.... but it's still usefull for testing).
>
> I don't see how that's really an issue at all.
>
> First of all, you'll need to tweak the packaging when you get to that
> day. The current packaging won't support what you're doing after you
> flip the switch -- so more is needed anyway, and the switch code isn't
> logically complete.
You forget the non-Solaris OpenSolaris distributions in this case. The
Makefile.ksh93switch is there to be used for testing and that they can
use it in their distributions (which will hopefully result in lots of
data how good/bad this works in the "wilderness". We need such data in
an ARC case which covers the migration of /usr/bin/ksh to ksh93 to
emphasise that this really works "out there" (OkOk, this is no "proof"
but collecting data is IMO important, too)).
> Secondly, if you're regenerating those links on demand, then in what
> possible case are the dependencies wrong? (It seems like that's what
> you're assuming here -- that the dependencies end up being wrong and
> the wrong thing gets linked -- but I just don't see how that's
> possible.)
What about the case where the switch gets flipped and someone does a
"make install" in usr/src/cmd/ksh/ and usr/src/lib/libc/ ?
> > > > > cmd/ksh/Makefile.com
> [...]
> > > That's not what I was "yuck"-ing. My comment was on the mkdir -p
> > > buried in here; it's very unmakefilish. (makefilian?)
> > >
> > > I'd prefer something roughly like:
> > >
> > > COBJS= \
> > > sh/pmain.o
> > >
> > > OBJDIRS= sh
> > >
> > > $(OBJDIRS):
> > > $(MKDIR) $@
> > >
> > > $(PROG): $(OBJDIRS) .WAIT $(COBJS)
> >
> > Ok, but see the comment about libast's Makefile.com below. The basic
> > idea is to keep the complexity down and avoid per-directory rules (it
> > may sound like an overkill for usr/src/cmd/ksh/Makefile.com but remeber
> > this file was carved out of usr/src/lib/libshell/Makefile.com and should
> > follow the same logic)...
>
> I don't see how causing hundreds of "mkdir -p" invocations in a
> custom-crafted build rule is any less complex.
>
> I suspect that the implication is that you feel that makefile syntax
> itself is just too hard to maintain, and that you'd rather just have a
> script. Is that the issue?
No, I just don't like the alternative of creating a maintaince nightmare
like OBJDIRS. usr/src/cmd/ksh/Makefile.com is easy but in the case of
usr/src/lib/libast/Makefile.com it will quickly become a pain. I want to
work on the ksh93 code, not the Makefile infrastructure. The whole idea
is to keep the required maintaince at a minimum level and (more or less)
automated...
... mhhh...
Ok...in theory we could change the code to:
-- snip --
# We are storing the object files into subdirs avoid the
# confusion with having too many object files in the toplevel pics/
# directory (this matches the way how the original AST build system
# deals with this "logistic" issue) - the rules below ensure that
# the destination directory is available.
GETCOBJDIRS = $(SHELL) -c 'for i in $(COBJS) ; do dirname "$${i}" ; done
| uniq'
COBJDIRS = $(GETCOBJDIRS:sh)
PICSDIRS= $(COBJDIRS:%=pics/%)
mkpicdirs:
mkdir -p $(PICSDIRS)
-- snip --
That calculates the list of directories automagically from COBJS
(unfortunately Solaris /usr/ccs/bin/make has no matching direcory
operator to split directories from files via $(...:...=...) mapping
which means we have to use something like $(GETCOBJDIRS) (the code could
be rewritten in ksh93 like this:
-- snip --
$ (typeset -A map ; for i in one two two three three three ; do
map[$i]="$i" ; done ; print ${map[*]})
one two three
-- snip --
, e.g. it would run completely using builtins and much faster...
)).
Unfortunately Solaris /usr/bin/ksh and /sbin/sh then have trouble in
usr/src/lib/libast/Makefile.com because the list of COBJS is too long.
SHELL=/usr/bin/ksh93 works... unfortunately /usr/bin/ksh93 is not
available yet which means we're running out of options in this case...
;-(
Short comment: xx@@@!!!... ;-(
I give up... ;-(
... I've hardcoded the list of directories... the problem is now that
the new solution is slightly slower and added a serialisation point
(e.g. ".WAIT") in the Makefile... ;-(
> > > > > 108: "seems to require"?
> > > >
> > > > See usr/src/cmd/ksh/Makefile.testshell, the comment about "io.sh"
> > > > (originally it was part of usr/cmd/ksh/Makefile.com which would be much
> > > > clearer in this case).
> > >
> > > I'm unclear on the use of the phrase "seems to."
> > >
> > > Does it indeed _require_ that ksh93 is on the path as "ksh," or does
> > > it not require that?
> >
> > It seems to require that the test suite is always called with
> > SHELL=/path/to/binary/ksh, therefore we hardlink /path/to/binary/ksh93
> > to /path/to/binary/ksh (or reverse for build symetry (to avoid that
> > Makefile.ksh93switch bites us if we switch in one build)).
>
> I still don't understand.
>
> Either it does require it, or it does not require it. Which is true?
>
> "Seems to" implies that the design work isn't done yet, and that you
> don't really know how (or if!) the test code works. Please clarify.
Erm... this has nothing todo with design work. And yes, the test code
works. I only did not crawl after the problem which triggers the "io.sh"
issue because it's somewhere deep at the bottom of the list which has
more than a hundred items and which will keep us busy for the next 2-3
years to drive out all possible incarnations of various bugs.
[snip]
> > $ ggrep -r HOSTTYPE $PWD # shows the following locations
> > -- snip --
> > usr/src/lib/libast/common/port/astconf.c: "HOSTTYPE",
> > usr/src/lib/libast/common/port/astconf.c: HOSTTYPE,
> > usr/src/lib/libast/common/path/pathprobe.c: p = path +
> > sfsprintf(path, PATH_MAX - 1, "%s/.%s/%s/", p, probe, HOSTTYPE);
> > -- snip --
> > 1. astconf.c is used for the |libast::astconf()| call to return this
> > value as platform identifier (that applications and/or scripts can use
> > this)
>
> I see. But what would they do with it?
>
> I still find this logic to be baffling. What's the usage case? Why
> does this matter here?
It matters because the code doesn't compile without a definition of
|HOSTTYPE| and I don't like to pass a (semi-)random value here.
> > 2. usr/src/lib/libast/common/path/pathprobe.c uses this value to lookup
> > user-specific code in ${HOME} using the value from HOSTTYPE as
> > platform-identifer (since ${HOME} may be shared across multiple
> > architectures (this isn't used directly in libshell/ksh93 but may affect
> > future libast-based applications))
>
> That's just frightening.
?!
Why ?
> Did I miss that in ARC review?
Grumpf. No, see above (and the previous email which described who is
using this particular function).
> > > What is `_ast_va_list', which is cited in the bug report? That looks
> > > to me like an AST-specific implementation of varargs. Is it actually
> > > something else?
> >
> > >From /usr/include/ast/ast_common.h
> > -- snip --
> > #define _ast_va_list va_list
> > -- snip --
> > ... it's a portability #define (see below).
>
> And, yet, that simple renaming confuses gcc ... ?
>
> That seems like something worthy of a little investigation.
The AST header have been generated for Sun Workshop/Forte/Studio and
matches the <stdargs.h> header from /usr/include/. gcc comes with it's
own <stdargs.h> (see
/usr/sfw/lib/gcc/i386-pc-solaris2.11/3.4.3/install-tools/include/stdarg.h
on x86) which overrides the system header in /usr/include/.
Note that this does not affect the public AST includes (those in
/usr/include/ast/), only the libast internals have a minor hiccup here
(and yes, I've looked at the problem. It's stupid&&harmless and that's
why I put the workaround in the Makefiles and wait for gcc4.x to appear
in OS/Net).
[snip]
> > > > The problem is <sys/isa_defs.h>. It's a system include in the sys/
> > > > directory. The Sun Workshop/Forte/Studio compiler does not define
> > > > |_LP64| which means it's not available unless you include system
> > > > includes which interfers with the AST includes ("interfers" is an
> > > > understatement... it completely screws-up the AST headers which depend
> > > > on precise include order of it's own and the system includes).
> > >
> > > That sounds like a potentially serious design problem to me.
> >
> > Why ?
>
> Because you're saying that you can be blown out of the water by
> ordinary -- even standards-conforming header files.
>
> That doesn't bode well for the future.
What do you expect from the #include files when you inject artifical
"#include" statements which include "random" other system headers (see
http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-February/002203.html).
I expect that many headers don't like that and start screaming&&wailing
around. The AST headers a little bit more sensitive since they are
headers which are designed to be portable, you'll have similar "fun"
with NSPR, Apache runtime etc. which try to be portable across more than
one platform in the one or the other way (and IMO AST is better than
both NSPR and Apache runtime (and works on more platforms AFAIK)).
AFAIK a long-term fix may be to ask the compiler team to set |_LP64| in
the compiler by default for 64bit platforms like gcc does - then we
could avoid relying on platform-specific symbols in this case.
> > > You also get those <sys/*.h> things just by using "ordinary" include
> > > files, such as <stddef.h>.
> >
> > Yes, but those are used later in the include chain. <sys/isa_defs.h> is
> > a platform-specific include and therefore not portable.
> > gcc makes this much easier since it #define's |_LP64| for 64bit
> > platforms, e.g.
> > -- snip --
> > $ (touch foo.h; /usr/sfw/bin/cpp -m64 -dM foo.h) | fgrep _LP64
> > #define __LP64__ 1
> > #define _LP64 1--
> > -- snip --
>
> Right; which is why you'll find that in sys/isa_defs.h on Solaris. It
> makes certain that _LP64 is *always* available.
Yes, but see
http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-February/002205.html
- it doesn't help for the initial "#ifdef <symbol>" and injecting a
"#include <sys/isa_defs.h>" isn't a good idea.
> > ... unfortunately Sun Workshop/Forte/Studio do not do that and that
> > limits our choices. Defining |_LP64| in our own Makefiles doesn't work
> > since we would have to modify all consumers which uses the AST includes,
> > too (and that includes future external software, too (which expect that
> > our includes work identical to the upstream includes)).
>
> I must be missing something. I still don't understand why including
> sys/isa_defs.h is a problem.
See
http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-February/002205.html
> Is that file toxic in some way? If so, isn't that a bug (either in
> Solaris or in AST or both)?
>
> If it's not toxic, then why would it be the case that it can't be
> included, at least when you're on Solaris?
>
> I just don't understand why you need to reinvent this mechanism. This
> is the sort of hack that'll come back to haunt us later when ports to
> other platforms end up being much harder than expected.
It's a result of the way we produce one unified set of 32bit and 64bit
includes (e.g. via "/usr/bin/diff -D" (which is dumb as a brick (plain
text diff))). There is little we can do expect throwing 64bit support
completely away - maintaining our own set of includes is too
time-intensive, invites all kind of bugs to creep in and would require
excessive maintaince - which steals time from other parts of the project
like doing development on the real ksh93 code.
It's really not perfect but IMO it's economical (unless someone finds a
quick way create some clones of myself and April... :-) ) and worked the
last year without problems across many updates (which means this
solution has been tested to hell and back...).
> > > > After
> > > > lots of trouble we've settled with avoiding the usage of |_LP64| since
> > > > it's not portable enougth (and using -D_LP64=1 at compile time does not
> > > > work either (it compiles but the resulting code will be garbage)).
> > >
> > > I'm very leery of this. Not because I think _LP64 is the greatest
> > > thing since sliced bread, but because I suspect it points to deeper
> > > design flaws that will end up producing unstable builds.
> >
> > Uhm... we had never problems with that (and as I said this is working
> > across more than 40 platforms and I really won't call such a thing a
> > "design flaw" (if you want to throw bricks aim at all the platforms
> > which think that standards are an optional feature... ;-( )).
> >
> > > How is it possible that AST is so dependent on system internals that
> > > use if <sys/isa_defs.h> "screws-up" the compilation?
> >
> > See Glenn's explanation at
> > http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-February/002191.html
>
> Sorry, but that doesn't answer the question. It contains no technical
> details describing what the problem might be.
>
> Theory is nice, but understanding the problem is better.
Did
http://mail.opensolaris.org/pipermail/ksh93-integration-discuss/2007-February/002205.html
help ?
[snip]
> > > That's the sort of thing I'd expect to find in SFW, not ON.
> >
> > What should we do instead ? The current approach has at least been
> > proven to work for the last year across many many updates and a redesign
> > of the whole AST codebase is a little bit more work than most people
> > imagine (this one of the reasons why this API is not "public" yet).
>
> As I've said many times, the "just integrate" plan is exactly what SFW
> is all about. ON has its own rules.
Right... is the current solution accepable for OS/Net in your opinion ?
> > > > > lib/libast/Makefile.com
> > > > >
> > > > > 33-602: suggest condensing this list with appropriate build rules.
> > > >
> > > > Erm... what do you mean with "appropriate build rules" in this case ?
> > > > Using pattern-matching to collect all *.c files won't work (not all *.c
> > > > files are directly compiled or used within the libast code directly).
> > >
> > > No, I don't mean collecting all the *.c files.
> > >
> > > Instead, I might have build rules that look like:
> > >
> > > objs/%.o: common/cdt/%.c
> > >
> > > and an object list that says:
> > >
> > > CDTOBJS= dtclose.o dtdisc.o dtextract.o ...
> > >
> > > COBJS= $(CDTOBJS) $(COMPOBJS) $(DIROBJS) ...
> >
> > Uhhrghh... this would be quite messy (look at
> > libast//$(MACH)|$(MACH64))/Makefile, you have to handle the
> > platform-specific sources, too (and avoid that this crossfires)).
>
> It certainly doesn't look difficult to me -- they're separate
> makefiles.
Uhm... usr/src/lib/libast/($(MACH)|$(MACH64))/Makefile "#include"
usr/src/lib/libast/Makefile.com (e.g. basically they result in one large
Makefile) ...
> > I would prefer something which does this stuff automatically (BTW: The
> > list of object files is fetched from the output log of "buildksh93.ksh",
> > filtered via various things, passed through /usr/bin/sort and then put
> > into the Makefile... this procedure keeps maintaince low, too (otherwise
> > you have the deal with the usual "where is my missing symbol"-search for
> > some larger updates)) ...
>
> Someone's going to be stuck maintaining that huge list.
Right... that's me... I am the one who's going to sacrifice himself and
maintains that little monster list... :-)
> > > > > 612,616: these shouldn't be necessary, I think.
> > > >
> > > > AFAIK they are required because the compiler doesn't create subdirs
> > > > itself.
> > >
> > > No, but your makefile should by using _targets_, not by invoking
> > > "mkdir -p" several hundred times during the build.
> >
> > The only thing which would work is a rule which dismantles $(COBJS) and
> > creates the subdirs as dependicies. The problem is that this is MUCH
> > slower than the single "mkdir -p" (on my machine processing 600 * 2 * 2
> > * 2 rules/targets is slower than 600 * "mkdir -p" (mkdir is even a
> > builtin in ksh93 making the thing even faster)).
>
> Ugh.
Well, /usr/ccs/bin/make doesn't really scale in such cases, basically
it's |malloc()|'ing itself to death (somewhere on my wishlist would be
to use something something like
http://mail.opensolaris.org/pipermail/perf-discuss/2007-February/001589.html
in the "make" codebase... but I am not sure how much this would
help...).
> > > > Remember my original posting for this wevrev - we put the *.o
> > > > files into subdirs to avoid having a few hundred *.o files in one single
> > > > directory... IMO the current solution is much more tidy (I wish libc
> > > > would use the same method...).
> > >
> > > It certainly adds complexity, and I don't see how it's really helpful.
> >
> > At least I consider this setup "cleaner" since you don't have 600 or
> > more object files in one flat directory. Instead it's all nicely stored
> > in subdirs.
>
> So what? It's the object directory -- not the source. Why does it
> matter whether the object directory is "tidy?"
Umpf... personal preference to avoid stuffing too much files into one
directory. Maybe it's too much old-school thinking about putting too
much into one directory.
[snip]
> > > I'd suggest rewording to explain what it is maintainers need to be
> > > careful about. Something like: "do not change the __OBSOLETE__ value
> > > without examining the symbols that will be removed, and evaluating
> > > whether that breaks compatibility."
> >
> > ... maybe adding as 2nd comment: "... make sure this is in sync with
> > upstream (see Mamfile) ..." ?
>
> That'd do.
Fixed.
> > > > > 682-708: not sure that the comments add much value here.
> > > >
> > > > Well, they are at least usefull for me that I know where I have to look
> > > > at... :-)
> > >
> > > It takes about three seconds to check out the file and remove those
> > > overrides.
> >
> > ... and around an half a hour to get usr/src/lib/libast/ compiled (in my
> > case) ... ;-(
> > ... I would prefer to keep the comments that I don't have to recompile
> > all the stuff just to get the warnings back.
>
> Who will actually keep those comments in sync with the code if someone
> adds or deletes a line?
>
> In my experience, stuff like this just rots over time. It's obsolete
> by the time it gets checked in. It's just plain noise.
Ok... I removed the comments.
> > > > > 709: that's worrisome.
> > > >
> > > > Uhm... I don't see anything obviously wrong in this code, the code works
> > > > and neither "dbx -check access" or "Rational Purity" complain (nor does
> > > > gcc) .. therefore I've opt'ed to silence this warning and add it into my
> > > > ToDo list for later (somewhere after dealing with this putback,
> > > > |posix_spawn()| and "shcomp"&&friends...) ...
> > >
> > > A const without initializer doesn't make any sense to me. What does
> > > the code look like? (I don't seem to have access to it.)
> >
> > See
> > http://www.nrubsig.org/people/gisburn/work/solaris/ksh93_integration/ksh93_integration_prototype004_webrev_20070127/allfiles/webrev/
> > for a webrev covering all files (the webrev's are split into
> > categories).
> >
> > The code looks like this:
> > -- snip --
> > 707 register int c;
> > 708 char* e;
> > 709 const Prefix_t* p;
> > 710
> > -- snip --
> >
> > s/const Prefix_t\* p/const Prefix_t\* p=NULL/ fixes the problem.
>
> The line numbers don't match up -- line 731 is blank.
... those cokments were for ast-ksh.2006-10-31... ;-/
> I don't see how the code you're citing could cause the error you're
> seeing, because what you've declared (for the 'p' variable) is a
> pointer to storage that is const -- and not a const that itself needs
> an initializer. I think that's worth a bug on the compiler, if it's
> real.
>
> Something's wrong here.
The output of "/opt/SUNWspro/bin/cc -E" doesn't look problematic either
and neither gcc3.x (Solaris 11/B51) or gcc4.0.2 (SuSE Linux 10.0)
complain in this case... may be possible that the Sun Studio compiler
has a small hiccup in this case... ;-(
> > > > > lib/libcmd/sparcv9/Makefile
> > > > >
> > > > > 28: not sure why this is needed.
> > > >
> > > > All Makefiles used for libast/libpp/libdll/libcmd/libshell/etc. use
> > > > SHELL=/bin/ksh
> > >
> > > But why do they care?
> >
> > Erm... what do you mean ? SHELL=/usr/bin/ksh is used uniformly across
> > the new Makefiles added to make sure we can use the normal POSIX-shell
> > syntax, including nested brackets (e.g. $ ( foo ; ( bar ; zapbambam ; )
> > ; meepmeep ) #). I would prefer /usr/xpg4/bin/sh but I doubt gatekeepers
> > would be happy to have a dependicy on /usr/xpg4/bin/ stuff without a
> > larger pro/contra discussion. That's why I picked /usr/bin/ksh instead
> > (IMO it would be nice to put something like that in Makefile.master but
> > that's more the domain of the "OS/Net Janitors" project to cleanup
> > things) and aim to replace that with SHELL=/sr/bin/ksh93 together with
> > the step which enables the building of the AST l10n files.
>
> In general, we've written makefiles in ON that assume that 'make'
> itself works, and that the shell is extremely minimal (Bourne). It
> worries me a bit that someone is wandering outside if this area
> deliberately ... but I can't give a specific case where I think it'll
> necessarily fail.
You already use things like awk, sed etc. in the build, sometimes even
in places where the shell could do the same job (and usually even
faster, at least in the case of ksh93 both builtins and associative
arrays can outperform filters like awk/sed/etc. for smaller datasets
(e.g. you don't have to |fork()| lots of external commands because the
shell itself can do the work)). Problem is that the Bourne shell is very
limited in it's functionality compared what even a plain POSIX shell
offers. IMO it would be better to think about switching the default
shell in the Makefiles from /bin/sh to /usr/xpg4/bin/sh or
/usr/bin/ksh93 (both are POSIX shells) since both offer extended
functionality and/or better performance.
> > [snip]
> > > > > pkgdefs/SUNWarc/prototype_i386
> > > > > pkgdefs/SUNWarc/prototype_sparc
> > > > >
> > > > > As above; can't tell why lint libraries are shipped.
> > > >
> > > > See above, the three-legged chimera. Basically we honor the rules and
> > > > still try to keep the value alive somehow... :-)
> > >
> > > I don't think they ought to be shipped.
> >
> > Ok... ;-(
> > Lint libraries are now scheduled for execution in boilling acid... ;-(
>
> Thanks. Though bleach would do. :-/
... that would take longer and I am not sure whether bleach can really
disolve all parts of a body...
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 7950090
(;O/ \/ \O;)
