Makefiles etc

[Ian Lynagh]

Hi all,

I've been looking at the fputils build system, and amongst various typos
and pet peeves[0] there are a few more serious things I have noticed and
I also have a few questions. I will refer mostly to names of the final
built Makefiles as that is what it was easiest to follow.




mk/config.mk:

    # Yuckage: for ghc/utils/parallel -- todo: nuke this dependency!!
    BASH                    = /usr/local/bin/bash

If it's going to be there shouldn't the correct location of bash be
detected? [just hardcoded in mk/config.mk.in]




mk/config.mk:

    PRINTER         = lpr

as far as I can see this is not used. Its presence seems very odd to me.



mk/config.mk:

    # Pin a suffix on executables? If so, what (Windows only).
    exeext=
[...]
    #
    # Under Windows, we have to deal with .exe suffixes
    #
    EXE_SUFFIX=

seems to be duplication.




mk/paths.mk:

    # make depend defaults
    #
    # The default set of files for the dependency generators to work on
    # is just their source equivalents.
[...]
    # make TAGS defaults
    #
    # The default set of files for the dependency generators to work on
    # is just their source equivalents.

Is the second comment wrong?




mk/suffix.mk:

    .PRECIOUS: %.hs

If my understanding of the special targets is correct then .SECONDARY is
what is wanted here (and elsewhere). If happy opens foo.hs for writing
and ^C is pressed then a 0 size file will be created, but it make is
rerun then it will have an uptodate timestamp so won't be recreated.
iI am not familiar with happy so it could be this isn't an issue in this
case.

    % cat Makefile

    .SECONDARY: foo
    .PRECIOUS: bar

    foo:
            touch foo
            sleep 10

    bar:
            touch bar
            sleep 10

    % make foo
    touch foo
    sleep 10
    make: *** Deleting file `foo'
    make: *** wait: No child processes.  Stop.
    make: *** Waiting for unfinished jobs....
    make: *** wait: No child processes.  Stop.
    % make bar
    touch bar
    sleep 10
    make: *** wait: No child processes.  Stop.
    make: *** Waiting for unfinished jobs....
    make: *** wait: No child processes.  Stop.
    % ls
    Makefile  bar
    %




mk/suffix.mk:
    ifneq "$(BootingFromHc)" "YES"
    %_hsc.c %_hsc.h %.hs : %.hsc
        $(HSC2HS) $(HSC2HS_OPTS) $<
        @touch $(patsubst %.hsc,%_hsc.c,$<)
    endif

Am I right in thinking that the touch is done because hsc2hs won't
always generate all the files and the rule doesn't want to be triggered
next time make is called if one of them isn't created?





mk/suffix.mk:
%.$(way_)o : %.$(way_)s
    @$(RM) $@
    $(HC) $(GHC_CC_OPTS) -c $< -o $@

Why is the target first deleted?




mk/suffix.mk:
    %.$(way_)o : %.$(way_)s
        @$(RM) $@
        $(AS) $(AS_OPTS) -o $@ $< || ( $(RM) $@ && exit 1 )

I can't remember if GNU make is required for building fptools, but if so
then .DELETE_ON_ERROR provides a much nicer way of doing this across all
rules:

    % cat Makefile

    .DELETE_ON_ERROR:

    foo:
            touch foo
            false

    % make foo
    touch foo
    false
    make: *** [foo] Error 1
    make: *** Deleting file `foo'
    %

While I'm here, you seem to have an ugly mixture of true/false and exit
0/1, including things like "then true; else exit 1" in Makefile.
Personally I prefer true and false, but I'm not hugely fussed.




mk/suffix.mk:
    %.tab.c %.tab.h : %.y
        @$(RM) $*.tab.h $*.tab.c y.tab.c y.tab.h y.output
        $(YACC) $(YACC_OPTS) $<
        $(MV) y.tab.c $*.tab.c
        @chmod 444 $*.tab.c
        $(MV) y.tab.h $*.tab.h
        @chmod 444 $*.tab.h

This of course has potential problems make -j. Also, why are the chmods
hidden?




mk/suffix.mk:
    %.html : %.sgml
        @$(RM) $@
    #   $(PERL) $(COLLATEINDEX) -N -o index.sgml
    #   $(JADE) -t sgml -V html-index -d $(SGMLSTYLESHEET) -c $(DOCBOOK_CATALOG) $<
    #   $(PERL) $(COLLATEINDEX) -N -o index.sgml
        $(SGML2HTML) $(SGML2HTML_OPTS) $<
    # touch the .html file so that it is seen to be built
        @touch $@

What is the touch for here?





mk/suffix.mk:
    % : %.pp
        rm -f $@
        $(CPP) $(CPP_OPTS) -x c $< | $(SED) -e '/^#/d' > $@

Shouldn't this use $(RM)?
There are more of these in Makefile and probably elsewhere, and for
other commands.

I would prefer a $(RMD) over $(RM) -r or rm -rf too.




On, for example, binary-dist-pre:: in Makefile which is something like

    foo:
        @for i in 1 2 3; \
        do \
            echo bar $$i; \
            bar $$i; \
        done'

this seems incredibly ugly, especially given the length of some of the
commands. Perhaps something like this could be used instead?

    foo:
        @sh -cx '\
        for i in 1 2 3; \
        do \
            bar $$i; \
        done'





    touch $(BIN_DIST_TMPDIR)/$(BIN_DIST_NAME)/Makefile.in

in Makefile is unneccesary as various things are about to be appended to
it.





I would write

    $(MV) $(BIN_DIST_TMPDIR)/$(BIN_DIST_NAME)/bin/$(TARGETPLATFORM)/$$i  $(BIN_DIST_TMPDIR)/$(BIN_DIST_NAME)/bin/$(TARGETPLATFORM)/$$i.prl;

(in Makefile) as

    $(MV)
    $(BIN_DIST_TMPDIR)/$(BIN_DIST_NAME)/bin/$(TARGETPLATFORM)/$$i{,.prl};

Admittedly that looks a bit strange if you haven't seen it before, but
it has the advantage of making it clear that the directory is staying
the same.




Makefile:

Given

    binary-pack::
        ( cd $(BIN_DIST_TMPDIR); $(TAR) chzf $(BIN_DIST_NAME).tar.gz $(BIN_DIST_NAME) )

why doesn't

    ifneq "$(way)" ""
    package-way-dist::
        ( cd $(BIN_DIST_TMPDIR); $(FIND) $(BIN_DIST_NAME)/ \( -name "*$(_way).a" -o -name "*.$(way_)hi" \) -print | xargs tar cvf $(BIN_DIST_TMPDIR)/ghc-$(ProjectVersion)-$(way)-$(TARGETPLATFORM).tar )
        gzip $(BIN_DIST_TMPDIR)/ghc-$(ProjectVersion)-$(way)-$(TARGETPLATFORM).tar
    endif

use the z switch to tar too? [and $(TAR) vs tar again]

package-way-dist isn't marked as PHONY either.




Makefile:

    dist-manifest ::
        cd $(SRC_DIST_DIR); $(FIND) . \( -type l -o -type f \) -exec ls -lLG {} \; | sed -e 's/\.\///' > MANIFEST.tmp ; mv MANIFEST.tmp MANIFEST

Why not write directly to MANIFEST?
And errors in early commands will not be noticed by make.



Makefile:

    SRC_DIST_PATHS = $(patsubst %, $(SRC_DIST_NAME)/%, $(SRC_DIST_FILES) $(SRC_DIST_DIRS))

This is just

    SRC_DIST_PATHS = $(addprefix $(SRC_DIST_NAME)/, $(SRC_DIST_FILES) $(SRC_DIST_DIRS))

no?




Thanks
Ian

[0] Reliance on tab stops being every 8 chars for alignment anyone?

_______________________________________________
Cvs-ghc mailing list
Cvs-ghc@haskell.org
http://www.haskell.org/mailman/listinfo/cvs-ghc