Bug 460304

Created attachment 315097 [details]
Version 2 of Macros for GHC

Created attachment 315737 [details]
macros.haskell

I would to propose these as a slightly revised and improved set of macros.

They have been tested with a ghc-X11.spec in bug 426751.

Please give them a try and see what you think. :)

These macros present a few problems.  They only work for our default compiler, and furthermore, when a user changes the default compiler, could introduce some weird breakage in mock when he/she forgets that it uses two different compilers.

There has been some demand for macros that work cross platform and build multiple packages for multiple compilers.  I'm going to have a look at a better way to do this.

I'd really rather we didn't spend much more time haggling over the macros.  Both Jens's and Yaakov's macros are good enough.

For the foreseeable future, we should not care about supporting multiple compilers, because everybody uses GHC.  If some crowd of users comes along who desperately wants hugs support, I would prefer that we consider their needs then, rather than adding further delay and contention now.

Unless there's a strong reason not to, I will commit Yaakov's existing macros file in a day or two.  I intend to do this only for ghc 6.8.3 in rawhide, unless someone thinks we should backport 6.8.3 to F9.

In that case, we'll need to make a few modifications.

1) the macros should be prefixed with ghc, not cabal or haskell
2) library packages should be prefixed with ghc-, not haskell-

The reasoning is that if someone asks for nhc support, i would rather just have them do their own nhc packages in a similar manner.  It seems the easiest for now, rather than having to rename haskell-* packages to ghc-* later.

Also, I would like to ask that you backport the macros to everything, because they are definitely needed in order to build packages.  Otherwise, there is no way we can build stuff on Koji boxes running on Fedora 9 or other versions.

To put this in perspective, the current workflow is to clean out mock, then copyin the macros.  This is not doable with Koji.  It's a pain in the you know what for most users.

I'd like to echo Bryan's comment that we should stop haggling and get on with this. Rather than dragging this on interminably I think it would be vastly preferable to get a few packages built and worry about minor tweaks later.

(In reply to comment #3)
> These macros present a few problems.  They only work for our default compiler,

Can you explain?  As far as I can see they should be useful for any Haskell implementation that support Cabal.  They look completely portable to be, as cabal indeed should be.

> and furthermore, when a user changes the default compiler, could introduce some
> weird breakage in mock when he/she forgets that it uses two different
> compilers.

details?

As others suggested I would like to see these macros get included in rpm eventually.

(In reply to comment #6)
> I'd like to echo Bryan's comment that we should stop haggling and get on with this.

We're not haggling - just trying to come up with a same set of macros that is actually useful.  I think mine are a good improvement on Yaakov's original proposal and would like to see them or something close to them adopted.

> Rather than dragging this on interminably I think it would be vastly
> preferable to get a few packages built and worry about minor tweaks later.

I disagree: I would rather get a decent of macros in place now than having to update lots of packages later to take in modifications.

As a person who actually owns some haskell packages in fedora and who started Haskell packaging for Fedora and the Haskell SIG I hope my suggestions at least carry a little weight...

A couple of dozen "good enough" packages would be preferable to zero perfect ones.

I was under the impression that Yaakov's macros had already been approved. Bryan also supports them, so do I, so can we just move on now please ...

I'm willing to give this a few more days to settle out, but Miles is right, time is of the essence.  See below.

Re comment #5:

> 1) the macros should be prefixed with ghc, not cabal or haskell

GHC-specific macros should be prefixed with ghc.  Many of the macros provided
by both of you are not specific to GHC.

> 2) library packages should be prefixed with ghc-, not haskell-

I would prefer this approach, too.  I do not believe that asking maintainers to
support multiple Haskell implementations will scale.

> Also, I would like to ask that you backport the macros to everything, because
> they are definitely needed in order to build packages.

OK.

I would like to suggest a deadline of next Thursday (Sep 11) to resolve the two
sets of macro files into one.  We will have to move quickly afterwards to adapt
to the changes in GHC 6.10.1 if we want to ship that in F10 (which I *very*
much want to do).  Those changes include shared library support, so the amount
of work involved will not be trivial.

The 6.10.1 release candidate should appear on the 19th, and I will have a little time at ICFP to whip the spec file and macros into shape.

I suggest everybody compare Jens's updated ghc-X11 spec (A) with the previous
version (B).  I scrutinized every change with ediff, and in each case I prefer
the updated version.

  A: https://bugzilla.redhat.com/attachment.cgi?id=315734

  B: http://ynemoy.fedorapeople.org/repo/ghc-X11.spec

Does anybody see any *technical* problems with Jens's macros?  If so, let's get those problems in the open so we can solve them.

Cheers,
Tom

Perhaps I should have been more specific.  Jen's macros call 'runhaskell'.  This defaults to the default compiler on any given system. I would prefer we deliberately called runghc.

Take the following case.  User A installs GHC and develops a package.  He posts a spec for review.

User B prefers NHC and has it set up as his default compiler.  He downloads the SRPM and builds it, which automatically gets built with NHC.  He then tests it in mock, and it also works.  Mock used GHC.  Weird but ok.

User C uses Hugs98, and creates a second package.  He doesn't test it in mock, because sometimes he's lazy.  User B downloads the SRPM and tries to build it.  For some odd reason, hugs chokes on it.  Weird, and not ok.  Then for some reason, the patch User B figures out makes GHC choke.  When the package is compiled with mock, we have total failure.

This use case rubs against me the wrong way, because we used runhaskell instead of runghc.

If we use runghc, then all three users will automatically use GHC, and automatically be working on the same platform.  The package will then be named ghc-* because it was tested on that.

When User B decides both packages should be done up in NHC, and User C decides hugs98 suits him better, they both copy the macros file, patch the compiler RPMs, and create nhc-* and hugs98-* variants.  The world is a happier place.

To make it absolutely clear, i'm not worried about the content of the macros.  Jen clearly knows what works better than I.  I just think, that unless any particular macro does not depend on the command 'runhaskell', namely can do things exactly the same way, no matter which compiler is on the system, then we can prefix it with  'cabal_'.  To the best of my knowledge, no macros meet these requirements.  Likewise, the prefix 'haskell_' is also wrong.  I would rather we stick to a consistent ghc- and ghc_ naming scheme.

On a side note, the packages I have submitted for review (the are blocked on this bug, so you can find them through that reference) were built and tested using my version.  Please be extra careful when testing them with Jens' version.

Yaakov, thanks for explaining your concerns in detail.

In reviewing Jens's spec for ghc-X11, it appears that it avoids your primary concern by passing the "--ghc" flag during the initial Cabal-setup invocation:

    %build
    %cabal_configure -p --ghc   <= HERE
    %cabal_makefile
    %cabal build
    %cabal haddock
    %ghc_gen_scripts

According to the Cabal documentation, this flag ensures that Cabal will use GHC for the later stages of Cabal processing, regardless of which Haskell implementation runhaskell invokes.  (Ref: http://www.haskell.org/ghc/docs/latest/html/Cabal/builders.html#setup-configure)  So even if runhaskell invoked NHC, for example, Jens's spec would still build a GHC package.

Do you think using Cabal-setup flags to specify the desired compiler will adequately resolve your concern about hidden compiler dependencies?

Cheers,
Tom

Created attachment 315829 [details]
macros.ghc

I talked a bit to Yaakov (loupgaroublond) on #fedora-devel
and hope we understand each other a bit better now.

Here is a slight update which fixes %ghc_gen_filelists
(renamed as it is implementation dependent as Yaakov pointed out to me).

I hope this can be close to a final version which is acceptable
to all parties.

I am kind of tempted to remove %cabal_install also since it is pretty
trivial, but maybe it is ok.

I am not sure if %_cabal_makefile is really necessary?  eg ghc-X11 seems to build fine without it.

Everyone, 

The default Yaakov's macros.ghc or Jens' macros.haskell are NOT adequate for building cabal packages that have mixed-case package names.  I had to modify those a little bit to get the test package (Cabal-1.4.0.2) working:

For the updated macros.ghc that could build a mixed-case cabal package, look at:
http://krishnan.cc/devel/repository/fedora/MISC/macros.ghc

For the updated macros.haskell that could build a mixed-case cabal package, look at:
http://krishnan.cc/devel/repository/fedora/MISC/macros.haskell

The files are quite small, so open either of those along with the corresponding original ones in Kompare (or any other favorite diff program you prefer) to note the differences (there are a few 'echo' traces in the file, let me know if they are not needed and I would remove those).

For an example of package (with a mixed-case package-name) that builds with this updated macros.ghc file, look at:
http://krishnan.cc/devel/repository/fedora/SRPMS/ghc-cabal-1.4.0.2-2.fc9.src.rpm

This is important because there are a lot of cabal based packages in Hackage that have mixed-case package names.  I have already mentioned this before on the Fedora Haskell List where I submitted Cabal-1.4.0.2 package for review.

I am attaching both here along with this comment.  

-Rajesh Krishnan

Created attachment 315875 [details]
macros.ghc that is required to build Hackage cabal based packages with mixed-case package names.

Place this file in /etc/rpm to build the following example package (that uses this):
http://krishnan.cc/devel/repository/fedora/SRPMS/ghc-cabal-1.4.0.2-2.fc9.src.rpm

-Rajesh

Here is another example of a Hackage cabal package that has a completely lowercase package name (polyparse), and builds fine with the updated macros.ghc file I mentioned above:

http://krishnan.cc/devel/repository/fedora/SRPMS/ghc-polyparse-1.1-1.fc9.src.rpm
http://krishnan.cc/devel/repository/fedora/SPECS/ghc-polyparse.spec

Note that this was tested on Fedora-8 (x86_64) with Ghc-6.8.3 installed.

-Rajesh Krishnan

Here is another problematic Hackage cabal package:
Crypto-4.1.0:   http://hackage.haskell.org/cgi-bin/hackage-scripts/package/Crypto

SRPM: http://krishnan.cc/devel/repository/fedora/SRPMS/ghc-crypto-4.1.0-1.fc9.src.rpm
SPEC: http://krishnan.cc/devel/repository/fedora/SPECS/ghc-crypto.spec
Macros:  http://krishnan.cc/devel/repository/fedora/MISC/macros.ghc
(tested on F9 + GHC-6.8.3 installed, on x86_64).



The problems with this package are:

1.  The package has mixed case package name (name starts with a capital letter).
2.  The package contains a number of executables (besides a library), but all of those are test executables (not normally executed by end-users).



The way I solved these issues are are follows:
1.  Mixed case is already taken care of the updated macros.ghc script I have attached.

2.  Because the package contains executables, the default package name as per the Fedora Haskell Specification would be crypto-*.rpm and not ghc-crypto-*.rpm.  



The following are noteworthy for this package:

1.  The library package are named ghc-crypto-<ver>.fc9.<arch>.rpm as per the specification because they contain the library and associated library documentation.

2.  The profile packages are named ghc-crypto-prof-<ver>.fc9.<arch>.rpm as per the spec.

3.  I have an additional devel package called ghc-crypto-devel-<ver>.fc9.<arch>.rpm that  contains all the test executables:
  $ rpm -ql ghc-crypto-devel
/usr/bin/HMACTest
/usr/bin/QuickTest
/usr/bin/RSATest
/usr/bin/SHA1Test
/usr/bin/SymmetricTest



for this case I am violating the current cabal package specification (which are not set in stone I hope) for some (IMHO) good reasons

1.  The test executables are not normally required by end-users.

2.  The test executables are not normally required by even developers who require compile their code against the library (contained in the main rpm) and would probably never need these executables either.

3.  These test executables don't seem to fit into the -prof package either.

4.  I thought of dumping these test executables under the libexec folder like (/usr/libexec/ghc-crypto) but for some reason didn't feel that was appropriate.

5.  So the only option was to spin-off another package that is named appropriately (-devel).

6.  I didn't feel like giving the too-important sounding name "crypto-*.rpm) of a package that contains just a handful of (mostly unnecessarily) test executables.  Hence I have named this ghc-crypto-devel-*.rpm (and not crypto-*.rpm or crypto-devel-*.rpm) in order to emphasize its affinity to the actual rpm and source (ghc-crypto*.rpm).

Please review the package and let me know if we could do this differently or in any other way.




YAAKOV/JENS:  
If you (mostly) agree with the above logic, then we might need to update the Fedora  Haskell Packaging specification as follows:

1.  For packages that contain executables that are only related to testing of the package (not something that end users would usually need) the RPM for such a package should be named as ghc-<PACKAGE>.*.rpm and not simply <PACKAGE>.*.rpm, assuming its cabal file contains a library section.

2. For packages that contain executables that are related to testing only (and other executables that may not normally be reuqired by end-users), the .SPEC for such an RPM may define a -devel section containing only those test executables.

3.  For other packages that contain end-user oriented executables and/or libraries, the general rules defined in the Fedora Haskell Packaging Specification would apply as usual.

Feedback is welcome.

-Rajesh Krishnan

Created attachment 315906 [details]
macros.ghc - Removed a bunch of 'echo' trace statements.

A minor cleanup of the macros.ghc file I attached earlier.  Removed a bunch of unimportant 'echo' statements.

-Rajesh Krishnan

Re comment #15:

The makefile step is not necessary for correctness, but it makes a big difference to build performance: it lets us build most of a package in parallel using _smp_mflags.  Cabal itself won't have the ability to do parallel builds for a long, long time.

Re comment #19:

Rajesh, if the build for a package is creating test executables that serve no useful purpose, just delete them after cabal installs them.  There is no reason we would want to package or ship useless binaries.

Rajesh, your macros are substantially different from Jens's current version.  Is there anything about Jens's that would cause you problems?

Rajesh, are any of those executables necessary at all?  Are they needed for
testing the package itself after being built?  Are they necessary for testing
packages that depend on them?

In any case, I think you should name the spec file 'ghc-crypto.spec' and iclude
a comment stating the exception.  There's no need to revise the guidelines for
something like this.  Secondly, if the executables are not needed once the
package has been built, note that there is a %test section in the RPM spec that
is not often used.  Feel free to take advantage of this.

If they are needed for dependencies, then just spit out a -devel package,
comment why it's needed, and comment on it in the review as well.

As for the makefile business, The macros are based on bryan's work.

BRYAN:
1.   The original set of macros published as part of Jens/Yaakov's updates would not correctly build cabal packages with mixed case package names.  The "internal_name" variable is necessary to take care of that.  

2.  For the argument against deleting the executables after building, see point (2.) in the next section below.

YAAKOV:
1.   I just compared the macros.ghc you submitted with the one I have and they are identical.  Thank you for taking care of my concerns in your updated macros.ghc submission.

2.   As for your question about the necessity of the  test executables, I have no idea, because I am not the original author of the Crypto cabal package.  So in order to avoid ticking-off the very few developers who might actually need those under some special circumstances (the tiniest bit of crypto-related software being too important these days), I did not feel it was appropriate to miss out that detail by simply deleting those executables after compilation.  Hence the logical choice of spinning out the -devel package with those executables.

3.  As for the %test macro, I think at this time the ghc-crypto rpm spec does not really need that.  I felt it would be just easier for me to provide the users of ghc-crypto with the means of testing it for themselves (with the executables  in ghc-crypto-devel package).

4.  I am building a lot of other cabal packages with our latest macros.ghc and will let everyone know if we  need any updates.  


-Rajesh

I agree, if a library creates some test or demo programs then that can be handled case-by-case by package review like any other package.  I don't think we need to specify in the guidelines explicitly whether should be be excluded or subpackaged, but one of those two options is likely in general.

I think the macros in comment 14 can handle Cabal just fine.  I'll attach a spec file.

Created attachment 316151 [details]
ghc-Cabal.spec

this builds with attachment 315829 [details]

Created attachment 316154 [details]
macros.ghc

I think to avoid confusion with whitespace it is better to reinstate %cabal_build and %cabal_haddock even though they are trivial.

Created attachment 316155 [details]
ghc-polyparse.spec

polyparse is fine too

Created attachment 316156 [details]
ghc-Crypto.spec

Just for good measure and the sake of argument, here is Crypto too.

Created attachment 316159 [details]
macros.ghc

Hopefully a final fix to ghc_gen_filelists.  (I am kind of tempted just to drop its argument though.)

I updated xmonad.spec (see attachment 316173 [details]) in the review (bug 426753) too,
so that it builds with these macros.

I added the final approved macros.ghc in ghc-6.8.3-4.fc10.

Once that has been tested let's backport it to f9 (and f8) as needed.

Any chance I can see a scratch build for f9, so I can do some testing?

(haddock09 hasn't been pushed yet to f9 updates... so) your best bet currently is either just to install the above macros.ghc in /etc/rpm or pull the koji builds for f10 - they work fine on f9 for me.

http://koji.fedoraproject.org/koji/buildinfo?buildID=63354

(should be in rawhide after the beta freeze).

Actually I messed up: my next build (probably ghc-6.8.3-5.fc10) should get macros.ghc in the right place.

ghc-6.8.3-5.fc10 is now built btw.

Should we close this bug?

Yes