2
\$\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
-------------------------------------------------------------------------------
Toby Speight
87.1k14 gold badges104 silver badges322 bronze badges
asked Aug 5, 2017 at 3:41
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Are you targeting GNU Make, or are you aiming for portability? \$\endgroup\$ Commented Aug 5, 2017 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\$ Commented Aug 5, 2017 at 6:20

2 Answers 2

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.

answered Aug 5, 2017 at 5:40
\$\endgroup\$
1
\$\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
answered Aug 7, 2017 at 10:20
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.