rules/haddock: Set __HADDOCK_VERSION__
ClosedPublic

Authored by bgamari on Nov 11 2015, 1:00 PM.

Details

Summary

This emulates Cabal's behavior in this regard.

Test Plan

Validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bgamari updated this revision to Diff 5037.Nov 11 2015, 1:00 PM
bgamari retitled this revision from to rules/haddock: Set __HADDOCK_VERSION__.
bgamari updated this object.
bgamari edited the test plan for this revision. (Show Details)
bgamari added a reviewer: austin.
thomie requested changes to this revision.Nov 11 2015, 1:54 PM
thomie added a reviewer: thomie.

This doesn't perform the calculation. You can inspect the value of HADDOCK_VERSION_STRING with make show:

$ make show VALUE=HADDOCK_VERSION_STRING
make --no-print-directory -f ghc.mk show
HADDOCK_VERSION_STRING="(2*1000 + 16*10 + 0)"
This revision now requires changes to proceed.Nov 11 2015, 1:54 PM
In D1467#42609, @thomie wrote:

This doesn't perform the calculation. You can inspect the value of HADDOCK_VERSION_STRING with make show:

$ make show VALUE=HADDOCK_VERSION_STRING
make --no-print-directory -f ghc.mk show
HADDOCK_VERSION_STRING="(2*1000 + 16*10 + 0)"

Well, I was going to say that this is okay because it will only be evaluated within C expressions but that, of course, is absolutely incorrect. Fair point.

bgamari added a comment.EditedNov 11 2015, 2:31 PM
In D1467#42609, @thomie wrote:

This doesn't perform the calculation. You can inspect the value of HADDOCK_VERSION_STRING with make show:

$ make show VALUE=HADDOCK_VERSION_STRING
make --no-print-directory -f ghc.mk show
HADDOCK_VERSION_STRING="(2*1000 + 16*10 + 0)"

Well, I was going to say that this is okay because it will only be evaluated within C expressions but that, of course, is absolutely incorrect. Fair point.

Actually, I take it back, the C preprocessor will do basic arithmetic since C90 (IIRC). See, for instance,

//#define hi (2*2000)
#define hi (2*2000-1)

#if hi < 4000
#error hi < 4000
#else
#error hi >= 4000
#endif
bgamari requested a review of this revision.Nov 11 2015, 2:36 PM
bgamari edited edge metadata.

Both GCC's cpp as well as hpp are able to evaluate this macro correctly. I think this ought to be okay.

thomie resigned from this revision.Nov 11 2015, 3:19 PM
thomie removed a reviewer: thomie.

Well, Cabal just supplies a version number, and since you're trying to emulate that..

And someone is sure going to look at the build log one day and see this --optghc="-D__HADDOCK_VERSION__=(2*1000 + 16*10 + 0)", and then mention it to us because they think there is a bug, and then we forgot what the deal was or whatever and we spend even more time on it than we do now!

Here's a contrived example that would get different haddock documentation between Cabal's and your implementation:

{-# LANGUAGE CPP #-}
-- | __HADDOCK_VERSION__
module Test where

I'll leave it up to you though. It's not a serious bug.

thomie accepted this revision.Nov 11 2015, 3:20 PM
thomie added a reviewer: thomie.
This revision is now accepted and ready to land.Nov 11 2015, 3:20 PM
In D1467#42642, @thomie wrote:

Well, Cabal just supplies a version number, and since you're trying to emulate that..

And someone is sure going to look at the build log one day and see this --optghc="-D__HADDOCK_VERSION__=(2*1000 + 16*10 + 0)", and then mention it to us because they think there is a bug, and then we forgot what the deal was or whatever and we spend even more time on it than we do now!

Indeed, the trouble is that I don't know of a good way to compute this without adding another dependency (bash or bc). Suggestions welcome!

thomie added a subscriber: kgardas.Nov 11 2015, 6:37 PM

Here are two versions that seem to work. But you'll have move it outside of this define block, for reasons I don't quite understand (but it is a good idea anyway, otherwise you recompute it for each package).

HADDOCK_VERSION_STRING := $(shell echo $$(($(HADDOCK_MAJOR_VER) * 1000 + $(HADDOCK_MINOR_VER) * 10 + $(HADDOCK_PATCH_VER))))
HADDOCK_VERSION_STRING := $(shell expr $(HADDOCK_MAJOR_VER) \* 1000 + $(HADDOCK_MINOR_VER) \* 10 + $(HADDOCK_PATCH_VER))

@kgardas: are these Posix compliant? Do they work on your systems?

bgamari updated this revision to Diff 5048.Nov 12 2015, 3:40 AM
bgamari edited edge metadata.

Use sh arithmetic to evaluate version string

Hi,
I've tested advised:

echo $((2*1000 + 16*10 + 0))

and on both Solaris and OpenBSD I got the expected 2160. So this works well...

This revision was automatically updated to reflect the committed changes.