Bug 810676

Summary: Review Request: aws - Ada Web Server (Web framework for Ada)
Product: [Fedora] Fedora Reporter: Pavel Zhukov <pavel>
Component: Package ReviewAssignee: Björn Persson <bjorn>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bjorn, julian, notting, package-review
Target Milestone: ---Flags: bjorn: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-12-20 16:18:41 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 836014, 852543, 868485    
Bug Blocks: 712332    

Description Pavel Zhukov 2012-04-08 06:04:28 UTC
Spec URL: http://landgraf.fedorapeople.org/packages/requested/aws/aws.spec
SRPM URL: http://landgraf.fedorapeople.org/packages/requested/aws/aws-2.10.0-3.fc17.src.rpm
Description: 
AWS is a complete framework to develop Web based applications. The main part of the framework is the embedded Web server. This small yet powerful Web server can be embedded into your application so your application will be able to talk with a standard Web browser like Microsoft Internet Explorer or Firefox for example. Around this Web server a lot of services have been developed.

Comment 2 Björn Persson 2012-04-09 11:34:52 UTC
This is what I see at first glance:

· The license differs between the main package and the -devel subpackage. Is that really true? If not, please remove the license field from the subpackage.

· The -devel subpackage lacks a dependency on fedora-gnat-project-common.

· The version field says 2.10.0 but in the changelog it says 2.8.0.

· I think the URL field should point to a page that describes AWS, that is http://libre.adacore.com/libre/tools/AWS/ instead of Adacore's front page.

· According to the copyright notices in aws.gpr and template_parser.gpr those files were written by Adacore. Is that true? If so, why are they in aws-fedora.tgz and not in aws-gpl-2.10.0-src.tgz?

I'll have a closer look later.

Comment 3 Pavel Zhukov 2012-04-10 18:13:35 UTC
https://koji.fedoraproject.org/koji/taskinfo?taskID=3977389
Spec URL: http://landgraf.fedorapeople.org/packages/requested/aws/aws.spec(In reply to comment #2)

> · The license differs between the main package and the -devel subpackage. Is
> that really true? If not, please remove the license field from the subpackage.
>
> · The -devel subpackage lacks a dependency on fedora-gnat-project-common.
> 
> · The version field says 2.10.0 but in the changelog it says 2.8.0.
> 
> · I think the URL field should point to a page that describes AWS, that is
> http://libre.adacore.com/libre/tools/AWS/ instead of Adacore's front page.

fixed all

> · According to the copyright notices in aws.gpr and template_parser.gpr those
> files were written by Adacore. Is that true? If so, why are they in
> aws-fedora.tgz and not in aws-gpl-2.10.0-src.tgz?
No. This files as well as makefile and other ones in aws-fedora.tgz have been changed (or even rewritten) by Ludovic Brenta for Debian and me for Fedora because AdaCpre prefers static libs instead dynamic. Do I have to change Copyrights for this files?

Comment 4 Björn Persson 2012-04-11 23:04:23 UTC
Without knowing in detail how you produced those files I can't say for certain what the copyright notices should look like.

Your files are dated "2003-2009". There are only two project files with that combination of years in aws-gpl-2.10.0-src.tgz, and those are very different from yours, so it doesn't look like your files are modified versions of Adacore's.

Debian's AWS package has an aws.gpr that looks fairly similar to yours, as if you had taken Ludovic's project file, adapted it for Fedora, and replaced Ludovic's copyright notice with Adacore's. Debian's templates_parser.gpr, on the other hand, looks quite different from your template_parser.gpr.

If major parts of the project file code stem from Ludovic's project files, then you should include Ludovic's copyright notice. If you have edited a file so much that you have essentially written it all yourself, then you can write something like "GNAT project file for Ada Web Server, Copyright 2012 Pavel Zhukov", or you can choose to omit the copyright notice.

That's at least how I understand it, but IANAL and so on. I suppose you can ask on the legal mailing list if you want other opinions. If you do, you'll probably have to describe in detail what you have done with the files.

Comment 5 Pavel Zhukov 2012-04-13 09:25:56 UTC
I have changed copyrights preambula for aws.gpr build_aws.gpr and tools.gpr files. Please check
http://koji.fedoraproject.org/koji/taskinfo?taskID=3987294

Comment 6 Björn Persson 2012-04-15 22:48:04 UTC
There are several files in the directory "include" that look like bundled libraries. Zlib is a clear case and easy to handle: The directory include/zlib must be deleted during the prep stage. It's not so obvious what should be done about the other files in the include directory. Are they distributed separately at other websites, or has Adacore taken over maintenance and become the canonical source of those files? Should they be considered "copylibs"? Have you investigated this? I can't even find a license statement for the sha* files.

I see that your build system divides the code into shared libraries quite differently from the upstream. Adacore's build system produced three libraries called "libaws.so", "libaws_ssl.so" and "libaws_include.so" when I tried it. Yours produces "libaws.so.2.10.0" and "libtemplates_parser.so.2.10.0". Could you explain the reasoning behind this? Do you think separate aws_ssl and aws_include libraries aren't useful, and if so why? Conversely, do you think a separate templates_parser library is useful, and why? Your choice may well be the better one but I'd like to see your reasons. Perhaps you should install a README.Fedora where you explain your reasoning?

I'm afraid you can't link to OpenSSL, as the OpenSSL license is incompatible with the GPL. I see that there is an option to link to GnuTLS instead. Please use that if possible.

Have you tried enabling IPv6? APNIC ran out of IPv4 address blocks one year ago, and RIPE NCC is expected to run out within a year. There are no valid excuses for not supporting IPv6 in 2012.

Shouldn't awsres and wsdl2aws be packaged? In a subpackage called "aws-tools" perhaps? And maybe aws_password and webxref could be built and installed too?

Comment 7 Björn Persson 2012-04-25 21:38:24 UTC
I found a few more small problems:

You don't use chrpath, and indeed it doesn't seem to be needed, so the dependency on it should be removed.

Please use cp --preserve=timestamps in the makefile, or simply cp -a, to preserve the timestamps on installed source files and documents.

%defattr is unnecessary with current RPM versions, but it's not forbidden so you can keep it if you want.

I think AUTHORS should go in the base package. Information about who the authors are is logically connected with the license file.

Comment 8 Pavel Zhukov 2012-05-20 18:42:34 UTC
(In reply to comment #7)
> I found a few more small problems:
> 
> You don't use chrpath, and indeed it doesn't seem to be needed, so the
> dependency on it should be removed.
Deleted
> Please use cp --preserve=timestamps in the makefile, or simply cp -a, to
> preserve the timestamps on installed source files and documents.
Replaced cp with ${CP} = /bin/cp -a 
> %defattr is unnecessary with current RPM versions, but it's not forbidden so
> you can keep it if you want.
Removed
> I think AUTHORS should go in the base package. Information about who the
> authors are is logically connected with the license file.
Fixed

http://koji.fedoraproject.org/koji/taskinfo?taskID=4090332
Spec URL: http://landgraf.fedorapeople.org/packages/requested/aws/aws.spec

Comment 9 Björn Persson 2012-06-01 23:28:19 UTC
I went and read John Halleck's website where his SHA-1 implementation is published. On http://home.utah.edu/~nahaj/ada/sha/ he writes "You can freely use this under the GPL or BSD Open Source licenses", and on http://home.utah.edu/~nahaj/ada/ he gives permission to use his code and requests to be credited. Therefore I think that there is no license problem with the sha* files.

I'm still waiting for answers to the other questions I asked in comment 6.

Comment 10 Björn Persson 2012-06-24 17:01:04 UTC
Templates Parser is apparently also bundled with GPS (bug 834747). This makes me think that Templates Parser needs to be broken out into a subpackage built from either the AWS or the GPS source package (but named templates-parser, not aws-templates-parser). I'm not sure which is best, but I think the risk of circular dependencies may be smaller if it's built from the GPS source.

Comment 11 Julian Leyh 2012-06-24 19:53:00 UTC
The project page of AWS lists templates_parser as external dependency. It is hosted in its own git repository.
GPS project page lists this, too, but still uses svn repositories. The GPS svn repository seems to include the source for templates_parser now.

Maybe we can use the templates_parser git repo as basis for its own package.

Comment 12 Björn Persson 2012-06-24 22:41:20 UTC
(In reply to comment #11)
> Maybe we can use the templates_parser git repo as basis for its own package.

Do you know if it has any release tags that can tell us which version we should package? Packaging random development snapshots might not be the best idea.

Comment 13 Julian Leyh 2012-06-25 18:07:59 UTC
(In reply to comment #12)
> Do you know if it has any release tags that can tell us which version we
> should package? Packaging random development snapshots might not be the best
> idea.

I just checked, what tags it has:

before_acte_copyright
first-release
gap-1.1.0
gnat-5.03a
gps-3.0.0
gps-3.0.1
gps-3.1.0
gps-3.1.1
gps-3.1.2
gps-3.1.3
gps-4.0.0
release-0.10.0
release-0.9.0
release-0.9.10
release-0.9.11
release-0.9.2
release-0.9.9
release-1.0.0
release-1.1.0
release-1.2.0
release-1.3.0
release-10.0.0
release-2.0.0
release-2.1a
release-3.0.0
release-4.0.0
release-5.0.0
release-6.0.0
release-7.0.0
release-8.0.0
release-8.1.0
release-8.3.0
release-9.0.0
v11.6.0

current release is 11.6.0, so the tag "v11.6.0" should be the one to use.

Comment 14 Julian Leyh 2012-08-16 07:19:15 UTC
removed dependency to gps, templates_parser has its own package now.

Comment 15 Pavel Zhukov 2012-08-16 08:58:26 UTC
(In reply to comment #6)
> There are several files in the directory "include" that look like bundled
> libraries. Zlib is a clear case and easy to handle: The directory
> include/zlib must be deleted during the prep stage. It's not so obvious what
> should be done about the other files in the include directory. Are they
> distributed separately at other websites, or has Adacore taken over
> maintenance and become the canonical source of those files? Should they be
> considered "copylibs"? Have you investigated this? I can't even find a
> license statement for the sha* files.
include/zlib removed.  
> I see that your build system divides the code into shared libraries quite
> differently from the upstream. Adacore's build system produced three
> libraries called "libaws.so", "libaws_ssl.so" and "libaws_include.so" when I
> tried it. Yours produces "libaws.so.2.10.0" and
> "libtemplates_parser.so.2.10.0". Could you explain the reasoning behind
> this? Do you think separate aws_ssl and aws_include libraries aren't useful,
> and if so why? Conversely, do you think a separate templates_parser library
> is useful, and why? Your choice may well be the better one but I'd like to
> see your reasons. Perhaps you should install a README.Fedora where you
> explain your reasoning?
This "my" buildsystem I've got from Debian. 
I'm not sure but Adacore devides the code into separate shared libraries becuase AWS can be built for different OS and architectures. Besides it's easy to hadle single lib in my opinion.
> I'm afraid you can't link to OpenSSL, as the OpenSSL license is incompatible
> with the GPL. I see that there is an option to link to GnuTLS instead.
> Please use that if possible.
Fixed. Using gnutls now
> Have you tried enabling IPv6? APNIC ran out of IPv4 address blocks one year
> ago, and RIPE NCC is expected to run out within a year. There are no valid
> excuses for not supporting IPv6 in 2012.
I'm working on it rigth now
> Shouldn't awsres and wsdl2aws be packaged? In a subpackage called
> "aws-tools" perhaps? And maybe aws_password and webxref could be built and
> installed too?
I'm working on it rigth now.

Comment 16 Pavel Zhukov 2012-08-16 11:03:29 UTC
New SRPM: http://landgraf.fedorapeople.org/packages/requested/aws/aws-2.11.0-2.fc17.src.rpm

Mock is OK.

Comment 17 Pavel Zhukov 2012-08-16 18:17:35 UTC
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4396685

Comment 19 Julian Leyh 2012-08-16 18:53:56 UTC
the URL is invalid, it should be http://libre.adacore.com/tools/aws and not http://libre.adacore.com/libre/tools/AWS/

you could reuse the manpages from the debian package for the tools

Comment 20 Björn Persson 2012-08-16 23:21:49 UTC
(In reply to comment #15)
> include/zlib removed.

And I see that you also delete the bundled Templates Parser now. That leaves Memory Streams, Strings Cutter, Zlib-Ada and the SHA-1 implementation. We still need to decide what to do with those.

> I'm not sure but Adacore devides the code into separate shared libraries
> becuase AWS can be built for different OS and architectures.

I don't see how separate shared libraries would help with portability. They're still compiled for a specific OS and architecture.

Dividing the code into multiple libraries can help if someone wants to use a part of AWS without pulling in all of its dependencies, but Adacore's libaws.so requires libaws_ssl.so and libaws_include.so, so their division doesn't help with that either.

I don't see that Adacore's division has any advantages over a single shared library, so I accept your choice. A different division could have advantages but I'm not going to require that of you.

> Using gnutls now

The installed aws.gpr still mentions OpenSSL. And shouldn't it point to aws-net-std__ipv6.adb rather than aws-net-std__gnat.adb?

Some other things I have noticed:

· The group of the base package should be System Environment/Libraries. AWS is a library, not a daemon. I think the -tools subpackage fits best in Development/Tools.

· template_parser.gpr should be deleted from aws-fedora.tgz.

· LDAP support seems to be missing. Is there a good reason for that?

· Please add comments explaining the patches. See <https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment>.

· The manual is somewhat large. Have you considered making a -doc subpackage? This is a judgment call and the choice is ultimately yours. See <https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation>. If you do make a -doc subpackage it may be a good idea to add a note in the description of the -devel subpackage to help users discover the -doc subpackage.

· /usr/share/doc/aws and its contents should be in the -devel subpackage if you don't make a -doc subpackage. Only COPYING3 and AUTHORS need to be in the base package. The rest of the documentation is relevant to developers who use AWS in their programs, but not to users who only use a program that is linked to AWS.

· INSTALL should not be packaged. It contains only build instructions so it's irrelevant to the binary package.

· readme.txt, on the other hand, contains some information that could be useful to developers, so it should be packaged.

· The directory docs also contains some more files that probably should be packaged. RFC.INDEX, TODO, known-problems and features-* seem useful to developers so they should be copied to /usr/share/doc/aws.

· The demos should also be included as documentation. /usr/share/doc/aws/demos seems like a good location. (Don't compile the demos though.)

(In reply to comment #19)
> the URL is invalid, it should be http://libre.adacore.com/tools/aws and not
> http://libre.adacore.com/libre/tools/AWS/

Indeed. They must have changed it because I'm sure it worked in April. Perhaps someone at Adacore doesn't know that cool URIs don't change. (http://www.w3.org/Provider/Style/URI)

Comment 21 Pavel Zhukov 2012-08-17 04:09:59 UTC
(In reply to comment #19)
> the URL is invalid, it should be http://libre.adacore.com/tools/aws and not
> http://libre.adacore.com/libre/tools/AWS/
Fixed

(In reply to comment #20)
> (In reply to comment #15)
> > include/zlib removed.
> 
> And I see that you also delete the bundled Templates Parser now. That leaves
> Memory Streams, Strings Cutter, Zlib-Ada and the SHA-1 implementation. We
> still need to decide what to do with those.
> 

> The installed aws.gpr still mentions OpenSSL. And shouldn't it point to
> aws-net-std__ipv6.adb rather than aws-net-std__gnat.adb?
> Some other things I have noticed:
Yes, I didn't change aws.gpr. Fixed
> · The group of the base package should be System Environment/Libraries. AWS
> is a library, not a daemon. I think the -tools subpackage fits best in
> Development/Tools.
Fixed
> · template_parser.gpr should be deleted from aws-fedora.tgz.
Done
> · LDAP support seems to be missing. Is there a good reason for that?
I don't plan to use this feature (and test it). I'll add support in next releases if anybody wants.
> · Please add comments explaining the patches. See
> <https://fedoraproject.org/wiki/Packaging:
> Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment>.
Done


To be continued...

Comment 22 Pavel Zhukov 2012-08-19 12:55:29 UTC
AWS with gnutls doen't work for me. I have to investigate this (or maybe link with OpenSSL?).

Comment 23 Björn Persson 2012-08-19 23:25:51 UTC
(In reply to comment #21)
> (In reply to comment #20)
> > · LDAP support seems to be missing. Is there a good reason for that?
> I don't plan to use this feature (and test it). I'll add support in next
> releases if anybody wants.

I think you should include it anyway. It is of course good if packagers use and test their own packages, but for a package with many features it must be expected that the packager doesn't use all of the features himself. If someone else wants to use it, and you have included it, then there's at least a chance that it will work. If you omit it, it's 100% certain that it won't work.

If you can't get it to build, or if you know that it's broken, then that would be a valid reason to omit it.

(In reply to comment #22)
> AWS with gnutls doen't work for me. 

That's very unfortunate if you can't get it to work. That could explain why Debian ships AWS without SSL support though.

> (or maybe link with OpenSSL?)

I don't think I can approve the package if you do that. The OpenSSL license is listed as incompatible with the GPL: http://fedoraproject.org/wiki/Licensing#SoftwareLicenses

This incompatibility was the reason why GnuTLS was written, so the FSF obviously takes it seriously. It's not a fun situation but we have to comply with the licenses.

Comment 24 Julian Leyh 2012-08-20 08:11:05 UTC
(In reply to comment #22)
> AWS with gnutls doen't work for me. I have to investigate this (or maybe
> link with OpenSSL?).

Official documentation only seems to mention OpenSSL, but the sources give hints that GnuTLS is (meant to be) supported, too.

If you set SOCKET to gnutls, how does the build fail?

Comment 25 Julian Leyh 2012-08-20 09:21:38 UTC
> License:    GPLv2+

This is not true for 2.11.0 any more, it is GPLv3+ with GCC Runtime Library Exception, version 3.1, as published by the Free Software Foundation.

So it should be (like GCC):

License: GPLv3+ and GPLv3+ with exceptions

(first one could be left out, if there are no files under pure GPLv3+)

Comment 26 Pavel Zhukov 2012-08-22 14:05:26 UTC
http://koji.fedoraproject.org/koji/taskinfo?taskID=4413929
SRPMS: http://landgraf.fedorapeople.org/packages/requested/aws/aws-2.11.0-6.fc17.src.rpm

Fixed license (Thank Julian). Fixed gnutls issue. Add LDAP support. Move all docs to separate package etc

Comment 27 Pavel Zhukov 2012-08-22 14:08:41 UTC
P.S. I've canged license tag after koji build. Sorry

Comment 28 Björn Persson 2012-10-12 12:27:43 UTC
Generic MUST Items:

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

  aws-doc.x86_64: E: devel-dependency aws(x86-64)-devel
    → ISSUE: I can't find a rule that says explicitly that a -doc package must not depend on a -devel package – except maybe that "files marked as documentation must not cause the package to pull in more dependencies than it would without the documentation" – but it's true that one can read the documents without having the library installed. I think this dependency should be removed. Note that this will make the -doc subpackage independent of the base package, so it will need a copy of the license file.
  aws-doc.x86_64: W: file-not-utf8 /usr/share/doc/aws/demos/zdemo/adains.png.gz
    → Nonsense.
  aws-doc.x86_64: W: file-not-utf8 /usr/share/doc/aws/demos/web_mail/wm_summary.thtml
    → You probably shouldn't touch that file.
  aws.x86_64: W: executable-stack /usr/lib64/aws/libaws.so.2.11.0
    → OK as noted in the Ada packaging guidelines.
  aws-tools.x86_64: W: no-documentation
  aws-tools.x86_64: W: no-manual-page-for-binary awsres
  aws-tools.x86_64: W: no-manual-page-for-binary wsdl2aws
    → See the separate point below.
  aws.src: W: invalid-url Source1: aws-fedora.tgz
    → I assume that you're going to check that file into Git. Otherwise you would need to provide a URL to it.
  aws.src: W: invalid-url Source0: aws-gpl-2.11.0-src.tgz
    → As usual, this is because Adacore and RPM have incompatible ideas of how to publish tarballs.
  aws-devel.x86_64: W: spelling-error %description -l en_US subpackage -> sub package, sub-package, package's
    → I think RPMlint is wrong here. "Subpackage" is consistently written without a hyphen in the guidelines.

· 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.
  → ISSUES:
    · The -doc subpackage should be in the Documentation group.
    · The comment about the SSL versions patch needs to be expanded to explain whether you have sent the patch upstream, or whether it's Fedora-specific for some reason.
    · It's unclear whether IPv6 is enabled. build_aws.gpr points to both aws-net-std__gnat.adb and aws-net-std__ipv6.adb as implementations of AWS.Net.Std, and it's not easy to know which of them takes effect. aws.gpr only points to aws-net-std__gnat.adb.

· The package must be licensed with a Fedora approved license and meet the Licensing Guidelines.
  → OK.

· The License field in the package spec file must match the actual license.
  → ISSUE: As far as I can see the license should be "GPLv3+ with exceptions and GPLv2+" on the main package, and "GPLv3+" on aws-tools. (Memory_Streams is GPLv2+.)

· 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.
  → NOTE: A copy of COPYING3 must be included in aws-doc if its dependency on aws-devel is removed.

· The spec file must be written in American English.
  → OK. The spelling and grammar aren'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 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, ExclusiveArch 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 except as noted below.

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

  → ISSUE: Zlib-Ada is now packaged so include/zlib* must be deleted. "-lz" should be deleted from build_aws.gpr, and «with "zlib_ada";» added to build_aws.gpr and aws.gpr. The build dependencies and the dependencies of aws-devel must reflect this.

  Templates_Parser and Zlib are deleted in %prep, which is right.

  Memory_Streams is not distributed independently anywhere and Dmitriy Anisimkov considers it a part of AWS. The situation with Strings_Cutter seems to be the same; I can't find it anywhere else on the web. These are not bundled libraries.

  John Halleck's SHA-1 code doesn't seem to be used. It was also recently deleted from AWS's Git repository, so we don't need to worry about it.

· 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. (Notable exception: license texts in specific situations)
  → 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 and documentation.

· Large documentation files must go in a -doc subpackage.
  → OK.

· 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".
  → ISSUE: gcc-gnat is missing from the build dependencies.

· The appropriate flags macro must be used for each GNAT tool that is invoked.
  → OK. GPRbuild_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}".
  → NOTE: Once the package is imported and the branches have been set up, please change to "ExclusiveArch: %{GNAT_arches}" in the f17, f18 and master branches.

· 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.
  → OK, there is a COPYING3.

· 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.
  → ISSUES: Trying to compile the demos reveals several problems:
    · aws.gpr tries to import "aws/template_parser", which doesn't exist.
    · aws.gpr imports xmlada.gpr, so aws-devel must require xmlada-devel.
    · AWS speces reference Templates Parser speces, so aws.gpr must import templates_parser.gpr and aws-devel must require templates_parser-devel.
    · aws.gpr points to aws-net-ssl__openssl.adb instead of aws-net-ssl__gnutls.adb.

· 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 this. I don't think this applies to the -doc subpackage.

· 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 awsres and wsdl2aws. Debian has man pages however, so you should try to adapt those for Fedora. In a quick check I found one thing that needs to be adjusted: They refer to the manual in the -doc subpackage, but Debian's package has a different name. You could change it to "aws-doc", but it might be better to remove the package name so that the man pages can be upstreamed.


Common sense stuff not mentioned in the guidelines:

· ISSUE: The group of the -tools subpackage is wrong. The tools aren't libraries. I think "Applications/Text" is the best choice.

· ISSUE: The -doc subpackage should be noarch (and thus not use _isa).

Comment 29 Pavel Zhukov 2012-10-13 04:23:58 UTC
>   aws-doc.x86_64: E: devel-dependency aws(x86-64)-devel
removed

>   aws-tools.x86_64: W: no-manual-page-for-binary wsdl2aws
>     → See the separate point below.
>   aws.src: W: invalid-url Source1: aws-fedora.tgz
>     → I assume that you're going to check that file into Git. Otherwise you
> would need to provide a URL to it.
Added github URL to comment

> · The package must meet the Packaging Guidelines.
>   → ISSUES:
>     · The -doc subpackage should be in the Documentation group.
fixed

>     · The comment about the SSL versions patch needs to be expanded to
> explain whether you have sent the patch upstream, or whether it's
> Fedora-specific for some reason.
fixed 
>     · It's unclear whether IPv6 is enabled. build_aws.gpr points to both
> aws-net-std__gnat.adb and aws-net-std__ipv6.adb as implementations of
> AWS.Net.Std, and it's not easy to know which of them takes effect. aws.gpr
> only points to aws-net-std__gnat.adb.
FIXME

> · The License field in the package spec file must match the actual license.
>   → ISSUE: As far as I can see the license should be "GPLv3+ with exceptions
> and GPLv2+" on the main package, and "GPLv3+" on aws-tools. (Memory_Streams
> is GPLv2+.)
Fixed
> 
> · 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.
>   → NOTE: A copy of COPYING3 must be included in aws-doc if its dependency
> on aws-devel is removed.
COPYING3 is bringing with main package all subpackages are depends from main. Do I have to inclide COPYING3 in aws-doc as well?
> · The package must NOT bundle copies of system libraries.
> 
>   → ISSUE: Zlib-Ada is now packaged so include/zlib* must be deleted. "-lz"
> should be deleted from build_aws.gpr, and «with "zlib_ada";» added to
> build_aws.gpr and aws.gpr. The build dependencies and the dependencies of
> aws-devel must reflect this.
Fixed

> 
> Ada-specific MUST items:
> 
> · The package must have "BuildRequires: gcc-gnat".
>   → ISSUE: gcc-gnat is missing from the build dependencies.
Main and -devel packages depend from fedora-gnat-projects-common which depend from gcc-gnat. 
package: fedora-gnat-project-common.noarch 3.5-1.fc17
  dependency: gcc-gnat
   provider: gcc-gnat.x86_64 4.7.2-2.fc17


> 
> · 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}".
>   → NOTE: Once the package is imported and the branches have been set up,
> please change to "ExclusiveArch: %{GNAT_arches}" in the f17, f18 and master
> branches.
OK
> 
> · The reviewer should test that the package functions as described.
>   → ISSUES: Trying to compile the demos reveals several problems:
>     · aws.gpr tries to import "aws/template_parser", which doesn't exist.
>     · aws.gpr imports xmlada.gpr, so aws-devel must require xmlada-devel.
>     · AWS speces reference Templates Parser speces, so aws.gpr must import
> templates_parser.gpr and aws-devel must require templates_parser-devel.
>     · aws.gpr points to aws-net-ssl__openssl.adb instead of
> aws-net-ssl__gnutls.adb.
Ohh. I'll patch samples :( 


> · 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 awsres and wsdl2aws. Debian has man
> pages however, so you should try to adapt those for Fedora. In a quick check
> I found one thing that needs to be adjusted: They refer to the manual in the
> -doc subpackage, but Debian's package has a different name. You could change
> it to "aws-doc", but it might be better to remove the package name so that
> the man pages can be upstreamed.
I'll check debian package but I wouldn't like to bring man pages from anywhere and patch it. It's hard to follow any changes in man pages or something else.
> 
> Common sense stuff not mentioned in the guidelines:
> 
> · ISSUE: The group of the -tools subpackage is wrong. The tools aren't
> libraries. I think "Applications/Text" is the best choice.
OK
> · ISSUE: The -doc subpackage should be noarch (and thus not use _isa).
Fixed

Comment 30 Björn Persson 2012-10-13 06:14:20 UTC
(In reply to comment #29)
> > · The reviewer should test that the package functions as described.
> >   → ISSUES: Trying to compile the demos reveals several problems:
> >     · aws.gpr tries to import "aws/template_parser", which doesn't exist.
> >     · aws.gpr imports xmlada.gpr, so aws-devel must require xmlada-devel.
> >     · AWS speces reference Templates Parser speces, so aws.gpr must import
> > templates_parser.gpr and aws-devel must require templates_parser-devel.
> >     · aws.gpr points to aws-net-ssl__openssl.adb instead of
> > aws-net-ssl__gnutls.adb.
> Ohh. I'll patch samples :( 

By all means don't start patching the demos! Fix aws.gpr!

Comment 32 Björn Persson 2012-10-27 19:39:30 UTC
(In reply to comment #29)
> > · The License field in the package spec file must match the actual license.
> >   → ISSUE: As far as I can see the license should be "GPLv3+ with exceptions
> > and GPLv2+" on the main package, and "GPLv3+" on aws-tools. (Memory_Streams
> > is GPLv2+.)
> Fixed

Version three on the tools, not two.

> >   → NOTE: A copy of COPYING3 must be included in aws-doc if its dependency
> > on aws-devel is removed.
> COPYING3 is bringing with main package all subpackages are depends from
> main. Do I have to inclide COPYING3 in aws-doc as well?

I don't see why the -doc subpackage would depend on the main package. The shared library file is required by programs that are linked to AWS. Users can read the documents just fine without having the library installed. Did you add this dependency only to avoid including a copy of COPYING3 in aws-doc? The way I read the guidelines, subpackages aren't supposed to pull in other packages only for the license file. Subpackages that depend on a base package for other reasons don't need to include license files, but the way to add a license file to an independent subpackage is to include a copy of the file rather than adding an otherwise unnecesssary dependency.

Relevant sections:
https://fedoraproject.org/wiki/Packaging:Guidelines#Documentation
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing

> >   → ISSUE: Zlib-Ada is now packaged so include/zlib* must be deleted. "-lz"
> > should be deleted from build_aws.gpr, and «with "zlib_ada";» added to
> > build_aws.gpr and aws.gpr. The build dependencies and the dependencies of
> > aws-devel must reflect this.
> Fixed

aws-devel also needs to require zlib-ada-devel, and build_aws.gpr still contains "-lz" in two places.

If you still can't get the package to build without "-lz", then I need more information about what errors you get and how to reproduce the problem.

> > · The package must have "BuildRequires: gcc-gnat".
> >   → ISSUE: gcc-gnat is missing from the build dependencies.
> Main and -devel packages depend from fedora-gnat-projects-common which
> depend from gcc-gnat.
> package: fedora-gnat-project-common.noarch 3.5-1.fc17
>   dependency: gcc-gnat
>    provider: gcc-gnat.x86_64 4.7.2-2.fc17

As I told you in the review of Matreshka, fedora-gnat-project-common does not abstract away Gnat. A package that requires Gnat to build shall say so in the spec file. As I wrote on the Ada mailing list, I'm considering removing fedora-gnat-projects-common's dependency on gcc-gnat.

> I'll check debian package but I wouldn't like to bring man pages from
> anywhere and patch it. It's hard to follow any changes in man pages or
> something else.

That's why the policy says that you should try to get the man pages included upstream.

Comment 33 Björn Persson 2012-10-27 19:52:58 UTC
Something weird happened to the metadata on this review request. I'm trying to clean it up.

Comment 34 Pavel Zhukov 2012-10-28 10:54:47 UTC
new build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4633190
- Remove "-lz" flag
- Remove dependencies -doc from base package
- Fix tools license
- Add man pages

Comment 35 Björn Persson 2012-11-01 20:31:32 UTC
I'm now reviewing this package:
http://landgraf.fedorapeople.org/packages/requested/aws/aws-2.11.0-10.fc17.src.rpm

Three issues remain:

1: build_aws.gpr still contains "-lz" in one place.

2: aws-devel still doesn't require zlib-ada-devel.

3: gcc-gnat is still missing from the build dependencies.

Comment 36 Pavel Zhukov 2012-11-02 06:56:33 UTC
Björn 
I'm sorry. I have to rest more....
SRPM: http://landgraf.fedorapeople.org/packages/requested/aws/aws-2.11.0-11.fc17.src.rpm

Comment 37 Björn Persson 2012-11-02 22:29:04 UTC
All the issues have now been solved. AWS is APPROVED.

Comment 38 Pavel Zhukov 2012-11-03 11:52:57 UTC
thank you for review and for your patience.

Comment 39 Pavel Zhukov 2012-11-03 11:53:42 UTC
New Package SCM Request
=======================
Package Name: aws
Short Description: ada web server
Owners: landgraf
Branches: f18
InitialCC:

Comment 40 Gwyn Ciesla 2012-11-03 15:58:45 UTC
Git done (by process-git-requests).

Comment 41 Fedora Update System 2012-11-03 17:27:05 UTC
aws-2.11.0-11.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/aws-2.11.0-11.fc18

Comment 42 Fedora Update System 2012-11-03 19:26:21 UTC
aws-2.11.0-11.fc18 has been pushed to the Fedora 18 testing repository.

Comment 43 Fedora Update System 2012-12-20 16:18:43 UTC
aws-2.11.0-11.fc18 has been pushed to the Fedora 18 stable repository.