Giza is an open, lightweight scientific plotting library built on top of cairo that provides uniform output to multiple devices. Provides uniform output to PDF, PS, PNG and X-Windows. Written in C with no dependencies (other than cairo) as a direct replacement for PGPLOT.
Created attachment 985455 [details] Sample spec file from https://build.opensuse.org/package/show/home:zhonghuaren/giza Spec file for release 0.7.6 of giza. Current version is 0.9.2.
Created attachment 985456 [details] Sample SRPM from https://build.opensuse.org/package/show/home:zhonghuaren/giza
Hi Joachim! Do you want to become a packager for Fedora? Then please take a look at https://fedoraproject.org/wiki/Join_the_package_collection_maintainers In addition, I recommend to read https://fedoraproject.org/wiki/Package_Review_Process Cheers, Flo
(In reply to Florian "der-flo" Lehner from comment #3) > Do you want to become a packager for Fedora? Thanks, I can take care of the giza package. I just need to unlock my user account at fedoraproject.org :o/
(In reply to Joachim Frieben from comment #4) > (In reply to Florian "der-flo" Lehner from comment #3) > > Do you want to become a packager for Fedora? > Thanks, I can take care of the giza package. I just need to unlock my user > account at fedoraproject.org :o/ Is your FAS-Account `jfrieben`? If so, please let me know. It seems - since you are not in the packager-group - you would need some privileges to access webspace on fedorapeople.org to host the spec-file / srpm. You can request them here [1] and select `initial package hosting request` from the drop-down mneu on the right. Assigning this review to me. Please let me know, when you are ready. =) [1] https://fedorahosted.org/packager-sponsors/newticket
(In reply to Björn "besser82" Esser from comment #5) I have not been able to recover the FAS user account jfrieben. I was therefore forced to create a new one for user name frieben. I have opened a ticket at https://fedorahosted.org/packager-sponsors/ticket/234. I have built an SRPM on my Fedora box for the current upstream release giza v0.9.3 which allows me to build functional binary RPMs giza and giza-devel.
Are you just copying something entirely from somewhere else as an initial submission and trying to become a Fedora packager? Björn, I'm afraid this was too hasty, I'm gonna close this one and submit this. Comments/feedbacks welcome.
*** This bug has been marked as a duplicate of bug 1275216 ***
(In reply to Christopher Meng from comment #7) No, I am not "copying something entirely from somewhere else as an initial submission". I had attached the publicly available and free SRPM as a first *sample* - that is how I had labeled the attachments - as an inspiration in order to accelerate things for some potentially interested and already active packager from the Fedora Project. Moreover, submssion to Fedora is not done by uploading spec file and SRPM to Bugzilla. I have proceeded later on to creating my own SRPM after Björn's invitation to take care of this package which I wanted to upload to the Fedora infrastructure. From the technical point of view, the package by "zhonghuaren" does not comply with Fedora packaging guide lines in several respects, and second, current upstream release is 0.9.3. In the meantime, I had prepared an SRPM from scratch based on release 0.9.3 that I was about to upload. Moreover, I am working together with upstream and have already repackaged giza-0.9.3 using GNU autotools. Thus, you are not only hasty yourself, you are even rude and offensive by interfering with a pending review request which had already been sponsored by another Fedora packager, namely Björn "besser82" Esser and without contacting Björn or me in advance.
Spec URL: http://1drv.ms/1ReBiIo SRPM URL: http://1drv.ms/1jToMQ3 Description: Scientific plotting library built on cairo that provides uniform output to multiple devices. It is written in standard ANSI C and has bindings for Fortran 90/95/2003. Devices currently supported are: the X Window system, PDF, PostScript, EPS, PNG and SVG. It also provides a drop-in replacement for the PGPLOT graphics library. Fedora Account System Username: frieben
Hello guys, this one is a head-scratcher. :-/ 1) Björn had assigned the ticket to himself already and apparently was willing to sponsor Joachim. 2) Christopher deleted the "fedora-review?" flag. Rude indeed. Btw, if deleting that flag, don't forget to restore the "Assigned To" field and readd the FE-NEEDSPONSOR to the "Blocks" field, or else the ticket does not appear on the needsponsor queue tracker page: http://fedoraproject.org/PackageReviewStatus/ 3) A second review ticket bug 1275216 for a completely different package is very bad style. Both packages differ a lot. Not just the src.rpm, also the build results. Do you think it's possible to agree on the packaging of "giza"? [...] > Spec URL: http://1drv.ms/1ReBiIo > SRPM URL: http://1drv.ms/1jToMQ3 Microsoft OneDrive is not compatible with fedora-review, as no direct download from such URLs is possible. Consider following comment 5 to acquire fedorapeople.org webspace. Then point the fedora-review tool at this ticket: fedora-review -b 1187030 > Name: giza > Group: Development/Libraries The base Group tag for runtime library packages has been "System Environment/Libraries" for many years dating back to Red Hat Linux. Nowadays, the Group tag is optional: https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag On the contrary, "Development/Libraries" is the Group for library -devel packages. > Patch0: makefile.patch > Patch1: marker.patch https://fedoraproject.org/wiki/Packaging:Guidelines#Patch_Guidelines > %build > %configure --disable-static > make https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags Build output is non-verbose. One cannot verify the compiler/linker options that are used during building. Try passing "V=1" to Make. If that doesn't work, it may be necessary need to search for ways to disable silent build rules. > %install > rm -rf $RPM_BUILD_ROOT https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > %doc AUTHORS ChangeLog COPYING INSTALL NEWS README https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text Plus, the INSTALL file does not contain anything worthwhile to RPM package users.
(In reply to Michael Schwendt from comment #11) > Both packages differ a lot. Not just the src.rpm, also the build results. Do > you think it's possible to agree on the packaging of "giza"? The SRPMs look different because for upstream release 0.9.4, I had replaced the homebrewn make system by standard GNU Autools, and this new package has been adopted by upstream. As a matter of fact, the main developer D. Price has granted me commit rights for the upstream code repository which is also beneficial for supporting a future Fedora package. The alternate package by C. Meng is based on the obsolete release 0.9.3 which requires a lot of dirty hacks in the spec file. Moreover, it completely lacks support for (c)pgplot which most users are more interested in than in using giza itself; it actually packages the giza backend alone and not the (c)pgplot wrapper libraries. > %install > rm -rf $RPM_BUILD_ROOT Ok, I can change that. I am unpleasantly surprised, however, that the spec file, and in particular the parts incriminated by you have been provided by the very official eclipse-fedorapackager plugin (!) after precisely choosing a Fedora library project.
> I am unpleasantly surprised Stay calm. It's very hard to please everyone. 1) Also some spec files in the Fedora package collection predate some of the changes in the packaging guidelines. Since no post-approval re-reviews are done for packages in the collection, this means that old or wrong things may remain in spec files for a very long time. 2) Some packagers insist on keeping a single spec file for multiple dist targets, such as EPEL 5 where it is still necessary to set up Buildroot and clean it, too. EL 5, however, also requires a %clean section, which is not in your spec file. 3) Some spec file template setup tools, such as rpmdev-newspec, still generate a spec file which cleans up the Buildroot at beginning of %install, because if it didn't, other people would complain about that. rpmdev-newspec doesn't output a %clean section, which is puzzling. But it tries to cover installations where e redhat-rpm-config is not installed: https://fedorahosted.org/rpmdevtools/ticket/25 And finally: 4) It's no major packaging mistake, no big issue. Really. It's just in the guidelines and worth knowing. Reviewers, who miss such things, sometimes get attacked/criticized. > eclipse-fedorapackager plugin Well, that sounds as if some of its output doesn't adhere to the packaging guidelines then. ;-p
Let's complete the reassignment from comment 8 onwards for the reason mentioned in comment 11.
(In reply to Michael Schwendt from comment #14) Since you have already reviewed my initial package and I have addressed your comments in a new release, why not simply assign it to yourself?
The new release should resolve the issues raised in comment 11. Spec URL: https://nrhbew-dm2305.files.1drv.com/y3mQPwBxoGdmPrM-ZUGWoHOjd6t4LGpj1gZMYuVNkShDbDSyq5RiNUlQclxyXaEqPfA65LCHuXaeRReKJeQVR4fd55dBmDmsZ7uWjNACh7F_mJXSXfNnu5BUbaaAwacGbh5Ge4GMc4y5VugFGY3DMWY6Wfd13dIo3w_MFfyLqnr7io/giza.spec SRPM URL: https://nrhbew-dm2305.files.1drv.com/y3m76-gUm22abilhtW34zPiJAj3QWGdAyhte7prO26B3q8N0lyy1WhO4nbE1v2Iz6yE5HgoB380Tk5-qTCZdH-91X3YvFK1whevu2p6McTtZLun0UY8bVo1OFZrUmU4SfvGxIL1L-eyF-zEM2RjncfjBfbPy1RSE91FYGioty8dPyo/giza-0.9.4-2.fc23.src.rpm
The new release should resolve the issues raised in comment 11. Spec URL: http://frieben.fedorapeople.org/giza.spec SRPM URL: http://frieben.fedorapeople.org/giza-0.9.4-2.fc23.src.rpm
Updated packages for the current release giza-1.1.0 are now available at COPR via 'dnf copr enable frieben/giza'.
Assuming: Spec URL: https://copr-be.cloud.fedoraproject.org/results/frieben/giza/fedora-rawhide-x86_64/00866796-giza/giza.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/frieben/giza/fedora-rawhide-x86_64/00866796-giza/giza-1.1.0-1.fc31.src.rpm - No need for the non useful macros like this %{__rm} → rm - In order to avoid unintentional SONAME bump, please do not glob the major soname version: %{_libdir}/*.so.0* - This is not needed anymore: %post -p /sbin/ldconfig %postun -p /sbin/ldconfig See https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets#Upgrade.2Fcompatibility_impact - add gcc as a BR - add docs to %doc - make setup quiet with "%setup -qn %{name}-%{version}" or just "%autosetup" - I didn't get any issue with parallel building, did you deactivate it because of race conditions? If not remove: %global _smp_mflags -j1 Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. Note: No gcc, gcc-c++ or clang found in BuildRequires See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - ldconfig not called in %post and %postun for Fedora 28 and later. Note: /sbin/ldconfig called in giza See: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GPL (v2 or later)", "Expat License", "FSF Unlimited License (with Retention) GNU General Public License", "FSF Unlimited License (with Retention)", "GNU General Public License". 71 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/giza/review- giza/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 194560 bytes in 4 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in giza , giza-debuginfo , giza-debugsource [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: giza-1.1.0-1.fc31.x86_64.rpm giza-devel-1.1.0-1.fc31.x86_64.rpm giza-debuginfo-1.1.0-1.fc31.x86_64.rpm giza-debugsource-1.1.0-1.fc31.x86_64.rpm giza-1.1.0-1.fc31.src.rpm giza.x86_64: W: shared-lib-calls-exit /usr/lib64/libgiza.so.0.1.0 exit.5 giza-devel.x86_64: W: no-documentation giza.src:31: W: setup-not-quiet 5 packages and 0 specfiles checked; 0 errors, 3 warnings.
(In reply to Robert-André Mauchin from comment #19) Hi Robert-André, first of all thanks a lot for reviewing the latest build! Regarding your comments, let me reply as follows: 1. Macros like "%{__rm} → rm" are official definitions provided by /usr/lib/rpm/macros which is part of redhat-rpm-config. I do agree that using such macros may be not very useful. However, this is a matter of taste, and I would actually rather keep them. 2. Globbing the major version has indeed been recently deprecated by Fedora. Your point is therefore absolutely valid, and I have changed %{_libdir}/*.so.* to %{_libdir}/*.so.0* as you have suggested. 3. Even though dropping ldconfig is rather a recommendation than a requirement, I have removed %post and %postun sections according to your suggestion. 4. Adding gcc as a BR is superfluous since gcc-gfortran is already a build requirement and already requires gcc itself. 5. I have silenced setup adopting "%setup -qn %{name}-%{version}" as you have suggested. 6. The build option "%global _smp_mflags -j1" is necessary because one Fortran source file depends on the presence of a Fortran module provided by some other Fortran source file. This dependency requires serial compilation. In the past, the corresponding race condition occasionally led to abortion of the build process when the necessary Fortran module could not be found by the compiler. Interestingly, during the few trials I have made with a parallel build of the current package, no problem was observed. Nevertheless, the root cause has not changed. I therefore prefer to disable parallel build for this package which only has a small size anyway. 7. I have added the content of docs/ to the newly created doc directory of giza-devel according to your request. An updated build giza-1.1.0-2.fc31 is available at: https://copr-be.cloud.fedoraproject.org/results/frieben/giza/fedora-rawhide-x86_64/00869543-giza/giza.spec https://copr-be.cloud.fedoraproject.org/results/frieben/giza/fedora-rawhide-x86_64/00869543-giza/giza-1.1.0-2.fc31.src.rpm
LGTM, package approved. You still need to find a sponsor.
review stalled
(In reply to Mattia Verga from comment #22) I am currently providing a package via the COPR repository which I do keep up-to-date with respect to upstream. Thus, Fedora users interested in using giza can just download it from that place.
Very well, so I'm going to close this ticket.
(In reply to Mattia Verga from comment #24) This was not the meaning of comment 23.
(In reply to Joachim Frieben from comment #23) > (In reply to Mattia Verga from comment #22) > I am currently providing a package via the COPR repository which I do keep > up-to-date with respect to upstream. Thus, Fedora users interested in using > giza can just download it from that place. Do you need help to find a sponsor to get this package into the distro?
(In reply to Robert-André Mauchin 🐧 from comment #26) I do need a sponsor in the first place, but getting help to find one is still better than nothing. :o/
(In reply to Joachim Frieben from comment #25) > (In reply to Mattia Verga from comment #24) > This was not the meaning of comment 23. Sorry, I misunderstood. If you need to find a sponsor, you can open a ticket on https://pagure.io/packager-sponsors/issues This review will also need to have a fresh positive review flag for the repository to be created. I suggest to start posting an updated spec and srpm.
@jfrieben I don't believe I can do a formal review, but I would be interested in taking a look. Are these the latest versions? Spec URL: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/02506790-giza/giza.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/02506790-giza/giza-1.2.1-1.fc35.src.rpm
(In reply to Mark E. Fuller from comment #29) This is correct, please go ahead. The upstream project is a low activity one, the drop-in compatibility with the original PGPLOT package is rather limited, and no noticeable progress has been made lately by upstream in improving this situation. For this reason, providing a COPR package which follows upstream closely is currently fine with me. For any comment or criticism regarding the functionality of the GIZA library itself please do not hesitate to contact upstream.
(In reply to Mark E. Fuller from comment #29) Btw, please avoid posting plain e-mail addresses in your comments, thanks.
Spec URL: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/03786112-giza/giza.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/03786112-giza/giza-1.3.2-1.fc37.src.rpm
Maybe this is of interest to the scientific computing SIG https://fedoraproject.org/wiki/Category:SciTech_SIG?rd=SIGs/SciTech
Unofficial review: Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. Note: No gcc, gcc-c++ or clang found in BuildRequires See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ - Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 1075200 bytes in 46 files. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_documentation ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [?]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GNU General Public License, Version 2", "[generated file]", "FSF Unlimited License (with License Retention) GNU General Public License v2.0 or later [generated file]", "FSF Unlimited License [generated file]", "GNU General Public License v2.0 or later [generated file]", "GNU General Public License v3.0 or later", "X11 License [generated file]", "GNU General Public License v2.0 or later", "FSF Unlimited License (with License Retention) GNU General Public License, Version 2", "FSF Unlimited License (with License Retention)", "GNU General Public License". 73 files have unknown license. Detailed output of licensecheck in /home/FedoraPackaging/giza/1187030-giza/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [?]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib64/gfortran [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [?]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [?]: Package is not known to require an ExcludeArch tag. [?]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [?]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [?]: Package should compile and build into binary rpms on all supported architectures. [!]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [?]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 1095680 bytes in /usr/share [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (debuginfo) ------------------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://github.com/danieljprice/giza/releases/download/v1.3.2/giza-v1.3.2.tar.gz : CHECKSUM(SHA256) this package : 87f14679923ba729a13bc18026178be684d7620aaf0a5eb4172a4a1c9c87c033 CHECKSUM(SHA256) upstream package : 87f14679923ba729a13bc18026178be684d7620aaf0a5eb4172a4a1c9c87c033 Requires -------- giza (rpmlib, GLIBC filtered): libX11.so.6()(64bit) libc.so.6()(64bit) libcairo.so.2()(64bit) libfontconfig.so.1()(64bit) libfreetype.so.6()(64bit) libgfortran.so.5()(64bit) libgfortran.so.5(GFORTRAN_10)(64bit) libgfortran.so.5(GFORTRAN_8)(64bit) libgiza.so.0()(64bit) libm.so.6()(64bit) rtld(GNU_HASH) giza-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config gcc-gfortran(x86-64) giza(x86-64) libcpgplot.so.0()(64bit) libgiza.so.0()(64bit) libpgplot.so.0()(64bit) giza-debuginfo (rpmlib, GLIBC filtered): giza-debugsource (rpmlib, GLIBC filtered): Provides -------- giza: giza giza(x86-64) libcpgplot.so.0()(64bit) libgiza.so.0()(64bit) libpgplot.so.0()(64bit) giza-devel: giza-devel giza-devel(x86-64) pkgconfig(cpgplot) pkgconfig(giza) pkgconfig(pgplot) giza-debuginfo: debuginfo(build-id) giza-debuginfo giza-debuginfo(x86-64) libcpgplot.so.0.1.2-1.3.2-1.fc37.x86_64.debug()(64bit) libgiza.so.0.1.2-1.3.2-1.fc37.x86_64.debug()(64bit) libpgplot.so.0.1.2-1.3.2-1.fc37.x86_64.debug()(64bit) giza-debugsource: giza-debugsource giza-debugsource(x86-64) Generated by fedora-review 0.8.0 (e988316) last change: 2022-04-07 Command line :/usr/bin/fedora-review -b 1187030 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: fonts, SugarActivity, Haskell, Ocaml, R, Python, Perl, PHP, Java Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Comments: 1) You have gcc-gfortran, but it would also be helpful to list gcc as a build dependency 2) Some of the comments on globbing of the filename seem not to have been addressed. As an example see https://music.fedorapeople.org/20220607/libaiff.spec 3) Perhaps also run the available tests 4) Provide a data subpackage 5) For your COPR builds, also enable AARCH64 and ARM-hfp to verify builds on all supported architectures which are listed at https://fedoraproject.org/wiki/Architectures#Primary_Architectures and linked from https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_support 6) Some of the files in the build directory have license GPLv3+ 7) Fortran packaging mod files are architecture specific, see https://docs.fedoraproject.org/en-US/packaging-guidelines/Fortran/
(In reply to Benson Muite from comment #34) Hi Benson, thanks for looking at my giza COPR package. Let me reply to your comments as follows: > 1) You have gcc-gfortran, but it would also be helpful to list gcc as a > build dependency The build requirement gcc-gfortran pulls in gcc already but I have added it explicitly in the latest build now in order to silence the package-review utility. > 2) Some of the comments on globbing of the filename seem not to have been > addressed. As an example see https://music.fedorapeople.org/20220607/libaiff.spec The suggested hack has been implemented but it breaks building the package for CentOS 7 whereas all other builds complete successfully. One could imagine a switch for CentOS but one might then simply stick to the previous expression, too. > 3) Perhaps also run the available tests Running 'make check' has been added. This option is actually implemented in the upstream package but it only compiles the test program and assumes that the user executes them individually and provides occasional user input. I had therefore left it out in the past but indeed checking for build failures is better than nothing. > 4) Provide a data subpackage I have split off the user documentation as recommended in the packaging guidelines. > 5) For your COPR builds, also enable AARCH64 and ARM-hfp to verify builds on > all supported architectures which are listed at > https://fedoraproject.org/wiki/Architectures#Primary_Architectures and > linked from https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_architecture_support I have enabled these additional build roots, and they do actually build successfully. > 6) Some of the files in the build directory have license GPLv3+ This merely seems to apply to some ancillary files used during the build process and which are part of autotools. According to the packaging guidelines they need not be taken into account. > 7) Fortran packaging mod files are architecture specific, see > https://docs.fedoraproject.org/en-US/packaging-guidelines/Fortran/ According to the packaging guidelines FORTRAN module files have to be installed in directory %{_fmoddir}. There are no further instructions for the multilib case which is a long-standing issue.
Can review but cannot sponsor. Please update the spec and srpm links. Perhaps use a switch for CentOS7. SPEC: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/04843933-giza/giza.spec SRPM: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/04843933-giza/giza-1.3.2-2.fc38.src.rpm
Spec URL: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/04863737-giza/giza.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/frieben/giza/fedora-rawhide-x86_64/04863737-giza/giza-1.3.2-3.fc38.src.rpm
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "GNU General Public License, Version 2", "[generated file]", "FSF Unlimited License (with License Retention) GNU General Public License v2.0 or later [generated file]", "FSF Unlimited License [generated file]", "GNU General Public License v2.0 or later [generated file]", "GNU General Public License v3.0 or later", "X11 License [generated file]", "GNU General Public License v2.0 or later", "FSF Unlimited License (with License Retention) GNU General Public License, Version 2", "FSF Unlimited License (with License Retention)", "GNU General Public License". 73 files have unknown license. Detailed output of licensecheck in /home/FedoraPackaging/reviews/giza/1187030-giza/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib64/gfortran [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 225280 bytes in 5 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (debuginfo) ------------------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.2.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/licenses.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 5 giza-doc.noarch: E: zero-length /usr/share/doc/giza-doc/documentation/api-ref.html giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libcpgplot.so.0.1.2 /lib64/libX11.so.6 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libcpgplot.so.0.1.2 /lib64/libcairo.so.2 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libcpgplot.so.0.1.2 /lib64/libfontconfig.so.1 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libcpgplot.so.0.1.2 /lib64/libfreetype.so.6 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libpgplot.so.0.1.2 /lib64/libX11.so.6 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libpgplot.so.0.1.2 /lib64/libcairo.so.2 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libpgplot.so.0.1.2 /lib64/libfontconfig.so.1 giza.x86_64: E: unused-direct-shlib-dependency /usr/lib64/libpgplot.so.0.1.2 /lib64/libfreetype.so.6 giza-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/.dwz/giza-1.3.2-3.fc38.x86_64 giza-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/usr/lib64/libcpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug giza-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/usr/lib64/libgiza.so.0.1.2-1.3.2-3.fc38.x86_64.debug giza-debuginfo.x86_64: W: unstripped-binary-or-object /usr/lib/debug/usr/lib64/libpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug giza-debuginfo.x86_64: E: statically-linked-binary /usr/lib/debug/.dwz/giza-1.3.2-3.fc38.x86_64 giza-debuginfo.x86_64: W: no-documentation giza-debugsource.x86_64: W: no-documentation giza-devel.x86_64: W: no-documentation giza-debuginfo.x86_64: E: missing-PT_GNU_STACK-section /usr/lib/debug/.dwz/giza-1.3.2-3.fc38.x86_64 giza-debuginfo.x86_64: E: ldd-failed /usr/lib/debug/usr/lib64/libcpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug /usr/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) ldd: warning: you do not have execution permission for `/usr/lib/debug/usr/lib64/libcpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug' giza-debuginfo.x86_64: E: ldd-failed /usr/lib/debug/usr/lib64/libgiza.so.0.1.2-1.3.2-3.fc38.x86_64.debug /usr/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) ldd: warning: you do not have execution permission for `/usr/lib/debug/usr/lib64/libgiza.so.0.1.2-1.3.2-3.fc38.x86_64.debug' giza-debuginfo.x86_64: E: ldd-failed /usr/lib/debug/usr/lib64/libpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug /usr/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) ldd: warning: you do not have execution permission for `/usr/lib/debug/usr/lib64/libpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug' giza-doc.noarch: W: invalid-license GPL-2.0 giza-debuginfo.x86_64: W: invalid-license GPL-2.0 giza.x86_64: W: invalid-license GPL-2.0 giza-debugsource.x86_64: W: invalid-license GPL-2.0 giza-devel.x86_64: W: invalid-license GPL-2.0 giza-debuginfo.x86_64: W: hidden-file-or-dir /usr/lib/debug/.dwz giza-debuginfo.x86_64: W: hidden-file-or-dir /usr/lib/debug/.dwz giza-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/11/5e220112899fca4a2b28f69c7da59518221c39 ../../../.build-id/11/5e220112899fca4a2b28f69c7da59518221c39 giza-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/54/d3de17eb0c2d6d2dc0ed46ade78acd9a21e013 ../../../.build-id/54/d3de17eb0c2d6d2dc0ed46ade78acd9a21e013 giza-debuginfo.x86_64: W: dangling-relative-symlink /usr/lib/debug/.build-id/63/88feac176a2a08810d1d78814c30f0bd5a2cb3 ../../../.build-id/63/88feac176a2a08810d1d78814c30f0bd5a2cb3 5 packages and 0 specfiles checked; 14 errors, 17 warnings, 14 badness; has taken 4.9 s Source checksums ---------------- https://github.com/danieljprice/giza/releases/download/v1.3.2/giza-v1.3.2.tar.gz : CHECKSUM(SHA256) this package : 87f14679923ba729a13bc18026178be684d7620aaf0a5eb4172a4a1c9c87c033 CHECKSUM(SHA256) upstream package : 87f14679923ba729a13bc18026178be684d7620aaf0a5eb4172a4a1c9c87c033 Requires -------- giza (rpmlib, GLIBC filtered): libX11.so.6()(64bit) libc.so.6()(64bit) libcairo.so.2()(64bit) libfontconfig.so.1()(64bit) libfreetype.so.6()(64bit) libgfortran.so.5()(64bit) libgfortran.so.5(GFORTRAN_10)(64bit) libgfortran.so.5(GFORTRAN_8)(64bit) libgiza.so.0()(64bit) libm.so.6()(64bit) rtld(GNU_HASH) giza-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config gcc-gfortran(x86-64) giza(x86-64) libcpgplot.so.0()(64bit) libgiza.so.0()(64bit) libpgplot.so.0()(64bit) giza-doc (rpmlib, GLIBC filtered): giza-debuginfo (rpmlib, GLIBC filtered): giza-debugsource (rpmlib, GLIBC filtered): Provides -------- giza: giza giza(x86-64) libcpgplot.so.0()(64bit) libgiza.so.0()(64bit) libpgplot.so.0()(64bit) giza-devel: giza-devel giza-devel(x86-64) pkgconfig(cpgplot) pkgconfig(giza) pkgconfig(pgplot) giza-doc: giza-doc giza-debuginfo: debuginfo(build-id) giza-debuginfo giza-debuginfo(x86-64) libcpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug()(64bit) libgiza.so.0.1.2-1.3.2-3.fc38.x86_64.debug()(64bit) libpgplot.so.0.1.2-1.3.2-3.fc38.x86_64.debug()(64bit) giza-debugsource: giza-debugsource giza-debugsource(x86-64) Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23 Command line :/usr/bin/fedora-review -b 1187030 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, C/C++, Generic Disabled plugins: PHP, Ruby, Haskell, Perl, R, Python, SugarActivity, fonts, Ocaml, Java Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Comments: a) Thanks for packaging this and your patience. Seems ok to me. b) You will need to find a sponsor. It helps to do a few unofficial reviews of other packages: https://docs.fedoraproject.org/en-US/fesco/Packager_sponsor_policy/#requesting_sponsorship
Some fixes that were not caught earlier: Can you update the license field to: GPL-2.0-only see: https://docs.fedoraproject.org/en-US/legal/allowed-licenses/ Can you remove the empty file? In the prep section add rm docs/documentation/api-ref.html $ rpmlint -e unused-direct-shlib-dependency unused-direct-shlib-dependency: The binary contains unused direct shared library dependencies. This may indicate gratuitously bloated linkage; check that the binary has been linked with the intended shared libraries only. Using $ ldd -u libgiza.so $ ldd -u libcpgplot.so Unused direct dependencies: /lib64/libX11.so.6 /lib64/libcairo.so.2 /lib64/libfontconfig.so.1 /lib64/libfreetype.so.6 $ ldd -u libpgplot.so Unused direct dependencies: /lib64/libX11.so.6 /lib64/libcairo.so.2 /lib64/libfontconfig.so.1 /lib64/libfreetype.so.6 It seems the makefile adds all libraries, and using -Wl,--as-needed seems not to help. https://unix.stackexchange.com/questions/473014/what-does-unused-direct-dependencies-mean Maybe further changes are needed in the setup?
I have asked upstream to remove api-ref.html as a likely left-over from a previous release. I have also suggested upstream to proceed to preparing a new release. I do agree with your comment correcting the license field but I would prefer to fix this in single build with the other issues. If no progress is being made by upstream in the next weeks I will release an updated COPR. The shared-library issue is a bit more tricky. I had actually converted the upstream package myself from a custom make-file based build system to using GNU autotools. If I remember correctly, the unused direct dependencies were introduced by the configure script checking for the presence of those libraries. This can probably be fixed but I found it convenient that compilation of Fortran code was possible by merely linking against libpgplot. I think that without these direct dependencies the whole bunch of required libraries needed to be appended explicitly which was not the traditional behaviour that I was used to when linking against the original PGPLOT library.
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time, but it seems that the review is still being working out by you. If this is right, please respond to this comment clearing the NEEDINFO flag and try to reach out the submitter to proceed with the review. If you're not interested in reviewing this ticket anymore, please clear the fedora-review flag and reset the assignee, so that a new reviewer can take this ticket. Without any reply, this request will shortly be resetted.
Happy to continue the review.
(In reply to Package Review from comment #41) Thanks, I will update the COPR package shortly to the latest upstream version 1.4.1 and address the suggested improvements as far as possible.