Spec URL: http://thias.fedorapeople.org/review/elisa-plugins-good/elisa-plugins-good.spec SRPM URL: http://thias.fedorapeople.org/review/elisa-plugins-good/elisa-plugins-good-0.3.5-1.src.rpm Description: This package contains the good set of plugins for the Elisa Media Center. Note: This package is split off from the previous "elisa" package, which now requires it.
I suggest require'ing libvisual-plugins - the default config uses libvisual_jess as visualisation and Elisa doesn't say anything about the vis plugin being missing atm.
Adding dependency on the pymetar review, since the weather plugin in 0.5 seems to have switched from python-metar to pymetar.
Note that the package has now been updated to 0.5.3, and is still available here : http://thias.fedorapeople.org/review/elisa-plugins-good/
I can't build this package. There are circular dependencies. The elisa base package provides elisa and elisa-common (which in turn provides a virtual elisa-devel) but they both require elisa-plugins-*. elisa-plugins-good requires elisa-devel. Is there even a need for an elisa-common package? Can elisa or anything else run without the common package? Can you use some other tool with elisa-common without the elisa package? (I know not really part of this review).
To reply to the last comment, the "elisa-common" package is a hack to break the circular dependency that exists. Here's the problem : You need to install elisa before you can "build" and install the plugins. Installing from source, this isn't really a problem. But from packages, it is, as normal users will want to install only "elisa" and have it pull in all the plugins it needs to run. So for packages, we need elisa to require the plugins, but we need the plugins to buildrequire elisa. Circular dependency. To break that circular dependency, I've split out most of the "elisa" files into a bogus "elisa-common" package, which doesn't require the plugins. To build the plugins, this is fine. To make it more obvious, I've used a virtual provides/requires of "elisa-devel", but haven't named the package "devel" since it will be installed for normal runtime. I've double checked, and the "elisa-common" package doesn't require the plugins, and it shouldn't as explained above. I don't know how you ran into a problem here. The only drawback I can see here is that after removing "elisa", the plugins will be gone, but not the "elisa-common" package. The only solution I can think of would be to duplicate the files currently in "elisa-common" in "elisa" and rename the "common" to "devel". We'd have two packages nearly identical, with one used only for building the plugins.
Hi, The problem with "To break that circular dependency, I've split out most of the "elisa" files into a bogus "elisa-common" package, which doesn't require the plugins." is that the elisa-common rpm is built from the elisa src rpm so even by splitting it out you still end up with the circular dependency. From the quick test I did it seems you can build elisa fine without the plugins installed (and it will even run but it just won't be of any use). So I think to fix the issue you could depend on it for install but not for build or something similar but I'm not sure whether that would word either. Peter
I'm really confused as to what the problem you've encountered is, because things should be as follows, thus avoiding any circular dependency : * The "elisa" package DOES NOT "BuildRequires" any of the plugins * The "elisa" package DOES "Requires" the plugins * The "elisa-common" package DOES NOT "Requires" the plugins (and provides "elisa-devel") * The plugins DO NOT "BuildRequire" "elisa" (to avoid the circular dep!) * The plugins DO "BuildRequire" "elisa-devel" (provided by "elisa-common") * The plugins DO "Require" "elisa" So as long as you rebuild "elisa" first, then the plugins (in any order), things should "just work". Obviously, after rebuilding only "elisa", you can install "elisa-common" (aka "elisa-devel") BUT NOT YET "elisa". Would you care to detail what your problem is? Maybe I have something busted in my build environment which is hiding a problem from me.
(In reply to comment #5) > The only drawback I can see here is that after removing "elisa", the plugins > will be gone, but not the "elisa-common" package. The only solution I can think > of would be to duplicate the files currently in "elisa-common" in "elisa" and > rename the "common" to "devel". We'd have two packages nearly identical, with > one used only for building the plugins. I was about to suggest combining elisa with elisa-plugins-good, but if the GUI itself is in elisa-plugins-bad... How about making the %postun of elisa be 'rpm -e elisa-common' ? Not that it is that big of a deal.
(In reply to comment #8) > How about making the %postun of elisa be 'rpm -e elisa-common' ? Not that it is > that big of a deal. Eek, that's waaaay too ugly :-) If the current way is to be changed, it would be to have the two nearly identical packages, wasting a little bit of mirror server space (less than 1MB).
That might be a good idea -- 1 MB is nothing, really nowadays. So elisa-common can be called elisa-devel, and should only be used for building plugins. Will proceed with the review (reviewing -bad as well) once I could actually install the whole thing. I commented on the twisted-web2 review asking if you could upload the twisted-core package you used for building -web2.
Will these new elisa packages go into Fedora 9? Please update Elisa for F9 because it currently is broken :(
The "new" elisa sub-packages can't go into Fedora until they get reviewed and approved...
* The original elisa-common split was fine, IMO. No need to duplicate files. And no need to add a virtual elisa-devel. We have tools in non-devel packages that are BuildRequires. elisa-common can be BR, too. * The %description could explain briefly what "good set of plugins" means and why it matters. * 0.5.13 is available. Packaging-wise this is fine. Could be imported and developed further in cvs until the deps are ready, too.
(In reply to comment #13) > * The original elisa-common split was fine, IMO. No need to duplicate > files. And no need to add a virtual elisa-devel. We have tools in > non-devel packages that are BuildRequires. elisa-common can be BR, > too. Indeed. What about using the "elisa-base" name instead of common or devel? I just thought of it, as those files are indeed common to the runtime and the build of elisa, but not to be required by anything else, only build required. So the name "elisa-base" seems better suited, since those files are the basic elisa files. Sounds good? > * The %description could explain briefly what "good set of plugins" > means and why it matters. I'll update to : This package contains the good set of plugins for the Elisa Media Center, plugins which are considered stable and do not present any licensing issues. > * 0.5.13 is available. > > Packaging-wise this is fine. Could be imported and developed further > in cvs until the deps are ready, too. I'm assuming you mean the main "elisa" package. Seems like a good idea, I'll do that. Thanks for all your comments.
elisa-base or elisa-common, both names sound reasonable. And no, the bottom of my comment 13 referred to elisa-plugins-good actually, not the main elisa package.
(In reply to comment #15) > And no, the bottom of my comment 13 referred to elisa-plugins-good > actually, not the main elisa package. Do you mean importing the plugins spec files into the "elisa" CVS directory? That seems like abusing the CVS structure to me, so maybe I'm missing something.
This is the review of _elisa-plugins-good_ and I suggested: > Packaging-wise this is fine. Could be imported and developed > further in cvs until the deps are ready, too. That translates to "I think this could be approved, so further development [until the deps are ready] can be done in cvs". Note that the review ticket is not assigned to me.
(In reply to comment #17) > This is the review of _elisa-plugins-good_ and I suggested: Package reviews must follow the guidelines outlined here https://fedoraproject.org/wiki/Packaging/ReviewGuidelines > > Packaging-wise this is fine. Could be imported and developed > > further in cvs until the deps are ready, too. > > That translates to "I think this could be approved, so further > development [until the deps are ready] can be done in cvs". How can it be imported and developed in the RPM cvs if it won't build against rawhide due to the lack of deps which is one of the requirements of the package review?
What is your problem? cvs-import.sh takes src.rpm packages. There is no requirement for imported packages to be built in koji immediately. "elisa" in rawhide cvs is up-to-date, though, but it would not make sense to publish an incomplete set of builds. It's just more convenient to work on pkgs cvs rather than downloading a full set of src.rpms from an external web site.
Has this review stalled? elisa 0.5.20 is now in rawhide but has broken deps because it requires elisa-plugins-good, see bug #429590 comment #43.
Michel : Could you please check the latest package and resume the review? Now that web2 has been approved, the new plugins packages should be able to go in easily. http://thias.fedorapeople.org/review/elisa-plugins-good/elisa-plugins-good.spec http://thias.fedorapeople.org/review/elisa-plugins-good/elisa-plugins-good-0.5.22-1.src.rpm
> -License: GPLv3 > +# Each plugin has its license information in its setup.py > +License: GPLv3 and MIT Doubtful. While the tiny gnome plugins setup.py file lists the MIT licence and Benjamin Kampmann as the author (and using a non-Fluendo address that doesn't appear anywhere else in Elisa), the main plugin code uses the GPL v3 and lists Gernot Klimscha as the author. Enough reason to believe this thing is just GPLv3.
You're absolutely right. I'll report this so that the PKG-INFO file gets fixed. I'm sure it was different before, the field was wrong because the files were cleaned up recently. But I've now double checked and changed to plain "GPLv3". Fixed package available.
What is your estimated time frame of when working version of elisa will be in Fedora 10 repos? Is it few days, weeks or months? I'm just not a developer so I can't keep track of your conversations but would like to test it as soon as it gets into testing Fedora repos. Thank you very much for your hard work!
(In reply to comment #10) > Will proceed with the review (reviewing -bad as well) once I could actually > install the whole thing. I commented on the twisted-web2 review asking if you > could upload the twisted-core package you used for building -web2. All requirements are now in F-9 and F-10 too. Testing for the review should be very easy now! Michel : Could you resume the review? TIA!
Is this review going anywhere? The lack of this package is still causing broken deps in rawhide: Broken deps for i386 ---------------------------------------------------------- elisa-0.5.20-1.fc11.noarch requires elisa-plugins-good = 0:0.5.20 elisa-0.5.20-1.fc11.noarch requires elisa-plugins-bad = 0:0.5.20 elisa-0.5.20-1.fc11.noarch requires mgopen-fonts If this review is abandoned, I will try and pick it up.
The URL has changed, presumably? http://thias.fedorapeople.org/review/elisa-plugins-good/elisa-plugins-good-0.5.23-1.src.rpm Reviewing right now.
ReviewTemplate PASSED • rpmlint: Clean, but no documentation. See licensing comments below • package name • spec file name • package guideline-compliant • license complies with guidelines: Yes • license field accurate • license file not deleted • spec in US English • spec legible • source matches upstream • builds under >= 1 archs, others excluded noarch • build dependencies complete • own all directories • no dupes in %files • permission • %clean RPM_BUILD_ROOT • macros used consistently • Package contains code • clean buildroot before install • filenames UTF-8 SHOULD • if license text missing, ask upstream to include it License file seems to come with the base elisa distribution. Could upstream be asked to ship this with the plugins tarball as well? • package build in mock on all architectures • package functioned as described: Works, tested with audio playback on F10/x86_64. Note that video playback locks up the machine, but that is probably an Elisa / Elisa-plugins-bad problem • scriplets are sane • other subpackages should require versioned base • require package not files Approved.
Thanks a lot for the review!!!! (In reply to comment #28) > SHOULD > • if license text missing, ask upstream to include it > License file seems to come with the base elisa distribution. Could upstream > be asked to ship this with the plugins tarball as well? Since there are circular deps between the plugins and the base, the plugins can't be installed without the base, so I'd consider this very minor and wouldn't even bother with it. > • other subpackages should require versioned base That's a tricky bit. All of the packages are usually released with the same version, but it has happened to see only one be immediately replaced with an "n.1" version, making the whole "Requires: foo = %{version}" (strict version) quite tricky, as it could involve useless rebuilds. Maybe having all of the defaults be "Requires: foo >= %{version}" then exceptionally hardcoding to "Requires: foo >= 0.5.24" in a single packages updated to 0.5.24.1? > • require package not files Where is there a file requirement? Again, thanks for the review, we're now one step closer to get the Fedora elisa back into shape at last!
New Package CVS Request ======================= Package Name: elisa-plugins-good Short Description: Good Plugins for the Elisa Media Center Owners: thias Branches: F-9 F-10 InitialCC:
cvs done.
Thanks a lot Kevin. The package has been imported and built fine. We only have but #438609 to go before elisa is usable again in Fedora!