From owner-freebsd-ports@FreeBSD.ORG Tue Jan 26 18:50:56 2010 Return-Path: Delivered-To: ports@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 07393106566C for ; Tue, 26 Jan 2010 18:50:56 +0000 (UTC) (envelope-from dougb@FreeBSD.org) Received: from mail2.fluidhosting.com (mx21.fluidhosting.com [204.14.89.4]) by mx1.freebsd.org (Postfix) with ESMTP id A10868FC08 for ; Tue, 26 Jan 2010 18:50:55 +0000 (UTC) Received: (qmail 17530 invoked by uid 399); 26 Jan 2010 18:50:54 -0000 Received: from localhost (HELO foreign.dougb.net) (dougb@dougbarton.us@127.0.0.1) by localhost with ESMTPAM; 26 Jan 2010 18:50:54 -0000 X-Originating-IP: 127.0.0.1 X-Sender: dougb@dougbarton.us Message-ID: <4B5F398E.5060500@FreeBSD.org> Date: Tue, 26 Jan 2010 10:50:54 -0800 From: Doug Barton Organization: http://SupersetSolutions.com/ User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.9.1.7) Gecko/20100123 Thunderbird/3.0.1 MIME-Version: 1.0 To: =?ISO-8859-1?Q?Francisco_de_Borja_L=F3pez_R=EDo?= References: <20100120165147.666592ef.borja@pexego.es> <20100120212640.GE6618@lonesome.com> <7be7a2801001201600j5c772267na6dea944334618be@mail.gmail.com> <20100121093649.7867d810.borja@pexego.es> <20100121095114.12bab6af.borja@pexego.es> <4B5BEDB1.8080308@FreeBSD.org> <20100126174903.b144472f.borja@pexego.es> In-Reply-To: <20100126174903.b144472f.borja@pexego.es> X-Enigmail-Version: 1.0 OpenPGP: id=D5B2F0FB Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Cc: ports@freebsd.org Subject: Re: New port: finance/openerp-web X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jan 2010 18:50:56 -0000 On 01/26/10 08:49, Francisco de Borja López Río wrote: > On Sat, 23 Jan 2010 22:50:25 -0800 > Doug Barton wrote: > >> On 01/21/10 00:51, Francisco de Borja López Río wrote: >>> >>> Sorry, I forgot to add the attachments in my previous email. >>> >>> These are both the rc.d script and the default configuration file I've >>> been using with the current openerp-server port for some time. >> >> A few notes on the rc.d script. >> >> 1. The name of the file, PROVIDE, and $name should all match. Perhaps >> openerpd, or even erpd? > > If there is no limitation on that, it should be > openerp-web/openerp-server (for the web and server scripts > respectively). Let me try if it works with the - I don't think it will work with the -, and generally simpler/shorter is better. I personally would rather see just openerp and openerpd, but I won't object to whatever you come up with as long as the 3 things are the same for each of the scripts. >> 2. There are quite a few examples of "/usr/local" in there (which is >> fine for a running script), please remember to s#/usr/local#%%PREFIX%%#g >> for the port. > > mmm, I've been taking a look at the rc.d scripts in my servers, and all > of them have those absolute paths. > > Did you mean to set %%PREFIX%% in the rc file, or did you mean to > generate the rc.d script dinamically when installing the port? Look at the files in the ports tree for those and you should see what I mean. Also see the porter's handbook link I posted regarding how to use rc.d stuff in your port. >> 3. The pidfile= should come after load_rc_config, and should look like >> this: >> pidfile="${_pidfile:-/var/run/openerp-server/openerp-server.pid}" >> (substitute of course). Or, consider not allowing the user to >> change the location of the pidfile unless there is a good reason to do so. > > mmm I do something similar to that, Yes, I saw what you did. :) It's overly complicated and IIRC won't actually work if the user tries to set ${name}_pidfile in rc.conf. OTOH, what I suggested will definitely work, and is simpler. Generally however it's not necessary to allow the user to change the location of the pidfile unless there is a compelling reason to do so. > I don't remember exactly where (probably somewhere in the porters > handbook), but I've read that the order is not a problem (using > openerpserver_pidfile before setting it) (IIRC) If you can find the reference please report it, since it's not accurate. You cannot access the values of any of the ${name}_* variables until after load_rc_config. >> 4. I'm not quite sure what eval "${rcvar}=\${${rcvar}:-'NO'}" is >> supposed to accomplish, but if it's "set ${name}_enable to NO by >> default" it's too clever by half. :) Take a look at >> http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html >> for other ways to simplify the default variable assignments (as well as >> some other pointers). > > I got that from this article: > > http://www.freebsd.org/doc/en_US.ISO8859-1/articles/rc-scripting/rcng-confdummy.html > > (check point 3) Perhaps it is not needed tough. Ok, I'll add that to the list of doc bugs. :) Much better (in the sense of easier to read, easier to maintain, etc.) to just use the default variable assignment method in the porter's handbook. >> 5. Just doing 'touch foo' is always cheaper than testing its existence >> first (although it's a micro-optimization) and there is no reason to >> test existence anyway. Same goes with the 'mkdir -p'. > > mmm, you are right, but if you try to create a directory that already > exist, a message about the already-existing directory will appear > (blabla File exists). You really need to test the things I suggested, and preferably read the man pages to educate yourself about these topics. (Hint: I said 'mkdir -p', not 'mkdir') >> 6. Instead of `dirname blah` you should use ${blah%/*} for two reasons, >> first dirname(1) is in /usr/bin, and may not be available at boot, and >> two there is no reason to use it in a shell script when the variable >> substitution is cheaper and easier. > > I got the dirname idea from the milter-bogom port rc.d script. It > seemed reasonable for me ;). Ok, thanks, I'll run a check for that too when I have more time. One of the reasons that I am so pedantic about doing the rc.d scripts right the first time is that bad code gets copied over and over again. > Anyway, with variable substitution you meant, for example, setting a > variable for the directory where the pidfile will be saved, then use > that variable when creating the pidfile variable? Nope. Test what I wrote. blah=/one/two/three echo ${blah%/*} This one is what I would consider an "intermediate" level shell scripting technique, which is why I am spelling it out for you in more detail, but seriously, when someone gives you suggestions, try them. It's the easiest way to learn. :) >> 7. You named your substitute stop_cmd "${name}_poststop" which is >> confusing, and I'm not sure why it's necessary. If you're afraid that >> rc.d isn't going to actually stop the process for some reason we should >> address that. If you feel that you absolutely must have a safety belt >> for this make it a real stop_postcmd. > > Typo, it should be _stop (will be fixed in just a matter of minutes :D, > thnx for the report) I'm glad you caught the typo, but you still haven't answered my question. Why do you need to redefine the _stop method? >> I realize that these are a lot of notes, but don't worry ... it's easy >> to see that a lot of thought and creativity went into this, which is why >> I'm taking the time to try and help improve it. > > That's exactly what I want, I'm working in more scripts and stuff, so > learning some basic stuff before going any further is always a good > thing. In that case I'm glad to help, and glad to put in a little more of my precious time than usual. :) > I've found some more issues while using more and more the port and the > scripts, for example, the openerp-server port should create a new > account _openerp (to use my rc.d script properly) and the openerp-web > port should check if the user exist and, if not, create it (useful for > servers that will serve the web client, connecting to a remote openerp > server). Actually the port itself should create the user, we generally don't have rc.d scripts do that. If the sysadmin wants a different user than the default it's up to them to configure that before starting the service. > I'm working on improved versions of the script, will send them soon to > the list. > > Thnk you a lot Doug > > (And thnx for the portmaster tool too! ;D) heh, you're welcome. Doug -- Improve the effectiveness of your Internet presence with a domain name makeover! http://SupersetSolutions.com/ Computers are useless. They can only give you answers. -- Pablo Picasso