Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1981739 - Review Request: klee - Symbolic execution engine
Summary: Review Request: klee - Symbolic execution engine
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2021-07-13 09:17 UTC by Lukáš Zaoral
Modified: 2021-07-15 13:54 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
msuchy: fedora-review?


Attachments (Terms of Use)

Description Lukáš Zaoral 2021-07-13 09:17:10 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/lzaoral/klee/fedora-rawhide-x86_64/02326979-klee/klee.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/lzaoral/klee/fedora-rawhide-x86_64/02326979-klee/klee-2.2-1.fc35.src.rpm
Description: Symbolic virtual machine built on top of the LLVM compiler infrastructure
Fedora Account System Username: lzaoral
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=71817839

KLEE is only supported on x86_64. Therefore, it fails to build on 32 bit architectures. On the other hand, remaining 64 bit architectures compile fine but the enclosed test suite fails.

Even though I'm not an upstream developer of KLEE, I've contributed some fairly important improvements such as support for releases of LLVM.
Also, this is my first package so I'm looking for a sponsor.

Comment 1 Miroslav Suchý 2021-07-13 10:53:33 UTC
I am going to do this review. And I am a sponsor so I can sponsor you.

Comment 2 Miroslav Suchý 2021-07-13 12:09:33 UTC
The description should end with a full stop. Unlike from summary. And the name of package should not be repeated in Summary. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_summary_and_description

--

Are any of BuildRequires needed just for %check? If yes then I recommend splitting them by comment like:

BuildRequires: gcc
# These are needed just for test
BuildRequires: some-test-package

This helps any future bootstrapping.

--

- Development (unversioned) .so files in -devel subpackage, if present.
  Note: Unversioned so-files directly in %_libdir.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_devel_packages

--

lee.x86_64: W: shared-lib-calls-exit /usr/lib64/libkleeRuntest.so.1.0 exit.5

--

klee.x86_64: W: no-manual-page-for-binary gen-bout
klee.x86_64: W: no-manual-page-for-binary gen-random-bout
klee.x86_64: W: no-manual-page-for-binary kleaver
klee.x86_64: W: no-manual-page-for-binary klee
klee.x86_64: W: no-manual-page-for-binary klee-replay
klee.x86_64: W: no-manual-page-for-binary klee-stats
klee.x86_64: W: no-manual-page-for-binary klee-zesti                                                                                                          
klee.x86_64: W: no-manual-page-for-binary ktest-tool

While this is not a blocker, it would be nice to have one. Probably co-operate with upstream on this.

--

BTW most of these issue I found using `/usr/bin/fedora-review -b 1981739`.

Comment 3 Lukáš Zaoral 2021-07-13 13:33:07 UTC
Thank you for the quick review. I'll fix the wording of the summary and description
and I'll contact upstream developers about the manual pages.

However, the warnings regarding the libkleetest library are not relevant for this package
as this library is meant to be used by users of KLEE and not by developers.

Users can replay a specific test-case generated by KLEE by linking this support library
directly to the original tested sources, therefore, -devel sub-package is not suitable
in this case.

The same goes for the "shared-lib-calls-exit" warning. Users can control the behaviour
of this library by environment variables and one of them results in a call of exit(3)
if something goes wrong with the replay environment itself.

More onto this topic can be found in this tutorial:
https://klee.github.io/tutorials/testing-function/#replaying-a-test-case

Thanks for pointing it out, though, I'll emphasize this fact more in the spec file.

Comment 4 Miroslav Suchý 2021-07-14 15:44:40 UTC
Ack. Can you please upload edited src.rpm and spec file?

Comment 6 Miroslav Suchý 2021-07-15 13:07:09 UTC
Note that you will have to open BZ later because of that exclude arch. And put that BZ link in spec. See
  https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures
  (you actually raised this yourself during our face2face meeting).

Comment 7 Miroslav Suchý 2021-07-15 13:51:44 UTC
Can you please run licensecheck on the tarball? It seems there are some files under a different license. These exceptions should be then described in the comment. E.g., 

# most files NCSA
# RNG.cpp under BSD
License: NCSA and BSD

This is just an example, not a complete list.

Comment 8 Miroslav Suchý 2021-07-15 13:54:33 UTC
Package requires other packages for directories it uses.
     Note: No known owner of /usr/include/klee

You should own it:

%files
%dir %{_includedir}/klee


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