Spec URL: https://raw.githubusercontent.com/intel/QATzip/master/qatzip.spec SRPM URL: - Description: QATzip is a user space library which builds on top of the Intel® QuickAssist Technology user space library, to provide extended accelerated compression and decompression services by offloading the actual compression and decompression request(s) to the Intel® Chipset Series. QATzip produces data using the standard gzip* format (RFC1952) with extended headers. The data can be decompressed with a compliant gzip* implementation. QATzip is designed to take full advantage of the performance provided by Intel® QuickAssist Technology. Fedora Account System Username: zm627
If you don’t have somewhere convenient to upload the SRPM, you can use https://fedoraproject.org/wiki/Infrastructure/fedorapeople.org.
Thank you, Ben! The package will be uploaded to fedora space. I'll update the link it's once uploaded. Here I just provide a link related to the review: https://bugzilla.redhat.com/show_bug.cgi?id=1747500#
I have some issues uploading files to fedora people space. So I create a project in fedora copr system. Here's the link to the srpm package: https://download.copr.fedorainfracloud.org/results/zm627/qatzip/fedora-33-x86_64/02166305-qatzip/qatzip-1.0.4-1.fc33.src.rpm
Thanks. This is a high-quality package with just a couple of changes needed before I can approve it. Thanks for taking the time to include this software in the official repositories. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== Issues ===== - Fedora build flags are not honored. For an autotools configure script, the best choice would be to replace ./configure --prefix=%{_prefix} --enable-symbol with %configure --enable-symbol However, since you have a custom configure script that uses different options, and since it does not pick up environment variables, your best bet is probably to set the necessary variables this way: %set_build_flags ./configure --prefix=%{_prefix} --enable-symbol and then adjust your configure script to respect the CFLAGS/LDFLAGS from the environment. - You should provide all of the installation directories explicitly even though the defaults seem to be OK on x86_64. ./configure \ --bindir=%{_bindir} \ --sharedlib-dir=%{_libdir} \ --includedir=%{_includedir} \ --mandir=%{_mandir} \ --prefix=%{_prefix} \ --enable-symbol - There are upstream tests, but no %check section. If there are any that can be run as an unprivileged user without special hardware, please add a %check section and run them. Otherwise, please add a brief comment explaining why this is not possible. ===== Notes (no change is required for approval) ===== - Did you want to install the contents of the config_file/ directory? You could do something like this if you did. %package examples Summary: Sample configuration files for the libqatzip package BuildArch: noarch License: BSD or GPLv2 %description examples This package contains sample configuration files for the libqatzip package. %files examples %license LICENSE config_file/LICENSE.GPL %doc config_file/*/ ===== 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: "BSD 3-clause "New" or "Revised" License", "Unknown or generated", "GNU General Public License, Version 2", "BSD 3-clause "New" or "Revised" License GNU General Public License, Version 2". 4 files have unknown license. Detailed output of licensecheck in /home/reviewer/1955394-qatzip/licensecheck.txt Files licensed (BSD or GPLv2) are not currently installed, so “License: BSD” is correct. If this changes, I suggest installing the (BSD or GPLv2) files in a subpackage. See https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_multiple_licensing_scenarios. [x]: License file installed when any subpackage combination is installed. [!]: %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. [!]: Package consistently uses macros (instead of hard-coded directory names). Should use RPM macros to provide all installation directories to configure script. [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. [-]: Package is not known to require an ExcludeArch tag. Package is ExclusiveArch and bugzillas will be filed. [x]: Package complies to the Packaging Guidelines (except as otherwise noted) [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 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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [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). [?]: 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. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [-]: Package should compile and build into binary rpms on all supported architectures. Package is ExclusiveArch [!]: %check is present and all tests pass. Can any tests be run as an unprivileged user? [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]: 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: qatzip-1.0.4-1.fc35.x86_64.rpm qatzip-devel-1.0.4-1.fc35.x86_64.rpm qatzip-debuginfo-1.0.4-1.fc35.x86_64.rpm qatzip-debugsource-1.0.4-1.fc35.x86_64.rpm qatzip-1.0.4-1.fc35.src.rpm qatzip.x86_64: W: name-repeated-in-summary C QATzip qatzip.x86_64: W: spelling-error %description -l en_US gzip -> zip, grip, g zip qatzip-devel.x86_64: W: spelling-error Summary(en_US) libqatzip -> libation qatzip.src: W: name-repeated-in-summary C QATzip qatzip.src: W: spelling-error %description -l en_US gzip -> zip, grip, g zip qatzip.src:43: W: configure-without-libdir-spec 5 packages and 0 specfiles checked; 0 errors, 6 warnings. Rpmlint (debuginfo) ------------------- Checking: qatzip-debuginfo-1.0.4-1.fc35.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- qatzip-devel.x86_64: W: spelling-error Summary(en_US) libqatzip -> libation qatzip.x86_64: W: name-repeated-in-summary C QATzip qatzip.x86_64: W: spelling-error %description -l en_US gzip -> zip, grip, g zip 4 packages and 0 specfiles checked; 0 errors, 3 warnings. Source checksums ---------------- https://github.com/intel/QATzip/archive/v1.0.4/qatzip-1.0.4.tar.gz : CHECKSUM(SHA256) this package : f6272d9b4b214f9c8621d293a72ca5b3a04d9a4c26469f30dccb34ece6fe3531 CHECKSUM(SHA256) upstream package : f6272d9b4b214f9c8621d293a72ca5b3a04d9a4c26469f30dccb34ece6fe3531 Requires -------- qatzip (rpmlib, GLIBC filtered): ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libpthread.so.0()(64bit) libqat.so.0()(64bit) libusdm.so.0()(64bit) libz.so.1()(64bit) libz.so.1(ZLIB_1.2.2)(64bit) rtld(GNU_HASH) qatzip-devel (rpmlib, GLIBC filtered): qatzip(x86-64) qatzip-debuginfo (rpmlib, GLIBC filtered): qatzip-debugsource (rpmlib, GLIBC filtered): Provides -------- qatzip: libqatzip.so.1()(64bit) qatzip qatzip(x86-64) qatzip-devel: qatzip-devel qatzip-devel(x86-64) qatzip-debuginfo: debuginfo(build-id) libqatzip.so.1.0.4-1.0.4-1.fc35.x86_64.debug()(64bit) qatzip-debuginfo qatzip-debuginfo(x86-64) qatzip-debugsource: qatzip-debugsource qatzip-debugsource(x86-64) Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -b 1955394 Buildroot used: fedora-rawhide-x86_64 Active plugins: C/C++, Shell-api, Generic Disabled plugins: Ruby, fonts, R, Ocaml, Python, Haskell, Java, SugarActivity, PHP, Perl Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
Please also split out the libraries present in the main package into a -libs subpackage so that multilib package filtering works correctly.
Thanks for your quick reply, Ben! We'll revise the spec according to your comments. Here's my reply inline with your comments. > Fedora build flags are not honored. For an autotools configure script, the > best choice would be to replace........ We'll try to revise the configuration script to take environment variables. > You should provide all of the installation directories explicitly even though > the defaults seem to be OK on x86_64. Will fix in next version of spec. > There are upstream tests, but no %check section. If there are any that can be > run as an unprivileged user without special hardware, please add a %check > section and run them. Otherwise, please add a brief comment explaining why > this is not possible. We'll look into this issue and see if this section is needed, though currently we have a software fallback for no hardware issue. Thank you for your example of this section! > Did you want to install the contents of the config_file/ directory? > You could do something like this if you did. We don't have the plan to include the contents in the config_file as I know. Since the package works with the intel-qatlib package, all the configuration is set by that package for the upstream version. Thanks!
Thank you Neal! However I'm a little confused with the -libs subpackage. Would you like to provide more information about it? What I got from your comment is that it's better to split the two .so files into two packages. The main package contains the .so.1.0.4 and the -libs package contains the .so.1 (version number is an example here based on the current upstream release version). Finally we should have totally 3 packages, the main package which is qatzip with .so.1.0.4, the -libs package qatzip-libs with .so.1, and the devel package qatzip-devel with .so. The libs subpackage should have dependency on the main package. Would you like to correct me if I'm wrong? Thanks!
(In reply to zm627 from comment #7) > Thank you Neal! > > However I'm a little confused with the -libs subpackage. Would you like to > provide more information about it? > > What I got from your comment is that it's better to split the two .so files > into two packages. The main package contains the .so.1.0.4 and the -libs > package contains the .so.1 (version number is an example here based on the > current upstream release version). > Finally we should have totally 3 packages, the main package which is qatzip > with .so.1.0.4, the -libs package qatzip-libs with .so.1, and the devel > package qatzip-devel with .so. > The libs subpackage should have dependency on the main package. > > Would you like to correct me if I'm wrong? Thanks! Basically, it should look like this: %package libs Summary: Libraries for the qatzip package %description libs This package contains libraries for applications to use the QATzip APIs. %files libs %{_libdir}/libqatzip.so.%{libqatzip_soversion} %{_libdir}/libqatzip.so.%{version} Additionally, the main and devel packages should have a dependency on the libs package, like so: Requires: %{name}-libs%{?_isa} = %{version}-%{release}
(In reply to Neal Gompa from comment #8) Got it now. Thank you Neal!
Hi Neal, I just have a question here. Why should we split the libs into another subpackage? Is it a bit overlapped with the devel package? I'm not quite understand what the "multilib package filtering" means. Would you like to help me with it?
I’m used to seeing and using -libs subpackages, but only for the purpose of allowing dependencies on a C or C++ API without pulling in substantial CLI or GUI applications, so I’d like to understand this recommendation too.
(In reply to Ben Beasley from comment #11) > I’m used to seeing and using -libs subpackages, but only for the purpose of > allowing dependencies on a C or C++ API without pulling in substantial CLI > or GUI applications, so I’d like to understand this recommendation too. The way that multi-arch works in Fedora is that 32-bit x86 and 64-bit x86 library packages are shipped in the x86_64 repo. In the infrastructure, we sort through all the packages and apply a set of rules to determine which packages qualify for this "dual arch" treatment. The most reliable way to make sure things get set up properly is to have runtime libraries split out into their own subpackage. When they *aren't* split out, we get a number of cases where this treatment gets inconsistently applied, which causes bugs for installations and upgrades as packages can appear and disappear randomly when repositories are updated.
> When they *aren't* split out, we get a number of cases where this treatment gets inconsistently applied, which causes bugs for installations and upgrades as packages can appear and disappear randomly when repositories are updated. Interesting. I’ve always been a little confused by how multilib filtering actually happens. It seems like the problem you describe applies only to libraries, like this one, that are potentially for use by other packages, rather than those only intended to support an associated executable. Would you agree? Perhaps this rationale should be added to https://docs.fedoraproject.org/en-US/packaging-guidelines/#_mixed_use_packages. Technically it does cover your recommendation, but I think the existing rationale in the Guidelines for splitting libraries and applications into subpackages is pretty weak for packages where either the library or application part is very small.
Thanks Ben and Neal. I'll update this thread once I finished the next version of spec with your comments!
Are you still planning to package this?
(In reply to Ben Beasley from comment #15) > Are you still planning to package this? Hi Ben, Sorry for the late reply. I'm in another project last month and I'm just back to focus on the rpm packaging this week. There will be a public release very soon, around a week, on github (I have to pass the release process...) Thanks a lot for your support!! Zheng
Hi Ben, Neal, For the reason it more than a month since the last update, here's a quick retro for the rpm spec and review comments. And I'd like to post the changes here first, waiting for your comments, before it's released to github. Because I have to pass go through the internal release process every time committing to public github. And the process is a little complicated. The spec is to include qatzip app and library into fedora. And after the first round of review, we need to revise the spec with these 4 comments. I'll put my changes here. 1. Revise configure script to provide all of the installation directories explicitly Fix as comment #4 2. Revise configure script to hold fedora compile flags - Add such lines in configure script as the "enable_symbol" one, to hold the CFLAGS and LDFLAGS set by %set_build_flags if [ "$enable_enval" = "yes" ] ; then CFLAGS+=" `echo "$CFLAGS"`" LDFLAGS+=" `echo $LDFLAGS`" fi - Configure with this option in spec file - Qatzip just use these 2 flags set by %set_build_flags, other flags such as FFLAGS are not honored here. 3. Split library package from main package into sub-package - Split as comment #8 - Main package will not depend on the lib package, but the devel package does. Main package only contains binary file and is not linked to libqatzip.so. The libqatzip.so is provided to other applications, so the devel package depends on the libs package. 4. About the %check section The upstream test source code are not invoked by the qatzip itself and is not called during the setup process. It maybe used by some benchmark tools. So I think we can add a brief comment in the spec file(in the spec file?) to explain it, such as # Check section is not available for these functional and performance tests require special hardware. Would you mind to give some comments for the fix, Ben, Neal? Thanks! Zheng
Well, it’s a little hard to review a hypothetical spec file, especially because I no longer remember much about this package, but I’ll try! > 1. Revise configure script to provide all of the installation directories explicitly > Fix as comment #4 > > 2. Revise configure script to hold fedora compile flags > […] It sounds like you understand my suggestion and plan to implement it, so if the details are correct, I would approve this. ;-) Check the actual compiler command lines used in the build and compare against the output of “rpm -E '%{optflags}'” to be sure the flags are what they ought to be. > - Qatzip just use these 2 flags set by %set_build_flags, other flags such as FFLAGS are not honored here. Obviously, you do only need to handle the environment variables that apply to your build; no need to consider FFLAGS when there are no Fortran sources, or CXXFLAGS when there are no C++ sources. For a C library, handling CFLAGS and LDFLAGS should be sufficient. Make sure you are not adding any other optimization flags such as -O3 on top of these without justification (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags). > 3. Split library package from main package into sub-package > - Split as comment #8 > - Main package will not depend on the lib package, but the devel package does. > Main package only contains binary file and is not linked to libqatzip.so. > The libqatzip.so is provided to other applications, so the devel package depends on the libs package. This sounds right. (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package) It’s common for the subpackage to be called -libs even when there is only one library file, but -lib would be acceptable too. Remember to you use a fully-versioned, arch-specific dependency like: > Requires: %{name}-libs%{?_isa} = %{version}-%{release} > 4. About the %check section > The upstream test source code are not invoked by the qatzip itself and is not called during the setup process. > It maybe used by some benchmark tools. > So I think we can add a brief comment in the spec file(in the spec file?) to explain it, such as > # Check section is not available for these functional and performance tests require special hardware. Agreed, this clearly justifies the lack of a %check section.
Thank you very much for your comments, Ben! I'll push this spec to github ASAP after we finished the functional test of the qatzip rpm package. Once it's uploaded, I'll update this thread again. We'll then have a "real" spec to review. :)
Hi Ben, Sorry for the delay of the release for the source code and spec. But we have one now :) Would you like to help to review it? Spec URL: https://download.copr.fedorainfracloud.org/results/zm627/qatzip/fedora-34-x86_64/02320102-qatzip/qatzip.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/zm627/qatzip/fedora-34-x86_64/02320102-qatzip/qatzip-1.0.5-1.fc34.src.rpm Thanks a lot!
I was hoping to be able to approve this revision, but I think there are still a couple of things that need to be revisited. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== Issues ===== - There are some issues around interdependencies among subpackages and license files related to the new -libs subpackage. * The base package is right, I think. Normally, in a library-and-tool package, the base package should depend on the -libs package with an arched and fully-versioned dependency, because the command-line tool would use the shared library at runtime. In this case, the library is statically linked into the tool, which is OK across subpackages in a single source RPM. So since there is no implicit dependency, it’s correct that the base package doesn’t have an explicit dependency on -libs, and that it has the LICENSE files. * The -libs subpackage is correct too (no Requires on other subpackages), but it needs %license LICENSE* added to its %files section too, since it can be installed independently of the base package. See https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#subpackage-licensing. * The -devel package correctly has Requires: %{name}-libs%{?_isa} = %{version}-%{release} and correctly does not have its own copy of the LICENSE file (since the -libs dependency will always provide a copy). However, I think Requires: %{name}%{?_isa} = %{version}-%{release} is bogus and should be removed, unless I’m missing some reason that the command-line tool and its man page should be required for compiling applications that link against the library. - ExcludeArch is basically correctly handled. Instead of “Placeholder comment,” you should really have something similar to what you would put in the Bugzilla report. Something like “The purpose of the package is to support hardware that only exists on x86_64 platforms” would be fine. Would ExcludeArch: %{arm} aarch64 %{power64} s390x i686 be more accurately written as an ExclusiveArch? ExclusiveArch: x86_64 (You would still handle it the same way as the ExcludeArch in terms of filing an issue for unsupported architectures.) - The latest changelog entry’s version, 1.0.4-1, does not match the package version 1.0.5-1. - The PDF documentation does not belong in /usr/share/man. That is only for actual man pages. Please put it in %{_pkgdocdir} instead. Since the existing configure/Makefile always installs the man pages and PDF documentation in the same place, you will have to clean up after it. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation for different methods of installing documentation. One reasonable approach would be to add rm -vf %{buildroot}%{_mandir}/*.pdf after “%make_install”, and then change %doc %{_mandir}/QATzip-man.pdf to %doc docs/QATzip-man.pdf in “%files devel”. That will install it as /usr/share/doc/qatzip-devel/QATzip-man.pdf. ===== Notes (no change required) ===== - You could, if you liked, write URL: https://github.com/intel/%{githubname} Source0: https://github.com/intel/%{githubname}/archive/v%{version}/%{name}-%{version}.tar.gz more concisely as URL: https://github.com/intel/%{githubname} Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz ===== 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: "BSD 3-clause "New" or "Revised" License", "Unknown or generated", "GNU General Public License, Version 2", "BSD 3-clause "New" or "Revised" License GNU General Public License, Version 2". 4 files have unknown license. Detailed output of licensecheck in /home/reviewer/1955394-qatzip/licensecheck.txt GPLv2 license applies only to config_file/, which does not contribute to the build and is intentionally not installed. [!]: License file installed when any subpackage combination is installed. Possible to install qatzip-libs alone with no license file. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [!]: Changelog in prescribed format. Version does not match [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. [!]: Package obeys FHS, except libexecdir and /usr/target. PDF documentation installed in /usr/share/man. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [!]: Requires correct, justified where necessary. Unless I am missing something, the -devel package should not require the base package, which contains only the command-line tool and its man page. [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. ExcludeArch is basically correctly handled. (Instead of “Placeholder comment,” you should really have something similar to what you would put in the Bugzilla report.) [x]: Package complies to the Packaging Guidelines (except as noted) [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 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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [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). (except already-mentioned dependency of -devel on base package) [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in qatzip- libs [?]: 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. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [-]: Package should compile and build into binary rpms on all supported architectures. Package has ExcludeArch [-]: %check is present and all tests pass. A comment properly explains why tests cannot be run. [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]: 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: qatzip-1.0.5-1.fc35.x86_64.rpm qatzip-libs-1.0.5-1.fc35.x86_64.rpm qatzip-devel-1.0.5-1.fc35.x86_64.rpm qatzip-debuginfo-1.0.5-1.fc35.x86_64.rpm qatzip-debugsource-1.0.5-1.fc35.x86_64.rpm qatzip-1.0.5-1.fc35.src.rpm qatzip.x86_64: W: name-repeated-in-summary C QATzip qatzip.x86_64: W: spelling-error %description -l en_US gzip -> zip, grip, g zip qatzip.x86_64: W: incoherent-version-in-changelog 1.0.4-1 ['1.0.5-1.fc35', '1.0.5-1'] qatzip-libs.x86_64: W: no-documentation qatzip-devel.x86_64: W: spelling-error Summary(en_US) libqatzip -> libation qatzip.src: W: name-repeated-in-summary C QATzip qatzip.src: W: spelling-error %description -l en_US gzip -> zip, grip, g zip qatzip.src:53: W: configure-without-libdir-spec 6 packages and 0 specfiles checked; 0 errors, 8 warnings. Rpmlint (debuginfo) ------------------- Checking: qatzip-libs-debuginfo-1.0.5-1.fc35.x86_64.rpm qatzip-debuginfo-1.0.5-1.fc35.x86_64.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://github.com/intel/QATzip/archive/v1.0.5/qatzip-1.0.5.tar.gz : CHECKSUM(SHA256) this package : 32c4aeac5541fcb6be2940172e5ab0738babf0f768fe808b7ec20c6651423c8b CHECKSUM(SHA256) upstream package : 32c4aeac5541fcb6be2940172e5ab0738babf0f768fe808b7ec20c6651423c8b Requires -------- qatzip (rpmlib, GLIBC filtered): glibc ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libqat.so.0()(64bit) libusdm.so.0()(64bit) libz.so.1()(64bit) libz.so.1(ZLIB_1.2.2)(64bit) rtld(GNU_HASH) qatzip-libs (rpmlib, GLIBC filtered): glibc ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libqat.so.0()(64bit) libusdm.so.0()(64bit) libz.so.1()(64bit) libz.so.1(ZLIB_1.2.2)(64bit) rtld(GNU_HASH) qatzip-devel (rpmlib, GLIBC filtered): qatzip(x86-64) qatzip-libs(x86-64) qatzip-debuginfo (rpmlib, GLIBC filtered): qatzip-debugsource (rpmlib, GLIBC filtered): Provides -------- qatzip: qatzip qatzip(x86-64) qatzip-libs: libqatzip.so.1()(64bit) qatzip-libs qatzip-libs(x86-64) qatzip-devel: qatzip-devel qatzip-devel(x86-64) qatzip-debuginfo: debuginfo(build-id) qatzip-debuginfo qatzip-debuginfo(x86-64) qatzip-debugsource: qatzip-debugsource qatzip-debugsource(x86-64) Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -b 1955394 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic, C/C++ Disabled plugins: Ruby, R, SugarActivity, Perl, Python, Haskell, fonts, PHP, Ocaml, Java Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
(In reply to Ben Beasley from comment #21) Hi Ben, Thanks a lot for your review!! > ===== Issues ===== > > - There are some issues around interdependencies among subpackages and > license > files related to the new -libs subpackage. > * The -devel package correctly has > > Requires: %{name}-libs%{?_isa} = %{version}-%{release} > > and correctly does not have its own copy of the LICENSE file (since the > -libs dependency will always provide a copy). However, I think > > Requires: %{name}%{?_isa} = %{version}-%{release} > > is bogus and should be removed, unless I’m missing some reason that the > command-line tool and its man page should be required for compiling > applications that link against the library. The command line tool is not required for compiling apps that link against the library. So this "Requires" is removed. > - ExcludeArch is basically correctly handled. > > Instead of “Placeholder comment,” you should really have something similar > to > what you would put in the Bugzilla report. Something like “The purpose of > the > package is to support hardware that only exists on x86_64 platforms” would > be > fine. > > Would > > ExcludeArch: %{arm} aarch64 %{power64} s390x i686 > > be more accurately written as an ExclusiveArch? > > ExclusiveArch: x86_64 Replaced ExcludeArch with ExclusiveArch. > (You would still handle it the same way as the ExcludeArch in terms of > filing > an issue for unsupported architectures.) > - The latest changelog entry’s version, 1.0.4-1, does not match the package > version 1.0.5-1. Changed the changelog. Since the spec is not included, I replaced the only line in changelog with the 1.0.5 one. > - The PDF documentation does not belong in /usr/share/man. That is only for > actual man pages. Please put it in %{_pkgdocdir} instead. > > Since the existing configure/Makefile always installs the man pages and PDF > documentation in the same place, you will have to clean up after it. See > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation > for > different methods of installing documentation. > > One reasonable approach would be to add > > rm -vf %{buildroot}%{_mandir}/*.pdf > > after “%make_install”, and then change > > %doc %{_mandir}/QATzip-man.pdf > > to > > %doc docs/QATzip-man.pdf > > in “%files devel”. That will install it as > /usr/share/doc/qatzip-devel/QATzip-man.pdf. Move the pdf out of the man directory to package doc directory with the commands you suggested. Thanks! > ===== Notes (no change required) ===== > > - You could, if you liked, write > > URL: https://github.com/intel/%{githubname} > Source0: > https://github.com/intel/%{githubname}/archive/v%{version}/%{name}- > %{version}.tar.gz > > more concisely as > > URL: https://github.com/intel/%{githubname} > Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz > I changed to this format as it does look more concise :) Latest build: SPEC: https://download.copr.fedorainfracloud.org/results/zm627/qatzip/fedora-34-x86_64/02329873-qatzip/qatzip.spec SRPM: https://download.copr.fedorainfracloud.org/results/zm627/qatzip/fedora-34-x86_64/02329873-qatzip/qatzip-1.0.5-1.fc34.src.rpm And I have some questions here: 1. Is there any mapping of versions between Fedora and Redhat? For example the fc33 -> rehl 8.0 ? Our team is preparing to make it included in Redhat 9.0 , so I'd like to ask which version of Fedora should this rpm package be included. Thanks again for your review, Ben! Zheng
> Latest build: Thanks. I’ll re-review this as soon as I have a chance. > And I have some questions here: > 1. Is there any mapping of versions between Fedora and Redhat? For example the fc33 -> rehl 8.0 ? > Our team is preparing to make it included in Redhat 9.0 , so I'd like to ask which version of Fedora should this rpm package be included. I’m a volunteer, not a Red Hatter, so I can’t give you a definitive answer. It’s my understanding that RHEL 8 was branched from Fedora 28, and that RHEL 9 is being developed via ELN (Enterprise Linux Next), https://fedoraproject.org/wiki/Changes/ELN_Buildroot_and_Compose, which at the moment seems to be still tracking Fedora 35/Rawhide. (Of course, only selected Fedora packages are included.) That said, you might as well build the package for the current release, F34, since the qatlib dependency is available there. If you have detailed questions about ELN and the RHEL 9 development process, I recommend contacting Stephen Gallagher (https://accounts.fedoraproject.org/user/sgallagh/), or directing a question at him via NEEDINFO.
Thank you, Ben and thanks for your information about the versions! I'll refer to Stephen for help if needed!
Package approved. Thanks for taking the time to work through all the details. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== 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: "BSD (3 clause)", "Unknown or generated", "GNU General Public License, Version 2", "BSD (3 clause) GNU General Public License, Version 2". 4 files have unknown license. Detailed output of licensecheck in /home/reviewer/1955394-qatzip/licensecheck.txt GPLv2 license applies only to config_file/, which does not contribute to the build and is intentionally not installed. [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. [-]: Package is not known to require an ExcludeArch tag. Package is ExclusiveArch, correctly justified with a comment. Please remember to file the necessary Bugzilla issues after import. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 215040 bytes in 1 files. Documentation is small enough that including it in the -devel package is acceptable. [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 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]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in qatzip- libs , qatzip-devel [?]: 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. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [-]: %check is present and all tests pass. A comment properly explains why tests cannot be run. [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]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Package should compile and build into binary rpms on all supported architectures. [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: qatzip-1.0.5-1.fc35.x86_64.rpm qatzip-libs-1.0.5-1.fc35.x86_64.rpm qatzip-devel-1.0.5-1.fc35.x86_64.rpm qatzip-debuginfo-1.0.5-1.fc35.x86_64.rpm qatzip-debugsource-1.0.5-1.fc35.x86_64.rpm qatzip-1.0.5-1.fc35.src.rpm qatzip.x86_64: W: name-repeated-in-summary C QATzip qatzip.x86_64: W: spelling-error %description -l en_US gzip -> zip, grip, g zip qatzip-libs.x86_64: W: no-documentation qatzip-devel.x86_64: W: spelling-error Summary(en_US) libqatzip -> libation qatzip.src: W: name-repeated-in-summary C QATzip qatzip.src: W: spelling-error %description -l en_US gzip -> zip, grip, g zip qatzip.src:52: W: configure-without-libdir-spec 6 packages and 0 specfiles checked; 0 errors, 7 warnings. Rpmlint (debuginfo) ------------------- Checking: qatzip-debuginfo-1.0.5-1.fc35.x86_64.rpm qatzip-libs-debuginfo-1.0.5-1.fc35.x86_64.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://github.com/intel/QATzip/archive/v1.0.5/qatzip-1.0.5.tar.gz : CHECKSUM(SHA256) this package : 32c4aeac5541fcb6be2940172e5ab0738babf0f768fe808b7ec20c6651423c8b CHECKSUM(SHA256) upstream package : 32c4aeac5541fcb6be2940172e5ab0738babf0f768fe808b7ec20c6651423c8b Requires -------- qatzip (rpmlib, GLIBC filtered): glibc ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libqat.so.0()(64bit) libusdm.so.0()(64bit) libz.so.1()(64bit) libz.so.1(ZLIB_1.2.2)(64bit) rtld(GNU_HASH) qatzip-libs (rpmlib, GLIBC filtered): glibc ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libqat.so.0()(64bit) libusdm.so.0()(64bit) libz.so.1()(64bit) libz.so.1(ZLIB_1.2.2)(64bit) rtld(GNU_HASH) qatzip-devel (rpmlib, GLIBC filtered): qatzip-libs(x86-64) qatzip-debuginfo (rpmlib, GLIBC filtered): qatzip-debugsource (rpmlib, GLIBC filtered): Provides -------- qatzip: qatzip qatzip(x86-64) qatzip-libs: libqatzip.so.1()(64bit) qatzip-libs qatzip-libs(x86-64) qatzip-devel: qatzip-devel qatzip-devel(x86-64) qatzip-debuginfo: debuginfo(build-id) qatzip-debuginfo qatzip-debuginfo(x86-64) qatzip-debugsource: qatzip-debugsource qatzip-debugsource(x86-64) Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -b 1955394 Buildroot used: fedora-rawhide-x86_64 Active plugins: C/C++, Generic, Shell-api Disabled plugins: Python, Haskell, Ruby, SugarActivity, PHP, fonts, Perl, R, Java, Ocaml Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
(In reply to Ben Beasley from comment #25) > Package approved. Thanks for taking the time to work through all the details. Great! Thanks a lot Ben and Neal ! We can't make it without your help. And besides, may I ask what should I do next to finish the inclusion process and close the ticket? Zheng
Assuming you are already a Fedora packager, please see https://fedoraproject.org/wiki/New_package_process_for_existing_contributors. If you still need to be sponsored, please see https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join.
Thanks a lot Ben! Hi Pragyan, Would you like to help me with the sponsor thing? Do we have to be sponsored by a certain person or group? Thanks
Hi Ben, May I ask if you would like to sponsor me to be a package maintainer? Thanks a lot!
A request for qatzip git repo request was opened just now: https://pagure.io/releng/fedora-scm-requests/issue/35896
See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_trademarks_in_summary_or_description I see the ® mark is no longer in the spec file, but it is in the title of this bugzilla and hence in the summary in this request: https://pagure.io/releng/fedora-scm-requests/issue/35896 Consider removing it (you should be able to edit the ticket).
(In reply to Miro Hrončok from comment #31) > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_trademarks_in_summary_or_description > > I see the ® mark is no longer in the spec file, but it is in the title of > this bugzilla and hence in the summary in this request: > https://pagure.io/releng/fedora-scm-requests/issue/35896 > > Consider removing it (you should be able to edit the ticket). Thanks Miro! I removed it from the title and updated the fedora git repo request. Zheng
Thanks for catching the ® in the summary, Miro. ----- Zheng, I am willing to sponsor you as a packager based on your responses to feedback in this issue, especially combined with the fact that you are an upstream maintainer for qatzip. As sponsor, I’m available to help answer questions about packaging and Fedora processes and policies. You can read about sponsor responsibilities here (https://docs.fedoraproject.org/en-US/fesco/Packager_sponsor_responsibilities/). I’ll also keep an eye on your Bugzilla activity and on the qatzip package to see if there is anything I need to help with. No amount of mentoring is a substitute for spending some time carefully reading the packaging guidelines, especially if you plan to submit more packages for review. A lot of details therein will make more sense after having worked through this review. Before I sponsor you, can you please make sure that you have joined at least the devel-announce mailing list (https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Join_the_important_Mailing_Lists) and have sent a self-introduction on the devel mailing list (https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Introduce_yourself)? Also, please make sure you have added the correct RHBZ (Red Hat Bugzilla) email, zheng.ma, to https://accounts.fedoraproject.org/user/zm627/. This is important; your git repo request will be automatically rejected without it.
(In reply to Ben Beasley from comment #33) Thanks a lot Ben! It's so great that you're willing to sponsor me to be a packager! I've just joined the mailing list and sent out the self introduction to our community :) User name and email address are confirmed correct. Zheng
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/qatzip
Hi Ben, Thanks for your help! Finally the repo is created and I can commit to the repo. I created the exclusive arch ticket: https://bugzilla.redhat.com/show_bug.cgi?id=1987280, and then made changes to the spec: https://download.copr.fedorainfracloud.org/results/zm627/qatzip/fedora-34-x86_64/02352668-qatzip/qatzip.spec However, I think here's a problem... I pushed the commit which is without the comment line of exclusive arch ticket. But the git source https://src.fedoraproject.org/rpms/qatzip does not accept the force update. I found ways to solve the problem, but would you like to give me some suggestions? 1. git revert 2. change the repo hook setting to accept force update: https://src.fedoraproject.org/rpms/qatzip/settings#hooks-tab (This does not work for me. Maybe it takes time to take effect?) 3. ask admin to clean up the package repo Zheng
(In reply to zm627 from comment #22) > And I have some questions here: > 1. Is there any mapping of versions between Fedora and Redhat? For example > the fc33 -> rehl 8.0 ? > Our team is preparing to make it included in Redhat 9.0 , so I'd like to > ask which version of Fedora should this rpm package be included. Hello, Zheng, RHEL9 is currently targeted to branch off Fedora 35 indeed. So, it would be great to build your package for the Rawhide and an active Fedora release, these being Fedora 35 (Rawhide) and 34. I've also run rpmlint with a .spec file, it says: qatzip.spec:52: W: configure-without-libdir-spec A configure script is run without specifying the libdir. configure options must be augmented with something like --libdir=%{_libdir} whenever the script supports it. Indeed, ./configure without --libdir= is called on line 52. Probably, this worth looking at. > 1. git revert > 2. change the repo hook For this I would just made necessary changes and pushed them as another commit without bumping a release and without updating a changelog.
> I've also run rpmlint with a .spec file, it says: > > qatzip.spec:52: W: configure-without-libdir-spec > A configure script is run without specifying the libdir. configure options > must be augmented with something like --libdir=%{_libdir} whenever the script > supports it. > > Indeed, ./configure without --libdir= is called on line 52. Probably, this worth > looking at. This diagnostic is intended for configure scripts that are generated by autoconf, or those that have compatible options. This configure script is hand-written with its own idiosyncratic options, so > --sharedlib-dir=%{_libdir} does the same job. See also https://bugzilla.redhat.com/show_bug.cgi?id=1955394#c4. ----- > For this I would just made necessary changes and pushed them as another commit > without bumping a release and without updating a changelog. I agree. In general, you can feel free to create arbitrary branches and rewrite history in a fork on https://src.fedoraproject.org/, but in the main dist-git repo you should expect to use only the release branches (rawhide/main, f35, f34, …), and to have to live with anything you push there forever. If you make a mistake, bump the release if you’ve built the package with the existing release number, fix the problem, and move forward.
(In reply to Vladis Dronov from comment #37) (In reply to Ben Beasley from comment #38) > For this I would just made necessary changes and pushed them as another > commit > without bumping a release and without updating a changelog. Thanks Valdis and Ben! But here's a simple question.. Should I bump the release number and add changelog to it? I saw different opinions from your suggestions. I prefer not to change bump the release number and changelog for the reason it's not an updated lease. (I did the 'fedpkg build' but not the 'fedpkg update') But I DO remember that the guideline told me to bump the release number once a build is failed. This confuses me. And another question is that should the release branch (eg f34) be exactly the same with the rawhide branch? I mean in both the commit and source code. It's certainly they can have different source code and git commits I think. Thanks a lot!
> But I DO remember that the guideline told me to bump the release number once a build is failed. > This confuses me. I would have said that this is not required, as the purpose of the release number is to correctly order builds. If you can find where in the documentation it says that the release should be bumped after a failed build, I’m very interested to see it. HOWEVER… if you have done a successful build at all (fedpkg build), other than a scratch build (fedpkg scratch-build, koji build --scratch), now you do need to bump the release. It doesn’t matter if you created an update from the build or not. So since you built qatzip-1.0.5-1.fc35 (https://koji.fedoraproject.org/koji/packageinfo?packageID=34264), you *will* need to bump the release for any changes. You should also be aware that when you build for Rawhide, an update is automatically created. See https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/#_rawhide. The build enters the Koji buildroot used for building new RPMs rather promptly, and then is included in the daily “compose” to produce the DNF repositories for Rawhide. Sometimes the compose is broken because some QA test fails, and it takes a few days for one to succeed. You can use packages not yet in the latest compose in mock builds with “fedpkg mockbuild --enablerepo=local”. It never hurts to bump the release if in doubt. It’s not really meaningful to users; it just needs to keep increasing to ensure that newer builds are always upgrades.
> And another question is that should the release branch (eg f34) be exactly the same with the rawhide branch? I mean in both the commit and source code. In general, no and no. The Rawhide branch will move ahead of the stable releases, due to at least the following: - Updates that would create breaking changes or otherwise should not go to a stable release (https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/#stable-releases)—so, yes, it is very common to have a newer upstream version in a newer release. - Mass rebuilds for upcoming release branches (this just happened for F35) or other system-wide changes (Python 3.10, for example); these add a changelog message and bump the release. - Changes by provenpackagers—these are supposed to be done with restraint, but are sometimes needed for accepted Change proposals or to apply an urgent fix when the maintainer is not available. Depending on your choices as maintainer, the various branches may also diverge. Some maintainers feel very strongly that the git history should be linear, with stable releases possibly “behind,” but maintaining the ability to fast-forward merge from Rawhide. These maintainers tend to use version conditionals in Rawhide to accommodate older releases. This approach can be convenient, but tends to break down when also maintaining much older versions for EPEL, especially when upstream issues bug fixes to old major versions or packaging practices have changed a lot over time, as they have where Python is involved. Others feel very strongly that stable release branches should not have anything “irrelevant” merged into their history. For example, these maintainers would never merge a changelog about a Fedora 35 mass rebuild into the Fedora 34 release branch as part of a version update. Instead, they would cherry-pick changes and keep each branch as clean as possible. Each branch is allowed to actually “branch”, or have its own history diverging from the others. Using this approach sometimes means being a little more careful about versioning; see https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_you_need_to_change_an_old_branch_without_rebuilding_the_others. Personally, I don’t have strong feelings about other approach. I tend to let my release branches diverge freely rather than using conditional macros in my spec files, and I’ve moved more in this direction over time—as someone who maintains a lot of Python packages and a lot of packages with EPEL branches, it’s proved to make more sense for me. However, I’m not very strict about changelog hygiene, and will still use fast-forward merges where it seems to make sense. There is no policy or community consensus about which approach is “correct.” Maintainers generally choose freely in their own packages depending on their personal philosophies, the realities of their particular packages, and their comfort level with git.
(In reply to Ben Beasley from comment #38) > This configure script is > hand-written with its own idiosyncratic options, so > > > --sharedlib-dir=%{_libdir} > > does the same job. See also https://bugzilla.redhat.com/show_bug.cgi?id=1955394#c4. Thank a ton for a clarification, Ben!
(In reply to Ben Beasley from comment #40) > > But I DO remember that the guideline told me to bump the release number once a build is failed. > > This confuses me. > > I would have said that this is not required, as the purpose of the release > number is to correctly order builds. If you can find where in the > documentation it says that the release should be bumped after a failed > build, I’m very interested to see it. Thanks a lot for your comment, Ben!! It really helps a lot. Here's the doc I refer to. https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Update_Your_Branches_.28if_desired.29 - If it fails to build, the build system will send you an email to report the failure and show you to the logs. Commit any needed changes to git, bump the SPEC release number, and request a new build. And if I'm not mistaken, I think I should simply bump the release number on the rawhide branch and add a line to changelog, then request a new build for the rawhide branch.
> And if I'm not mistaken, I think I should simply bump the release number on the rawhide branch and add a line to changelog, then request a new build for the rawhide branch. Sounds good to me.
Hi Ben, Would you mind me to close the ticket, since we have uploaded the package? Thank you very much for the great help with this package review process!! Hope to work with you again! Zheng
> Would you mind me to close the ticket, since we have uploaded the package? Sure! I have done so. You should have been able to do this yourself, if you liked. > Thank you very much for the great help with this package review process!! You’re welcome! Please note that, having sponsored you, I remain your sponsor until/unless you become a sponsor yourself. I’ll try to keep an eye on your packaging and offer advice when I can, but please feel free to ask questions directly or set NEEDINFO for me in an issue even if it wouldn’t otherwise have anything to do with me. Please consider reading this comment (https://bugzilla.redhat.com/show_bug.cgi?id=1908526#c21), where I explained how to use Bodhi to create updates for a stable release, and let me know if you have any questions about adding qatzip to Fedora 34. This, plus requesting a git branch for F34, is also covered in https://fedoraproject.org/wiki/New_package_process_for_existing_contributors. Please also take a look at https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/, and note that F35 is scheduled to be branched from Rawhide in just a few days, on August 10.