Build Failure: GIT-GUI-VARS

"make all" fails with "NO_TCLTK=y" because gitk-wish depends on GIT- GUI-VARS which is defined inside the ifndef block.

AFAICT, either gitk-wish should be moved inside a ifndef block or an else should be added with a phony GIT-GUI-VARS. I don't know which is preferred.

Actually, I can't tell why make is trying to build it in the first place. :-/ gitk-wish is added to OTHER_PROGRAMS inside a NO_TCLTK block. (Debian oldstable, GNU Make 3.80, git repo v1.5.2-rc3)

And, while I'm poking through here, there's a line labeled "Backward compatibility -- to be removed after 1.0". Git IDs itself as 1.5.2.rc3. Isn't that after 1.0? :-)

~~ Brian

Brian Gernhardt <benji@silverinsanity.com> writes:

I do not either, and obviously does not reproduce for me. Time for you to try "make -d" perhaps?

On May 10, 2007, at 10:30 PM, Junio C Hamano wrote:

Sorry for the semi-coherent message. It started out as "we should fix this" and I ended up going "why is it broken", all a little too late at night. I'm also not used to debugging Makefiles.

And I figured it out (make -p actually was more useful than -d): It's another issue with doing things before including config.mak. My config.mak includes the "NO_TCLTK=y" line, which takes effect at Makefile:485. But gitk-wish is getting included in OTHER_PROGRAMS at Makefile:272. So gitk-wish gets included because NO_TCLTK isn't defined at that point, but all the other rules involving gitk don't get executed because they're after the NO_TCLTK definition.

I think we need to be including the config.mak much earlier in the Makefile as this is not the first time issues like this have come up for me. And they tend to be issues that only come up with specific configurations and are therefore harder to track down. Things I can see being an issue for people who put their options in config.mak:

149: ifeq ($(prefix),/usr) 224: ifdef WITH_P4IMPORT 271: ifndef NO_TCLTK # My issue today 279-285: ifndef {SHELL,PERL,PYTHON}_PATH

~~ Brian

Brian Gernhardt <benji@silverinsanity.com> writes:

Ahh, config.mak.

I have not been very sympathetic to config.mak from the beginning, although people seem to want it. As I try to arrange variable overrides to be passed from the command line anyway, I've not used config.mak myself.

However, it needs to be able to override NO_*, NEEDS_* and friends which means it should come after platform specific defaults. But one platform wants to change the default behaviour depend on the settings of NO_*, possibly set in config.mak.

It is a mess. Moving it around would fix one person's problem while breaking others. I think the best course of action is

 (1) first for somebody more fluent in speaking Makefile than me
     to volunteer to tackle this problem, and ask help from the
     list;

 (2) have people on various "non-stock" configurations sign-up
     as testers; this does not have to cover the systems listed
     in the ifeq ($(uname_S),XXX) part of the current Makefile;
     what is of interest is to see what kind of variables are
     overridden on the real-world systems via config.mak;

 (3) The volunteer cooks up an improved Makefile, using
     config.mak "non-stock" testers have.

This would be something that "volunteer" would be deciding, but I have a feeling that we can have two config.mak include in the makefile, one early and one late. But I suspect that picking the right two places would have the same "moving would fix one while breaking another" issue.

On Fri, May 11, 2007 at 10:10:26AM -0700, Junio C Hamano wrote:

I wonder if we would be better served by moving these sorts of build-time configuration decisions into the actual make dependency tree. E.g., something like:

  openssl.lib: mklib-openssl.sh conf-openssl
    sh mklib-openssl.sh <conf-ssl >openssl.lib

  program: main.o openssl.lib:
    cc -o "$@" main.o `cat openssl.lib`

where conf-openssl specifies the user's preference (either actual library paths, "auto" for autodetection, or "none" not to use it at all), and mklib-openssl is a script that converts that into the command line options for the link.

You can of course do the same with creating a .h file to choose an implementation (you just make a file that #define's the correct thing).

The nice thing about this approach is that:
  1. You move configuration cruft out of the Makefile, making it much
     easier to read. Instead, you have a series of very small and
     obvious shell scripts.
  2. The dependency chain is actually correct. If I edit conf-openssl,
     then that should trigger a re-link for everything which compiles
     against it.

You can also use this for portability fixes:

  program: main.o strcasestr.o
    cc -o "$@" main.o strcasestr.o

  strcasestr.o choose try_strcasestr.c compat/strcasestr.c
    sh choose try_strcasestr.c compat/strcasestr.c

where choose is a script that compiles and runs some test program and uses the result to choose a source file to become strcasestr.c. Thus you _always_ link against strcasestr.o, it's just that sometimes there's an implementation of strcasestr in it (if required by the platform) and sometimes it's empty (or an alternate implementation, etc).

I have used this technique many times, and would be happy to be involved in changing the Makefile. However, it's going to be quite a large change, and I recognize that this style is not familiar to most people, so obviously that should be taken into account.

-Peff