Bug 529548 (mingw32-libogg) - Review Request: mingw32-libogg - MinGW build of the libogg bitstream library
Summary: Review Request: mingw32-libogg - MinGW build of the libogg bitstream library
Keywords:
Status: CLOSED DUPLICATE of bug 613697
Alias: mingw32-libogg
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Peter Lemenkov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: mingw32-libvorbis mingw32-speex mingw32-libtheora
TreeView+ depends on / blocked
 
Reported: 2009-10-18 13:47 UTC by Mihai Limbășan
Modified: 2010-10-25 11:15 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-10-25 11:15:08 UTC
Type: ---
Embargoed:
lemenkov: fedora-review+


Attachments (Terms of Use)

Description Mihai Limbășan 2009-10-18 13:47:11 UTC
Spec URL: http://rpms.limbasan.ro/fedora/11/SPECS/mingw32-libogg.spec
SRPM URL: http://rpms.limbasan.ro/fedora/11/SRMPS/mingw32-libogg-1.1.4-2.fc11.src.rpm
Description: This is MinGW crosscompiler port of libogg.

The spec file was created based on the example at

http://hg.et.redhat.com/cgi-bin/hg-misc.cgi/fedora-mingw--devel/file/3d9d4c1db042/example/mingw32-example.spec#l1

and on the main Fedora libogg spec, taking into account the MinGW SIG packaging guildelines at http://fedoraproject.org/wiki/Packaging/MinGW .

rpmlint output:

[mimock@home syncup]$ rpmlint mingw32-libogg-1.1.4-2.fc11.noarch.rpm mingw32-libogg-1.1.4-2.fc11.src.rpm mingw32-libogg-static-1.1.4-2.fc11.noarch.rpm
mingw32-libogg-static.noarch: E: arch-independent-package-contains-binary-or-object /usr/i686-pc-mingw32/sys-root/mingw/lib/libogg.a
mingw32-libogg-static.noarch: W: no-documentation
3 packages and 0 specfiles checked; 1 errors, 1 warnings.

which are OK accoring to the packaging guidelines.

Since there is no current mingw32-libogg package I decided to go for the latest upstream version, 1.1.4, instead of Fedora's 1.1.3.

The -m4 patch is Fedora's -m4 patch, corrected to apply cleanly on 1.1.4.

The -acconf patch helps in replacing the ancient config.sub, config.guess, and libtool shipped by upstream with current versions.

The main deviation from Fedora's build step is using sed instead of perl to replace upstream's (slightly insane) CFLAGS.

Here's my ~/.rpmmacros so far:

%_topdir      %(echo $HOME)/rpmbuild
%__arch_install_post   /usr/lib/rpm/check-rpaths   /usr/lib/rpm/check-buildroot
%_query_all_fmt %%{name}-%%{version}-%%{release}.%%{arch}
%_smp_mflags  -j3
%packager Mihai Limbasan <mihai>

... hope this is OK - this is my first package.

Comment 1 Mihai Limbășan 2009-10-18 14:30:31 UTC
Koji scratch build here, looks OK: http://koji.fedoraproject.org/koji/taskinfo?taskID=1752861

Wasn't able to figure out how to run make check in mock. The tests complete successfully in ~/rpmbuild, but since they generate Wiin32-PE .exe files the mock run croaks due to the missing Wine binary loader, and I'd really, really want to avoid having a mingw package BR Wine.

Comment 2 Richard W.M. Jones 2009-10-19 12:16:24 UTC
Package looks sane, but any reason why you want to keep
the static library?

(In reply to comment #1)
> Wasn't able to figure out how to run make check in mock. The tests complete
> successfully in ~/rpmbuild, but since they generate Wiin32-PE .exe files the
> mock run croaks due to the missing Wine binary loader, and I'd really, really
> want to avoid having a mingw package BR Wine.  

That's simply not supported.  Koji might happen to run on (eg) x86-64
or PPC builder, where Wine either can't be installed or couldn't run even
if it could be installed.  Don't worry about it.

Comment 3 Mihai Limbășan 2009-10-19 12:59:26 UTC
(In reply to comment #2)
> Package looks sane, but any reason why you want to keep
> the static library?

Not really, no - I just aped the behavior of some other mingw32-* library packages. Can be dropped any time (from the other dependent packages as well.)

> That's simply not supported.  Koji might happen to run on (eg) x86-64
> or PPC builder, where Wine either can't be installed or couldn't run even
> if it could be installed.  Don't worry about it.  

Thanks, I thought I was missing something. I'm still running all tests I can manually - e.g., working on mingw32-xz now, and some edge case tests fail depending on the MINGW32_CFLAGS you pick, which would seem to point to some compiler problems, so properly passing all tests should be a priority.

Comment 4 Richard W.M. Jones 2009-10-19 13:34:36 UTC
(In reply to comment #3)
> > That's simply not supported.  Koji might happen to run on (eg) x86-64
> > or PPC builder, where Wine either can't be installed or couldn't run even
> > if it could be installed.  Don't worry about it.  

I should explain that there are three related issues here which
stop us from running Wine.  Two are bugs (IMHO) in RPM
or Koji.  One is a bug (IMHO) in Wine itself.

(1) RPM/Koji doesn't understand cross-compilation at all.  Our
target packages are all noarch which is (sort of) correct within
the bounds of RPM[*].  Because the package is noarch, Koji decides
that it can be built on any machine, so it can choose a PPC
builder for example.  On PPC, the cross-compiler works, but Wine
could never run.  But there is no way to say to Koji that a
package is noarch, but should be built anyway on i386.

[*] If you prefer you can think of wine as an interpreter,
interpreting Win32 PE executables, something like:

 foo.pl   is interpreted by /usr/bin/perl
 foo.exe  is interpreted by /usr/bin/wine

(2) Koji won't install Wine on x86-64, because Wine is an i386
package.  Koji doesn't understand multilib (probably a good
thing, because multilib is crack).

(3) Wine after installation doesn't "just work".  You have to
fiddle with a difficult configuration file:

http://fedoraproject.org/wiki/MinGW/Configure_wine

which is difficult to automate.  Wine used to have an
environment variable (WINE_PATH or something?) but they
removed it(!)

> Thanks, I thought I was missing something. I'm still running all tests I can
> manually - e.g., working on mingw32-xz now, and some edge case tests fail
> depending on the MINGW32_CFLAGS you pick, which would seem to point to some
> compiler problems, so properly passing all tests should be a priority.  

As long as you're testing it, that's good.  It doesn't look like
we'll ever get the Koji issues fixed.

Comment 5 Mihai Limbășan 2009-10-19 18:52:36 UTC
Thanks for the advice, Richard.

Improved package, bumped revision. New URLs:

Spec URL: http://rpms.limbasan.ro/fedora/11/SPECS/mingw32-libogg.spec
SRPM URL: http://rpms.limbasan.ro/fedora/11/SRPMS/mingw32-libogg-1.1.4-3.fc11.src.rpm

Changes as follows:

- Removed -static package.
- Added explicit Requirement on mingw32-filesystem.
- aclocal and pkgconfig directories under _mingw32_libdir are
  no longer installed, mingw32-filesystem already provides them.
- Removed redundant BuildRequire mingw32-binutils which is
  already Required by mingw32-gcc.
- Moved checks to the proper build stage.
- Corrected aclocal invocation.
- Cosmetic cleanup.

rpmlint says:

[mimock@home mingw32-libogg]$ rpmlint -v *rpm
mingw32-libogg.noarch: I: checking
mingw32-libogg.src: I: checking
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Koji scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1755213

Comment 6 Peter Lemenkov 2009-11-26 08:52:51 UTC
(In reply to comment #4)

> But there is no way to say to Koji that a
> package is noarch, but should be built anyway on i386.

Few months ago koji and/or rpm were upgraded, so we can specify builder for the noarch subpackages!

See this specs as a starting point:

http://cvs.fedoraproject.org/viewvc/rpms/openbios/devel/openbios.spec?view=markup

Comment 7 Peter Lemenkov 2009-11-26 08:57:09 UTC
I'll review it.

Comment 8 Peter Lemenkov 2009-11-26 09:13:03 UTC
+ rpmlint is silent

[petro@Sulaco SPECS]$ rpmlint ../RPMS/noarch/mingw32-libogg-1.1.4-3.fc12.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[petro@Sulaco SPECS]$ 

+ The package is named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.

+/- The package meets the Packaging Guidelines, except I don't see any reasons for uysing %defattr(0644,roor,root,0755) instead of %defattr(-,roor,root,-). Could you, please, comment this?

+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

[petro@Sulaco SOURCES]$ sha256sum libogg-1.1.4.tar.gz*
9354c183fd88417c2860778b60b7896c9487d8f6e58b9fec3fdbf971142ce103  libogg-1.1.4.tar.gz
9354c183fd88417c2860778b60b7896c9487d8f6e58b9fec3fdbf971142ce103  libogg-1.1.4.tar.gz.1
[petro@Sulaco SOURCES]$ 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture (ppc).
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No need to run ldconfig for mingw32 libraries.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No need to separate header files from main package for mingw32-related package.
0 No static libraries.
0 No need to specifically handle pkgconfig(.pc) files for mingw32 (mingw32-filesystem already contains %{_mingw32_libdir}/pkgconfig directory).
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
0 No devel sub-package for mingw32 packages, since they are intended for devel entirely.
0 The mingw32 package may contain necessary .la libtool archives. This is not a blocker.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Please, comment/correct %defattr and I'll continue.

Comment 9 Peter Lemenkov 2009-12-18 09:15:47 UTC
Ping, Mihai!

Comment 10 Jason Tibbitts 2010-01-21 05:46:50 UTC
Needinfo has been set for over a month now; if there's no response in a week, this and the other submitted review tickets will be closed.

Comment 11 Josh Matthews 2010-02-02 12:05:39 UTC
I'm really interested in getting the mingw32 versions of libogg and libvorbis packaged up.  What can I do to help this process, seeing as Mihai's apparently AWOL?

Comment 12 Richard W.M. Jones 2010-02-02 12:28:59 UTC
See:
http://fedoraproject.org/wiki/PackageMaintainers/Join

Comment 13 Stefan Riemens 2010-02-02 13:16:42 UTC
Alternatively, I'd be happy to maintain this package as well. I don't have a clue how that works though. Should I just post a cvs admin request, with my name as the maintainer? Or should I open a new review request?

Thanks,
Stefan

Comment 14 Richard W.M. Jones 2010-02-02 13:23:23 UTC
(In reply to comment #13)
> Alternatively, I'd be happy to maintain this package as well. I don't have a
> clue how that works though. Should I just post a cvs admin request, with my
> name as the maintainer? Or should I open a new review request?

Well the review isn't finished yet.  Peter had some questions
in comment 8 which require you to post a fixed package, and
for Peter (once he is happy with it) to approve it.

I assume you have a FAS account and are a packager?  If not
you'll also need sponsorship.

This is all explained in the link I posted in comment 12.

Comment 15 Stefan Riemens 2010-02-02 14:07:16 UTC
Alright, please find an updated package here:
http://www.riemens.org/fs/temp/mingw32-libogg.spec
http://www.riemens.org/fs/temp/mingw32-libogg-1.1.4-4.fc12.src.rpm

rpmlint output:
rpmlint /home/makerpm/rpmbuild/SRPMS/mingw32-libogg-1.1.4-4.fc12.src.rpm /home/makerpm/rpmbuild/RPMS/noarch/mingw32-libogg-1.1.4-4.fc12.noarch.rpm /home/makerpm/rpmbuild/RPMS/noarch/mingw32-libogg-debuginfo-1.1.4-4.fc12.noarch.rpm
mingw32-libogg-debuginfo.noarch: E: empty-debuginfo-package
3 packages and 0 specfiles checked; 1 errors, 0 warnings.

Koji scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=1958859

Thanks,
Stefan

PS, I'm already sponsored by Peter.

Comment 16 Stefan Riemens 2010-02-02 14:08:06 UTC
Changelog entry: (forgot to add it, sorry)
* Tue Feb 2 2010 Stefan Riemens <fgfs.stefan> - 1.1.4-4
- Fix %%defattr line
- Automatically create a debuginfo package

Comment 17 Peter Lemenkov 2010-02-04 12:20:42 UTC
Great! I don't see other issues with Stefan's srpm, so this package is

APPROVED.


Stefan, will you continue with packaging the rest of the Xiph's stack? If not, then, I believe, Josh could step in.

Comment 18 Kalev Lember 2010-02-04 12:55:40 UTC
I have a few late comments.

The -debuginfo subpackage is empty. This is caused by a missing define:
%define __debug_install_post %{_mingw32_debug_install_post}

You have this:
# Automatically create a debuginfo package
%{_mingw32_debug_package}

Instead of using %{_mingw32_debug_package}, it's better to use %{?_mingw32_debug_package} (notice the question mark) to work around a problem with macros showing up unexpanded in description in Koji. See [1] for an explanation of this issue.
Can you do a new Koji scratch build to be sure that debuginfo issues are fixed?

Now that the binaries are stripped automatically, you can kill those two lines:
# Uncomment and unescape to strip the debug symbols from the resulting artifacts
#%%{_mingw32_strip} --strip-unneeded %%{buildroot}%%{_mingw32_bindir}/libogg-0.dll

Requires: pkgconfig is unneccessary in Fedora (but needed if you plan to build for EPEL). RPM in current Fedora releases is capable of extracting pkgconfig deps automatically. See [2] for updated guidlines. You don't neccessarily have to change anything here, just pointing it out.

[1] http://lists.fedoraproject.org/pipermail/devel/2010-February/130357.html
[2] https://fedoraproject.org/wiki/Packaging/Guidelines#Pkgconfig_Files

Comment 19 Mihai Limbășan 2010-04-27 19:15:03 UTC
Sorry for the MIA - real life issues prevented me from being able to focus on anything Fedora- or personal life related.

I'll try to get back on this after F13 is released, targeting F12, F13 and Rawhide.

Apologies for any waste of time I may have caused.

Comment 20 Peter Lemenkov 2010-10-25 11:15:08 UTC

*** This bug has been marked as a duplicate of bug 613697 ***


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