Thanks Nilesh!
Your review is much appreciated. Here are some responses and
clarifications from me.
Hi there! On Fri, May 31, 2024 at 01:06:59PM +0200, Attila Kovacs wrote:The updated package passes the salsa CI (except the test-crossbuild-arm64 job, which cannot to install the gcc build dependency).This is because you declared a gcc in d/control which is not needed. It is guaranteed to be present during build as a part of build-essential set[1].
Great!. I see you removed gcc as a build-dep already. Thanks!
Next, the cross-build once again fails because the Makefile forcefully runs the test as well.
I'm not sure what you mean 'forcefully'. The build runs 'make', and the tests are not included in that. They are run by the separate 'make test' target, which I believe is invoked by 'dh_auto_test'. If I understand it correctly, that is exactly what 'dh_auto-test' is meant to do, i.e. running some sort of self-testing of the build, which is exactly what 'make test' does.
Running cross-compiled binary in native environment is not
something that can realistically happen so it gives you the weird
| bin/cio_file data/CIO_RA.TXT cio_ra.bin
| bin/cio_file: 1: Syntax error: "(" unexpected
as seen here[2] (when CI runs).
Unfortunately, it seems cio_ra.bin is absolutely required to be installed and it
is also arch specific from what I see in your package so we might not be able to
move forward from here w/o finding a way to not generate the .bin file at
runtime.
That's not entirely true. The test uses the 'cio_ra.bin' from the
build directory (not the one that is optionally installed by
'dh_install').
However, you are correct, that the cio_ra.bin file is a platform-specific (actually endian-specific) binary data file, and hence is normally produced by executing 'cio_file' on the target platform -- which of course is not possible when cross-compiling. I cannot think of an easy way to produce the correct cio_ra.bin file when cross-compiling. Is that a show-stopper?
Alternatively, we could exclude the cio_ra.bin from the
cross-build, and if necessary not offer the 'supernovas-cio-data'
sub-package for aarch64, unless it cannot be provided via a
suitable native build otherwise. It's an optional package, which
enhances the functionality of the library, but is not required by
it.
What do you think?
-- Attila.