[Bug 2335652] New: Review Request: rocm-omp - ROCm OpenMP

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

Bug ID: 2335652
           Summary: Review Request: rocm-omp - ROCm OpenMP
           Product: Fedora
           Version: rawhide
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

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

In F42 ROCm is using a bundled llvm in rocm-compilersupport.
The compat llvm 18 is being orphaned.

This package provides the OpenMP functionality of compat llvm.
Because it also adds offload to gpu functionality, it needs to be built after
the rocm runtime.  So it needs it's own package.

This package is needed now by the rocm-rpp package.

Reproducible: Always

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2335650
           Doc Type|---                         |If docs needed, set a value

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=2335650
[Bug 2335650] Review Request: mivisionx - AMD toolkit for computer vision and
machine intelligence

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

Fedora Review Service  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |AutomationTriaged
                URL|                            |https://github.com/ROCm/%%7Bu
                   |                            |pstreamname}

--- Comment #1 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8472432
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

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.

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #2 from Jeremy Newton  ---
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.

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #3 from Jeremy Newton  ---
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

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

Jeremy Newton  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]    |[e-mail]

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #4 from [e-mail] ---
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-...

%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

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #5 from Fedora Review Service  ---
Created attachment 2067008
  --> https://bugzilla.redhat.com/attachment.cgi?id=2067008&action=edit
The .spec file difference from Copr build 8472432 to 8558142

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #6 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8558142
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

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.

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2341286

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=2341286
[Bug 2341286] rocm-rpp: FTBFS in Fedora rawhide/f42

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #7 from Jeremy Newton  ---
Just realised some files are apache licensed.

We might need it in rocm-llvm too:
https://download.copr.fedorainfracloud.org/results/%40fedora-review/fedora-r...

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.

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #8 from [e-mail] ---
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

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #9 from Fedora Review Service  ---
Created attachment 2074043
  --> https://bugzilla.redhat.com/attachment.cgi?id=2074043&action=edit
The .spec file difference from Copr build 8558142 to 8578163

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

--- Comment #10 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8578163
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

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.

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

Jeremy Newton  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review+
             Status|NEW                         |POST

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

Fedora Admin user for bugzilla script actions  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |RELEASE_PENDING

--- Comment #11 from Fedora Admin user for bugzilla script actions  ---
The Pagure repository was created at
https://src.fedoraproject.org/rpms/rocm-omp

[Bug 2335652] Review Request: rocm-omp - ROCm OpenMP

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |CURRENTRELEASE
             Status|RELEASE_PENDING             |CLOSED
        Last Closed|                            |2025-04-10 00:22:09