On Fri, Nov 16, 2007 at 04:51:37PM -0800, rahul wrote:

> http://cr.opensolaris.org/~vrthra/apache-modules/

I'm going to echo Arvind's note about having separate targets for each
module in the top-level Makefile.  You should probably do like the
higher-level makefiles do with

    all := TARGET=all
    ...

and put each module in its own subdirectory.  It's a method that's worked
well for the rest of the consolidation, and seems highly appropriate for
this as well.

I'm confused by your explanation of the dummy "all" target.  Nightly by
default doesn't invoke "all", it just makes install, so you wouldn't
normally be building the objects twice.  But shouldn't a hand-run "make
all" build the files without putting them in $PROTO, even if a later
invocation of "make install" would rebuild them?

Could you point me at a built workspace?  I'm curious to see what state the
apxs-generated binaries are in.  The mod_perl Makefile.sfw seems to go to
quite some lengths to work around issues with the apxs output, and you
don't.  Arvind seems to be the last person to have touched that bit of
mod_perl -- can you explain what needed fixing here?  I don't recall seeing
discussion of that code-review request, though that might have happened on
the webstack list.

Makefile.sfw.fcgid:

  - line 30: Please use "ISAINFO:sh = isainfo -n" here instead; it stands
    out better as a shell-out command

  - line 67: Why do you need gtar for this?  Is tar xopf not enough?

Makefile.sfw.jk:

  - lines 37, 50: Why do you need to set LD_LIBRARY_PATH?  Does apxs
    require a library here, or does it run something it builds?

install-fcgid:

  - line 1: you run this with ksh93, but the shebang says ksh.  Please pick
    one and use it in both places.

  - the other two install-* scripts are remarkably similar; perhaps the
    code could be shared?

SUNWapch22m-fcgid/copyright:

  - I believe you'll need the few lines of "v2-only" disclaimer that we're
    putting in the rest of the copyright files, unless these already don't
    have the "(at your option) any later version" clause.

SUNWapch22m-fcgid/depend:

  - I don't think there's any reason to depend on SUNWapch22r.

Danek

Reply via email to