sambamba icon indicating copy to clipboard operation
sambamba copied to clipboard

Version 0.6.7 fails to build on i386 but version 0.6.6 has build successfully

Open tillea opened this issue 6 years ago • 9 comments

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.

tillea avatar Mar 21 '18 11:03 tillea

Thanks @tillea, will do soon.

pjotrp avatar Mar 21 '18 11:03 pjotrp

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

LocutusOfBorg avatar Mar 21 '18 11:03 LocutusOfBorg

regression is in commit https://github.com/biod/sambamba/commit/0583f4940a81166d856b5537a920d12113c86530

LocutusOfBorg avatar Mar 21 '18 11:03 LocutusOfBorg

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

LocutusOfBorg avatar Apr 18 '18 09:04 LocutusOfBorg

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.

tillea avatar Apr 18 '18 11:04 tillea

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.

pjotrp avatar Apr 19 '18 08:04 pjotrp

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.)

lomereiter avatar Apr 20 '18 07:04 lomereiter

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.

tillea avatar Apr 20 '18 08:04 tillea

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.

pjotrp avatar Apr 21 '18 05:04 pjotrp