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: