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
