On Fri, Apr 16, 2021 at 1:31 PM Michael Tremer michael.tremer@ipfire.org wrote:
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”).
Understood. I initially wrote the message explaining the individual parts and intentionally denoting which files contain particular changes. I was also not sure if a slew of few-line changes in separated patches would really be preferred.
I will split and send in the individual fixes as separate patches, based on the concept of 'smallest logical change' to allow bisecting/provide a cleaner trail, and keep it in mind for future.
- 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?
Yes - I don't see why not, if so preferred. Will poke around with this one then.
- 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?
Yes - a short script can easily be done to facilitate the updating process. Do you want a separate script in debian/ to do this, or to be included as part of some already existing tooling?
- 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.
It could probably be set up to run the same test suite direct invocation (not via Makefile). Would require a duplicated reference to the test suite itself, but avoid duplicating the test logic itself. That feels like the best approach to this.
--Valters