Bug 835686 - Review Request: wine-mono - Mono library required for Wine
Review Request: wine-mono - Mono library required for Wine
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Cronenworth
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 834762
  Show dependency treegraph
 
Reported: 2012-06-26 16:16 EDT by Andreas Bierfert
Modified: 2012-07-05 11:23 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-07-05 11:23:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mike: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Andreas Bierfert 2012-06-26 16:16:07 EDT
Spec URL: http://fedora.lowlatency.de/review/mingw-wine-mono.spec
SRPM URL: http://fedora.lowlatency.de/review/mingw-wine-mono-0.0.4-4.fc17.src.rpm
Description: Mono library required for Wine.
Fedora Account System Username: awjb
Comment 1 Erik van Pienbroek 2012-06-26 16:59:24 EDT
I see you're not using mingw32-wine-mono or mingw64-wine-mono binary package names but put the .msi file in a binary rpm named mingw-wine-mono. Is this intentional? Does it contain both the win32 and win64 pieces?
Comment 2 Andreas Bierfert 2012-06-27 10:55:28 EDT
Yes this is intentional. I'd prefer a setup like wine-gecko but this is what wine currently supports. 

Here is the quote from: http://wiki.winehq.org/Mono
"Unlike gecko, there is only one package containing the code for both x86 and x86_64, as most of the code does not depend on the architecture."
Comment 3 Andreas Bierfert 2012-06-27 12:33:44 EDT
http://fedora.lowlatency.de/review/mingw-wine-mono.spec
http://fedora.lowlatency.de/review/mingw-wine-mono-0.0.4-5.fc17.src.rpm

https://koji.fedoraproject.org/koji/taskinfo?taskID=4201110

* Wed Jun 27 2012 Andreas Bierfert <andreas.bierfert[AT]lowlatency.de>
- 0.0.4-5
- add conditional so package builds on x86-64 builders as well
Comment 4 Kalev Lember 2012-06-28 07:07:52 EDT
Regarding the naming issue that Erik pointed out:

The MinGW Packaging Guidelines are for library packages that can be used for building Windows apps. But this package is different; it only installs a .msi and no dlls / header files and is apparently only meant for use within Wine.

As such, perhaps it would be clearer if it's called 'wine-mono'? This package is really just another component for the wine stack, even though it's built using the mingw cross compiler. I don't think the mingw naming guidelines are applicable here.
Comment 5 Andreas Bierfert 2012-06-28 09:45:58 EDT
I am fine with it either way. However, if we decide on wine-mono we should rename the gecko stuff accordingly...
Comment 6 Erik van Pienbroek 2012-06-28 14:02:58 EDT
I was just about to propose the exact same thing about the package naming :)
I'm +1 to use the package name wine-mono given the situation
Comment 7 Andreas Bierfert 2012-06-29 15:09:04 EDT
http://fedora.lowlatency.de/review/wine-mono.spec
http://fedora.lowlatency.de/review/wine-mono-0.0.4-6.fc17.src.rpm

* Fri Jun 29 2012 Andreas Bierfert <andreas.bierfert[AT]lowlatency.de>
- 0.0.4-6
- rename to wine-mono
Comment 8 Michael Cronenworth 2012-07-03 16:17:19 EDT
Taking for review.
Comment 9 Michael Cronenworth 2012-07-03 17:22:21 EDT
Even though you are not packaging a traditional MinGW package, you are using the MinGW toolkit to build it so I feel it should try to follow the MinGW packaging guidelines.

$ md5sum Downloads/wine-mono-0.0.4.tar.gz 
61c5ee49b8847c4dccfdab1fbc0706ae  Downloads/wine-mono-0.0.4.tar.gz
$ md5sum rpmbuild/SOURCES/wine-mono-0.0.4.tar.gz 
61c5ee49b8847c4dccfdab1fbc0706ae  rpmbuild/SOURCES/wine-mono-0.0.4.tar.gz

$ rpmlint rpmbuild/SPECS/wine-mono.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
$ rpmlint Downloads/wine-mono-0.0.4-6.fc17.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint /home/michael/rpmbuild/RPMS/noarch/wine-mono-0.0.4-6.fc17.noarch.rpm
wine-mono.noarch: E: incorrect-fsf-address /usr/share/doc/wine-mono-0.0.4/mono-COPYING.LIB
wine-mono.noarch: E: incorrect-fsf-address /usr/share/doc/wine-mono-0.0.4/mono-mcs-LICENSE.GPL
wine-mono.noarch: E: incorrect-fsf-address /usr/share/doc/wine-mono-0.0.4/mono-mcs-LICENSE.LGPL
1 packages and 0 specfiles checked; 3 errors, 0 warnings.

+ OK
! Needs to be looked into
/ Not applicable

[+] Compliant with generic Fedora Packaging Guidelines
[/] Source package name is prefixed with 'mingw-'
[!] Spec file starts with %{?mingw_package_header}
[!] BuildRequires: mingw32-filesystem >= 95 is in the .spec file
[!] BuildRequires: mingw64-filesystem >= 95 is in the .spec file
[/] Spec file contains %package sections for both mingw32 and mingw64 packages
[+] Binary mingw32 and mingw64 packages are noarch
[/] Spec file contains %{?mingw_debug_package} after the %description section
[/] Uses one of the macros %mingw_configure, %mingw_cmake, or %mingw_cmake_kde4
    to configure the package
[/] Uses the macro %mingw_make to build the package
[/] Uses the macro %mingw_make to install the package
[/] If package contains translations, the %mingw_find_lang macro must be used
[/] No binary package named mingw-$pkgname is generated
[/] Libtool .la files are not bundled
[/] .def files are not bundled
[/] Man pages which duplicate native package are not bundled
[/] Info files which duplicate native package are not bundled
[/] Provides of the binary mingw32 and mingw64 packages are equal
[/] Requires of the binary mingw32 and mingw64 packages are equal

The incorrect-fsf-address rpmlint warning should be reported upstream.

Please check the items I marked that need looking into before I pass the review:
-%?mingw_package_header should be %{?mingw_package_header}
-The BRs for the filesystem packages are missing.
Comment 10 Andreas Bierfert 2012-07-04 11:19:33 EDT
incorrect-fsf-address reported upstream: http://bugs.winehq.org/show_bug.cgi?id=31121

http://fedora.lowlatency.de/review/wine-mono.spec
http://fedora.lowlatency.de/review/wine-mono-0.0.4-7.fc17.src.rpm

* Wed Jul 04 2012 Andreas Bierfert <andreas.bierfert[AT]lowlatency.de>
- 0.0.4-7
- add mingw-filesystem BR
- fix header macro
Comment 11 Michael Cronenworth 2012-07-04 12:33:49 EDT
Looks good.

================================================
 The package wine-mono is APPROVED by mooninite
================================================
Comment 12 Andreas Bierfert 2012-07-04 12:43:45 EDT
New Package SCM Request
=======================
Package Name: wine-mono
Short Description: Mono library required for Wine
Owners: awjb
Branches: f17
Comment 13 Gwyn Ciesla 2012-07-04 17:08:13 EDT
Git done (by process-git-requests).
Comment 14 Andreas Bierfert 2012-07-05 11:23:02 EDT
Thanks for the review!

Note You need to log in before you can comment on or make changes to this bug.