[Bug 836014] Review Request: templates_parser - template library from AWS

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

 



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

Björn Persson <bjorn@xxxxxxxxxxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|lemenkov@xxxxxxxxx          |bjorn@xxxxxxxxxxxxxxxxxxxx
              Flags|                            |needinfo?(julian@xxxxxxx)

--- Comment #10 from Björn Persson <bjorn@xxxxxxxxxxxxxxxxxxxx> ---
Peter says he doesn't mind if I take over this ticket, so here's my formal
review:


Generic MUST Items:

· rpmlint must be run on the source rpm and all binary rpms the build produces.

  templates_parser.src: W: invalid-url Source0: templates_parser-11.6.0.tar.xz
  templates_parser-debuginfo.x86_64: W: hidden-file-or-dir
/usr/src/debug/templates_parser-11.6.0/.build
  templates_parser-debuginfo.x86_64: W: hidden-file-or-dir
/usr/src/debug/templates_parser-11.6.0/.build
  templates_parser-tools.x86_64: W: executable-stack /usr/bin/templates2ada
  templates_parser-tools.x86_64: W: no-manual-page-for-binary templates2ada
  templates_parser-tools.x86_64: W: no-manual-page-for-binary templatespp
  templates_parser.x86_64: W: executable-stack
/usr/lib64/templates_parser/libtemplates_parser-11.6.0.so
  templates_parser.x86_64: W: no-documentation

  · There is no URL to the upstream source because it was taken from Git.
  · The hidden directory in the debuginfo package is odd, but not something a
packager should be required to change. 
  · Executable stack is OK as noted in the Ada packaging guidelines.
  · There are no documentation files to include in the base package. (README
contains only installation instructions.)

  About man pages, see the separate point below. None of the other warnings are
blocking issues.

· The package must be named according to the Package Naming Guidelines.
  → OK.

· The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption.
  → OK. The names match.

· The package must meet the Packaging Guidelines.
  → OK.

· The package must be licensed with a Fedora approved license and meet the
Licensing Guidelines.
  → OK. The license is GPLv2+ with exceptions (GMGPL) according to the source
file headers.

· The License field in the package spec file must match the actual license.
  → OK.

· 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 must be included in %doc.
  → N/A. There is no separate license file.

· The spec file must be written in American English.
  → OK. The grammar isn't perfect but it's comprehensible.

· The spec file for the package MUST be legible.
  → OK.

· The sources used to build the package must match the upstream source, as
provided in the spec URL.
  → OK. The contents of the tarball are identical to what I got from upstream
Git.

· The package MUST successfully compile and build into binary rpms on at least
one primary architecture.
  → OK. It builds in Koji on at least x86 and x86-64.

· If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch.
  → OK. GNAT_arches is used.

· All build dependencies must be listed in BuildRequires, except for any that
are listed in the exceptions section of the Packaging Guidelines.
  → OK.

· The spec file MUST handle locales properly.
  → N/A. No translations are included.

· Every binary RPM package (or subpackage) which stores shared library files
(not just symlinks) in any of the dynamic linker's default paths, must call
ldconfig in %post and %postun.
  → OK. ldconfig is called.

· The package must NOT bundle copies of system libraries.
  → OK.

· If the package is designed to be relocatable, the packager must state this
fact in the request for review, along with the rationalization for relocation
of that specific package.
  → OK. The package isn't relocatable.

· The package must own all directories that it creates.
  → OK.

· The package must not list a file more than once in the spec file's %files
listings.
  → OK.

· Permissions on files must be set properly.
  → OK.

· The package must consistently use macros.
  → OK.

· The package must contain code, or permissable content.
  → OK. Code.

· Large documentation files must go in a -doc subpackage.
  → OK. The documentation can reasonably be considered to not be large.

· If a package includes something as %doc, it must not affect the runtime of
the application.
  → OK.

· Static libraries must be in a -static package.
  → N/A. Only shared libraries are packaged.

· Development files must be in a -devel package.
  → OK.

· In the vast majority of cases, devel packages must require the base package
using a fully versioned dependency.
  → OK.

· Packages must NOT contain any .la libtool archives.
  → OK.

· Packages containing GUI applications must include a %{name}.desktop file.
  → N/A. This isn't a GUI application.

· Packages must not own files or directories already owned by other packages.
  → OK.

· All filenames in rpm packages must be valid UTF-8.
  → OK.


Ada-specific MUST items:

· The package must have "BuildRequires: gcc-gnat".
  → OK.

· The appropriate flags macro must be used for each GNAT tool that is invoked.
  → OK. Gnatmake_optflags is used.

· If there is a need to prevent attempts to build the package on secondary
architectures where GNAT has not been bootstrapped, then this MUST be done with
"ExclusiveArch: %{GNAT_arches}". 
  → OK. GNAT_arches is used.

· The package must have "BuildRequires: fedora-gnat-project-common".
  → OK.

· Ada library packages MUST have a -devel subpackage containing all the files
that are necessary for compilation of code that uses the library.
  → OK.

· The -devel package MUST NOT contain any makefiles or other files that are
only used for recompiling the library.
  → OK.

· The -devel package MUST NOT contain any *.o files. 
  → OK.

· The -devel package MUST contain one or more GNAT project files to be imported
by other projects that use the library.
  → OK.

· Project files MUST be architecture-independent.
  → OK.

· Project files MUST NOT contain hard-coded directory names.
  → OK.

· If the "directories" project is used, then the -devel package MUST have an
explicit "Requires: fedora-gnat-project-common".
  → OK.

· Project files MUST have an Externally_Built attribute equal to "true". 
  → OK.

· Ada source files in -devel packages MUST be placed in the %{_includedir}
directory or a subdirectory thereof.
  → OK.

· Ada library information files MUST be placed in a subdirectory of %{_libdir}.
  → OK.

· GNAT projects files MUST be placed in the %{_GNAT_project_dir} directory or a
subdirectory thereof. 
  → OK.


SHOULD items:

· If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
  → NOTE: There is no separate license file. This is not required by the GPL,
but you should contact the developers and encourage them to add a license file.

· The description and summary sections in the package spec file should contain
translations for supported Non-English languages, if available.
  → No translations are included but that's OK.

· The reviewer should test that the package builds in mock.
  → OK.

· The package should compile and build into binary rpms on all supported
architectures.
  → OK. It builds on all two primary architectures.

· The reviewer should test that the package functions as described. A package
should not segfault instead of running, for example.
  → OK. The tools pass a smoke test, which implies that the library does too.

· If scriptlets are used, those scriptlets must be sane.
  → OK.

· Usually, subpackages other than devel should require the base package using a
fully versioned dependency.
  → OK. The -tools subpackage has an automatic dependency on the soname.

· The placement of pkgconfig(.pc) files depends on their usecase, and this is
usually for development purposes, so should be placed in a -devel pkg.
  → N/A. There are no pkgconfig files.

· If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin,
or /usr/sbin consider requiring the package which provides the file instead of
the file itself.
  → OK. There are no file dependencies other than /sbin/ldconfig.

· The package should contain man pages for binaries/scripts. If it doesn't,
work with upstream to add them where they make sense.
  → ISSUE: There are no man pages for templates2ada and templatespp. Debian has
man pages however, so you should look at those to see if they can be used in
Fedora. You should also try to get the man pages upstreamed.


To summarize: Add Debian's man pages to the -tools subpackage (unless you find
some big problem with them). You should also ask the developers to add the man
pages and the GPL version 2 (an updated version with the FSF's new address) to
the upstream Git repository. (The best way is probably to send them Git
patches. They told me it's OK to send patches for Templates Parser to the
aws-patches mailing list.)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]