Bug 646611

You should probably rename drupal-cck-fedora-README.txt to drupal6-cck-fedora-README.txt and maybe use %{name} there.

SRPMS:
http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck-6.x.2.8-2.fc14.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck.spec

Fixed.

Please align Requires, URL and the likes.

The summary doesn't match upstream's and is not proper English. I personally think it's strange either way: "I can create a field with my browser now?"

The description has the same problems and the part about Drupal 4 and 5 is obsolete.

I think the version number for "Requires: drupal6" is unnecessary, since drupal6 will only contain version six -- no matter what.

"content-module" has no execution permissions, so you can drop the chmod.

Copying Source1 does not belong into the build section. Move it to the prep section. The %build section should remain empty here.

Maybe delete the text files from the buildroot at the end of the install section. That way you don't have to exclude them in the files section.

rm -f %{buildroot}%{drupaldir}/modules/cck/*.txt

or %exclude {drupaldir}/modules/cck/*.txt, for the sake of brevity.

I must have a look at the obsolete.

SRPMS:
http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck-6.x.2.8-3.fc14.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck.spec

All addressed, except I moved the cp into setup, because prep doesn't work.

The Requires: drupal6 will make this work for older Fedora branches, where the "drupal" package has a virtual Provides: drupal6, as well as the newer Fedora branches where the package is actually named "drupal6".

Since the module uses Drupals CVS, it must be GPLv2+.

http://drupal.org/licensing/faq/#q4

Jon,

you'll need to change the version to 2.8 from 6.x.2.8 

This is what started the renaming discussion - a version of 6.x.2.8 violates the packaging guidelines. Also the 6.x is implied by the package name drupal6-modulename now.

Fixed license and version.

SRPMS:
http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck-2.8-1.fc14.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck.spec

Thanks Jon.  Volker, can you complete a full review as soon as practical?  We need this package badly for Insight.

Yes, within the next 8 hours.

Awesome.  I've finished the reviews for all the other pending blockers on our InsightReviews tracker, so much appreciated for taking this one on Volker.

I just wrote you an e-mail. I fear that all Provides/Obsoletes in the renamed packages are wrong. Even in the base package.

Here we go:

The original drupal package has a provide called drupal6. As the cck spec file only requires drupal6, that won't work properly. If you only decide to update cck, but leave drupal as it is, the update will run and install cck into /usr/share/drupal6. I assume it would also delete the original module from /usr/share/drupal.

The way out might be declaring an epoch in the drupal6 package and require that in cck.

By the way, there is a new version of cck!

If you can find the time, please align the columns!

____________
drupal6.spec

...

Epoch:     1
Obsoletes: drupal <= 6.20

...
____________
drupal6-cck.spec

...

Requires:  drupal6 >= 2:6.20
Obsoletes: drupal-cck <= 6.x.2.8

...
____________

Please also notice the operator <= instead of >=! The operators are also wrong in the other renamed packages, I think.

Please always write detailed changelogs for the spec file.

The review so far:


[+] Good
[x] Needs work
[0] Does not apply

MUST:
=====

[+] Naming according to the Package Naming Guidelines
[+] Spec file matches base package name
[x] Packaging guidelines met: See comments on obsoletes
[+] License approved for Fedora
[+] License field in spec matches
[+] License file included, if source package includes it
[+] Spec in American English
[+] Spec is legible (but please align!)
[+] Sources match upstream md5sum: b4ee90587dacefcb290f7f9bbf49ea40
[+] Builds into binary RPMs on at least one primary architecture:
[0] ExcludeArch is specified and commented:
[0] All build dependencies listed
[0] Calls ldconfig for its shared libraries:
[0] No bundled system libraries
[0] Stated as relocatable package
[+] Owns all its directories or requires package that does
[+] No file listing duplicates
[+] File permissions correct
[+] Consistent use of macros
[+] Code or permissible content
[0] Large documentation in -doc subpackage
[+] No runtime dependency of files listed as %doc
[0] Header files in -devel subpackage
[0] Static files in -static subpackage
[0] Library files without suffix in -devel subpackage
[0] Devel-package requires base package
[0] No .la libtool archives
[0] GUI application includes properly installed %{name}.desktop file
[0] No files or directories owned, that other packages own
[+] Filenames in packages are UTF-8

SHOULD:
=======

[0] Query upstream if no license text is included
[+] Package builds in mock: (tested fedora-rawhide-x86_64)
[0] Scriptlets are sane, if used
[0] Subpackages other than -devel should require base package (versioned)
[0] pkgconfig files in -devel subpackage
[0] Dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself
[0] Contain man pages, where they make sense

I didn't try whether it works, but can't see a reason why it shouldn't.

MISSING 
=======

[] rpmlint: (will run it on the final file):

(In reply to comment #13)

> The original drupal package has a provide called drupal6. As the cck spec file
> only requires drupal6, that won't work properly. If you only decide to update
> cck, but leave drupal as it is, the update will run and install cck into
> /usr/share/drupal6. I assume it would also delete the original module from
> /usr/share/drupal.
> The way out might be declaring an epoch in the drupal6 package and 
> require that in cck.

The plan is to update the modules and the drupal package at the same time. Also this rename is only for rawhide. There shouldn't be a need for an epoch.

As Sven pointed out, since it is only for Rawhide, there is no danger of only updating the modules alone. So you can forget about the epoch, but the operator must still be <= instead of >=.

About the Obsoletes, I thought a little more about this:

1. Aren't future versions of the module still going to be called "drupal6-cck"? Wouldn't obsoleting the old name "drupal-cck," even with the larger EVR, work OK?

2. What about people who are upgrading from F14 to F15?  Wouldn't this obsolete be necessary since they'd be moving from a package named "drupal" using /usr/share/drupal to "drupal6" using /usr/share/drupal6?

Here is an example:

I have 

drupal-cck-6.x.2.7-1.fc14.noarch.rpm

installed on my F14 box.

On upgrading to F15, when the spec file has
Obsoletes: drupal-cck >= 6.x.2.8-1

then nothing will happen because 6.x.2.7-1 is not >= 2.x.2.8-1

But if the specfile reads:
Obsoletes: drupal-cck <= 6.x.2.8-1

magic will happen and yum/packagekit/whatever will install drupal6-cck.

We need to do them together because of the path change, an orthogonal problem to the RPM Requires.

Corrected obsoletes direction, updated to new version.

SRPMS:
http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck-2.9-1.fc14.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck.spec

[+] md5sum is 9e30f22592b7ecf08d020e0c626efc5b for 2.9.
[+] rpmlint:

drupal6-cck.noarch: W: obsolete-not-provided drupal-cck
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

Rationale: drupal-cck is not obsoleted, because the installation path for the new package is different.

--------
APPROVED
--------

New Package SCM Request
=======================
Package Name: drupal6-cck
Short Description: Allows you to add custom fields to nodes using a web browser
Owners: limb
Branches: EL-5 EL-6
InitialCC:

Git done (by process-git-requests).

PS: Could you stick to the normal format of summary for reviews? 
Anything else shows up as a flag in the processing script and also makes it harder
to find this review later... ie: 
Review Request: drupal6-whatever - whatever

drupal6-views-2.12-2.el6,drupal6-cck-2.9-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/drupal6-views-2.12-2.el6,drupal6-cck-2.9-1.el6

drupal6-views-2.12-2.el5,drupal6-cck-2.9-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/drupal6-views-2.12-2.el5,drupal6-cck-2.9-1.el5

Noted, thanks!

drupal6-views-2.12-2.el5, drupal6-cck-2.9-1.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update drupal6-views drupal6-cck'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/drupal6-views-2.12-2.el5,drupal6-cck-2.9-1.el5

drupal6-views-2.12-2.el5, drupal6-cck-2.9-1.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.

drupal6-views-2.12-2.el6, drupal6-cck-2.9-1.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.

drupal6-cck-2.9-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/drupal6-cck-2.9-1.fc15

drupal6-cck-2.9-1.fc15 has been pushed to the Fedora 15 stable repository.