HI Looks good to me except for couple of suggestions - Did you try compiling with -xO4 ? With respect to Apache, I noticed around 5% gain by simply compiling with apache with -xO4. It seems that Apache 2.2.9 is going to come with this flag (-xO4). Af course, you could integrate as is and then file a bug to track this issue.
- If I am not mistaken, there is no /usr/bin/php-cgi . php-cgi is only under /usr/php5/5.2.4/bin/php-cgi. Once this is integrated, I will file a bug to ensure that PHP 5.2.6 integration handles this change in location as well. - Sriram Amanda Waite wrote: > http://cr.opensolaris.org/~tekgrrl/lighttpd-putback-1 is an updated > webrev based on comments received so far. > > Items addressed: > > - Indentation of Makefile.sfw > - Changed the fcgi-php.conf file to use a variable for the php command > string and added a note > in the comments at the head of the file. > - Created a sunman-stability file and modified the man page to remove > duplicate information. > - Changed the install script to use _install M to install the man page > > Items not addressed > > - Use of gtar -xpf > - We didn't actually change this as it seems the most popular way of > doing it and because I'm > scared of breaking what works > > - service instance name change. > - Needs further discussion and some kind of agreement on convention, > is currently consistent with > Apache 2.2 > > - Running _install to install binaries > - We believe that post_process and post_process_so do the same thing > and it seems that all > binaries are run through one of these commands > > Please review and let us have your feedback. > > Thanks > > Amanda > > > Amanda Waite wrote: > >> I've looked at the options for having lighttpd pick up the PHP paths >> dynamically and there really are none. The only thing I can do is to >> make the fcgi-php.conf file into a sample file or remove it >> altogether. Leaving it as it is doesn't seem to be an option as the >> path to the ini file will be invalid soon. >> >> I can define variables in the config file so I could set the paths to >> be variables and then make it clear that they need to be set in order >> to make it work correctly. >> >> Thanks >> >> Amanda >> >> >> Amanda Waite wrote: >> >>> Thanks for reviewing this. See inline comments: >>> >>> Sriram Natarajan wrote: >>> >>> >>>> HI >>>> >>>> - You might want to take a look at the indentation within >>>> lighthttpd/Makefile.sfw >>>> >>>> - Why not simply do it as gtar xf and avoid manually changing the >>>> permission later >>>> >>>> + $(GTAR) xpf - --no-same-owner >>>> + touch $(LIGHTTPD)/configure >>>> + find $(LIGHTTPD) -type d -exec /usr/bin/chmod 755 "{}" \; >>>> + find $(LIGHTTPD) -type f -exec /usr/bin/chmod ugo+r "{}" \; >>>> >>>> >>> The way we are doing it here maybe over cautious but it does seem to >>> avoid any possible permissions problems. I'd be happy to do it the >>> simpler way, but am thinking that albeit historical, this was done >>> for other integrations for a reason. I wonder if anyone has any >>> knowledge of why this was done this way, if not I'll just change it >>> to the simpler form. >>> >>> >>> >>>> - Instead of using hard coded PHP path , you might want to use it >>>> in a dynamic way. This is because PHP 5.2.6 integration is coming >>>> pretty soon and is going to have a different file layout. >>>> >>>> >>> I could just use /usr/bin/php-cgi for the executable. Will 5.2.6 use >>> the same /usr/bin/php-cgi symlink? Would that be stable enough? For >>> the php.ini file things are a little more difficult and I need to >>> look into this some more. >>> >>> >>> >>>> - With MySQL, you will now start the server as svcadm enable >>>> mysql:version_50. Looks like, Apache and Lighttpd is going to start >>>> their servers like svcadm enable http:apache22 and >>>> http:lighthttpd14. Should we not all do this in a consistent way ? >>>> I guess, this is a separate discussion. >>>> >>>> >>> I agree, we should have this discussion. version_50 is somewhat >>> different from lighttpd14, it feels that 'version' and '50' need to >>> be separated with an underscore, but I don't get the same feeling for >>> lighttpd14 or apache22. I wonder if mysql:version_50 might have >>> worked as mysql:50 >>> >>> >>> >>> >>> >>>> - within the man page, there is a reference to memcached. Probably >>>> a miss in cut and paste . >>>> >>>> >>> Great spot, I shall fix both of these references >>> >>> >>> >>>> - While installing man pages, should we not use the convention of >>>> _install M ? >>>> >>>> >>> I've done this with the main lighttpd.1 page, which is a page that I >>> created, to make this work I created the sunman-stability sed script >>> required by _install M. The Lighttpd man pages are patched to add >>> some more detail but they are installed by the standard make, not by >>> the script. I can changed this, but looking at PHP, MySQL and Apache, >>> none of their community provided man pages have the sunman-stability >>> data appended. >>> >>> >>> >>> >>>> - If the binaries and libraries are installed using _install , then >>>> we can be sure that the symbols get stripped out. I see some >>>> components doing this and some not. I am not sure, what is the >>>> convention within SFW ? >>>> >>>> >>> All 3 of the executables are stripped by the $SRC/tools/post_process >>> command and all of the libs are stripped by >>> $SRC/tools/post_process_so. I don't think I'm missing any and I was >>> fairly confident that this was the right way to do it if not using >>> _install. Did you see any specifically that I missed? >>> >>> >>> Thanks >>> >>> Amanda >>> >>> >>>> Hope this helps >>>> Sriram >>>> >>>> Amanda Waite wrote: >>>> >>>> >>>>> Hi there, >>>>> >>>>> Before we do the putback on Lighttpd we'd like to get a few more >>>>> eyeballs to go over the code and give us a thorough review. Not a >>>>> great deal has changed since the last code review, but we have >>>>> removed the horrible LD_LIBRARY_PATH statement from Makefile.sfw >>>>> and no longer ask the configure script to use mysql_config. >>>>> >>>>> The code review is at >>>>> http://cr.opensolaris.org/~tekgrrl/lighttpd-putback/ >>>>> >>>>> We would really appreciate your feedback. >>>>> >>>>> Thanks >>>>> >>>>> Amanda - ISV-Engineering >>>>> _______________________________________________ >>>>> >>>>> >>>>> webstack-discuss mailing list >>>>> webstack-discuss at opensolaris.org >>>>> http://mail.opensolaris.org/mailman/listinfo/webstack-discuss >>>>> >>>>> >>> _______________________________________________ >>> >>> >>> webstack-discuss mailing list >>> webstack-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/webstack-discuss >>> >>> >> > > _______________________________________________ > > > webstack-discuss mailing list > webstack-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/webstack-discuss >
