So I'll start with the painful stuff: 

There are bundled libraries in WebCalendar which will need to be packaged separately.  

Those I have found in my brief initial search: 

hkit ( includes/classes/hKit ) 
http://allinthehead.com/code/hkit/
Illustrating one of the problems with bundled libraries, this particular library is at 0.4, while upstream has been at 0.5 for at least a year. 

PHPMailer ( includes/classes/phpmailer ) 
http://sourceforge.net/projects/phpmailer
This appears to be using a version that's almost 6 years old, while upstream has been releasing as recently as this week. 


captcha ( includes/classes/captcha )
In addition to being a bundled library that needs to be separately packaged:

The original was Public Domain, changes were made by Ray Jones, but with no indication of how his changes were licensed. While I am sure a very liberal reading would mean that this is also under public domain, given the difficulty of actually giving up rights available under copyright, I'd feel better if Ray Jones communicated a specific license - lets at least try and ask him. 

Interestingly, the link to the freshmeat site no longer works. 

  


From a licensing perspective the icon bit below causes me a bit of concern (includes/menu/index.php):
 * This menu was created using some fantastic free tools out on the internet:
 *  - Most icons by everaldo at http://en.crystalxp.net/ (with his permission )
 *  - Javascript & CSS by JSCookMenu at http://www.cs.ucla.edu/~heng/JSCookMenu/

That said JSCookMenu at the above location is another bundled lib

Thanks for your review, David.

From your remarks, I can imagine I have to package all those bundled libraries separately myself, since none of them is available on Fedora yet. This may take a while. I'll attempt to do so in the very next weeks and submit separate review requests for them, so don't expect a lot of activity on the current one during this time.

The link to captcha is now http://freshmeat.net/projects/captchaphp. Ray Jones is one of WebCalendar developers, so I can imagine the modifications he made are public domain or GPL. But I'll ask him. Anyway, that will be another package...

Yes, and unfortunately I can empathize with your pain. 
Once you start getting some of the libraries packaged, please don't hesitate to ping me and I'll be happy to review those as well, and hopefully get things moving pretty quickly. 

Thanks

Many thanks for your cooperation and help proposals

New SRPM: http://monnerat.fedorapeople.org/WebCalendar-1.2.0-5.fc12.src.rpm
_ hkit unbundled. php-hkit (review request 505358) required
_ PHPMailer unbundled. php-PHPMailer (review request 505356) required
_ captcha unbundled. php-captchaphp (review request 505354) required
_ JSCookMenu unbundled. JSCookMenu (review request 505360) required

koji scratch build at http://koji.fedoraproject.org/koji/taskinfo?taskID=1405564

rpmlint output:
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.smtp.php /usr/share/php/class.smtp.php
WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar/settings.php apache
WebCalendar.noarch: E: non-readable /etc/WebCalendar/settings.php 0660
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/hKit/hkit.class.php /usr/share/php/hkit.class.php
WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar apache
WebCalendar.noarch: E: non-standard-dir-perm /etc/WebCalendar 0775
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.phpmailer.php /usr/share/php/class.phpmailer.php

New warnings (i.e.: dangling-symlinks) are symbolic links to files in the required packages mentioned above.

_ I did not contact Ray Jones, since its captcha updates are now deleted from the package.
_ There are still files in the menu subdir: these are WebCalendar-specific menu themes for JSCookMenu.
_ Everaldo's icons are retained for now: they are used by the specific menu themes. I left a question at WebCalendar's forum about icons licensing (https://sourceforge.net/forum/forum.php?thread_id=3299158&forum_id=11587), but I have no answer yet.
_ There is still a module I have tried to unbundle: popups.php. It uses some Javascript code (infobox.js) from Klaus Knopper's web site (http://www.knopper.net), but although this original code is GPL2, it has never been publicly released (there is no home page for this script: it is only accessible as a part of Knopper's site). In addition, both versions have been changed in incompatible ways. Thus unbundling this code and making it a maintainable package is hardly feasible: I suggest we live with it as now.
_ Unbundling captchaphp was a pain the the ass, because it was not initially intended to be used this way !

New version: http://monnerat.fedorapeople.org/WebCalendar-1.2.0-6.fc10.src.rpm
* Tue Jun 23 2009 Patrick Monnerat <pm> 1.2.0-6
- Patch "eventstatus" to allow rejecting an accepted event.
- Replace "ed" by "sed -i" in build script to get rid of ed requirement.
- Tag translation files.
- Relocate external classes to the proper subdirectories.

rpmlint output:
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.smtp.php /usr/share/php/PHPMailer/class.smtp.php
WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar/settings.php apache
WebCalendar.noarch: E: non-readable /etc/WebCalendar/settings.php 0660
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/hKit/hkit.class.php /usr/share/php/hkit/hkit.class.php
WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar apache
WebCalendar.noarch: E: non-standard-dir-perm /etc/WebCalendar 0775
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.phpmailer.php /usr/share/php/PHPMailer/class.phpmailer.php

Same remarks as above.

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1458827

Patrick: 

I'll get this reviewed today by COB in the states. 

Thanks for your work on this,

David

Hey Patrick I am still working through this review - and much later than I originally intended :( 
A quick question for you; where did all of the patches come from? 
Did you generate them? 
Did they come from upstream? 
Take a look at this: 
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment 

If this is stuff you generated - we need to push them to upstream and annotate the spec file. If these patches are from upstream we need to document that as well, though I don't think there is much of a point in doing so to each one.

Package review for webcalendar.

Package Review Guidelines

MUST: rpmlint must be run on every package. The output should be posted in the review.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ./WebCalendar.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ../SRPMS/WebCalendar-1.2.0-6.fc11.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[ke4qqq@nalleyt61 SPECS]$ rpmlint ../RPMS/noarch/WebCalendar-1.2.0-6.fc11.noarch.rpm 
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.smtp.php /usr/share/php/PHPMailer/class.smtp.php
WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar/settings.php apache
WebCalendar.noarch: E: non-readable /etc/WebCalendar/settings.php 0660
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/hKit/hkit.class.php /usr/share/php/hkit/hkit.class.php
WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar apache
WebCalendar.noarch: E: non-standard-dir-perm /etc/WebCalendar 0775
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.phpmailer.php /usr/share/php/PHPMailer/class.phpmailer.php
1 packages and 0 specfiles checked; 2 errors, 5 warnings.

OK: 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 package must meet the Packaging Guidelines .
OK: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines .

GPLv2 throughout source code and in GPL.html

OK: 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: The spec file must be written in American English.
OK: 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. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines 
[ke4qqq@nalleyt61 SOURCES]$ md5sum WebCalendar-1.2.0.tar.gz*
520bd108e9e4d2a14425d2b5b8cc2e1e  WebCalendar-1.2.0.tar.gz
520bd108e9e4d2a14425d2b5b8cc2e1e  WebCalendar-1.2.0.tar.gz.1

OK: The package MUST successfully compile and build into binary rpms on at least one primary architecture. 
NA: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. 
OK: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense.
NA: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
OK: 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. 
NA: 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. Without this, use of Prefix: /usr is considered a blocker. 
OK: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. 
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. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. 
OK: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). 
OK: Each package must consistently use macros. 
OK: The package must contain code, or permissable content. 


QUESTION: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). 
This is up to your discretion - I personally don't think it constitutes a separate subpackage - however upstream did package it separately. 
You may also want to consider moving this to %doc and symlinking datadir/name/docs to that directory. 


OK: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. 

NA: Header files must be in a -devel package. 
NA: Static libraries must be in a -static package. 
NA: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). 
NA: 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. 
NA: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} 
NA: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
NA: 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. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. 
OK: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. 
OK: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). 
OK: All filenames in rpm packages must be valid UTF-8. 


This is starting to look pretty close - I am going to reply to one other item in this review shortly.

(In reply to comment #5)
> New SRPM: http://monnerat.fedorapeople.org/WebCalendar-1.2.0-5.fc12.src.rpm
> _ hkit unbundled. php-hkit (review request 505358) required
> _ PHPMailer unbundled. php-PHPMailer (review request 505356) required
> _ captcha unbundled. php-captchaphp (review request 505354) required
> _ JSCookMenu unbundled. JSCookMenu (review request 505360) required

Wow, that's a ton of work - I know I have reviewed one, I'll check on the rest this weekend and see their status and if not reviewed will punch them out this weekend. 

<snip>


> 
> rpmlint output:
> WebCalendar.noarch: W: dangling-symlink
> /usr/share/WebCalendar/includes/classes/phpmailer/class.smtp.php
> /usr/share/php/class.smtp.php
> WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar/settings.php apache
> WebCalendar.noarch: E: non-readable /etc/WebCalendar/settings.php 0660
> WebCalendar.noarch: W: dangling-symlink
> /usr/share/WebCalendar/includes/classes/hKit/hkit.class.php
> /usr/share/php/hkit.class.php
> WebCalendar.noarch: W: non-standard-gid /etc/WebCalendar apache
> WebCalendar.noarch: E: non-standard-dir-perm /etc/WebCalendar 0775
> WebCalendar.noarch: W: dangling-symlink
> /usr/share/WebCalendar/includes/classes/phpmailer/class.phpmailer.php
> /usr/share/php/class.phpmailer.php
> 
> New warnings (i.e.: dangling-symlinks) are symbolic links to files in the
> required packages mentioned above.
> 
> _ I did not contact Ray Jones, since its captcha updates are now deleted from
> the package.

OK

> _ There are still files in the menu subdir: these are WebCalendar-specific menu
> themes for JSCookMenu.

OK

> _ Everaldo's icons are retained for now: they are used by the specific menu
> themes. I left a question at WebCalendar's forum about icons licensing
> (https://sourceforge.net/forum/forum.php?thread_id=3299158&forum_id=11587), but
> I have no answer yet.

So I see - 
Are the icons replaceable with the crystal stuff? 
The crystal project is LGPL (http://www.everaldo.com/crystal/?action=license) - so it works, but as you noted they aren't 1 to 1 exacts. 

This is the largest issue that is looming. 


> _ There is still a module I have tried to unbundle: popups.php. It uses some
> Javascript code (infobox.js) from Klaus Knopper's web site
> (http://www.knopper.net), but although this original code is GPL2, it has never
> been publicly released (there is no home page for this script: it is only
> accessible as a part of Knopper's site). In addition, both versions have been
> changed in incompatible ways. Thus unbundling this code and making it a
> maintainable package is hardly feasible: I suggest we live with it as now.

I don't foresee a problem with this at the moment, largely because the entire Javascript within the packaging guidelines is a murky matter. There may come a time in the future when this is different, but certainly for now this seems acceptable. 

> _ Unbundling captchaphp was a pain the the ass, because it was not initially
> intended to be used this way !  

I bet

  All patches are from me but one. They all have been reported some way upstream, except those designed for "unbundling" (this is specific to Fedora). Some patches have been accepted upstream (for next release ?), some others have not even been commented. None has been rejected. Commenting patches seemed enough to me (links are not eternal!) but I'll provide upstream links in the next spec version.

  Documentation tarball is ~600k: I think this is too small for a -doc subpackage. In addition, it provides the online user manual that is accessible from the WebCalendar's help menu.

There are 65 icons in the menu subdirectory :-( Replacing them one by one is a huge job. Since nobody answers to my question on this subject on the WebCalendar forum, I'll try to ask Everaldo himself (this can save hours of work and package obfuscation). More to come... stay tuned !

Many thanks for the time you spend on this

(In reply to comment #11)
>   All patches are from me but one. They all have been reported some way
> upstream, except those designed for "unbundling" (this is specific to Fedora).
> Some patches have been accepted upstream (for next release ?), some others have
> not even been commented. None has been rejected. Commenting patches seemed
> enough to me (links are not eternal!) but I'll provide upstream links in the
> next spec version.

WORKSFORME

> 
>   Documentation tarball is ~600k: I think this is too small for a -doc
> subpackage. In addition, it provides the online user manual that is accessible
> from the WebCalendar's help menu.

WORKSFORME
> 
> There are 65 icons in the menu subdirectory :-( Replacing them one by one is a
> huge job. Since nobody answers to my question on this subject on the
> WebCalendar forum, I'll try to ask Everaldo himself (this can save hours of
> work and package obfuscation). More to come... stay tuned !


Any response from Everaldo or others??? 


Have you seen this thread? 

https://www.redhat.com/archives/fedora-infrastructure-list/2009-July/msg00184.html

> Have you seen this thread?

No, I missed it. It seems that WebCalendar is pretty much expected !
By the way, the test instance linked from this thread (at http://publictest15.fedoraproject.org/calender/) uses Everaldo's icons...

> Any response from Everaldo or others???
No. Neither on the WebCalendar forum, nor from Everaldo himself, although I insisted for an answer. They may be on vacations or they just don't care/don't know :-(

So I think we must either live with the current icons assuming the "permission" statement is enough, or replace them in the RPM script (huge job and break of readability) or give up on this project (I'm not ready for that :-) Your opinion ?

Anyway, I still need 3 +ve reviews of unbundled packages (see comment #5): php-hkit is complete (thanks to you) and php-PHPMailer is assigned (though being reviewed slowly!).

Thanks for your support.

I mentioned the need for reviews and the icon licensing issue in my post to -infra. Hopefully someone will start on the reviews for the dependencies shortly, if not I'll grab them this weekend. 

I flagged this FE-LEGAL so we can get an opinion on this from people qualified to speak on the subject. 

Spot/other legal person: can you take a look at the icon licensing issue. Patrick has tried to contact upstream and original author with no response.

We can't live with the icons as is. Either remove them or replace them.

(We should also stop using them on publictest15 asap.)

Mo and the Fedora Design Team might be able to help whip up some freely licensed replacements for you.

Thank you Spot for your advice.

New version: http://monnerat.fedorapeople.org/WebCalendar-1.2.0-7.fc10.src.rpm
* Fri Aug 14 2009 Patrick Monnerat <pm> 1.2.0-7
- Patch and tarball "newmenuicons" to replace menu icons that have an unclear
  license.
- Upstream patch references added.

rpmlint output:
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.smtp.php /usr/share/php/PHPMailer/class.smtp.php
WebCalendar.noarch: E: non-readable /etc/WebCalendar/settings.php 0660
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/hKit/hkit.class.php /usr/share/php/hkit/hkit.class.php
WebCalendar.noarch: E: non-standard-dir-perm /etc/WebCalendar 0775
WebCalendar.noarch: W: dangling-symlink /usr/share/WebCalendar/includes/classes/phpmailer/class.phpmailer.php /usr/share/php/PHPMailer/class.phpmailer.php
1 packages and 0 specfiles checked; 2 errors, 3 warnings.

Same explanations as for 1.2.0-5

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1604916

Since all problematic icons have been removed and replaced, we can probably lift FE_LEGAL. David, I let you do it if you agree.

php-PHPMailer package has been approved, so we still have 2 blocking pending review requests:
_ php_captchaphp (review request 505354)
_ JSCookMenu (review request 505360)

I'll defer to spot for lifting FE-Legal. That said I'll try an do the balance of the review work this weekend and get all of the pending reviews for this package knocked out.

I think you're going to need to make a new custom tarball that doesn't have the icons, since it is not clear that we even have the permission to redistribute them in the tarball (and in the SRPM).

Sorry for not making that clear originally.

Thanks for the precision, Spot. I'll do it very soon.
But I wonder how to name and/or version the package, since this case is not listed in the package naming guidelines, and the packaged tarball will not match upstream's. I can imagine commenting the spec file like in the CVS snapshot case, but it is nether a prerelease, nor a postrelease (in fact, it is a custom release)... Can you help me on this topic ? Thank you.

(In reply to comment #19)
> Thanks for the precision, Spot. I'll do it very soon.
> But I wonder how to name and/or version the package, since this case is not
> listed in the package naming guidelines, and the packaged tarball will not
> match upstream's. I can imagine commenting the spec file like in the CVS
> snapshot case, but it is nether a prerelease, nor a postrelease (in fact, it is
> a custom release)... Can you help me on this topic ? Thank you.  

So, what you need to do is this:

Take the tarball upstream provides and unpack it. Then, remove the files which we do not have permission to redistribute, and then make a new tarball with the same basename as the upstream tarball, but append "-clean" to the end of it. Finally, in the spec file, refer to the -clean tarball (without a URL), and add comments that point to the original upstream tarball and how you generated the new -clean tarball:

# The upstream tarball contains icons which we could not determine the licensing # for. The original source was found here: http://foo.bar/baz.tar.gz
# To generate the clean tarball, run:
# tar xvfz baz.tar.gz
# rm -rf baz/icons/*.png
# tar cvfz baz-clean.tar.gz baz/
Source: baz-clean.tar.gz

Thanks Spot, but this was the "clear" part, and my question was not!

My question is about package NAMING: is is a fourth case (after prerelease, snapshot and postrelease) where the %release should contain a specific alpha string ? (AFAIK, these are the only kind of packages that are allowed to contain a main tarball that is not directly downloadable from upstream). Or do I keep the regular package naming, breaking the "downloadable from upstream" rule ?

Ah, I see. You're fine to keep the naming and versioning the same as if it was a normal package, just leave the "clean" off it.

OK, thanks for the info.
So here we are:
New version: http://monnerat.fedorapeople.org/WebCalendar-1.2.0-8.fc10.src.rpm
* Fri Aug 14 2009 Patrick Monnerat <pm> 1.2.0-8
- Use a custom source tarball to get rid of upstream icons with license issue.

rpmlint output unchanged.
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1605649

Lifting FE-Legal.

Hi Patrick:

sorry for the lag on my part. 

I don't see anything else blocking this. (There are the dependencies of course, but those are separate review requests.) but this request is APPROVED 

Thanks for all of your work on this.

... and many thanks for your work too, David.

I've noted this package is ready for CVS, but I will wait for the two other reviews, to avoid dependency problems in rawhide.

New Package CVS Request
=======================
Package Name: WebCalendar
Short Description: Single/multi-user web-based calendar application
Owners: monnerat
Branches: F-12 F-13
InitialCC:

CVS done (by process-cvs-requests.py).

WebCalendar-1.2.0-8.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/WebCalendar-1.2.0-8.fc13

WebCalendar-1.2.0-8.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/WebCalendar-1.2.0-8.fc12

WebCalendar-1.2.0-8.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

WebCalendar-1.2.0-8.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.