httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject Re: [Patch]:Move SSI "exec" directive support from mod_include to mod_cgi(d).
Date Fri, 12 Jan 2001 21:00:48 GMT

> > I have reviewed this patch, and I am -1 for it's inclusion right now.  I
> > have two major problems with it.
> > 
> > 1)  This won't link unless both cgi(d) and include modules are
> > present.  Until we figure out how to allow a module to have a function
> > that other modules can call even if the module isn't there, this patch
> > can't go it.
> I understand the concern, but this was the design that was decided upon on
> the list. In fact I had done an earlier implementation using hooks that
> I don't think would have this problem, but was requested to go with the
> direct call implementation instead.

This is the correct design, but it isn't complete.  The hook mechanism is
the wrong paradigm for this.  Besides, the hook solution was fixed with
the introduction of the generic hooks, so two weeks ago, even the hooks
wouldn't have worked.

> > 2)  We need to be in feature freeze.  This patch must wait until after the
> > beta.
> I am not pleased about this. The code freeze is only hours old. This code
> was done a week ago but was held up because Apache wouldn't build and because
> the code had to be re-ported because of the hook changes (including the
> problems with AddHandler that I commented on). This was discussed and agreed
> to weeks ago, long before the code freeze was even discussed. I could also
> argue that this is not a new feature, though I would have a harder time
> arguing that this is just a bug fix ;).

This may not be a new feature, but it is much more than a bug fix, and it
has the potential to break the server.  In fact, it does break the server
when mod_include is not used but mod_cgi is.  If we add this now, we need
to solve the other problem as well.  We want to see a beta soon,
regardless of whether EVERYTHING is perfect or not.  This patch looks
good, but it is only half of the solution, and we can't apply this until
the other half is solved, or we definately break the server.

We will get this patch in, but it should wait until after the beta IMNSHO.

Ryan Bloom               
406 29th St.
San Francisco, CA 94131

View raw message