https://bugzilla.redhat.com/show_bug.cgi?id=2383635

[email protected] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?([email protected])  |
            Summary|Review Request:             |Review Request:
                   |rocm_bandwidth_test -       |rocm-bandwidth-test -
                   |Bandwidth test for ROCm     |Bandwidth test for ROCm



--- Comment #4 from [email protected] ---
Spec URL: https://trix.fedorapeople.org/rocm-bandwidth-test.spec
SRPM URL:
https://trix.fedorapeople.org/rocm-bandwidth-test-6.4.2-1.fc43.src.rpm

- It seems like most of the code is under the NCSA license.
  Thanks for filing an upstream bug.
  Since this package includes a LICENSE file, please also extract the NCSA
  license into a file and install it alongside the existing one.

# From base_test.cpp
Source1:    LICENSE.NSCA.txt
%license LICENSE.txt LICENSE.NSCA.txt

- I'm waiting for the devel chat on Matrix to clarify if we can use underscores
  in the package name; the "naturally" exception to the guidelines is
confusing.
  https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_separators

Note the name of specfile and srpm changed.
Name:       rocm-bandwidth-test

- %autorelease and/or %autochangelog are strongly recommended by the
guidelines.

Fedora shares its ROCm packages with OpenSUSE. OpenSUSE does not have
%autorelease or %autochangelog, so we are using the old way.

- The utility has no manual page; I'm not particularly bothered by that but
  it might be worth an upstream bug or generating one with help2man.
  Alternatively,
- Do you think it would make sense to ship the included documentation (sphinx
  or PDF) just to give a little more guidance on how to use the program?

There is a pdf in the tarball, so I used it.
%doc README.md ROCmBandwithTest_UserGuide.pdf

- Given that ROCm is platform-specific, I don't see a way to do a nontrivial
  %check section testing this package.
  Would you mind putting a comment where the %check section would go,
  for future reference?

With rpmbuild --with check and a local system with a GPU, this should work.
I tested it on my rx 9070.

%check
%if %{with check}
%{_vpath_builddir}/rocm-bandwidth-test
# On a gfx1201, the start should look something like
#          RocmBandwidthTest Version: 2.6.0
#
#          Launch Command is: redhat-linux-build/rocm-bandwidth-test
(rocm_bandwidth -a + rocm_bandwidth -A)
#
#
#          Device: 0,  Intel(R) Core(TM) i5-10400 CPU @ 2.90GHz
#          Device: 1,  AMD Radeon Graphics,  GPU-7b2a57bc7a036a5f,  03:0.0
# ...
%endif

Thanks for the review.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2383635

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202383635%23c4

-- 
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]
Do not reply to spam, report it: 
https://pagure.io/fedora-infrastructure/new_issue

Reply via email to