[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: daloradius (updated package)



Hey Chris,

On Dec 25, 2007 10:08 PM, Chris Lamb <chris@chris-lamb.co.uk> wrote:
liran tal wrote:

> I am looking for a sponsor for the new version 0.9.5-2
> of my package "daloradius".

This is (at least) the second time you have posted this package to the list
and not corrected serious problems. As Romain Beauxis previously pointed out,
this is rather annoying. I'm still spotting a number of problems I sent to
you in September.

I have replied to the last emails of the previous post although my replies,
asking for a hand were ignored but let's move on, I wish not to dwell
in the past.

Please, Liran, you must either attempt to fix some of these problems (we will
gladly help) or you should reply to this message justifying them.

I am doing everything in my power to get this package built right.
There is an enormous amount of pages about building debian packages
and none of them is consistent about how to build such package.
Unfortunately the New Maintainer's Guide is hardly friendly,
but I am putting more effort into it again so let's try to make something
out of it.
 

As it stands, the package does not just exhibit poor "style"; it simply does
not install any Daloradius files, and you cannot expect it to be sponsored
as-is.


 * Wrong 'Architecture:' in debian/control

If I look at http://www.debian.org/doc/maint-guide/ch-dreq.en.html#s-control
The example on line 9 shows exactly that:
       9  Architecture: any
From looking at other packages on packages.debian.net I changed
this to 'all'.
 

 * Incorrect spacing and indentation of description in debian/control

 * Incorrect punctuation/layout in debian/control Description

I'm not sure what you mean but I have attempted to "fix" the description
You could be more clear on what is a correct spacing/indentation and
punctuation/layout?
 

 * Missing Homepage: line in debian/control

I've removed the Homepage from the description and added it as a
line of it's own sorrounded by < > as seen elsewhere.

Here is an evidence of the overwhelming confusion which so many
resources on the net are causing:

"

We recommend that you add the URL for the package's home page to the package description in debian/control. This information should be added at the end of description, using the following format:

      
. Homepage: http://some-project.some-place.org/
"

Link: http://debid.vlsm.org/share/Debian-Doc/manuals/developers-reference/ch-best-pkging-practices.en.html
 

 * Missing ITP number from debian/changelog

Do you mean stating in the changelog that it closes some bug
or rfs? Where is the ITP number coming from?


 * "Old" debhelper debian/compat compatibility number

What should it be set to?


 * Setting 'apache' and 'php' as "Depends:" is probably not what you want

How about that: (?)
Depends: apache2, libapache2-mod-php5, php5-cli, php5-common, php-db, php-pear


 * Freeradius should probably not be a 'Suggests':

Why not? it integrates well with FreeRADIUS.
Maybe FreeRADIUS and php5-mysql should go into Recommends?


 * Why does it create a database called 'radius'? Why can't this be the same
  name of your package? Why do you have to create it at all?

1. The FreeRADIUS package provides a .sql schema which by default
uses the name 'radius' for the database, as well as this is the default
in all of their configuration and is extremely common across all deployments
of FreeRADIUS.
2. Why to create another database? this will only cause confusion among
daloRADIUS users AND it's not possible anyway because daloRADIUS
relies on tables from FreeRADIUS's radius database.
3. If the 'radius' database isn't created yet then I prompt the user to create it
and populate it with tables for FreeRADIUS and daloRADIUS.

I note again, daloRADIUS *makes use* and is *dependent* of tables that
are used BOTH in daloRADIUS and FreeRADIUS.
 

 * Lots of pointless and incorrect stuff in debian/rules. For example,
  CFLAGS, dh_strip, etc.

True.
I've no idea how to approach this file as I have nothing to compile,
but rather take a bunch of files and directories (the web app) and put
them in the correct place (/usr/share I assume)
 

 * Documentation in /usr/share/daloradius (eg. FAQS, etc.)

How should that be populated?
Should I create under the debian/ dir another /usr/share/daloradius
subdir and put everything there?
 

In addition:

 * The package simply does not work - no files are installed.

    % dpkg -c daloradius_0.9.5-2_amd64.deb
    drwxr-xr-x root/root         0 2007-12-25 20:55 ./
    drwxr-xr-x root/root         0 2007-12-25 20:55 ./usr/
    drwxr-xr-x root/root         0 2007-12-25 20:55 ./usr/share/
    drwxr-xr-x root/root         0 2007-12-25 20:55 ./usr/share/doc/
    drwxr-xr-x root/root         0 2007-12-25 20:55
                                                 ./usr/share/doc/daloradius/
    -rw-r--r-- root/root       177 2007-12-25 20:42
                              ./usr/share/doc/daloradius/changelog.Debian.gz
    -rw-r--r-- root/root       507 2007-12-25 20:42
                                        ./usr/share/doc/daloradius/copyright


That's exactly what I stated in my last email which no one replied.
You have guided me to have an .orig.tar.gz tarball which is exactly
my original package and NOT to put any of these files in the package
I create because that is the upstream package and it should contain no
source files.

Could you please make it more clear?


 

 * Out-of-date Standards-Version in debian/control

What should it be set to?
 


 * Extraneous whitespace on end of lines in debian/control and
  debian/copyright

I will remove any CR or LF  or any spaces there.

 

 * Maintainer scripts are using `read`. You should use Debconf to ask users
  questions like this. It has many, many benefits over `read'.

I will take another look at some manual  for proper Debconf usage
for the maintenance scripts.



 * Maintainer scripts (debian/{postinst,postrm,preinst}) seem to have lines
  like the followiung:

    echo "**** postrm script *****"
    echo "**** preinst " "$*" "is performing ****"
    echo "**** postinst " "$*" "is performing ****" 


  These are rather unneccessary (and possibly against policy, or at least
  recommended against, I can't remember)

The same as above, I'll take another look at the manual and
make the appropriate changes.


Thanks,
Liran.



Reply to: