threads / patch / 25743

patch, 4 partsMakefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Subject: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

## tl;dr

25 messages between Nov 14, 2010 and Nov 17, 2010. Diffs are folded; open one to read it.

replies: 24people: 4as markdown or json

Ævar Arnfjörð Bjarmason· Nov 14, 2010, 14:44 UTC · lore

[PATCH 0/4] [PULL] ab/i18n-prereqs, prerequisites for ab/i18n

These are the first four patches in the ab/i18n series which aren't strictly related to it, but merely fixes to issues I encountered.

I'm submitting these separately now in the hope that they'll make it to the next & master branches before ab/i18n. Having these separately reviewed should be easier for everyone.

These patches can also be fetched from:
    git://github.com/avar/git.git ab/i18n-prereqs
Ævar Arnfjörð Bjarmason (4):
  Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}
  t7004-tag.sh: re-arrange git tag comment for clarity
  tests: use test_cmp instead of piping to diff(1)
  builtin: use builtin.h for all builtin commands
 Makefile                         |  687 +++++++++++++++++++-------------------
 builtin/clone.c                  |    3 +-
 builtin/fetch-pack.c             |    2 +-
 builtin/hash-object.c            |    2 +-
 builtin/index-pack.c             |    2 +-
 builtin/merge-index.c            |    2 +-
 builtin/merge-recursive.c        |    2 +-
 builtin/merge-tree.c             |    2 +-
 builtin/mktag.c                  |    2 +-
 builtin/pack-redundant.c         |    2 +-
 builtin/pack-refs.c              |    2 +-
 builtin/patch-id.c               |    2 +-
 builtin/receive-pack.c           |    2 +-
 builtin/remote.c                 |    3 +-
 builtin/reset.c                  |    2 +-
 builtin/send-pack.c              |    2 +-
 builtin/unpack-file.c            |    2 +-
 builtin/var.c                    |    2 +-
 t/t4041-diff-submodule-option.sh |   96 ++++--
 t/t7004-tag.sh                   |   20 +-
 t/t7401-submodule-summary.sh     |   47 ++-
 wt-status.c                      |    2 +-
 22 files changed, 472 insertions(+), 416 deletions(-)
-- 
1.7.2.3
Ævar Arnfjörð Bjarmason· Nov 14, 2010, 14:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

Change the Makefile so that the "Platform specific tweaks" section comes before the assignments to LIB_H and LIB_OBJS.

In the ab/i18n series I only want to build gettext.o (by adding it to LIB_OBJS) if NO_GETTEXT is unset. It's not possible to do that without an ugly hack if we haven't applied our platform specific tweaks before LIB_{H,OBJS} gets assigned to.

See <201008140002.40587.j6t@kdbg.org> (subject: "[PATCH] Do not build i18n on Windows.") for Johannes's original report, and my follow-up in <AANLkTiku5R+idX-C8f0AcCikBLmfEb5ZEhdft+CSRzU0@mail.gmail.com> where I suggested that the problem be solved in the manner of this patch.

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile |  687 +++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 344 insertions(+), 343 deletions(-)
Show changes to Makefile +344 −343
diff --git a/Makefile b/Makefile
index 1f1ce04..b1f5d8c 100644
--- a/Makefile
+++ b/Makefile
@@ -483,278 +483,6 @@ LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a
 VCSSVN_LIB=vcs-svn/lib.a
 
-LIB_H += advice.h
-LIB_H += archive.h
-LIB_H += attr.h
-LIB_H += blob.h
-LIB_H += builtin.h
-LIB_H += cache.h
-LIB_H += cache-tree.h
-LIB_H += color.h
-LIB_H += commit.h
-LIB_H += compat/bswap.h
-LIB_H += compat/cygwin.h
-LIB_H += compat/mingw.h
-LIB_H += compat/win32/pthread.h
-LIB_H += csum-file.h
-LIB_H += decorate.h
-LIB_H += delta.h
-LIB_H += diffcore.h
-LIB_H += diff.h
-LIB_H += dir.h
-LIB_H += exec_cmd.h
-LIB_H += fsck.h
-LIB_H += git-compat-util.h
-LIB_H += graph.h
-LIB_H += grep.h
-LIB_H += hash.h
-LIB_H += help.h
-LIB_H += levenshtein.h
-LIB_H += list-objects.h
-LIB_H += ll-merge.h
-LIB_H += log-tree.h
-LIB_H += mailmap.h
-LIB_H += merge-recursive.h
-LIB_H += notes.h
-LIB_H += notes-cache.h
-LIB_H += object.h
-LIB_H += pack.h
-LIB_H += pack-refs.h
-LIB_H += pack-revindex.h
-LIB_H += parse-options.h
-LIB_H += patch-ids.h
-LIB_H += pkt-line.h
-LIB_H += progress.h
-LIB_H += quote.h
-LIB_H += reflog-walk.h
-LIB_H += refs.h
-LIB_H += remote.h
-LIB_H += rerere.h
-LIB_H += resolve-undo.h
-LIB_H += revision.h
-LIB_H += run-command.h
-LIB_H += sha1-lookup.h
-LIB_H += sideband.h
-LIB_H += sigchain.h
-LIB_H += strbuf.h
-LIB_H += string-list.h
-LIB_H += submodule.h
-LIB_H += tag.h
-LIB_H += transport.h
-LIB_H += tree.h
-LIB_H += tree-walk.h
-LIB_H += unpack-trees.h
-LIB_H += userdiff.h
-LIB_H += utf8.h
-LIB_H += xdiff-interface.h
-LIB_H += xdiff/xdiff.h
-
-LIB_OBJS += abspath.o
-LIB_OBJS += advice.o
-LIB_OBJS += alias.o
-LIB_OBJS += alloc.o
-LIB_OBJS += archive.o
-LIB_OBJS += archive-tar.o
-LIB_OBJS += archive-zip.o
-LIB_OBJS += attr.o
-LIB_OBJS += base85.o
-LIB_OBJS += bisect.o
-LIB_OBJS += blob.o
-LIB_OBJS += branch.o
-LIB_OBJS += bundle.o
-LIB_OBJS += cache-tree.o
-LIB_OBJS += color.o
-LIB_OBJS += combine-diff.o
-LIB_OBJS += commit.o
-LIB_OBJS += config.o
-LIB_OBJS += connect.o
-LIB_OBJS += convert.o
-LIB_OBJS += copy.o
-LIB_OBJS += csum-file.o
-LIB_OBJS += ctype.o
-LIB_OBJS += date.o
-LIB_OBJS += decorate.o
-LIB_OBJS += diffcore-break.o
-LIB_OBJS += diffcore-delta.o
-LIB_OBJS += diffcore-order.o
-LIB_OBJS += diffcore-pickaxe.o
-LIB_OBJS += diffcore-rename.o
-LIB_OBJS += diff-delta.o
-LIB_OBJS += diff-lib.o
-LIB_OBJS += diff-no-index.o
-LIB_OBJS += diff.o
-LIB_OBJS += dir.o
-LIB_OBJS += editor.o
-LIB_OBJS += entry.o
-LIB_OBJS += environment.o
-LIB_OBJS += exec_cmd.o
-LIB_OBJS += fsck.o
-LIB_OBJS += graph.o
-LIB_OBJS += grep.o
-LIB_OBJS += hash.o
-LIB_OBJS += help.o
-LIB_OBJS += hex.o
-LIB_OBJS += ident.o
-LIB_OBJS += levenshtein.o
-LIB_OBJS += list-objects.o
-LIB_OBJS += ll-merge.o
-LIB_OBJS += lockfile.o
-LIB_OBJS += log-tree.o
-LIB_OBJS += mailmap.o
-LIB_OBJS += match-trees.o
-LIB_OBJS += merge-file.o
-LIB_OBJS += merge-recursive.o
-LIB_OBJS += name-hash.o
-LIB_OBJS += notes.o
-LIB_OBJS += notes-cache.o
-LIB_OBJS += object.o
-LIB_OBJS += pack-check.o
-LIB_OBJS += pack-refs.o
-LIB_OBJS += pack-revindex.o
-LIB_OBJS += pack-write.o
-LIB_OBJS += pager.o
-LIB_OBJS += parse-options.o
-LIB_OBJS += patch-delta.o
-LIB_OBJS += patch-ids.o
-LIB_OBJS += path.o
-LIB_OBJS += pkt-line.o
-LIB_OBJS += preload-index.o
-LIB_OBJS += pretty.o
-LIB_OBJS += progress.o
-LIB_OBJS += quote.o
-LIB_OBJS += reachable.o
-LIB_OBJS += read-cache.o
-LIB_OBJS += reflog-walk.o
-LIB_OBJS += refs.o
-LIB_OBJS += remote.o
-LIB_OBJS += replace_object.o
-LIB_OBJS += rerere.o
-LIB_OBJS += resolve-undo.o
-LIB_OBJS += revision.o
-LIB_OBJS += run-command.o
-LIB_OBJS += server-info.o
-LIB_OBJS += setup.o
-LIB_OBJS += sha1-lookup.o
-LIB_OBJS += sha1_file.o
-LIB_OBJS += sha1_name.o
-LIB_OBJS += shallow.o
-LIB_OBJS += sideband.o
-LIB_OBJS += sigchain.o
-LIB_OBJS += strbuf.o
-LIB_OBJS += string-list.o
-LIB_OBJS += submodule.o
-LIB_OBJS += symlinks.o
-LIB_OBJS += tag.o
-LIB_OBJS += trace.o
-LIB_OBJS += transport.o
-LIB_OBJS += transport-helper.o
-LIB_OBJS += tree-diff.o
-LIB_OBJS += tree.o
-LIB_OBJS += tree-walk.o
-LIB_OBJS += unpack-trees.o
-LIB_OBJS += url.o
-LIB_OBJS += usage.o
-LIB_OBJS += userdiff.o
-LIB_OBJS += utf8.o
-LIB_OBJS += walker.o
-LIB_OBJS += wrapper.o
-LIB_OBJS += write_or_die.o
-LIB_OBJS += ws.o
-LIB_OBJS += wt-status.o
-LIB_OBJS += xdiff-interface.o
-
-BUILTIN_OBJS += builtin/add.o
-BUILTIN_OBJS += builtin/annotate.o
-BUILTIN_OBJS += builtin/apply.o
-BUILTIN_OBJS += builtin/archive.o
-BUILTIN_OBJS += builtin/bisect--helper.o
-BUILTIN_OBJS += builtin/blame.o
-BUILTIN_OBJS += builtin/branch.o
-BUILTIN_OBJS += builtin/bundle.o
-BUILTIN_OBJS += builtin/cat-file.o
-BUILTIN_OBJS += builtin/check-attr.o
-BUILTIN_OBJS += builtin/check-ref-format.o
-BUILTIN_OBJS += builtin/checkout-index.o
-BUILTIN_OBJS += builtin/checkout.o
-BUILTIN_OBJS += builtin/clean.o
-BUILTIN_OBJS += builtin/clone.o
-BUILTIN_OBJS += builtin/commit-tree.o
-BUILTIN_OBJS += builtin/commit.o
-BUILTIN_OBJS += builtin/config.o
-BUILTIN_OBJS += builtin/count-objects.o
-BUILTIN_OBJS += builtin/describe.o
-BUILTIN_OBJS += builtin/diff-files.o
-BUILTIN_OBJS += builtin/diff-index.o
-BUILTIN_OBJS += builtin/diff-tree.o
-BUILTIN_OBJS += builtin/diff.o
-BUILTIN_OBJS += builtin/fast-export.o
-BUILTIN_OBJS += builtin/fetch-pack.o
-BUILTIN_OBJS += builtin/fetch.o
-BUILTIN_OBJS += builtin/fmt-merge-msg.o
-BUILTIN_OBJS += builtin/for-each-ref.o
-BUILTIN_OBJS += builtin/fsck.o
-BUILTIN_OBJS += builtin/gc.o
-BUILTIN_OBJS += builtin/grep.o
-BUILTIN_OBJS += builtin/hash-object.o
-BUILTIN_OBJS += builtin/help.o
-BUILTIN_OBJS += builtin/index-pack.o
-BUILTIN_OBJS += builtin/init-db.o
-BUILTIN_OBJS += builtin/log.o
-BUILTIN_OBJS += builtin/ls-files.o
-BUILTIN_OBJS += builtin/ls-remote.o
-BUILTIN_OBJS += builtin/ls-tree.o
-BUILTIN_OBJS += builtin/mailinfo.o
-BUILTIN_OBJS += builtin/mailsplit.o
-BUILTIN_OBJS += builtin/merge.o
-BUILTIN_OBJS += builtin/merge-base.o
-BUILTIN_OBJS += builtin/merge-file.o
-BUILTIN_OBJS += builtin/merge-index.o
-BUILTIN_OBJS += builtin/merge-ours.o
-BUILTIN_OBJS += builtin/merge-recursive.o
-BUILTIN_OBJS += builtin/merge-tree.o
-BUILTIN_OBJS += builtin/mktag.o
-BUILTIN_OBJS += builtin/mktree.o
-BUILTIN_OBJS += builtin/mv.o
-BUILTIN_OBJS += builtin/name-rev.o
-BUILTIN_OBJS += builtin/notes.o
-BUILTIN_OBJS += builtin/pack-objects.o
-BUILTIN_OBJS += builtin/pack-redundant.o
-BUILTIN_OBJS += builtin/pack-refs.o
-BUILTIN_OBJS += builtin/patch-id.o
-BUILTIN_OBJS += builtin/prune-packed.o
-BUILTIN_OBJS += builtin/prune.o
-BUILTIN_OBJS += builtin/push.o
-BUILTIN_OBJS += builtin/read-tree.o
-BUILTIN_OBJS += builtin/receive-pack.o
-BUILTIN_OBJS += builtin/reflog.o
-BUILTIN_OBJS += builtin/remote.o
-BUILTIN_OBJS += builtin/replace.o
-BUILTIN_OBJS += builtin/rerere.o
-BUILTIN_OBJS += builtin/reset.o
-BUILTIN_OBJS += builtin/rev-list.o
-BUILTIN_OBJS += builtin/rev-parse.o
-BUILTIN_OBJS += builtin/revert.o
-BUILTIN_OBJS += builtin/rm.o
-BUILTIN_OBJS += builtin/send-pack.o
-BUILTIN_OBJS += builtin/shortlog.o
-BUILTIN_OBJS += builtin/show-branch.o
-BUILTIN_OBJS += builtin/show-ref.o
-BUILTIN_OBJS += builtin/stripspace.o
-BUILTIN_OBJS += builtin/symbolic-ref.o
-BUILTIN_OBJS += builtin/tag.o
-BUILTIN_OBJS += builtin/tar-tree.o
-BUILTIN_OBJS += builtin/unpack-file.o
-BUILTIN_OBJS += builtin/unpack-objects.o
-BUILTIN_OBJS += builtin/update-index.o
-BUILTIN_OBJS += builtin/update-ref.o
-BUILTIN_OBJS += builtin/update-server-info.o
-BUILTIN_OBJS += builtin/upload-archive.o
-BUILTIN_OBJS += builtin/var.o
-BUILTIN_OBJS += builtin/verify-pack.o
-BUILTIN_OBJS += builtin/verify-tag.o
-BUILTIN_OBJS += builtin/write-tree.o
-
-GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 EXTLIBS =
 
 #
@@ -1077,78 +805,351 @@ ifeq ($(uname_S),Windows)
 	BLK_SHA1 = YesPlease
 	NATIVE_CRLF = YesPlease
 
-	CC = compat/vcbuild/scripts/clink.pl
-	AR = compat/vcbuild/scripts/lib.pl
-	CFLAGS =
-	BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
-	COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o
-	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
-	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
-	EXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib
-	PTHREAD_LIBS =
-	lib =
-ifndef DEBUG
-	BASIC_CFLAGS += -GL -Os -MT
-	BASIC_LDFLAGS += -LTCG
-	AR += -LTCG
-else
-	BASIC_CFLAGS += -Zi -MTd
-endif
-	X = .exe
-endif
-ifneq (,$(findstring MINGW,$(uname_S)))
-	pathsep = ;
-	NO_PREAD = YesPlease
-	NEEDS_CRYPTO_WITH_SSL = YesPlease
-	NO_LIBGEN_H = YesPlease
-	NO_SYMLINK_HEAD = YesPlease
-	NO_SETENV = YesPlease
-	NO_UNSETENV = YesPlease
-	NO_STRCASESTR = YesPlease
-	NO_STRLCPY = YesPlease
-	NO_STRTOK_R = YesPlease
-	NO_MEMMEM = YesPlease
-	NEEDS_LIBICONV = YesPlease
-	OLD_ICONV = YesPlease
-	NO_C99_FORMAT = YesPlease
-	NO_STRTOUMAX = YesPlease
-	NO_MKDTEMP = YesPlease
-	NO_MKSTEMPS = YesPlease
-	NO_SVN_TESTS = YesPlease
-	NO_PERL_MAKEMAKER = YesPlease
-	RUNTIME_PREFIX = YesPlease
-	NO_POSIX_ONLY_PROGRAMS = YesPlease
-	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-	NO_NSEC = YesPlease
-	USE_WIN32_MMAP = YesPlease
-	USE_NED_ALLOCATOR = YesPlease
-	UNRELIABLE_FSTAT = UnfortunatelyYes
-	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
-	NO_REGEX = YesPlease
-	NO_PYTHON = YesPlease
-	BLK_SHA1 = YesPlease
-	ETAGS_TARGET = ETAGS
-	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch -Icompat/win32
-	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
-	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o \
-		compat/win32/pthread.o
-	EXTLIBS += -lws2_32
-	PTHREAD_LIBS =
-	X = .exe
-ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
-	htmldir=doc/git/html/
-	prefix =
-	INSTALL = /bin/install
-	EXTLIBS += /mingw/lib/libz.a
-	NO_R_TO_GCC_LINKER = YesPlease
-	INTERNAL_QSORT = YesPlease
-else
-	NO_CURL = YesPlease
-endif
-endif
+	CC = compat/vcbuild/scripts/clink.pl
+	AR = compat/vcbuild/scripts/lib.pl
+	CFLAGS =
+	BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
+	COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o
+	COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
+	BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
+	EXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib
+	PTHREAD_LIBS =
+	lib =
+ifndef DEBUG
+	BASIC_CFLAGS += -GL -Os -MT
+	BASIC_LDFLAGS += -LTCG
+	AR += -LTCG
+else
+	BASIC_CFLAGS += -Zi -MTd
+endif
+	X = .exe
+endif
+ifneq (,$(findstring MINGW,$(uname_S)))
+	pathsep = ;
+	NO_PREAD = YesPlease
+	NEEDS_CRYPTO_WITH_SSL = YesPlease
+	NO_LIBGEN_H = YesPlease
+	NO_SYMLINK_HEAD = YesPlease
+	NO_SETENV = YesPlease
+	NO_UNSETENV = YesPlease
+	NO_STRCASESTR = YesPlease
+	NO_STRLCPY = YesPlease
+	NO_STRTOK_R = YesPlease
+	NO_MEMMEM = YesPlease
+	NEEDS_LIBICONV = YesPlease
+	OLD_ICONV = YesPlease
+	NO_C99_FORMAT = YesPlease
+	NO_STRTOUMAX = YesPlease
+	NO_MKDTEMP = YesPlease
+	NO_MKSTEMPS = YesPlease
+	NO_SVN_TESTS = YesPlease
+	NO_PERL_MAKEMAKER = YesPlease
+	RUNTIME_PREFIX = YesPlease
+	NO_POSIX_ONLY_PROGRAMS = YesPlease
+	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
+	NO_NSEC = YesPlease
+	USE_WIN32_MMAP = YesPlease
+	USE_NED_ALLOCATOR = YesPlease
+	UNRELIABLE_FSTAT = UnfortunatelyYes
+	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
+	NO_REGEX = YesPlease
+	NO_PYTHON = YesPlease
+	BLK_SHA1 = YesPlease
+	ETAGS_TARGET = ETAGS
+	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch -Icompat/win32
+	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
+	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o \
+		compat/win32/pthread.o
+	EXTLIBS += -lws2_32
+	PTHREAD_LIBS =
+	X = .exe
+ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
+	htmldir=doc/git/html/
+	prefix =
+	INSTALL = /bin/install
+	EXTLIBS += /mingw/lib/libz.a
+	NO_R_TO_GCC_LINKER = YesPlease
+	INTERNAL_QSORT = YesPlease
+else
+	NO_CURL = YesPlease
+endif
+endif
+
+-include config.mak.autogen
+-include config.mak
+
+LIB_H += advice.h
+LIB_H += archive.h
+LIB_H += attr.h
+LIB_H += blob.h
+LIB_H += builtin.h
+LIB_H += cache.h
+LIB_H += cache-tree.h
+LIB_H += color.h
+LIB_H += commit.h
+LIB_H += compat/bswap.h
+LIB_H += compat/cygwin.h
+LIB_H += compat/mingw.h
+LIB_H += compat/win32/pthread.h
+LIB_H += csum-file.h
+LIB_H += decorate.h
+LIB_H += delta.h
+LIB_H += diffcore.h
+LIB_H += diff.h
+LIB_H += dir.h
+LIB_H += exec_cmd.h
+LIB_H += fsck.h
+LIB_H += git-compat-util.h
+LIB_H += graph.h
+LIB_H += grep.h
+LIB_H += hash.h
+LIB_H += help.h
+LIB_H += levenshtein.h
+LIB_H += list-objects.h
+LIB_H += ll-merge.h
+LIB_H += log-tree.h
+LIB_H += mailmap.h
+LIB_H += merge-recursive.h
+LIB_H += notes.h
+LIB_H += notes-cache.h
+LIB_H += object.h
+LIB_H += pack.h
+LIB_H += pack-refs.h
+LIB_H += pack-revindex.h
+LIB_H += parse-options.h
+LIB_H += patch-ids.h
+LIB_H += pkt-line.h
+LIB_H += progress.h
+LIB_H += quote.h
+LIB_H += reflog-walk.h
+LIB_H += refs.h
+LIB_H += remote.h
+LIB_H += rerere.h
+LIB_H += resolve-undo.h
+LIB_H += revision.h
+LIB_H += run-command.h
+LIB_H += sha1-lookup.h
+LIB_H += sideband.h
+LIB_H += sigchain.h
+LIB_H += strbuf.h
+LIB_H += string-list.h
+LIB_H += submodule.h
+LIB_H += tag.h
+LIB_H += transport.h
+LIB_H += tree.h
+LIB_H += tree-walk.h
+LIB_H += unpack-trees.h
+LIB_H += userdiff.h
+LIB_H += utf8.h
+LIB_H += xdiff-interface.h
+LIB_H += xdiff/xdiff.h
+
+LIB_OBJS += abspath.o
+LIB_OBJS += advice.o
+LIB_OBJS += alias.o
+LIB_OBJS += alloc.o
+LIB_OBJS += archive.o
+LIB_OBJS += archive-tar.o
+LIB_OBJS += archive-zip.o
+LIB_OBJS += attr.o
+LIB_OBJS += base85.o
+LIB_OBJS += bisect.o
+LIB_OBJS += blob.o
+LIB_OBJS += branch.o
+LIB_OBJS += bundle.o
+LIB_OBJS += cache-tree.o
+LIB_OBJS += color.o
+LIB_OBJS += combine-diff.o
+LIB_OBJS += commit.o
+LIB_OBJS += config.o
+LIB_OBJS += connect.o
+LIB_OBJS += convert.o
+LIB_OBJS += copy.o
+LIB_OBJS += csum-file.o
+LIB_OBJS += ctype.o
+LIB_OBJS += date.o
+LIB_OBJS += decorate.o
+LIB_OBJS += diffcore-break.o
+LIB_OBJS += diffcore-delta.o
+LIB_OBJS += diffcore-order.o
+LIB_OBJS += diffcore-pickaxe.o
+LIB_OBJS += diffcore-rename.o
+LIB_OBJS += diff-delta.o
+LIB_OBJS += diff-lib.o
+LIB_OBJS += diff-no-index.o
+LIB_OBJS += diff.o
+LIB_OBJS += dir.o
+LIB_OBJS += editor.o
+LIB_OBJS += entry.o
+LIB_OBJS += environment.o
+LIB_OBJS += exec_cmd.o
+LIB_OBJS += fsck.o
+LIB_OBJS += graph.o
+LIB_OBJS += grep.o
+LIB_OBJS += hash.o
+LIB_OBJS += help.o
+LIB_OBJS += hex.o
+LIB_OBJS += ident.o
+LIB_OBJS += levenshtein.o
+LIB_OBJS += list-objects.o
+LIB_OBJS += ll-merge.o
+LIB_OBJS += lockfile.o
+LIB_OBJS += log-tree.o
+LIB_OBJS += mailmap.o
+LIB_OBJS += match-trees.o
+LIB_OBJS += merge-file.o
+LIB_OBJS += merge-recursive.o
+LIB_OBJS += name-hash.o
+LIB_OBJS += notes.o
+LIB_OBJS += notes-cache.o
+LIB_OBJS += object.o
+LIB_OBJS += pack-check.o
+LIB_OBJS += pack-refs.o
+LIB_OBJS += pack-revindex.o
+LIB_OBJS += pack-write.o
+LIB_OBJS += pager.o
+LIB_OBJS += parse-options.o
+LIB_OBJS += patch-delta.o
+LIB_OBJS += patch-ids.o
+LIB_OBJS += path.o
+LIB_OBJS += pkt-line.o
+LIB_OBJS += preload-index.o
+LIB_OBJS += pretty.o
+LIB_OBJS += progress.o
+LIB_OBJS += quote.o
+LIB_OBJS += reachable.o
+LIB_OBJS += read-cache.o
+LIB_OBJS += reflog-walk.o
+LIB_OBJS += refs.o
+LIB_OBJS += remote.o
+LIB_OBJS += replace_object.o
+LIB_OBJS += rerere.o
+LIB_OBJS += resolve-undo.o
+LIB_OBJS += revision.o
+LIB_OBJS += run-command.o
+LIB_OBJS += server-info.o
+LIB_OBJS += setup.o
+LIB_OBJS += sha1-lookup.o
+LIB_OBJS += sha1_file.o
+LIB_OBJS += sha1_name.o
+LIB_OBJS += shallow.o
+LIB_OBJS += sideband.o
+LIB_OBJS += sigchain.o
+LIB_OBJS += strbuf.o
+LIB_OBJS += string-list.o
+LIB_OBJS += submodule.o
+LIB_OBJS += symlinks.o
+LIB_OBJS += tag.o
+LIB_OBJS += trace.o
+LIB_OBJS += transport.o
+LIB_OBJS += transport-helper.o
+LIB_OBJS += tree-diff.o
+LIB_OBJS += tree.o
+LIB_OBJS += tree-walk.o
+LIB_OBJS += unpack-trees.o
+LIB_OBJS += url.o
+LIB_OBJS += usage.o
+LIB_OBJS += userdiff.o
+LIB_OBJS += utf8.o
+LIB_OBJS += walker.o
+LIB_OBJS += wrapper.o
+LIB_OBJS += write_or_die.o
+LIB_OBJS += ws.o
+LIB_OBJS += wt-status.o
+LIB_OBJS += xdiff-interface.o
+
+BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/annotate.o
+BUILTIN_OBJS += builtin/apply.o
+BUILTIN_OBJS += builtin/archive.o
+BUILTIN_OBJS += builtin/bisect--helper.o
+BUILTIN_OBJS += builtin/blame.o
+BUILTIN_OBJS += builtin/branch.o
+BUILTIN_OBJS += builtin/bundle.o
+BUILTIN_OBJS += builtin/cat-file.o
+BUILTIN_OBJS += builtin/check-attr.o
+BUILTIN_OBJS += builtin/check-ref-format.o
+BUILTIN_OBJS += builtin/checkout-index.o
+BUILTIN_OBJS += builtin/checkout.o
+BUILTIN_OBJS += builtin/clean.o
+BUILTIN_OBJS += builtin/clone.o
+BUILTIN_OBJS += builtin/commit-tree.o
+BUILTIN_OBJS += builtin/commit.o
+BUILTIN_OBJS += builtin/config.o
+BUILTIN_OBJS += builtin/count-objects.o
+BUILTIN_OBJS += builtin/describe.o
+BUILTIN_OBJS += builtin/diff-files.o
+BUILTIN_OBJS += builtin/diff-index.o
+BUILTIN_OBJS += builtin/diff-tree.o
+BUILTIN_OBJS += builtin/diff.o
+BUILTIN_OBJS += builtin/fast-export.o
+BUILTIN_OBJS += builtin/fetch-pack.o
+BUILTIN_OBJS += builtin/fetch.o
+BUILTIN_OBJS += builtin/fmt-merge-msg.o
+BUILTIN_OBJS += builtin/for-each-ref.o
+BUILTIN_OBJS += builtin/fsck.o
+BUILTIN_OBJS += builtin/gc.o
+BUILTIN_OBJS += builtin/grep.o
+BUILTIN_OBJS += builtin/hash-object.o
+BUILTIN_OBJS += builtin/help.o
+BUILTIN_OBJS += builtin/index-pack.o
+BUILTIN_OBJS += builtin/init-db.o
+BUILTIN_OBJS += builtin/log.o
+BUILTIN_OBJS += builtin/ls-files.o
+BUILTIN_OBJS += builtin/ls-remote.o
+BUILTIN_OBJS += builtin/ls-tree.o
+BUILTIN_OBJS += builtin/mailinfo.o
+BUILTIN_OBJS += builtin/mailsplit.o
+BUILTIN_OBJS += builtin/merge.o
+BUILTIN_OBJS += builtin/merge-base.o
+BUILTIN_OBJS += builtin/merge-file.o
+BUILTIN_OBJS += builtin/merge-index.o
+BUILTIN_OBJS += builtin/merge-ours.o
+BUILTIN_OBJS += builtin/merge-recursive.o
+BUILTIN_OBJS += builtin/merge-tree.o
+BUILTIN_OBJS += builtin/mktag.o
+BUILTIN_OBJS += builtin/mktree.o
+BUILTIN_OBJS += builtin/mv.o
+BUILTIN_OBJS += builtin/name-rev.o
+BUILTIN_OBJS += builtin/notes.o
+BUILTIN_OBJS += builtin/pack-objects.o
+BUILTIN_OBJS += builtin/pack-redundant.o
+BUILTIN_OBJS += builtin/pack-refs.o
+BUILTIN_OBJS += builtin/patch-id.o
+BUILTIN_OBJS += builtin/prune-packed.o
+BUILTIN_OBJS += builtin/prune.o
+BUILTIN_OBJS += builtin/push.o
+BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/receive-pack.o
+BUILTIN_OBJS += builtin/reflog.o
+BUILTIN_OBJS += builtin/remote.o
+BUILTIN_OBJS += builtin/replace.o
+BUILTIN_OBJS += builtin/rerere.o
+BUILTIN_OBJS += builtin/reset.o
+BUILTIN_OBJS += builtin/rev-list.o
+BUILTIN_OBJS += builtin/rev-parse.o
+BUILTIN_OBJS += builtin/revert.o
+BUILTIN_OBJS += builtin/rm.o
+BUILTIN_OBJS += builtin/send-pack.o
+BUILTIN_OBJS += builtin/shortlog.o
+BUILTIN_OBJS += builtin/show-branch.o
+BUILTIN_OBJS += builtin/show-ref.o
+BUILTIN_OBJS += builtin/stripspace.o
+BUILTIN_OBJS += builtin/symbolic-ref.o
+BUILTIN_OBJS += builtin/tag.o
+BUILTIN_OBJS += builtin/tar-tree.o
+BUILTIN_OBJS += builtin/unpack-file.o
+BUILTIN_OBJS += builtin/unpack-objects.o
+BUILTIN_OBJS += builtin/update-index.o
+BUILTIN_OBJS += builtin/update-ref.o
+BUILTIN_OBJS += builtin/update-server-info.o
+BUILTIN_OBJS += builtin/upload-archive.o
+BUILTIN_OBJS += builtin/var.o
+BUILTIN_OBJS += builtin/verify-pack.o
+BUILTIN_OBJS += builtin/verify-tag.o
+BUILTIN_OBJS += builtin/write-tree.o
 
--include config.mak.autogen
--include config.mak
+GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 
 ifdef CHECK_HEADER_DEPENDENCIES
 COMPUTE_HEADER_DEPENDENCIES =

-- 
1.7.2.3
Jonathan Nieder· Nov 14, 2010, 17:23 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Ævar Arnfjörð Bjarmason wrote:
> Change the Makefile so that the "Platform specific tweaks" section
> comes before the assignments to LIB_H and LIB_OBJS.
Currently the Makefile is structured like this:
	A. default target
	B. basics
	   1. basic configuration section
	      a. -include GIT-VERSION-FILE and recipe to generate it
	      b. uname_S := $(shell uname -s) and similar variables
	      c. user-facing compilation variables: CFLAGS, LDFLAGS, STRIP
	      d. user-facing paths: prefix, bindir_relative, etc
	      e. program names: CC, AR, etc
	   2. basic cflags and ldflags (almost configurable)
	   3. main list of program targets:
	      SCRIPTS, PROGRAMS, TEST_PROGRAMS, BUILT_INS,
	      OTHER_PROGRAMS, BINDIR_PROGRAMS
	   4. defaults for SHELL_PATH, PERL_PATH, PYTHON_PATH
	   5. main list of library targets:
	      LIB_FILE, XDIFF_LIB, LIB_H, LIB_OBJS, BUILTIN_OBJS
	   6. GITLIBS, EXTLIBS for the linker command line
	   7. platform-specific tweaks
	   8. -include config.mak, config.mak.autogen
	C. preparations
	   1. handling of the various NO_THIS_OR_THAT options.
	      This affects BASIC_CFLAGS, COMPAT_CFLAGS,
	      COMPAT_OBJS, PROGRAMS, EXTLIBS, LIB_OBJ, LIB_H, etc
	   2. machinery for non-noisy build
	   3. shell-quoted and C-quoted variables
	   4. ALL_CFLAGS, ALL_LDFLAGS
	D. main build rules
	   1. all:: targets for the main build, subdirs
	   2. shell sanity check
	   3. building the git binary and built-ins
	   4. scripts and gitweb
	   5. autoconf
	   6. building objects:
	      a. %.o: %.c rule, header deps, dependency checking
	      b. target-specific -D flags
	   7. building non-builtins, remote-curl
	   8. libs
	   9. subdirs
	E. GIT-CFLAGS, GIT-BUILD-OPTIONS, GIT-GUI-VARS
	F. bin-wrappers
	G. tests
	H. installation rules
	I. maintainer's dist rules, check-doc, coverage, etc

This patch proposes moving A5 (main list of library targets) after A8 (end of configuration).

Show 9 quoted lines
> In the ab/i18n series I only want to build gettext.o (by adding it to
> LIB_OBJS) if NO_GETTEXT is unset. It's not possible to do that without
> an ugly hack if we haven't applied our platform specific tweaks before
> LIB_{H,OBJS} gets assigned to.
> 
> See <201008140002.40587.j6t@kdbg.org> (subject: "[PATCH] Do not build
> i18n on Windows.") for Johannes's original report, and my follow-up in
> <AANLkTiku5R+idX-C8f0AcCikBLmfEb5ZEhdft+CSRzU0@mail.gmail.com> where I
> suggested that the problem be solved in the manner of this patch.

This doesn't motivate the patch all all to me. Is changing the list of LIB_OBJS in section C1 really an ugly hack? It is where configuration-specific things go and how BLK_SHA1, PROGRAM_OBJS, etc work already.

That said, I can see another reason to move A3 and A5 lower down in the makefile. Namely, they don't seem to have anything obvious to do with configuration. Including the basic list of objects that high up may make the makefile easier to read straight through, but I don't think anyone is reading it straight through.

So I wouldn't have anything against moving both A3 and A5 to right before C1, I just think it needs different motivation.

Sensible? Jonathan

Ævar Arnfjörð Bjarmason· Nov 14, 2010, 17:51 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

On Sun, Nov 14, 2010 at 18:23, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 49 quoted lines
> Ævar Arnfjörð Bjarmason wrote:
>
>> Change the Makefile so that the "Platform specific tweaks" section
>> comes before the assignments to LIB_H and LIB_OBJS.
>
> Currently the Makefile is structured like this:
>
>        A. default target
>        B. basics
>           1. basic configuration section
>              a. -include GIT-VERSION-FILE and recipe to generate it
>              b. uname_S := $(shell uname -s) and similar variables
>              c. user-facing compilation variables: CFLAGS, LDFLAGS, STRIP
>              d. user-facing paths: prefix, bindir_relative, etc
>              e. program names: CC, AR, etc
>           2. basic cflags and ldflags (almost configurable)
>           3. main list of program targets:
>              SCRIPTS, PROGRAMS, TEST_PROGRAMS, BUILT_INS,
>              OTHER_PROGRAMS, BINDIR_PROGRAMS
>           4. defaults for SHELL_PATH, PERL_PATH, PYTHON_PATH
>           5. main list of library targets:
>              LIB_FILE, XDIFF_LIB, LIB_H, LIB_OBJS, BUILTIN_OBJS
>           6. GITLIBS, EXTLIBS for the linker command line
>           7. platform-specific tweaks
>           8. -include config.mak, config.mak.autogen
>        C. preparations
>           1. handling of the various NO_THIS_OR_THAT options.
>              This affects BASIC_CFLAGS, COMPAT_CFLAGS,
>              COMPAT_OBJS, PROGRAMS, EXTLIBS, LIB_OBJ, LIB_H, etc
>           2. machinery for non-noisy build
>           3. shell-quoted and C-quoted variables
>           4. ALL_CFLAGS, ALL_LDFLAGS
>        D. main build rules
>           1. all:: targets for the main build, subdirs
>           2. shell sanity check
>           3. building the git binary and built-ins
>           4. scripts and gitweb
>           5. autoconf
>           6. building objects:
>              a. %.o: %.c rule, header deps, dependency checking
>              b. target-specific -D flags
>           7. building non-builtins, remote-curl
>           8. libs
>           9. subdirs
>        E. GIT-CFLAGS, GIT-BUILD-OPTIONS, GIT-GUI-VARS
>        F. bin-wrappers
>        G. tests
>        H. installation rules
>        I. maintainer's dist rules, check-doc, coverage, etc

That listing should be in a comment at the start of the Makefile. Please submit a patch for that!

> This patch proposes moving A5 (main list of library targets) after
> A8 (end of configuration).
Right. Thanks for the helpful outline.
Show 14 quoted lines
>> In the ab/i18n series I only want to build gettext.o (by adding it to
>> LIB_OBJS) if NO_GETTEXT is unset. It's not possible to do that without
>> an ugly hack if we haven't applied our platform specific tweaks before
>> LIB_{H,OBJS} gets assigned to.
>>
>> See <201008140002.40587.j6t@kdbg.org> (subject: "[PATCH] Do not build
>> i18n on Windows.") for Johannes's original report, and my follow-up in
>> <AANLkTiku5R+idX-C8f0AcCikBLmfEb5ZEhdft+CSRzU0@mail.gmail.com> where I
>> suggested that the problem be solved in the manner of this patch.
>
> This doesn't motivate the patch all all to me.  Is changing the list
> of LIB_OBJS in section C1 really an ugly hack?  It is where
> configuration-specific things go and how BLK_SHA1, PROGRAM_OBJS, etc
> work already.

Yeah, because if it hadn't come before A8 it would have been *not* assigned to in the first place like we do everywhere else. Moving A8 before A5 enables us to do that.

Show 8 quoted lines
> That said, I can see another reason to move A3 and A5 lower down in
> the makefile.  Namely, they don't seem to have anything obvious to
> do with configuration.  Including the basic list of objects that
> high up may make the makefile easier to read straight through, but
> I don't think anyone is reading it straight through.
>
> So I wouldn't have anything against moving both A3 and A5 to right
> before C1, I just think it needs different motivation.

I was thinking about moving A3 & A5 further down. But since I just needed A5 now I only ended up doing that. Moving them both would clean things up though. I think we should do A8 as early as possible.

Jonathan Nieder· Nov 14, 2010, 18:51 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Ævar Arnfjörð Bjarmason wrote:
> That listing should be in a comment at the start of the
> Makefile. Please submit a patch for that!

How about this? An annoying piece of GNU makefile syntax makes this uglier than it needs to be (search for HACK for details).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Show changes to Makefile +80 −15
diff --git a/Makefile b/Makefile
index 1f1ce04..38edd65 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,8 @@
 # The default target of this Makefile is...
 all::
 
+outline:: ; @echo "Configuration."
+
 # Define V=1 to have a more verbose compile.
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
@@ -238,6 +240,12 @@ all::
 #
 # Define NATIVE_CRLF if your platform uses CRLF for line endings.
 
+outline::
+	@echo " Basic configuration section."
+	@echo "  -include GIT-VERSION-FILE and recipe to generate it."
+	@echo "  uname_S and similar variables."
+	@echo "  user-facing compilation variables: CFLAGS, LDFLAGS, STRIP"
+
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
@@ -263,6 +271,7 @@ ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 STRIP ?= strip
 
+outline:: ; @echo "  User-facing paths: prefix, bindir_relative, etc."
 # Among the variables below, these:
 #   gitexecdir
 #   template_dir
@@ -301,6 +310,7 @@ pathsep = :
 
 export prefix bindir sharedir sysconfdir gitwebdir
 
+outline:: ; @echo "  Program names: CC, AR, etc."
 CC = gcc
 AR = ar
 RM = rm -f
@@ -325,6 +335,7 @@ SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powerpc__
 
 ### --- END CONFIGURATION SECTION ---
 
+outline:: ; @echo " Basic cflags and ldflags (almost configurable)"
 # Those must not be GNU-specific; they are shared with perl/ which may
 # be built by a different compiler. (Note that this is an artifact now
 # but it still might be nice to keep that distinction.)
@@ -353,6 +364,11 @@ TEST_PROGRAMS_NEED_X =
 # interactive shell sessions without exporting it.
 unexport CDPATH
 
+outline::
+	@echo " Main list of program targets:"
+	@echo "  SCRIPTS, PROGRAMS, TEST_PROGRAMS, BUILT_INS,"
+	@echo "  OTHER_PROGRAMS, BINDIR_PROGRAMS"
+
 SCRIPT_SH += git-am.sh
 SCRIPT_SH += git-bisect.sh
 SCRIPT_SH += git-difftool--helper.sh
@@ -465,6 +481,10 @@ BINDIR_PROGRAMS_NEED_X += git-shell
 
 BINDIR_PROGRAMS_NO_X += git-cvsserver
 
+outline:: ; @echo " Defaults for SHELL_PATH, PERL_PATH, PYTHON_PATH"
+# HACK: end of outline recipe
+reset =
+
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
 	SHELL_PATH = /bin/sh
@@ -479,6 +499,9 @@ endif
 export PERL_PATH
 export PYTHON_PATH
 
+outline::
+	@echo " Main list of library targets:"
+	@echo "  LIB_FILE, XDIFF_LIB, LIB_H, LIB_OBJS, BUILTIN_OBJS"
 LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a
 VCSSVN_LIB=vcs-svn/lib.a
@@ -758,9 +781,12 @@ GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 EXTLIBS =
 
 #
-# Platform specific tweaks
+outline:: ; @echo " Platform specific tweaks"
 #
 
+# HACK: end of outline recipe
+reset =
+
 # We choose to avoid "if .. else if .. else .. endif endif"
 # because maintaining the nesting to match is a pain.  If
 # we had "elif" things would have been much nicer...
@@ -1147,9 +1173,14 @@ else
 endif
 endif
 
+outline:: ; @echo " -include config.mak and config.mak.autogen"
 -include config.mak.autogen
 -include config.mak
 
+#
+outline:: ; @echo "Preparations"
+#
+
 ifdef CHECK_HEADER_DEPENDENCIES
 COMPUTE_HEADER_DEPENDENCIES =
 USE_COMPUTED_HEADER_DEPENDENCIES =
@@ -1159,6 +1190,12 @@ ifdef COMPUTE_HEADER_DEPENDENCIES
 USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
 endif
 
+outline::
+	@echo " Handling of the various NO_THIS_OR_THAT options"
+	@echo "  This affects BASIC_CFLAGS, COMPAT_CFLAGS,"
+	@echo "  COMPAT_OBJS, PROGRAMS, EXTLIBS, LIB_OBJ, LIB_H, etc"
+reset =
+
 ifdef SANE_TOOL_PATH
 SANE_TOOL_PATH_SQ = $(subst ','\'',$(SANE_TOOL_PATH))
 BROKEN_PATH_FIX = 's|^\# @@BROKEN_PATH_FIX@@$$|git_broken_path_fix $(SANE_TOOL_PATH_SQ)|'
@@ -1491,6 +1528,7 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON=NoThanks
 endif
 
+outline:: ; @echo " Machinery for non-noisy build"
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -1522,7 +1560,8 @@ ifdef ASCIIDOC8
 	export ASCIIDOC8
 endif
 
-# Shell quote (do not use $(call) to accommodate ancient setups);
+outline:: ; @echo " Shell-quoted and C-quoted variables"
+# Do not use $(call) to accommodate ancient setups.
 
 SHA1_HEADER_SQ = $(subst ','\'',$(SHA1_HEADER))
 ETC_GITCONFIG_SQ = $(subst ','\'',$(ETC_GITCONFIG))
@@ -1545,12 +1584,6 @@ PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 
-LIBS = $(GITLIBS) $(EXTLIBS)
-
-BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
-	$(COMPAT_CFLAGS)
-LIB_OBJS += $(COMPAT_OBJS)
-
 # Quote for C
 
 ifdef DEFAULT_EDITOR
@@ -1567,16 +1600,25 @@ DEFAULT_PAGER_CQ_SQ = $(subst ','\'',$(DEFAULT_PAGER_CQ))
 BASIC_CFLAGS += -DDEFAULT_PAGER='$(DEFAULT_PAGER_CQ_SQ)'
 endif
 
+outline:: ; @echo " ALL_CFLAGS, ALL_LDFLAGS"
+LIBS = $(GITLIBS) $(EXTLIBS)
+
+BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \
+	$(COMPAT_CFLAGS)
+LIB_OBJS += $(COMPAT_OBJS)
+
 ALL_CFLAGS += $(BASIC_CFLAGS)
 ALL_LDFLAGS += $(BASIC_LDFLAGS)
 
 export DIFF TAR INSTALL DESTDIR SHELL_PATH
 
-
-### Build rules
+#
+outline:: ; @echo "Main build rules"
+#
 
 SHELL = $(SHELL_PATH)
 
+outline:: ; @echo " all:: targets for the main build, subdirs"
 all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
@@ -1598,11 +1640,13 @@ endif
 please_set_SHELL_PATH_to_a_more_modern_shell:
 	@$$(:)
 
+outline:: ; @echo " Shell sanity check"
 shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
+outline:: ; @echo " Git binary and built-ins"
 git.o: common-cmds.h
 git.s git.o: EXTRA_CPPFLAGS = -DGIT_VERSION='"$(GIT_VERSION)"' \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
@@ -1628,6 +1672,7 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt
 common-cmds.h: $(wildcard Documentation/git-*.txt)
 	$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@
 
+outline:: ; @echo " Scripts and gitweb"
 define cmd_munge_script
 $(RM) $@ $@+ && \
 sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -1743,6 +1788,7 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
 	mv $@+ $@
 endif # NO_PYTHON
 
+outline:: ; @echo " Autoconf"
 configure: configure.ac
 	$(QUIET_GEN)$(RM) $@ $<+ && \
 	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -1756,6 +1802,10 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+outline::
+	@echo " Building objects"
+	@echo "  %.o: %.c rule, header deps, dependency checking"
+
 TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
 GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
 	git.o
@@ -1895,6 +1945,7 @@ $(VCSSVN_OBJS): \
 	vcs-svn/svndump.h
 endif
 
+outline:: ; @echo "  Target-specific -D flags"
 exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1922,6 +1973,7 @@ compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \
 	-DNDEBUG -DOVERRIDE_STRDUP -DREPLACE_SYSTEM_ALLOCATOR
 endif
 
+outline:: ; @echo " Non-builtin programs, remote-curl"
 git-%$X: %.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
@@ -1946,6 +1998,7 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
+outline:: ; @echo " Libraries"
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
 
@@ -1955,6 +2008,7 @@ $(XDIFF_LIB): $(XDIFF_OBJS)
 $(VCSSVN_LIB): $(VCSSVN_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(VCSSVN_OBJS)
 
+outline:: ; @echo " Subdirs"
 doc:
 	$(MAKE) -C Documentation all
 
@@ -1982,6 +2036,10 @@ cscope:
 	$(RM) cscope*
 	$(FIND) . -name '*.[hcS]' -print | xargs cscope -b
 
+#
+outline:: ; @echo "GIT-CFLAGS, GIT-BUILD-OPTIONS, GIT-GUI-VARS"
+#
+
 ### Detect prefix changes
 TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
@@ -2024,6 +2082,9 @@ GIT-GUI-VARS: FORCE
             fi
 endif
 
+#
+outline:: ; @echo "Wrappers for running built git in place"
+#
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -2041,8 +2102,9 @@ bin-wrappers/%: wrap-for-bin.sh
 
 export NO_SVN_TESTS
 
-### Testing rules
-
+#
+outline:: ; @echo "Testing rules"
+#
 test: all
 	$(MAKE) -C t/ all
 
@@ -2083,7 +2145,9 @@ check: common-cmds.h
 remove-dashes:
 	./fixup-builtins $(BUILT_INS) $(PROGRAMS) $(SCRIPTS)
 
-### Installation rules
+#
+outline:: ; @echo "Installation rules"
+#
 
 ifneq ($(filter /%,$(firstword $(template_dir))),)
 template_instdir = $(template_dir)
@@ -2183,8 +2247,9 @@ quick-install-html:
 	$(MAKE) -C Documentation quick-install-html
 
 
-
-### Maintainer's dist rules
+#
+outline:: ; @echo "Maintainer's dist rules, check-doc, coverage testing"
+#
 
 git.spec: git.spec.in
 	sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
Junio C Hamano· Nov 15, 2010, 16:07 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 23 quoted lines
> Ævar Arnfjörð Bjarmason wrote:
>
>> That listing should be in a comment at the start of the
>> Makefile. Please submit a patch for that!
>
> How about this?  An annoying piece of GNU makefile syntax makes this
> uglier than it needs to be (search for HACK for details).
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> diff --git a/Makefile b/Makefile
> index 1f1ce04..38edd65 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,8 @@
>  # The default target of this Makefile is...
>  all::
>  
> +outline:: ; @echo "Configuration."
> +
>  # Define V=1 to have a more verbose compile.
>  #
>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.

Hmm, I don't quite get what you are trying to do here. What good does the output from "make outline" do to a potential/actual developer? You can learn that there are sections for "Testing rules", but then what? You will look for that string in the editor or pager.

Or is this some sort of outline/org-mode hack?
Jonathan Nieder· Nov 15, 2010, 16:11 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

On Mon, Nov 15, 2010 at 08:07:08AM -0800, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Ævar Arnfjörð Bjarmason wrote:
Show 26 quoted lines
>>> That listing should be in a comment at the start of the
>>> Makefile. Please submit a patch for that!
>>
>> How about this?  An annoying piece of GNU makefile syntax makes this
>> uglier than it needs to be (search for HACK for details).
>>
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>> ---
>> diff --git a/Makefile b/Makefile
>> index 1f1ce04..38edd65 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,6 +1,8 @@
>>  # The default target of this Makefile is...
>>  all::
>>  
>> +outline:: ; @echo "Configuration."
>> +
>>  # Define V=1 to have a more verbose compile.
>>  #
>>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
>
> Hmm, I don't quite get what you are trying to do here.  What good does the
> output from "make outline" do to a potential/actual developer?  You can
> learn that there are sections for "Testing rules", but then what?  You
> will look for that string in the editor or pager.

I dunno. I wrote it because Ævar asked for it (and to make visible where the trivial obstacles would lie in getting target-listing "make help" to work, which is more interesting to me).

Ævar Arnfjörð Bjarmason· Nov 15, 2010, 16:16 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

On Mon, Nov 15, 2010 at 17:11, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 34 quoted lines
> On Mon, Nov 15, 2010 at 08:07:08AM -0800, Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>> Ævar Arnfjörð Bjarmason wrote:
>
>>>> That listing should be in a comment at the start of the
>>>> Makefile. Please submit a patch for that!
>>>
>>> How about this?  An annoying piece of GNU makefile syntax makes this
>>> uglier than it needs to be (search for HACK for details).
>>>
>>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>>> ---
>>> diff --git a/Makefile b/Makefile
>>> index 1f1ce04..38edd65 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1,6 +1,8 @@
>>>  # The default target of this Makefile is...
>>>  all::
>>>
>>> +outline:: ; @echo "Configuration."
>>> +
>>>  # Define V=1 to have a more verbose compile.
>>>  #
>>>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
>>
>> Hmm, I don't quite get what you are trying to do here.  What good does the
>> output from "make outline" do to a potential/actual developer?  You can
>> learn that there are sections for "Testing rules", but then what?  You
>> will look for that string in the editor or pager.
>
> I dunno.  I wrote it because Ævar asked for it (and to make visible
> where the trivial obstacles would lie in getting target-listing "make help"
> to work, which is more interesting to me).
I just meant that we should add your comment at the top at some point.
But doing it via something like outline is probably better in the long term.
But let's do it in another patch series :)
Junio C Hamano· Nov 14, 2010, 19:12 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 8 quoted lines
>> This doesn't motivate the patch all all to me.  Is changing the list
>> of LIB_OBJS in section C1 really an ugly hack?  It is where
>> configuration-specific things go and how BLK_SHA1, PROGRAM_OBJS, etc
>> work already.
>
> Yeah, because if it hadn't come before A8 it would have been *not*
> assigned to in the first place like we do everywhere else. Moving A8
> before A5 enables us to do that.

Hmm, in the existing platform specific tweaks section, we set BLK_SHA1 based on $(uname_S) and that symbol is used to control what is added to LIB_H and LIB_OBJS. Why is your gettext.o any different?

Jonathan Nieder· Nov 14, 2010, 17:53 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Jonathan Nieder wrote:
> Ævar Arnfjörð Bjarmason wrote:
Show 11 quoted lines
>> In the ab/i18n series I only want to build gettext.o (by adding it to
>> LIB_OBJS) if NO_GETTEXT is unset. It's not possible to do that without
>> an ugly hack if we haven't applied our platform specific tweaks before
>> LIB_{H,OBJS} gets assigned to.
>> 
>> See <201008140002.40587.j6t@kdbg.org> (subject: "[PATCH] Do not build
>> i18n on Windows.") for Johannes's original report, and my follow-up in
>> <AANLkTiku5R+idX-C8f0AcCikBLmfEb5ZEhdft+CSRzU0@mail.gmail.com> where I
>> suggested that the problem be solved in the manner of this patch.
>
> This doesn't motivate the patch all all to me.

Erm, sorry for the gibberish. For "all all", please read "at all" or "clearly enough".

Jonathan who ought to slow down his replies; please feel free to point out when I send nonsense like this so I can see when the rate-limiting is working :)

Junio C Hamano· Nov 14, 2010, 19:08 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 7 quoted lines
> Change the Makefile so that the "Platform specific tweaks" section
> comes before the assignments to LIB_H and LIB_OBJS.
>
> In the ab/i18n series I only want to build gettext.o (by adding it to
> LIB_OBJS) if NO_GETTEXT is unset. It's not possible to do that without
> an ugly hack if we haven't applied our platform specific tweaks before
> LIB_{H,OBJS} gets assigned to.

Isn't this similar to block-sha1, which is controlled by the feature macro BLK_SHA1 that can be inferred from platform tweak? The mechanism is not a hack but is a way LIB_H and LIB_OBJS are intended to be used, so there must be something missing in your explanation.

Ævar Arnfjörð Bjarmason· Nov 14, 2010, 14:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 2/4] t7004-tag.sh: re-arrange git tag comment for clarity

Split the "message in editor has initial comment" test into three tests. The motivation is to be able to only skip the middle part under NO_GETTEXT_POISON.

In addition the return value of 'git tag' was being returned. We now check that it's non-zero. I used ! instead of test_must_fail so that the GIT_EDITOR variable was only used in this command invocation, and because the surrounding tests use this style.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t7004-tag.sh |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)
Show changes to t/t7004-tag.sh +15 −5
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index ac943f5..700b556 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1051,13 +1051,23 @@ test_expect_success \
 
 test_expect_success \
 	'message in editor has initial comment' '
-	GIT_EDITOR=cat git tag -a initial-comment > actual
+	! (GIT_EDITOR=cat git tag -a initial-comment > actual)
+'
+
+test_expect_success \
+	'message in editor has initial comment: first line' '
 	# check the first line --- should be empty
-	first=$(sed -e 1q <actual) &&
-	test -z "$first" &&
+	echo >first.expect &&
+	sed -e 1q <actual >first.actual &&
+	test_cmp first.expect first.actual
+'
+
+test_expect_success \
+	'message in editor has initial comment: remainder' '
 	# remove commented lines from the remainder -- should be empty
-	rest=$(sed -e 1d -e '/^#/d' <actual) &&
-	test -z "$rest"
+	>rest.expect
+	sed -e 1d -e '/^#/d' <actual >rest.actual &&
+	test_cmp rest.expect rest.actual
 '
 
 get_tag_header reuse $commit commit $time >expect

-- 
1.7.2.3
Jonathan Nieder· Nov 14, 2010, 17:32 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 2/4] t7004-tag.sh: re-arrange git tag comment for clarity

Ævar Arnfjörð Bjarmason wrote:
>                           I used ! instead of test_must_fail so that
> the GIT_EDITOR variable was only used in this command invocation,
	(
		GIT_EDITOR=cat &&
		export GIT_EDITOR &&
		test_must_fail ...
	)
?
>                                                                   and
> because the surrounding tests use this style.

Oh, okay. I had the test_must_fail version sitting in my local tree as part of the unsent part of the &&-chaining series http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160419

I am happy to see this fixed sooner.
[...]
Show 19 quoted lines
> +test_expect_success \
> +	'message in editor has initial comment: first line' '
>  	# check the first line --- should be empty
> -	first=$(sed -e 1q <actual) &&
> -	test -z "$first" &&
> +	echo >first.expect &&
> +	sed -e 1q <actual >first.actual &&
> +	test_cmp first.expect first.actual
> +'
> +
> +test_expect_success \
> +	'message in editor has initial comment: remainder' '
>  	# remove commented lines from the remainder -- should be empty
> -	rest=$(sed -e 1d -e '/^#/d' <actual) &&
> -	test -z "$rest"
> +	>rest.expect
> +	sed -e 1d -e '/^#/d' <actual >rest.actual &&
> +	test_cmp rest.expect rest.actual
>  '

Nice. Note that you've changed the semantics of the test here (probably for the better): the $() backquoting operator strips trailing newlines, while your sed expression does not.

I assume you've run the tests and made sure they still pass.
For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Ævar Arnfjörð Bjarmason· Nov 14, 2010, 17:57 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/4] t7004-tag.sh: re-arrange git tag comment for clarity

On Sun, Nov 14, 2010 at 18:32, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 21 quoted lines
> Ævar Arnfjörð Bjarmason wrote:
>
>>                           I used ! instead of test_must_fail so that
>> the GIT_EDITOR variable was only used in this command invocation,
>
>        (
>                GIT_EDITOR=cat &&
>                export GIT_EDITOR &&
>                test_must_fail ...
>        )
>
> ?
>
>>                                                                   and
>> because the surrounding tests use this style.
>
> Oh, okay.  I had the test_must_fail version sitting in my local tree
> as part of the unsent part of the &&-chaining series
> http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160419
>
> I am happy to see this fixed sooner.

Better to fix it up later IMO in something like your series. One surgery at a time.

Show 26 quoted lines
> [...]
>> +test_expect_success \
>> +     'message in editor has initial comment: first line' '
>>       # check the first line --- should be empty
>> -     first=$(sed -e 1q <actual) &&
>> -     test -z "$first" &&
>> +     echo >first.expect &&
>> +     sed -e 1q <actual >first.actual &&
>> +     test_cmp first.expect first.actual
>> +'
>> +
>> +test_expect_success \
>> +     'message in editor has initial comment: remainder' '
>>       # remove commented lines from the remainder -- should be empty
>> -     rest=$(sed -e 1d -e '/^#/d' <actual) &&
>> -     test -z "$rest"
>> +     >rest.expect
>> +     sed -e 1d -e '/^#/d' <actual >rest.actual &&
>> +     test_cmp rest.expect rest.actual
>>  '
>
> Nice.  Note that you've changed the semantics of the test here
> (probably for the better): the $() backquoting operator strips
> trailing newlines, while your sed expression does not.
>
> I assume you've run the tests and made sure they still pass.

Yeah, I've been running all these (also the others in the series) for months on several different platforms with different options. They all work.

> For what it's worth,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
Ævar Arnfjörð Bjarmason· Nov 14, 2010, 14:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 3/4] tests: use test_cmp instead of piping to diff(1)

Change submodule tests that piped to diff(1) to use test_cmp. The resulting unified diff is easier to read.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t4041-diff-submodule-option.sh |   96 +++++++++++++++++++++++++-------------
 t/t7401-submodule-summary.sh     |   47 ++++++++++++------
 2 files changed, 95 insertions(+), 48 deletions(-)
Show changes to 2 files +95 −48

t/t4041-diff-submodule-option.sh, t/t7401-submodule-summary.sh

diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 995bdfa..bf9a752 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -37,9 +37,10 @@ head1=$(add_file sm1 foo1 foo2)
 test_expect_success 'added submodule' "
 	git add sm1 &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 0000000...$head1 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm1 &&
@@ -47,33 +48,36 @@ head2=$(add_file sm1 foo3)
 
 test_expect_success 'modified submodule(forward)' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head1..$head2:
   > Add foo3
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule(forward)' "
 	git diff --submodule=log >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head1..$head2:
   > Add foo3
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule(forward) --submodule' "
 	git diff --submodule >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head1..$head2:
   > Add foo3
 EOF
+	test_cmp expected actual
 "
 
 fullhead1=$(cd sm1; git rev-list --max-count=1 $head1)
 fullhead2=$(cd sm1; git rev-list --max-count=1 $head2)
 test_expect_success 'modified submodule(forward) --submodule=short' "
 	git diff --submodule=short >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 diff --git a/sm1 b/sm1
 index $head1..$head2 160000
 --- a/sm1
@@ -82,6 +86,7 @@ index $head1..$head2 160000
 -Subproject commit $fullhead1
 +Subproject commit $fullhead2
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm1 &&
@@ -93,24 +98,26 @@ head3=$(
 
 test_expect_success 'modified submodule(backward)' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head2..$head3 (rewind):
   < Add foo3
   < Add foo2
 EOF
+	test_cmp expected actual
 "
 
 head4=$(add_file sm1 foo4 foo5) &&
 head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD)
 test_expect_success 'modified submodule(backward and forward)' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head2...$head4:
   > Add foo5
   > Add foo4
   < Add foo3
   < Add foo2
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm1 &&
@@ -123,7 +130,7 @@ mv sm1-bak sm1
 
 test_expect_success 'typechanged submodule(submodule->blob), --cached' "
 	git diff --submodule=log --cached >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 41fbea9...0000000 (submodule deleted)
 diff --git a/sm1 b/sm1
 new file mode 100644
@@ -133,11 +140,12 @@ index 0000000..9da5fb8
 @@ -0,0 +1 @@
 +sm1
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'typechanged submodule(submodule->blob)' "
 	git diff --submodule=log >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 diff --git a/sm1 b/sm1
 deleted file mode 100644
 index 9da5fb8..0000000
@@ -147,13 +155,14 @@ index 9da5fb8..0000000
 -sm1
 Submodule sm1 0000000...$head4 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 rm -rf sm1 &&
 git checkout-index sm1
 test_expect_success 'typechanged submodule(submodule->blob)' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head4...0000000 (submodule deleted)
 diff --git a/sm1 b/sm1
 new file mode 100644
@@ -163,6 +172,7 @@ index 0000000..$head5
 @@ -0,0 +1 @@
 +sm1
 EOF
+	test_cmp expected actual
 "
 
 rm -f sm1 &&
@@ -171,15 +181,16 @@ head6=$(add_file sm1 foo6 foo7)
 fullhead6=$(cd sm1; git rev-list --max-count=1 $head6)
 test_expect_success 'nonexistent commit' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head4...$head6 (commits not present)
 EOF
+	test_cmp expected actual
 "
 
 commit_file
 test_expect_success 'typechanged submodule(blob->submodule)' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 diff --git a/sm1 b/sm1
 deleted file mode 100644
 index $head5..0000000
@@ -189,21 +200,24 @@ index $head5..0000000
 -sm1
 Submodule sm1 0000000...$head6 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm1 &&
 test_expect_success 'submodule is up to date' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'submodule contains untracked content' "
 	echo new > sm1/new-file &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains untracked content
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'submodule contains untracked content (untracked ignored)' "
@@ -224,18 +238,20 @@ test_expect_success 'submodule contains untracked content (all ignored)' "
 test_expect_success 'submodule contains untracked and modifed content' "
 	echo new > sm1/foo6 &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains untracked content
 Submodule sm1 contains modified content
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'submodule contains untracked and modifed content (untracked ignored)' "
 	echo new > sm1/foo6 &&
 	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains modified content
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'submodule contains untracked and modifed content (dirty ignored)' "
@@ -253,45 +269,50 @@ test_expect_success 'submodule contains untracked and modifed content (all ignor
 test_expect_success 'submodule contains modifed content' "
 	rm -f sm1/new-file &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains modified content
 EOF
+	test_cmp expected actual
 "
 
 (cd sm1; git commit -mchange foo6 >/dev/null) &&
 head8=$(cd sm1; git rev-parse --verify HEAD | cut -c1-7) &&
 test_expect_success 'submodule is modified' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked content' "
 	echo new > sm1/new-file &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains untracked content
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked content (untracked ignored)' "
 	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked content (dirty ignored)' "
 	git diff-index -p --ignore-submodules=dirty --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked content (all ignored)' "
@@ -302,31 +323,34 @@ test_expect_success 'modified submodule contains untracked content (all ignored)
 test_expect_success 'modified submodule contains untracked and modifed content' "
 	echo modification >> sm1/foo6 &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains untracked content
 Submodule sm1 contains modified content
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked and modifed content (untracked ignored)' "
 	echo modification >> sm1/foo6 &&
 	git diff-index -p --ignore-submodules=untracked --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains modified content
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked and modifed content (dirty ignored)' "
 	echo modification >> sm1/foo6 &&
 	git diff-index -p --ignore-submodules=dirty --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule contains untracked and modifed content (all ignored)' "
@@ -338,19 +362,21 @@ test_expect_success 'modified submodule contains untracked and modifed content (
 test_expect_success 'modified submodule contains modifed content' "
 	rm -f sm1/new-file &&
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 contains modified content
 Submodule sm1 $head6..$head8:
   > change
 EOF
+	test_cmp expected actual
 "
 
 rm -rf sm1
 test_expect_success 'deleted submodule' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6...0000000 (submodule deleted)
 EOF
+	test_cmp expected actual
 "
 
 test_create_repo sm2 &&
@@ -359,41 +385,45 @@ git add sm2
 
 test_expect_success 'multiple submodules' "
 	git diff-index -p --submodule=log HEAD >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6...0000000 (submodule deleted)
 Submodule sm2 0000000...$head7 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'path filter' "
 	git diff-index -p --submodule=log HEAD sm2 >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm2 0000000...$head7 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm2
 test_expect_success 'given commit' "
 	git diff-index -p --submodule=log HEAD^ >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6...0000000 (submodule deleted)
 Submodule sm2 0000000...$head7 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'given commit --submodule' "
 	git diff-index -p --submodule HEAD^ >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6...0000000 (submodule deleted)
 Submodule sm2 0000000...$head7 (new submodule)
 EOF
+	test_cmp expected actual
 "
 
 fullhead7=$(cd sm2; git rev-list --max-count=1 $head7)
 
 test_expect_success 'given commit --submodule=short' "
 	git diff-index -p --submodule=short HEAD^ >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 diff --git a/sm1 b/sm1
 deleted file mode 160000
 index $head6..0000000
@@ -409,6 +439,7 @@ index 0000000..$head7
 @@ -0,0 +1 @@
 +Subproject commit $fullhead7
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'setup .git file for sm2' '
@@ -420,10 +451,11 @@ test_expect_success 'setup .git file for sm2' '
 
 test_expect_success 'diff --submodule with .git file' '
 	git diff --submodule HEAD^ >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 Submodule sm1 $head6...0000000 (submodule deleted)
 Submodule sm2 0000000...$head7 (new submodule)
 EOF
+	test_cmp expected actual
 '
 
 test_done
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 2945844..9963f29 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -37,11 +37,12 @@ head1=$(add_file sm1 foo1 foo2)
 test_expect_success 'added submodule' "
 	git add sm1 &&
 	git submodule summary >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 * sm1 0000000...$head1 (2):
   > Add foo2
 
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm1 &&
@@ -49,20 +50,22 @@ head2=$(add_file sm1 foo3)
 
 test_expect_success 'modified submodule(forward)' "
 	git submodule summary >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 * sm1 $head1...$head2 (1):
   > Add foo3
 
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success 'modified submodule(forward), --files' "
 	git submodule summary --files >actual &&
-	diff actual - <<-EOF
+	cat >expected <<-EOF &&
 * sm1 $head1...$head2 (1):
   > Add foo3
 
 EOF
+	test_cmp expected actual
 "
 
 commit_file sm1 &&
@@ -74,19 +77,20 @@ head3=$(
 
 test_expect_success 'modified submodule(backward)' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head2...$head3 (2):
   < Add foo3
   < Add foo2
 
 EOF
+	test_cmp expected actual
 "
 
 head4=$(add_file sm1 foo4 foo5) &&
 head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD)
 test_expect_success 'modified submodule(backward and forward)' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head2...$head4 (4):
   > Add foo5
   > Add foo4
@@ -94,17 +98,19 @@ test_expect_success 'modified submodule(backward and forward)' "
   < Add foo2
 
 EOF
+	test_cmp expected actual
 "
 
 test_expect_success '--summary-limit' "
     git submodule summary -n 3 >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head2...$head4 (4):
   > Add foo5
   > Add foo4
   < Add foo3
 
 EOF
+    test_cmp expected actual
 "
 
 commit_file sm1 &&
@@ -117,30 +123,33 @@ mv sm1-bak sm1
 
 test_expect_success 'typechanged submodule(submodule->blob), --cached' "
     git submodule summary --cached >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head4(submodule)->$head5(blob) (3):
   < Add foo5
 
 EOF
+	test_cmp actual expected
 "
 
 test_expect_success 'typechanged submodule(submodule->blob), --files' "
     git submodule summary --files >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head5(blob)->$head4(submodule) (3):
   > Add foo5
 
 EOF
+    test_cmp actual expected
 "
 
 rm -rf sm1 &&
 git checkout-index sm1
 test_expect_success 'typechanged submodule(submodule->blob)' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head4(submodule)->$head5(blob):
 
 EOF
+    test_cmp actual expected
 "
 
 rm -f sm1 &&
@@ -148,31 +157,34 @@ test_create_repo sm1 &&
 head6=$(add_file sm1 foo6 foo7)
 test_expect_success 'nonexistent commit' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head4...$head6:
   Warn: sm1 doesn't contain commit $head4_full
 
 EOF
+    test_cmp actual expected
 "
 
 commit_file
 test_expect_success 'typechanged submodule(blob->submodule)' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head5(blob)->$head6(submodule) (2):
   > Add foo7
 
 EOF
+    test_cmp expected actual
 "
 
 commit_file sm1 &&
 rm -rf sm1
 test_expect_success 'deleted submodule' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head6...0000000:
 
 EOF
+    test_cmp expected actual
 "
 
 test_create_repo sm2 &&
@@ -181,39 +193,42 @@ git add sm2
 
 test_expect_success 'multiple submodules' "
     git submodule summary >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head6...0000000:
 
 * sm2 0000000...$head7 (2):
   > Add foo9
 
 EOF
+    test_cmp expected actual
 "
 
 test_expect_success 'path filter' "
     git submodule summary sm2 >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm2 0000000...$head7 (2):
   > Add foo9
 
 EOF
+    test_cmp expected actual
 "
 
 commit_file sm2
 test_expect_success 'given commit' "
     git submodule summary HEAD^ >actual &&
-    diff actual - <<-EOF
+    cat >expected <<-EOF &&
 * sm1 $head6...0000000:
 
 * sm2 0000000...$head7 (2):
   > Add foo9
 
 EOF
+    test_cmp expected actual
 "
 
 test_expect_success '--for-status' "
     git submodule summary --for-status HEAD^ >actual &&
-    test_cmp actual - <<EOF
+	test_cmp actual - <<EOF
 # Submodule changes to be committed:
 #
 # * sm1 $head6...0000000:

-- 
1.7.2.3
Jonathan Nieder· Nov 14, 2010, 17:41 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 3/4] tests: use test_cmp instead of piping to diff(1)

Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -37,9 +37,10 @@ head1=$(add_file sm1 foo1 foo2)
>  test_expect_success 'added submodule' "
>  	git add sm1 &&
>  	git diff-index -p --submodule=log HEAD >actual &&
> -	diff actual - <<-EOF
> +	cat >expected <<-EOF &&
>  Submodule sm1 0000000...$head1 (new submodule)
>  EOF
> +	test_cmp expected actual
>  "

Nice. Reminds me of http://thread.gmane.org/gmane.comp.version-control.git/160168/focus=160222 (use of "cmp - foo.expect" in git svn tests); anyone want to take a stab at fixing that, too?

> --- a/t/t7401-submodule-summary.sh
> +++ b/t/t7401-submodule-summary.sh
[...]
>  test_expect_success '--for-status' "
>      git submodule summary --for-status HEAD^ >actual &&
> -    test_cmp actual - <<EOF
> +	test_cmp actual - <<EOF
I think this tab should be spaces?  Also, shouldn't this be
	cat >expect <<EOF &&
	...
	EOF
	test_cmp expect actual
to match the others?  There are a few similar cases in the patch.
I assume you've checked the patched tests still pass.  So with the exception
just mentioned,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
Ævar Arnfjörð Bjarmason· Nov 14, 2010, 14:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 4/4] builtin: use builtin.h for all builtin commands

Change the builtin/*.c files and wt-status.c files which weren't using builtin.h to use it, also remove any git-compat-util.h, strbuf.h and cache.h from those files. We can trust on builtin.h including them.

builtin.h also includes commit.h and notes.h. But I haven't removed these includes. They're redundant now, but we may want to remove them from builtin.h in the future.

This impeded my efforts to gettextize git, since I'd otherwise have to add gettext.h to all of these. Using builtin.h is a good idea in any case, since it's defining the prototypes for the cmd_* functions that these files define.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/clone.c           |    3 +--
 builtin/fetch-pack.c      |    2 +-
 builtin/hash-object.c     |    2 +-
 builtin/index-pack.c      |    2 +-
 builtin/merge-index.c     |    2 +-
 builtin/merge-recursive.c |    2 +-
 builtin/merge-tree.c      |    2 +-
 builtin/mktag.c           |    2 +-
 builtin/pack-redundant.c  |    2 +-
 builtin/pack-refs.c       |    2 +-
 builtin/patch-id.c        |    2 +-
 builtin/receive-pack.c    |    2 +-
 builtin/remote.c          |    3 +--
 builtin/reset.c           |    2 +-
 builtin/send-pack.c       |    2 +-
 builtin/unpack-file.c     |    2 +-
 builtin/var.c             |    2 +-
 wt-status.c               |    2 +-
 18 files changed, 18 insertions(+), 20 deletions(-)
Show changes to 18 files +18 −20

builtin/clone.c, builtin/fetch-pack.c, builtin/hash-object.c, builtin/index-pack.c, builtin/merge-index.c, builtin/merge-recursive.c, builtin/merge-tree.c, builtin/mktag.c, builtin/pack-redundant.c, builtin/pack-refs.c, builtin/patch-id.c, builtin/receive-pack.c, builtin/remote.c, builtin/reset.c, builtin/send-pack.c, builtin/unpack-file.c, builtin/var.c, wt-status.c

diff --git a/builtin/clone.c b/builtin/clone.c
index 19ed640..994d527 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -8,7 +8,7 @@
  * Clone a repository into a different directory that does not yet exist.
  */
 
-#include "cache.h"
+#include "builtin.h"
 #include "parse-options.h"
 #include "fetch-pack.h"
 #include "refs.h"
@@ -16,7 +16,6 @@
 #include "tree-walk.h"
 #include "unpack-trees.h"
 #include "transport.h"
-#include "strbuf.h"
 #include "dir.h"
 #include "pack-refs.h"
 #include "sigchain.h"
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index dbd8b7b..fd70b4c 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "refs.h"
 #include "pkt-line.h"
 #include "commit.h"
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 080af1a..8a5670f 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -4,7 +4,7 @@
  * Copyright (C) Linus Torvalds, 2005
  * Copyright (C) Junio C Hamano, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "blob.h"
 #include "quote.h"
 #include "parse-options.h"
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 8dc5c0b..8d1af06 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "delta.h"
 #include "pack.h"
 #include "csum-file.h"
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 2c4cf5e..adc2a6d 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "run-command.h"
 #include "exec_cmd.h"
 
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index c33091b..3a64f5d 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "commit.h"
 #include "tag.h"
 #include "merge-recursive.h"
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 9b25ddc..1991742 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "tree-walk.h"
 #include "xdiff-interface.h"
 #include "blob.h"
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 1cb0f3f..9148cc0 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "tag.h"
 #include "exec_cmd.h"
 
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 41e1615..a15e366 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -6,7 +6,7 @@
 *
 */
 
-#include "cache.h"
+#include "builtin.h"
 #include "exec_cmd.h"
 
 #define BLKSIZE 512
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 091860b..39a9d89 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "parse-options.h"
 #include "pack-refs.h"
 
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 5125300..33e9725 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "exec_cmd.h"
 
 static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 760817d..f64b79c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "pack.h"
 #include "refs.h"
 #include "pkt-line.h"
diff --git a/builtin/remote.c b/builtin/remote.c
index e9a6e09..37c3595 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1,9 +1,8 @@
-#include "cache.h"
+#include "builtin.h"
 #include "parse-options.h"
 #include "transport.h"
 #include "remote.h"
 #include "string-list.h"
-#include "strbuf.h"
 #include "run-command.h"
 #include "refs.h"
 
diff --git a/builtin/reset.c b/builtin/reset.c
index 0037be4..c198b8e 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -7,7 +7,7 @@
  *
  * Copyright (c) 2005, 2006 Linus Torvalds and Junio C Hamano
  */
-#include "cache.h"
+#include "builtin.h"
 #include "tag.h"
 #include "object.h"
 #include "commit.h"
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 481602d..a55a517 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "commit.h"
 #include "refs.h"
 #include "pkt-line.h"
diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c
index 608590a..c905d80 100644
--- a/builtin/unpack-file.c
+++ b/builtin/unpack-file.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "blob.h"
 #include "exec_cmd.h"
 
diff --git a/builtin/var.c b/builtin/var.c
index 0744bb8..1787771 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -3,7 +3,7 @@
  *
  * Copyright (C) Eric Biederman, 2005
  */
-#include "cache.h"
+#include "builtin.h"
 #include "exec_cmd.h"
 
 static const char var_usage[] = "git var (-l | <variable>)";
diff --git a/wt-status.c b/wt-status.c
index fc2438f..bfc97fb 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "builtin.h"
 #include "wt-status.h"
 #include "object.h"
 #include "dir.h"

-- 
1.7.2.3
Sverre Rabbelier· Nov 14, 2010, 15:01 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

Heya,
On Sun, Nov 14, 2010 at 15:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 7 quoted lines
> diff --git a/wt-status.c b/wt-status.c
> index fc2438f..bfc97fb 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1,4 +1,4 @@
> -#include "cache.h"
> +#include "builtin.h"

Doesn't this suggest wt-status.c should be moved to builtin? Or something like that.

-- 
Cheers,

Sverre Rabbelier
Ævar Arnfjörð Bjarmason· Nov 14, 2010, 15:11 UTC · re: Sverre Rabbelier · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

On Sun, Nov 14, 2010 at 16:01, Sverre Rabbelier <srabbelier@gmail.com> wrote:
Show 11 quoted lines
> On Sun, Nov 14, 2010 at 15:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> diff --git a/wt-status.c b/wt-status.c
>> index fc2438f..bfc97fb 100644
>> --- a/wt-status.c
>> +++ b/wt-status.c
>> @@ -1,4 +1,4 @@
>> -#include "cache.h"
>> +#include "builtin.h"
>
> Doesn't this suggest wt-status.c should be moved to builtin? Or
> something like that.

It just suggests that the header was useful. But maybe it should be moved to buildin/status.c.

Junio C Hamano· Nov 14, 2010, 18:55 UTC · re: Sverre Rabbelier · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

Sverre Rabbelier <srabbelier@gmail.com> writes:
Show 11 quoted lines
> On Sun, Nov 14, 2010 at 15:44, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> diff --git a/wt-status.c b/wt-status.c
>> index fc2438f..bfc97fb 100644
>> --- a/wt-status.c
>> +++ b/wt-status.c
>> @@ -1,4 +1,4 @@
>> -#include "cache.h"
>> +#include "builtin.h"
>
> Doesn't this suggest wt-status.c should be moved to builtin? Or
> something like that.

Why? builtin.h is to declare cmd_foo() and to be included by files that implement cmd_foo() (i.e. builtin/*.c) and the file that needs to see cmd_foo() declarations (i.e. git.c).

wt-status.c is about giving common service routines for status related commands in builtin/commit.c to implement both commit and status. Its services may have gained callers outside builtin/cocmmit.c, but that is not a good reason to export its services via builtin.h nor for it to include builtin.h.

This is a tangent but I think for the same reason the types and functions related to notes-rewrite currently in builtin.h do not belong there; they should be moved to notes.h or somewhere.

Jonathan Nieder· Nov 14, 2010, 17:47 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

Ævar Arnfjörð Bjarmason wrote:
> This impeded my efforts to gettextize git, since I'd otherwise have to
> add gettext.h to all of these. Using builtin.h is a good idea in any
> case, since it's defining the prototypes for the cmd_* functions that
> these files define.
Yes, I think Junio acked this before, though I can't it in the archive.
Also compare: http://thread.gmane.org/gmane.comp.version-control.git/49523
Show 8 quoted lines
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1,4 +1,4 @@
> -#include "cache.h"
> +#include "builtin.h"
>  #include "wt-status.h"
>  #include "object.h"
>  #include "dir.h"

IMHO this deserves a separate mention in the log message. If it is just to get gettext.h, why not include gettext.h directly?

Junio C Hamano· Nov 17, 2010, 19:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> Change the builtin/*.c files and wt-status.c files which weren't using
> builtin.h to use it, also remove any git-compat-util.h, strbuf.h and
> cache.h from those files. We can trust on builtin.h including them.
As I already said, buitlin/*.c is fine, but wt-status.c is very dubious.
> This impeded my efforts to gettextize git, since I'd otherwise have to
> add gettext.h to all of these. Using builtin.h is a good idea in any
> case, since it's defining the prototypes for the cmd_* functions that
> these files define.

What about non-builtin commands? Don't they need to include gettext.h anyway?

I would have expected that gettext.h would be included from cache.h, so that all gitty commands, whether they are builtin or standalone, have access to the symbols defined in it.

Ævar Arnfjörð Bjarmason· Nov 17, 2010, 20:08 UTC · re: Junio C Hamano · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

On Wed, Nov 17, 2010 at 20:54, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Change the builtin/*.c files and wt-status.c files which weren't using
>> builtin.h to use it, also remove any git-compat-util.h, strbuf.h and
>> cache.h from those files. We can trust on builtin.h including them.
>
> As I already said, buitlin/*.c is fine, but wt-status.c is very dubious.

So I should resend this patch with `#include "gettext.h"` in wt-status.c and the rest unchanged, along with a new commit message?

Show 7 quoted lines
>> This impeded my efforts to gettextize git, since I'd otherwise have to
>> add gettext.h to all of these. Using builtin.h is a good idea in any
>> case, since it's defining the prototypes for the cmd_* functions that
>> these files define.
>
> What about non-builtin commands?  Don't they need to include gettext.h
> anyway?

In my patch to add gettext infrastructure I add gettext.h to imap-send.c and friends which have their own main.

> I would have expected that gettext.h would be included from cache.h, so
> that all gitty commands, whether they are builtin or standalone, have
> access to the symbols defined in it.

I can do that too. I just got the impression previously that we wanted to only include things in headers that made sense according to their name. Gettext has nothing to do with the cache, and cache.h itself doesn't need it.

But if we want to stick it in there that's fine by me, I just care about the ends, not the means. So just say which way you want it and I can cook up a patch.

Jonathan Nieder· Nov 17, 2010, 20:15 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 4/4] builtin: use builtin.h for all builtin commands

Ævar Arnfjörð Bjarmason wrote:
> I can do that too. I just got the impression previously that we wanted
> to only include things in headers that made sense according to their
> name. Gettext has nothing to do with the cache, and cache.h itself
> doesn't need it.

A little bit of history: git was once called dircache. cache.h is practically speaking git.h, which is why it declares:

 - get_git_dir
 - ALLOC_GROW
 - zlib_compression_level
 - safe_crlf
 - mksnpath
 - git_pathdup
 - hashclr
 - validate_headref
 - peel_to_type
 - approxidate_careful
 - packed_git
 - path_match
 - fsync_or_die
 - setup_pager
 - checkout_fast_forward

to list a random assortment. See "A birds-eye view of Git's source code" in the user manual for more on this.

Jonathan Nieder· Nov 14, 2010, 17:50 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 0/4] [PULL] ab/i18n-prereqs, prerequisites for ab/i18n

Ævar Arnfjörð Bjarmason wrote:
Show 6 quoted lines
> These are the first four patches in the ab/i18n series which aren't
> strictly related to it, but merely fixes to issues I encountered.
> 
> I'm submitting these separately now in the hope that they'll make it
> to the next & master branches before ab/i18n. Having these separately
> reviewed should be easier for everyone.

Thank you! Hopefully others can pitch in and extract more patches like this from ab/i18n.

I've sent comments on all four patches. I agree with the spirit of all of them and would be happy to see them tweaked and integrated soon (patch 2 does not even seem to need tweaking).

← back to recent threads