Bug 2404856 - Review Request: xpu-manager - Intel XPU System Management Interface
Summary: Review Request: xpu-manager - Intel XPU System Management Interface
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Simone Caronni
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/intel/xpumanager
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-10-17 22:20 UTC by pavel.androniychuk
Modified: 2025-12-15 20:10 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
negativo17: fedora-review?


Attachments (Terms of Use)

Description pavel.androniychuk 2025-10-17 22:20:14 UTC
Looking to submit a new package into Fedora.
Successfully built in copr
please review

https://copr.fedorainfracloud.org/coprs/androniychuk/xpu-manager/

SPEC Url: https://download.copr.fedorainfracloud.org/results/androniychuk/xpu-manager/fedora-rawhide-x86_64/09701003-xpu-manager/xpu-manager.spec
SRPM Url: https://download.copr.fedorainfracloud.org/results/androniychuk/xpu-manager/fedora-rawhide-x86_64/09701003-xpu-manager/xpu-manager-1.3.3-1.fc44.src.rpm

This package does several things for Intel GPU's
It is responsible for GPU administration, location, topology, telemetry, diagnostics, firmware updating, and GPU configuration. It supports local command-line interface and a local library call interface for integration with third-party solutions.

Reproducible: Always

Comment 1 Fedora Review Service 2025-10-17 22:22:47 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9701073
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2404856-xpu-manager/fedora-rawhide-x86_64/09701073-xpu-manager/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 3 Fedora Review Service 2025-10-20 16:29:47 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9706683
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2404856-xpu-manager/fedora-rawhide-x86_64/09706683-xpu-manager/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 4 pavel.androniychuk 2025-10-20 16:42:29 UTC
The auto build here will fail because there are dependencies that need to be created first before we can make this package via Fedora Review service.

Comment 6 Fedora Review Service 2025-10-24 23:26:38 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9731576
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2404856-xpu-manager/fedora-rawhide-x86_64/09731576-xpu-manager/fedora-review/review.txt

Found issues:

- License file LICENSE is not marked as %license
  Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text

Please know that there can be false-positives.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 7 pavel.androniychuk 2025-10-27 16:23:19 UTC
Flagged issue, seems like a non issue for me.
"""
...
%files -n xpu-smi
%config(noreplace)
%license LICENSE.md
...
"""

Comment 8 pavel.androniychuk 2025-10-27 16:24:21 UTC
@negativo17  This is ready for review.

Comment 9 Package Review 2025-11-27 00:45:25 UTC
This is an automatic action taken by review-stats script.

The ticket reviewer failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we reset the status and the assignee of this ticket.

Comment 10 Simone Caronni 2025-11-27 08:14:14 UTC
Sorry I'm terribly busy at work. Starting now.

Comment 11 Simone Caronni 2025-11-27 08:38:42 UTC
A few items to adjust...

Comment 12 Simone Caronni 2025-11-27 08:42:07 UTC
The package does not generate deubinfo information because the compiler flags set in CMakelists.txt override the default one from Fedora. To avoid patching, please do the following.

Remove these lines:

#Does not properly generate debug info
%define debug_package %{nil}

Set CMAKE_BUILD_TYPE to a random string, for example:

CMAKE_BUILD_TYPE=Fedora

This will make sure the compiler flags (which are already quite aggressive) are properly set for the project, and debug files are properly generated.

Comment 13 Simone Caronni 2025-11-27 08:46:52 UTC
1- The two empty %config(noreplace) lines in the %files section do not make any sense, please remove them.

2- Please use %{_includedir}/*.h in the files section to avoid this:

xpu-manager.spec:92: W: shared-dir-glob-in-files %{_includedir}/*

Comment 14 Simone Caronni 2025-11-27 08:47:14 UTC
(In reply to Simone Caronni from comment #12)
> Set CMAKE_BUILD_TYPE to a random string, for example:
> 
> CMAKE_BUILD_TYPE=Fedora

Sample output:

Wrote: /builddir/build/RPMS/xpu-smi-devel-1.3.3-1.fc43.x86_64.rpm
Wrote: /builddir/build/RPMS/xpu-manager-debugsource-1.3.3-1.fc43.x86_64.rpm
Wrote: /builddir/build/RPMS/xpu-smi-1.3.3-1.fc43.x86_64.rpm
Wrote: /builddir/build/RPMS/xpu-smi-debuginfo-1.3.3-1.fc43.x86_64.rpm

Comment 15 Simone Caronni 2025-11-27 08:53:30 UTC
3 - For each of the third_party subfolders:

$ ls -1d xpumanager-1.3.3/third_party/*
xpumanager-1.3.3/third_party/CLI11
xpumanager-1.3.3/third_party/hwloc
xpumanager-1.3.3/third_party/json
xpumanager-1.3.3/third_party/pcm
xpumanager-1.3.3/third_party/spdlog

You need to see if you can build from packages already in Fedora (unbundling), and if not, you need to declare:

Provides: bundled(<item>) = <version>

If you don't know the version, this is enough:

Provides: bundled(<item>)

While at it, please add after the %autosetup line the following:

rm -fr 3rd_party_src/

Doesn't make any difference, but we are sure there is no accidental bundling.

Comment 16 Simone Caronni 2025-11-27 09:00:26 UTC
4 - OPTIONAL - You can deduplicate the description as shown here: https://src.fedoraproject.org/fork/kenryo/rpms/igsc/c/4699dc88f341b8056a909cd3308f9c46225041f1

5 - Please sort BuildRequires, Requires and Recommends in each section.

Comment 17 Simone Caronni 2025-11-27 09:19:23 UTC
(In reply to Simone Caronni from comment #15)
> You need to see if you can build from packages already in Fedora
> (unbundling), and if not, you need to declare:

I tried with hwloc (which you already list as a BuildRequires):

diff -Naur xpumanager-1.3.3.old/CMakeLists.txt xpumanager-1.3.3/CMakeLists.txt
--- xpumanager-1.3.3.old/CMakeLists.txt 2025-11-27 09:27:17.158704338 +0100
+++ xpumanager-1.3.3/CMakeLists.txt 2025-11-27 10:11:29.921798210 +0100
@@ -93,7 +93,6 @@
   add_subdirectory(third_party/googletest)
 endif()
 add_subdirectory(third_party/spdlog)
-add_subdirectory(third_party/hwloc)
 add_subdirectory(third_party/pcm/pcm-iio-gpu)

 add_subdirectory(core)
@@ -119,11 +118,6 @@
   endif()
 endif()

-add_dependencies(xpum lib_hwloc)
-if(EXISTS ${CMAKE_CURRENT_LIST_DIR}/core/test)
-  add_dependencies(test_xpum_api lib_hwloc)
-endif()
-
 if(NOT DEFINED CPACK_GENERATOR)
   if(${os_name} MATCHES "debian" OR ${os_name} MATCHES "ubuntu" OR ${os_name} MATCHES "Ubuntu")
     set(CPACK_GENERATOR "DEB")

And then changing the prep section as follows:

%prep
%autosetup -n %{upstream_name}-%{version} -p1
rm -fr 3rd_party_src third_party/hwloc

And as you can see works just fine:

$ rpm -qp --requires xpu-smi-1.3.3-1.fc43.x86_64.rpm | grep libhwloc
libhwloc.so.15()(64bit)

Please try to unbundle the other components.

Comment 18 Simone Caronni 2025-11-27 09:50:49 UTC
6 - Please patch/adjust build options so configuration files are not under /usr:

$ rpm -qpl xpu-smi-1.3.3-1.fc43.x86_64.rpm | grep config
/usr/lib/xpu-smi/config
/usr/lib/xpu-smi/config/diagnostics.conf
/usr/lib/xpu-smi/config/pci.conf
/usr/lib/xpu-smi/config/pci.ids
/usr/lib/xpu-smi/config/perf_metrics.conf
/usr/lib/xpu-smi/config/vgpu.conf
/usr/lib/xpu-smi/config/xpum.conf
/usr/lib/xpu-smi/config/xpum.conf.template

And make sure they are properly marked as %config(noreplace) in the %files section if required.

Comment 19 Simone Caronni 2025-11-27 10:36:58 UTC
7 - Just a suggestion; maybe name it intel-xpumanager? You can grep all intel relevant packages with:

dnf list intel\*

Comment 20 pavel.androniychuk 2025-12-01 22:54:47 UTC
The user is not expected to edit source, this is conf files that support the program /usr/bin/xpu-smi.
Like which cards are supported and configurations, etc..

Comment 21 pavel.androniychuk 2025-12-01 23:41:51 UTC
As for naming, this command line utility technically supports cards that are not just Intel.

I would like to leave the "intel-"  name out if possible.

Comment 22 Simone Caronni 2025-12-02 10:28:39 UTC
(In reply to pavel.androniychuk from comment #20)
> The user is not expected to edit source, this is conf files that support the
> program /usr/bin/xpu-smi.
> Like which cards are supported and configurations, etc..

Then leave them where they are and not mark them as config files.

(In reply to pavel.androniychuk from comment #21)
> As for naming, this command line utility technically supports cards that are
> not just Intel.
> 
> I would like to leave the "intel-"  name out if possible.

Up to you, was just an idea.

Please post the updated:

SPEC Url: ...
SRPM Url: ...

Thanks.

Comment 24 Simone Caronni 2025-12-05 13:40:03 UTC
All good, but this point has not been addressed: https://bugzilla.redhat.com/show_bug.cgi?id=2404856#c17

You left in place the hwloc-devel build requirement without using, and you did not unbundle any library or added Provides: bundled(<item>) = <version> where required.

Please unbundle libraries where possible.

Comment 25 pavel.androniychuk 2025-12-15 20:10:15 UTC
The fix for using installed pkgs from the distro and not the third_party sources is trending to land in a new version release that is pending SDL internally at Intel.
No ETA yet.


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