Bug 675557 - Review Request: matreshka - set of Ada libraries to help to develop information systems
Summary: Review Request: matreshka - set of Ada libraries to help to develop informati...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Björn Persson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 675478
Blocks: 712332
TreeView+ depends on / blocked
 
Reported: 2011-02-06 16:04 UTC by Pavel Zhukov
Modified: 2011-11-07 13:20 UTC (History)
4 users (show)

Fixed In Version: matreshka-0.1.1-9.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-09-01 16:48:20 UTC
Type: ---
Embargoed:
bjorn: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
This build log shows an example of libraries being recompiled by "make check". (136.15 KB, text/plain)
2011-05-14 22:03 UTC, Björn Persson
no flags Details

Description Pavel Zhukov 2011-02-06 16:04:31 UTC
Spec URL: http://landgraf.fedorapeople.org/packages/requested/matreshka/matreshka.spec
SRPM URL: http://landgraf.fedorapeople.org/packages/requested/matreshka/matreshka-0.0.6-1.fc14.src.rpm

Just for attention: this is the first attempt of contribution matreshka.

Description: 
* League --- provides support for localization, internationalization and globalization; including:
   o unbounded form of string of Unicode characters; cursors to iterate other characters and grapheme clusters; advanced locale tailored operations such as case conversion, case folding, collation, normalization;
   o regular expression engine with Perl-style syntax and Unicode extensions;
   o text codec to convert data streams into/from internal representation;
   o message translator to translate messages into natural language which is selected by user;
   o access to command line arguments and environment variables as Unicode encoded strings. 
* XML processor --- provides capability to manipulate with XML streams ans documents; including:
   o SAX reader to read XML streams and documents; it supports both XML1.0/XML1.1 specifications as well as corresponding Namespaces in XML specifications;
   o SAX writer to generate XML streams and documents from application. 
* Web framework
   o FastCGI module allows to develop server side applications completely in Ada and use them with standard HTTP servers ( demo). 
Large number of specifications are used and are supported, see Conformance for complete information about conformance to specifications. 

Koji build is unavailable (Please see depends)

Comment 1 Pavel Zhukov 2011-02-06 16:05:48 UTC
Add depend



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 2 Pavel Zhukov 2011-02-06 16:49:41 UTC

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 3 Pavel Zhukov 2011-02-22 21:11:47 UTC

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 4 Björn Persson 2011-03-20 14:40:09 UTC
I'll review this.

I assume you'll update to 0.0.7 and correct the same things that you corrected in the other two packages.

You could use http://adaforge.qtada.com/cgi-bin/tracker.fcgi/matreshka/downloader/download/file/9/matreshka-source-0.0.7.tar.gz in the Source field to enable automated downloading, and I think http://adaforge.qtada.com/cgi-bin/tracker.fcgi/matreshka is a better URL to put in the URL field.

Have you considered adding Russian translations of the summary and the description?

Comment 5 Pavel Zhukov 2011-03-20 18:52:42 UTC
(In reply to comment #4)
> I assume you'll update to 0.0.7 and correct the same things that you corrected
> in the other two packages.
Currently I'm working on MySQL driver for Matreshka_SQL and after I will pack new rpm (with SQLite, OCI and PostgreSQL drivers) and remove NotReady flag. On Wednesday I think

> Have you considered adding Russian translations of the summary and the
> description?
yes, of course.

Comment 7 Pavel Zhukov 2011-03-28 14:54:01 UTC
Fixed gpr files
http://dl.dropbox.com/u/3349355/matreshka-0.1.0-20110326svn.fc14.src.rpm

Comment 8 Björn Persson 2011-04-17 13:16:48 UTC
This isn't a complete review yet, but here are some things that need to be addressed:

· When you package a snapshot from Subversion, there should still be a normal release number before the so-called alphatag, for example "Release: 1.20110326svn%{?dist}".
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

· If you think it's better to have "http://Adaforge.qtAda.com" than "http://Adaforge.qtAda.com/cgi-bin/tracker.fcgi/matreshka" in the URL field, then please explain why you think so. I think the latter is better because it points directly to a page about the Matreshka project.

· All the -devel subpackages should be in the group Development/Libraries, not System Environment/Libraries.

· Many of the dependencies between the subpackages lack "%{?_isa}". As far as I can see all of these dependencies are architecture-specific, so "%{?_isa}" should be used.

· The -devel subpackages should have "Requires: fedora-gnat-project-common >= 2", not "BuildRequires". Only the base package should have "BuildRequires: fedora-gnat-project-common >= 2". There's no need to repeat build-time dependencies in subpackages as they're all built together in one go.

· matreshka-sql-sqlite gets an automatic dependency on libsqlite3.so.0, so the explicit dependency on sqlite is unnecessary and should be removed. Quoting the guidelines: "Packages must not contain explicit Requires on libraries except when absolutely necessary. When explicit library Requires are necessary, there should be a spec file comment justifying it."
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

· Does matreshka-sql-postgresql really depend on the PostgreSQL client programs in the package postgresql? If it only depends on the libraries in postgresql-libs, then the automatic dependency on libpq.so.5 should be sufficient and the explicit dependency should be removed.

· You pass GNATFLAGS to Make but there is no such variable in the makefiles. Did you mean GPRBUILD_FLAGS?

· I think the "make check" command should probably be in a %check section rather than in the %build section.

· No package owns the directory /usr/include/matreshka.


Some tips, just in case you didn't know:

· You can write "%make_install" instead of "make install DESTDIR=%{buildroot}" (but be careful so that you don't write "%makeinstall" by mistake).

· You can add translations of the summary and description fields as "Summary(ru)" and "%description -l ru". Look at the PragmARC spec as an example:
http://pkgs.fedoraproject.org/gitweb/?p=PragmARC.git;a=blob;f=PragmARC.spec;h=78486a757cb2bdafe0628abb09cf2de290979aa4;hb=HEAD

Comment 9 Pavel Zhukov 2011-04-20 18:15:48 UTC
(In reply to comment #8)
> · When you package a snapshot from Subversion, there should still be a normal
> release number before the so-called alphatag, for example "Release:
> 1.20110326svn%{?dist}".
fixed
> · If you think it's better to have "http://Adaforge.qtAda.com" than
> "http://Adaforge.qtAda.com/cgi-bin/tracker.fcgi/matreshka" in the URL field,
> then please explain why you think so. I think the latter is better because it
> points directly to a page about the Matreshka project.
fixed
> · All the -devel subpackages should be in the group Development/Libraries, not
> System Environment/Libraries.
fixed
> · Many of the dependencies between the subpackages lack "%{?_isa}". As far as I
> can see all of these dependencies are architecture-specific, so "%{?_isa}"
> should be used.
fixed
> · The -devel subpackages should have "Requires: fedora-gnat-project-common >=
> 2", not "BuildRequires". Only the base package should have "BuildRequires:
> fedora-gnat-project-common >= 2". There's no need to repeat build-time
> dependencies in subpackages as they're all built together in one go.
fixed
> · matreshka-sql-sqlite gets an automatic dependency on libsqlite3.so.0, so the
> explicit dependency on sqlite is unnecessary and should be removed. Quoting the
> guidelines: "Packages must not contain explicit Requires on libraries except
> when absolutely necessary. When explicit library Requires are necessary, there
> should be a spec file comment justifying it."
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
removed
> · Does matreshka-sql-postgresql really depend on the PostgreSQL client programs
> in the package postgresql? If it only depends on the libraries in
> postgresql-libs, then the automatic dependency on libpq.so.5 should be
> sufficient and the explicit dependency should be removed.
removed
> · You pass GNATFLAGS to Make but there is no such variable in the makefiles.
> Did you mean GPRBUILD_FLAGS?
> 
> · I think the "make check" command should probably be in a %check section
> rather than in the %build section.
moved
> · No package owns the directory /usr/include/matreshka.
fixed
> 
> Some tips, just in case you didn't know:
> 
> · You can add translations of the summary and description fields as
> "Summary(ru)" and "%description -l ru". Look at the PragmARC spec as an
> example:
There is no Russian translation of matreshka. I'll add it later.

Comment 11 Björn Persson 2011-04-24 11:41:59 UTC
· I see that you added /usr/include/matreshka to the base package. I think matreshka-devel is the best package to put this directory in, because only -devel subpackages have files under /usr/include/matreshka and they all depend on matreshka-devel (directly or indirectly). The way it is now, there will be an empty and unnecessary /usr/include/matreshka directory when the compiled libraries are installed but the -devel packages are not. It's not a big issue though.

· I had to add quotes in the export command to get the build to work, like this:
export GPRBUILD_FLAGS="%{GNAT_optflags}"
Without the quotes Bash set GPRBUILD_FLAGS to "-cargs" and then tried to export variables named "-O2", "-g" and so on.

· Even with the quotes, the parameters from GNAT_optflags weren't passed to GCC and Gnatlink. I tried making GPRBUILD_FLAGS a parameter to Make instead of an environment variable, and then the parameters were passed correctly. Apparently you'll need to make more changes to the makefiles if you want them to pick up this variable from the environment.

· The build log in Koji shows GPRBUILD_FLAGS being passed to Make as a parameter. Obviously the package you submitted to Koji was different from the one you linked to here, despite the version and release numbers being the same. Don't do that.

· You're neglecting the changelog. Every time you make changes to the package you should increment the release number and write a new changelog entry.

· The project files have too generic names. Project file names are a shared namespace that we need to be careful not to pollute. A name like "sql.gpr" is too prone to collisions. It would be a good name if there were one library that could claim to be *the* SQL library for Ada, but that's not the case. There are several database interface libraries and it wouldn't be fair to let one of them claim the name "SQL". "fastcgi.gpr" is a similar case. Somebody might want to package another library that also implements the FastCGI interface, and there's no reason why one library should have a greater right to that name than another.

To prevent name collisions the Ada packaging policies we wrote state that either the name of the library must be included in the filenames of the project files (that is, "matreshka_sql.gpr" or something similar), or else they must be in a subdirectory named after the library (in which case other projects would use «with "matreshka/sql.gpr";» et cetera to import them).

Whether you change the names or use a subdirectory, it would be best if this change were made upstream so that the project files can be referred to in the same way in all distributions that include Matreshka as well as when users download the source package and compile it themselves. Otherwise project files will have to be patched left and right.

Comment 12 Björn Persson 2011-04-24 15:09:00 UTC
Yikes! There's a "pragma Suppress (All_Checks)" in matreshka_league__release.adc. It's commented out, so it does no harm at this time, but I really hope it won't be turned on in some future release.

Comment 13 Vadim Godunko 2011-04-25 08:35:42 UTC
Don't worry about pragma Suppress in global configuration file, it will be enabled only when correctness of source code will be proved formally.

Comment 14 Pavel Zhukov 2011-05-02 08:05:17 UTC
(In reply to comment #11)
> · I see that you added /usr/include/matreshka to the base package. I think
> matreshka-devel is the best package to put this directory in, because only
> -devel subpackages have files under /usr/include/matreshka and they all depend
> on matreshka-devel (directly or indirectly). The way it is now, there will be
> an empty and unnecessary /usr/include/matreshka directory when the compiled
> libraries are installed but the -devel packages are not. It's not a big issue
> though.
fixed


> · Even with the quotes, the parameters from GNAT_optflags weren't passed to GCC
> and Gnatlink. I tried making GPRBUILD_FLAGS a parameter to Make instead of an
> environment variable, and then the parameters were passed correctly. Apparently
> you'll need to make more changes to the makefiles if you want them to pick up
> this variable from the environment.

It's strange, because It works for me. Changed.

> · The project files have too generic names. Project file names are a shared
> namespace that we need to be careful not to pollute. A name like "sql.gpr" is
> too prone to collisions. It would be a good name if there were one library that
> could claim to be *the* SQL library for Ada, but that's not the case. There are
> several database interface libraries and it wouldn't be fair to let one of them
> claim the name "SQL". "fastcgi.gpr" is a similar case. Somebody might want to
> package another library that also implements the FastCGI interface, and there's
> no reason why one library should have a greater right to that name than
> another.
> 
> To prevent name collisions the Ada packaging policies we wrote state that
> either the name of the library must be included in the filenames of the project
> files (that is, "matreshka_sql.gpr" or something similar), or else they must be
> in a subdirectory named after the library (in which case other projects would
> use «with "matreshka/sql.gpr";» et cetera to import them).
I'll work with upstream for this issue. 
Vadim, can we change gpr names?
> Whether you change the names or use a subdirectory, it would be best if this
> change were made upstream so that the project files can be referred to in the
> same way in all distributions that include Matreshka as well as when users
> download the source package and compile it themselves. Otherwise project files
> will have to be patched left and right.
Vadim (in CC of bug) is the main maintainer of matreshka.

Comment 15 Björn Persson 2011-05-14 22:01:24 UTC
In my tests it looks like "make check" does more than just checking. It seems to also recompile the libraries, at least partly. I'm not sure if it does any harm other than wasting some CPU time, but it should probably be investigated.

If recompilation can't be avoided, then I think GNAT_optflags should be used even in the "make check" command.

(I am of course still testing 0.1.0-1.20110326svn (slightly modified as described in comment 11), as that's the latest package I've seen.)

(In reply to comment #13)
> Don't worry about pragma Suppress in global configuration file, it will be
> enabled only when correctness of source code will be proved formally.

That sounds somewhat reassuring, but I'm not convinced that it's a good idea to suppress *all* checks. There is always the possibility of errors in the proof. It's particularly unwise to suppress the range checks on arrays, which is a very important line of defense against remote code execution.

Comment 16 Björn Persson 2011-05-14 22:03:25 UTC
Created attachment 498962 [details]
This build log shows an example of libraries being recompiled by "make check".

Comment 17 Pavel Zhukov 2011-06-04 19:49:21 UTC
Working on 0.1.0 release.

Comment 18 Björn Persson 2011-07-18 08:57:10 UTC
How is it going?

It's now about a week left until Feature Freeze. According to the Feature Freeze Policy, new features shall be "substantially complete and in a testable state" at Feature Freeze. I interpret this to mean that Matreshka and the other packages you listed as parts of the Ada developer tools feature must be approved and imported before Feature Freeze, as packages are obviously not testable if they are missing.

Do you still think you can make it? If not, I guess your feature will have to be rescheduled for Fedora 17.

Comment 19 Pavel Zhukov 2011-07-18 20:34:40 UTC
Currently I'm working on bug 722732 because I cannot update gprbuild and some other tools...
I've built aws and qtada but I need gnat src for GPS and new gprbuild.

Comment 20 Fedora Update System 2011-08-10 12:45:48 UTC
florist-2011-6.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/florist-2011-6.fc16

Comment 21 Pavel Zhukov 2011-08-10 12:52:12 UTC
I'm sorry for this confusion.

Comment 23 Björn Persson 2011-08-20 23:01:02 UTC
So far I've found two blocking problems in 0.1.1-5:

· GNAT_optflags is not in effect during the build, only in the check phase. I recommend overriding GPRBUILD_FLAGS on the Make command line.

· config.gpr is architecture-specific. It needs to be patched to make use of Directories.Libdir. Something also needs to be done about the variable Architecture. It looks like its default value varies between architectures, which will cause a multilib conflict. I think a case statement could be used to derive Architecture from Directories.Hardware_Platform.

I also wonder why you're claiming that RPMbuild can't create debuginfo packages, because I get debuginfo packages built for my Ada packages, and they work as they should in GDB as far as I can see. I tried removing "%define debug_package %{nil}" from matreshka.spec, and I got a debuginfo package that seems to contain the files it should contain. What problem do you have with debuginfo packages?

Comment 24 Pavel Zhukov 2011-08-23 05:38:59 UTC
(In reply to comment #23)
> So far I've found two blocking problems in 0.1.1-5:
> 
> · GNAT_optflags is not in effect during the build, only in the check phase. I
> recommend overriding GPRBUILD_FLAGS on the Make command line.
fixed. Added fedora-gnat-project >= 3 to BR
> · config.gpr is architecture-specific. It needs to be patched to make use of
> Directories.Libdir. Something also needs to be done about the variable
> Architecture. It looks like its default value varies between architectures,
> which will cause a multilib conflict. I think a case statement could be used to
> derive Architecture from Directories.Hardware_Platform.
Fixed
> I also wonder why you're claiming that RPMbuild can't create debuginfo
> packages, because I get debuginfo packages built for my Ada packages, and they
> work as they should in GDB as far as I can see. I tried removing "%define
> debug_package %{nil}" from matreshka.spec, and I got a debuginfo package that
> seems to contain the files it should contain. What problem do you have with
> debuginfo packages?
Fixed. I had problem with debuginfo and rpmlint warnings in zeromq-ada package. It's Copy-Paste error :)

http://landgraf.fedorapeople.org/packages/requested/matreshka/matreshka-0.1.1-6.fc16.src.rpm
http://landgraf.fedorapeople.org/packages/requested/matreshka/matreshka.spec

Koji OK.

Comment 25 Björn Persson 2011-08-25 22:55:33 UTC
Simply assigning Hardware_Platform to Architecture won't work. The data types are quite different. I think something along these lines should work:

case Directories.Hardware_Platform is
  when "i386" =>
    Architecture : Architectures := external ("ARCHITECTURE", "x86");
  when "x86_64" =>
    Architecture : Architectures := external ("ARCHITECTURE", "x86_64");
  when "arm" =>
    Architecture : Architectures := external ("ARCHITECTURE", "portable_32_le");
  when "alpha" | "ia64" =>
    Architecture : Architectures := external ("ARCHITECTURE", "portable_64_le");
  when "ppc" | "s390" | "sparc" =>
    Architecture : Architectures := external ("ARCHITECTURE", "portable_32_be");
  when "ppc64" | "s390x" | "sparc64" =>
    Architecture : Architectures := external ("ARCHITECTURE", "portable_64_be");
end case;

Beware! The above code is completely untested and I don't guarantee that the endianness is correct for all the architectures.

Comment 26 Björn Persson 2011-08-26 22:50:31 UTC
matreshka-devel should own the directory %{_GNAT_project_dir}/%{name}.

Comment 29 Björn Persson 2011-08-31 01:00:44 UTC
Generic MUST Items:

· rpmlint must be run on the source rpm and all binary rpms the build produces.
  → OK. None of the warnings are real problems:

  matreshka.src: W: spelling-error %description -l en_US grapheme -> ephemera
  matreshka.src: W: spelling-error %description -l en_US codec -> codex, code, codes
  matreshka.src:143: W: macro-in-comment %{GPRbuild_optflags}
  matreshka.i686: W: spelling-error %description -l en_US grapheme -> ephemera
  matreshka.i686: W: spelling-error %description -l en_US codec -> codex, code, codes
  matreshka.i686: W: executable-stack /usr/lib/libleague.so.0.1.1
  matreshka.x86_64: W: spelling-error %description -l en_US grapheme -> ephemera
  matreshka.x86_64: W: spelling-error %description -l en_US codec -> codex, code, codes
  matreshka.x86_64: W: executable-stack /usr/lib64/libleague.so.0.1.1
  matreshka-debuginfo.i686: W: hidden-file-or-dir /usr/src/debug/matreshka-0.1.1/.objs
  matreshka-debuginfo.i686: W: hidden-file-or-dir /usr/src/debug/matreshka-0.1.1/.objs
  matreshka-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/matreshka-0.1.1/.objs
  matreshka-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/matreshka-0.1.1/.objs
  matreshka-fastcgi.i686: W: unused-direct-shlib-dependency /usr/lib/libmatreshka-fastcgi.so.0.1.1 /usr/lib/libgnarl-4.6.so
  matreshka-fastcgi.i686: W: executable-stack /usr/lib/libmatreshka-fastcgi.so.0.1.1
  matreshka-fastcgi.i686: W: no-documentation
  matreshka-fastcgi.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmatreshka-fastcgi.so.0.1.1 /usr/lib64/libgnarl-4.6.so
  matreshka-fastcgi.x86_64: W: executable-stack /usr/lib64/libmatreshka-fastcgi.so.0.1.1
  matreshka-fastcgi.x86_64: W: no-documentation
  matreshka-fastcgi-devel.i686: W: no-documentation
  matreshka-fastcgi-devel.x86_64: W: no-documentation
  matreshka-sql-core.i686: W: no-documentation
  matreshka-sql-core.x86_64: W: no-documentation
  matreshka-sql-core-devel.i686: W: no-documentation
  matreshka-sql-core-devel.x86_64: W: no-documentation
  matreshka-sql-postgresql.i686: W: no-documentation
  matreshka-sql-postgresql.x86_64: W: no-documentation
  matreshka-sql-postgresql-devel.i686: W: no-documentation
  matreshka-sql-postgresql-devel.x86_64: W: no-documentation
  matreshka-sql-sqlite.i686: W: no-documentation
  matreshka-sql-sqlite.x86_64: W: no-documentation
  matreshka-sql-sqlite-devel.i686: W: no-documentation
  matreshka-sql-sqlite-devel.x86_64: W: no-documentation

  · The warnings about spelling errors are false positives; the spelling is correct.
  · The macro in a comment won't cause any problems in this case.
  · Executable stack is OK as noted in the Ada packaging guidelines.
  · The hidden directory in the debuginfo package is odd, but not something a packager should be required to change. 
  · The unused dependency on libgnarl seems to be something that Gnat does, and is probably not something that a packager should tamper with.
  · Lack of documentation in the subpackages is OK because they require the main package and the devel subpackage.

· 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 three-clause BSD according to the file LICENSE and a few sampled 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.
  → OK. LICENSE is in the main package.

· 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 tarballs are identical.

· 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.
  → Possible issue: You may need to add ExclusiveArch, or maybe not. People working on secondary architectures where Gnat isn't available seem to have problems with some Ada packages and not with others, and I've never understood what makes the difference.

· All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines.
  → ISSUE: No build-time dependencies are missing, but one seems to be included unnecessarily. I don't see why mysql-devel is needed. The build seems to work fine without it. Unnecessary build-time dependencies aren't formally forbidden as far as I can see, but common sense says you should remove this one unless it really is used for something. You can add it back later when the MySQL driver is ready.

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

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

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

· A Fedora 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.

· Each 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. There is very little documentation.

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

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

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

· If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go 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".
  → ISSUE: "BuildRequires: gcc-gnat" is missing. The build works only because gcc-gnat is pulled in by fedora-gnat-project-common, and this should not be relied on. fedora-gnat-project-common does not in any way abstract away Gnat. Matreshka's makefiles invoke Gnat tools directly, and therefore there should also be a direct dependency on gcc-gnat.

· The RPM macro %{GNAT_optflags} MUST be used in the compilation.
  → OK. (Using the name Gnatmake_optflags instead is fine.)

· 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 all the source files of the library, only those that are necessary for compilation of code that uses the library.
  → It seems to contain all the source files except for a few that are specific to other platforms. I'm starting to think that this rule was a bad idea. It's very difficult for a reviewer to know which files are needed and which ones aren't, and it's a lot of work for a packager to rework makefiles that don't follow the rule. The code is free so we have no reason to hide it, and disk space won't be an issue on developers' workstations or on build servers. I'm going to ignore this rule for now and later propose to weaken it to "should not" or even remove it from the guidelines altogether.

· 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 >= 2".
  → OK.

· Project files MUST have an Externally_Built attribute equal to "true". 
  → 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.
  → N/A. There is 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 available 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.
  → There is a %check section with a test suite. I haven't tested the libraries manually.

· If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity.
  → OK.

· Usually, subpackages other than devel should require the base package using a fully versioned dependency.
  → OK.

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

· The package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.
  → N/A. There are no programs, only libraries.


To summarize: Add a dependency on gcc-gnat, and remove the one on mysql-devel or explain why it's needed. Then this package will be good to go, although you may choose to also add ExclusiveArch.

Comment 30 Pavel Zhukov 2011-08-31 19:40:23 UTC
>>> To summarize: Add a dependency on gcc-gnat, and remove the one on mysql-devel
or explain why it's needed. Then this package will be good to go, although you
may choose to also add ExclusiveArch.
Fixed all: http://landgraf.fedorapeople.org/packages/requested/matreshka/matreshka-0.1.1-9.fc15.src.rpm
Spec: http://landgraf.fedorapeople.org/packages/requested/matreshka/matreshka.spec

Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3316670

Comment 31 Björn Persson 2011-08-31 20:28:17 UTC
Looks good. This package is APPROVED.

Comment 32 Pavel Zhukov 2011-09-01 02:17:42 UTC
Thank you for review (and patience :-) )

Comment 33 Pavel Zhukov 2011-09-01 02:19:09 UTC
New Package SCM Request
=======================
Package Name: matreshka
Short Description: Set of Ada libraries to help to develop information systems
Owners: landgraf
Branches: F16
InitialCC:

Comment 34 Gwyn Ciesla 2011-09-01 11:55:54 UTC
Git done (by process-git-requests).

Comment 35 Fedora Update System 2011-09-01 16:40:16 UTC
matreshka-0.1.1-9.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/matreshka-0.1.1-9.fc16

Comment 36 Fedora Update System 2011-09-09 17:03:23 UTC
matreshka-0.1.1-9.fc16 has been pushed to the Fedora 16 stable repository.

Comment 37 Pavel Zhukov 2011-11-07 12:41:04 UTC
Package Change Request
======================
Package Name: matreshka
New Branches: el6
Owners: landgraf
InitialCC:

Comment 38 Gwyn Ciesla 2011-11-07 13:20:04 UTC
Git done (by process-git-requests).


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