Bug 4540

Summary:	Review Request: nvidia-modprobe - Load the NVIDIA kernel module and create NVIDIA character device files
Product:	Package Reviews	Reporter:	Nicolas Chauvet <kwizart>
Component:	Review Request	Assignee:	Simone Caronni <negativo17>
Status:	RESOLVED FIXED
Severity:	enhancement	CC:	hobbes1069, leigh123linux, negativo17, rpmfusion-package-review
Priority:	P1	Flags:	negativo17: fedora-review+
Version:	Current
Hardware:	x86_64
OS:	GNU/Linux
namespace:

Description Nicolas Chauvet 2017-05-17 16:05:18 CEST

SRPM: http://dl.kwizart.net/review/nvidia-modprobe-381.22-1.fc27.src.rpm
SPEC: http://dl.kwizart.net/review/nvidia-modprobe.spec
Description: Load the NVIDIA kernel module and create NVIDIA character device files

This package is for the nonfree section because it's only relevant along nvidia driver.

Comment 1 Richard 2017-05-17 17:39:07 CEST

Bah, fedora-review is broken... I guess I'll have to do this the hard way.

Comment 2 Simone Caronni 2017-05-18 09:41:50 CEST

Passing DEBUG does not strip the binaries, so the following changes should actually give the same results with slightly shorter commands. NV_VERBOSE during install should not be needed; I don't see any difference; it's just installing 2 directories and 2 files.

%prep
%setup -q
# Remove additional CFLAGS added when enabling DEBUG
sed -i '/+= -O0 -g/d' utils.mk

%build
export CFLAGS="%{optflags}"
export LDFLAGS="%{?__global_ldflags}"
%make_build NV_VERBOSE=1 PREFIX=%{_prefix} DEBUG=1


%install
%make_install PREFIX=%{_prefix}

Comment 3 Nicolas Chauvet 2017-06-01 18:53:31 CEST

(In reply to Simone Caronni from comment #2)
> Passing DEBUG does not strip the binaries, so the following changes should
> actually give the same results with slightly shorter commands. NV_VERBOSE
Yep, but at the cost of using a dynamic patch (sed), which at the end, make it worth (gendiff will fails  to create a ).
Creating a static patch based on this will unlikely be accepted upstream , then will need to be rebase on each releases.

So, thx for sharing your way, but that was indeed on purpose.
Of course one can still fix it better upstream. But that fine enough for me.


Any other issue with this review ?

Comment 4 Simone Caronni 2017-06-02 08:42:30 CEST

(In reply to Nicolas Chauvet from comment #3)
> Any other issue with this review ?

I see no issues.

(In reply to Richard from comment #1)
> Bah, fedora-review is broken... I guess I'll have to do this the hard way.

Richard, will you do it or I can do it?

Comment 5 Richard 2017-06-02 14:15:15 CEST

If you've got time go for it. Thanks!

Comment 6 Simone Caronni 2017-06-08 09:35:10 CEST

Sorry for the late reply, I've been extremely busy.

Comment 7 Simone Caronni 2017-06-08 09:36:42 CEST

I see no issues with the package, rpmlint, licenses, fedora-review, etc.
Package approved.

Comment 8 Nicolas Chauvet 2017-06-08 10:08:48 CEST

Thx for the review

Comment 9 Nicolas Chauvet 2017-07-05 08:13:27 CEST

I've imported nvidia-modprobe. We will make use of it with 384+