Bug 546998 - Feature Request: Add mingw32-libgomp package
Summary: Feature Request: Add mingw32-libgomp package
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: mingw32-gcc
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Kalev Lember
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-12-13 04:23 UTC by Chris Bagwell
Modified: 2009-12-17 21:39 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-12-17 21:39:30 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
create mingw32-libgomp (4.78 KB, patch)
2009-12-14 14:28 UTC, Chris Bagwell
no flags Details | Diff
Updated diff. (5.20 KB, patch)
2009-12-14 14:42 UTC, Chris Bagwell
no flags Details | Diff
Updated diff to rawhide. (2.80 KB, patch)
2009-12-16 20:46 UTC, Chris Bagwell
no flags Details | Diff

Description Chris Bagwell 2009-12-13 04:23:24 UTC
The current mingw32-gcc package distributed by fedora has some base support for libgomp.  That is it does understand the -fopenmp flag but it will error out since it can not find libgomp support files.
 
The gcc package supports OpenMP on mingw platform via the libgomp library; included in gcc source.  Its configure script disables it by default on that platform but can easily be enabled with --enable-libgomp flag.

I am maintainer of SoX program which makes use of OpenMP.  I am attempting to switch to Fedora for building mingw binaries since its much faster then same process under windows.  libgomp support is only thing missing for me to be able to do this.

To help me do this, I have updated the existing mingw32-gcc.spec file to enable libgomp support and create a new mingw32-libgomp package (parallel to libgomp package already in Fedora).

The main item I'd like someone to review is that gcc package installs libgomp-1.dll under /usr/lib/gcc path but I think it better lives under /usr/i686-pc-mingw/sys-root since mingw apps that link in libgomp need access to that DLL at run time.

Comment 1 Chris Bagwell 2009-12-13 04:27:51 UTC
Addition item I'm not sure about.  If you link a mingw application to libgomp, it will fail to link unless you also link in pthread library.  Should mingw32-pthreads be added as a requirement to install mingw32-libgomp?

Comment 2 Richard W.M. Jones 2009-12-14 11:12:45 UTC
(In reply to comment #0)
> To help me do this, I have updated the existing mingw32-gcc.spec file to enable
> libgomp support and create a new mingw32-libgomp package (parallel to libgomp
> package already in Fedora).

Can you attach the diffs for the changes you made to mingw32-gcc.spec?

Also, I don't understand - why is there an existing mingw32-libgomp
package, and how does it fit in with this?

Comment 3 Chris Bagwell 2009-12-14 14:25:33 UTC
(In reply to comment #2)
> Can you attach the diffs for the changes you made to mingw32-gcc.spec?

I've attached diff compared to Fedora 12's mingw32-gcc.spec file (sorry, haven't check if rawhide is different yet).  This makes it easy to see they are minor changes.  I biggest part is I could no longer use a *.h since I needed to move one newly installed header file to the mingw32-libgomp package.
 
> Also, I don't understand - why is there an existing mingw32-libgomp
> package, and how does it fit in with this?  

Sorry for confusion.  I was attempting to say that the non-mingw32 gcc.spec file will create gcc-* packages and a libgomp package.  I paralleled that and added a mingw32-libgomp package to go along with the mingw32-gcc-* packages already being created by mingw32-gcc.spec.

On POSIX platform's, gcc's configure will use --enable-libgomp by default.  It works on mingw32 platform though but just needs to be enabled during compile.

Comment 4 Chris Bagwell 2009-12-14 14:28:09 UTC
Created attachment 378221 [details]
create mingw32-libgomp

Comment 5 Chris Bagwell 2009-12-14 14:42:48 UTC
Created attachment 378229 [details]
Updated diff.

Reviewing first diff, I noticed I mistakenly left in the old include/*.h line.

Comment 6 Richard W.M. Jones 2009-12-14 15:17:20 UTC
Chris - a few friendly tips for future reference:

Use unified diffs (diff -u) since they are much easier to review.

Mark old attachments obsolete in Bugzilla.

Mark patches as "[X] patch" in Bugzilla, otherwise it wants to
download them.

-----------------

Patch looks good to me.  Erik, Kalev, any objections?

Comment 7 Richard W.M. Jones 2009-12-14 15:18:27 UTC
Comment on attachment 378221 [details]
create mingw32-libgomp

Mark earlier patch as obsolete, patch.

Comment 8 Richard W.M. Jones 2009-12-14 15:18:43 UTC
Comment on attachment 378229 [details]
Updated diff.

Mark latest patch as patch.

Comment 9 Kalev Lember 2009-12-14 16:06:26 UTC
I've got a few comments.

1) The subpackage which has libgomp-1.dll probably also needs to get manual provides because the automatic dependency scripts aren't run:
Provides: mingw32(libgomp-1.dll)

2) %{_libdir}/gcc/i686-pc-mingw32/%{version}/finclude/ directory looks unowned.

3) In native gcc package finclude/ is part of gfortran subpackage, and include/omp.h is part of main gcc package. Why did you choose to include these under mingw32-libgomp instead? (I'm not saying it's bad, just trying to explore different options.)

4) Does the libgomp.spec which gets installed in {_libdir}/gcc/i686-pc-mingw32/%{version}/ really belong in there?

Comment 10 Kalev Lember 2009-12-14 16:17:33 UTC
(In reply to comment #9)
> 4) Does the libgomp.spec which gets installed in
> {_libdir}/gcc/i686-pc-mingw32/%{version}/ really belong in there?  

Never mind that comment. I was confusing gcc spec files with rpm spec files for a moment.

Comment 11 Erik van Pienbroek 2009-12-14 18:05:26 UTC
(In reply to comment #6)
> Patch looks good to me.  Erik, Kalev, any objections?  
The patch is against F-12's version of mingw32-gcc. In rawhide we already have a newer version (mingw32-gcc-4_4_2-1_fc13). Please post a unified diff against the rawhide version of mingw32-gcc. The .spec can be found at http://cvs.fedoraproject.org/viewvc/rpms/mingw32-gcc/devel/mingw32-gcc.spec?view=log

In addition to Kalev's remarks I also discovered something:
The %package section is missing a Requires: %{name} = %{version}-%{release} 

Kalev, perhaps this would also be a good opportunity to enable DWARF2 exceptions which we were already planning to do anyway.

Comment 12 Chris Bagwell 2009-12-15 17:24:03 UTC
(In reply to comment #9)
> I've got a few comments.
> 
> 1) The subpackage which has libgomp-1.dll probably also needs to get manual
> provides because the automatic dependency scripts aren't run:
> Provides: mingw32(libgomp-1.dll)

Makes sense.  But should it be the package name?  I'll add once its confirmed.

Provides: mingw32-libgomp(libgomp-1.dll)

> 
> 2) %{_libdir}/gcc/i686-pc-mingw32/%{version}/finclude/ directory looks unowned.

OK, will fix.

> 
> 3) In native gcc package finclude/ is part of gfortran subpackage, and
> include/omp.h is part of main gcc package. Why did you choose to include these
> under mingw32-libgomp instead? (I'm not saying it's bad, just trying to explore
> different options.)

This was a judgment call that I'll probably revert but would like opinions first.  Since libgomp has headers for C and fortran *and* is optional, some choices have to be made on where to package them.  Options: 1) Include both types of headers with mingw32-gcc package which means you can see C header file omp.h even when libgomp is not installed or fortran files when fortran compile is not installed. 2) Include both of them in mingw32-libgomp which has nice side affect that gcc won't see either header nor library when no libgomp libraries installed but it can mean fortran headers installed without fortran compiler.  3) Break into a mingw32-libgomp and mingw32-libgomp-fortran with appropriate Requires.

gcc.spec chose #1 which I find strange in that my configure scripts have to check for both header file omp.h and additionally do a -lgomp test to verify its really supported; when often the short cut of just looking for header file is enough.  So I chose #2 to prevent gcc from seeing header file which I left slightly better.  

I already have to deal with issues from #1 because of i686-redhat-linux-gcc compilers, so for consistency I should probably chose #1 for i686-pc-mingw32-gcc.  Agree?

> 
> 4) Does the libgomp.spec which gets installed in
> {_libdir}/gcc/i686-pc-mingw32/%{version}/ really belong in there?  

Ignoring this item as requested in later response.

Comment 13 Chris Bagwell 2009-12-15 17:48:29 UTC
(In reply to comment #11)
> (In reply to comment #6)
> > Patch looks good to me.  Erik, Kalev, any objections?  
> The patch is against F-12's version of mingw32-gcc. In rawhide we already have
> a newer version (mingw32-gcc-4_4_2-1_fc13). Please post a unified diff against
> the rawhide version of mingw32-gcc. The .spec can be found at
> http://cvs.fedoraproject.org/viewvc/rpms/mingw32-gcc/devel/mingw32-gcc.spec?view=log

I will do soon.  I'll attempt to upgrade compiler to rawhide on my Fedora 12 box.

> 
> In addition to Kalev's remarks I also discovered something:
> The %package section is missing a Requires: %{name} = %{version}-%{release} 

Make sense and I can add in next revision.  Is this also a gcc.spec issue as well and if so should one of us bugzilla it?

And to be safe, since I'm using "package -n mingw32-libgomp", will %{name} get converted to mingw32-gcc or mingw32-libgomp within that block or is it always fixed to "Name:" value?

Chris

Comment 14 Kalev Lember 2009-12-16 16:35:15 UTC
(In reply to comment #12)
> I already have to deal with issues from #1 because of i686-redhat-linux-gcc
> compilers, so for consistency I should probably chose #1 for
> i686-pc-mingw32-gcc.  Agree?

There's one more option: kill the mingw32-libgomp subpackage and include header files and dlls directly in the mingw32-gcc package.

As I understand it, the reason why gcc has headers + .so symlinks in gcc package and .so.* files in libgomp is to split development files and runtime files. For development you'd install gcc, and get all the headers and other stuff needed to link against the library. End users, however, would only get the libgomp package, which contains the shared lib (no development stuff in this package).

For mingw32 packages, splitting development and runtime in this way doesn't make much sense. All mingw32- packages are meant for development.

What I'd suggest is to include the dll and import libs in mingw32-gcc package. mingw32-fortran would get the finclude/ directory, and rest of the additional headers files would go in mingw32-gcc.

This setup would also reduce the complexity of the .spec file, where you are currently listing lots of header files. Those could probably be just replaced with a simple *.h glob.

Comment 15 Chris Bagwell 2009-12-16 20:41:32 UTC
(In reply to comment #14)
> (In reply to comment #12)
> > I already have to deal with issues from #1 because of i686-redhat-linux-gcc
> > compilers, so for consistency I should probably chose #1 for
> > i686-pc-mingw32-gcc.  Agree?
> 
> There's one more option: kill the mingw32-libgomp subpackage and include header
> files and dlls directly in the mingw32-gcc package.

Rational makes sense to me and solve multiple problems.  I'll attach updated diff that gets rid of libgomp subpackage.

Comment 16 Chris Bagwell 2009-12-16 20:46:38 UTC
Created attachment 378841 [details]
Updated diff to rawhide.

- Update unified diff to be based on current rawhide.
- Fix missing Provides: for libgomp-1.dll.  
- Add missing %dir for finclude.  
- Fold mingw32-libgomp subpackage into base mingw32-gcc to remove unneeded complications.  
- Move fortran headers to mingw32-gcc-gfortran subpackage.

Comment 17 Kalev Lember 2009-12-17 19:45:23 UTC
Allright, mingw32-gcc is now building for rawhide with your patch. Please give it a try when it finishes: http://koji.fedoraproject.org/koji/taskinfo?taskID=1878251

I also had to add 'BuildRequires: mingw32-pthreads' so it'd build,
and 'Requires: mingw32-pthreads' because libgomp-1.dll is linked against pthreads, but it doesn't get picked up automatically since we aren't running the automatic dependency scripts within this package.

Comment 18 Chris Bagwell 2009-12-17 21:28:39 UTC
(In reply to comment #17)
> Allright, mingw32-gcc is now building for rawhide with your patch. Please give
> it a try when it finishes:

Downloaded and tried.  No issues found in my Fedora 12 environment.

Thank you all for help with spec file and incorporating into rawhide!

Comment 19 Kalev Lember 2009-12-17 21:39:30 UTC
Thank you too Chris for helping us improve the mingw toolchain.

Closing the ticket.


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