Bug 1391950 - Review Request: ocaml-ocamlbuild - Build tool for OCaml libraries and programs
Summary: Review Request: ocaml-ocamlbuild - Build tool for OCaml libraries and programs
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: gil cattaneo
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-11-04 13:04 UTC by Richard W.M. Jones
Modified: 2016-11-04 22:53 UTC (History)
2 users (show)

Fixed In Version: ocaml-ocamlbuild-0.9.3-5.fc26
Clone Of:
Environment:
Last Closed: 2016-11-04 22:53:48 UTC
Type: ---
Embargoed:
puntogil: fedora-review+


Attachments (Terms of Use)

Description Richard W.M. Jones 2016-11-04 13:04:53 UTC
Spec URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild.spec
SRPM URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild-0.9.3-1.fc25.src.rpm
Description: Build tool for OCaml libraries and programs
Fedora Account System Username: rjones

Comment 1 Richard W.M. Jones 2016-11-04 13:06:42 UTC
rpmlint output is:

ocaml-ocamlbuild.x86_64: W: only-non-binary-in-usr-lib

This is a problem in rpmlint.

ocaml-ocamlbuild.x86_64: W: no-manual-page-for-binary ocamlbuild.native
ocaml-ocamlbuild.x86_64: W: no-manual-page-for-binary ocamlbuild.byte
ocaml-ocamlbuild.x86_64: W: no-manual-page-for-binary ocamlbuild

There is no upstream man page, but there is comprehensive documentation in
the -doc subpackage.

ocaml-ocamlbuild-debuginfo.x86_64: E: debuginfo-without-sources

This is a problem but seems to be a bug in RPM.  In any case I
was able to run 'gdb' on ocamlbuild and see references to
files/lines in the source so I guess the debuginfo package is
useful and shouldn't be completely suppressed.

Comment 2 Richard W.M. Jones 2016-11-04 13:07:41 UTC
I should say that ocamlbuild was previously part of the ocaml package,
but in OCaml >= 4.03 it has been spun out into a new package, hence
the need to add a package to Fedora.

Comment 3 Richard W.M. Jones 2016-11-04 13:10:20 UTC
It's not possible to do a scratch build in Koji, because it needs
OCaml 4.04 which we're in the process of rebuilding.

Comment 4 Richard W.M. Jones 2016-11-04 13:40:38 UTC
I have compiled this package successfully on x86_64, aarch64, ppc64
and ppc64le.

You will need a scratch build of OCaml 4.04 in order to build it, see:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16286440

Comment 5 gil cattaneo 2016-11-04 18:49:57 UTC
have time for review this https://bugzilla.redhat.com/show_bug.cgi?id=1390156 ?

Comment 6 gil cattaneo 2016-11-04 18:53:48 UTC
you should use %license LICENSE instead of %doc LICENSE in each (sub) packages

Comment 7 gil cattaneo 2016-11-04 18:55:32 UTC
other issues: "Requires:      %{name} = %{version}-%{release}"
please use "Requires:      %{name}%{?_isa} = %{version}-%{release}"

Comment 8 Richard W.M. Jones 2016-11-04 19:38:22 UTC
Spec URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild.spec
SRPM URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild-0.9.3-3.fc25.src.rpm

The updates are:

 - Don't package the META file.  This is replaced by a corrected META
   file by the ocaml-ocamlfind package.

 - Use %license

 - Add %{?_isa}

Comment 9 gil cattaneo 2016-11-04 19:43:04 UTC
Package Review
==============

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


Issues:
=======
- 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.
  Note: License file LICENSE is not marked as %license
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[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.
[?]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GPL (unversioned/unknown version)", "Unknown or generated".
     232 files have unknown license. Detailed output of licensecheck in
     /home/gil/1391950-ocaml-ocamlbuild/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/lib/ocaml/ocamlbuild(ocaml)
[x]: %build honors applicable compiler flags or justifies otherwise.
[?]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[?]: 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.
[!]: Useful -debuginfo package or justification otherwise.
 ocaml-ocamlbuild-debuginfo.i686: E: debuginfo-without-sources
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 163840 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]: 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]: Packages must not store files under /srv, /opt or /usr/local

Ocaml:
[x]: This should never happen

===== 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).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in ocaml-
     ocamlbuild-devel , ocaml-ocamlbuild-doc , ocaml-ocamlbuild-debuginfo
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: 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]: Uses parallel make %{?_smp_mflags} macro.
[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
-------
Checking: ocaml-ocamlbuild-0.9.3-1.fc26.i686.rpm
          ocaml-ocamlbuild-devel-0.9.3-1.fc26.i686.rpm
          ocaml-ocamlbuild-doc-0.9.3-1.fc26.i686.rpm
          ocaml-ocamlbuild-debuginfo-0.9.3-1.fc26.i686.rpm
          ocaml-ocamlbuild-0.9.3-1.fc26.src.rpm
ocaml-ocamlbuild.i686: W: only-non-binary-in-usr-lib
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild.byte
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild.native
ocaml-ocamlbuild-debuginfo.i686: E: debuginfo-without-sources
5 packages and 0 specfiles checked; 1 errors, 4 warnings.




Rpmlint (debuginfo)
-------------------
Checking: ocaml-ocamlbuild-debuginfo-0.9.3-1.fc26.i686.rpm
ocaml-ocamlbuild-debuginfo.i686: E: debuginfo-without-sources
1 packages and 0 specfiles checked; 1 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
ocaml-ocamlbuild-debuginfo.i686: E: debuginfo-without-sources
ocaml-ocamlbuild.i686: W: only-non-binary-in-usr-lib
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild.native
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild.byte
4 packages and 0 specfiles checked; 1 errors, 4 warnings.

./ocamlbuild-0.9.3/man/ocamlbuild.1

Requires
--------
ocaml-ocamlbuild-debuginfo (rpmlib, GLIBC filtered):

ocaml-ocamlbuild-devel (rpmlib, GLIBC filtered):
    ocaml-ocamlbuild

ocaml-ocamlbuild-doc (rpmlib, GLIBC filtered):

ocaml-ocamlbuild (rpmlib, GLIBC filtered):
    /usr/bin/ocamlrun
    libc.so.6
    libdl.so.2
    libm.so.6
    ocaml(Arg)
    ocaml(Array)
    ocaml(Buffer)
    ocaml(Bytes)
    ocaml(CamlinternalFormatBasics)
    ocaml(CamlinternalLazy)
    ocaml(Char)
    ocaml(Digest)
    ocaml(Filename)
    ocaml(Format)
    ocaml(Hashtbl)
    ocaml(Lazy)
    ocaml(Lexing)
    ocaml(List)
    ocaml(Map)
    ocaml(Ocamlbuild_pack)
    ocaml(Ocamlbuild_unix_plugin)
    ocaml(Pervasives)
    ocaml(Printexc)
    ocaml(Printf)
    ocaml(Queue)
    ocaml(Scanf)
    ocaml(Set)
    ocaml(String)
    ocaml(Sys)
    ocaml(Unix)
    ocaml(runtime)
    rtld(GNU_HASH)



Provides
--------
ocaml-ocamlbuild-debuginfo:
    ocaml-ocamlbuild-debuginfo
    ocaml-ocamlbuild-debuginfo(x86-32)

ocaml-ocamlbuild-devel:
    ocaml-ocamlbuild-devel
    ocaml-ocamlbuild-devel(x86-32)

ocaml-ocamlbuild-doc:
    ocaml-ocamlbuild-doc
    ocaml-ocamlbuild-doc(x86-32)

ocaml-ocamlbuild:
    ocaml(Ocamlbuild)
    ocaml(Ocamlbuild_executor)
    ocaml(Ocamlbuild_pack)
    ocaml(Ocamlbuild_plugin)
    ocaml(Ocamlbuild_unix_plugin)
    ocaml-ocamlbuild
    ocaml-ocamlbuild(x86-32)



Source checksums
----------------
https://github.com/ocaml/ocamlbuild/archive/0.9.3.tar.gz#/ocaml-ocamlbuild-0.9.3.tar.gz :
  CHECKSUM(SHA256) this package     : 32e4824906888c61244909eab0d2c22d31f18fc9579873a070a4cf7947c2c0a9
  CHECKSUM(SHA256) upstream package : 32e4824906888c61244909eab0d2c22d31f18fc9579873a070a4cf7947c2c0a9


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1391950 --plugins Ocaml -m fedora-rawhide-i386 -L ~/deps
Buildroot used: fedora-rawhide-i386
Active plugins: Generic, Shell-api, Ocaml, C/C++
Disabled plugins: Java, Python, SugarActivity, fonts, Haskell, Perl, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Built with local dependencies:
    ~/deps/ocaml-compiler-libs-4.04.0-0.1.beta2.fc26.i686.rpm
    ~/deps/ocaml-4.04.0-0.1.beta2.fc26.i686.rpm
    ~/deps/ocaml-runtime-4.04.0-0.1.beta2.fc26.i686.rpm

Comment 10 gil cattaneo 2016-11-04 19:48:00 UTC
Issues:

- 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.

[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in ocaml-
     ocamlbuild-devel , ocaml-ocamlbuild-doc , ocaml-ocamlbuild-debuginfo

Fixed in ocaml-ocamlbuild-0.9.3-3


ocaml-ocamlbuild-debuginfo.i686: E: debuginfo-without-sources

Other:
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild.native
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild
ocaml-ocamlbuild.i686: W: no-manual-page-for-binary ocamlbuild.byte

a man page is available here ./ocamlbuild-0.9.3/man/ocamlbuild.1
can you install it?

Comment 11 Richard W.M. Jones 2016-11-04 20:01:25 UTC
I should note that:

[?]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by:
     /usr/lib/ocaml/ocamlbuild(ocaml)

is expected, because ocamlbuild is currently part of ocaml.

Comment 12 Richard W.M. Jones 2016-11-04 20:02:33 UTC
[!]: Useful -debuginfo package or justification otherwise.
 ocaml-ocamlbuild-debuginfo.i686: E: debuginfo-without-sources

I'd prefer to leave the debuginfo alone for now until we work
out why (across all OCaml builds) debuginfo doesn't include
source files.

Comment 13 Richard W.M. Jones 2016-11-04 20:05:53 UTC
Spec URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild.spec
SRPM URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild-0.9.3-4.fc25.src.rpm

This changes:

 - Packages up ocamlbuild(1) man page.

 - Also adds the Requires dependency to the -doc subpackage.
   This is not strictly needed because in theory you could install the
   doc package on its own, but it seems better to have the dependency.
   One important reason is so that uninstalling ocaml-ocamlbuild also
   uninstalls ocaml-ocamlbuild-doc.

Comment 14 gil cattaneo 2016-11-04 20:14:10 UTC
(In reply to Richard W.M. Jones from comment #13)
> Spec URL:
> http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild.spec
> SRPM URL:
> http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild-0.9.3-4.
> fc25.src.rpm
> 
> This changes:
> 
>  - Packages up ocamlbuild(1) man page.
> 
>  - Also adds the Requires dependency to the -doc subpackage.
>    This is not strictly needed because in theory you could install the
>    doc package on its own, but it seems better to have the dependency.
>    One important reason is so that uninstalling ocaml-ocamlbuild also
>    uninstalls ocaml-ocamlbuild-doc.

Some problems "Connect to oirase.annexia.org (oirase.annexia.org) | 81.187.83.230 |: 80 ... failed: Network is unreachable."

Approved

Comment 15 gil cattaneo 2016-11-04 20:15:23 UTC
Please use "install -pm 0644 man/ocamlbuild.1 $RPM_BUILD_ROOT%{_mandir}/man1/"
for preserve file timestamp

Comment 16 Richard W.M. Jones 2016-11-04 21:25:43 UTC
Spec URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild.spec
SRPM URL: http://oirase.annexia.org/reviews/ocaml-ocamlbuild/ocaml-ocamlbuild-0.9.3-5.fc25.src.rpm

That fixes everything, thanks for the approval.

Comment 17 Gwyn Ciesla 2016-11-04 21:38:51 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/ocaml-ocamlbuild


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