tirex review

[Felix Delattre]

Hi Bas,

Trying to treat briefly each point:

On 9/3/21 8:54 AM, Sebastiaan Couwenberg wrote:
> tirex:
> 
> 
>  * debian/changelog
> 
>    "Targeted packaging to debian archive." and "Initial package" entries
>    can be merged, and the entry closing the ITP should be the first
>    changelog entry. Also treat the changelog entry as a sentence ending
>    with a period.

Done.
 
>    Version using ~exp1 suffix but distribution is set to UNRELEASED, the
>    suffix is only appropriate when distribution is set to experimental.
>    Which is correct for an package like this that has to pass NEW before
>    entering the archive.

Changed to: `experimental`.

> 
>  * debian/control:
> 
>    tirex-syncd is Architecture: any but lacks ${shlibs:Depends}.
>    Since it only contains a Perl script, it should be Architecture: all.

Removed `tirex-syncd`, after I talked with the maintainers and they told me that this can not be considered stable, yet.

> 
>    Trailing comma for the last dependency is unusual, should be removed.
>    Should use HTTPS for Homepage URL.

Done.

> 
>  * debian/copyright:
> 

Done, what has been mentioned on this file.

> 
>  * debian/etc/logrotate.d/tirex-master
> 
>    File is included in tirex package and hence should not use the
>    -master suffix.

Done.
 
> 
>  * debian/etc/tirex/renderer/mapnik.conf
> 
>    Might want to make this a template and set plugindir based on the
>    version of mapnik the package it built with.
> 
> 
>  * debian/get-orig-source
> 
>    Should be removed since debian/watch is used.

Done.

> 
>  *
> debian/patches/0002-Disconnect-configuration-overlap-between-build-and-d.patch
> 
>    Could be marked as Forwarded: not-needed
> 
Done.
> 
>  * debian/rules
> 
>    Why are test failures ignored?
> 

Removed the part to ignore the test failures and it still works. I guess this was legacy from the old packaging.

> 
>  * debian/tirex.install
> 
>    Use usr/lib/systemd instead of lib/systemd.

When changing this I get `E: tirex: systemd-service-file-outside-lib` (related to the point below you mentioned).

Can you confirm that this is still the way to go?

> 
>    I: tirex: package-contains-empty-directory lib/systemd/
> 
>  * debian/tirex-syncd.install:
> 
>    Remove prefixed / like tirex.install.

Removed `tirex-syncd` entirely.

> 
>  * lintian:
> 
>    systemd-service-file-outside-lib
>    Can be ignored, check not updated yet for new path.

See question above about a second confirmation that this error is to good to be ignored.

> 
>    debug-file-with-no-debug-symbols
>    Not sure why mapnik backend has not debug symbols.

I was able to add a patch for this: https://salsa.debian.org/debian-gis-team/tirex/-/blob/master/debian/patches/0003-Add-debug-information-to-tirex-backend-manager.patch
It doesn't seem to be possible to give the Makefile any variables. I'm not sure if this is the best way on how this should be done. Some input would be helpful.
> 
>    odd-historical-debian-changelog-version
>    changelog can be cleaned up.

Done.

> 
>    package-contains-empty-directory
>    See above for new usr/lib/systemd change.

Not an issue anymore.

> 
>    package-supports-alternative-init-but-no-init.d-script
>    Adding an init script is optional if systemd service is used,
>    add an init script or lintian override.
> 
Added lintian override.


Thanks always for your support!
Best wishes,
Felix