https://bugzilla.redhat.com/show_bug.cgi?id=2217097 --- Comment #6 from Jeremy Newton <alexjnewt@xxxxxxxxxxxx> --- > My opinion on the naming is not a strong opinion, we can keep moving this review forward. Honestly the idea has kind of grown on me, I'll change to use the old fedora name in my next update. It shouldn't slow down this review as un-retiring a package is pretty much the same as a new package (review + pagure request). > The one that jumps out at me is the *64 suffix, is that really needed ? > I know it would be difficult to chase all the lib uses now, but it would be nice if the library name was something like librocmsmi.so. instead of librocm_smi64.so. A lot of the low level ROCm packages have this suffix. I believe it's a requirement for Windows, as they share a lot of source with Windows. I can propose a cmake patch for only using this suffix on Windows, but my gut feels like they will reject pulling it in and I'd rather not maintain any long term patches in Fedora if possible. > how stable is the versioning ? will the so.1.0 hold when rocm 6.0 comes out or would it be better to have a so.5.5.0, so.6.0.0 ? I'll contact the developers on Monday to confirm, but I think it's actually wrong because the logic tries to look for a git tag and fails, falling back to 1.0.0 as a default. I believe the soversion should be "5" and when they bump to "6" it will change to that. > Here is a warning cleanup that is a nice to have ACK > fedora-review is failing for me after this morning update of rawhide with. Yeah it's why I always make COPR builds, it has an option to generate the review form automatically, so you can just use that instead of running locally. I noticed it didn't actually generate for rawhide, which is probably because of the breakage, but you can use the f38 copy for now: https://download.copr.fedorainfracloud.org/results/mystro256/rocm-hip/fedora-38-x86_64/06109029-rocm_smi_lib/fedora-review/review.txt > ExclusiveArch, for the other rocm packages we have be setting the to just x86_64, are we going to confuse folks when just this rpm shows up on ppc ? There's initial unofficial support for ppc64le and aarch64. I'd like to try to build on all three platforms when possible to encourage adoption. To be clear, SMI just requires amdgpu kernel support, which has been in Fedora for a while now. On the other hand, the HIP math libraries are pretty broken on non-x86, so I'm ok with just x86_64 there, as the other parts of the stack is still useful without the mathlibs. -- 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 https://bugzilla.redhat.com/show_bug.cgi?id=2217097 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202217097%23c6 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
