Re: [PATCH 3/3] Makefile: split prefix flags from GIT-CFLAGS

From: Jeff King <peff@peff.net>
Date: Jun 19, 2012, 21:43 UTC
Message-ID: <20120619214308.GA22166@sigill.intra.peff.net>
In-Reply-To: <20120619210426.GA21838@sigill.intra.peff.net>

On Tue, Jun 19, 2012 at 05:04:26PM -0400, Jeff King wrote:

Show 20 quoted lines

> > The damage is not too bad from the point of view of linecount, but
> > this embeds the implicit knowledge of dependencies from $prefix to
> > various path variables to selected object files that embed these
> > paths variables by scattering dependencies on GIT-PREFIX in the
> > Makefile, which does not seem to scale very well.  I wonder if it
> > makes sense to have a single default-paths.o file that holds these
> > strings and recompile only that file when any of the paths change,
> > to localize the damage.
> > 
> > Of course, the current users of GIT_HTML_PATH that expect they can
> > do sizeof(GIT_HTML_PATH)-1 in place of strlen(GIT_HTML_PATH) may
> > need to be adjusted if we go that route.
> 
> Yeah, I think that would be nicer overall. If we move to a link-time
> dependency, then we can even put _all_ of the Makefile-based strings in
> there without ever having to care about who uses it. Of course, it won't
> work for things that truly need to be preprocessor macros (for
> conditional compilation), so we'd still be stuck with those (most of
> them just end up in CFLAGS and trigger a full rebuild, which is probably
> OK).

I started on this, but it is a little bit trickier than that. We can cover C compilation with one strategy, but make variables end up going lots of other places, too, like shell scripts, the perl Makefile generation, etc. Those places all need to individually respect a separate sentinel-file dependency. So I don't think there is an easy way out of the sprinkling of dependencies.

At the very least, I tried to keep the dependency close to the point-of-use, like:

  foo.o: GIT-PREFIX
  foo.o: EXTRA_CPPFLAGS = \
          '-DPREFIX=$(prefix_SQ)'

even if the actual build rules for foo.o are found elsewhere (and typically they are, as they are part of a big pattern-based rule).

Some of the existing locations did not do a great job of that, and instead it like this:

  version.o git.spec \
          $(patsubst %.sh,%,$(SCRIPT_SH)) \
          $(patsubst %.perl,%,$(SCRIPT_PERL)) \
          : GIT-VERSION-FILE

  [... 500 lines later ...]

  git.spec: git.spec.in
          sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' <$< >$@+

If you read the latter hunk and wanted to emulate it, there is nothing to indicate that you must also modify the earlier hunk. I think putting them together makes more sense (even though it technically takes more lines).

-Peff