1
\$\begingroup\$

My Makefile looks like this

opsh:   shellparser.c main.c util.c errors.c
    gcc -std=c99 -oopsh shellparser.c main.c util.c errors.c -lreadline -ltermcap

PREFIX = /usr/local

.PHONY: install
install: opsh
    mkdir -p $(DESTDIR)$(PREFIX)/bin
    cp $< $(DESTDIR)$(PREFIX)/bin/opsh

.PHONY: uninstall
uninstall:
    rm -f $(DESTDIR)$(PREFIX)/bin/opsh

The project is available on github. The statistics are

$ cloc .
      30 text files.
      30 unique files.                              
      45 files ignored.

http://cloc.sourceforge.net v 1.60  T=0.08 s (283.8 files/s, 123084.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C                                6            837           2025           7105
C/C++ Header                     7             53             15            143
Bourne Shell                     7              6              6             93
yacc                             1              8              2             56
CMake                            1             12              0             28
make                             1              3              0             10
YAML                             1              1              0              5
-------------------------------------------------------------------------------
SUM:                            24            920           2048           7440
-------------------------------------------------------------------------------
\$\endgroup\$
  • 2
    \$\begingroup\$ Are you targeting GNU Make, or are you aiming for portability? \$\endgroup\$ – 200_success Aug 5 '17 at 6:15
  • 2
    \$\begingroup\$ @200_success I feel that I must have portable code. I don't have enough experience to make the decision. I suppose I rather want portability. The project is a command-line interpreter (a custom Unix shell) and my goal is to learn autotools and how to package it for portability. \$\endgroup\$ – Niklas Aug 5 '17 at 6:20
2
\$\begingroup\$
  • You should investigate whether GNU Make (or whichever Make implementation your users are supposed to use) has a special variable for all dependencies of the current rule, so that you don't need to list the source files twice.

  • The command line for gcc is missing the -Wall -Wextra -O2 options to catch common mistakes and to make the program run faster.

  • If you want the Makefile to be portable to systems other than GNU Linux on x86_64, you should switch to GNU autoconf, but that's a lot of work. If your project is just a toy project, it's fine as it is.

\$\endgroup\$
0
\$\begingroup\$

Instead of specifying a particular compiler and flags, it's good practice to use Make's standard variables CC and CFLAGS, to allow users to pass in their preferences as necessary. Assuming that your source files are separately compilable, we usually separate compilation and linking, so that a change to shellparser.c doesn't require a rebuild of util.c and errors.c, for example. With those changes, the first two lines can be replaced with

CFLAGS += -std=c99
LDLIBS += -lreadline -ltermcap

opsh: shellparser.o main.o util.o errors.o

Built-in rules will do the rest.


If you have any header files, you'll want to make them dependencies of the object files they affect. You can do this manually, but a low-maintenance approach is to generate dependencies automatically. That's a whole question in itself, but since you're using GCC, you will probably want Combined Compilation and Dependency Generation:

DEPFLAGS = -MT $@ -MMD -MP -MF $*.Td
COMPILE.c = $(CC) $(DEPFLAGS) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
POSTCOMPILE = @mv -f $*.Td $*.d && touch $@

OBJECTS := shellparser.o main.o util.o errors.o
opsh: $(OBJECTS)

%.o : %.c
%.o : %.c %.d
        $(COMPILE.c) $(OUTPUT_OPTION) $<
        $(POSTCOMPILE)


%.d: ;
.PRECIOUS: %.d
include $(OBJECTS:.o=.d)

The install target would be better to use the install command, in order to set the correct ownership and permissions on the created files:

install: opsh
        install -m755 -D -d $(DESTDIR)$(PREFIX)/bin
        install -m755 $< $(DESTDIR)$(PREFIX)/bin/opsh

All Makefiles should have .DELETE_ON_ERROR, to cause the target to be removed if its recipe fails. Otherwise, a subsequent make would use in improperly-built target. The only case when you wouldn't want .DELETE_ON_ERROR is when debugging a compiler; it may be necessary to inspect the incomplete or invalid target file.


Modified Makefile

With the above changes, you have

OBJECTS := shellparser.o main.o util.o errors.o
opsh: $(OBJECTS)

CFLAGS += -std=c99
LDLIBS += -lreadline -ltermcap

DEPFLAGS = -MT $@ -MMD -MP -MF $*.Td
COMPILE.c = $(CC) $(DEPFLAGS) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
POSTCOMPILE = @mv -f $*.Td $*.d && touch $@

OBJECTS := shellparser.o main.o util.o errors.o
opsh: $(OBJECTS)

%.o : %.c
%.o : %.c %.d
        $(COMPILE.c) $(OUTPUT_OPTION) $<
        $(POSTCOMPILE)


%.d: ;
include $(OBJECTS:.o=.d)

.PRECIOUS: %.d

install: opsh
        install -m755 -D -d $(DESTDIR)$(PREFIX)/bin
        install -m755 $< $(DESTDIR)$(PREFIX)/bin/opsh

uninstall:
        $(RM) $(DESTDIR)$(PREFIX)/bin/opsh

.PHONY: install uninstall
.DELETE_ON_ERROR
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.