Page MenuHomeFreeBSD

Introduce USES=cabal and use it to build Haskell applications
ClosedPublic

Authored by arrowd on Mar 27 2019, 7:27 PM.

Details

Summary

This change makes ports for Haskell applications include all dependencies in
DISTFILES. This brings dependencies on hs-* ports to 0.
Benefits of this:

  • Instant installation for resulting packages.
  • Improved build time.
  • We can remove almost all Haskell libraries from ports (see D19244).
Test Plan

Ports touched by this commit can be inferred from lang/ghc/bsd.hackage.mk changes.
The diff passes poudriere bulk on FreeBSD 12 amd64.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
arrowd marked an inline comment as done.
  • Use _USES_extract to run post-extract target.
Mk/Uses/cabal.mk
33 ↗(On Diff #55665)

Is there any cabal port that actually sets DISTFILES ?

39–60 ↗(On Diff #55665)

There are helpers that are for qmake, cmake, meson, they are all in bsd.options.mk.

I would prefer that all helpers be defined in one place, instead of having some magically happen when you add a feature.

arrowd marked 5 inline comments as done.
  • Use = when setting DISTFILES instead of ?=.
  • Move cabal option helpers to bsd.options.mk.

It turned out that

.      if ${PORT_OPTIONS:M${opt}}
CABAL_FLAGS+=	${${opt}_CABAL_FLAGS}
.      else
CABAL_FLAGS+=	-${${opt}_CABAL_FLAGS}
.      endif

construction is wrong. When the option is unset, the .else branch doesn't get executed.

What is the proper way to handle option setting?

  • Fix cabal helpers in bsd.options.mk.

Ignore my previous comment, it was a mistake on my side. Got lost in nested .ifs.

Mk/Uses/cabal.mk
35 ↗(On Diff #55764)

This is not right, see Mk/bsd.port.mk lines 5186+ to see what to add to that variable.

106 ↗(On Diff #55764)

Remove trailing /

109–117 ↗(On Diff #55764)

Please remove this, we already have a standard way for ports to "do" things when they need, do not invent a new one
Ports should have their own post-install* targets.

120 ↗(On Diff #55764)

Please do not piggy-back on an existing target, create your own.

arrowd added inline comments.
Mk/Uses/cabal.mk
35 ↗(On Diff #55764)

I saw that, but don't get what's wrong with this line. Should I prepend some integer before the target name?

arrowd marked 5 inline comments as done.
  • Get rid of INSTALL_PORTDATA and INSTALL_PORTEXAMPLES knobs.

@mat I've addressed all your comments. Please take another look.

 > poudriere bulk -j130amd64 -pghc -tC */hs*     
[...]
[00:00:08] Warning: www/hs-yesod dependency on textproc/hs-yaml has wrong PKGNAME of 'hs-yaml' but should be 'yaml'
[00:00:08] Warning: www/hs-yesod-static dependency on www/hs-wai-app-static has wrong PKGNAME of 'hs-wai-app-static' but should be 'wai-app-static'
[...]
 > poudriere bulk -j130amd64 -pghc -tC */hs*     
[...]
[00:00:08] Warning: www/hs-yesod dependency on textproc/hs-yaml has wrong PKGNAME of 'hs-yaml' but should be 'yaml'
[00:00:08] Warning: www/hs-yesod-static dependency on www/hs-wai-app-static has wrong PKGNAME of 'hs-wai-app-static' but should be 'wai-app-static'
[...]

Yep, but these ports are deprecated and going to be removed.

  • Fix extra patch in x11/hs-xmobar and PORTEXAMPLES entries in devel/hs-{alex,happy}.
  • Add SKIP_CABAL_PLIST knob to use it in devel/hs-haddock.
devel/hs-happy/Makefile
17 ↗(On Diff #56159)

Wrong place in the Makefile. Should happen before OPTIONS_DEFINE. See Chapter 15. Order of Variables in Port Makefiles.

devel/hs-hspec/Makefile
16 ↗(On Diff #56159)

Shouldn't this just be removed?

arrowd marked 2 inline comments as done.
  • Fix PORTEXAMPLES variable order.
devel/hs-hspec/Makefile
16 ↗(On Diff #56159)

Sure, I will remove it when landing this.

tobik added inline comments.
Mk/Uses/cabal.mk
10–11 ↗(On Diff #56342)

You should check if cabal_ARGS is empty and set IGNORE= when it is like other USES to prevent someone from adding bogus USES=cabal:foobar things.

99–101 ↗(On Diff #56342)

It might be good to pass through MAKE_ENV too. This is important for stuff like BINARY_ALIAS etc. to work correctly.

Ports might want to overwrite do-build, so maybe guard it with .if !target(do-build). Same for do-install.

Mk/bsd.options.mk
519–597 ↗(On Diff #56342)

The new options helpers need to be documented in the header and added to _OPTIONS_FLAGS/_ALL_OPTIONS_HELPERS too.

522–524 ↗(On Diff #56342)

Is this really needed when we already have the opt_USE helper?

devel/hs-darcs/Makefile
28–45 ↗(On Diff #56342)

These are in the wrong place now. See 15. Order of Variables in Port Makefiles.

47 ↗(On Diff #56342)

What does MAN1PAGES do? With bsd.cabal.mk gone it does not seem to do anything.

arrowd marked 4 inline comments as done.

Address comments.

Mk/bsd.options.mk
519–597 ↗(On Diff #56342)

Not sure about last part. ${opt}_CABAL_FLAGS works differently from other opt helpers - it appends some value in any case, so it actually handles _OFF case too.

As for ${opt}_USE_CABAL and ${opt}_EXECUTABLES - they don't need any handling for _OFF case.

522–524 ↗(On Diff #56342)

It is not, but I'm hesistant to change that now.

Moreover, I find opt_USE to look untidy when there are many items and you have to list them separated by , instead of .

Mk/bsd.options.mk
519–597 ↗(On Diff #56342)

Adding them to just _ALL_OPTIONS_HELPERS directly should be fine then.

Mk/bsd.options.mk
519–597 ↗(On Diff #56342)

grep -r _ALL_OPTIONS_HELPERS Mk yields nothing. What's _ALL_OPTIONS_HELPERS and where is it located?

Mk/bsd.options.mk
519–597 ↗(On Diff #56342)

In bsd.options.mk. Update your ports tree to after rP499267.

arrowd marked 3 inline comments as done.
  • Add cabal.mk options helpers to _ALL_OPTIONS_HELPERS.
Mk/Uses/cabal.mk
141–146 ↗(On Diff #56590)

Put the .if !defined(SKIP_CABAL_PLIST) around the target, no need to create an empty target.

Mk/bsd.options.mk
531–533 ↗(On Diff #56590)

Note that this is not needed, you already have <opt>_USE= cabal=foo.

534–536 ↗(On Diff #56590)

If all you are doing is:

.if defined(<opt>_FOO)
FOO+= <opt>_FOO
.endif

all you need to do is add FOO to _OPTIONS_FLAGS.

arrowd marked 2 inline comments as done.
  • Address comments.
Mk/bsd.options.mk
531–533 ↗(On Diff #56590)

Please, let's leave these 3 lines as they are. While opt_USE= xorg:xft,xwhatever looks nice, because there aren't many arguments for USE_XORG exists, the opt_USE= cabal:foo-1.2.3_4,bar-5.6.7_8,baz-9.10.11_12,... is awful, IMO. And these lines can be pretty large, in case some option pulls a bunch of Haskell packages.

Mk/bsd.options.mk
200 ↗(On Diff #56840)

Remove EXECUTABLES here.

203 ↗(On Diff #56840)

Remove USE_CABAL here.

arrowd marked 2 inline comments as done.
  • Address comments.

Other than my last small comment, the part in Mk looks ok to me.

x11/hs-xmobar/pkg-plist
2 ↗(On Diff #57118)

empty line. (only noticed it because it's the last file in the review, make sure there are no other.)

x11/hs-xmobar/pkg-plist
1 ↗(On Diff #57168)

Note that this file can be replaced by having:

PORTEXAMPLES=   xmobar.config

in the port's Makefile.

  • Create games/hedgewars-server port using cabal.mk.

@tcberner , @mat I missed another port that was using Haskell directly - games/hedgewars. I split off Haskell part into separate port and had to make some more changes to cabal.mk. Please, review these changes.

@AMDmi3 , you're maintainer for games/hedgewars. Please also take a look at my changes to it and new games/hedgewars-server port.

Looks good to me.

games/hedgewars/Makefile
54 ↗(On Diff #57196)

CMAKE_ON=NOSERVER is nicer

This revision was not accepted when it landed; it landed in state Needs Review.May 9 2019, 2:39 PM
This revision was automatically updated to reflect the committed changes.