https://bugzilla.redhat.com/show_bug.cgi?id=1668010 --- Comment #7 from Tom Stellard <tstellar@xxxxxxxxxx> --- Spec URL: https://copr-be.cloud.fedoraproject.org/results/tstellar/rocm-2.0/fedora-rawhide-x86_64/00873694-hip/hip.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/tstellar/rocm-2.0/fedora-rawhide-x86_64/00873694-hip/hip-1.5.18494-1.rocm2.0.0.fc31.src.rpm (In reply to Felix Schwarz from comment #6) > 1. "LICENSE" file not installed (%license) > > 2. /usr/bin/hipconfig contains some unpatched paths: > $CUDA_PATH=$ENV{'CUDA_PATH'} // '/usr/local/cuda'; > $HCC_HOME=$ENV{'HCC_HOME'} // '/opt/rocm/hcc'; > $HSA_PATH=$ENV{'HSA_PATH'} // '/opt/rocm/hsa'; > Fixed these 2. > 3. /usr/bin/hipcc - unpatched paths > - HIP_PATH defaults to "/usr" but $HIP_INCLUDE_PATH is defined as > "$HIP_PATH/include" (-> $HIP_PATH/include/hip?). I think $HIP_PATH/include is correct, because the test cases I've seen use the hip/ prefix in #include directives. > - from a quick glance it seems as if »$HIP_PLATFORM = "clang"« won't work > as no default path is set? (just from reading the code) Right, you would need to set the HIP_CLANG_PATH environment variable to make it work. This is by design, since I couldn't get it to work with clang-7.0.0. Long-term, I hope to make HIP_PLATFORM=clang the default once I can get it working. > - missing requires: hipcc requires coreutils, grep, gcc, gcc-c++ (the > latter two are only required for a CentOS 7-specific workaround) > Fixed. > 4. /usr/lib64/libhip_hcc.so.1.5 > - executable-stack > - E: binary-or-shlib-defines-rpath /usr/lib64/libhip_hcc.so.1.5 > ['/usr/lib64'] > Fixed. > 5. hipexamine.sh, hipconvertinplace.sh non-functional > - "hipify-clang" is no built (not built by default), both scripts rely on > that > - probably best to remove these scripts entirely (I guess there is a reason > why upstream does not build them by default) > Fixed. > 6. In general I find that the hip package "clutters" /usr/bin/ a bit too > much. Things like 'findcode.sh' could go into "libexec" (but that might be > just personal preference). Also I'm a bit worried about the "ca" binary - > that name is extremly generic ("certificate authority"?) > Fixed. > 7. from a quick glance it seems as if "clara.hpp" is a header-only copylib. > "bundles(clara)"? > I wasn't sure what to do here. There is no clara package in Fedora, so I'm not sure there is any reason to do bundle(). > 8. license tag only lists MIT but licensecheck disagrees: > *No copyright* GNU Lesser General Public License (v2.1 or later) > ---------------------------------------------------------------- > HIP-roc-2.0.0/util/gedit/hip.lang > > BSD 3-clause "New" or "Revised" License > --------------------------------------- > HIP-roc-2.0.0/samples/1_Utils/hipBusBandwidth/LICENSE.txt > HIP-roc-2.0.0/samples/1_Utils/hipCommander/LICENSE.txt > > BSL (v1.0) > ---------- > HIP-roc-2.0.0/lpl_ca/pstreams/pstream.h > Fixed. > > 9. hipdemangleatp.sh: missing requires: > - sed, binutils, coreutils > Fixed. > > I have to say that I find "hip" even scarier to review with its static > libraries and all the duplicated shell/perl code... > > Also I did not have time yet to actually try the hip so there may be more > issues. Anyway thanks for your work so far :-) -- 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
