Bug 2335652

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8472432
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2335652-rocm-omp/fedora-rawhide-x86_64/08472432-rocm-omp/fedora-review/review.txt

Please take a look if any issues were found.


---
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.

As a good practice, you should clean up the source you don't need in %prep if you don't need most of this.
It makes it more clear what subset you're building, and it makes the source licensing clearer.

Furthermore, you should add a rocm-llvm-filesystem package that owns the following, and have this package require it:
/usr/lib64/rocm/llvm/lib/clang
/usr/lib64/rocm/llvm/lib/cmake
/usr/lib64/rocm/llvm/lib/clang/18/include
/usr/lib64/rocm/llvm/include
/usr/lib64/rocm/llvm/lib
/usr/lib64/rocm/llvm/lib/clang/18
/usr/lib64/rocm/llvm
/usr/lib64/rocm/llvm/bin

If a user installs just rocm-omp, then uninstalls the package, it will orphan those directories. Having the filesystem package allows them to still have ownership, and dnf will auto clean up the leaf package.

I noticed it complains of:
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in rocm-
     omp-static

But static actually requires devel which requires the base package. Anyway, false positive.

Also you used defines here:
     Note: %define requiring justification: %define __sourcedir openmp,
     %define _vpath_srcdir openmp
Not sure of the reason, but you need to comment it.

Finally, not sure what this rpmlint output is about:
rocm-omp-static.x86_64: E: static-library-without-symtab /usr/lib64/rocm/llvm/lib/libomptarget.devicertl.a
rocm-omp-static.x86_64: E: static-library-without-debuginfo /usr/lib64/rocm/llvm/lib/libomptarget.devicertl.a
rocm-omp-static.x86_64: E: lto-no-text-in-archive /usr/lib64/rocm/llvm/lib/libomptarget.devicertl.a
rocm-omp-devel.x86_64: E: invalid-soname /usr/lib64/rocm/llvm/lib/libomp.so libomp.so

Spec URL: https://trix.fedorapeople.org/rocm-omp.spec
SRPM URL: https://trix.fedorapeople.org/rocm-omp-6.3.1-1.fc42.src.rpm

[ ]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib64/rocm/llvm,
     /usr/lib64/rocm, /usr/lib64/rocm/llvm/lib

rocm-compilersupport / rocm-rpm-macros are being reworked so the ownership of the %libdir/rocm belongs to rocm-compilersupport, here

https://src.fedoraproject.org/rpms/rocm-compilersupport/blob/rawhide/f/rocm-compilersupport.spec#_924

%define _vpath_srcdir openmp
was simplied to 
cd openmp

rocm-omp, like the rest of the rocm packages will be also used for SUSE, and SUSE does its %cmake a little differently. 

I am not sure about errors for libomptarget.devicertl.a, it looks like this library is made up of AMDGPU objects and not normal X86 objects.

libomp.so.  This is consistent with system and compat clang, there are no versions.

libomp-19.1.6-2.fc42.x86_64 : OpenMP runtime for clang
Repo         : @System
Matched From : 
Filename     : /usr/lib64/libomp.so

libomp18-18.1.8-3.fc42.x86_64 : OpenMP runtime for clang
Repo         : rawhide
Matched From : 
Filename     : /usr/lib64/llvm18/lib/libomp.so

Created attachment 2067008 [details]
The .spec file difference from Copr build 8472432 to 8558142

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8558142
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2335652-rocm-omp/fedora-rawhide-x86_64/08558142-rocm-omp/fedora-review/review.txt

Please take a look if any issues were found.


---
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.

Just realised some files are apache licensed.

We might need it in rocm-llvm too:
https://download.copr.fedorainfracloud.org/results/%40fedora-review/fedora-review-2335652-rocm-omp/fedora-rawhide-x86_64/08558142-rocm-omp/fedora-review/licensecheck.txt

Can you give it a quick skim?

Other than that, it seems fine. Some false positives in the fedora review output, but it seems fine.

Spec URL: https://trix.fedorapeople.org/rocm-omp.spec
SPRM URL: https://trix.fedorapeople.org/rocm-omp-6.3.1-1.fc42.src.rpm

Change is to copy the license from the old libomp package
https://src.fedoraproject.org/rpms/libomp/blob/f40/f/libomp.spec#_41
Apache-2.0 WITH LLVM-exception OR NCSA

Then for ROCm parts add
AND MIT

Created attachment 2074043 [details]
The .spec file difference from Copr build 8558142 to 8578163

Copr build:
https://copr.fedorainfracloud.org/coprs/build/8578163
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2335652-rocm-omp/fedora-rawhide-x86_64/08578163-rocm-omp/fedora-review/review.txt

Please take a look if any issues were found.


---
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.

The Pagure repository was created at https://src.fedoraproject.org/rpms/rocm-omp