sambamba
sambamba copied to clipboard
Version 0.6.7 fails to build on i386 but version 0.6.6 has build successfully
Hi, after uploading version 0.6.7 to Debian a bug report was filed that the package does not build on i386 architecture. Here you can see a full build log and this is the problematic part:
cd obj-i686-linux-gnu && LC_ALL=C.UTF-8 ninja -j4 -v
[1/74] ldc2 -Isambamba@exe -I. -I.. -I/usr/include/d -enable-color -O -g -release -wi -of 'sambamba@exe/main.d.o' -c ../main.d
FAILED: sambamba@exe/main.d.o
ldc2 -Isambamba@exe -I. -I.. -I/usr/include/d -enable-color -O -g -release -wi -of 'sambamba@exe/main.d.o' -c ../main.d
../sambamba/markdup.d(609): Error: static assert `36u == 40u` is false
[2/74] ldc2 -Isambamba@exe -I. -I.. -I/usr/include/d -enable-color -O -g -release -wi -of 'sambamba@exe/sambamba_fixbins.d.o' -c ../sambamba/fixbins.d
[3/74] ldc2 -Isambamba@exe -I. -I.. -I/usr/include/d -enable-color -O -g -release -wi -of 'sambamba@exe/sambamba_flagstat.d.o' -c ../sambamba/flagstat.d
[4/74] ldc2 -Isambamba@exe -I. -I.. -I/usr/include/d -enable-color -O -g -release -wi -of 'sambamba@exe/sambamba_depth.d.o' -c ../sambamba/depth.d
ninja: build stopped: subcommand failed.
Could you please check this. Kind regards, Andreas.
Thanks @tillea, will do soon.
For sake of completeness, I already did report and some additional posts are available on #300. Anyhow, somewhere the "uint" is not properly being fixed size, I tried both uint32_t and uint64_t but I failed so far
regression is in commit https://github.com/biod/sambamba/commit/0583f4940a81166d856b5537a920d12113c86530
I think I'll upload the assert disabled for now in Debian, to make it build again :). From my perspective, it makes things no worse, since this was a bug also in previous release, just hidden by the missing assert
On Wed, Apr 18, 2018 at 02:59:14AM -0700, Gianfranco Costamagna wrote:
I think I'll upload the assert disabled for now in Debian, to make it build again :). Fine for me. Thanks for caring, Andreas.
Thanks. I'll look into the problem when I have time again. I looked into it quickly, but I am not sure whether the old assert or the new assert is actually the bug. Sorry about that.
FWIW, I almost never cared about i386 architecture, as it's very tough to process data being limited by 4GB. (Edit: I've just removed myself from BioD organization and this repo owners/watchers. Pjotr, it's all yours now.)
On Fri, Apr 20, 2018 at 12:49:16AM -0700, Artem Tarasov wrote:
FWIW, I almost never cared about i386 architecture, as it's very tough to process data being limited by 4GB. Fair enough. However, there is a difference between the statement "I almost never cared about i386" in a random issue or whether you write into your docs: "sambamba supports 64 bit architectures only". In the latter case we could restrict the build of the according Debian package technically to 64 bit architectures only. In the former case it sounds like: If you provide a patch we could include it into sambamba.
I agree we should support i386. Not so much because I care about the target (few people will use i386), but because it brings out architecture assumptions. The number of processors is going up, rather than down, in the coming years. Good software will allow for that. Above assertion failure points out something changes on different targets (in this case the target is not that different, really, it is just the use of int and ptr). The problem is that the assertion got 'fixed' in a commit but it is not clear (to me) whether the fix was actually correct. I need to chase it. But chasing bugs is often not trivial and I need to make time for this.
So, pending.