On Wed, Feb 11, 2015 at 04:33:39PM +0100, 'Petr Pudlak' via ganeti-devel wrote:
> .. in Haskell code.
> 
> This patch should be removed starting from 2.14, as there cabal
> provides proper macros for all packages.
> 
> The macro in this patch is intentionally kept compatible with the cabal
> macros.
> 
> Signed-off-by: Petr Pudlak <[email protected]>
> ---
>  Makefile.am        | 26 +++++++++++++++++++-------
>  src/Ganeti/Lens.hs | 10 +++++++++-
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 8033db1..c881e83 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -272,6 +272,7 @@ CLEANFILES = \
>       $(addsuffix /*.o,$(HS_DIRS)) \
>       $(addsuffix /*.$(HTEST_SUFFIX)_hi,$(HS_DIRS)) \
>       $(addsuffix /*.$(HTEST_SUFFIX)_o,$(HS_DIRS)) \
> +     hs-lens-version \
>       Makefile.ghc \
>       Makefile.ghc.bak \
>       $(PYTHON_BOOTSTRAP) \

Normally we also .gitignore automatically generated files.

> @@ -738,6 +739,11 @@ if HTEST
>  HFLAGS += -DTEST
>  endif
>  
> +# lens version; this should be removed in 2.14 in favor of cabal-generated
> +# versions for all packages
> +# see target hs-lens-version
> +HFLAGS += $(shell cat hs-lens-version)
> +
>  HTEST_FLAGS = $(HFLAGS) -fhpc -itest/hs \
>       -osuf .$(HTEST_SUFFIX)_o \
>       -hisuf .$(HTEST_SUFFIX)_hi
> @@ -1171,11 +1177,17 @@ install-exec-hook:
>  
>  HS_SRCS = $(HS_LIBTESTBUILT_SRCS)
>  
> +hs-lens-version:
> +     ghc-pkg list --simple-output lens \
> +     | sed -r -e 's/^lens-([0-9]+)\.([0-9]+)\.([0-9]+)(\.([0-9]+))?/\
> +       -DLENS_MAJOR=\1 -DLENS_MINOR=\2 -DLENS_REV=\3/' \
> +     > $@
> +
>  HS_MAKEFILE_GHC_SRCS = $(HS_SRC_PROGS:%=%.hs)
>  if WANT_HSTESTS
>  HS_MAKEFILE_GHC_SRCS += $(HS_TEST_PROGS:%=%.hs)
>  endif
> -Makefile.ghc: $(HS_MAKEFILE_GHC_SRCS) Makefile \
> +Makefile.ghc: $(HS_MAKEFILE_GHC_SRCS) Makefile hs-lens-version \
>                | $(built_base_sources) $(HS_BUILT_SRCS)
>       $(GHC) -M -dep-makefile $@ -dep-suffix $(HPROF_SUFFIX) \
>               -dep-suffix $(HTEST_SUFFIX) $(HFLAGS) -itest/hs \
> @@ -1194,7 +1206,7 @@ Makefile.ghc: $(HS_MAKEFILE_GHC_SRCS) Makefile \
>  
>  @include_makefile_ghc@
>  
> -%.o:
> +%.o: hs-lens-version

This is bad Makefile style: you add the explicit dependency here (because
you have to) but you hide the actual usage in a unrelated variable; this
is hard for maintainance having to guess that hs-lens-version is used in
HFLAGS. Can we just make the use explicit and drop adding the call to HFLAGS?

>       @echo '[GHC]: $@ <- $^'
>       @$(GHC) -c $(HFLAGS) \
>               $(HS_PARALLEL3) $(HS_REGEX_PCRE) $(HEXTRA_COMBINED) 
> $(@:%.o=%.hs)

Thanks,
Klaus

-- 
Klaus Aehlig
Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to