Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://download.copr.fedorainfracloud.org/results/stsoe/xrt/srpm-builds/10485434/xrt.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/stsoe/xrt/srpm-builds/10485434/xrt-2.21.75-1.fc42.src.rpm Description: AMD Xilinx Runtime (XRT) provides a runtime environment for AMD Xilinx Alveo FPGAs and AMD Ryzen NPUs. It includes core runtime libraries, Python bindings, development files, and utilities for managing and programming AMD Xilinx devices. Fedora Account System Username: stsoe
Some broad strokes. # SPDX-License-Identifier: Apache-2.0 # Spec layout mirrors src/debian/control and src/debian/rules. # Assisted-by: Generic LLM chatbot Fedora's default license is MIT, Apache is fine. Remove the next two lines, no one will have the context of the debian package. Assisted-by: and any other signoff should be in the git commit log and can be done later. Release: 1%{?dist} consider using the Release: %autorelease and later %changelog %autochangelog Summary: AMD Xilinx FPGA and ACAP runtime (XRT) I usually copy the 'about' section of the github page consider using Summary: Run Time for AIE and FPGA based platforms License: Apache-2.0 AND MIT AND MIT-Khronos-old # License breakdown: # - Core XRT runtime: Apache-2.0 # - AIE binary utilities: MIT # - Core XRT OpenCL library: Apache-2.0 and MIT-Khronos-old Review the fedorareview output file licensecheck. A lot of licenses were not captured in the license: tag The breakdown of the licenses is too brief. There are many license.txt or similar files not captured in the later %license in file If code is not used, remove it in the %prep stage to make the license review easier. Patch102: license.patch remove this patch. The full licenses are fine and any license changes should be done in the upstream not in distro patches. # Debian patches # Fedora patches These are all fedora patches now, these comments are not needed Patch0: 6.18.patch Patch1: 6.19.patch Patches should have a comment on what the patch does and if possible reference the upstream issue or pr this is a general problem. I find using git format patches more helpful as any updated to the package will require rebasing. consider reworking these patches to be git friendly. # Man pages not installed by CMake Source10: aiebu-asm.1 Source11: aiebu-dump.1 How are these man pages generated ? I see from aiebu-asm SEE ALSO The full documentation for aiebu-asm is maintained as a Texinfo manual. If the info and aiebu-asm programs are properly in‐ stalled at your site, the command info aiebu-asm they reference info, which afaik isn't in this package so these man pages will confuse users. ExclusiveArch: aarch64 x86_64 Does this package and all its features work on aarch64 ? hip i know is only x86_64. BuildRequires: cmake >= 3.16 only 1 version of cmake, this is not needed. The reset of the buildrequires looks autogenerated. review what is actually needed and put then in a better order. The packages are a matter of taste. Mine is to limit the packages I think the xrt-utils-* could be collapsed to the main package. Similar for xrt-npu package. For consistency with ROCm the programming interfaces opencl and hip should be xrt-opencl, xrt-opencl-devel and xrt-hip and xrt-hip-devel %cmake \ -DCMAKE_BUILD_TYPE=Release \ Release should change to RelWithDebInfo %check does this really work without hw ? %files ... %{_libdir}/libxrt++.so.* This globbing it too aggressive. At least the major so version needs to be captured. consider something like https://src.fedoraproject.org/rpms/rocclr/blob/rawhide/f/rocclr.spec#_440
The latest COPR build https://copr.fedorainfracloud.org/coprs/stsoe/xrt/build/10650329/ addresses most of the mentioned issues. What I am unsure about is the license breakdown. I removed all files under "Unknown or generated" that ware not needed, the remaining ones are needed and covered by what I listed under the breakdown. I have made upstream changes to fix some of the missing or incorrect licenses also. I did not combine or rename any packages, as mentioned it is a matter of choice and current packages reflect what we are doing for Debian. If a must, then of course this can be changed. However, I do not feel that utils* can be collapsed to main package; the main package is all that needed to for run time. The %check runs user space tests, there are no HW tests. Please advise what further changes are required. Thank you for your feedback.
(In reply to Soren Soe from comment #2) > The latest COPR build > https://copr.fedorainfracloud.org/coprs/stsoe/xrt/build/10650329/ addresses > most of the mentioned issues. > > What I am unsure about is the license breakdown. I removed all files under > "Unknown or generated" that ware not needed, the remaining ones are needed > and covered by what I listed under the breakdown. I have made upstream > changes to fix some of the missing or incorrect licenses also. > > I did not combine or rename any packages, as mentioned it is a matter of > choice and current packages reflect what we are doing for Debian. If a > must, then of course this can be changed. However, I do not feel that > utils* can be collapsed to main package; the main package is all that needed > to for run time. > > The %check runs user space tests, there are no HW tests. > > Please advise what further changes are required. Thank you for your > feedback. Forgot to mention that the man pages are copied from Debian. The reference to info is the Linux info command. I can remove the man pages if necessary.
Please add the updated spec and srpm this bz. copr builds are temporary, use a url that will be around for a while. Use Spec/SRPM URL: <url to> To be useful, this package should do something. I likely have the hw needed to run this package, could you recommend something maybe using the lemonade server ? here is what i get on strix halo, is aie2p supported by xrt runtime ? ******* Agent 3 ******* Name: aie2p Uuid: AIE-XX Marketing Name: RyzenAI-npu5 Vendor Name: AMD Feature: AGENT_DISPATCH Profile: BASE_PROFILE Float Round Mode: NEAR Max Queue Number: 1(0x1) Queue Min Size: 64(0x40) Queue Max Size: 64(0x40) Queue Type: SINGLE Node: 0 Device Type: DSP Cache Info: L2: 2048(0x800) KB L3: 32768(0x8000) KB Chip ID: 0(0x0) ASIC Revision: 0(0x0) Cacheline Size: 0(0x0) Max Clock Freq. (MHz): 0 BDFID: 0 Internal Node ID: 0 Compute Unit: 0 SIMDs per CU: 0 Shader Engines: 0 Shader Arrs. per Eng.: 0 WatchPts on Addr. Ranges:0 Memory Properties: Features: AGENT_DISPATCH Pool Info: Pool 1 Segment: GLOBAL; FLAGS: KERNARG, COARSE GRAINED Size: 131151216(0x7d13570) KB Allocatable: TRUE Alloc Granule: 4KB Alloc Recommended Granule:4KB Alloc Alignment: 4KB Accessible by all: TRUE Pool 2 Segment: GLOBAL; FLAGS: COARSE GRAINED Size: 65536(0x10000) KB Allocatable: TRUE Alloc Granule: 4KB Alloc Recommended Granule:0KB Alloc Alignment: 4KB Accessible by all: TRUE Pool 3 Segment: GLOBAL; FLAGS: COARSE GRAINED Size: 131151216(0x7d13570) KB Allocatable: TRUE Alloc Granule: 4KB Alloc Recommended Granule:4KB Alloc Alignment: 4KB Accessible by all: TRUE ISA Info: *** Done ***