Spec URL: http://dominichopf.de/geany-plugins/geany-plugins.spec SRPM URL: http://dominichopf.de/geany-plugins/geany-plugins-0.17-5.fc11.src.rpm Description: eany-plugins is a bundle of several plugins. Plugins included are: Addons (various small addons); Geanygdb (provides integration with gdb); Geanylatex (improved support for LaTeX documents); Geanylipsum (for inserting blocks of Lorem Ipsum text); Geanylua (provides support for scripting with Lua); Geanysendmail (allows sending of documents from within Geany); Geanyvc (support for various version control systems); Shiftcolumn (for moving blocks of text horizontally); Spellcheck (for spell checking documents). Note: This is my first package for Fedora and I'm looking forward to get sponsored.
*** Bug 512216 has been marked as a duplicate of this bug. ***
Good: + Basename of the SPEC file matches with package name + Package name fullfill naming guidelines + URL tag shows on proper project homepage + Package contains most recent release of the application + Could download package tar ball via spectool -g + Package contains valid License tag + License tag states GPLV2+ as a valid OSS license + License state in the license tag matches with copyright notes on the souces + Package contains proper BuildRoot definition + BuildRoot will cleaned at the beginning of %clean and %install + Package contains verbain copy of the license tag + Package tar ball matches with upstream (md5sum: 9da6ab5bebd5e7de306b4dd3d1c9a5b4) + Consistently usage of rpm macros + Package has no subpackages + Package has a SMP-enabled build step + Local build works fine + Debuginfo package contains sources + Scratch build on koji works fine. + Package files has proper file permission + All packaged files are owned by the package + %file stanza has no duplicate entries + No package file belong to another packer + Package contains a small %doc stanza Bad: - You should start with review 1 for the Fedora package - Rpmlint has issues with source package: geany-plugins.src:64: W: macro-in-%changelog _datadir geany-plugins.src:67: W: macro-in-%changelog files geany-plugins.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12) 1 packages and 0 specfiles checked; 0 errors, 3 warnings. Please keep in mind to escape '%' in the changelog section with a additional '%' char. - Rpmlint complaints on binary package: geany-plugins.x86_64: W: obsolete-not-provided geanyvc geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins-0.17/geanyvc/AUTHORS geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins-0.17/geanylipsum/AUTHORS geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins-0.17/geanyvc/ChangeLog geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins-0.17/addons/ChangeLog 1 packages and 0 specfiles checked; 4 errors, 1 warnings. - Because the package contains empty document files I would to avoid the usage of the --docdir option on ./configure
Update with fixes available: Spec URL: http://dominichopf.de/geany-plugins/geany-plugins.spec SRPM URL: http://dominichopf.de/geany-plugins/geany-plugins-0.17-1.fc11.src.rpm Note: There is still an issue with the docs. geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins/geanyvc/AUTHORS geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins/geanyvc/ChangeLog geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins/geanylipsum/AUTHORS geany-plugins.x86_64: E: zero-length /usr/share/doc/geany-plugins/addons/ChangeLog I will talk to upstream because of this to get the files filled with additional information. Another idea I have is to write general documentation files for the whole geany-plugins project. I'll suggest this to upstream too.
Hi Dominic, Jochen, Removing the docdir congifure option is totally the wrong thing to do - not ALL of the doc files are empty, only some of them. Removing the empty files would be more appropriate. doc files are supposed to be installed under a versioned directory in /usr/share/doc By removing the docdir configure option, the doc files end up installed in /usr/share/doc/geany-plugins rather than /usr/share/doc/geany-plugins-0.17 Also, while I realize that Jochen requested the release number to be rewound to 1, it's a little bit disingenious to remove the changelog attribution to the work done by Pierre Yvess and myself in generating the spec file prior to review. It doesn't actually matter to me, and may not matter to Pierre either, but in general you shouldn't remove record of people's work as it might affect their employment when they work on things for Fedora as part of their day job. Say I was asked by my boss to package geany plugins and work on getting it into Fedora. If he looks now, it looks like I didn't do any work, and I get the sack (ok, this isn't a real example, but you see my point).
Also, please try to use %{name} rather than "geany-plugins" in the file list.
(In reply to comment #4) > Hi Dominic, Jochen, > > Removing the docdir congifure option is totally the wrong thing to do - not ALL > of the doc files are empty, only some of them. Removing the empty files would > be more appropriate. Instead of installing all files and removing some of them later IMO one can better remove them all from the wrong location and then use the %doc directive to install the ones needed: %install rm -rf $RPM_BUILD_ROOT make install -p DESTDIR=$RPM_BUILD_ROOT rm -rf $RPM_BUILD_ROOT%{_docdir}/%{name} ... %files -f %{name}.lang %defattr(-,root,root,-) %doc AUTHORS ChangeLog All-the-non-empty-files-you-need > 1, it's a little bit disingenious to remove the changelog attribution to the > work done by Pierre Yvess and myself in generating the spec file prior to > review. I fully agree with you. This spec is based on the old one from bug 512216 and this fact should be visible in to all users. (In reply to comment #5) > Also, please try to use %{name} rather than "geany-plugins" in the file list. Why? The guidelines only demand that macro usage is consistent. This means not to mix e. g. %{buildroot} and $RPM_BUILD_ROOT. The rest is up to the maintainer. Last but not least I suggest to split this package into different subpackages.
Incidentally, a one liner to remove zero length doc files would be something like find ${RPM_BUILD_ROOT}%{_geany_plug_docdir} -size 0 -exec rm -f '{}' \; which is more robust than removing them each by hand.
(In reply to comment #6) > (In reply to comment #5) > > Also, please try to use %{name} rather than "geany-plugins" in the file list. > > Why? The guidelines only demand that macro usage is consistent. This means not > to mix e. g. %{buildroot} and $RPM_BUILD_ROOT. The rest is up to the > maintainer. Because at present there is an *inconsistent* mixture of %{name} and "geany-plugins" used in the specfile. Consistency is the key here, I agree. > Last but not least I suggest to split this package into different subpackages. Yes - Dominic and I discussed this. The present barrier to doing that is that the po files are not split up for each plugin. Once that's resolved it would be possible to split them up. I think Dominic has a scheme to do that from the unified po files, but I'll let him respond on that :).
(In reply to comment #6) > > 1, it's a little bit disingenious to remove the changelog attribution to the > > work done by Pierre Yvess and myself in generating the spec file prior to > > review. > > I fully agree with you. This spec is based on the old one from bug 512216 and > this fact should be visible in to all users. Yes, but that spec file had the changelog entries from Pierre and myself removed as well, so the point stands.
(In reply to comment #6) > Instead of installing all files and removing some of them later IMO one can > better remove them all from the wrong location and then use the %doc directive > to install the ones needed: > > %install > rm -rf $RPM_BUILD_ROOT > make install -p DESTDIR=$RPM_BUILD_ROOT > rm -rf $RPM_BUILD_ROOT%{_docdir}/%{name} > ... > > %files -f %{name}.lang > %defattr(-,root,root,-) > %doc AUTHORS ChangeLog All-the-non-empty-files-you-need That's a good solution, but the issue is that there is a tree of doc files, one directory for each plugin. The %doc macro when used in that way flattens them all into one directory - is there a way around that? (This has puzzled me many times).
(In reply to comment #4) (In reply to comment #4) > Also, while I realize that Jochen requested the release number to be rewound to > 1, it's a little bit disingenious to remove the changelog attribution to the > work done by Pierre Yvess and myself in generating the spec file prior to > review. It doesn't actually matter to me, and may not matter to Pierre either, > but in general you shouldn't remove record of people's work as it might affect > their employment when they work on things for Fedora as part of their day job. > Say I was asked by my boss to package geany plugins and work on getting it into > Fedora. If he looks now, it looks like I didn't do any work, and I get the sack > (ok, this isn't a real example, but you see my point). I definitely see this point. Please note that it was not my intention to be disingenuous. My idea was to leave the fedora review and the changelog as it was with Pierres and your work. Since Jochen told me to start with review 1 I was not sure what the right way was. Would it be also ok if I mention your somewhere else, e.g. in the changelog text? I reverted back to the docdir-solution suggested by Jonathan so far to fix the issue with the versioned directory, but I still want to clarify the zero-length-files issue with upstream. Also replaced geany-plugins with the %{name} macro in the %files section. Updated files under same URLs: http://dominichopf.de/geany-plugins/geany-plugins.spec http://dominichopf.de/geany-plugins/geany-plugins-0.17-1.fc11.src.rpm md5sums to avoid confusion: 128df7eaa577b77024e9c84a94849c57 geany-plugins.spec aae25a11d3cf2ce8c464e6f41323bd63 geany-plugins-0.17-1.fc11.src.rpm
(In reply to comment #11) > (In reply to comment #4) > (In reply to comment #4) > > Also, while I realize that Jochen requested the release number to be rewound to > > 1, it's a little bit disingenious to remove the changelog attribution to the > > work done by Pierre Yvess and myself in generating the spec file prior to > > review. It doesn't actually matter to me, and may not matter to Pierre either, > > but in general you shouldn't remove record of people's work as it might affect > > their employment when they work on things for Fedora as part of their day job. > > Say I was asked by my boss to package geany plugins and work on getting it into > > Fedora. If he looks now, it looks like I didn't do any work, and I get the sack > > (ok, this isn't a real example, but you see my point). > I definitely see this point. Please note that it was not my intention to be > disingenuous. My idea was to leave the fedora review and the changelog as it > was > with Pierres and your work. Since Jochen told me to start with review 1 I was > not sure what the right way was. Well, probably a better option is just to keep the changelog entries in the spec, but change the release numbers to 0.1, 0.2, 0.3 etc.
New release is available: Spec URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins.spec SRPM URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins-0.17-4.fc11.src.rpm md5sums: ec184ee39ed6b9b251e065233e775647 geany-plugins.spec 21a212e88038a4f547cc52b8f5b76a72 geany-plugins-0.17-4.fc11.src.rpm What I actually changed since the last version was: - remove zero-length documentation files - fix the changelog, hopefully Jonathan is okay with this? :) - remove static *.la-files - split up packages
The guidelines mandate removal of *.la files for normal library files, but that doesn't apply for plugins, which often need the la files in order to function - do the plugins still work in the absence of the .la files? If so, great, but just thought I'd mention it.
The Plugins work fine without those *.la-files, yes. The neccessary files are the *.so-files which get loaded at the startup of Geany. (And some plugins need other files their self, of course.)
Excellent - great work in splitting the sub-packages out. Rebuilding in mock right now.
rpmlint *.rpm geany-plugins.x86_64: E: no-binary --> That's fine, metapackage. geany-plugins.x86_64: W: no-documentation --> Fine, metapackage. geany-plugins-common.x86_64: W: no-documentation 13 packages and 0 specfiles checked; 1 errors, 2 warnings. I would recommend calling the subpackages %{name}-foo (i.e. geany-plugins-foo) rather than geany-plugin-foo for consistency with other plugin bundles (eg. claws-mail-plugins-*). For the geany-plugins metapackage to serve any purpose you need to add Requires: %{name}-geanyvc %{name}-geanygdb %{name}-addons %{name}-geanylatex %{name}-geanylipsum %{name}-geanylua %{name}-geanysendmail %{name}-shiftcolumn %{name}-spellcheck
(In reply to comment #8) > (In reply to comment #6) > > The guidelines only demand that macro usage is consistent. This means not > > to mix e. g. %{buildroot} and $RPM_BUILD_ROOT. The rest is up to the > > maintainer. > > Because at present there is an *inconsistent* mixture of %{name} and > "geany-plugins" used in the specfile. Consistency is the key here, I agree. Let me repeat what I said before: The *macro* usage needs to be consistent, this means not mixing different macro styles. There is no need to replace every appearance of "geany-plugins" with a macro, this is up to the maintainer, e. g. many people consider using %{name} in the URL or Source0 tag, since it makes copy & paste harder. (In reply to comment #17) > I would recommend calling the subpackages %{name}-foo (i.e. geany-plugins-foo) > rather than geany-plugin-foo for consistency with other plugin bundles (eg. > claws-mail-plugins-*). Blame me, this was my suggestion. IMO it should be "plugin", because each package only contains a single plugin. We had this in other packages as well in the past (e. g. audaciuos-plugin-*), but obviously this has be changed in the meantime, so I agree with you.
(In reply to comment #18) > (In reply to comment #17) > > I would recommend calling the subpackages %{name}-foo (i.e. geany-plugins-foo) > > rather than geany-plugin-foo for consistency with other plugin bundles (eg. > > claws-mail-plugins-*). > > Blame me, this was my suggestion. IMO it should be "plugin", because each > package only contains a single plugin. We had this in other packages as well in > the past (e. g. audaciuos-plugin-*), but obviously this has be changed in the > meantime, so I agree with you. Hm. Spurred on by that comment, I just did a yum list \*plugin\* and there is a usage of both foo-plugin-bar and foo-plugins-bar. So, I am no longer sure which is correct! I can certainly see that your rationale for "-plugin-" makes sense. Perhaps this is a case of "leave it up to the packager". It's probably something that the FPC should standardize, as well.
Updated files: Spec URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins.spec SRPM URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins-0.17-5.fc11.src.rpm 8c9a9c0ab5f26113a49a11e81e7bc66c geany-plugins.spec 27cb75749e46e4dfbdfd9ec0c49dd639 geany-plugins-0.17-5.fc11.src.rpm I fixed the Requires in this release as Jonathan suggested and renamed the sub-packages back to geany-plugins-* from geany-plugin-*.
(In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #17) > > > I would recommend calling the subpackages %{name}-foo (i.e. geany-plugins-foo) > > > rather than geany-plugin-foo for consistency with other plugin bundles (eg. > > > claws-mail-plugins-*). > > > > Blame me, this was my suggestion. IMO it should be "plugin", because each > > package only contains a single plugin. We had this in other packages as well in > > the past (e. g. audaciuos-plugin-*), but obviously this has be changed in the > > meantime, so I agree with you. > > Hm. Spurred on by that comment, I just did a yum list \*plugin\* and there is a > usage of both foo-plugin-bar and foo-plugins-bar. So, I am no longer sure which > is correct! I can certainly see that your rationale for "-plugin-" makes sense. > Perhaps this is a case of "leave it up to the packager". It's probably > something that the FPC should standardize, as well. Jonathan was a bit faster with answering than me, so in the release 5 which I just posted the sub-packages got renamed to geany-plugins-*. I totally agree with Christophs point of view. It definitely does make sense to name the subpackages geany-plugin-* since they include just one plugin at a time (except geany-plugins-addons, in fact). Anyway I tend to leave the naming geany-plugins-* since the upstream project is named "geany-plugins".
(In reply to comment #19) > I just did a yum list \*plugin\* and there is a > usage of both foo-plugin-bar and foo-plugins-bar. So, I am no longer sure which > is correct! I think we should stick with plugins (plural) now because - it follows the source package name - it's what most other packages do if they are built from a source that includes several plugins. The *-plugin-* packages you see are usually built from standalone sources. > Perhaps this is a case of "leave it up to the packager". It's probably > something that the FPC should standardize, as well. IMO yes, just like the naming guidelines for panel applets/plugins. From my POV everything is fine now. What do you think, Jochen?
Shouldn't the spellcheck plugin also require an enchant backend eg. hunspell?
The spellcheck plugin requires just enchant. A backend like hunspell or aspell is required by enchant itself so that yum already would resolve this dependency. I think adding this dependency to the spellcheck plugin again is not necessary.
(In reply to comment #24) > The spellcheck plugin requires just enchant. A backend like hunspell or aspell > is required by enchant itself so that yum already would resolve this > dependency. I think adding this dependency to the spellcheck plugin again is > not necessary. Yes, you're right - I'd missed that enchant requires libhunspell. More coffee needed.
(In reply to comment #25) > Yes, you're right - I'd missed that enchant requires libhunspell. More coffee > needed. Don't worry about that. Just for Information: Upstream is planning to release geany-plugins-0.17.1 soon. This version will include miscellaneous fixes and also fill up the zero-length documentation files.
Excellent. Where are we with the review - is Christian mentoring and Jochen reviewing the package, or is Christoph taking the review as well? Dominic - I'd happily welcome help on the main geany package as well if you wanted - request co-maintainership of that package in pkgdb when you're all set up as a fedora contributor.
(In reply to comment #27) > or is Christoph taking the review as well? No need to, I trust in you and Jochen. I think we all (including Dominic) did a pretty great job so far and the spec is in good shape.
OK - hopefully Jochen is happy to approve the package now - I agree that the spec is in great shape, great work Dominic.
Oups I'm a bit late on this (I was away from the net for a couple of days). Just curiosity where: > - You should start with review 1 for the Fedora package comes from ? I never saw that before and have never been told so :-) Thanks to Christoph and Jonathan for the review and Dominic for the packaging. I'm looking forward to see it approved :-)
Thanks very much to all of you for your compliments! ;) As I told few days ago, geany-plugins 0.17.1 got released yesterday, so I rebuild geany-plugins with this version of geany-plugins. Spec URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins.spec SRPM URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins-0.17.1-1.fc11.src.rpm md5sums: 8ce2d3185063e56cb53f6d6fd4ad500f geany-plugins.spec c5d131c6f7b3be22224cf497e3a55fff geany-plugins-0.17.1-1.fc11.src.rpm As announced, this also includes the fixes for the zero-length documentation files, so that removing them is not necessary anymore. I also fixed the required geany version, since it is not necessary to require exactly the same version as geany-plugins has. This also caused a little conflict when you have Geany 0.17 and try to build geany-plugins 0.17.1. In fact, geany-plugins 0.17 does require Geany 0.16 or later. For the whole list of changes in geany-plugins, see http://www.geany.org/Main/20090721.
Good: + Could download of upstream tar ball via spectool -g + Package tar ball matches with upstream (md5sum: 9587a8d8680c73833c7c55b9e1fd58aa) + Local build works fine + Rpmlint is silent for source rpm * Scratch build on koji works fine. * Static liraries will been removed. * Provides/Obsoltes statement for renaming geanyvc subpackage seems ok. Bad: - I dislike the creation of the metapackage. So please remove all Req. to the supackages from the main package - I have founds Requires geany = %{version}. this should replaced into Requires geany >= %{reg_geany_ver} - Rpmlint complaints binary rpms: geany-plugins.x86_64: W: no-documentation geany-plugins-common.x86_64: W: no-documentation 12 packages and 0 specfiles checked; 1 errors, 2 warnings. - It's better to wrote Requires: geany-plugins-common = %{version}-%{release} instead of Requires: geany-plugins-common = %{version}
Updated again: Spec URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins.spec SRPM URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins-0.17.1-2.fc11.src.rpm md5sums b053df529867cd89ed4849e37b00e517 geany-plugins.spec 1af718c4fdc7952b9991f982e901b01a geany-plugins-0.17.1-2.fc11.src.rpm I fixed the required geany version also in the subpackages, thanks to Jochens hint. Also removed the requires to sub-packages to avoid building the metapackage since all geany plugins also can be installed by something like 'yum install geany-plugins-*'. Also fixed the requires of geany-plugins-common to include the release.
Bad: - Please remove the Requires: geany >= %{req_geany_vers} from the main package - Please change the rpm group of the main package from Development/Language to Development/Tools - Please change Provides: geanyvc = %{version} into Provides geanyvc = %{version}-%{release} - Remove the %files stanza for the main package entierely
Fixed in third release: Spec URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins.spec SRPM URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins-0.17.1-3.fc11.src.rpm md5sums 52254429d24cb9590b354629fe320b37 geany-plugins.spec be393a1b6140816cc7face0b3e3e144d geany-plugins-0.17.1-3.fc11.src.rpm
Jochen - what's the objection to the metapackage? I find it's generally useful to have a container package and it doesn't prevent installation of individual plugins if the user wishes. It's common and normal practice with other collections of plugins (eg. claws-mail-plugins, nagios-plugins, libextractor-plugins, audacious-plugins). AFAIK FPC haven't outlawed metapackages. [One might argue that the package grouping is better done with comps groups, but presently the functionality required for that is missing and it seems noone is working on it actively, so it's a moot point].
Dominic - a small stylistic note: you've moved the BuildRequires for gtk-spell etc down to the spell check plugin - it's generally better practice IMO to have the BuildRequires together up near the top with the main package stuff to avoid confusion.
Fixed Jonathans stylistic note in release 4: Spec URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins.spec SRPM URL: http://dmaphy.fedorapeople.org/geany-plugins/geany-plugins-0.17.1-4.fc11.src.rpm md5sums 0dd43379e4d449e60dcba9e503d8d0ef geany-plugins.spec 57d2109687e6169c3ad1cd9731de769b geany-plugins-0.17.1-4.fc11.src.rpm
*** Bug 512210 has been marked as a duplicate of this bug. ***
Ok, I have tanke a last look on the package and it's look nice, so the package is APPROVED and I will sponsored you.
New Package CVS Request ======================= Package Name: geany-plugins Short Description: A bundle of plugins for Geany Owners: dmaphy jgu pingou Branches: F-11 InitialCC:
cvs done.
Dominic & Pierre - since this obsoletes geanyvc, we can presumably retire that package for the releases that geany-plugins is built for. That process is described here: https://fedoraproject.org/wiki/PackageMaintainers/PackageEndOfLife
Sure I'll take care of this, shall I start it now or wait for geany-plugins to be in the repo/built ?
Don't think it matters too much, but might be cleaner to wait until geany-plugins hits the repos, I suppose.
Step 1 to 3 are done (4 is not needed), I am not sure about 5 and I this I will wait for geany-plugins to hits the repo to fill the ticket to rel-eng Thanks :)
For geany-plugins, Dominic requested a branch for F-11, and devel is automatically created. Since geany-plugins isn't being built for F-10, but geanyvc exists for F-10, I'd say that you don't need to do 5 (at least until F-10 is EOL). I presume you'll keep the geanyvc package in the repos for F-10?
yes I will keep it in F-10, so 5 will wait and I'll request only for F-11 and devel to rel-eng. Thanks :)
Just added geany-plugins to the Fedora CVS, built it and added it to the updates-testing repository. According to [1] the status is still pending at the present. Sorry for my delayed work on this. ;) [1] https://admin.fedoraproject.org/updates/geany-plugins-0.17.1-4.fc11
Should we close this bug ?
Since the review is done I think we could do so, yes.
Package Change Request ====================== Package Name: geany-plugins New Branches: el6 Owners: dmaphy jgu pingou InitialCC:
Git done (by process-git-requests).