[Git][debian-gis-team/libapache2-mod-tile][master] 3 commits: added systemd.service

[Felix Delattre]

Hi Bas,

Thanks for the check-in! Please find my answers in between lines:

On 8/26/20 4:19 PM, Sebastiaan Couwenberg wrote:
> On 8/26/20 5:39 PM, Felix Delattre wrote:
>> Felix Delattre pushed to branch master at Debian GIS Project / libapache2-mod-tile
>> Commits:
>> 24720515 by Felix Delattre at 2020-08-25T20:24:18+00:00
>> added systemd.service
>>
>> - - - - -
>> 2757a3eb by Felix Delattre at 2020-08-26T15:18:33+00:00
>> moved helper scripts to a suitable location
>>
>> - - - - -
>> 77d80483 by Felix Delattre at 2020-08-26T15:27:34+00:00
>> prepared libapache2-mod-tile to work equally with renderd and tirex
>>
>> - - - - -
> 
> Commit messages are sentences, they start with a capital and end with a
> period.


Is this a suggestion? Or a rule of the team?

>> =====================================
>> debian/control
>> =====================================
>> @@ -1,7 +1,7 @@
>>  Source: libapache2-mod-tile
>>  Maintainer: Debian GIS Project <pkg-grass-devel at lists.alioth.debian.org>
>>  Uploaders: Felix Delattre <debian at xama.nu>
>> -Section: science
>> +Section: web
> 
> Don't change the section of the source package.
> 
>>  Priority: optional
>>  Build-Depends: debhelper (>= 10~),
>>                 dh-apache2,
>> @@ -18,11 +18,12 @@ Homepage: https://wiki.openstreetmap.org/wiki/Mod_tile
>>  
>>  Package: libapache2-mod-tile
>>  Architecture: any
>> -Section: httpd
>> -Depends: libjs-openlayers,
>> -         renderd | tirex-master,
>> -         ${shlibs:Depends},
>> +Section: web
>> +Depends: ${shlibs:Depends},
> 
> Especially when you duplicate that same section for a binary package.

I assumed they should be the same. And I can't really see why this should fit into "science".
Can you give me some more context please, why do they have to be different? And why wouldn't it be better to change it to "web"?


>> =====================================
>> debian/libapache2-mod-tile.apache2
>> =====================================
>> @@ -1,3 +1,2 @@
>>  mod src/.libs/mod_tile.so
>>  mod debian/tile.load
>> -conf debian/libapache2-mod-tile.conf
> 
> Why do you remove the config?

It is not removed. It is renamed and handled in the `renderd` package. Thus this prepares `libapache2-mod-tile` for opening to be used by either `renderd` or `tirex`.

> 
>> =====================================
>> debian/libapache2-mod-tile.dirs deleted
>> =====================================
>> @@ -1,2 +0,0 @@
>> -usr/share/libapache2-mod-tile
>> -var/lib/mod_tile
> 
> Why is this removed?
> 

It is not removed. It is created through other mechanisms.

> 
>> =====================================
>> debian/libapache2-mod-tile.install
>> =====================================
>> @@ -1,5 +1,3 @@
>>  debian/tile.load                  etc/apache2/mods-available
>>  slippymap.html                    usr/share/libapache2-mod-tile
>>  munin/mod_tile*                   usr/share/munin/plugins
>> -openstreetmap-tiles-update-expire usr/bin
>> -osmosis-db_replag                 usr/bin
> 
> Why aren't these binaries installed?

Have you seen them? :) I moved them to via package.examples into `/usr/share/doc/libapache2-mod-tile/examples`
I don't think they are even close to be responsibly usable in `/usr/bin`.
 
>> =====================================
>> debian/libapache2-mod-tile.links deleted
>> =====================================
>> @@ -1 +0,0 @@
>> -usr/share/javascript/openlayers usr/share/libapache2-mod-tile/openlayers
> 
> Why is this removed?

The example map should not be created in `libapache2-mod-tile`. It is too specific and is going to be factored into its own package.

>> =====================================
>> debian/libapache2-mod-tile.postinst
>> =====================================
>> @@ -3,9 +3,6 @@
>>  set -e
>>  
>>  if [ "$1" = configure ] ; then
>> -    if [ -e /var/lib/mod_tile ]; then
>> -        chown www-data:www-data /var/lib/mod_tile
>> -    fi
> 
> Why is this removed?

Because permission on this directory needs to be handled either by `renderd` or `tirex`.
I'm considering to avoid the use of `www-data` for this. And introduce rather a `_renderd` user.

> 
>>      # only enable on new installs, not on upgrades
>>      if [ -z "$2" ] ; then
>>
>>
>> =====================================
>> debian/libapache2-mod-tile.conf → debian/libapache2-renderd.conf
>> =====================================
>> @@ -1,6 +1,6 @@
>> -Alias /mod_tile /usr/share/libapache2-mod-tile
>> +Alias /mod_tile /var/lib/mod_tile
>>  
>> -<Directory /usr/share/libapache2-mod-tile>
>> +<Directory  /var/lib/mod_tile>
>>      Options Indexes FollowSymLinks MultiViews
>>      AllowOverride None
> 
> Why this change?
> 
> /usr/share/<package> is the appropriate place for webapps.

The whole package expects currently things to be in `/var/lib/mod_tile`. This adjusts it for the sake of consistency.

I'm happy to move (all of) it to another path. But I don't think `/usr/share/<package>` is appropriate. As this is not any web app living there! But rather the generated tiles used for serving. From my (naive) understanding it should go under `/var` and instead of `/var/lib/<package>` I would preferably choose `/var/cache/<package>`. What do you think?


> 
> https://wiki.debian.org/Apache/PackagingFor24
> 
> Please motivate your changes better.

Please excuse. There are a lot of moving pieces in order to get this to work. The original files are not in a good shape and many things have to change. Explaining everything through mail would be tedious. Please look at the commits and their messages. I usually bundle everything into logical steps. And I'm always happy to answer questions and react to your comments.

Best,
Felix

> 
> Kind Regards,
> 
> Bas
>