Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: remmina-plugins - Plugins for Remmina Remote Desktop Client
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Summary: Review Request: remmina-plugins - Plugins for Remmina Remote Desktop Client Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: cwickert@fedoraproject.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |617144
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #1 from Christoph Wickert cwickert@fedoraproject.org 2010-11-21 17:31:21 EST --- (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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |tomspur@fedoraproject.org AssignedTo|nobody@fedoraproject.org |tomspur@fedoraproject.org
--- Comment #2 from Thomas Spura tomspur@fedoraproject.org 2010-11-22 06:38:58 EST --- I needed to get freerdp from koji, because it seems to not yet be on my mirror...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #3 from Thomas Spura tomspur@fedoraproject.org 2010-11-22 06:44:06 EST --- 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. :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #4 from Bill McGonigle bill-bugzilla.redhat.com@bfccomputing.com 2010-11-22 23:25:38 EST --- Created attachment 462224 --> https://bugzilla.redhat.com/attachment.cgi?id=462224 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #5 from Christoph Wickert cwickert@fedoraproject.org 2010-11-25 13:16:10 EST --- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #6 from Bill McGonigle bill-bugzilla.redhat.com@bfccomputing.com 2010-11-29 13:39:49 EST --- (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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #7 from Christoph Wickert cwickert@fedoraproject.org 2010-11-30 06:54:20 EST --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #8 from Christoph Wickert cwickert@fedoraproject.org 2010-11-30 07:13:11 EST --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #9 from Thomas Spura tomspur@fedoraproject.org 2010-11-30 07:48:24 EST --- 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?)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #10 from Christoph Wickert cwickert@fedoraproject.org 2010-11-30 08:25:42 EST --- (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!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #11 from Thomas Spura tomspur@fedoraproject.org 2010-11-30 08:47:58 EST --- (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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #12 from Christoph Wickert cwickert@fedoraproject.org 2010-11-30 09:39:01 EST --- 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=27...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #13 from Christoph Wickert cwickert@fedoraproject.org 2011-01-19 16:26:05 EST --- 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:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
--- Comment #14 from Jason Tibbitts tibbs@math.uh.edu 2011-01-19 16:59:08 EST --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=655599
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |0.9.2-1.fc15 Resolution| |NEXTRELEASE Last Closed| |2011-01-21 17:23:02
--- Comment #15 from Christoph Wickert cwickert@fedoraproject.org 2011-01-21 17:23:02 EST --- http://koji.fedoraproject.org/koji/taskinfo?taskID=2735968
package-review@lists.fedoraproject.org