ltratt / multitime

Time command execution over multiple executions
http://tratt.net/laurie/src/multitime/
MIT License
115 stars 13 forks source link

Remove override of link rule #16

Closed iustin closed 6 years ago

iustin commented 6 years ago

Currently, the compile rule (.c→.o) is left as is, so it picks up CFLAGS correctly from the environment. However, the link rule is explicitly defined without LDFLAGS, so this prevents the nevironment from passing custom flags.

Since the only reason for the explicit rule is to define the dependencies on libm and the object files, do just that: declare libm in LDLIBS (which will cause it to be passed), declare the objects files, and drop the explicit (redundant) rule.

ltratt commented 6 years ago

I feel I might be missing out on something, but why would we want to delete the line in the Makefile that builds the actual binary?

iustin commented 6 years ago

make has built-in rules for many common actions. One of them is .c to .o (compile step), another one is .o to binary (link step).

In the current version of Makefile, the .c→.o step is left unchanged - note you don't specify how to compile a .c to a .o file, you just say how to assemble the .o files together into the binary. But make already knows how do that too, it just doesn't know what object files should go into the binary (and what libraries to link as well). To specify this, it's enough to say that multitime depends on format.o and multitime.o and libm, but not say how to assemble all those - that's known to make.

Example using current makefile:

$ make multitime
gcc -g -O2 -Wall  -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE   -c -o multitime.o multitime.c
gcc -g -O2 -Wall  -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE   -c -o format.o format.c
gcc -o multitime format.o multitime.o -lm

Note that the first two are result of built-in (implicit) rules, the third one is the current (explicit) rule.

With my patch:

$ make multitime
gcc -g -O2 -Wall  -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE   -c -o multitime.o multitime.c
gcc -g -O2 -Wall  -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE   -c -o format.o format.c
gcc   multitime.o format.o  -lm -o multitime

The third line is now also automatically generated by make itself, based on the object dependencies and the LDLIBS variable.

With my patch and custom LDFLAGS variable (picked up automatically):

$ make multitime LDFLAGS=-Wl,-z,relro
gcc -g -O2 -Wall  -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE   -c -o multitime.o multitime.c
gcc -g -O2 -Wall  -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE   -c -o format.o format.c
gcc -Wl,-z,relro  multitime.o format.o  -lm -o multitime

In the original version, LDFLAGS changes are ignored (due to override), but (e.g.) CFLAGS are picked up.

Does this make more sense? I'm happy to expand the commit message if you want.

iustin commented 6 years ago

Maybe it also helps to explain why I need this - I'm preparing multitime for inclusion in Debian, and in Debian, binaries should be built hardened, and this is done via compiler and linker flags. If the makefile doesn't pick up custom CFLAGS/LDFLAGS, it's extra work for the maintainer to plumb that in (via a patch that has to be maintained forever in Debian). If the makefile does this automatically, everything works well with just a flag change in the packaging rules.

ltratt commented 6 years ago

That extra rule, unfortunately (or fortunately... I don't know!), is only available in GNU make. In BSD make it's absent, so the explicit rule needs to stay in the Makefile. I'm quite willing to think about a way of satisfying both BSD make and GNU make!

In terms of Debian packaging, multitime is overdue for a 1.4 release. Unfortunately some of the work in master is incomplete. However, once we've got things sorted here, I'm happy to make a release branch that collects together the important things, makes a 1.4 release, and then master can chug forward at its own (not very fast) speed.

iustin commented 6 years ago

I'm happy to rework the patch to just pass LDFLAGS along (and any other needed flags). I'm surprised though at the (BSD) make behaviour - it indeed has implicit rules for the compilation step, but not for the link step. Yay for consistency?

Then I'll resend patch to just plug in LDFLAGS and any other needed flags, and I'll test behaviour on FreeBSD as well. Thanks!

iustin commented 6 years ago

Abandoned in favour of #17.