Aw: Re: Aw: Re: cvs commit: ports/www/redmine Makefile ports/www/redmine/files redmine.in

[Bernhard Fröhlich]

----- Ursprüngliche Mitteilung -----
> On 03/14/10 10:56, Bernhard Fröhlich wrote:
> > ----- Ursprüngliche Mitteilung -----
> > > On 03/13/10 20:07, Wesley Shields wrote:
> > > > wxs                              2010-03-14 04:07:41 UTC
> > > >
> > > >        FreeBSD ports repository
> > > >
> > > >        Modified files:
> > > >                www/redmine                                  Makefile
> > > >        Added files:
> > > >                www/redmine/files          redmine.in
> > > >        Log:
> > > >        - Add RC script.
> > > >     
> > > >        PR:                                              ports/144299
> > > > http://www.FreeBSD.org/cgi/query-pr.cgi?pr=144299      Submitted by:   
> > > > Bernhard Froehlich <decke at bluelife.at> (maintainer)   
> > > >        Revision  Changes          Path
> > > >        1.8                      +6 -0                  ports/www/redmine/Makefile
> > > >        1.1                      +32 -0              ports/www/redmine/files/redmine.in (new)
> > > >
> > > > http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/redmine/Makefile.diff?&r1=1.7&r2=1.8&f=h
> > > > http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/www/redmine/files/redmine.in
> > >
> > > There are several problems with this script.
> >
> > Thanks for your comment. I will have a look at the issues in the next few days.
> >
> > > 1. The REQUIRE line needs to be changed to LOGIN. This is preferred for
> > > all local scripts, but mandatory for those which run with a non-root
> > > [ug]id (which this one does).
> > >
> > > 2. Instead of command=%%RUBY_NAME%% it's likely you want to use
> > > command_interpreter instead. Does the stop command work with this script
> > > as it is currently?
> >
> > Yes it does work fine. I use it since a few months. The binary that is shown
> > with top is ruby18 and RUBY_NAME is exactly that.
>
> My concern would be that for users with other versions of ruby installed
> it might not work, but if what you have works now (and no one more
> knowledgeable about ruby speaks up) then that's fine.
>
> > > 3. _flags should not be included in command_args, it will result in them
> > > being included twice.
> >
> > Oh did not know that the flags are automatically append to command_args. I
> > will check and remove this then.
>
> Ok.
>
> > > 4. Are all of the options included in _flags actually things that a user
> > > might wish to twiddle? Should some of them be in command_args instead?
> >
> > Yes they are but the user should have no reason to change user and group so
> > this could be moved to command_args.
> >
> > > 5. The pidfile location looks dubious to me, but I'm not sure if putting
> > > pid files in WWWDIR has somehow become routine?
> >
> > That is a bigger disussion because redmine is designed to work completely
> > inside of his home directory and that includes scripts, pidfiles, tmp and
> > everything else. I have already looked at spliting that up but decided against
> > it because it would be a bad hack.
>
> I'm not willing to tilt at that windmill, so if anyone cares about this,
> speak up. :)
>
> > > 6. And finally a minor issue, %%RC_SUBR%% should just be /etc/rc.subr.
> > > I'll be making that change for all ports once the ports freeze is over,
> > > so no reason not to do it now.
> >
> > Did not know that either. When thinking about that issues it would have been
> > good to have a few good examples or a documentation. Does that probably
> > already exist and i just haven't found it?
>
> http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html
>
> > Thanks for your comments and i will also have a look at my other startscripts
> > if they have the same problems.
>
> Ok. Meanwhile I've attached a patch for the current version with the
> non-controversial changes I mentioned above, and also Adding a
> $FreeBSD$, and using %%WWWDIR%% throughout instead of creating a
> variable for it. The SUB routine for the ports can handle as many
> substitutions of the same value as you need it to, and this way is
> (slightly) more efficient, and (slightly) less confusing. With your
> permission I'll commit this fix since Wesley is going to be traveling.

Thanks for the patch! At the moment i do not have a possibility to double check them but the changes look good to me so please feel free to commit them.

--
Bernhard Fröhlich 
http://www.bluelife.at/