[Bug 2005989] Review Request: liborc- Apache ORC library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=2005989

Kaleb KEITHLEY <kkeithle@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(kkeithle@redhat.c |needinfo?(misc@xxxxxxxx)
                   |om)                         |



--- Comment #12 from Kaleb KEITHLEY <kkeithle@xxxxxxxxxx> ---
thanks for reviewing.

tl;dnr. Updated .spec and src.rpm at
https://kkeithle.fedorapeople.org/liborc-20220131/

see below for individual responses inline.


(In reply to Michael S. from comment #11)
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> Issues:
> =======
> - Sources used to build the package match the upstream source, as provided
>   in the spec URL.
>   Note: Upstream MD5sum check error, diff is in
>   /home/fedora/2005989-liborc/diff.txt
>   See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
> 
> - Directory %{_libdir}/cmake/orc/ is unowned (same for /usr/include/orc)
> 
> - Sources need to be verified with gpgverify
> 
> - Latest version is 1.7.2
> 
> - Patch should be sent upstream (or have a comment)
> 
> - debuginfo is disabled, should be explained why
> 
> - package is 64 bits only, but that's a upstream issue, just mentioning for
> the record
> 
> ===== 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]: 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", "*No copyright* Apache License 2.0",
>      "Apache License 2.0". 1223 files have unknown license. Detailed output
>      of licensecheck in /home/fedora/2005989-liborc/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/cmake/orc, /usr/include/orc
> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/lib64/cmake/orc,
>      /usr/include/orc

fixed

> [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.
> [x]: 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.
> [x]: Package contains systemd file(s) if in need.
> [!]: Useful -debuginfo package or justification otherwise.

fixed

> [!]: Package is not known to require an ExcludeArch tag.

package does not build on 32-bit arches. There is a big comment addressing that
at lines 9-12 of the .spec

> [x]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 10240 bytes in 2 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 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]: 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 liborc1
>      , liborc-devel
> [-]: Package functions as described.
> [!]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [!]: Patches link to upstream bugs/comments/lists or are otherwise
>      justified.

?? I don't understand this. The one patch contains the changes to the source
— i.e. the cmake files — necessary to build without also downlloading and
building bundled (vendored) dependencies from the downloaded sources.

As we know, downloading during a mock build (in koji or otherwise) doesn't
work.


> [!]: 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.

It does. See comment about ExcludeArch

> [-]: %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]: 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:
> [!]: Spec file according to URL is the same as in SRPM.
>      Note: Bad spec filename: /home/fedora/2005989-liborc/srpm-
>      unpacked/liborc.spec
>      See: (this test has no URL)

???

> [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.
> 
> 
> Rpmlint
> -------
> Checking: liborc1-1.6.6-1.fc36.x86_64.rpm
>           liborc-devel-1.6.6-1.fc36.x86_64.rpm
>           liborc-1.6.6-1.fc36.src.rpm
> liborc1.x86_64: W: spelling-error %description -l en_US liborc -> liberator
> liborc1.x86_64: W: description-shorter-than-summary
> liborc1.x86_64: W: unstripped-binary-or-object /usr/lib64/liborc.so.1.6.6
> liborc-devel.x86_64: W: no-dependency-on liborc/liborc-libs/libliborc

It does have a dependency on liborc1, at line 57 of the .spec. That's the 
subpackage with liborc.so.* in it. If it's preferable to have the dependency
on the parent, wrapper package (liborc) we can do that too.

> liborc-devel.x86_64: W: no-documentation
> liborc.src: W: spelling-error %description -l en_US pushdown -> push down,
> push-down, splashdown
> liborc.src: W: spelling-error %description -l en_US structs -> struts,
> destructs, obstructs
> liborc.src: W: file-size-mismatch orc-1.6.6.tar.gz = 13939853,
> https://archive.apache.org/dist/orc/orc-1.6.6/orc-1.6.6.tar.gz = 14048870

fixed. Size difference is due to the tarball from the src.rpm in apache's
package repo which untars into .../orc-rel-release-1.6.6/*. I have replaced
that tarball with the tarball from the apache archive. (Which apart from the 
dirname has exactly the same contents according to diff -ur ...)

> 3 packages and 0 specfiles checked; 0 errors, 8 warnings.
> 
> 
> 
> 
> Rpmlint (installed packages)
> ----------------------------
> Cannot parse rpmlint output:
> 
> 
> Source checksums
> ----------------
> https://archive.apache.org/dist/orc/orc-1.6.6/orc-1.6.6.tar.gz :
>   CHECKSUM(SHA256) this package     :
> 9a92b3eaad1d8b88a18cee41058feb34eb43d919df363e0b74a0b857724e4ba0
>   CHECKSUM(SHA256) upstream package :
> 93d2e5f7c9f76ea5cdf29073c73a00a37c54281a5c02bcc6395f1cf521ea246c
> diff -r also reports differences
> 
> 
> Requires
> --------
> liborc1 (rpmlib, GLIBC filtered):
>     libc.so.6()(64bit)
>     libgcc_s.so.1()(64bit)
>     libgcc_s.so.1(GCC_3.0)(64bit)
>     libgcc_s.so.1(GCC_3.3.1)(64bit)
>     liblz4.so.1()(64bit)
>     libm.so.6()(64bit)
>     libprotobuf.so.30()(64bit)
>     libsnappy.so.1()(64bit)
>     libstdc++.so.6()(64bit)
>     libstdc++.so.6(CXXABI_1.3)(64bit)
>     libz.so.1()(64bit)
>     libzstd.so.1()(64bit)
>     rtld(GNU_HASH)
> 
> liborc-devel (rpmlib, GLIBC filtered):
>     cmake-filesystem(x86-64)
>     liborc.so.1()(64bit)
>     liborc1(x86-64)
> 
> 
> 
> Provides
> --------
> liborc1:
>     liborc.so.1()(64bit)
>     liborc1
>     liborc1(x86-64)
> 
> liborc-devel:
>     cmake(orc)
>     liborc-devel
>     liborc-devel(x86-64)
> 
> 
> 
> Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10
> Command line :/usr/bin/fedora-review -b 2005989
> Buildroot used: fedora-rawhide-x86_64
> Active plugins: C/C++, Shell-api, Generic
> Disabled plugins: R, fonts, Python, SugarActivity, Haskell, Java, Ocaml,
> Perl, PHP
> Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2005989
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux