Bug 655599
Summary: | Review Request: remmina-plugins - Plugins for Remmina Remote Desktop Client | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Christoph Wickert <christoph.wickert> | ||||
Component: | Package Review | Assignee: | Thomas Spura <tomspur> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | bfay, bill-bugzilla.redhat.com, d.bz-redhat, fedora-package-review, mads, notting, tomspur, vengmd | ||||
Target Milestone: | --- | Flags: | tomspur:
fedora-review+
j: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | 0.9.2-1.fc15 | Doc Type: | Bug Fix | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2011-01-21 22:23:02 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 617144 | ||||||
Attachments: |
|
Description
Christoph Wickert
2010-11-21 22:27:18 UTC
(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). |