[Bug 2045955] New: Review Request: ROCm-CompilerSupport - Various AMD ROCm LLVM related services
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Bug ID: 2045955 Summary: Review Request: ROCm-CompilerSupport - Various AMD ROCm LLVM related services Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: alexjnewt@fastmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://mystro256.fedorapeople.org/ROCm-CompilerSupport.spec SRPM URL: https://mystro256.fedorapeople.org/ROCm-CompilerSupport-4.5.2-1.fc36.src.rpm Description: This package currently contains one library, the Code Object Manager (Comgr) Fedora Account System Username: mystro256
Notes: This requires ROCm-Device-Libs, which is currently in review (RHBZ#2044664). This package is required before packaging ROCm's HIP or OpenCL packages.
I packaged it so I can add more sub-packages later. I'm not sure if more "compiler support" libraries will be added in the future, but the documentation seems to imply it's possible.
As well, comgr compiles against static clang, but this isn't provided by Fedora. I've emailed upstream to see if this is a technical requirement, or if patching comgr to use dynamically linked clang is fine, and if so, if they're open to allowing it as an option.
RPMlint output:
ROCm-Comgr.x86_64: W: shared-lib-calls-exit /usr/lib64/libamd_comgr.so.2.1.0 exit@GLIBC_2.2.5
I let upstream know about this. I'm waiting to hear back.
ROCm-Comgr-devel.x86_64: W: spelling-error %description -l en_US amd -> AMD, mad, and ROCm-Comgr-devel.x86_64: W: no-documentation
Ignored for obvious reasons :)
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Link ID| |Red Hat Bugzilla 2044664 Doc Type|--- |If docs needed, set a value
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Link ID| |Red Hat Bugzilla 2044664
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2044664
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2044664 [Bug 2044664] Review Request: ROCm-Device-Libs - AMD ROCm LLVM bit code libraries
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On|2044664 |
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2044664 [Bug 2044664] Review Request: ROCm-Device-Libs - AMD ROCm LLVM bit code libraries
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |2044664
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2044664 [Bug 2044664] Review Request: ROCm-Device-Libs - AMD ROCm LLVM bit code libraries
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #1 from Jeremy Newton alexjnewt@fastmail.com --- Sorry for the noise, I forgot how to correctly set blockers :)
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Felix Schwarz fschwarz@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fschwarz@fedoraproject.org
--- Comment #2 from Felix Schwarz fschwarz@fedoraproject.org --- Similar to bug 2044664 I'd prefer a lower-case name. Debian uses "rocm-compilersupport" (https://salsa.debian.org/rocm-team/rocm-compilersupport), Arch "comgr" (https://github.com/rocm-arch/rocm-arch/blob/master/comgr/PKGBUILD). I prefer Debian's naming as upstream might decide to ship also other binaries as part of the source tarball and renaming the base package would be a bit painful in Fedora.
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #3 from Jeremy Newton alexjnewt@fastmail.com --- Thanks Felix, updated:
Spec URL: https://mystro256.fedorapeople.org/rocm-compilersupport.spec SRPM URL: https://mystro256.fedorapeople.org/rocm-compilersupport-4.5.2-1.fc36.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #4 from Jeremy Newton alexjnewt@fastmail.com --- Update to 5.0.0:
Spec URL: https://mystro256.fedorapeople.org/rocm-compilersupport.spec SRPM URL: https://mystro256.fedorapeople.org/rocm-device-libs-5.0.0-1.fc36.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #5 from Jeremy Newton alexjnewt@fastmail.com --- COPR builds: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-opencl/builds/
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #6 from Felix Schwarz fschwarz@fedoraproject.org ---
SRPM URL: https://mystro256.fedorapeople.org/rocm-compilersupport-5.0.0-1.fc36.src.rpm
rocm-comgr contains LICENSE.txt, NOTICES.txt, and README.md in /usr/share/amd_comgr (duplicated, should be removed)
Some patches could use a link or an explanation:
- "Upstream is ok with the patch, but my patch isn't currently upstreamable": why is the patch ok for Fedora but not for upstream? - "Fix location of cmake file, I've already emailed upstream a patch" - link?
One surprising thing was the number of required compilers:
BuildRequires: gcc BuildRequires: gcc-c++ BuildRequires: clang-devel
the package needs gcc AND clang?
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |ROCm-CompilerSupport - |rocm-compilersupport - |Various AMD ROCm LLVM |Various AMD ROCm LLVM |related services |related services
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #7 from Jeremy Newton alexjnewt@fastmail.com --- (In reply to Felix Schwarz from comment #6)
SRPM URL: https://mystro256.fedorapeople.org/rocm-compilersupport-5.0.0-1.fc36.src.rpm
rocm-comgr contains LICENSE.txt, NOTICES.txt, and README.md in /usr/share/amd_comgr (duplicated, should be removed)
Ok done, thanks! I should tell upstream this is a poor location to install them: share/doc/amd_comgr would be better.
Some patches could use a link or an explanation:
- "Upstream is ok with the patch, but my patch isn't currently
upstreamable": why is the patch ok for Fedora but not for upstream?
Sure, I realize my comment was very confusing, see updated. The issue is that upstream still wants to compile statically, but my patch forces dynamic linking. I need to figure out how to fix my patch to autodetect before submitting upstream.
- "Fix location of cmake file, I've already emailed upstream a patch" - link?
I emailed the patch directly to the developers, not a public mailing list. Alternatively, I can make a pull request on github if desired. As far a I know, github is just a mirror of an internal repo they have, so I thought it would be easier for them to apply the patch if I email the developers directly.
One surprising thing was the number of required compilers:
BuildRequires: gcc BuildRequires: gcc-c++ BuildRequires: clang-devel
the package needs gcc AND clang?
My bad, I always add gcc out of habit :)
Spec URL: https://mystro256.fedorapeople.org/rocm-compilersupport.spec SRPM URL: https://mystro256.fedorapeople.org/rocm-device-libs-5.0.0-1.fc36.src.rpm COPR: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-opencl/build/3460575/
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #8 from Jeremy Newton alexjnewt@fastmail.com --- Here's the rocm-opencl build if you're interested: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-opencl/builds/
The rocm-opencl package is full of problems, but it might help if you want to test something.
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #9 from Felix Schwarz fschwarz@fedoraproject.org ---
Sure, I realize my comment was very confusing, see updated. The issue is that upstream still wants to compile statically, but my patch forces dynamic linking. I need to figure out how to fix my patch to autodetect before submitting upstream.
Makes sense, thank you for the explanation.
I emailed the patch directly to the developers, not a public mailing list. Alternatively, I can make a pull request on github if desired. As far a I know, github is just a mirror of an internal repo they have, so I thought it would be easier for them to apply the patch if I email the developers directly.
Well, that's fine by me if it helps getting the patch applied. In general it would be nice if ROCm development would happen more in the open. As a Fedora packager please be aware of your special position as an AMD employee. I know it's very tempting to use "private" (and more efficient) communication but it excludes other (also potential) Fedora packagers. So in general it would be nice if the public git repos would put everyone on the same footing (I think also AMD would benefit from that eventually).
%{_libdir}/libamd_comgr.so.*
Please list a more specific so name: The idea is that the packager should be aware of soname changes and this is more likely if (s)he needs to adapt the file name in the .spec explicitely. See also: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_listing_shared_l...
Other than that I think the package looks good, will try a review soon.
Here's the rocm-opencl build if you're interested: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-opencl/builds/
Great, thank you!
The rocm-opencl package is full of problems, but it might help if you want to test something.
packaging problems or should I also expect functional problems?
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #10 from Felix Schwarz fschwarz@fedoraproject.org --- Why does "rocm-comgr-devel-5.0.0-1.fc37.x86_64.rpm" provide "rocm-compilersupport-devel"? If nothing depends on that specific name, I guess we should not provide it.
One thought about naming: The package description says: "The AMD Code Object Manager (Comgr) is a shared library which provides operations for creating and inspecting code objects." If this is only a shared library maybe the subpackage should be called "libcomgr"? Not sure about this though.
These were the only issues I came across.
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #11 from Jeremy Newton alexjnewt@fastmail.com --- (In reply to Felix Schwarz from comment #9)
Well, that's fine by me if it helps getting the patch applied. In general it would be nice if ROCm development would happen more in the open. As a Fedora packager please be aware of your special position as an AMD employee. I know it's very tempting to use "private" (and more efficient) communication but it excludes other (also potential) Fedora packagers. So in general it would be nice if the public git repos would put everyone on the same footing (I think also AMD would benefit from that eventually).
Yes, I figured it was one line, so it would be submitted fast, but that was about a month ago now... I'll follow up, and if I don't get a response, I'll make a pull request.
%{_libdir}/libamd_comgr.so.*
Please list a more specific so name: The idea is that the packager should be aware of soname changes and this is more likely if (s)he needs to adapt the file name in the .spec explicitely. See also: https://docs.fedoraproject.org/en-US/packaging-guidelines/ #_listing_shared_library_files
Understood, I'll use libfoo.so.X{,.*} as per the guidelines (e.g. include major).
packaging problems or should I also expect functional problems?
While I haven't specifically tested 5.0.0 on real HW, I don't expect functional issues. The issues are mostly packaging related.
(In reply to Felix Schwarz from comment #10)
Why does "rocm-comgr-devel-5.0.0-1.fc37.x86_64.rpm" provide "rocm-compilersupport-devel"? If nothing depends on that specific name, I guess we should not provide it.
That's fair, I'll drop it. I'm not sure of my original rational
One thought about naming: The package description says: "The AMD Code Object Manager (Comgr) is a shared library which provides operations for creating and inspecting code objects." If this is only a shared library maybe the subpackage should be called "libcomgr"? Not sure about this though.
"libcomgr" is a very Debian style of naming packages. I prefer rocm-comgr because it's more descriptive to users, which seems like the more fedora thing to do :) E.g. Fedora has:
icd-ocl (icd loader for opencl)
where Debian has:
ocl-icd-libopencl1
Although with that said, the library is actually called libamd_comgr, so alternatively we can match the cmake file name and call the package "amd-comgr". I can hold off updating the spec to give it some thought.
In contrast, upstream just calls their package "comgr".
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #12 from Felix Schwarz fschwarz@fedoraproject.org ---
"libcomgr" is a very Debian style of naming packages. I prefer rocm-comgr because it's more descriptive to users, which seems like the more fedora thing to do :)
Although with that said, the library is actually called libamd_comgr, so alternatively we can match the cmake file name and call the package "amd-comgr".
Fair enough. I'd prefer not to use company names for package names if not really necessary (abundance of caution, trademarks and all of that). While Debian has a lot of 'lib*' packages we also have some of these in Fedora as well though Fedora also uses '*-libs' for fine-grained dependencies. Anyway - no strong feelings on my side and in the end even Debian calls the package "rocm-comgr" if I understood Debian's git repo correctly.
Feel free to submit the tweaked spec ("rocm-compilersupport-devel") and I'll do a review to move this forward.
While I haven't specifically tested 5.0.0 on real HW, I don't expect functional issues.
Just fyi: I recompiled the OpenCL package for F35 but was unable to get "clinfo" to recognize my gfx card. However this is a RX570/gfx803 and the card is not officially supported so it might be also due to unsupported hardware.
$ rocm-clinfo Number of platforms: 1 Platform Profile: FULL_PROFILE Platform Version: OpenCL 2.2 AMD-APP (3406.0) Platform Name: AMD Accelerated Parallel Processing Platform Vendor: Advanced Micro Devices, Inc. Platform Extensions: cl_khr_icd cl_amd_event_callback
Platform Name: AMD Accelerated Parallel Processing Number of devices: 0
https://bugzilla.redhat.com/show_bug.cgi?id=2045955 Bug 2045955 depends on bug 2044664, which changed state.
Bug 2044664 Summary: Review Request: rocm-device-libs - AMD ROCm LLVM bit code libraries https://bugzilla.redhat.com/show_bug.cgi?id=2044664
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #13 from Jeremy Newton alexjnewt@fastmail.com --- (In reply to Felix Schwarz from comment #12)
Fair enough. I'd prefer not to use company names for package names if not really necessary (abundance of caution, trademarks and all of that). While Debian has a lot of 'lib*' packages we also have some of these in Fedora as well though Fedora also uses '*-libs' for fine-grained dependencies. Anyway
- no strong feelings on my side and in the end even Debian calls the package
"rocm-comgr" if I understood Debian's git repo correctly.
I just checked and you're indeed correct. Not sure why. Colour me surprised :) Regardless, since the source package is different, it gives flexibility to change it later.
Feel free to submit the tweaked spec ("rocm-compilersupport-devel") and I'll do a review to move this forward.
Done, see update below
Just fyi: I recompiled the OpenCL package for F35 but was unable to get "clinfo" to recognize my gfx card. However this is a RX570/gfx803 and the card is not officially supported so it might be also due to unsupported hardware.
Yes, I think to enable the pre-vega experimental support, you need to export ROC_ENABLE_PRE_VEGA=true or something like that. I don't see it documented anywhere, but I noticed it in when I was trying to debug the aarch64 build. As well, I'm told it's really buggy, so be warned :) I should have a vega, so I can test it later.
I also reviewed the licensing and noticed some issues, so I ended up making a pull request anyway.
Spec URL: https://mystro256.fedorapeople.org/rocm-compilersupport.spec SRPM URL: https://mystro256.fedorapeople.org/rocm-compilersupport-5.0.0-1.fc37.src.rpm COPR: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-opencl/build/3487501/
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #14 from Jeremy Newton alexjnewt@fastmail.com --- Note I also made some upstream issue tickets: https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/issues/40 https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/issues/41
I add the first ticket link to the spec in my next revision. I'll try my best to duplicate internal and external conversation as much as possible.
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #15 from Jeremy Newton alexjnewt@fastmail.com --- Updated, upstream provided some feedback on patch0
Spec URL: https://mystro256.fedorapeople.org/rocm-compilersupport.spec SRPM URL: https://mystro256.fedorapeople.org/rocm-compilersupport-5.0.0-1.fc37.src.rpm COPR: https://copr.fedorainfracloud.org/coprs/mystro256/rocm-opencl/build/3493216/
Also, they plan to merge https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/pull/39 into their trunk (amd-stg-open) today.
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Felix Schwarz fschwarz@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fschwarz@fedoraproject.org Status|NEW |ASSIGNED Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Felix Schwarz fschwarz@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #16 from Felix Schwarz fschwarz@fedoraproject.org --- Thank you for filing patches upstream and great that you were able to break the cyclic dependency.
Short version: package approved
Two things I'd like to see fixed/changed.
- "0002-Update-NOTICES.txt.patch": The patch seems obviously correct (no "yaml-cpp" directory present anymore) but (imho) we should not tweak licensing documents downstream. Would you mind dropping that patch before the initial import? (I think we don't have to do anything else like changing the license tag, just ship the NOTICES.txt unmodified.)
- Upstream also has a test suite. Can we run these in %check? (If this is too hard or not possible due to missing dependencies, we can ignore the test suite of course.)
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang.
(Package requires clang-devel so this is a non-issue imho.)
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present.
Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [x]: Latest version is packaged. [!]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [x]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [!]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Cannot parse rpmlint output:
Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output:
Source checksums ---------------- https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/archive/refs/tags/... : CHECKSUM(SHA256) this package : da1bbc694bd930a504406eb0a0018c2e317d8b2c136fb2cab8de426870efe9a8 CHECKSUM(SHA256) upstream package : da1bbc694bd930a504406eb0a0018c2e317d8b2c136fb2cab8de426870efe9a8
Requires -------- rocm-comgr (rpmlib, GLIBC filtered): libLLVM-13.so()(64bit) libLLVM-13.so(LLVM_13)(64bit) libc.so.6()(64bit) libclang-cpp.so.13()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) liblldCommon.so.13()(64bit) liblldELF.so.13()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) rtld(GNU_HASH)
rocm-comgr-devel (rpmlib, GLIBC filtered): cmake-filesystem(x86-64) libamd_comgr.so.2()(64bit) rocm-comgr(x86-64)
rocm-compilersupport-debugsource (rpmlib, GLIBC filtered):
Provides -------- rocm-comgr: libamd_comgr.so.2()(64bit) libamd_comgr.so.2(amd_comgr_1.8)(64bit) libamd_comgr.so.2(amd_comgr_2.0)(64bit) libamd_comgr.so.2(amd_comgr_2.2)(64bit) libamd_comgr.so.2(amd_comgr_2.3)(64bit) libamd_comgr.so.2(amd_comgr_2.4)(64bit) rocm-comgr rocm-comgr(x86-64)
rocm-comgr-devel: cmake(amd_comgr) rocm-comgr-devel rocm-comgr-devel(x86-64)
rocm-compilersupport-debugsource: rocm-compilersupport-debugsource rocm-compilersupport-debugsource(x86-64)
Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review --bug=2045955 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: R, Perl, PHP, Haskell, SugarActivity, Java, Ocaml, fonts, Python Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #17 from Jeremy Newton alexjnewt@fastmail.com --- (In reply to Felix Schwarz from comment #16)
Thank you for filing patches upstream and great that you were able to break the cyclic dependency.
Short version: package approved
Thanks! :)
Two things I'd like to see fixed/changed.
- "0002-Update-NOTICES.txt.patch": The patch seems obviously correct (no
"yaml-cpp" directory present anymore) but (imho) we should not tweak licensing documents downstream. Would you mind dropping that patch before the initial import? (I think we don't have to do anything else like changing the license tag, just ship the NOTICES.txt unmodified.)
That's fine, it'll be merged upstream soon anyway.
- Upstream also has a test suite. Can we run these in %check? (If this is
too hard or not possible due to missing dependencies, we can ignore the test suite of course.)
Yeah I'm going to look into this. If I can enable it easily, I will.
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
--- Comment #18 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rocm-compilersupport
https://bugzilla.redhat.com/show_bug.cgi?id=2045955
Jeremy Newton alexjnewt@fastmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE Last Closed| |2022-02-15 04:29:02
--- Comment #19 from Jeremy Newton alexjnewt@fastmail.com --- Thanks Gwyn, built successfully in rawhide.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com