[intrepid] menu buttons don't work on main page

Tomas, thank you once again for your bug report.
I can confirm this bug.
This bug is due to the link not being created properly. dh_install -Xprototype.js would also remove the directory prototype and not just the file prototype.js. So changed dh_install to dh_install -xprototype this way the directory and file named prototype are removed. Added to ampache.postinst code to make the prototype directory and added the creation of the symlink to prototype.js to ampache.postinst. Removed the prototype symlink from debian/links. Added code to ampache.postrm to properly purge ampache. Still not sure why 0ubuntu2 succeeded when I tested it. I have tested this fix on 3 different fresh installs of intrepid on my test computer, along with 2 ampache.org community members.

Hi Charlie,

Thanks for working on this. I have a few questions about your patch.

- #Stop and purge debconf.
- db_stop || true
- db_purge || true
-

why don't we want to do that any more?

+ if [ -d /usr/share/ampache ]; then
+ rm -rf /usr/share/ampache
+ rm -f /etc/apache2/conf.d/ampache
+ fi

is /etc/apache2/conf.d/ampache a conffile?

+ mkdir /usr/share/ampache/www/modules/prototype
+ if [ ! -h /usr/share/ampache/www/modules/prototype ]; then
+ ln -s /usr/share/javascript/prototype/prototype.js /usr/share/ampache/www/modules/prototype/
+ fi

why doesn't this work if the link is in the package? I think you tried to explain
this along with the patch, but I didn't get it I'm afraid.

+ cd $BIN
+ php ./migrate_config.inc

can that not be called with the full pathname, instead of using "cd"?

Thanks,

James

On Fri, 2008-10-03 at 17:41 +0000, James Westby wrote:
> Hi Charlie,
>
> Thanks for working on this. I have a few questions about your patch.
>
> - #Stop and purge debconf.
> - db_stop || true
> - db_purge || true
> -
>
> why don't we want to do that any more?
For reasons that elude me (need to do further research as to why), this
bit of code causes ampache.postrm to hang and not complete when it is
present. When removed ampache.postrm completes and purges ampache
properly.
>
> + if [ -d /usr/share/ampache ]; then
> + rm -rf /usr/share/ampache
> + rm -f /etc/apache2/conf.d/ampache
> + fi
>
> is /etc/apache2/conf.d/ampache a conffile?
Yes it is the conf file to create a apache2 alias, it is not ampache's
main configuration file. However there is symlink
from /etc/ampache/ampache.conf to /etc/apache2/conf.d/ampache. When
testing the package I noticed that after
purging, /etc/apache2/conf.d/ampache remained, which I believe is a
violation of debian policy, so I added the code to remove it.
>
> + mkdir /usr/share/ampache/www/modules/prototype
> + if [ ! -h /usr/share/ampache/www/modules/prototype ]; then
> + ln -s /usr/share/javascript/prototype/prototype.js /usr/share/ampache/www/modules/prototype/
> + fi
>
> why doesn't this work if the link is in the package? I think you tried to explain
> this along with the patch, but I didn't get it I'm afraid.
>
Ok let's see if I can explain this a little better. Originally
prototyp.js was actually part of the .orig.tar.gz, but with the new
version of lintian it complained of prototype.js being part of
the .orig.tar.gz as being a "Convenience copy of code" which is a
violation of section 4.13 of the debian policy. So to correct this I
added -Xprototype.js to dh_install to remove prototype.js from the .deb.
I then made the package depend on libjs-prototype.js and used
debian/links to create the symlink. Prototype.js resides
in /usr/share/ampache/www/modules/prototype/prototype.js, the problem is
that dh_install -Xprototype.js would remove the prototype *directory*
along with the file prototype.js thus the symlink being created
improperly, thus not allowing the package to perform correctly. With
Intrepid release rapidly approaching I decided it was easier, and
quicker to move all this to ampache.postinst. And yes before you ask I
did test 0ubuntu2 in a chroot, which in itself is a part of the problem.
I quickly found out that I am not proficient in setting up a proper
chroot for testing, after several frustrating attempts and came up with
the same results that the package installed and performed correctly,
which I new was wrong. So to correct this I went out and bought a used
computer and set it up with an Intrepid system and use it exclusively
for testing purposes. I hope this explains things better
> + cd $BIN
> + php ./migrate_config.inc
>
> can that not be called with the full pathname, instead of using "cd"?
>
done, used full pathname
> Thanks,
>
> James
>

On Sat, 2008-10-04 at 03:16 +0000, Charlie_Smotherman (porthose) wrote:
> On Fri, 2008-10-03 at 17:41 +0000, James Westby wrote:
> > Hi Charlie,
> >
> > Thanks for working on this. I have a few questions about your patch.
> >
> > - #Stop and purge debconf.
> > - db_stop || true
> > - db_purge || true
> > -
> >
> > why don't we want to do that any more?
> For reasons that elude me (need to do further research as to why), this
> bit of code causes ampache.postrm to hang and not complete when it is
> present. When removed ampache.postrm completes and purges ampache
> properly.

I'm just guessing, but perhaps the purge should be before the stop?

> >
> > + if [ -d /usr/share/ampache ]; then
> > + rm -rf /usr/share/ampache
> > + rm -f /etc/apache2/conf.d/ampache
> > + fi
> >
> > is /etc/apache2/conf.d/ampache a conffile?
> Yes it is the conf file to create a apache2 alias, it is not ampache's
> main configuration file. However there is symlink
> from /etc/ampache/ampache.conf to /etc/apache2/conf.d/ampache. When
> testing the package I noticed that after
> purging, /etc/apache2/conf.d/ampache remained, which I believe is a
> violation of debian policy, so I added the code to remove it.

By "conffile" I meant "conffile" in the Debian policy sense, not
as in "configuration file". A conffile is a file in which user's
modifications are supposed to be preserved. However, I now understand
that this is during purge, so the distinction is not important.

> >
> > + mkdir /usr/share/ampache/www/modules/prototype
> > + if [ ! -h /usr/share/ampache/www/modules/prototype ]; then
> > + ln -s /usr/share/javascript/prototype/prototype.js /usr/share/ampache/www/modules/prototype/
> > + fi
> >
> > why doesn't this work if the link is in the package? I think you tried to explain
> > this along with the patch, but I didn't get it I'm afraid.
> >
> Ok let's see if I can explain this a little better. Originally
> prototyp.js was actually part of the .orig.tar.gz, but with the new
> version of lintian it complained of prototype.js being part of
> the .orig.tar.gz as being a "Convenience copy of code" which is a
> violation of section 4.13 of the debian policy. So to correct this I
> added -Xprototype.js to dh_install to remove prototype.js from the .deb.
> I then made the package depend on libjs-prototype.js and used
> debian/links to create the symlink. Prototype.js resides
> in /usr/share/ampache/www/modules/prototype/prototype.js, the problem is
> that dh_install -Xprototype.js would remove the prototype *directory*
> along with the file prototype.js thus the symlink being created
> improperly, thus not allowing the package to perform correctly. With
> Intrepid release rapidly approaching I decided it was easier, and
> quicker to move all this to ampache.postinst.

Thanks for the clarification. Does adding
/usr/share/ampache/www/modules/prototype/ to debian/dirs help in
having the directory in the deb at all?

Adding a couple of commands to debian/rules to get the .deb to contain
the correct link would be preferable to using postinst if you can't get
dh_* to do what you want.

Thanks,

James

On Mon, 2008-10-06 at 10:19 +0000, James Westby wrote:
> On Sat, 2008-10-04 at 03:16 +0000, Charlie_Smotherman (porthose) wrote:
> > On Fri, 2008-10-03 at 17:41 +0000, James Westby wrote:
> > > Hi Charlie,
> > >
> > > Thanks for working on this. I have a few questions about your patch.
> > >
> > > - #Stop and purge debconf.
> > > - db_stop || true
> > > - db_purge || true
> > > -
> > >
> > > I'm just guessing, but perhaps the purge should be before the stop?

ampache.postrm chokes either way?? Can we just remove this for now? I
am going to discuss this with my DD sponsor to see figure out what is
causing this to happen, and will include any fixes in with my next
debian upload. Would this be acceptable?
>
> > >
> > > + if [ -d /usr/share/ampache ]; then
> > > + rm -rf /usr/share/ampache
> > > + rm -f /etc/apache2/conf.d/ampache
> > > + fi
> > >
> > >This is actually cruft left over from the symlink. So changed it to be
          if [ -h /etc/apache2/conf.d/ampache.conf ]; then
                rm -f /etc/apache2/conf.d/ampache.conf
          fi
> > >
> > Thanks for the clarification. Does adding
> /usr/share/ampache/www/modules/prototype/ to debian/dirs help in
> having the directory in the deb at all?

added debian/dirs, and moved prototype symlink creation to debian/links
this works perfectly now, thanks

Charlie
(porthose)

Hold on don't use that diff yet! during testing, passed, passed, failed, failed. hmmm let me get back to yea.

Three different installs of intrepid, with a successful install, remove and purge on each. Made one minor tweak, but good debdiff

I couldn't apply that patch (amp3+2-debdiff) to 3.4.3-0ubuntu2 sources. Some parts of ampache.postinst script didn't line up. I copied changes from previous patch to clean sources and made a new one, in case someone encounters the same problem. Thanks for the patch, Ampache rocks again.

Hi,

This one looks pretty good, I just have a couple of questions left.

- ln -s /etc/ampache/ampache.conf /etc/$webserver/conf.d/ampache
+ ln -s /etc/ampache/ampache.conf /etc/$webserver/conf.d/ampache.conf

is this needed? There's no transition, so forever more code in maintainer
scripts would have to take account of both paths, e.g.

+ #This is needed to manually remove symlink.
+ if [ -h /etc/apache2/conf.d/ampache.conf ]; then
+ rm -f /etc/apache2/conf.d/ampache.conf

this now won't remove the old symlink will it? So people who use ampache on
Hardy, upgrade to Intrepid, and then purge will still have this symlink.

+ * Added prototype symlink back to debian/links reworked server restart
+ script. (LP: #276945)
+ * Fixed debian/ampache.postrm by fixing server restart and by adding
+ db_purge and db_stop in the correct order.
+ remove /etc/apache2/conf.d/ampache to make ampache purge
+ correctly.
+ * Added mkdir /usr/share/ampache/www/modules/prototype to debian/rules.
+ * Updated debian rules to read -Xprototype.

I'd like it if you could rework this, as I think the iterative updates have muddled
it slightly. For instance the first bullet point seems to be two conceptual changes,
your diff doesn't add db_purge or db_stop, and you don't add the mkdir call
to debian/rules.

Thanks,

James

On Wed, 2008-10-08 at 00:10 +0000, James Westby wrote:

> is this needed? There's no transition, so forever more code in maintainer
> scripts would have to take account of both paths, e.g.

Changes to
 ln -s /etc/ampache/ampache.conf /etc/$webserver/conf.d/ampache

> this now won't remove the old symlink will it? So people who use ampache on
> Hardy, upgrade to Intrepid, and then purge will still have this symlink.
removed from postrm
>

> I'd like it if you could rework this, as I think the iterative updates have muddled
> it slightly.
debian/changelog redone

> your diff doesn't add db_purge or db_stop,
added back to postrm

> and you don't add the mkdir call to debian/rules.
added debian/dirs which creates the
 /usr/share/ampache/www/modules/prototype directory per your suggestion.
added prototype symlink creation to debian/links.
Works great thanks for the suggestion =)

Thanks for all your help.

Attached amp3+4-debdiff

Charlie
(porthose)

Hi Charlie,

It looks almost perfect now. There is one issue left

 exit 0
+
+if [ "$1" = "purge" ] && [ -e /usr/share/debconf/confmodule ]; then
+ . /usr/share/debconf/confmodule
+ db_purge
+ db_stop
+fi

so the code you added will never execute as it comes after an
exit 0 call.

The last bit of

  http://www.fifi.org/doc/debconf-doc/tutorial.html#AEN158

explains what should happen on purge, and states that using
dh_installdebconf will do this for you. ampache uses debhelper,
and so that will be taken care of.

Therefore I think you can safely delete this bit of code and not
worry about the db_purge command.

I would do this for you and upload, but I would like you to double
check and perhaps test.

Another comment I would have, but it's not something that will stop
me uploading this change, is that you should try and explain
why you are doing things in the changelog. For example

+ * Updated debian/rules to read dh_install -Xprototype.

explains what was done, but I can see that just by reading the diff.
What the changelog gives you is a chance to explain why you did
these things. Doing that makes it a lot easier for other people to
understand your changes.

Thanks,

James

On Wed, 2008-10-08 at 14:09 +0000, James Westby wrote:

> http://www.fifi.org/doc/debconf-doc/tutorial.html#AEN158

Bookmarked

> Therefore I think you can safely delete this bit of code and not
> worry about the db_purge command.

Removed from ampache.postrm
>

> explains what was done, but I can see that just by reading the diff.
> What the changelog gives you is a chance to explain why you did
> these things. Doing that makes it a lot easier for other people to
> understand your changes.

Added additional comments to changelog to hopefully clarify why the
changes where made.

With a fresh install of intrepid, ampache installed, removed and purged
correctly. Completed the ampache installer, created db, added music to
server, created music catalog, browsing by artist, genre, and song works
perfectly, tested music streaming with and without XPSF flash player.
All test completed successfully.

Including amp3+5-debdiff

Thank for all your help and the above mentioned link

Charlie
(porthose)

This bug was fixed in the package ampache - 3.4.3-0ubuntu3

---------------
ampache (3.4.3-0ubuntu3) intrepid; urgency=low

    * Added prototype symlink creation to debian/links. This means that
      ampache no longer includes a convenience copy of the prototype
      javascript library, and instead is able to use the system one by
      symlinking it to where ampache expects to find it. (LP: #276945)
    * Added debian/dirs to create /usr/share/ampache/www/modules/prototype
      directory, allowing the above symlink to be created within
    * Updated debian/rules to read dh_install -Xprototype.
    * Reworked server restart and removed db_purge and db_stop in
      debian/ampache.postrm so ampache.postrm would complete.
    * Used full pathname for migrate_config_inc in debian/ampache.postinst.

 -- Charlie Smotherman <email address hidden> Tue, 07 Oct 2008 22:07:16 -0500