Why do not write Makefiles in this way

During my work I sometimes see strange code which works, but there are good reasons to not write it in this way. I would like to share with you things that I see and ways how to fix them.

I choose one Makefile from G11n CWS project I am working on (sun_src/closed/lib/4lib/libale/zh_TW/Makefile)

SRCS    = hcode.c hcode_c.c map_b5_to_euc.c
HDRS    = hanyu.h

LIBA    = libhle.a
LIBSO   = libhle.so
VERSION = 1
UTIL =  invertmap_b5_to_euc

include ../Makefile.libale

#
# This script derives map_euctob5 from map_b5_to_euc.
#

$(UTIL) : invertmap_b5_to_euc.c objs/map_b5_to_euc.o
        $(CC) -D_REENTRANT -O -Xt -c -o objs/map_b5_to_euc.32bit.o map_b5_to_euc.c
        $(CC) -D_REENTRANT -O -Xt -o $@ -xregs=no%appl $@.c objs/map_b5_to_euc.32bit.o
        ./$@ | sort > map_euc_to_b5.c
        cat /dev/null > tmpedmap
        for i in  1i \\
            "/% table derived from map_b5_to_euc.c %/" \\
            "#include \\"hanyu.h\\"" "struct map_euctob5" \\
            "map_euctob5[] = {" \\
            . "+a" "};" . w q ; \\
        do  echo $$i | tr "%+" "\\052\\044" >> tmpedmap ;  done
        ed map_euc_to_b5.c < tmpedmap
        cc ${CFLAGS_A} -o objs/map_euc_to_b5.o map_euc_to_b5.c
        cc ${CFLAGS_SO} -o pics/map_euc_to_b5.o map_euc_to_b5.c
        cc ${CFLAGS_A_64} -o $(MACH64)/objs/map_euc_to_b5.o map_euc_to_b5.c
        cc ${CFLAGS_SO_64} -o $(MACH64)/pics/map_euc_to_b5.o map_euc_to_b5.c
        rm tmpedmap

Now let's go throw the code.

First five lines are just a place holder - it defines something which is not used:

SRCS    = hcode.c hcode_c.c map_b5_to_euc.c
HDRS    = hanyu.h

LIBA    = libhle.a
LIBSO   = libhle.so
VERSION = 1
nobody needs them, so fire them.

First command of $(UTIL) rule compile map_b5_to_euc.c but the file is missing in dependency list. If somebody change the source, this target will be not re-compiled:

        $(CC) -D_REENTRANT -O -Xt -c -o objs/map_b5_to_euc.32bit.o map_b5_to_euc.c
It is better to create new rule for this:
objs/map_b5_to_euc.32bit.o: map_b5_to_euc.c
        $(CC) -D_REENTRANT -O -Xt -c -o $@ $<

Next command builds invertmap_b5_to_euc. Automatic variables are very helpful but construct $@.c is ugly. But worse is that we already build target for our rule - if some next command fails, make will not be run them next time:

        $(CC) -D_REENTRANT -O -Xt -o $@ -xregs=no%appl $@.c objs/map_b5_to_euc.32bit.o
It again better to create new rule for this:
invertmap_b5_to_euc.o: invertmap_b5_to_euc.c
        $(CC) -D_REENTRANT -O -Xt -o $@ $<

invertmap_b5_to_euc: invertmap_b5_to_euc.o objs/map_b5_to_euc.32bit.o
        $(CC) -D_REENTRANT -O -Xt -o $@ -xregs=no%appl nvertmap_b5_to_euc.c objs/map_b5_to_euc.32bit.o
BTW SunStudio 11 does not know '-xregs=no%appl'. It is better to put compilation parameters to a varible and use it.

Command sort is locale dependent so the result is different in another locale. In this case if user has cs_CZ locale, map_euc_to_b5.c is not generated correctly and compilation fails. And again - to use automatic variable instead of cmd name is confusing. As well as cat /dev/null > tmpedmap instead of rm tmpedmap

        ./$@ | sort > map_euc_to_b5.c
         cat /dev/null > tmpedmap
Let's continue with next crypto commands. I like UNIX, each thing could be done in many ways:
        for i in  1i \\
            "/% table derived from map_b5_to_euc.c %/" \\
            "#include \\"hanyu.h\\"" "struct map_euctob5" \\
            "map_euctob5[] = {" \\
            . "+a" "};" . w q ; \\
        do  echo $$i | tr "%+" "\\052\\044" >> tmpedmap ;  done
        ed map_euc_to_b5.c < tmpedmap
The same can be done by the code (and again we make new rule):
map_euc_to_b5.c: invertmap_b5_to_euc
        ( \\
                echo '#include "hanyu.h" struct map_euctob5 map_euctob5[] = {'; \\
                ./$@ | LANG=C sort; \\
                echo "};" \\
        ) > map_euc_to_b5.c
('invertmap_b5_to_euc' generates some table, and this add a header and closure. Easier way could be to add two 'printf' and qsort() to invertmap_b5_to_euc.c)

Last four commands use 'cc' instead of $(CC). If you had another cc compiler in your PATH, strange things could happen:

        cc ${CFLAGS_A} -o objs/map_euc_to_b5.o map_euc_to_b5.c
        cc ${CFLAGS_SO} -o pics/map_euc_to_b5.o map_euc_to_b5.c
        cc ${CFLAGS_A_64} -o $(MACH64)/objs/map_euc_to_b5.o map_euc_to_b5.c
        cc ${CFLAGS_SO_64} -o $(MACH64)/pics/map_euc_to_b5.o map_euc_to_b5.c

There is new version of the Makefile:

include ../Makefile.libale

all: objs/map_euc_to_b5.o pics/map_euc_to_b5.o $(MACH64)/objs/map_euc_to_b5.o $(MACH64)/pics/map_euc_to_b5.o

invertmap_b5_to_euc.o: invertmap_b5_to_euc.c
        $(CC) -D_REENTRANT -O -Xt -o $@ $\^

objs/map_b5_to_euc.32bit.o: map_b5_to_euc.c
        $(CC) -D_REENTRANT -O -Xt -c -o $@ $<

map_euc_to_b5.c: invertmap_b5_to_euc
        (       echo '#include "hanyu.h" struct map_euctob5 map_euctob5[] = {'; \\
                ./$@ | LANG=C sort; \\
                echo "};" \\
        ) > map_euc_to_b5.c

invertmap_b5_to_euc: invertmap_b5_to_euc.o objs/map_b5_to_euc.32bit.o
        $(CC) -D_REENTRANT -O -Xt -o $@ -xregs=no%appl $\^

objs/%.o: %.c 
        $(CC) ${CFLAGS_A} -o $@ $\^

pics/%.o: %.c 
        $(CC) ${CFLAGS_SO} -o $@ $\^

$(MACH64)/objs/%.o: %.c
        $(CC) ${CFLAGS_A_64} -o $@ $\^

$(MACH64)/pics/%.o: %.c
        $(CC) ${CFLAGS_SO_64} -o $@ $\^

So, what do we have? We have new Makefile which do the same things like the previous one. Is it better? Yes, a bit - it always fails if some error occures and rebuild all necessary things if user modify some source file. But it costed time - to rewrite it and test it. Should we spend our time on such things?

Comments:

Post a Comment:
  • HTML Syntax: NOT allowed
About

Solaris l10n & i18n, locales, keyboards, fonts and related topics.

Search

Archives
« April 2014
SunMonTueWedThuFriSat
  
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
   
       
Today