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

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

[Ævar Arnfjörð Bjarmason]

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

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Jonathan Nieder]

Æ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).

> 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

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

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 14, 2010 at 18:23, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Æ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.

>> 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.

> 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.

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

[Jonathan Nieder]

Æ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.
@@ -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' < $< > $@+

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

[Junio C Hamano]

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.

Or is this some sort of outline/org-mode hack?

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

[Jonathan Nieder]

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).

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

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 15, 2010 at 17:11, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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 :)

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> 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?

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

[Jonathan Nieder]

Jonathan Nieder wrote:
> Ævar Arnfjörð Bjarmason wrote:

>> 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 :)

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

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> 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.

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Jonathan Nieder]

Æ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.

[...]
> +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>

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

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 14, 2010 at 18:32, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Æ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.

> [...]
>> +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.

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> --- 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.

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

[Ævar Arnfjörð Bjarmason]

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(-)

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

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

[Sverre Rabbelier]

Heya,

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.

-- 
Cheers,

Sverre Rabbelier

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

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 14, 2010 at 16:01, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> 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.

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

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> 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.

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

[Jonathan Nieder]

Æ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

> --- 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?

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

[Junio C Hamano]

Æ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.

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

[Ævar Arnfjörð Bjarmason]

On Wed, Nov 17, 2010 at 20:54, Junio C Hamano <gitster@pobox.com> wrote:
> Æ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?

>> 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.

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

[Jonathan Nieder]

Æ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.

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

[Jonathan Nieder]

Ævar Arnfjörð Bjarmason wrote:

> 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).