[Bug 746848] Review Request: dbus-sharp - C# bindings for D-Bus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=746848

Christian Krause <chkr@xxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?(chkr@xxxxxxxxxxx) |

--- Comment #4 from Christian Krause <chkr@xxxxxxxxxxx> 2011-10-18 18:14:02 EDT ---
Thank you very much for the review.

(In reply to comment #3)
> Review of dbus-sharp-0.7.0-1: NEEDSWORK
> === NEEDS WORK ===
> These issue(s) are problematic and prevent the package from being approved.
> 
> (1) rpmlint output is not clean on the binary RPMs:
> 
>  dbus-sharp.x86_64: E: no-binary
>  dbus-sharp.x86_64: W: only-non-binary-in-usr-lib
>  dbus-sharp-devel.x86_64: W: no-documentation 
>  2 packages and 0 specfiles checked; 1 errors, 2 warnings.
> 
> According to Packaging:Mono on the Wiki [1], it should install its files in the
> GAC in /usr/lib or /usr/lib/dbus-sharp, and *not* in an arch-specific location
> such as /usr/lib64. That should fix the first two complaints. The
> no-documentation warning is probably safe to ignore; but if you'd want to do so
> for clarity, you could opt to put a copy of the COPYING and/or README file in
> that too.

Although the current packaging guidelines already refer to the new directory
scheme, it is not yet implemented. Even the mono core packages still expect the
old-style arch-independent paths. We are currently in the process of updating
rawhide, but until this has finished, we still have to use the old paths.

For reference:
https://fedoraproject.org/wiki/User:Chkr/MonoMultiarchChanges
https://fedorahosted.org/fpc/ticket/91

>  dbus-sharp.x86_64: E: no-binary
>  dbus-sharp.x86_64: W: only-non-binary-in-usr-lib

These two warnings are "normal" for mono packages (even if we would solely use
/usr/lib), they are false positives caused by the fact that mono assemblies are
not treated as binaries.

>  dbus-sharp-devel.x86_64: W: no-documentation 

I have not found an API documentation in the package, so I would rather leave
it as it is than to package the README again just for keeping rpmlint quiet.
;-)

> (2) After compilation and installation to the buildroot, the DLL needs to be
> registered with gacutil. (This is also explained on the wiki page.) Add
> something like the following to your %install section:
> 
>  gacutil -i mono/dbus-sharp/dbus-sharp.dll -f -package dbus-sharp -root
> %{buildroot}/usr/lib
> 
> (You may need to adjust the directory as necessary.)

That's only necessary if it is not done automatically via make. In this case it
happens in the "make install" step.

To verify you can look into the binary package:

rpm -qlp /tmp/dbus-sharp-0.7.0-1.fc17.i686.rpm 
/usr/lib/mono/dbus-sharp-1.0
/usr/lib/mono/dbus-sharp-1.0/dbus-sharp.dll
/usr/lib/mono/gac/dbus-sharp
/usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5
/usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5/dbus-sharp.dll
/usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5/dbus-sharp.dll.config
/usr/lib/mono/gac/dbus-sharp/1.0.0.0__5675b0c3093115b5/dbus-sharp.dll.mdb
[...]

The assembly is already correctly installed/registered in the GAC.

> (3) The %defattr line in your %files section is not necessary with current RPM
> versions. Please remove it. (This particular one is a nitpick of mine. It's
> technically optional; but I feel the spec file would be cleaner with it
> removed.)

Fixed.

Here is now the updated package:

Spec URL: http://chkr.fedorapeople.org/review/dbus-sharp.spec
SRPM URL: http://chkr.fedorapeople.org/review/dbus-sharp-0.7.0-2.fc17.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]