Hello,
On 15 Apr 2021, at 13:31, Valters Jansons valter.jansons@gmail.com wrote:
On Thu, Apr 15, 2021 at 2:06 PM Michael Tremer michael.tremer@ipfire.org wrote:
- d/.gitignore: Ignore all temporary files and subdirectories such as
debian/location-importer/ and debian/location-importer.debhelper.log with the exception of debian/source/ and potential debian/patches/ which may be used for Quilt, considering the source format is set to '3.0 (quilt)'.
Okay. If this would have been an individual patch I would have merged this straight away.
Didn't feel like splitting things out from the debian/ cleanup here.
I would strongly recommend doing this, because it makes reviewing things a lot easier. Also parts could be reverted later if there is a problem in the patch.
This way, I struggled to find the bits that belong together and only the well-structured commit message has helped. If that wasn’t there I probably would have rejected it straight away, because generally it is too dangerous to accept patches that cannot be understood (in reasonable time).
I wrote more in this here: https://wiki.ipfire.org/devel/submit-patches (see "Separate your changes”).
- d/clean: Remove m4/intltool.m4 and po/Makefile.in.in autogenerated
files prior to building/in-between builds. Without removal of these autogenerated files, build tooling complains about unexpected changes to the source tree.
Don't the scripts call “make distclean” or something similar?
Yes, `make -j$(nproc) distclean` is invoked. Those two (symlink) files are left dangling however after make finishes.
Maybe we should add them to CLEANFILES?
Keep in mind those two files are currently coming in from `intltoolize --force --automake`, either from the override_dh_auto_configure target or the autogen.sh script.
- d/copyright: Update format link to use HTTPS instead of HTTP
(lintian: insecure-copyright-format-uri).
- d/libloc1.symbols: Added symbols export file
(lintian: no-symbols-control-file). For generation: $ debuild -uc -us # to easily build everything to debian/tmp/ $ dpkg-gensymbols -plibloc1 -Odebian/libloc1.symbols $ sed -i -E -e 's/( [0-9.]+)-.+$/\1/' debian/*.symbols
Hmm, this feels very ugly to me.
We already have a file that has all exported symbols. What does Debian use this for?
To quote documentation: dpkg can use symbols files in order to generate more accurate library dependencies for applications, based on the symbols from the library that are actually used by the application.
In essence: dpkg-shlibdeps can for example check what version a symbol was introduced at, and check for compatibility that way. This new file is like a changelog for the symbol information.
Yes, I know what it does. I would just like to avoid generating a file like this manually every time something is being updated. It is easily forgotten or goes wrong.
Can we script this?
- d/location-python.examples: Add the examples/ to documentation
(lintian: package-does-not-install-examples).
I am not if it is a good idea to install the example key just in case that people start using it.
We have seen this in various places where the “development/testing” key suddenly “slipped” into production.
Will remove the examples change then.
- d/watch: Add uscan configuration, as expected for '3.0 (quilt)' format
(lintian: debian-watch-file-is-missing).
What does this do?
Allows automatic detection of an updated upstream (source.ipfire.org) version having been packaged.
You can see the verbose behind-the-scenes by running: $ uscan --no-download --verbose --debug
Understood.
- `location-importer` not having a manpage.
Good question if it is worth writing one. Is anyone up for this?
I will leave this one hanging for now from my side. Would of course be very good if someone were to pick it up.
- An out-dated Standards-Version.
?
d/control has Standards-Version which signals compliance with the Policy Manual. This note just means the listed changes should be evaluated, and the Standards-Version updated accordingly.
I wanted this patch now to provide individual chunks that deal with the particular listed Lintian issues. I will try to pick Standards-Version up in an individual separate commit afterwards, so that it is clear that any changes taking place there are a part of the upgrade.
https://www.debian.org/doc/debian-policy/upgrading-checklist.html
- An old debhelper compatibility level.
Can we not change this?
Yes, it's perfectly doable. Buster has debhelper v12 available, and I will switch to debhelper-compat along the way.
There are changes associated with this upgrade -- I am aware of the systemd change affecting this repository, and there could be others. As a result, the reasoning for not doing it here is the same as for Standards-Version above.
- Lack of an autopkgtest testsuite.
?
Debian has autopkgtest for Continuous Integration testing. Very shortly, you can specify d/tests/control with a listing of test cases, and then provide the particular commands along with their metadata (dependencies, restrictions, etc) in d/tests/ files.
During build, the Makefile's testcases are run, however autopkgtest additionally allows re-testing a package at a later point when the dependencies have changed.
Interesting. We already have a test suite and I would like to avoid duplicating more code. Loading the library probably won’t be enough.
Another one of those things that is just nice to have configured, but is not 'critical' in of itself.
Agreed.
-Michael
--Valters