https://bugzilla.redhat.com/show_bug.cgi?id=1545479 --- Comment #12 from Tom Stellard <tstellar@xxxxxxxxxx> --- SPEC URL: https://copr-be.cloud.fedoraproject.org/results/tstellar/rocm-2.0/fedora-rawhide-x86_64/00862882-hcc/hcc.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/tstellar/rocm-2.0/fedora-rawhide-x86_64/00862882-hcc/hcc-1.3.18505-2.rocm2.0.0.fc30.src.rpm (In reply to Felix Schwarz from comment #11) > I'm mostly done with the review (still hoping that someone more experienced > will join here :-). Some questions+bikeshedding and one bigger issue. > It's up to you, but there is no experience requirement for doing a review. I'd say this has been a very good review so far. > Let's start with the most important one: > .so files in /usr/libexec/hcc/lib64/ are not stripped (manual stripping > trims the size from 1.5 GB to 47.2 MB). Unless there is a really good reason > not to strip these I think that needs to be taken care of. > This is fixed. > Some questions/notes: > 1. hcc depends on "hcc-runtime-devel" but I was wondering why "hcc-runtime" > is not sufficient. "hcc-runtime-devel" just contains unversioned .so files. hcc's linker invocation references the unversioned libraries, which is why the hcc-runtime-devel files are needed. > 2. hcc also requires "rocminfo" but I was wondering where this is used or > why this is a dependency. If I grep for "rocminfo" in the sources it is only > mentioned when building a docker container. hcc needs rocm_agent_enumerator which is shipped with the rocminfo package. It is used to determine the GPU target when one isn't specified. It's referenced from the clang source code. > 3. "compiler-rt" and "cmake-tests" are removed. Would you mind providing > some rationale for this? Did I see correctly that "cmake-tests" requires > that "hc::accelerator()" returns something? (which might not work on build > machines) It doesn't seem like compiler-rt is actually needed, I think libgcc is enough. I also wanted hcc to use the system compiler-rt. I don't recall why I removed cmake-tests, probably because it wasn't working on the builders, but I can look at that again. > 4. rpmlint complains about "executable stack" (.so files in /usr/lib64/). Is > this necessary for hcc? > (https://fedoraproject.org/wiki/Packaging_tricks#Executable_stack) I filed an issue on hcc github for this, I'm not sure why the stack is being marked as electable. I fixed this in the specfile too. > 5. rpmlint complains that "/usr/libexec/hcc/lib64/libLLVM-8-rocm.so" > contains an invalid soname but AFAIK this is just an internal library which > has no soname at all. Yes, it's just an internal library. > 6. The upstream "hcc" package declares a "NCSA" license but it contains some > files with a different license: > Apache: > - stl-test/*.pl.in > - tests/Conformance > > Expat license: > - lib/hsa/unpinned_copy_engine.* > - hc2/external/elfio > > 3-clause BSD: > - utils/gtest/ > > Expat is probably similar enough to NCSA but I'm not sure what to do with > the others (only test code as far as I can see). > I've updated the license in the spec file. > > Bikeshedding: > - changelog: "1.3.18505-2.rocm2.0.0" is the latest but > "1.3.19020-1.rocm2.0.0" comes before (higher version number). Is that just > actual recording of history or due to some mishap? > - "HCC": I think you should use consistent upper-case spelling of HCC > - spelling in spec file: "trunck", "lincense" > - rpmlint says: "summary-not-capitalized" for various RPMs I've fixed all these too. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
