Git development
 help / color / mirror / Atom feed
* [PATCH 05/12] Makefile: drop dependency on $(wildcard */*.h)
From: Jonathan Nieder @ 2010-01-26 15:46 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

The files this pulls in are already pulled in by other dependency
rules (some recently added).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 593801a..98810b2 100644
--- a/Makefile
+++ b/Makefile
@@ -1655,7 +1655,7 @@ git-remote-curl$X: remote-curl.o http.o http-walker.o $(GITLIBS)
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H) $(wildcard */*.h)
+$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
 builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
 builtin-bundle.o bundle.o transport.o: bundle.h
 builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
-- 
1.6.6

^ permalink raw reply related

* [PATCH 06/12] Makefile: transport.o depends on branch.h now
From: Jonathan Nieder @ 2010-01-26 15:47 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Since commit e9fcd1e2 (Add push --set-upstream, 2010-01-16),
transport.c uses branch.h.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 59a46e2..5678991 100644
--- a/Makefile
+++ b/Makefile
@@ -1719,7 +1719,7 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o $(GITLIBS)
 
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
-builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o: branch.h
+builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
 builtin-bundle.o bundle.o transport.o: bundle.h
 builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
 builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
-- 
1.6.6

^ permalink raw reply related

* [PATCH 08/12] Makefile: disable default implicit rules
From: Jonathan Nieder @ 2010-01-26 15:51 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

The git makefile never uses any default implicit rules.
Unfortunately, if a prerequisite for one of the intended rules is
missing, a default rule can be used in its place:

	$ make var.s
	    CC var.s
	$ rm var.c
	$ make var.o
	    as   -o var.o var.s

Avoiding the default rules avoids this hard-to-debug behavior.
It also should speed things up a little in the normal case.

Future patches may restrict the scope of the %.o: %.c pattern.
This patch would then ensure that for targets not listed, we do
not fall back to the default rule.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Sadly, this does not really speed things up in the normal case.
I am not sure why, but make needs the -r option to avoid
considering the default rules, even though clearing the .SUFFIXES:
guarantees it will never use them.

 Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 84ce137..60ef711 100644
--- a/Makefile
+++ b/Makefile
@@ -1672,6 +1672,8 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
 XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
 	xdiff/xmerge.o xdiff/xpatience.o
 
+.SUFFIXES:
+
 %.o: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS FORCE
-- 
1.6.6

^ permalink raw reply related

* [PATCH 09/12] Makefile: list generated object files in OBJECTS
From: Jonathan Nieder @ 2010-01-26 15:52 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Set the OBJECTS variable to a comprehensive list of all object
file targets.  To make sure it is truly comprehensive, restrict
the scope of the %.o pattern rule to only generate objects in
this list.

Attempts to build other object files will fail loudly:

	$ touch foo.c
	$ make foo.o
	make: *** No rule to make target `foo.o'.  Stop.

providing a reminder to add the new object to the OBJECTS list.

The new variable is otherwise unused.  The intent is for later
patches to take advantage of it.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 60ef711..0e4e8ff 100644
--- a/Makefile
+++ b/Makefile
@@ -1671,14 +1671,19 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
 	$(patsubst git-%$X,%.o,$(PROGRAMS))
 XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
 	xdiff/xmerge.o xdiff/xpatience.o
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
+
+ASM_SRC := $(wildcard $(OBJECTS:o=S))
+ASM_OBJ := $(ASM_SRC:S=o)
+C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
 .SUFFIXES:
 
-%.o: %.c GIT-CFLAGS
+$(C_OBJ): %.o: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS FORCE
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S GIT-CFLAGS
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
 $(GIT_OBJS): $(LIB_H)
-- 
1.6.6

^ permalink raw reply related

* [PATCH 10/12] Makefile: lazily compute header dependencies
From: Jonathan Nieder @ 2010-01-26 15:52 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Use the gcc -MMD -MP -MF options to generate dependency rules as
a byproduct when building .o files if the
COMPUTE_HEADER_DEPENDENCIES variable is defined.  That variable
is left undefined by default for now.

As each object file is built, write a makefile fragment
containing its dependencies in the deps/ subdirectory of its
containing directory.  The deps/ directories should be generated
if they are missing at the start of each build.  So let each
object file depend on $(missing_dep_dirs), which lists only the
directories of this kind that are missing to avoid needlessly
regenerating files when the directories' timestamps change.

gcc learned the -MMD -MP -MF options in version 3.0, so most gcc
users should have them by now.

The dependencies this option computes are more specific than the
rough estimates hard-coded in the Makefile, greatly speeding up
rebuilds when only a little-used header file has changed.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 .gitignore |    1 +
 Makefile   |   49 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/.gitignore b/.gitignore
index 8df8f88..7b3acb7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
 *.exe
 *.[aos]
 *.py[co]
+*.o.d
 *+
 /config.mak
 /autom4te.cache
diff --git a/Makefile b/Makefile
index 0e4e8ff..2e75f05 100644
--- a/Makefile
+++ b/Makefile
@@ -217,6 +217,10 @@ all::
 #   DEFAULT_EDITOR='~/bin/vi',
 #   DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
 #   DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
+#
+# Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
+# and you want to avoid rebuilding objects when an unrelated header file
+# changes.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1677,14 +1681,48 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
+ifdef COMPUTE_HEADER_DEPENDENCIES
+dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS))))
+$(dep_dirs):
+	mkdir -p $@
+
+missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
+else
+dep_dirs =
+missing_dep_dirs =
+endif
+
 .SUFFIXES:
 
-$(C_OBJ): %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS FORCE
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-$(ASM_OBJ): %.o: %.S GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+# Take advantage of gcc's on-the-fly dependency generation
+# See <http://gcc.gnu.org/gcc-3.0/features.html>.
+dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
+ifneq ($(dep_files),)
+include $(dep_files)
+endif
+
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+else
+dep_args =
+
+# Dependencies on header files, for platforms that do not support
+# the gcc -MMD option.
+#
+# Dependencies on automatically generated headers such as common-cmds.h
+# should _not_ be included here, since they are necessary even when
+# building an object for the first time.
+#
+# XXX. Please check occasionally that these include all dependencies
+# gcc detects!
 
 $(GIT_OBJS): $(LIB_H)
 builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
@@ -1700,10 +1738,10 @@ builtin-pack-objects.o: thread-utils.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o remote-curl.o: http.h
 
-
 xdiff-interface.o $(XDIFF_OBJS): \
 	xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
 	xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+endif
 
 exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
@@ -2011,6 +2049,7 @@ clean:
 	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) -r bin-wrappers
+	$(RM) -r $(dep_dirs)
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
 	$(RM) -r autom4te.cache
 	$(RM) config.log config.mak.autogen config.mak.append config.status config.cache
-- 
1.6.6

^ permalink raw reply related

* [PATCH 11/12] Makefile: list standalone program object files in PROGRAM_OBJS
From: Jonathan Nieder @ 2010-01-26 15:54 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Because of new commands like git-remote-http, the OBJECTS list
contains fictitious objects such as remote-http.o.  Thus any
out-of-tree rules that require all $(OBJECTS) to be buildable
are broken.  Add a list of real program objects to avoid this
problem.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Preparing for patch 12.

 Makefile |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 2e75f05..ceaae1c 100644
--- a/Makefile
+++ b/Makefile
@@ -341,6 +341,7 @@ COMPAT_CFLAGS =
 COMPAT_OBJS =
 LIB_H =
 LIB_OBJS =
+PROGRAM_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
@@ -390,12 +391,15 @@ EXTRA_PROGRAMS =
 
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS += $(EXTRA_PROGRAMS)
-PROGRAMS += git-fast-import$X
-PROGRAMS += git-imap-send$X
-PROGRAMS += git-shell$X
-PROGRAMS += git-show-index$X
-PROGRAMS += git-upload-pack$X
-PROGRAMS += git-http-backend$X
+
+PROGRAM_OBJS += fast-import.o
+PROGRAM_OBJS += imap-send.o
+PROGRAM_OBJS += shell.o
+PROGRAM_OBJS += show-index.o
+PROGRAM_OBJS += upload-pack.o
+PROGRAM_OBJS += http-backend.o
+
+PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
 TEST_PROGRAMS_NEED_X += test-ctype
@@ -1139,10 +1143,12 @@ else
 	REMOTE_CURL_PRIMARY = git-remote-http$X
 	REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X
 	REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
+	PROGRAM_OBJS += http-fetch.o
 	PROGRAMS += $(REMOTE_CURL_NAMES) git-http-fetch$X
 	curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
 	ifeq "$(curl_check)" "070908"
 		ifndef NO_EXPAT
+			PROGRAM_OBJS += http-push.o
 			PROGRAMS += git-http-push$X
 		endif
 	endif
@@ -1163,6 +1169,7 @@ endif
 EXTLIBS += -lz
 
 ifndef NO_POSIX_ONLY_PROGRAMS
+	PROGRAM_OBJS += daemon.o
 	PROGRAMS += git-daemon$X
 endif
 ifndef NO_OPENSSL
@@ -1670,9 +1677,8 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
-GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
-	git.o http.o http-walker.o remote-curl.o \
-	$(patsubst git-%$X,%.o,$(PROGRAMS))
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
+	git.o http.o http-walker.o remote-curl.o
 XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
 	xdiff/xmerge.o xdiff/xpatience.o
 OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
-- 
1.6.6

^ permalink raw reply related

* [PATCH 07/12] Makefile: rearrange dependency rules
From: Jonathan Nieder @ 2010-01-26 15:49 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Put rules listing dependencies of compiled objects (.o files) on
header files (.h files) in one place, to make them easier to
compare and modify all at once.

Add a GIT_OBJS variable listing objects that depend on LIB_H,
for similar reasons.

No change in build-time behavior intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |   49 +++++++++++++++++++++++++------------------------
 1 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/Makefile b/Makefile
index 5678991..84ce137 100644
--- a/Makefile
+++ b/Makefile
@@ -1666,6 +1666,12 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(TEST_OBJS) \
+	git.o http.o http-walker.o remote-curl.o \
+	$(patsubst git-%$X,%.o,$(PROGRAMS))
+XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
+	xdiff/xmerge.o xdiff/xpatience.o
+
 %.o: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS FORCE
@@ -1673,6 +1679,25 @@ git.o git.spec \
 %.o: %.S GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
+$(GIT_OBJS): $(LIB_H)
+builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
+builtin-bundle.o bundle.o transport.o: bundle.h
+builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
+builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
+builtin-send-pack.o transport.o: send-pack.h
+builtin-log.o builtin-shortlog.o: shortlog.h
+builtin-prune.o builtin-reflog.o reachable.o: reachable.h
+builtin-commit.o builtin-revert.o wt-status.o: wt-status.h
+builtin-tar-tree.o archive-tar.o: tar.h
+builtin-pack-objects.o: thread-utils.h
+http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
+http.o http-walker.o http-push.o remote-curl.o: http.h
+
+
+xdiff-interface.o $(XDIFF_OBJS): \
+	xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
+	xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
+
 exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1696,10 +1721,6 @@ git-imap-send$X: imap-send.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL)
 
-http.o http-walker.o http-push.o remote-curl.o: http.h
-
-http.o http-walker.o remote-curl.o: $(LIB_H)
-
 git-http-fetch$X: revision.o http.o http-walker.o http-fetch.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL)
@@ -1717,29 +1738,9 @@ $(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)
 
-$(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
-$(patsubst git-%$X,%.o,$(PROGRAMS)) $(TEST_OBJS) git.o: $(LIB_H)
-builtin-branch.o builtin-checkout.o builtin-clone.o builtin-reset.o branch.o transport.o: branch.h
-builtin-bundle.o bundle.o transport.o: bundle.h
-builtin-bisect--helper.o builtin-rev-list.o bisect.o: bisect.h
-builtin-clone.o builtin-fetch-pack.o transport.o: fetch-pack.h
-builtin-send-pack.o transport.o: send-pack.h
-builtin-log.o builtin-shortlog.o: shortlog.h
-builtin-prune.o builtin-reflog.o reachable.o: reachable.h
-builtin-commit.o builtin-revert.o wt-status.o: wt-status.h
-builtin-tar-tree.o archive-tar.o: tar.h
-builtin-pack-objects.o: thread-utils.h
-http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
-
 $(LIB_FILE): $(LIB_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(LIB_OBJS)
 
-XDIFF_OBJS=xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
-	xdiff/xmerge.o xdiff/xpatience.o
-xdiff-interface.o $(XDIFF_OBJS): \
-	xdiff/xinclude.h xdiff/xmacros.h xdiff/xdiff.h xdiff/xtypes.h \
-	xdiff/xutils.h xdiff/xprepare.h xdiff/xdiffi.h xdiff/xemit.h
-
 $(XDIFF_LIB): $(XDIFF_OBJS)
 	$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(XDIFF_OBJS)
 
-- 
1.6.6

^ permalink raw reply related

* [PATCH 12/12] Teach Makefile to check header dependencies
From: Jonathan Nieder @ 2010-01-26 15:57 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Add a target to use the gcc-generated makefile snippets for
dependencies on header files to check the hard-coded dependencies.

With this patch applied, if any dependencies are missing, then

	make clean
	make COMPUTE_HEADER_DEPENDENCIES=YesPlease
	make CHECK_HEADER_DEPENDENCIES=YesPlease

will produce an error message like the following:

	CHECK fast-import.o
	missing dependencies: exec_cmd.h
	make: *** [fast-import.o] Error 1

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That’s the end of the series.  Thanks for reading.

 Makefile |   95 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index ceaae1c..be357a8 100644
--- a/Makefile
+++ b/Makefile
@@ -221,6 +221,9 @@ all::
 # Define COMPUTE_HEADER_DEPENDENCIES if your compiler supports the -MMD option
 # and you want to avoid rebuilding objects when an unrelated header file
 # changes.
+#
+# Define CHECK_HEADER_DEPENDENCIES to check for problems in the hard-coded
+# dependency rules.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1088,6 +1091,14 @@ endif
 -include config.mak.autogen
 -include config.mak
 
+ifdef CHECK_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES =
+endif
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+endif
+
 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)|'
@@ -1683,9 +1694,7 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
 	xdiff/xmerge.o xdiff/xpatience.o
 OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
 
-ASM_SRC := $(wildcard $(OBJECTS:o=S))
-ASM_OBJ := $(ASM_SRC:S=o)
-C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+dep_files := $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d)
 
 ifdef COMPUTE_HEADER_DEPENDENCIES
 dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS))))
@@ -1693,33 +1702,89 @@ $(dep_dirs):
 	mkdir -p $@
 
 missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
-else
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+ifdef CHECK_HEADER_DEPENDENCIES
+$(error cannot compute header dependencies outside a normal build. \
+Please unset CHECK_HEADER_DEPENDENCIES and try again)
+endif
+endif
+
+ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef CHECK_HEADER_DEPENDENCIES
 dep_dirs =
 missing_dep_dirs =
+dep_args =
+endif
+endif
+
+ifdef CHECK_HEADER_DEPENDENCIES
+ifndef PRINT_HEADER_DEPENDENCIES
+missing_deps = $(filter-out $(notdir $^), \
+	$(notdir $(shell $(MAKE) -s $@ \
+		CHECK_HEADER_DEPENDENCIES=YesPlease \
+		USE_COMPUTED_HEADER_DEPENDENCIES=YesPlease \
+		PRINT_HEADER_DEPENDENCIES=YesPlease)))
+endif
 endif
 
+ASM_SRC := $(wildcard $(OBJECTS:o=S))
+ASM_OBJ := $(ASM_SRC:S=o)
+C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
+
 .SUFFIXES:
 
+ifdef PRINT_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c FORCE
+	echo $^
+$(ASM_OBJ): %.o: %.S FORCE
+	echo $^
+
+ifndef CHECK_HEADER_DEPENDENCIES
+$(error cannot print header dependencies during a normal build. \
+Please set CHECK_HEADER_DEPENDENCIES and try again)
+endif
+endif
+
+ifndef PRINT_HEADER_DEPENDENCIES
+ifdef CHECK_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c $(dep_files) FORCE
+	@set -e; echo CHECK $@; \
+	missing_deps="$(missing_deps)"; \
+	if test "$$missing_deps"; \
+	then \
+		echo missing dependencies: $$missing_deps; \
+		false; \
+	fi
+$(ASM_OBJ): %.o: %.S $(dep_files) FORCE
+	@set -e; echo CHECK $@; \
+	missing_deps="$(missing_deps)"; \
+	if test "$$missing_deps"; \
+	then \
+		echo missing dependencies: $$missing_deps; \
+		false; \
+	fi
+endif
+endif
+
+ifndef CHECK_HEADER_DEPENDENCIES
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS FORCE
-	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+endif
 
-ifdef COMPUTE_HEADER_DEPENDENCIES
+%.s: %.c GIT-CFLAGS FORCE
+	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+
+ifdef USE_COMPUTED_HEADER_DEPENDENCIES
 # Take advantage of gcc's on-the-fly dependency generation
 # See <http://gcc.gnu.org/gcc-3.0/features.html>.
-dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
-ifneq ($(dep_files),)
-include $(dep_files)
+dep_files_present := $(wildcard $(dep_files))
+ifneq ($(dep_files_present),)
+include $(dep_files_present)
 endif
-
-dep_file = $(dir $@)deps/$(notdir $@).d
-dep_args = -MF $(dep_file) -MMD -MP
 else
-dep_args =
-
 # Dependencies on header files, for platforms that do not support
 # the gcc -MMD option.
 #
-- 
1.6.6

^ permalink raw reply related

* Re: [PATCH 00/12] Re: Makefile: add missing header dependency rules
From: Jonathan Nieder @ 2010-01-26 16:19 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <20100126154357.GA4895@progeny.tock>

Jonathan Nieder wrote:

> The remainder of this series is based on a merge of master and
> patches 1-5:
[...]
> Jonathan Nieder (12):
>   Makefile: add missing header file dependencies
>   Makefile: make sure test helpers are rebuilt when headers change
>   Makefile: remove wt-status.h from LIB_H
>   Makefile: clean up http-walker.o dependency rules
>   Makefile: drop dependency on $(wildcard */*.h)
>   Makefile: transport.o depends on branch.h now
>   Makefile: rearrange dependency rules
>   Makefile: disable default implicit rules
>   Makefile: list generated object files in OBJECTS macro
>   Makefile: lazily compute header dependencies
>   Makefile: list standalone program object files in PROGRAM_OBJS
>   Teach Makefile to check header dependencies

For those would like to avoid redoing the conflict resolution (and
who wouldn’t?), this series is also available in the git repository
at

  git://repo.or.cz/git/jrn.git autodep

If you’re interested, please take a look around and try it out.  I’d
be happy to hear about any bugs you find.

Thanks again,
Jonathan

^ permalink raw reply

* Re: [PATCH v4] Threaded grep
From: Benjamin Kramer @ 2010-01-26 16:30 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Fredrik Kuivinen, Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260728260.3574@localhost.localdomain>

BSD and glibc have an extension to regexec which takes a buffer + length pair
instead of a NUL-terminated string. Since we already have the length
computed this can save us a strlen call.
---

On 26.01.10 16:28, Linus Torvalds wrote:
> so it's sadly internal to regex. It would be nice if there was a 
> non-string interface to regexec (ie a "buffer + length" instead of a 
> NUL-terminated string).

BSD and glibc have an "REG_STARTEND" flag to do that. I made a small
PoC patch to use it if it's available but it didn't give any significant
speedup on my system.



 grep.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/grep.c b/grep.c
index d281a02..60cce46 100644
--- a/grep.c
+++ b/grep.c
@@ -675,8 +675,15 @@ static int look_ahead(struct grep_opt *opt,
 
 		if (p->fixed)
 			hit = !fixmatch(p->pattern, bol, p->ignore_case, &m);
-		else
+		else {
+#ifdef REG_STARTEND
+			m.rm_so = 0;
+			m.rm_eo = *left_p;
+			hit = !regexec(&p->regexp, bol, 1, &m, REG_STARTEND);
+#else
 			hit = !regexec(&p->regexp, bol, 1, &m, 0);
+#endif
+		}
 		if (!hit || m.rm_so < 0 || m.rm_eo < 0)
 			continue;
 		if (earliest < 0 || m.rm_so < earliest)
--
1.7.0.rc0.12.gc33c3

^ permalink raw reply related

* Re: [PATCH v4] Threaded grep
From: Linus Torvalds @ 2010-01-26 16:44 UTC (permalink / raw)
  To: Benjamin Kramer
  Cc: Fredrik Kuivinen, Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <4B5F1894.4070509@googlemail.com>



On Tue, 26 Jan 2010, Benjamin Kramer wrote:
> 
> BSD and glibc have an "REG_STARTEND" flag to do that. I made a small
> PoC patch to use it if it's available but it didn't give any significant
> speedup on my system.

Goodie.  It's noticeable for me. This is what I reported earlier:

> > $ /usr/bin/time git grep void
> 
> Before:
> 
>         real    0m1.144s
>         user    0m0.988s
>         sys     0m0.148s
> 
> After:
>         real    0m0.290s
>         user    0m1.732s
>         sys     0m0.232s

and with your patch I get

	real	0m0.239s
	user	0m1.392s
	sys	0m0.276s

and the profile shows no strlen in it:

    57.12%      git  libc-2.11.1.so                 [.] re_search_internal
     5.59%      git  [kernel]                       [k] copy_user_generic_string
     4.09%      git  [kernel]                       [k] _raw_spin_lock
     2.57%      git  [kernel]                       [k] intel_pmu_enable_all
     2.46%      git  [kernel]                       [k] __d_lookup
     1.94%      git  libc-2.11.1.so                 [.] re_string_reconstruct
     1.87%      git  [kernel]                       [k] kmem_cache_alloc
     1.68%      git  libc-2.11.1.so                 [.] _int_free
     1.53%      git  [kernel]                       [k] find_get_page
     1.43%      git  [kernel]                       [k] update_curr
     1.27%      git  libc-2.11.1.so                 [.] __GI___libc_malloc
     1.17%      git  [kernel]                       [k] _atomic_dec_and_lock
     1.00%      git  libc-2.11.1.so                 [.] __GI_memcpy

Side note: the tailing end of the profiles aren't very stable, probably 
because the grep executes so quickly and in so many threads, so the 
functions in the one-percent range will move up and down the list 
depending on just exactly where we happened to get profile hits. 
Similarly, the raw_spin_lock numbers vary.

But the big picture is stable, and that 57% number (and the nonlock 
copy_user_generic_string) is consistent. And your patch definitely helped 
both actual performance and is visible in the profile: re_search_internal 
went from ~52% to ~57%.

So ack on that patch. Looks like a good thing to do, and with the #ifdef, 
it looks like it should just automatically DTRT based on regexec 
implementation.

		Linus

^ permalink raw reply

* Re: [PATCH v4] Threaded grep
From: Linus Torvalds @ 2010-01-26 16:56 UTC (permalink / raw)
  To: Benjamin Kramer
  Cc: Fredrik Kuivinen, Git Mailing List, Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260836520.3574@localhost.localdomain>



On Tue, 26 Jan 2010, Linus Torvalds wrote:
>
> Goodie.  It's noticeable for me. This is what I reported earlier:
> 
> > > $ /usr/bin/time git grep void
> > 
> > Before:
> > 
> >         real    0m1.144s
> > 
> > After:
> >         real    0m0.290s
> 
> and with your patch I get
> 
> 	real	0m0.239s

Btw, I have to also say that this whole performance reduction _feels_ 
good. It's very noticeable in normal use. "git grep" was always fast (it's 
been getting a bit slower as the kernel has grown, though), but it used to 
be still a noticeable pause.

Now it just -feels- very immediate. That quarter second is short enough 
that I can see the pause, but I don't feel it. It's like the results just 
"are there" rather than get searched for.

But perhaps even more importantly, it's also noticeable for me in the 
cold-cache case. IOW, after I do

	echo 3 > /proc/sys/vm/drop_caches

the threaded grep is able to do much better at reading the disk:

Before threading:

	[torvalds@nehalem linux]$ time git grep void > /dev/null 

	real	0m11.745s
	user	0m2.380s
	sys	0m1.200s

After:

	[torvalds@nehalem linux]$ time ~/git/git grep void > /dev/null 

	real	0m3.710s
	user	0m2.564s
	sys	0m2.076s

although it is worth noting that that machine has an Intel SSD, which is 
why it gets sped up so much by parallel IO (there's no seek penalty, and 
it is able to read multiple channels in parallel, so this gives much 
better IO patterns for it - with rotational media the numbers might be 
very different).

IOW, the whole threaded grep thing is a 4x performance improvement in 
hot-cache, and a 3x improvement in cold-cache.

Major good mojo.

		Linus

^ permalink raw reply

* Re: [PATCH v4] Threaded grep
From: Mike Hommey @ 2010-01-26 17:19 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Benjamin Kramer, Fredrik Kuivinen, Git Mailing List,
	Junio C Hamano, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260846330.3574@localhost.localdomain>

On Tue, Jan 26, 2010 at 08:56:50AM -0800, Linus Torvalds wrote:
<snip>
> although it is worth noting that that machine has an Intel SSD, which is 
> why it gets sped up so much by parallel IO (there's no seek penalty, and 
> it is able to read multiple channels in parallel, so this gives much 
> better IO patterns for it - with rotational media the numbers might be 
> very different).

For rotational disks, using FIEMAP to get the position of the files on
disk to reorder how we read them could help.

Mike

^ permalink raw reply

* Re: [PATCH v4] Threaded grep
From: Junio C Hamano @ 2010-01-26 17:21 UTC (permalink / raw)
  To: Fredrik Kuivinen; +Cc: git, Linus Torvalds, Johannes Sixt
In-Reply-To: <20100126114303.GA1854@fredrik-laptop>

Fredrik Kuivinen <frekui@gmail.com> writes:

> I just noticed that I forgot to take the read_sha1 lock in
> grep_tree. The result is a race condition in read_sha1_file.
>
> Here is a patch to fix this. It applies on top of ae35c68 (Threaded
> grep). Could you please squash it in?

Thanks; will do.

^ permalink raw reply

* [PATCH] grep: use REG_STARTEND (if available) to speed up regexec
From: Benjamin Kramer @ 2010-01-26 17:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Fredrik Kuivinen, Git Mailing List, Linus Torvalds, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.1001260846330.3574@localhost.localdomain>

BSD and glibc have an extension to regexec which takes a buffer + length pair
instead of a NUL-terminated string. Since we already have the length computed
this can save us a strlen call inside regexec.

Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Resend of my previous patch with SOB and correct title.

 grep.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/grep.c b/grep.c
index 8e1f7de..452c2cb 100644
--- a/grep.c
+++ b/grep.c
@@ -640,8 +640,15 @@ static int look_ahead(struct grep_opt *opt,
 
 		if (p->fixed)
 			hit = !fixmatch(p->pattern, bol, p->ignore_case, &m);
-		else
+		else {
+#ifdef REG_STARTEND
+			m.rm_so = 0;
+			m.rm_eo = *left_p;
+			hit = !regexec(&p->regexp, bol, 1, &m, REG_STARTEND);
+#else
 			hit = !regexec(&p->regexp, bol, 1, &m, 0);
+#endif
+		}
 		if (!hit || m.rm_so < 0 || m.rm_eo < 0)
 			continue;
 		if (earliest < 0 || m.rm_so < earliest)
-- 
1.7.0.rc0.12.gc33c3

^ permalink raw reply related

* [PATCH 0/4] Fix various integer overflows
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git

Fix integer overflows in patch_delta(), unpack_sha1_rest() and
unpack_compressed_entry().

These at least can cause git to segfault, possibly worse. Operations
that cause integer overflow are not possible to do (even whole virtual
memory space would not be sufficient), so die() instead.

Ilari Liusvaara (4):
  Add xmallocz()
  Fix integer overflow in patch_delta()
  Fix integer overflow in unpack_sha1_rest()
  Fix integer overflow in unpack_compressed_entry()

 git-compat-util.h |    1 +
 patch-delta.c     |    3 +--
 sha1_file.c       |    5 ++---
 wrapper.c         |   12 +++++++++++-
 4 files changed, 15 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH 3/4] Fix integer overflow in unpack_sha1_rest()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>


Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 sha1_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 12478a3..39f0844 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1166,7 +1166,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
 static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, const unsigned char *sha1)
 {
 	int bytes = strlen(buffer) + 1;
-	unsigned char *buf = xmalloc(1+size);
+	unsigned char *buf = xmallocz(size);
 	unsigned long n;
 	int status = Z_OK;
 
-- 
1.6.6.1.439.gf06b6

^ permalink raw reply related

* [PATCH 2/4] Fix integer overflow in patch_delta()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>


Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 patch-delta.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/patch-delta.c b/patch-delta.c
index e02e13b..d218faa 100644
--- a/patch-delta.c
+++ b/patch-delta.c
@@ -33,8 +33,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
 
 	/* now the result size */
 	size = get_delta_hdr_size(&data, top);
-	dst_buf = xmalloc(size + 1);
-	dst_buf[size] = 0;
+	dst_buf = xmallocz(size);
 
 	out = dst_buf;
 	while (data < top) {
-- 
1.6.6.1.439.gf06b6

^ permalink raw reply related

* [PATCH 1/4] Add xmallocz()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>

Add routine for allocating NUL-terminated memory block without risking
integer overflow in addition of +1 for NUL byte.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 git-compat-util.h |    1 +
 wrapper.c         |   12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 620a7c6..a3c4537 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -348,6 +348,7 @@ extern void release_pack_memory(size_t, int);
 
 extern char *xstrdup(const char *str);
 extern void *xmalloc(size_t size);
+extern void *xmallocz(size_t size);
 extern void *xmemdupz(const void *data, size_t len);
 extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);
diff --git a/wrapper.c b/wrapper.c
index c9be140..dd7b6ee 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -34,6 +34,16 @@ void *xmalloc(size_t size)
 	return ret;
 }
 
+void *xmallocz(size_t size)
+{
+	void *ret;
+	if (size + 1 < size)
+		die("Data too large to fit into virtual memory space.");
+	ret = xmalloc(size + 1);
+	((char*)ret)[size] = 0;
+	return ret;
+}
+
 /*
  * xmemdupz() allocates (len + 1) bytes of memory, duplicates "len" bytes of
  * "data" to the allocated memory, zero terminates the allocated memory,
@@ -42,7 +52,7 @@ void *xmalloc(size_t size)
  */
 void *xmemdupz(const void *data, size_t len)
 {
-	char *p = xmalloc(len + 1);
+	char *p = xmallocz(len);
 	memcpy(p, data, len);
 	p[len] = '\0';
 	return p;
-- 
1.6.6.1.439.gf06b6

^ permalink raw reply related

* [PATCH 4/4] Fix integer overflow in unpack_compressed_entry()
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>


Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 sha1_file.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 39f0844..ea2ea75 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1517,8 +1517,7 @@ static void *unpack_compressed_entry(struct packed_git *p,
 	z_stream stream;
 	unsigned char *buffer, *in;
 
-	buffer = xmalloc(size + 1);
-	buffer[size] = 0;
+	buffer = xmallocz(size);
 	memset(&stream, 0, sizeof(stream));
 	stream.next_out = buffer;
 	stream.avail_out = size + 1;
-- 
1.6.6.1.439.gf06b6

^ permalink raw reply related

* [PATCH 0/2] Allow using ':' in git:// hostname.
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git

This series fixes two problems with using addresses containing ':'
(e.g. IPv6 numeric addresses) with git://:

1) ':' in hostname makes vhost headers impossible to parse

If there is ':' in address, the vhost headers become impossible
to parse because ':' is also splits host and port and port is
optional. Change git-daemon to be able to perform address unwrapping
so there is uniquely parseable syntax for hostnames containg
':' (this is compatible to how git-remote-gits encodes such vhost
headers and how git-daemon2[1] decodes them).

2) Client double-unwraps addresses

With git://, the addresses are unwrapped twice, which breaks
address parsing for addresses enclosed by [], which in turn is
required for hostnames containing ':'.  This is changed to unwarp
the addresses only once. This also changes wrapped addresses to
be sent as wrapped for vhost headers (the first patch adds ability
to parse this).

[1] The reference implementation of gits:// server daemon.

Ilari Liusvaara (2):
  Support addresses with ':' in git-daemon
  Allow use of []-wrapped addresses in git://

 connect.c |   10 ++++++++--
 daemon.c  |   34 ++++++++++++++++++++++++++++++----
 2 files changed, 38 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCH 1/2] Support addresses with ':' in git-daemon
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1264530282-4783-1-git-send-email-ilari.liusvaara@elisanet.fi>

If host address could have ':' in it (e.g. numeric IPv6 address), then
host and port could not be uniquely parsed. Fix this by parsing the
"["<host>"]":<port> and "["<host>"]" notations. Currently the built-in
git:// client would send <host>:<port> or <host> for such thing, but
it doesn't matter as due to bugs, resolving address fails if <host>
contains ':'.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 daemon.c |   34 ++++++++++++++++++++++++++++++----
 1 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/daemon.c b/daemon.c
index 360635e..6c2bd97 100644
--- a/daemon.c
+++ b/daemon.c
@@ -399,6 +399,33 @@ static char *xstrdup_tolower(const char *str)
 	return dup;
 }
 
+static void parse_host_and_port(char *hostport, char **host,
+	char **port)
+{
+	if (*hostport == '[') {
+		char *end;
+
+		end = strchr(hostport, ']');
+		if (!end)
+			die("Invalid reqeuest ('[' without ']')");
+		*end = '\0';
+		*host = hostport + 1;
+		if (!end[1])
+			*port = NULL;
+		else if (end[1] == ':')
+			*port = end + 2;
+		else
+			die("Garbage after end of host part");
+	} else {
+		*host = hostport;
+		*port = strrchr(hostport, ':');
+		if (*port) {
+			*port = '\0';
+			++*port;
+		}
+	}
+}
+
 /*
  * Read the host as supplied by the client connection.
  */
@@ -415,11 +442,10 @@ static void parse_host_arg(char *extra_args, int buflen)
 			vallen = strlen(val) + 1;
 			if (*val) {
 				/* Split <host>:<port> at colon. */
-				char *host = val;
-				char *port = strrchr(host, ':');
+				char *host;
+				char *port;
+				parse_host_and_port(val, &host, &port);
 				if (port) {
-					*port = 0;
-					port++;
 					free(tcp_port);
 					tcp_port = xstrdup(port);
 				}
-- 
1.6.6.1.439.gf06b6

^ permalink raw reply related

* [PATCH 2/2] Allow use of []-wrapped addresses in git://
From: Ilari Liusvaara @ 2010-01-26 18:24 UTC (permalink / raw)
  To: git
In-Reply-To: <1264530282-4783-1-git-send-email-ilari.liusvaara@elisanet.fi>

Allow using "["<host>"]":<port> and "["<host>"]" notations in git://
host addresses. This is needed to be able to connect to addresses
that contain ':' (e.g. numeric IPv6 addresses). Also send the host
header []-wrapped so it can actually be parsed by remote end.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 connect.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index 3a12562..20054e4 100644
--- a/connect.c
+++ b/connect.c
@@ -502,12 +502,18 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 		c = ':';
 	}
 
+	/*
+	 * Don't do destructive transforms with git:// as that
+	 * protocol code does '[]' dewrapping of its own.
+	 */
 	if (host[0] == '[') {
 		end = strchr(host + 1, ']');
 		if (end) {
-			*end = 0;
+			if (protocol != PROTO_GIT) {
+				*end = 0;
+				host++;
+			}
 			end++;
-			host++;
 		} else
 			end = host;
 	} else
-- 
1.6.6.1.439.gf06b6

^ permalink raw reply related

* Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
From: Junio C Hamano @ 2010-01-26 19:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Johannes Schindelin
In-Reply-To: <20100126130745.GB28179@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The first one is that @{usptream} silently becomes @{0}. I think
> we need to double-check whether approxidate found absolutely nothing,
> and complain if that is the case.

This is not a new problem introduced by Dscho's @{u} series; it was there
even before 861f00e (fix reflog approxidate parsing bug, 2008-04-30).

Nevertheless, it would be nice to fix it.

-- >8 --
Subject: approxidate_careful() reports errorneous date string

For a long time, the time based reflog syntax (e.g. master@{yesterday})
didn't complain when the "human readable" timestamp was misspelled, as
the underlying mechanism tried to be as lenient as possible.  The funny
thing was that parsing of "@{now}" even relied on the fact that anything
not recognized by the machinery returned the current timestamp.

Introduce approxidate_careful() that takes an optional pointer to an
integer, that gets assigned 1 when the input does not make sense as a
timestamp.

As I am too lazy to fix all the callers that use approxidate(), most of
the callers do not take advantage of the error checking, but convert the
code to parse reflog to use it as a demonstration.

Tests are mostly from Jeff King.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h              |    3 ++-
 date.c               |   43 +++++++++++++++++++++++++++++++++++--------
 sha1_name.c          |    5 ++++-
 t/t0101-at-syntax.sh |   45 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/cache.h b/cache.h
index b3370eb..f0fea2d 100644
--- a/cache.h
+++ b/cache.h
@@ -762,7 +762,8 @@ const char *show_date_relative(unsigned long time, int tz,
 			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
-unsigned long approxidate(const char *);
+#define approxidate(s) approxidate_careful(s, NULL)
+unsigned long approxidate_careful(const char *, int *);
 unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
diff --git a/date.c b/date.c
index 45f3684..002aa3c 100644
--- a/date.c
+++ b/date.c
@@ -696,6 +696,11 @@ static unsigned long update_tm(struct tm *tm, struct tm *now, unsigned long sec)
 	return n;
 }
 
+static void date_now(struct tm *tm, struct tm *now, int *num)
+{
+	update_tm(tm, now, 0);
+}
+
 static void date_yesterday(struct tm *tm, struct tm *now, int *num)
 {
 	update_tm(tm, now, 24*60*60);
@@ -770,6 +775,7 @@ static const struct special {
 	{ "PM", date_pm },
 	{ "AM", date_am },
 	{ "never", date_never },
+	{ "now", date_now },
 	{ NULL }
 };
 
@@ -790,7 +796,7 @@ static const struct typelen {
 	{ NULL }
 };
 
-static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm *now, int *num)
+static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm *now, int *num, int *touched)
 {
 	const struct typelen *tl;
 	const struct special *s;
@@ -804,6 +810,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 		int match = match_string(date, month_names[i]);
 		if (match >= 3) {
 			tm->tm_mon = i;
+			*touched = 1;
 			return end;
 		}
 	}
@@ -812,6 +819,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 		int len = strlen(s->name);
 		if (match_string(date, s->name) == len) {
 			s->fn(tm, now, num);
+			*touched = 1;
 			return end;
 		}
 	}
@@ -821,11 +829,14 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 			int len = strlen(number_name[i]);
 			if (match_string(date, number_name[i]) == len) {
 				*num = i;
+				*touched = 1;
 				return end;
 			}
 		}
-		if (match_string(date, "last") == 4)
+		if (match_string(date, "last") == 4) {
 			*num = 1;
+			*touched = 1;
+		}
 		return end;
 	}
 
@@ -835,6 +846,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 		if (match_string(date, tl->type) >= len-1) {
 			update_tm(tm, now, tl->length * *num);
 			*num = 0;
+			*touched = 1;
 			return end;
 		}
 		tl++;
@@ -852,6 +864,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 			diff += 7*n;
 
 			update_tm(tm, now, diff * 24 * 60 * 60);
+			*touched = 1;
 			return end;
 		}
 	}
@@ -866,6 +879,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 			tm->tm_year--;
 		}
 		tm->tm_mon = n;
+		*touched = 1;
 		return end;
 	}
 
@@ -873,6 +887,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 		update_tm(tm, now, 0); /* fill in date fields if needed */
 		tm->tm_year -= *num;
 		*num = 0;
+		*touched = 1;
 		return end;
 	}
 
@@ -929,9 +944,12 @@ static void pending_number(struct tm *tm, int *num)
 	}
 }
 
-static unsigned long approxidate_str(const char *date, const struct timeval *tv)
+static unsigned long approxidate_str(const char *date,
+				     const struct timeval *tv,
+				     int *error_ret)
 {
 	int number = 0;
+	int touched = 0;
 	struct tm tm, now;
 	time_t time_sec;
 
@@ -951,33 +969,42 @@ static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 		if (isdigit(c)) {
 			pending_number(&tm, &number);
 			date = approxidate_digit(date-1, &tm, &number);
+			touched = 1;
 			continue;
 		}
 		if (isalpha(c))
-			date = approxidate_alpha(date-1, &tm, &now, &number);
+			date = approxidate_alpha(date-1, &tm, &now, &number, &touched);
 	}
 	pending_number(&tm, &number);
+	if (!touched)
+		*error_ret = 1;
 	return update_tm(&tm, &now, 0);
 }
 
 unsigned long approxidate_relative(const char *date, const struct timeval *tv)
 {
 	char buffer[50];
+	int errors = 0;
 
 	if (parse_date(date, buffer, sizeof(buffer)) > 0)
 		return strtoul(buffer, NULL, 0);
 
-	return approxidate_str(date, tv);
+	return approxidate_str(date, tv, &errors);
 }
 
-unsigned long approxidate(const char *date)
+unsigned long approxidate_careful(const char *date, int *error_ret)
 {
 	struct timeval tv;
 	char buffer[50];
+	int dummy = 0;
+	if (!error_ret)
+		error_ret = &dummy;
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+	if (parse_date(date, buffer, sizeof(buffer)) > 0) {
+		*error_ret = 0;
 		return strtoul(buffer, NULL, 0);
+	}
 
 	gettimeofday(&tv, NULL);
-	return approxidate_str(date, &tv);
+	return approxidate_str(date, &tv, error_ret);
 }
diff --git a/sha1_name.c b/sha1_name.c
index 9215ad1..ed4c028 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -413,8 +413,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		} else if (0 <= nth)
 			at_time = 0;
 		else {
+			int errors = 0;
 			char *tmp = xstrndup(str + at + 2, reflog_len);
-			at_time = approxidate(tmp);
+			at_time = approxidate_careful(tmp, &errors);
+			if (errors)
+				die("Bogus timestamp '%s'", tmp);
 			free(tmp);
 		}
 		if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
diff --git a/t/t0101-at-syntax.sh b/t/t0101-at-syntax.sh
new file mode 100755
index 0000000..ccabc37
--- /dev/null
+++ b/t/t0101-at-syntax.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+test_description='various @{whatever} syntax tests'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit one &&
+	test_commit two
+'
+
+check_at() {
+	echo "$2" >expect &&
+	git log -1 --format=%s "$1" >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success '@{0} shows current' '
+	check_at @{0} two
+'
+
+test_expect_success '@{1} shows old' '
+	check_at @{1} one
+'
+
+test_expect_success '@{now} shows current' '
+	check_at @{now} two
+'
+
+test_expect_success '@{30.years.ago} shows old' '
+	check_at @{30.years.ago} one
+'
+
+test_expect_success 'silly approxidates work' '
+	check_at @{3.hot.dogs.and.30.years.ago} one
+'
+
+test_expect_success 'notice misspelled upstream' '
+	test_must_fail git log -1 --format=%s @{usptream}
+'
+
+test_expect_success 'complain about total nonsense' '
+	test_must_fail git log -1 --format=%s @{utter.bogosity}
+'
+
+test_done

^ permalink raw reply related

* Re: [PATCH 0/4] Fix various integer overflows
From: Junio C Hamano @ 2010-01-26 19:58 UTC (permalink / raw)
  To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1264530255-4682-1-git-send-email-ilari.liusvaara@elisanet.fi>

Looks trivially correct; thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox