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<do...@freebsd.org> 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="${<name>_pidfile:-/var/run/openerp-server/openerp-server.pid}"
(substitute<name> 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
_______________________________________________
freebsd-ports@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ports
To unsubscribe, send any mail to "freebsd-ports-unsubscr...@freebsd.org"
