Bug 2353412
|
Description
Steve
2025-03-19 18:35:33 UTC
Copr build: https://copr.fedorainfracloud.org/coprs/build/8794946 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/08794946-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. > Patch0: 00-fix-makefile.patch > [...] > sed -i 's|$< $(CFLAGS)|$< $(CFLAGS) -Wno-incompatible-pointer-types|' src/Makefile You're already patching the Makefile, why use sed? Why not add this change to the patch? > %build > %set_build_flags Calling %set_build_flags manually is not needed since Fedora 36. Also, looking at the build log: > cp `pwd`/hash.h /builddir/build/BUILD/libgenht-1.1.3-build/BUILDROOT/usr/include/genht/hash.h > cp `pwd`/siphash24.h /builddir/build/BUILD/libgenht-1.1.3-build/BUILDROOT/usr/include/genht/siphash24.h > cp `pwd`/ht.h /builddir/build/BUILD/libgenht-1.1.3-build/BUILDROOT/usr/include/genht/ht.h > cp `pwd`/ht.c /builddir/build/BUILD/libgenht-1.1.3-build/BUILDROOT/usr/include/genht/ht.c It might be good to patch the Makefile (or alter the make call) to use "cp -p" in order to preserve file timestamps. Spec URL: https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/08819141-libgenht/libgenht.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/08819141-libgenht/libgenht-1.1.3-5.fc43.src.rpm Created attachment 2081959 [details]
The .spec file difference from Copr build 8794946 to 8819222
Copr build: https://copr.fedorainfracloud.org/coprs/build/8819222 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/08819222-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. Artur, thanks for the feedback >> Patch0: 00-fix-makefile.patch >> [...] >> sed -i 's|$< $(CFLAGS)|$< $(CFLAGS) -Wno-incompatible-pointer-types|' src/Makefile >you're already patching the Makefile, why use sed? >Why not add this change to the patch? The patch isn't mine, it comes from my upstream, who is a Debian Dude :) >> %build >> %set_build_flags >Calling %set_build_flags manually is not needed since Fedora 36. Done >Also, looking at the build log: >> cp `pwd`/hash.h /builddir/build/BUILD/libgenht-1.1.3-build/BUILDROOT/usr/include/genht/hash.h >>[...] >It might be good to patch the Makefile (or alter the make call) to use "cp -p" in order to preserve file timestamps. Done, also in the spec file https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/09462125-libgenht/libgenht.spec https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/09462125-libgenht/libgenht-1.1.3-6.fc44.src.rpm Created attachment 2104586 [details]
The .spec file difference from Copr build 8819222 to 9462133
Copr build: https://copr.fedorainfracloud.org/coprs/build/9462133 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/09462133-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. > %global debug_package %{nil}
No-go. If this creates .so files, it has to generate debuginfo.
Also, the -static subpackage should Require the -devel subpackage, as only -devel contains the include headers.
https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/09516997-libgenht/libgenht.spec https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/09516997-libgenht/libgenht-1.1.3-7.fc44.src.rpm Created attachment 2105642 [details]
The .spec file difference from Copr build 9462133 to 9516999
Copr build: https://copr.fedorainfracloud.org/coprs/build/9516999 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/09516999-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. Steve, are you still interested in this? Can you reupload the spec and SRPM somewhere? The current links are 404. https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10002364-libgenht/libgenht-1.1.3-7.fc44.src.rpm https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10002364-libgenht/libgenht.spec Artur, yes, definitely still interested. Copr build: https://copr.fedorainfracloud.org/coprs/build/10002396 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/10002396-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. (In reply to Steve from comment #15) > https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora- > rawhide-x86_64/10002364-libgenht/libgenht-1.1.3-7.fc44.src.rpm > https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora- > rawhide-x86_64/10002364-libgenht/libgenht.spec These links are also 404. To get the source rpm, people are forced to dig down into the automated copr build. Could you please place files in a location that they will not be downloaded by accident. Onto the package... * Builds fine. * Nothing from the upstream tarball 'doc' is being installed. Is there a reason for this? * 'static' package needlessly requires 'Requires: %{name}%{?_isa} = %{version}-%{release}'. Regards Phil Phil, Thanks for the feedback. Here is a fresh COPR build. Added a doc sub-package and removed the Requires from the static package. Steve https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10178021-libgenht/libgenht-1.1.3-8.fc45.src.rpm https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10178021-libgenht/libgenht.spec Created attachment 2131363 [details]
The .spec file difference from Copr build 10002396 to 10178464
Copr build: https://copr.fedorainfracloud.org/coprs/build/10178464 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/10178464-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. Hi, Making changes to 'src/Makefile' via patch and in the spec file using se is very messy. I would be inclined to update the patch to incorporate all changes to that file. Regards Phil Phil, as requested Steve https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10264110-libgenht/libgenht-1.1.3-9.fc45.src.rpm https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10264110-libgenht/libgenht.spec Hi,
Thanks for the new upload.
A few things.
* Add 'make test' below '%check'. All works well when enabling building of tests.
* Possibly add '%dir %{_includedir}/%{srcname}/' to the '-devel'. This will give the package ownership and control of the directories and its contents.
* Pedantic. Each 'BuildRequires' should have its own line. Also keep them in alphabetical order.
Looking good. If you could make some of the changes above, we can then move forward.
Regards
Phil
Phil, make test added BuildRequires each on its own line... As a member of the organization SVSE (Stop Vertical Scrolling Everywhere) I strenuously object to this, however if it is the way that everyone else does spec file I will also do it. As for the %dir line, I must resist, as per my training with SVSE. :) I commented on the other package that you are reviewing... I can copy it here if you need. https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10265300-libgenht/libgenht-1.1.3-10.fc45.src.rpm https://download.copr.fedorainfracloud.org/results/s-kro/libgenht/fedora-rawhide-x86_64/10265300-libgenht/libgenht.spec Steve Hi, The %dir is perfectly valid here. I have dropped the review and hope you can find another reviewer in the future. Regards Phil Created attachment 2135026 [details]
The .spec file difference from Copr build 10178464 to 10265673
Copr build: https://copr.fedorainfracloud.org/coprs/build/10265673 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/10265673-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. Created attachment 2135028 [details]
The .spec file difference from Copr build 10265673 to 10265679
Copr build: https://copr.fedorainfracloud.org/coprs/build/10265679 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353412-libgenht/fedora-rawhide-x86_64/10265679-libgenht/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libgenht Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- 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. |