Bug 1475961 - Review Request: cmrt - C for Media Runtime
Review Request: cmrt - C for Media Runtime
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Zbigniew Jędrzejewski-Szmek
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-27 11:57 EDT by Robert-André Mauchin
Modified: 2017-08-28 12:18 EDT (History)
2 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Robert-André Mauchin 2017-07-27 11:57:41 EDT
SPEC: https://github.com/eclipseo/packaging/blob/cbc4487/cmrt.spec
SRPM: https://kojipkgs.fedoraproject.org//work/tasks/2614/20802614/cmrt-1.0.6-3.fc27.src.rpm
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20802613

Description: Media GPU kernel manager for Intel G45 & HD Graphics family

FAS username: eclipseo

This library is needed to build libva-intel-hybrid-driver (which I'll post in a moment).

I am looking for a sponsorship, please consider me.

Thanks.
Comment 1 Zbigniew Jędrzejewski-Szmek 2017-08-04 12:30:00 EDT
A small cleanup:
https://github.com/01org/cmrt/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gzhttps://github.com/01org/cmrt/archive/%{version}/%{name}-%{version}.tar.gz

%description should end in a dot (those are full sentences). It would be great if you could add a sentence or two what this package does (something like "It allows ... to ... with ...".)

Please don't put multiple Requires/BuildRequires/etc on the same line, especially when there are versions. It's more legible and looks much better in diffs when it's one-per-line.

Hmm, "%autosetup -p1 n %{name}-%{version}" that "n" looks strange. And %{name}-%{version} is the default, so '%autosetup -p1' should suffice.

> find %{buildroot} -regex ".*\.la$" | xargs rm -f --
Just do find '%{buildroot} -name "*.la" -delete'

The -devel package requires you main package, so you don't need to put %license in %files devel again.

--

I liked the reviews you did on other packages. I'll sponsor you once this package is approved.
Comment 2 Zbigniew Jędrzejewski-Szmek 2017-08-04 12:33:18 EDT
Please consider adding your irc nick in FAS.
Comment 3 Robert-André Mauchin 2017-08-05 03:22:54 EDT
Thank you very much for taking the time to review.


I added a more detailed description, separated the BR for -devel, fixed autosetup, used your command for removing *.la and removed %license from the -devel package.

SPEC: https://github.com/eclipseo/packaging/blob/72ff61c/cmrt.spec
SRPM: https://kojipkgs.fedoraproject.org//work/tasks/6721/21066721/cmrt-1.0.6-4.fc27.src.rpm
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=21066720


I added my IRC nick to my FAS account, they're both the same, eclipseo.
Comment 5 Zbigniew Jędrzejewski-Szmek 2017-08-05 11:44:59 EDT
+ package name is OK
+ license is acceptable for Fedora (MIT)
+ license is specified correctly
+ %license macro is used
+ builds and installs OK
+ P/R/BR look correct
+ scriptlets follow the guidelines

The source tarball contains a pre-compiled library: igfxcmjit32.so, igfxcmjit64.so. That library is not present in the resulting binary rpm. Those libs are BSD licensed. So it is not a problem that they are in the src rpm, as long as they are not used.

I'm not sure what the effect of the lack of those precompiled objects is. I hope cmrt is still usable without them. Debian packages is like that, so I assume that it is.

Package is APPROVED.

A note for the future: please link directly to the raw spec file. Otherwise, fedora-review and other tools have trouble. Also, koji scratch builds are not good for the srpm. They are garbage-collected after a few weeks, and the srpm should stay around "forever". Sometimes reviews get stalled, and the original submitter goes away, and as a general policy it's good for another submitter or reviewer to just pick the review up again at any time.
Comment 6 Zbigniew Jędrzejewski-Szmek 2017-08-05 11:51:19 EDT
You have been added to the packagers group. User your powers for good!
If you have any questions about packaging or issues with the process, feel free to ping me (at my bugzilla e-mail, or 'zbyszek' on #fedora-devel).

You should now head over to https://fedoraproject.org/wiki/Infrastructure/WhatHappenedToPkgdb#How_do_I_request_a_new_package_or_a_new_branch. From what I heard yesterday, fedrepo-req is still in updates-testing, errrr, actually only in rawhide. It seems you have to install it from koji! https://apps.fedoraproject.org/packages/fedrepo-req/builds/, just great ;( If you run into issues, #fedora-devel or #fedora-admin are probably the places to ask.
Comment 7 Robert-André Mauchin 2017-08-06 03:36:03 EDT
Thank you for your review and helpful comments.
Comment 8 Fedora Update System 2017-08-15 12:19:25 EDT
cmrt-1.0.6-4.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-e727a51e23
Comment 9 Fedora Update System 2017-08-18 17:52:56 EDT
cmrt-1.0.6-4.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-c8bda49173
Comment 10 Fedora Update System 2017-08-19 14:51:42 EDT
cmrt-1.0.6-4.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-e727a51e23
Comment 11 Fedora Update System 2017-08-27 02:22:41 EDT
cmrt-1.0.6-4.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.
Comment 12 Fedora Update System 2017-08-28 12:18:53 EDT
cmrt-1.0.6-4.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

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