Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1516411 - Review Request: libijs - IJS Raster Image Transport Protocol Library
Summary: Review Request: libijs - IJS Raster Image Transport Protocol Library
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Zdenek Dohnal
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-11-22 15:08 UTC by David Kaspar [Dee'Kej]
Modified: 2017-11-28 11:24 UTC (History)
1 user (show)

Fixed In Version: libijs-0.35-1.gitd26d2bb.fc28
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-11-28 11:24:23 UTC
zdohnal: fedora-review+


Attachments (Terms of Use)

Description David Kaspar [Dee'Kej] 2017-11-22 15:08:33 UTC
Spec URL: https://dkaspar.fedorapeople.org/share/reviews/libijs/libijs.spec
SRPM URL: https://dkaspar.fedorapeople.org/share/reviews/libijs/libijs-0.35-1.gitd26d2bb.fc28.src.rpm

Description: The IJS (InkJet Server) Raster Image Transport Protocol is a library, which
is no longer actively developed, and often other alternatives are used instead.

This library, however, still seem to be useful for Ghostscript application
to be able to connect to the HP IJS server to print on an HP printer.

Fedora Account System Username: dkaspar

Comment 1 David Kaspar [Dee'Kej] 2017-11-22 15:09:35 UTC
Hello Zdenek,

could you please do a Fedora Review for me? Thank you! :)

 -- Dee'Kej --

Comment 2 Zdenek Dohnal 2017-11-22 15:13:11 UTC
Hi,

sure thing.

Comment 3 Zdenek Dohnal 2017-11-24 11:37:05 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed

---ISSUES---
- no major issues, I commented it in text



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[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.

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: "MIT/X11 (BSD like)", "Unknown or generated". 19 files have
     unknown license. Detailed output of licensecheck in /root/libijs
     /review-libijs/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[?]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/include/ijs(ghostscript-
     devel)

This is expected. Ghostscript-devel owns the directory now, but 
it won't after adding libijs package.

[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).
[?]: Package is named according to the Package Naming Guidelines.

Versioning of package is in form libijs-0.35-1.gitHASH, which is 
not according FPG. Packager is aware of it - some new packages is
using this way and he thinks the versioning part of FPG is 
deprecated. Packager is willing to change it if that causes
problems for someone.

[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.
[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]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[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]: 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:
[x]: 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 libijs-
     doc , libijs-debuginfo

False positive.

[x]: Package functions as described.
[x]: Latest version is packaged.
[?]: Package does not include license text files separate from upstream.

Upstream will create mirror with own license file, this review was
done with license files added as Source, but packager agreed with 
upstream on license and license file.

[x]: Scriptlets must be sane, if used.
[?]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments

Upstream will create mirror in near future, that's why it is only
tarball right now.

[x]: 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]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[!]: Package should not use obsolete m4 macros
     Note: Some obsoleted macros found, see the attachment.
     See: https://fedorahosted.org/FedoraReview/wiki/AutoTools

This requirement is optional and package works with it. Packager
was informed about the issue.

[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: libijs-0.35-1.gitd26d2bb.fc26.x86_64.rpm
          libijs-devel-0.35-1.gitd26d2bb.fc26.x86_64.rpm
          libijs-doc-0.35-1.gitd26d2bb.fc26.noarch.rpm
          libijs-debuginfo-0.35-1.gitd26d2bb.fc26.x86_64.rpm
          libijs-0.35-1.gitd26d2bb.fc26.src.rpm
libijs.x86_64: W: no-documentation
libijs.x86_64: E: postun-without-ldconfig /usr/lib64/libijs-0.35.so
libijs.x86_64: E: non-empty-%postun /sbin/ldconfig

False positive.

libijs-devel.x86_64: W: spelling-error %description -l en_US subpackage -> sub package, sub-package, package's

Word 'subpackage' is correct.

libijs-devel.x86_64: W: spelling-error %description -l en_US pc -> PC, p, c

This is file extension, it it correct.

libijs-devel.x86_64: W: only-non-binary-in-usr-lib
libijs-devel.x86_64: W: no-documentation

Documentation is packaged separately.

libijs-doc.noarch: W: spelling-error %description -l en_US subpackage -> sub package, sub-package, package's

Same as above.

libijs.src: W: invalid-url Source0: libijs-0.35-d26d2bb.tar.xz

Upstream will create mirror in near future, that's why it is only
tarball right now.

5 packages and 0 specfiles checked; 2 errors, 7 warnings.




Rpmlint (debuginfo)
-------------------
Checking: libijs-debuginfo-0.35-1.gitd26d2bb.fc26.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
libijs-devel.x86_64: W: spelling-error %description -l en_US subpackage -> sub package, sub-package, package's

Same as above.

libijs-devel.x86_64: W: spelling-error %description -l en_US pc -> PC, p, c


libijs-devel.x86_64: W: invalid-url URL: https://ghostscript.com/ <urlopen error [Errno -2] Name or service not known>

This is expected, fedora-review tool was run in 1minutetip. URL works.

libijs-devel.x86_64: W: only-non-binary-in-usr-lib
libijs-devel.x86_64: W: no-documentation
libijs-doc.noarch: W: spelling-error %description -l en_US subpackage -> sub package, sub-package, package's

Same as above.

libijs-doc.noarch: W: invalid-url URL: https://ghostscript.com/ <urlopen error [Errno -2] Name or service not known>

Same as above.

libijs.x86_64: W: invalid-url URL: https://ghostscript.com/ <urlopen error [Errno -2] Name or service not known>

Same as above.

libijs.x86_64: W: no-documentation
libijs.x86_64: E: postun-without-ldconfig /usr/lib64/libijs-0.35.so
libijs.x86_64: E: non-empty-%postun /sbin/ldconfig
libijs-debuginfo.x86_64: W: invalid-url URL: https://ghostscript.com/ <urlopen error [Errno -2] Name or service not known>

Same as above.

4 packages and 0 specfiles checked; 2 errors, 10 warnings.



Requires
--------
libijs-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libijs(x86-64)

libijs-doc (rpmlib, GLIBC filtered):
    libijs

libijs (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libc.so.6()(64bit)
    rtld(GNU_HASH)

libijs-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
libijs-devel:
    libijs-devel
    libijs-devel(x86-64)
    pkgconfig(ijs)

libijs-doc:
    libijs-doc

libijs:
    libijs
    libijs(x86-64)
    libijs-0.35.so()(64bit)

libijs-debuginfo:
    libijs-debuginfo
    libijs-debuginfo(x86-64)



Unversioned so-files
--------------------
libijs: /usr/lib64/libijs-0.35.so

AutoTools: Obsoleted m4s found
------------------------------
  AM_PROG_LIBTOOL found in: libijs-0.35/configure.ac:114,
  libijs-0.35/configure.ac:120
  AM_CONFIG_HEADER found in: libijs-0.35/configure.ac:109


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -n libijs
Buildroot used: fedora-26-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 4 David Kaspar [Dee'Kej] 2017-11-24 12:02:21 UTC
(In reply to Zdenek Dohnal from comment #3)
> Generic:
> [?]: Package does not own files or directories owned by other packages.
>      Note: Dirs in package are owned also by: /usr/include/ijs(ghostscript-
>      devel)
> 
> This is expected. Ghostscript-devel owns the directory now, but 
> it won't after adding libijs package.

Yes. One of the reason to create this package is to de-bundle the Ghostscript package as possible, because upstream bundles a lot of other software in it. AFAICT, libijs is the last part needed to completely de-bundle the Ghostscript.

> [?]: Package is named according to the Package Naming Guidelines.
> 
> Versioning of package is in form libijs-0.35-1.gitHASH, which is 
> not according FPG. Packager is aware of it - some new packages is
> using this way and he thinks the versioning part of FPG is 
> deprecated. Packager is willing to change it if that causes
> problems for someone.

I wouldn't say exactly deprecated, but rather outdated. To clarify - the exact advised format advised by FPG is:

pkg-%{version}-%{release}.YYYYMMDDgitSHORTHASH{?dist}

I decided to omit the YYYMMDD part, because it is derived from the day of commit, which can be easily derived from the git repository and short hash, and it does not add any useful value because of it. IMHO more shorter package NVR is favorable in this matter.

NOTE: For example docker package are using the same versioning as I have used for 'libijs'.
 
> [?]: SourceX tarball generation or download is documented.
>      Note: Package contains tarball without URL, check comments
> 
> Upstream will create mirror in near future, that's why it is only
> tarball right now.

The tarball was created via git-archive from upstream's git repository. Because of their internal meetings they can't create a new Github mirror for it right now. I expect to switch the SourceX to the mirror once it is available.

> 
> ===== EXTRA items =====
> 
> Generic:
> [!]: Package should not use obsolete m4 macros
>      Note: Some obsoleted macros found, see the attachment.
>      See: https://fedorahosted.org/FedoraReview/wiki/AutoTools
> 
> This requirement is optional and package works with it. Packager
> was informed about the issue.

This software falls into 'legacy' category and is in maintenance mode only. AFAICT upstream tries not to touch it unless really necessary. IMHO fixing this issue is not worth it just for the sake of fedora-review.

-----------------

NOTE: The license files were added manually after discussion & agreement with upstream. They will add (and probably modify a little bit) the license files once the mirror is created, so these files are part of resulting tarballs.

-----------------

Zdenek, if there's nothing else wrong with the specfile, could you please provide the fedora-review+, flag? Thank you! :)

Comment 5 Zdenek Dohnal 2017-11-24 12:06:37 UTC
Here it is :)

Comment 6 Patrick Uiterwijk 2017-11-24 15:37:34 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libijs


Note You need to log in before you can comment on or make changes to this bug.