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

Re: OpenMRS package is ready, I believe



On Tue, Sep 28, 2010 at 06:18:08PM -0500, Misha Koshelev wrote:
> 
> I started from scratch today and was able to make a package that I
> tested on my squeeze system and seems to work.

I have found your work in SVN as well as on mentors.debian.net - thanks
for your intensive work on OpenMRS!

I'm not really sure why you started from scratch - but for educational
issues this is a good thing in any case.  In any case it results in a
quite reasonable package - that's quite an advance after such a short
time for a beginner!  Great job.

Now comes the nitpicking part. ;-)

  1. You dropped the debian/watch file.  The reason might be that it is
     currently non-functional but I would recommend keeping it by including
     a comment about this problem.  The debian/watch file is in so far
     important that maintainers will be notificated automatically about
     new upstream versions, we are using the information on the tasks
     pages (http://debian-med.alioth.debian.org/tasks/his#openmrs) where
     we can inform about new upstream versions as well.

     So I would recommend keeping the watch file for now - even better
     would be to talk to upstream to release tarballs which makes life
     easier not only for Debian maintainers anyway.

     See also
	$ lintian -I openmrs_1.6.1-1.dsc 
	I: openmrs source: debian-watch-file-is-missing

  2. You moved the code from debian/get-orig-source right into the
     apropriate target in debian/rules.  This is fine in principle but
     then you should call 'make -f debian/rules get-orig-source' in
     the watch file.  For my taste this is not the best solution but
     it is just a matter of taste and I will not question your decision
     if it was made in awareness of this consequence.

  3. I told you I would start nitpicking and here I am: Please do not
     refer to the "Sample debian/rules that uses debhelper".  Just
     remove the template comment and replace it by something like

      # debian/rules for openmrs
      # Author: Misha Koshelev <misha680@gmail.com>
      # License: GPL / BSD / whatever

     or something like this.

  4. debian/README.source:  Please follow the advise given in this file.
     I would drop there a note that the source tarball was obtained from
     SVN via get-orig-source target in debian/rules.  Moreover the
     text of README.Debian rather belongs here because it is an information
     for packagers rather for users.  The README.Debian file is targeting
     at users who installed the package.  Chances are good that they have
     no idea what you are talking about and it is simply confusing there.

  5. debian/README.Debian:  This file should give straight and simple
     information to users who have installed the package and now are
     wondering what to do next.  From my point of view - and I can really
     serve as such a user because I have no idea about OpenMRS - this is
     the weakest part of your package:  Please specify the next steps
     after the installation.  This is most probably one of the weakest
     part of the package and it is a consequence of your non policy conform
     debian/postinst file (see below).

     README.Debian should probably say something like

         Go to http://localhost/openmrs to complete the setup

     (I *intentionally* dropped the port number - see below) at least.
     Some hint to some online manual or somethink like this is the bare
     minimum.  Just mention everything a new OpenMRS user should know to
     get a running system.

  6. debian/control: Standards-Version should be 3.9.1 (see the lintian
     warning)

  7. debian/copyright: The copyright file is fine.  However, in the Debian Med
     team we try to support machine readable copyright format as proposed in

        http://wiki.debian.org/Proposals/CopyrightFormat

     but that's nitpicking.  Feel free to ignore this hint - the other items
     are more important.

  8. debian/docs: This file is mentioning readme.txt twice which is finally
     no problem but I wonder whether you forgot to mention a different file
     instead.  Moreower I really wonder whether it makes sense to install it
     in the docs directory of the installed package.  IMHO it provides no
     help for a user of the package and is at best confusing.  Please keep
     developer oriented information out of the installed package.

  9. Please follow the advises of

       lintian -i -I *.dsc *.deb

     and ask here if something remains unclear.

Nitpicking part is over, now to the really weak part of your packaging
which is debian/postinst.  Besides the bashisms mentioned by lintian
this is the wrong way to do the installation.  I'm to busy to seek for
the exact place of the docs but there are definitely statements about
the fact that postinst should work non-interactively.  Any user
interaction should be done via debconf in a debian/config file and now
we start diving into the advanced packaging.

However, I do not think that there is any user interaction needed at
all.  I do not know tomcat but I'm definitely sure that in your postinst
you are asking for things which are not questionable but just defined in
Debian.  Please have a look into the tomcat packaging or other packages
which are using tomcat.

If OpenMRS needs tomcat6 just use

   Depends: tomcat6

in debian/control and the "if [ -d "$tomcathome" ] then" condition becomes
moot, because you can assume it is installed.  Please also do not echo
things like "Deploying the OpenMRS web application" - that's just noise in
a postinst script.

I'm really sure that the question for the Tomcat webapps directory is
useless.  A search

  $ apt-file search tomcat6 | grep webapps
  tomcat6: /etc/tomcat6/policy.d/04webapps.policy
  tomcat6: /usr/share/tomcat6/webapps/default_root/META-INF/context.xml
  tomcat6: /usr/share/tomcat6/webapps/default_root/index.html

uncovers some hints - please read the docs accompanying the tomcat
package in Debian (or if everything else fails the list
debian-java@lists.debian.org might be a proper place to ask for advise).
So finally there is no real need to move openmrs.war around - just
install it to the proper location inside the package.

Similarly there is probably no need to ask for a tomcat user.

Finally restarting a service is usually (I know it from apache2 and
just guess it is the same for tomcat6) via:

            if [ -x /usr/sbin/invoke-rc.d ]; then
                invoke-rc.d tomcat6 reload 3>/dev/null || true
            else
                /etc/init.d/tomcat6 reload 3>/dev/null || true
            fi

In the postinst script you also try to do something which is a no-go

   sed -i 's/TOMCAT6_SECURITY=yes/TOMCAT6_SECURITY=no/' /etc/init.d/tomcat6

Changing other packages configuration is strictly forbidden by policy!
There are several way to handle this.

  1. The soft way which might be sufficient for the moment:
     Give advises for the local admin how to change /etc/init.d/tomcat6
     (and why) so he can do the edit manually.
  1a. You might check in the postinst script whether the change is needed
      or was done for some other purpose before and issue a warning like:
 
        You need to change /etc/init.d/tomcat6 to run OpenMRS successfully.
        Please read /usr/share/doc/openmrs/README.Debian for further advise.

     This should suffice for the moment.

  2. The more elegant way:
     Use debconf and verify in debian/config whether /etc/init.d/tomcat6
     is configured properly to run OpenMRS.  If not issue a warning with
     proper advise what to do.
     While this would be my prefered solution I would not like to see you
     as a beginner spending to much time in debconf issues which is a bit
     tricky (specifically because at the moment when the configure script
     is running it is not sure that /etc/init.d/tomcat6 really exists -
     so this is really an advanced task).

  3. The brave mans way:
     File a wishlist(!) bug against tomcat6 and ask for implementing a
     debconf configuration option for the TOMCAT6_SECURITY variable.  A word
     of warning: Quite frequently maintainers of packages are not happy
     about such requests.  I would not start with this option in the first
     place but keep it in mind for the future when openmrs is an established
     Debian package.

If you follow all these hints you postinst becomes a) lintian clean and
b) policy conform.  You might also notice that this is on one hand the
hard part of the packaging (because you need to dive into specifics of
the tomcat6 policy (=reading even more docs) but on the other hand it
makes clear what the strength of Debian is:  Preparing a system which
works out of the box with the least user interaction.

Ahh, and I forgot to say:  Once you have read the tomcat6 policy you
probably know which port is used and if my suspection is right then
there will be an advise to drop an apache rewriting rule in
/etc/apache2/conf.d to enable a simple URL without mentioning any
specific port - at least I would do so in a package of mine.

Now back to your remaining questions.

> I have not yet progressed
> to using pbuilder for sid. If I need to do this, please let me know.

There is no real need to.  Your sponsor (probably me) will surely use
pbuilder and report issues if there are any.  So if you get a proper
package with all needed Build-Depends (which seems to be the case) there
is no real need to do even if it is simply a good idea.  (Didn't you
reported success in using pbuilder in your last mail???)

> (I
> uploaded the relevant version to the subversion repository as well -
> thank you so much.)

Found it there.
 
> I have attached the package (UPDATE: it is quite large, I will send in a separate
> email once I am home).

There is no need to send any packages via mail (and I think you did not
luckily).  Commiting to SVN is sufficient, having a copy on mentors.d.n
is an additional plus (for me personally not really needed but some
sponsors prefer this).
 
> Additionally, I have figured out how to build the package.
> 
> Yours comments were quite helpful, but this page:
> http://wiki.debian.org/DebianMed
> also proved quite useful. 

Good to know.  If you think something is missing there - just tell us
(or edit the Wiki!).
 
> Additionally, I now understand the correct steps to package to be:
> sudo aptitude install openjdk-6-jdk ant debhelper quilt svn-buildpackage
> devscripts -y
> alias svn-b='svn-buildpackage -us -uc -rfakeroot --svn-ignore'
> svn checkout svn://cvs.alioth.debian.org/svn/debian-med/trunk/packages/openmrs/trunk
> cd trunk
> ./debian/rules get-orig-source
> echo "origDir=.." >> .svn/deb-layout
> svn-b

That's correct.
 
> These work on both Debian squeeze and Ubuntu 10.04.
> 
> In any case, please let me know what further steps are necessary to get the deb in the 
> debian-med repository, and perhaps into the Ubuntu PPA.

For the moment: Fixing things above, most importantly the postinst issues.
Then I might have another look.
 
> p.s. I finally had time to carefully read the Debian New Package Maintainers Guide
> http://www.debian.org/doc/maint-guide/
> You are correct. It is very useful although, I must say, somewhat long :(

Yes, it's longish.  But cherypicking for the parts which are most
interesting for you works.
 
> p.p.s. If you could please reply all, much appreciated. Thank you!

BTW, there is no need to CC me - I'm reading the list.  If you confirm
that you read the list as well, I will also stop CCing you.

Hope the hints are helpful

      Andreas.

-- 
http://fam-tille.de


Reply to: