Bug 4928

# install AppData and add modalias provides
mkdir -p %{buildroot}%{_datadir}/appdata/
install -pm 0644 %{SOURCE12} %{buildroot}%{_datadir}/appdata/

/usr/share/appdata is deprecated in favour of /usr/share/metainfo

See: https://www.freedesktop.org/software/appstream/docs/sect-Metadata-Application.html
and: https://www.freedesktop.org/software/appstream/docs/chap-Metadata.html



install -p -m 0644 %{SOURCE21} %{buildroot}%{_unitdir}

Shouldn't you run the systemd scriptlets for this service file?

%post
%systemd_post nvidia-fallback.service

%preun
%systemd_preun nvidia-fallback.service

%postun
%systemd_postun_with_restart nvidia-fallback.service


 - xorg-x11-drv-nvidia-390xx.src:16: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 16)

Line 16, 19, 33, 34: replace the tabs with spaces

 - Would be nice to escape all the macros in comment



Package Review
==============

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


Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines
- systemd_post is invoked in %post, systemd_preun in %preun, and
  systemd_postun in %postun for Systemd service files.
  Note: Systemd service file(s) in xorg-x11-drv-nvidia-390xx
  See: https://fedoraproject.org/wiki/Packaging:Scriptlets#Scriptlets
- Development (unversioned) .so files in -devel subpackage, if present.
  Note: Unversioned so-files directly in %_libdir.
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages
- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 1699840 bytes in 53 files.
  See:
  http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation


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

C/C++:
[-]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[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.

Generic:
[-]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL", "MIT/X11 (BSD like)", "SGI Free Software License B
     (v1.1)", "Unknown or generated", "BSD (3 clause)". 257 files have
     unknown license. Detailed output of licensecheck in
     /home/bob/packaging/review/xorg-x11-drv-nvidia-390xx/review-xorg-x11
     -drv-nvidia-390xx/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[-]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib/udev,
     /usr/share/glvnd/egl_vendor.d, /usr/share/glvnd, /usr/lib/modprobe.d,
     /usr/lib/udev/rules.d, /usr/lib/dracut/dracut.conf.d, /usr/lib/dracut
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /etc/nvidia(xorg-x11-drv-
     nvidia), /usr/lib/nvidia(xorg-x11-drv-nvidia-libs, xorg-x11-drv-
     nvidia), /usr/include/nvidia(xorg-x11-drv-nvidia-devel),
     /usr/include/nvidia/GL(xorg-x11-drv-nvidia-devel),
     /usr/share/nvidia(xorg-x11-drv-nvidia)
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[-]: Sources contain only permissible code or content.
[x]: %config files are marked noreplace or the reason is justified.
     Note: No (noreplace) in %config /etc/OpenCL/vendors/nvidia.icd
[-]: Package contains desktop file if it is a GUI application.
[x]: 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
     names).
[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.
[x]: Package contains systemd file(s) if in need.
[-]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[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 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
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[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
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

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

Generic:
[-]: 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).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in xorg-x11
     -drv-nvidia-390xx-devel , xorg-x11-drv-nvidia-390xx-cuda , xorg-x11
     -drv-nvidia-390xx-cuda-libs , xorg-x11-drv-nvidia-390xx-kmodsrc ,
     xorg-x11-drv-nvidia-390xx-libs
[?]: 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.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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
     $RPM_BUILD_ROOT)
[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]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[-]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
[-]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 10291200 bytes in /usr/share
     See:
     http://fedoraproject.org/wiki/Packaging:ReviewGuidelines#Package_Review_Guidelines
[x]: Spec file according to URL is the same as in SRPM.



Rpmlint
-------
Checking: xorg-x11-drv-nvidia-390xx-390.67-1.fc29.x86_64.rpm
          xorg-x11-drv-nvidia-390xx-devel-390.67-1.fc29.x86_64.rpm
          xorg-x11-drv-nvidia-390xx-cuda-390.67-1.fc29.x86_64.rpm
          xorg-x11-drv-nvidia-390xx-cuda-libs-390.67-1.fc29.x86_64.rpm
          xorg-x11-drv-nvidia-390xx-kmodsrc-390.67-1.fc29.x86_64.rpm
          xorg-x11-drv-nvidia-390xx-libs-390.67-1.fc29.x86_64.rpm
          xorg-x11-drv-nvidia-390xx-390.67-1.fc29.src.rpm
xorg-x11-drv-nvidia-390xx.x86_64: W: spelling-error %description -l en_US chipsets -> chip sets, chip-sets, Chiclets
xorg-x11-drv-nvidia-390xx.x86_64: W: obsolete-not-provided nvidia-390xx-kmod
xorg-x11-drv-nvidia-390xx.x86_64: W: devel-file-in-non-devel-package /usr/lib64/nvidia-390xx/xorg/libglx.so
xorg-x11-drv-nvidia-390xx.x86_64: W: no-manual-page-for-binary nvidia-bug-report.sh
xorg-x11-drv-nvidia-390xx.x86_64: W: dangerous-command-in-%pre rm
xorg-x11-drv-nvidia-390xx.x86_64: W: dangerous-command-in-%preun mv
xorg-x11-drv-nvidia-390xx-devel.x86_64: E: description-line-too-long C This package provides the development files of the xorg-x11-drv-nvidia-390xx package,
xorg-x11-drv-nvidia-390xx-devel.x86_64: W: no-documentation
xorg-x11-drv-nvidia-390xx-cuda.x86_64: W: only-non-binary-in-usr-lib
xorg-x11-drv-nvidia-390xx-cuda.x86_64: W: conffile-without-noreplace-flag /etc/OpenCL/vendors/nvidia.icd
xorg-x11-drv-nvidia-390xx-cuda.x86_64: W: no-manual-page-for-binary nvidia-cuda-mps-server
xorg-x11-drv-nvidia-390xx-cuda.x86_64: W: no-manual-page-for-binary nvidia-debugdump
xorg-x11-drv-nvidia-390xx-cuda-libs.x86_64: W: no-documentation
xorg-x11-drv-nvidia-390xx-cuda-libs.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libcuda.so
xorg-x11-drv-nvidia-390xx-kmodsrc.x86_64: W: summary-not-capitalized C xorg-x11-drv-nvidia-390xx kernel module source code
xorg-x11-drv-nvidia-390xx-kmodsrc.x86_64: W: spelling-error %description -l en_US kmod -> mod, k mod, mood
xorg-x11-drv-nvidia-390xx-kmodsrc.x86_64: E: description-line-too-long C Source tree used for building kernel module packages (xorg-x11-drv-nvidia-390xx-kmod)
xorg-x11-drv-nvidia-390xx-kmodsrc.x86_64: W: no-documentation
xorg-x11-drv-nvidia-390xx-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libGLX_nvidia.so.390.67 exit@GLIBC_2.2.5
xorg-x11-drv-nvidia-390xx-libs.x86_64: W: no-documentation
xorg-x11-drv-nvidia-390xx.src: W: spelling-error %description -l en_US chipsets -> chip sets, chip-sets, Chiclets
xorg-x11-drv-nvidia-390xx.src: W: strange-permission parse-readme.py 755
xorg-x11-drv-nvidia-390xx.src:7: E: hardcoded-library-path in %{_prefix}/lib/nvidia
xorg-x11-drv-nvidia-390xx.src:19: E: hardcoded-library-path in %{_prefix}/lib/dracut/dracut.conf.d
xorg-x11-drv-nvidia-390xx.src:20: E: hardcoded-library-path in %{_prefix}/lib/modprobe.d/
xorg-x11-drv-nvidia-390xx.src:86: W: macro-in-comment %{version}
xorg-x11-drv-nvidia-390xx.src:88: W: macro-in-comment %{version}
xorg-x11-drv-nvidia-390xx.src:90: W: macro-in-comment %{version}
xorg-x11-drv-nvidia-390xx.src:147: W: macro-in-comment %{version}
xorg-x11-drv-nvidia-390xx.src:153: W: macro-in-comment %{version}
xorg-x11-drv-nvidia-390xx.src:434: W: deprecated-grep ['fgrep']
xorg-x11-drv-nvidia-390xx.src:16: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 16)
xorg-x11-drv-nvidia-390xx.src: E: specfile-error warning: Macro expanded in comment on line 86: %{?_isa} = %{version}
xorg-x11-drv-nvidia-390xx.src: E: specfile-error 
xorg-x11-drv-nvidia-390xx.src: E: specfile-error warning: Macro expanded in comment on line 87: %{?fedora}
xorg-x11-drv-nvidia-390xx.src: E: specfile-error 
xorg-x11-drv-nvidia-390xx.src: E: specfile-error warning: Macro expanded in comment on line 88: %{?_isa} = %{version}
xorg-x11-drv-nvidia-390xx.src: E: specfile-error 
xorg-x11-drv-nvidia-390xx.src: E: specfile-error warning: Macro expanded in comment on line 90: %{?_isa} = %{version}
xorg-x11-drv-nvidia-390xx.src: E: specfile-error 
xorg-x11-drv-nvidia-390xx.src: E: specfile-error warning: Macro expanded in comment on line 147: %{?_isa} = %{version}
xorg-x11-drv-nvidia-390xx.src: E: specfile-error 
xorg-x11-drv-nvidia-390xx.src: E: specfile-error warning: Macro expanded in comment on line 153: %{?_isa} = %{version}
7 packages and 0 specfiles checked; 16 errors, 27 warnings.

(In reply to Nicolas Chauvet from comment #0)
 
> TODO:
> Restore either builtin tools (nvidia-xconfig/nvidia-settings/others?)
> At this step we can assume the same can be used from the 396+ serie.

Maybe consider stripping vulkan support as this driver is intended for Fermi cards.
People that want vulkan support can use nvidia main.

P.S vulkan-filesystem has been replaced by vulkan-loader package.

(In reply to Robert-André Mauchin from comment #1)

The service is started by a conditional, missing nvidia.ko triggers it

https://pkgs.rpmfusion.org/cgit/nonfree/xorg-x11-drv-nvidia.git/tree/nvidia-fallback.service#n6
 
> Shouldn't you run the systemd scriptlets for this service file?
> 
> %post
> %systemd_post nvidia-fallback.service
> 
> %preun
> %systemd_preun nvidia-fallback.service
> 
> %postun
> %systemd_postun_with_restart nvidia-fallback.service

Is there anything else that needs fixing so it passes review?

This missing tar file error from

https://bugzilla.rpmfusion.org/show_bug.cgi?id=4929

(In reply to Robert-André Mauchin from comment #1)
> + tar --use-compress-program xz -xf
> /usr/share/nvidia-390xx-kmod-390.67/nvidia-390xx-kmod-390.67-x86_64.tar.xz
> BUILDSTDERR: tar (child):
> /usr/share/nvidia-390xx-kmod-390.67/nvidia-390xx-kmod-390.67-x86_64.tar.xz:
> Cannot open: No such file or directory
> BUILDSTDERR: tar (child): Error is not recoverable: exiting now
> BUILDSTDERR: tar: Child returned status 2
> BUILDSTDERR: tar: Error is not recoverable: exiting now
> 
> The name of the archive starts with nvidia-kmod, not nvidia-390xx-kmod.
> Use:
> 
> tar --use-compress-program xz -xf
> %{_datadir}/nvidia-kmod-%{version}/nvidia-kmod-%{version}-%{_target_cpu}.tar.
> xz



is caused by this review, see

#Install the nvidia kernel modules sources archive
mkdir -p %{buildroot}%{_datadir}/nvidia-kmod-%{version}
tar Jcf %{buildroot}%{_datadir}/nvidia-kmod-%{version}/nvidia-kmod-%{version}-%{_target_cpu}.tar.xz kernel

Hi no other issue besides the appdata → metainfo recommendation.

I can approve it since it's minor.

(In reply to leigh scott from comment #4)
> This missing tar file error from
> 
> https://bugzilla.rpmfusion.org/show_bug.cgi?id=4929
> 
> (In reply to Robert-André Mauchin from comment #1)
> > + tar --use-compress-program xz -xf
> > /usr/share/nvidia-390xx-kmod-390.67/nvidia-390xx-kmod-390.67-x86_64.tar.xz
> > BUILDSTDERR: tar (child):
> > /usr/share/nvidia-390xx-kmod-390.67/nvidia-390xx-kmod-390.67-x86_64.tar.xz:
> > Cannot open: No such file or directory
> > BUILDSTDERR: tar (child): Error is not recoverable: exiting now
> > BUILDSTDERR: tar: Child returned status 2
> > BUILDSTDERR: tar: Error is not recoverable: exiting now
> > 
> > The name of the archive starts with nvidia-kmod, not nvidia-390xx-kmod.
> > Use:
> > 
> > tar --use-compress-program xz -xf
> > %{_datadir}/nvidia-kmod-%{version}/nvidia-kmod-%{version}-%{_target_cpu}.tar.
> > xz
> 
> 
> 
> is caused by this review, see
> 
> #Install the nvidia kernel modules sources archive
> mkdir -p %{buildroot}%{_datadir}/nvidia-kmod-%{version}
> tar Jcf
> %{buildroot}%{_datadir}/nvidia-kmod-%{version}/nvidia-kmod-%{version}-
> %{_target_cpu}.tar.xz kernel

Yes either fix it here or in the other package.

If any volunteer with the hardware can help to maintain the 390xx serie, please rise your hand.
I don't plan to maintain the 390xx serie myslef.

Thx for the review Robert, any future maintainer should take your remark into account.

Nicolas, I could probably help to maintain 390xx as I have GPU requiring this driver version. Please provide more info what do you need from me.

@Vaclav,
Thx for your offer. You just need to create another review with the previous remarks from Robert-André Fixed to finish the review.

Once done you will have to maintain the driver in RPM Fusion. This means:
- Updating and fixing the driver if any issue are reported in our bugzilla.
- Verify that the kmod still build on recent Fedora
- Sync packaging features from the main driver if relevant. (such as the nvidia-fallback).

There is a "Nice to have" todo list:
- Make the driver parallele installable with the main serie (Ubuntu did something like this IIRC)
- Build nvidia-settings and others tools from source and have them parallel installable (they can be bundled in the main package the first step).
- Maintain the driver in EL6/EL7 branches.

Hopefully there will be others maintainer to help to co-maintain.

Still no volunteer to maintain the 390xx driver in RPM Fusion ?

I would really like to see this supported but I don't think I can keep up with the frequency of kernel releases.

(In reply to Richard from comment #10)
> I would really like to see this supported but I don't think I can keep up
> with the frequency of kernel releases.

It isn't that much work keeping up with the kernel, see

https://pkgs.rpmfusion.org/cgit/nonfree/nvidia-kmod.git/log/?qt=grep&q=Patch+for

It's a lot easier than it used to be now it's akmod only.


P.S I could assist with kernel patching.

If most of the work is kernel patching I'm definitely not qualified :) I'm willing to co-maintain though.

(In reply to Richard from comment #12)
> If most of the work is kernel patching I'm definitely not qualified :) I'm
> willing to co-maintain though.

Thx for your help in this area.
We really need to have someone using a NVIDIA Fermi GPU to use and maintain this serie.
Please have a look on the reviewer comments, you need to create another review and update to 390.87 which is the latest of the serie.

I have a GTS 450 which per google is a Fermi2...

Yeah, sorry. I would help out, but my GT 710 is (barely) a Kepler card.

BTW, if anyone needs to know where they stand driver-wise, Fermi cards have been left behind as of April 2018. (That's when future support for new-feature updates — starting from the 400.xx driver release — was limited to Kepler, Maxwell, and Pascal series GPUs.) Fermi will be getting security updates only through January 2019, at which point they're orphaned completely. 

Nvidia's official list of the affected cards can be found here:

https://nvidia.custhelp.com/app/answers/detail/a_id/4656


And the (incredibly brief) Support Plan announcement regarding that change, which honestly contains nothing I didn't cover in this comment:

https://nvidia.custhelp.com/app/answers/detail/a_id/4654

(In reply to FeRD (Frank Dana) from comment #16)
...
> updates only through January 2019, at which point they're orphaned
> completely. 
The date was updated to 2022

https://devtalk.nvidia.com/default/topic/1032650/linux/unix-graphics-feature-deprecation-schedule/

It means that the 390xx serie can still be relevant from el6 to any recent fedora.
As a side note, el7.6 plans to enable libglvnd support, so I expect to keep a version that will work with both glvnd and non-glvnd until rhel 7.6 launched.
By RHEL 7.6 GA, we might clean-up support for non-glvnd in the 390xx serie and keep el6 as a fork.

@Richard
Any issue with that ?

(In reply to Nicolas Chauvet from comment #17)
> It means that the 390xx serie can still be relevant from el6 to any recent
> fedora.
> As a side note, el7.6 plans to enable libglvnd support, so I expect to keep
> a version that will work with both glvnd and non-glvnd until rhel 7.6
> launched.
> By RHEL 7.6 GA, we might clean-up support for non-glvnd in the 390xx serie
> and keep el6 as a fork.
> 
> @Richard
> Any issue with that ?

Sound good to me. If leigh is good with helping with the kernel patches when needed I'll try to work on the review this weekend.

(In reply to FeRD (Frank Dana) from comment #15)
> Yeah, sorry. I would help out, but my GT 710 is (barely) a Kepler card.

Sounds perfect to me for 390xx support

https://download.nvidia.com/XFree86/Linux-x86_64/390.87/README/supportedchips.html

(In reply to Richard from comment #18)
> (In reply to Nicolas Chauvet from comment #17)
> > It means that the 390xx serie can still be relevant from el6 to any recent
> > fedora.
> > As a side note, el7.6 plans to enable libglvnd support, so I expect to keep
> > a version that will work with both glvnd and non-glvnd until rhel 7.6
> > launched.
> > By RHEL 7.6 GA, we might clean-up support for non-glvnd in the 390xx serie
> > and keep el6 as a fork.
> > 
> > @Richard
> > Any issue with that ?
> 
> Sound good to me. If leigh is good with helping with the kernel patches when
> needed I'll try to work on the review this weekend.

I'm good with doing the patching.

My old notebook have a 425m GPU which needs 390.xxx driver.
If i get latest 390.87 successful running for testing hybrid graphic, i am happy to help out as co-maintainer for testing, etc.

(In reply to leigh scott from comment #19)
> (In reply to FeRD (Frank Dana) from comment #15)
> > Yeah, sorry. I would help out, but my GT 710 is (barely) a Kepler card.
> 
> Sounds perfect to me for 390xx support
> 
> https://download.nvidia.com/XFree86/Linux-x86_64/390.87/README/
> supportedchips.html

Maybe I misunderstood kwizart's comment about Fermi. My Kepler will definitely WORK with 390.87, but because it's also compatible with the newer 396.54, that's what I'm currently running day-to-day, and I plan to move to the 400-series drivers (looks like 410.57 is the current latest?) when we have them packaged.

I figured we were looking for someone with a Fermi chip to maintain, because they'd also be using the 390xx series, not any later releases.

Closing in favor of:

https://bugzilla.rpmfusion.org/show_bug.cgi?id=5030

That has been updated to 390.87.