Bug 532590 - Review Request: yaws - Web server for dynamic content written in Erlang
Summary: Review Request: yaws - Web server for dynamic content written in Erlang
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-02 21:52 UTC by Lubomir Rintel
Modified: 2018-04-11 18:58 UTC (History)
7 users (show)

Fixed In Version: yaws-1.89-2.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-11-12 15:52:38 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Lubomir Rintel 2009-11-02 21:52:44 UTC
SPEC: http://v3.sk/~lkundrak/SPECS/yaws.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/yaws-1.85-1.fc11.src.rpm
scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1783838

Description:

HTTP 1.0 and HTTP 1.1 web server capable of both static content page
delivery and dynamic content generation using embedded Erlang code in the
HTML pages. It provides virtual hosting capabilities and implements
HTTP tracing and other debugging functionality such as interactive
interpreter environment. Performance can be boosted with built-in support
for RAM caching and streaming capabilities of dynamically generated
content. Among security features are SSL and support for WWW-Authenticated
pages.

Comment 1 Lubomir Rintel 2009-11-03 11:51:57 UTC
Fiddled with a package a bit to make it build in el5, but hussssh, don't tell anyone!

SPEC: http://v3.sk/~lkundrak/SPECS/yaws.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/yaws-1.85-1.fc11.src.rpm
scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1784864

Comment 2 Lubomir Rintel 2009-11-21 11:28:23 UTC
New package, modified to install to erlang lib directory and bundle source.
First change causes the package to be no longer noarch, despite it contains arch-independent bytecode only, because it installs file in %{_libdir} hierarchy. Source is essential for run-time debugging and is not split to separate subpackage since it's not done for other erlang packages (such as erlang itself).

SPEC: http://v3.sk/~lkundrak/SPECS/yaws.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/yaws-1.85-2.fc13.src.rpm

Comment 3 Lubomir Rintel 2009-11-21 11:29:50 UTC
New package, modified to install to erlang lib directory and bundle source.
First change causes the package to be no longer noarch, despite it contains arch-independent bytecode only, because it installs file in %{_libdir} hierarchy. Source is essential for run-time debugging and is not split to separate subpackage since it's not done for other erlang packages (such as erlang itself).

SPEC: http://v3.sk/~lkundrak/SPECS/yaws.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/yaws-1.85-2.fc13.src.rpm

Comment 4 Josephine Tannhäuser 2009-11-21 13:11:29 UTC
NOT OKAY - MUST: $ rpmlint 
yaws.i686: W: unstripped-binary-or-object /usr/lib/erlang/lib/yaws/priv/lib/setuid_drv.so
yaws.i686: W: no-soname /usr/lib/erlang/lib/yaws/priv/lib/setuid_drv.so
yaws.i686: W: unstripped-binary-or-object /usr/lib/erlang/lib/yaws/priv/lib/yaws_sendfile_drv.so
yaws.i686: W: no-soname /usr/lib/erlang/lib/yaws/priv/lib/yaws_sendfile_drv.so
yaws.i686: W: unstripped-binary-or-object /usr/lib/erlang/lib/yaws/priv/lib/epam
yaws.i686: E: zero-length /usr/lib/erlang/lib/yaws/src/charset.def
yaws-devel.i686: W: no-documentation
- You have *.so-files, so you should have a Debug-Package
- zero lenght? I think this can be removed
OK - MUST: Named according to the Package Naming Guidelines
OK - MUST: Spec file name matches the base package %{name}
OK - MUST: Package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines
OK - MUST: License field in spec file matches the actual license
NOT OKAY - MUST: License files included in %doc
./test/ibrowse/LICENSE is missing
(you have to rename it to avoid doubled filenames in %doc)
OK - MUST: Spec is in American English
OK - MUST: Spec is legible
OK - MUST: Sources match the upstream source by MD5
2d6bd52af002f356d6738900a67550c5531a0b4a
OK - MUST: Successfully compiles and builds into binary rpms on i686
CAN'T TEST, BECAUSE I HAVEN'T AN ACCESS TO KOJI - MUST: 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 - MUST: All build dependencies are listed in BuildRequires.
N/A - MUST: Handles locales properly with %find_lang
N/A - MUST: 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.
N/A - MUST: If the package is designed to be relocatable, the packager muststate this fact in the request for review.
OK - MUST: Owns all directories that it creates
OK - MUST: No duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: Package has a %clean section, which contains rm -rf %{buildroot}.
OK - MUST: Consistently uses macros
OK - MUST: Package contains code, or permissable content
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
N/A - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
NOT OKAY - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
N/A - MUST: If a package contains library files with a suffix, then library
files that end in .so must go in a -devel package.
N/A - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
OK - MUST: Package does not own files or directories already owned by other packages.
OK - MUST: At the beginning of %install, the package runs rm -rf %{buildroot}.
OK - MUST: All filenames valid UTF-8


SHOULD Items:
NOT OKAY, BECAUSE NOT ALL LICENSE TEXTS ARE IN A SEPERATE LICENSE FILE- SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: Builds in mock.
OK - SHOULD: Compiles and builds into binary rpms on all supported architectures.
OK - SHOULD: Functions as described.
OK - SHOULD: Scriptlets are used, those scriptlets must be sane.
OK - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
OK - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
N/A - SHOULD: 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.


Other items:
OK - latest stable version
OK - SourceURL valid
OK - Compiler flags ok
NOT OKAY - Debuginfo complete
You have .so-files you should have a debugpackage

Comment 5 Josephine Tannhäuser 2009-11-21 13:11:59 UTC
This is just a test review of the release 2 for myself, not an official one!

Comment 6 Simon 2009-11-21 14:10:44 UTC
It's not a test review for yourself, it's an informal one!


OK - MUST: License field in spec file matches the actual license

@Lubomir @Josephine: License must be LGPLv2+
<quote>
This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version..
</quote>
Josephine: Licensetexts with the "any later version" phrase, always means to add a + to the current license in the license-field in spec-file.



OK - MUST: Sources match the upstream source by MD5
2d6bd52af002f356d6738900a67550c5531a0b4a

@ Josephine: This is the sha1sum, but it is equal to the md5sum 6941ea52638805246973bf94fd6e9a52



CAN'T TEST, BECAUSE I HAVEN'T AN ACCESS TO KOJI - MUST: If the package does not
successfully compile, build or work on an architecture, then those
architectures should be listed in the spec in ExcludeArch.

should be okay, because Lubomir already built it in koji, see top post




N/A - MUST: devel packages must require the base package using a fully
versioned dependency


why N/A? The devel package meets this requirement. it's okay



OK - SHOULD: Functions as described.

You tested it? Me not, but i trust Lubomir that this pkg will work.


NOT OKAY, BECAUSE NOT ALL LICENSE TEXTS ARE IN A SEPERATE LICENSE FILE- SHOULD:
Source package includes license text(s) as a separate file.

It's just a should item and the guideline says:
If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
I leave it up to Lubomir, but this isn't a blocker!

Comment 7 Peter Lemenkov 2009-12-02 09:48:05 UTC
Yet another bunch of notes:

* Requires erlang-erlsom (needs patch - I'll try to provide it)
* Requires egssapi (still not packaged)

Comment 8 Peter Lemenkov 2010-02-09 12:46:24 UTC
Oh, I almost forgot about my promise to provide a patch (very simple one, actually). Here it is:

http://peter.fedorapeople.org/yaws--Use-system-wide-erlsom-header.patch

The only stopper remaining is the absence of egssapi in Fedora. Actually, I'm not sure, that egssapi really needed for normal operation.

Comment 9 Peter Lemenkov 2010-04-28 13:54:19 UTC
FYI - yaws 1.88 is out

Comment 11 Peter Lemenkov 2010-09-13 15:09:06 UTC
Simon, would you mind if I re-assign this ticket to myself?

Comment 12 Simon 2010-09-15 09:14:52 UTC
This pkg needs a lot of more love, and you are the erlang expert.
Your assignment makes more sense than mine and is a good idea. 
Maybe this is a hard-earned prestige-point for review :-)

Comment 13 Peter Lemenkov 2010-10-01 13:48:35 UTC
Hello All!
Upstream released ver. 1.89 which is now cleanly compiles in Koji - I strongly encourage you to update src.rpm. There are some notable (for packaging) changes since 1.88:

+ No longer required patch for compiling using system-wide erlsom.
+ Ver. 1.89 now requires erlang-ibrowse library

I also did some work on shortening list of BuildRequires - you need only the following Erlang libraries for building:

BuildRequires:  erlang-erts
BuildRequires:  erlang-erlsom
BuildRequires:  erlang-kernel
BuildRequires:  erlang-mnesia
BuildRequires:  erlang-sasl
BuildRequires:  erlang-ssl
BuildRequires:  erlang-stdlib

Unfortunately it doesn't significantly save compilation time.

Comment 14 Lubomir Rintel 2010-10-11 15:04:36 UTC
Peter, thank you. I'm wondering, what's blocking this review? Would it make sense for me to post an updated package now, or should I wait for the rest of review?

Comment 15 Peter Lemenkov 2010-10-11 15:10:48 UTC
(In reply to comment #14)
> Peter, thank you. I'm wondering, what's blocking this review? Would it make
> sense for me to post an updated package now, or should I wait for the rest of
> review?

Right now I'm only waiting for 1.89 src.rpm. Just provide it, and I'll finish this review (if no new issues will be discovered).

Comment 16 Lubomir Rintel 2010-10-20 14:39:50 UTC
Okie, thank you. Here you are:

SPEC: http://v3.sk/~lkundrak/SPECS/yaws.spec
SRPM: http://v3.sk/~lkundrak/SRPMS/yaws-1.89-1.fc13.src.rpm

Comment 17 Peter Lemenkov 2010-10-20 15:10:34 UTC
Good. One minor note - you accidentally forgot to actually list erlang-ibrowse as a dependency. 

I'll review it in a couple of hours.

Comment 18 Peter Lemenkov 2010-10-20 18:13:46 UTC
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is not silent but all its messages may be ignored.

sulaco ~/rpmbuild/SPECS: rpmlint ../RPMS/ppc/yaws-*
yaws.ppc: W: no-soname /usr/lib/erlang/lib/yaws/priv/lib/setuid_drv.so
yaws.ppc: W: no-soname /usr/lib/erlang/lib/yaws/priv/lib/yaws_sendfile_drv.so

^^^ this doesn't affect the runtime of the application. These libraries are designed to be dlopened. 

yaws.ppc: W: manual-page-warning /usr/share/man/man5/yaws_api.5.gz 201: warning: `yaws' not defined
yaws.ppc: W: manual-page-warning /usr/share/man/man5/yaws.conf.5.gz 231: warning: `pp' not defined
yaws.ppc: W: manual-page-warning /usr/share/man/man5/yaws.conf.5.gz 289: warning: `..' not defined

^^^ I'll report upstream about these warnings.

yaws.ppc: E: zero-length /usr/lib/erlang/lib/yaws/src/charset.def

^^^ this file is autogenerated and should be removed. In fact I'd rather to remove the entire %{_libdir}/erlang/lib/yaws/src contents - this is just a copy of the original sources, but I'm leaving it to you to decide. Just removing %{_libdir}/erlang/lib/yaws/src/charset.def would be enough to fix this particular warning.

yaws-devel.ppc: W: spelling-error %description -l en_US config -> con fig, con-fig, configure

^^^ false positive

yaws-devel.ppc: W: no-documentation

^^^ exactly what it says - no documentation for this package so far.

3 packages and 0 specfiles checked; 1 errors, 7 warnings.
sulaco ~/rpmbuild/SPECS: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

- The package does not fully meet the Packaging Guidelines. You need 

a) Actually add erlang-ibrowse as a runtime dependency.
b) remove empty %{_libdir}/erlang/lib/yaws/examples directories
c) I just found that this package requires erlang >= R13B (missing httpc:request/1, httpc:request/4 and httpc:set_options/1 functions, and I'm not sure that this can be fixed easily). So no luck for poor EL-5 users for now.
d) remove empty %{_libdir}/erlang/lib/yaws/src/charset.def (or even entire %{_libdir}/erlang/lib/yaws/src/ )

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

sulaco ~/rpmbuild/SOURCES: sha256sum yaws-1.89.tar.gz*
577c8adde339b700373b83b57b7eca952a41624fcf5f963e43977399de54f170  yaws-1.89.tar.gz
577c8adde339b700373b83b57b7eca952a41624fcf5f963e43977399de54f170  yaws-1.89.tar.gz.1
sulaco ~/rpmbuild/SOURCES: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture (my ppc on F-12).
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files in some of the dynamic linker's default paths.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
+ The pkgconfig(.pc) file is stored in a -devel package.
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
+ The -devel package requires the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).


So, please, fix the remaining four issues and I'll finish this review.

Comment 20 Peter Lemenkov 2010-10-21 08:35:58 UTC
Great. Now I don't see any other blocking issues so this package is

APPROVED.

Comment 21 Lubomir Rintel 2010-10-21 12:31:38 UTC
Thanks a lot!

New Package SCM Request
=======================
Package Name: yaws
Short Description: Web server for dynamic content written in Erlang
Owners: lkundrak peter
Branches: f12 f13 f14 el6

Comment 22 Kevin Fenzi 2010-10-21 13:24:03 UTC
Git done (by process-git-requests).

Comment 23 Peter Lemenkov 2010-11-12 15:52:38 UTC
Ok, yaws was built and pushed to F-14, so it's time to close this ticket.


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