Bug 646611 - Rename review: drupal-cck -> drupal6-cck
Rename review: drupal-cck -> drupal6-cck
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
low Severity medium
: ---
: ---
Assigned To: Volker Fröhlich
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 646612 646663 InsightReviews
  Show dependency treegraph
 
Reported: 2010-10-25 14:31 EDT by Jon Ciesla
Modified: 2011-03-28 02:13 EDT (History)
5 users (show)

See Also:
Fixed In Version: drupal6-cck-2.9-1.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-02-23 16:53:01 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
volker27: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jon Ciesla 2010-10-25 14:31:28 EDT
Will be renaming entire drupal stack to drupal6, etc, to support parallell
installable drupal7 stack when that's available.

SRPMS: http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck-6.x.2.8-1.fc13.src.rpm
SPEC: http://zanoni.jcomserv.net/fedora/drupal6-cck/drupal6-cck.spec
Comment 1 Sven Lankes 2010-10-29 13:06:41 EDT
You should probably rename drupal-cck-fedora-README.txt to drupal6-cck-fedora-README.txt and maybe use %{name} there.
Comment 3 Volker Fröhlich 2010-12-20 16:30:12 EST
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.
Comment 4 Jon Ciesla 2011-01-06 13:02:48 EST
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.
Comment 5 Paul W. Frields 2011-01-06 13:59:38 EST
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".
Comment 6 Volker Fröhlich 2011-01-14 06:04:23 EST
Since the module uses Drupals CVS, it must be GPLv2+.

http://drupal.org/licensing/faq/#q4
Comment 7 Sven Lankes 2011-01-18 08:24:13 EST
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.
Comment 9 Paul W. Frields 2011-01-19 11:48:44 EST
Thanks Jon.  Volker, can you complete a full review as soon as practical?  We need this package badly for Insight.
Comment 10 Volker Fröhlich 2011-01-19 12:00:20 EST
Yes, within the next 8 hours.
Comment 11 Paul W. Frields 2011-01-19 20:11:19 EST
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.
Comment 12 Volker Fröhlich 2011-01-19 20:19:58 EST
I just wrote you an e-mail. I fear that all Provides/Obsoletes in the renamed packages are wrong. Even in the base package.
Comment 13 Volker Fröhlich 2011-01-19 21:57:59 EST
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):
Comment 14 Sven Lankes 2011-01-20 03:32:30 EST
(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.
Comment 15 Volker Fröhlich 2011-01-20 05:49:26 EST
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 >=.
Comment 16 Paul W. Frields 2011-01-20 09:58:22 EST
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?
Comment 17 Sven Lankes 2011-01-20 10:16:37 EST
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.
Comment 18 Jon Ciesla 2011-01-20 10:58:25 EST
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
Comment 19 Volker Fröhlich 2011-01-20 15:30:49 EST
[+] 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
--------
Comment 20 Jon Ciesla 2011-02-03 15:11:21 EST

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:
Comment 21 Kevin Fenzi 2011-02-06 17:48:23 EST
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
Comment 22 Fedora Update System 2011-02-07 09:29:27 EST
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
Comment 23 Fedora Update System 2011-02-07 09:29:39 EST
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
Comment 24 Jon Ciesla 2011-02-07 09:30:27 EST
Noted, thanks!
Comment 25 Fedora Update System 2011-02-07 12:53:59 EST
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
Comment 26 Fedora Update System 2011-02-23 16:52:50 EST
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.
Comment 27 Fedora Update System 2011-02-23 16:54:52 EST
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.
Comment 28 Fedora Update System 2011-03-23 09:45:43 EDT
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
Comment 29 Fedora Update System 2011-03-28 02:13:31 EDT
drupal6-cck-2.9-1.fc15 has been pushed to the Fedora 15 stable repository.

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