Interdiff
diff --git a/Makefile.am b/Makefile.am
index def2fdb..c52e7fb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1010,6 +1010,7 @@ src/hs2py-constants: $(HS2PY_CONSTANTS_SRCS) \
$(GHC) --make $(HFLAGS) \
-osuf $(HSUFFIX) \
-hisuf $(patsubst %.o,%.hi,$(HSUFFIX)) \
+ $(HEXTRA) \
src/hs2py-constants.hs
HS_SRCS = $(HS_LIBTESTBUILT_SRCS)
@@ -1026,7 +1027,7 @@ $(HS_ALL_PROGS): %: %.hs $(HS_SRCS) Makefile
$(GHC) --make $(HFLAGS) \
-osuf $(HSUFFIX) \
-hisuf $(patsubst %.o,%.hi,$(HSUFFIX)) \
- $(HS_PARALLEL3) $(HS_REGEX_PCRE) $@
+ $(HS_PARALLEL3) $(HS_REGEX_PCRE) $(HEXTRA) $@
@touch "$@"
# for the test/hs/htest binary, we need to enable profiling/coverage
@@ -1042,27 +1043,14 @@ test/hs/hpc-mon-collector: HCOVERAGE = true
# test dependency
test/hs/offline-test.sh: test/hs/hpc-htools test/hs/hpc-mon-collector
-# rules for building profiling-enabled versions of the haskell
-# programs: hs-prof does the full two-step build, whereas
-# hs-prof-quick does only the final rebuild (hs-prof must have been
-# run before)
-.PHONY: hs-prof hs-prof-quick
+# rule for building profiling-enabled versions of the haskell programs
+.PHONY: hs-prof
hs-prof:
@if [ -z "$(TARGET)" ]; then \
echo "You need to define TARGET when running this rule" 1>&2; \
exit 1; \
fi
- $(MAKE) $(AM_MAKEFLAGS) clean
- $(MAKE) $(AM_MAKEFLAGS) $(TARGET) HEXTRA="-osuf o"
- rm -f $(HS_ALL_PROGS)
- $(MAKE) $(AM_MAKEFLAGS) hs-prof-quick
-
-hs-prof-quick:
- @if [ -z "$(TARGET)" ]; then \
- echo "You need to define TARGET when running this rule" 1>&2; \
- exit 1; \
- fi
- $(MAKE) $(AM_MAKEFLAGS) $(TARGET) HEXTRA="-osuf prof_o -prof -auto-all"
+ $(MAKE) $(AM_MAKEFLAGS) HPROFILE=y $(TARGET)
On Tue, Nov 05, 2013 at 02:33:06PM +0100, Michele Tartara wrote:
> On Mon, Nov 4, 2013 at 7:58 PM, Jose A. Lopes <[email protected]> wrote:
> > On Mon, Nov 04, 2013 at 05:00:26PM +0100, Michele Tartara wrote:
> >> On Mon, Nov 4, 2013 at 4:53 PM, Jose A. Lopes <[email protected]> wrote:
> >> > On Mon, Nov 04, 2013 at 10:58:01AM +0100, Michele Tartara wrote:
> >> >> On Mon, Oct 7, 2013 at 2:53 PM, Jose A. Lopes <[email protected]>
> >> >> wrote:
> >> >> > The idea is to compile (on demand, that is, when necessary) each
> >> >> > source file to a normal object file, a coverage object file, a
> >> >> > profiling object file, and a test object file. Also, a given Haskell
> >> >> > binary is linked with the proper object files. This is achieved with
> >> >> > the following Makefile variables:
> >> >> >
> >> >> > Compilation modes (profiles):
> >> >> > 1. HPROFILE enables/disables profiling
> >> >> > 2. HCOVERAGE enables/disables coverage
> >> >> > 3. HTEST enables/disables 'TEST' preprocessor definition
> >> >> >
> >> >> > A few words on testing: testing means the problem described in issue
> >> >> > 535: https://code.google.com/p/ganeti/issues/detail?id=535. With
> >> >> > HTEST enable, ghc will be instructed to define the preprocessor
> >> >> > definition TEST, for modules that use '#ifdef TEST'.
> >> >> >
> >> >> > Haskell binary targets fetch the proper dependencies. They are also
> >> >> > '.PHONY' targets so that 'make' does not check for the file timestamp
> >> >> > and instead it will always call 'ghc --make ...'. This is not a
> >> >> > problem because 'ghc' with the '--make' flag only compiles the
> >> >> > necessary object files.
> >> >> >
> >> >> > Signed-off-by: Jose A. Lopes <[email protected]>
> >> >> > ---
> >> >> > Makefile.am | 66
> >> >> > ++++++++++++++++++++++++++++++++++++++++++-------------------
> >> >> > 1 file changed, 46 insertions(+), 20 deletions(-)
> >> >> >
> >> >> > diff --git a/Makefile.am b/Makefile.am
> >> >> > index bd6872e..279448e 100644
> >> >> > --- a/Makefile.am
> >> >> > +++ b/Makefile.am
> >> >> > @@ -965,45 +965,71 @@ install-exec-hook:
> >> >> > done
> >> >> > endif
> >> >> >
> >> >> > +HNORMAL_SUFFIX = .o
> >> >> > +HPROFILE_SUFFIX = .prof.o
> >> >> > +HCOVERAGE_SUFFIX = .hpc.o
> >> >> > +HTEST_SUFFIX = .test.o
> >> >> > +
> >> >> > +HSUFFIX = $(if $(HPROFILE),$(HPROFILE_SUFFIX), \
> >> >> > + $(if $(HCOVERAGE),$(HCOVERAGE_SUFFIX), \
> >> >> > + $(if $(HTEST),$(HTEST_SUFFIX), \
> >> >> > + $(HNORMAL_SUFFIX))))
> >> >> > +
> >> >> > +HFLAGS += $(if $(HPROFILE),-prof -auto-all,)
> >> >> > +HFLAGS += $(if $(HCOVERAGE),-fhpc,)
> >> >> > +HFLAGS += $(if $(HTEST),-DTEST,)
> >> >>
> >> >> Are HPROFILE, HCOVERAGE and HTEST always alternative to each other? Or
> >> >> might they be defined together?
> >> >
> >> > Always exclusive.
> >>
> >> Ok.
> >>
> >> >
> >> >>
> >> >> > +
> >> >> > +HS2PY_CONSTANTS_SRCS = src/hs2py-constants.hs \
> >> >> > + src/AutoConf.hs \
> >> >> > + src/Ganeti/BasicTypes.hs \
> >> >> > + src/Ganeti/ConstantUtils.hs \
> >> >> > + src/Ganeti/JSON.hs \
> >> >> > + src/Ganeti/THH.hs \
> >> >> > + src/Ganeti/Hs2Py/GenConstants.hs \
> >> >> > + src/Ganeti/Hs2Py/ListConstants.hs \
> >> >> > + src/Ganeti/HsConstants.hs \
> >> >> > + src/Ganeti/PyValueInstances.hs
> >> >> > +
> >> >> > # This target cannot be merged with the '$(HS_ALL_PROGS)' target
> >> >> > # because 'hs2py-constants' cannot depend on 'Ganeti.Constants'. And
> >> >> > # the reason for this is because 'hs2py-constants' needs to generate
> >> >> > # Python code, and 'Ganeti.Constants' is generated by Python.
> >> >> > -src/hs2py-constants: src/hs2py-constants.hs src/AutoConf.hs \
> >> >> > - src/Ganeti/BasicTypes.hs
> >> >> > src/Ganeti/ConstantUtils.hs \
> >> >> > - src/Ganeti/JSON.hs src/Ganeti/THH.hs \
> >> >> > - src/Ganeti/Hs2Py/GenConstants.hs \
> >> >> > - src/Ganeti/Hs2Py/ListConstants.hs \
> >> >> > - src/Ganeti/HsConstants.hs \
> >> >> > - src/Ganeti/PyValueInstances.hs \
> >> >> > +.NOTPARALLEL: src/hs2py-constants
> >> >> > +.PHONY: src/hs2py-constants
> >> >> > +src/hs2py-constants: $(HS2PY_CONSTANTS_SRCS) \
> >> >> > | stamp-srclinks
> >> >> > - $(GHC) --make \
> >> >> > - $(HFLAGS) \
> >> >> > - -osuf $(notdir $@).o -hisuf $(notdir $@).hi \
> >> >> > - $(HEXTRA) $(HEXTRA_INT) src/hs2py-constants.hs
> >> >> > + @rm -f [email protected]
> >> >> > + $(GHC) --make $(HFLAGS) \
> >> >> > + -osuf $(HSUFFIX) \
> >> >> > + -hisuf $(patsubst %.o,%.hi,$(HSUFFIX)) \
> >> >> > + src/hs2py-constants.hs
> >> >>
> >> >> I think that while rewriting the rule $(HEXTRA) and $(HEXTRA_INT) were
> >> >> unintentionally left out.
> >> >
> >> > HEXTRA_INT was being used for coverage, for example, to pass '-fhpc'.
> >> > Now, we have a HPROFILE mode, therefore, HEXTRA_INT is no longer
> >> > necessary.
> >> >
> >> > HEXTRA is only used by 'hs-prof' and 'hs-prof-quick' which were needed
> >> > for profiling. I guess these can be eliminated because of 'HPROFILE'.
> >> >
> >> > What do you think ?
> >>
> >> Ok, for HEXTRA_INT (which, I guess, means "internal") but NACK for
> >> HEXTRA. It should be anywhere ghc is used. It is meant to be a way to
> >> inject extra compiler parameters from the environment, e.g. as done
> >> here:
> >> http://docs.ganeti.org/ganeti/master/html/devnotes.html#haskell-development-notes
> >>
> >> It's a bit like the CFLAGS variable for Makefiles running gcc.
> >> Please, add it back.
> >>
> >> The rest looks really good!
> >
> > What about removing 'hs-prof' and 'hs-prof-quick' ?
> > Remove ?
>
> This would be a user-facing change, so I'm not convince we should
> remove them (even if only facing the users compiling Ganeti).
>
> I'd replace their current version with one that properly sets the
> HPROFILE variable and then launches the compilation.
>
> Thanks,
> Michele
> --
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
>
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
--
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
