Bug 1473320 - Review Request: vkmark - Vulkan benchmarking suite
Review Request: vkmark - Vulkan benchmarking suite
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: c72578
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2017-07-20 09:16 EDT by Yanko Kaneti
Modified: 2017-07-27 10:32 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2017-07-27 03:02:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
c72578: fedora‑review+

Attachments (Terms of Use)

  None (edit)
Description Yanko Kaneti 2017-07-20 09:16:39 EDT
Spec URL: http://declera.com/~yaneti/vkmark/vkmark.spec
SRPM URL: http://declera.com/~yaneti/vkmark/vkmark-2017.07-0.1.gitfd9f927.fc27.src.rpm
Description: vkmark is an extensible extensible Vulkan benchmarking suite with targeted, configurable scenes
Fedora Account System Username: yaneti
Comment 1 c72578 2017-07-25 09:35:45 EDT
YYYYMMDD is missing in the Release: tag
e.g. for fd9f927:
Release:        0.1.20170720git%{shortcommit0}%{?dist}
Comment 2 Yanko Kaneti 2017-07-25 10:49:51 EDT

- Add date to the revision, as per review (#1473320)

Comment 3 c72578 2017-07-25 11:14:22 EDT
Wed Jul 25 2017 -> Tue Jul 25 2017
Comment 4 Yanko Kaneti 2017-07-25 11:19:18 EDT
(In reply to c72578 from comment #3)
> Wed Jul 25 2017 -> Tue Jul 25 2017

Thanks. Fixed in place
Comment 5 c72578 2017-07-26 03:44:57 EDT
vkmark includes tests. Could you please include them in the %check section of the spec file.

Tests are here after building:
Comment 6 Yanko Kaneti 2017-07-26 03:57:14 EDT
- Add %%check, running the meson tests, as per review (#1473320)

Comment 7 c72578 2017-07-26 04:02:33 EDT
Taking this review.
Comment 8 c72578 2017-07-26 05:27:29 EDT
mesa-vulkan-devel is only available for x86_64.
See results of Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20753589

This means, that other architectures have to be excluded using ExcludeArch.
See "Architecture Build Failures" https://fedoraproject.org/wiki/Packaging:Guidelines on how to handle this.
Comment 9 Yanko Kaneti 2017-07-26 05:59:20 EDT
- Vulkan backend only available on x86_64, condition the mesa-vulkan-devel

SPEC: http://declera.com/~yaneti/vkmark/vkmark.spec
SRPM: http://declera.com/~yaneti/vkmark/vkmark-2017.07-0.4.20170725gitaa0de26.fc27.src.rpm

I am not sure if it has a purpose on vulkan-less architectures but at least it builds.

With the exception of s390x which maybe someone could investigate more

Comment 10 Yanko Kaneti 2017-07-26 06:12:57 EDT
- ExcludeArch s390x for now

SPEC: http://declera.com/~yaneti/vkmark/vkmark.spec
SRPM: http://declera.com/~yaneti/vkmark/vkmark-2017.07-0.5.20170725gitaa0de26.fc27.src.rpm

As the comment says
+# Tests fail on s390x
+# Since s390x has no vulkan anyway exclude for now
+ExcludeArch:    s390x
Comment 11 c72578 2017-07-26 06:23:37 EDT
Thanks for the update.

It is a few tests, which are failing on s390x
test cases:  14 |  13 passed | 1 failed
assertions: 176 | 173 passed | 3 failed

Further investigations concerning s390x make sense. Maybe together with upstream. Alternatively, (some failing) tests could be excluded for now for s390x.

vulkan is available for s390x
See e.g.: https://kojipkgs.fedoraproject.org//work/tasks/5948/20755948/root.log
Comment 12 Yanko Kaneti 2017-07-26 06:34:49 EDT
Right, should have probably said "mesa has no vulkan drivers on s390x"

I am not seeing myself investigating the s390x test failures anytime soon. So its either ExcludeArch, or someone could peruse the current spec and push this further in a different review ticket.

Thanks for your efforts so far.
Comment 13 c72578 2017-07-26 08:49:50 EDT
Package Review

[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed

===== MUST items =====

[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "LGPL (v2.1 or later)", "GPL (v3 or later)", "Unknown or
     generated". 35 files have unknown license. Detailed output of
     licensecheck in /home/makerpm/fedora-
[x]: License file installed when any subpackage combination is installed.
[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[!]: Package is not known to require an ExcludeArch tag.
   # ExcludeArch: s390x. Some tests fail on s390x. Comments are in spec file
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in vkmark-
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.

Checking: vkmark-2017.07-0.5.20170725gitaa0de26.fc27.x86_64.rpm
vkmark.x86_64: W: spelling-error Summary(en_US) Vulkan -> Vulcan
vkmark.x86_64: W: spelling-error Summary(en_US) benchmarking -> bench marking, bench-marking, benchmark
vkmark.x86_64: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
vkmark.x86_64: W: hidden-file-or-dir /usr/lib/.build-id
vkmark.x86_64: W: hidden-file-or-dir /usr/lib/.build-id
vkmark-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
# build-id related error reported by rpmlint. Currently these errors/warnings only appear in fedora-rawhide-x86_64, not fedora-26-x86_64
vkmark.src: W: spelling-error Summary(en_US) Vulkan -> Vulcan
vkmark.src: W: spelling-error Summary(en_US) benchmarking -> bench marking, bench-marking, benchmark
vkmark.src: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
3 packages and 0 specfiles checked; 1 errors, 8 warnings.

Rpmlint (debuginfo)
Checking: vkmark-debuginfo-2017.07-0.5.20170725gitaa0de26.fc27.x86_64.rpm
vkmark-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
1 packages and 0 specfiles checked; 1 errors, 0 warnings.
# build-id related error reported by rpmlint

Rpmlint (installed packages)
sh: /usr/bin/python: No such file or directory
vkmark-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
vkmark.x86_64: W: spelling-error Summary(en_US) Vulkan -> Vulcan
vkmark.x86_64: W: spelling-error Summary(en_US) benchmarking -> bench marking, bench-marking, benchmark
vkmark.x86_64: W: spelling-error %description -l en_US benchmarking -> bench marking, bench-marking, benchmark
2 packages and 0 specfiles checked; 1 errors, 3 warnings.
# build-id related error reported by rpmlint

vkmark-debuginfo (rpmlib, GLIBC filtered):

vkmark (rpmlib, GLIBC filtered):



Unversioned so-files
vkmark: /usr/lib64/vkmark/kms.so
vkmark: /usr/lib64/vkmark/wayland.so
vkmark: /usr/lib64/vkmark/xcb.so

Source checksums
https://github.com/vkmark/vkmark/archive/aa0de26fff41524489b06e32151ac605081c774b.tar.gz#/vkmark-2017.07-aa0de26.tar.gz :
  CHECKSUM(SHA256) this package     : 771368de499d269f8d24e5e0c32124e16a432fafd45493b546e3706812880847
  CHECKSUM(SHA256) upstream package : 771368de499d269f8d24e5e0c32124e16a432fafd45493b546e3706812880847

Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1473320 -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Comment 14 c72578 2017-07-26 08:50:31 EDT
Package looks good.

Comment 15 Gwyn Ciesla 2017-07-26 10:11:12 EDT
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/vkmark
Comment 16 Yanko Kaneti 2017-07-27 03:02:02 EDT
Built in rawhide.
Filed a bug about the s390x ExcludeArch test issue (#1475561)

Will consider adding this to f26 when/if there is some sort of release maybe...
Or co=maintainers of f26 and older welcome.

Comment 17 c72578 2017-07-27 10:14:31 EDT
Dear Yanko,
I would contribute as a co-maintainer of f26 and older, if it is OK from your side.

- Concerning the versioning and releases I got the following info from Alexandros Frantzis:

This YYYY.MM versioning scheme will continue for vkmark, although at
this time this scheme is meant more as a representation of the
approximate date of the code rather than an exact version. I am
considering at some point each month (probably near the end) to update
the version and to tag a commit as a somewhat official release for that
month, assuming there have been changes in that month.

Since 2017.07 is the first one, I set the version a bit early (and no
tag), but from August the scheme should make more sense: a 2017.08
release near the end of August, a 2017.09 end of September if there are
changes etc.
Comment 18 Yanko Kaneti 2017-07-27 10:23:57 EDT
(In reply to c72578 from comment #17)
> Dear Yanko,
> I would contribute as a co-maintainer of f26 and older, if it is OK from
> your side.

Thanks for volunteering. I've added f26 and f25 and assinged you as hocho there.
Cheers ;)
Comment 19 Yanko Kaneti 2017-07-27 10:32:31 EDT
ugh I mean honcho as in Package administrator, not hocho :)
Please login in pkgdb and sort out your permissions for vkmark if I've missed something.

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