Hide Forgot
Spec URL: http://fedora.lowlatency.de/review/mingw32-wine-gecko.spec SRPM URL: http://fedora.lowlatency.de/review/mingw32-wine-gecko-1.0.0-3.fc13.src.rpm Description: MinGW Gecko library required for Wine
https://koji.fedoraproject.org/koji/taskinfo?taskID=2082448
Hi, I'll leave the review up to somebody else as this package was initially created by me, but I was wondering if you already have patches ready for wine itself to use this package (as wine expects a .CAB file while this package provides a folder containing the gecko files) ?
I might have a look at this tomorrow, but I'm interested to know why wine needs a Win32-cross-compiled library?
See the discussion on the fedora-mingw mailing list from november last year: http://lists.fedoraproject.org/pipermail/mingw/2009-November/002267.html
Also take a look here: http://wiki.winehq.org/Gecko (In reply to comment #2) > by me, but I was wondering if you already have patches ready for wine itself to > use this package (as wine expects a .CAB file while this package provides a > folder containing the gecko files) ? Yes, I have a patch sitting here which adds this to latest wine.
I'm seeing a build failure on F-13 now, though F-14 (rawhide) seem okay. http://koji.fedoraproject.org/koji/taskinfo?taskID=2202453
https://koji.fedoraproject.org/koji/taskinfo?taskID=2290258 Just did another build on F-13 seems to work again (probably was a bug in the mingw stack). It would be nice if someone could review the package. It works quite well from me an we are very very close to the 1.2 release of wine and this would be a cool thing to have...
(In reply to comment #5) > > (In reply to comment #2) > > by me, but I was wondering if you already have patches ready for wine itself to > > use this package (as wine expects a .CAB file while this package provides a > > folder containing the gecko files) ? > > Yes, I have a patch sitting here which adds this to latest wine. Has the patch been sent upstream and did wine upstream accept it?
It has not been submitted upstream yet. It however is included in the latest wine builds in updates-testing. However once it has been tested a bit more I will send it upstream.
http://fedora.lowlatency.de/review/mingw32-wine-gecko-1.0.0-3.fc13.src.rpm does not build here on F13: Creating Resource file: module.res i686-pc-mingw32-windres -O coff --use-temp-file -DSQLITE_SECURE_DELETE=1 -DSQLITE_THREADSAFE=1 -DSQLITE_CORE=1 -DSQLITE_ENABLE_FTS3=1 -DOSTYPE=\"WINNT\" -DOSARCH=WINNT --include-dir /home/nerijus/rpmbuild/BUILD/wine-mozilla/db/sqlite3/src --include-dir /home/nerijus/rpmbuild/BUILD/wine-mozilla/db/sqlite3/src --include-dir . --include-dir ../../../dist/include --include-dir ../../../dist/include/nsprpub --include-dir /home/nerijus/rpmbuild/BUILD/wine-mozilla/wine_gecko/dist/include/nspr --include-dir /home/nerijus/rpmbuild/BUILD/wine-mozilla/wine_gecko/dist/include/nss -o module.res /home/nerijus/rpmbuild/BUILD/wine-mozilla/wine_gecko/db/sqlite3/src/module.rc i686-pc-mingw32-windres: /home/nerijus/rpmbuild/BUILD/wine-mozilla/wine_gecko/db/sqlite3/src/module.rc:54: syntax error make[5]: *** [module.res] Error 1 $ rpm -qf /usr/bin/i686-pc-mingw32-windres mingw32-binutils-2.19.51.0.14-1.fc12.i686 module.rc from line 53: 1 VERSIONINFO FILEVERSION 1,9,2,3845.95833333333 PRODUCTVERSION 1,9,2,3845.95833333333
Applying the following patch should fix the build (it truncates the last component of the generated versions): --- wine-gecko-1.0.0.orig/config/version_win.pl +++ wine-gecko-1.0.0/config/version_win.pl @@ -54,8 +54,8 @@ sub daysFromBuildID $d || die("Unrecognized buildid string."); my $secondstodays = 60 * 60 * 24; - return (POSIX::mktime(00, 00, 00, $d, $m - 1, $y - 1900) - - POSIX::mktime(00, 00, 00, 01, 00, 100)) / $secondstodays; + return int((POSIX::mktime(00, 00, 00, $d, $m - 1, $y - 1900) - + POSIX::mktime(00, 00, 00, 01, 00, 100)) / $secondstodays); } #Creates version resource file
After applying the patch there is another error when compiling wine-mozilla/media/libtheora/lib/dec/x86_vc/mmxfrag.c: In file included from /home/nerijus/rpmbuild/BUILD/wine-mozilla/media/libtheora/lib/dec/x86_vc/../../internal.h:25, from /home/nerijus/rpmbuild/BUILD/wine-mozilla/media/libtheora/lib/dec/x86_vc/mmxfrag.c:17: ../../../dist/include/theora/theora.h:192: warning: comma at end of enumerator list /home/nerijus/rpmbuild/BUILD/wine-mozilla/media/libtheora/lib/dec/x86_vc/mmxfrag.c: In function 'oc_frag_recon_intra_mmx': /home/nerijus/rpmbuild/BUILD/wine-mozilla/media/libtheora/lib/dec/x86_vc/mmxfrag.c:36: error: '_asm' undeclared (first use in this function)
Please clear the whiteboard if providing a version which builds.
The build can be fixed by applying this small patch: --- js/src/jslock.cpp.orig 2010-11-17 19:34:59.449610055 +0100 +++ js/src/jslock.cpp 2010-11-17 20:07:18.980134638 +0100 @@ -66,7 +66,7 @@ /* Implement NativeCompareAndSwap. */ -#if defined(_WIN32) && defined(_M_IX86) +#if defined(_WIN32) && defined(_M_IX86) && !defined(__MINGW32__) #pragma warning( disable : 4035 ) JS_BEGIN_EXTERN_C extern long __cdecl A scratch build of this was done at http://koji.fedoraproject.org/koji/taskinfo?taskID=2607153 For this build I added this patch to the wine-mozilla-gcc4-compile-fix.patch file
This patch does not help on F14, I still get error as in Comment 12. But there is no more error as in Comment 10 w/o applying patch in Comment 11, as it was on F13.
I don't know what's causing the problem in Comment 12, but I have figured out why the patch in Comment 11 was necessary previously: the daysFromBuildID function in config/version_win.pl extracts the build date from the build id, then calculates the number of days between the build date and Jan 1, 2000. This works fine just now because daylight savings time is not in effect; but at the time I posted Comment 11, it was, so the result of the calculation was one hour off and returned a non-integer value which caused the build to fail. So the patch in Comment 11 is still necessary for builds to work all year round...
*** Bug 675422 has been marked as a duplicate of this bug. ***
Regarding error in Comment 12 - it seems it tries to compile in wine-mozilla/media/libtheora/lib/dec/x86_vc directory, which is for Visual C, while it should probably compile in wine-mozilla/media/libtheora/lib/dec/x86. Am I right?
Hi all, I'm maintainer of Wine Gecko. We've just released the new version: http://www.winehq.org/pipermail/wine-devel/2011-March/089273.html This is the first version that makes it possible to do proper packaging, like you've tried to do. We've improved the building process a lot, so your problem is probably already fixed. Please let me know if you have any problems, so we can work on them.
All new fun build on f16 with cross enabled. Please test and leave feedback. Once all pieces of cross/mingw64 are in rawhide I hope someone will review: http://fedora.lowlatency.de/review/mingw-wine-gecko.spec http://fedora.lowlatency.de/review/mingw-wine-gecko-1.2.0-2.fc16.src.rpm http://fedora.lowlatency.de/review/mingw32-wine-gecko-1.2.0-2.fc16.noarch.rpm http://fedora.lowlatency.de/review/mingw64-wine-gecko-1.2.0-2.fc16.noarch.rpm
Very nice to see that you based this package on the new MinGW packaging guidelines (https://fedoraproject.org/wiki/Packaging:MinGW_Future) ! Some small comments: Please start the .spec file with these set of lines: %global __strip %{mingw_strip} %global __objdump %{mingw_objdump} %define __debug_install_post %{mingw_debug_install_post} And add a line with %{?mingw_debug_package} right before the start of the first %package tag. This is needed to automatically extract debug information I noticed you used the %{description} tag in some places. Last time I tried, this didn't give the expected results, so you may need to manually add a real description instead of this macro.
Thanks for you comments. Should be fixed/changed in this version: http://fedora.lowlatency.de/review/mingw-wine-gecko.spec http://fedora.lowlatency.de/review/mingw-wine-gecko-1.2.0-3.fc16.src.rpm http://fedora.lowlatency.de/review/mingw32-wine-gecko-1.2.0-3.fc16.noarch.rpm http://fedora.lowlatency.de/review/mingw64-wine-gecko-1.2.0-3.fc16.noarch.rpm
The mingw-w64 toolchain isn't in Fedora and there is no clear indication how long it might take. What is sure is that it won't be in F16. My suggestion at this point is to change mingw-wine-gecko spec file to work with the current MinGW toolchain in Fedora; once the mingw-w64 toolchain is imported it is easy to convert it over. I would be happy to review this package if you make these changes. Note that similar to the "future" guidelines, the "current" guidelines now suggest to use mingw- as source package prefix to ease the transition to the "future" guidelines. Current guidelines: https://fedoraproject.org/wiki/Packaging/MinGW Example spec file: https://fedoraproject.org/wiki/Packaging/MinGW#Example_Specfile
(In reply to comment #23) > My suggestion at this point is to change mingw-wine-gecko spec file to work > with the current MinGW toolchain in Fedora This is not possible. MinGW is not sufficient for Wine Gecko compilation.
Hey Andreas, The mingw-w64 toolchain has just been added to Fedora 17 so this review should now be able to continue. Here are some minor improvements you could make: - The first 3 lines of the .spec file (%global/%define) can be replaced by %?mingw_package_header - The BuildRoot tag and the line 'rm -rf %{buildroot}' can be removed now - The -DMINGW_HAS_SECURE_API shouldn't be needed any more with mingw-w64. Could you test if this package can be built without that define set? - We (the Fedora MinGW SIG) decided to get rid of the %global mingw_pkg_name in all mingw-* packages as it didn't have any added value. You also might want to remove it - The new MinGW packaging guidelines don't use brackets in RPM macro calls any more, so you might want to change %{?mingw_debug_package} to %?mingw_debug_package
Thanks for the info. I will try to upgrade to latest wine-gecko during the week and maybe we can get this in soon.
updated srpm/spec: http://fedora.lowlatency.de/review/mingw-wine-gecko.spec http://fedora.lowlatency.de/review/mingw-wine-gecko-1.5-1.fc17.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=3914963 changes: - upgrade to latest wine gecko (1.5) - cleanup spec including changes from comment 25 I have prepared an upgrade to wine 1.5.0 for f17 and rawhide. If wine-gecko gets a fast review I will hold wine 1.5.0 and release both at the same time. This would ensure that wine automatically pulls in the fedora mingw gecko on upgrading the prefix to 1.5.
Hi Andreas, lets get wine-gecko in Fedora! $ md5sum rpmbuild/SOURCES/wine-mozilla-1.5-src.tar.bz2 e1d9ba8914582d382b808332e4db0a54 wine-mozilla-1.5-src.tar.bz2 # wget $SOURCE0 $ md5sum $SOURCE0 e1d9ba8914582d382b808332e4db0a54 wine-mozilla-1.5-src.tar.bz2 Your spec looks great. Let's check rpmlint. $ rpmlint mingw32-wine-gecko 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint mingw64-wine-gecko 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Looks great there, too. Everything looks in order to me.
Thanks for picking it up so fast. I have made scratch builds for wine 1.5.0 so it is easier to do some testing. rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=3916624 f17: https://koji.fedoraproject.org/koji/taskinfo?taskID=3916632
What packages are going to own the directories %{_datadir}/wine/gecko/ and %{_datadir}/wine/ ? If none of the core wine packages do, might want to have both mingw32-wine-gecko and mingw64-wine-gecko own these directories to make sure they get removed on uninstall. By the way, if you want to get rid of some more rpm boilerplate code, these are no longer needed with latest mingw-filesystem: %global mingw_build_win32 1 %global mingw_build_win64 1 ... and the BuildRoot definition isn't needed either with the version of RPM we have in Fedora. Otherwise the spec file looks great, really nice to finally have this in Fedora!
The directories are owned by wine-common. I will add a requires for it and cleanup the spec after import. Thanks for the comment!
New Package SCM Request ======================= Package Name: mingw-wine-gecko Short Description: Gecko library required for Wine Owners: awjb Branches: f17
Git done (by process-git-requests).
Thanks for the review, help, and comments. And of course thanks to the mingw sig for the endurance.