[DRE-maint] Some comments on the redmine package

[Richard Hurt]

On Aug 30, 2008, at 5:11 AM| Aug 30, 2008, Jean-Philippe Garcia  
Ballester wrote:
> 	However, since this is the first rails app that will be package in
> Debian, I think that others will take it as an example, and I fear  
> that if
> the package has a few things to fix, these few things will have to be
> fixed in more that one package. AFAIK redmine won't be in lenny, so  
> I'd
> rather see the first rails package to be as good as possible.

Agreed!

> Anyway, here is what I've seen so far:
> * Use debconf in your postinst, to ask the user if
> 	* he wants to configure the database.yml file
> 	* he wants to create the db
> 	* he wants to load default data
> 	* he wants to configure a webserver (apache2 fcgid / apache2
> mod_rails / …)

I'm not sure I want to use debconf, at least at first.  I've never  
played with it and have no idea where to start.  If someone had some  
input or wanted to take this on I would be grateful.  Otherwise I  
think it will have to wait until the package gets a bit more time  
under its belt.

> * The sample apache configuration you provide has AFAIK the
> following problems:
> 	* It will not work out of the box. If you do not wish to use
> debconf to ask the user which way he wants, you should add in the
> apache2.conf a working configuration (e.g. with mod_fcgid), and in
> comments other working configurations (e.g. with mod_rails).

This apache.conf does work out of the box on my test system.  I'm not  
sure what else to put in here.  Are you saying that the comments  
should be a little more verbose and explain how to set up mod_fcgid?

> 	* Why put a VirtualHost with a DocumentRoot? This is
> confusing, the Alias alone would work better IHMO.

I think the upstream has it like this; I didn't even notice the  
problem.  However, the DocuemntRoot is required unless you have a  
valid one somewhere else.  If I remove it Apache complains about not  
being able to find "/htdocs", which is a default value.

Also, I couldn't get the alias to work without reworking the routes so  
I am going to remove it completely.  Is there a standard way to set up  
Apache?  Should each app use its own VirtualHost or should it use Alias?

Did I hear that the .htaccess file is a bad thing and will be going  
away in the future?  If so, should I move all that stuff to the  
<Directory> section?

>
> 	* The commented VirtualHost DocumentRoot lacks “/public” at
> the end.

Fixed.

> * The README.Debian files contains requirements for running
> redmine. They are IMHO useless, because it is the job of the package  
> to
> ensure the requirements are fullfilled (except for optionnal stuff  
> like db
> and webserver configuration). Thus, I think removing the first two  
> lines
> and modifying debian/control to add dependency on ruby1.8 and rails
> (>= 2.1), rails (<= 2.2) is better.

Agreed & corrected.

> * The debian/redmine.lintian-overrides overrides things that should
> not be overriden, and hides bugs.
> 	* The “script-not-executable” warnings: having a shebang and
> not being executable is contradictory. Either the script can be used  
> as a
> binary, and should be executable, or it is not and then the shebang
> useless.

The feedback I got from upstream was that these scripts were optional  
that are copied to other locations when setting up Apache controlled  
subversion access.  Upstream decided that they should not be  
executable by default.  I can patch them if its something that needs  
to be done.  I agree with you but being as this is my first package I  
have no background on which to base my decision on.  Anyone else have  
any comments?

> 	* The ”package-contains-empty-directory”: your comment
> says “These directories hold temporary data”. However, /usr/share is
> meant to be read-only, and shipping empty directories there is a bug.
> Either the program write something in the directory, and this is a  
> bug,
> or the program doesn't write anything and the directory is useless.

I think maybe my Rails inexperience is showing through.  Can anyone  
shed some light on these directories and where they should be placed?   
I'm thinking that they are standard Rails procedures but might not be  
used in Redmine:
   * .../apps/view/members - I think this is part of scaffolding and  
can be removed.
   * .../apps/sweepers - If its empty the app doesn't use it and it  
can be removed.
   * .../lib/plugins - Future expansion and can be removed.
   * .../vendor/plugins/gloc-1.1.0/lang - No idea???

> * I think the “public” directory should go in /usr/share, because
> AFAIK it is not meant to be changed, neither at runtime nor by the  
> user.

I have really struggled with what to do with "public" but looking back  
on it I can see that you are probably correct.  Based on this feedback  
I have moved .../public back to /usr/share.  Does this work for  
everyone?

The only weird thing is the .../public/plugin_assets directory.   
According to the README in that directory, assets are copied from each  
plugin into this directory each time Rails starts.  It is fully  
automatic and any edits made will not persist across restarts so I  
created it and made the owner www-data.  Does this still belong in / 
usr/share?

> * Plugins should go in separate Debian packages. I am against a
> package in Debian which ships totally different and independant
> software.

Agreed.  But until such time as these plugins are debianized I have  
included them in the base package.  As they become available I'll  
migrate the Redmine package to use them fully.

> * In the “debian/redmine.logrotate” file, you create the log files in
> mode 0666. I think it should be 0644, or maybe 0640. Also, according
> to man page, the create option has no effect when copytruncate is  
> used,
> so maybe you should just remove the create option.

Fixed.  create has been removed.

> * You are using the svn version of redmine to build the package. I
> am not sure, but I think the 0.7.4 release will not be a snapshot of  
> the
> svn, but rather backporting bugfixes without introducing new features.
> Thus, if you ship the svn now, and then the 0.7.4 when it is released,
> there might be regression features. It might be better to use 0.7.3  
> and
> backport patches for rails 2.1.

I was under the impression that they just exported the project and  
crated a tarball, but I have no proof.  Either way I don't know how to  
backport patches for rails 2.1.  I am open to any and all help with  
this.  :)

> * In the “debian/copyright” file, remove references to
> “vendor/rails”, since they are not shipped in the Debian package.

Fixed.

> * You mix in the debian/patches/dispatch.patch two different
> things: using /usr/bin/env instead of calling ruby directly, and  
> fixing the
> path to RAILS_ROOT. I think it should be splitted in two, with a short
> comment of what the patch does and why.

Fixed.  I didn't know where to put the comments though.  Quilt appears  
to have a comment function but I couldn't find any information on how  
to use it.

> * The patch also contains whitespace changes (adding a newline at
> the end of files).

Is that bad?

> * I think symlinking directories in /usr/share is not, as a user's  
> point
> of view, the best thing to do. I prefer to have on my system a clean
> directory structure, rather that symlinks everywhere to fix software  
> not
> respecting the FHS. This seems to me more like a workaround instead of
> a real fix, and I think we should try to see how much work it is to  
> patch
> things to allow configurating the different paths. Some work would  
> need
> to be done directly in Rails, but if a clean patch is made and applied
> upstream, this would fix it for other packages and distributions.

Agreed.  I would much rather have a properly configured system than to  
have symlinks everywhere, but at this point in the game it just  
hasen't happened.  If someone with Rails knowledge could point me in  
the right direction I can look at it.  The .../config directory is by  
far the hardest piece I would think.  .../log & .../files should be  
pretty easy to move, and .../public is no longer a symlink.

I have updated the Pkg-ruby-extras repository with these changes.   
Please let me know if you have any problems / questions / comments.

Thanx!
   Richard