https://bugzilla.redhat.com/show_bug.cgi?id=2430590
--- Comment #31 from Michael Katzmann <[email protected]> --- Corrected files... Spec URL: https://download.copr.fedorainfracloud.org/results/vk2bea/GPIB-User/fedora-44-x86_64/10413434-gpib/gpib.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/vk2bea/GPIB-User/fedora-44-x86_64/10413434-gpib/gpib-4.3.7%5E202604302b4cefb-1.fc44.src.rpm [fedora-review-service-build] - `%bcond_*` are quite confusing indeed. If you do not port it to epel8 (don't know why you would at this time), please use `%bcond (option) (default_value)` instead ++ I removed these (evolutionary artifacts) - I try to stay away from sourceforge, but can't you use `Source0` pointing to `https://sourceforge.net/projects/linux-gpib/files/linux-gpib%20for%203.x.x%20and%202.6.x%20kernels/%{version}/linux-gpib-%{version}.tar.gz`? I took it from looking at what other spec files with sourceforge do [1] ++ I changed the Source0: to a URL but I'm not sure that helps - it seems that the file still needs to be downloaded into SOURCES - Getting a clean git archive is of course also encouraged ever since the libxz saga. I just don't know if sourceforge can automatically provide one. `%forgemeta` at least is not aware of one, but if you could find one out, do let us know. ++ I don't understand this - the Sourceforge archive IS git. ( https://sourceforge.net/p/linux-gpib/git/ci/master/tree/ ) - Note that the git archive contains the files of the `linux-gpib-kernel`, so you cannot use that as a source until the firmware parts are cleared ++ There is no firmware files in `linux-gpib-kernel`, these are the files that are now also part of the linux-kernel. Everything in the Sourceforge git archive (and consequently .zip file) is GPL - If you are using a git snapshot, please follow the snapshot naming guidelines ** Done - Please use `%autorelease`/`%autochangelog` [3] unless you have reasons not to. It helps other Fedora maintainers ** Done - `linux-gpib` and others are not a known package in Fedora, right? please remove the Obsoletes ** No, I was using that to obsolete the original COPR package. I've removed it. - Have you confirmed with upstream that the license is `GPL-2.0-or-later` or `GPL-2.0-only`? Afaik the distinction cannot be made from the license text alone since those are identical ** I updated to the GPL-3 license which is what the upstream package now is. - What are the reasons for all of the `%bcond_*`. Usually these are used to break circular dependencies or if a feature often breaks ** removed (see above) - Please use `%autosetup` or at least `%autopatch` instead of manual `%patch` commands ** done - Using `3` instead of `%{python3_pkgversion}` is sufficient and more readable. The latter is used for systems like centos where they can have a secondary python version. Unless it is requested to use `%{python3_pkgversion}`, please do not use it eagerly ** done - Please build the manpages regardless of `%with_docs` - Conversely, consider if the non-man documentations are really necessary and worth the effort to maintain. Latex dependencies can be quite heavy ** done (removed all the conditionals as they are not needed) - The `%bcond` do not gate the `BuildRequires` properly ** removed - For python packages you are supposed to use `%pyproject_buildrequires` instead of adding dependencies like `pyproject-rpm-macros` manually [4]. See other python projects for other macros that you should be using ** removed requirement for pyproject-rpm-macros - Please use a macro for the SOVERSION instead of the `.so.*` glob ** I define soversion and used %{soversion} but this is probably not what you want .. please check and let me know - Are the `%ldconfig` parts necessary, i.e. do other libraries expect it to be loaded as if they were installed in `/usr/lib64`? ** I removed the ldconfig_scriptlets as they are not used in Fedora (if I reads the documentation correctly). I think the ldconfig is still needed. - The `rm ..` in %postun is a big no-no. For that matter any scriplets should be avoided as much as possible ** removed ... (not needed) - Please drop all of the comments separating the sections and instead use 2 newlines when you switch to a new section. Current form can get quite hard to read due to non-standard comments on section separators ** What do you mean by 'Sections' - please elaborate - Please confirm that all the `find %{buildroot} -delete` are necessary. If so please report it to upstream because that is a bug ** removed (without apparent impact) - The comment about `EPEL7` does not apply, epel7 is no longer buildable in either Fedora or copr ++ removed - Please confirm the providence of all the Source/Patch files ++ yes all GPL files - The link to the tcl comment is dead. Please use the packaging guideline instructions instead ++ the packaging guideline recommended way to do this is supposed to be ... %{!?tcl_version: %global tcl_version %(echo 'puts $tcl_version' | tclsh)} %{!?tcl_sitearch: %global tcl_sitearch %{_libdir}/tcl%{tcl_version}} but this doesn't work on copr because tclsh is not available. I included the "BuildRequires: tcl-devel" (as described in the guideline) but it still fails to find tclsh. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2430590 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202430590%23c31 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected] Do not reply to spam, report it: https://forge.fedoraproject.org/infra/tickets/issues/new
