Bug 1291021 (debruijn) - Review Request: debruijn - Software for the generation de Bruijn sequences for neuroscience experiments
Summary: Review Request: debruijn - Software for the generation de Bruijn sequences fo...
Keywords:
Status: CLOSED WONTFIX
Alias: debruijn
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW fedora-neuro, NeuroFedora
TreeView+ depends on / blocked
 
Reported: 2015-12-12 21:37 UTC by Igor Gnatenko
Modified: 2018-08-22 10:35 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-08-22 07:31:21 UTC
Type: ---
Embargoed:
zbyszek: fedora-review?


Attachments (Terms of Use)

Description Igor Gnatenko 2015-12-12 21:37:39 UTC
Spec URL: https://ignatenkobrain.fedorapeople.org/neurofedora/debruijn.spec
SRPM URL: https://ignatenkobrain.fedorapeople.org/neurofedora/debruijn-0.0.0-0.1.git1563f6f.fc24.src.rpm
Description: Software for the generation de Bruijn sequences for neuroscience experiments.
Fedora Account System Username: ignatenkobrain

Comment 1 Zbigniew Jędrzejewski-Szmek 2015-12-13 03:34:29 UTC
- software is under an acceptable license (BSD with adv.)
- latest (git) snapshot
- license file is wrong, but upstream has been notified
- no scriptlets
- requires/provides are OK
- new python packaging template is used
- builds and installs OK
- fedora-review is happy

You can drop '%doc README.md', the README is essentially empty.

There's a compiled binary repo in the archive, remove it in %prep to be sure it's not packaged by mistake.

Add %check:
%{buildroot}%{_bindir}/debruijn 12 3

It seems fairly easy to crash it by specifying sufficiently large values:
$ debruijn 12 8
terminate called after throwing an instance of 'std::bad_alloc'
  what():  std::bad_alloc
Aborted (core dumped)

valgrind in general is not too happy with the binary ;)

rpmlint:
debruijn.i686: W: spelling-error Summary(en_US) de -> DE, ed, d
debruijn.i686: W: spelling-error Summary(en_US) neuroscience -> pseudoscience
debruijn.i686: W: spelling-error %description -l en_US de -> DE, ed, d
debruijn.i686: W: spelling-error %description -l en_US neuroscience -> pseudoscience
rpmlint is prescient!

debruijn.i686: W: no-manual-page-for-binary debruijn
debruijn.src: W: spelling-error Summary(en_US) de -> DE, ed, d
debruijn.src: W: spelling-error Summary(en_US) neuroscience -> pseudoscience
debruijn.src: W: spelling-error %description -l en_US de -> DE, ed, d
debruijn.src: W: spelling-error %description -l en_US neuroscience -> pseudoscience
debruijn-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/DeBruijn-1563f6f8833d88b7cde399cbf93f35b8b4b81586/debruijn.cpp
debruijn-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/DeBruijn-1563f6f8833d88b7cde399cbf93f35b8b4b81586/debruijn.h
3 packages and 0 specfiles checked; 0 errors, 11 warnings.

All OK.

Comment 2 Igor Gnatenko 2015-12-13 11:41:44 UTC
Fixed. Links are the same.

I tried to figure out what happened with this program, but I cant fix crashing =(

Comment 3 Zbigniew Jędrzejewski-Szmek 2015-12-19 04:57:28 UTC
chmod -x ... build ..., and then pushd build fails with permission denied :(

%check is missing the name of the program to run...

Why do you need cmake? Isn't the upstream Makefile good enough?

It seems that it makes very large allocations on the stack. But even if those are changed to use the heap, function calls recurse very deeply and fill the stack. So there's no obvious "error", just the program is designed in a way where it cannot be used with large arguments.

Comment 4 Zbigniew Jędrzejewski-Szmek 2015-12-19 15:00:24 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> It seems that it makes very large allocations on the stack. But even if
> those are changed to use the heap, function calls recurse very deeply and
> fill the stack. So there's no obvious "error", just the program is designed
> in a way where it cannot be used with large arguments.
In case the intent wasn't clear: I'm trying to say this crashiness is ugly, but not something to hold up the review.

Comment 5 Zbigniew Jędrzejewski-Szmek 2017-03-05 22:33:45 UTC
?

Comment 6 Igor Raits 2018-08-22 07:31:21 UTC
Unfortunately I don't have time to work on these review requests anymore, sorry.


Note You need to log in before you can comment on or make changes to this bug.