Spec URL: http://cwickert.fedorapeople.org/review/remmina-plugins.spec SRPM URL: http://cwickert.fedorapeople.org/review/remmina-plugins-0.8.4-1.fc14.src.rpm Description: Remmina is a remote desktop client written in GTK+, aiming to be useful for system administrators and travelers, who need to work with lots of remote computers in front of either large monitors or tiny netbooks. Remmina supports multiple network protocols in an integrated and consistent user interface. Currently NX, RDP, Telepathy, VNC, XDMCP and SSH are supported. This package contains the plugins for Remmina. Note: In order to build this package you will also need remmina.
(In reply to comment #0) > Note: In order to build this package you will also need remmina. That should read "you also remmina-0.8.3" http://cwickert.fedorapeople.org/review/remmina-0.8.3-1.fc14.src.rpm
I needed to get freerdp from koji, because it seems to not yet be on my mirror...
Mostly this package seems ok, but there are some spelling errors, so will have a closer look, when that is fixed: - %{name} is remmina-pluginS (with 's' at the end), but most subpackages don't have that 's'. It would be better to be consistend with 's' at the end. - It can't be installed because of: rpm -U ../x86_64/remmina-plugin-* ../x86_64/remmina-plugins-common-0.8.4-1.fc13.x86_64.rpm Fehler: Fehlgeschlagende Abhängigkeiten: freenx-client wird benötigt von remmina-plugin-nx-0.8.4-1.fc13.x86_64 remmina-plugins-plugins-common = 0.8.4-1.fc13 wird benötigt von remmina-plugin-nx-0.8.4-1.fc13.x86_64 remmina-plugins-plugins-common = 0.8.4-1.fc13 wird benötigt von remmina-plugin-rdp-0.8.4-1.fc13.x86_64 remmina-plugins-plugins-common = 0.8.4-1.fc13 wird benötigt von remmina-plugin-telepathy-0.8.4-1.fc13.x86_64 remmina-plugins-plugins-common = 0.8.4-1.fc13 wird benötigt von remmina-plugin-vnc-0.8.4-1.fc13.x86_64 remmina-plugins-plugins-common = 0.8.4-1.fc13 wird benötigt von remmina-plugin-xdmcp-0.8.4-1.fc13.x86_64 That's because you require %{name}-plugins where %{name} already contains the "plugins". Will do the rest of the review, when it's installable. :)
Created attachment 462224 [details] regularize the package names Here's a patch to fix the .spec per Thomas's suggestions. After doing that, I was able to build and install the RPM's and remmina found the plugins: Remmina plugin XDMCP (type=Protocol) registered. Remmina plugin telepathy (type=Entry) registered. Remmina plugin VNC (type=Protocol) registered. Remmina plugin VNCI (type=Protocol) registered. Remmina plugin NX (type=Protocol) registered. Remmina plugin SFTP (type=Protocol) registered. Remmina plugin SSH (type=Protocol) registered. I was then able to successfully connect to an NX server. Kinda slow, but the packaging worked OK for my case. Thanks, guys.
(In reply to comment #3) > Mostly this package seems ok, but there are some spelling errors, so will have > a closer look, when that is fixed: What spelling errors? My spell checker didn't find any. > - %{name} is remmina-pluginS (with 's' at the end), but most subpackages don't > have that 's'. > It would be better to be consistend with 's' at the end. The packages only contain one plugin each, this is why I used singular plugin instead of plugins. The packages ending with s will not show up in comps (but be pulled in through dependencies), so it is consistent to the user. > That's because you require %{name}-plugins where %{name} already contains the > "plugins". Ouch, my bad. I will fix provide a new package once we agreed on the naming.
(In reply to comment #5) > The packages only contain one plugin each, this is why I used singular plugin > instead of plugins. That's one way to think about it, and that's valid. I've grown used to thinking about rpm names as hierarchies. In that case: remmina |--------plugins |--------NX |--------VNC |--------etc. also makes sense. Nagios is packaged this way, as an example. Some, like nagios, have a meta package, nagios-plugins. This makes shell-glob operations easy for the sysadmin. e.g.: yum remove nagios-plugins\* Anyway, 'yum list | grep plugin' will show both opinions in use. There's also some elegance to being able to use %{name}-foo in RPM, rather than having to think about the correct pluralization for each sub-package. Seems to me, that'll lead to fewer errors for the next maintainer. I'm not informed enough to have an opinion on how this applies to non-English speakers - all I know is that not all languages pluralize the same way.
Thanks for your input. 'yum remove nagios-plugins\*' is something we indeed should consider. I didn't of this one, only of installation whee the plugin*s* packages will be pulled in. I will change the names accordingly.
Spec URL: http://cwickert.fedorapeople.org/review/remmina-plugins.spec SRPM URL: http://cwickert.fedorapeople.org/review/remmina-plugins-0.8.4-2.fc14.src.rpm
The missing plural 's' was my 'spelling error' from above... Thanks for fixing it :-) Review: - name ok - spec readable - %files ok - all packages require the common package -> licensing deps ok - BR ok - R seem ok - no static libs - no *.la - lang correctly installed - optflags used - parallel make - preserving timestamps everywhere - macros everywhere - rpmlint ok (22 wrong spelling errors and wrong no-documentaion) - source matches upstream: 873f1cf8afb46a911e1a4bd90d776f15 - license GPLv2+ ok (maybe) TODO: The description is confusing: "This package contains the plugins for Remmina." But 'this package' doesn't require it. So if you want to install all plugins with yum install remmina-plugins it won't work. How about requiring the subpackages all together to get that functionality? (just a MAYBE, not even a SHOULD) BLOCKER: - remmina-plugins bundles libvncserver, so you need an exception from FPC (or am I wrong here?)
(In reply to comment #9) > The description is confusing: > "This package contains the plugins for Remmina." > But 'this package' doesn't require it. So if you want to install all plugins > with yum install remmina-plugins it won't work. > How about requiring the subpackages all together to get that functionality? > (just a MAYBE, not even a SHOULD) As you can see from %files there is no remmina-plugins binary, the description is only for the SRPM. For the SRPM I consider the description correct, but I don't want to build a meta-package. Meta-packages are discouraged in Fedora. > BLOCKER: > - remmina-plugins bundles libvncserver, so you need an exception from FPC > (or am I wrong here?) Yes and no. :)If I were to use the bundled library, I'd need an exception. But thankfully I just need to use add "--enable-vnc=dl" (=dynamic library), then configure looks like this: Remmina-Plugins configure result: * RDP plugin: yes * VNC plugin: link to libvncclient externally * XDMCP plugin: yes * NX plugin: yes * Telepathy plugin: yes This got lost during the transition from remmina to remmina-plugins. Thanks for catching this!
(In reply to comment #10) > (In reply to comment #9) > > The description is confusing: > > "This package contains the plugins for Remmina." > > But 'this package' doesn't require it. So if you want to install all plugins > > with yum install remmina-plugins it won't work. > > How about requiring the subpackages all together to get that functionality? > > (just a MAYBE, not even a SHOULD) > > As you can see from %files there is no remmina-plugins binary, the description > is only for the SRPM. For the SRPM I consider the description correct, but I > don't want to build a meta-package. Meta-packages are discouraged in Fedora. Yes, I was thinking of a meta-package and the description sounds like one. But ok ;-) > > BLOCKER: > > - remmina-plugins bundles libvncserver, so you need an exception from FPC > > (or am I wrong here?) > > Yes and no. :)If I were to use the bundled library, I'd need an exception. But > thankfully I just need to use add "--enable-vnc=dl" (=dynamic library), then > configure looks like this: > > Remmina-Plugins configure result: > > * RDP plugin: yes > * VNC plugin: link to libvncclient externally > * XDMCP plugin: yes > * NX plugin: yes > * Telepathy plugin: yes > > This got lost during the transition from remmina to remmina-plugins. Thanks for > catching this! Ok, please try to delete the folder libvncserver in %prep, just to be sure it's not used ;-) __________________________________________________________________________ APPROVED
Hmm, the configure switch is no longer working properly, it's still building against the bundled library. I have opened a bug upstream at https://sourceforge.net/tracker/?func=detail&aid=3123529&group_id=278330&atid=1181674
OK, the bundled library thing was resolved. New Package SCM Request ======================= Package Name: remmina-plugins Short Description: Plugins for Remmina Remote Desktop Client Owners: cwickert Branches: InitialCC:
Git done (by process-git-requests).
http://koji.fedoraproject.org/koji/taskinfo?taskID=2735968