Git development
 help / color / mirror / Atom feed
* Re: [spf:guess] Re: [PATCH 1/2] git-svn: ignore changeless commits when checking for a cherry-pick
From: Eric Wong @ 2010-01-07  6:49 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Andrew Myrick, git
In-Reply-To: <4B4510EE.4090504@vilain.net>

Sam Vilain <sam@vilain.net> wrote:
> Eric Wong wrote:
> > Andrew Myrick <amyrick@apple.com> wrote:
> >   
> >> diff --git a/git-svn.perl b/git-svn.perl
> >
> > Hi Andrew,
> >
> > I'll again defer to Sam for Acks on these.  Test cases would be nice to
> > have, too.
> >   
> 
> They look fine to me, agreed a test case would be nice and make sure the
> features aren't lost later inadvertently.

Thanks Sam,

Shall I consider this an Ack and push Andrew's new changes
up while waiting for test cases?

-- 
Eric Wong

^ permalink raw reply

* [PATCH v2 0/5] Lazily generate header dependencies
From: Jonathan Nieder @ 2010-01-07  7:13 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100101090550.6117@nanako3.lavabit.com>

Nanako Shiraishi wrote:
> Junio, could you tell us what happened to this thread?
> 
> Makefile improvements.  No discussion.

My bad.  The previous version was very rough because I was not sure
yet how this could help in making the header dependency rules more
maintainable.  If all compilers worth using support something like
gcc's -MD option (does MSVC?), we could switch over completely;
otherwise, we need some way to use the generated dependencies to
check the static ones, or the static ones will go stale.

That is, maybe something like this.  With these patches applied,
running

	echo COMPUTE_HEADER_DEPENDENCIES=YesPlease >> config.mak
	make clean
	make
	make CHECK_HEADER_DEPENDENCIES=YesPlease

will fail unless the dependency rules in the Makefile account for
all #includes gcc noticed with the current configuration.

Patch 5 is a little rough around the edges, but I am hoping it
will convey the idea.

Enjoy,
Jonathan Nieder (5):
  Makefile: rearrange dependency rules
  Makefile: clear list of default rules
  Makefile: list generated object files in OBJECTS macro
  Makefile: lazily compute header dependencies
  Teach Makefile to check header dependencies

 .gitignore |    1 +
 Makefile   |  158 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 130 insertions(+), 29 deletions(-)

^ permalink raw reply

* [PATCH 1/5] Makefile: rearrange dependency rules
From: Jonathan Nieder @ 2010-01-07  7:16 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>

Collect header dependency rules after the pattern rules to make
it easier to modify them all at once.  No change in behavior is
intended.

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

diff --git a/Makefile b/Makefile
index 8ce6fd7..fa08535 100644
--- a/Makefile
+++ b/Makefile
@@ -1630,6 +1630,11 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.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
@@ -1637,6 +1642,14 @@ git.o git.spec \
 %.o: %.S GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
+$(GIT_OBJS): $(LIB_H)
+http.o http-walker.o http-push.o: http.h
+$(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
+builtin-revert.o wt-status.o: wt-status.h
+
+$(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)"' \
@@ -1661,10 +1674,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: http.h
-
-http.o http-walker.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)
@@ -1676,18 +1685,9 @@ git-remote-curl$X: 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)) git.o: $(LIB_H) $(wildcard */*.h)
-builtin-revert.o wt-status.o: wt-status.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_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.rc2

^ permalink raw reply related

* [PATCH 2/5] Makefile: clear list of default rules
From: Jonathan Nieder @ 2010-01-07  7:18 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>

The git makefile never uses any default implicit rules.  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 increases performance and avoids
hard-to-debug behaviour.  Especially, once the scope of the
%.o: %.c pattern rule is restricted, we should not fall back
to the default %.o: %.c pattern rule.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
'make -d' reveals that GNU make still ponders the default rules with
this patch applied, though at least it does not use them any more.  Is
it possible to set something like the make '-r' option from within a
makefile?

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

diff --git a/Makefile b/Makefile
index fa08535..9a5d897 100644
--- a/Makefile
+++ b/Makefile
@@ -1635,6 +1635,8 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.o \
 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.rc2

^ permalink raw reply related

* [PATCH 3/5] Makefile: add OBJECTS variable listing object files
From: Jonathan Nieder @ 2010-01-07  7:19 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>

To find the generated dependencies to include, we will need 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 that 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.

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

diff --git a/Makefile b/Makefile
index 9a5d897..87de3c3 100644
--- a/Makefile
+++ b/Makefile
@@ -388,6 +388,18 @@ SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 # Empty...
 EXTRA_PROGRAMS =
 
+TEST_PROGRAMS += test-chmtime$X
+TEST_PROGRAMS += test-ctype$X
+TEST_PROGRAMS += test-date$X
+TEST_PROGRAMS += test-delta$X
+TEST_PROGRAMS += test-dump-cache-tree$X
+TEST_PROGRAMS += test-genrandom$X
+TEST_PROGRAMS += test-match-trees$X
+TEST_PROGRAMS += test-parse-options$X
+TEST_PROGRAMS += test-path-utils$X
+TEST_PROGRAMS += test-sha1$X
+TEST_PROGRAMS += test-sigchain$X
+
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS += $(EXTRA_PROGRAMS)
 PROGRAMS += git-fast-import$X
@@ -1634,14 +1646,20 @@ GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) git.o http.o http-walker.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
+TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
+OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(TEST_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)
@@ -1757,18 +1775,6 @@ endif
 
 ### Testing rules
 
-TEST_PROGRAMS += test-chmtime$X
-TEST_PROGRAMS += test-ctype$X
-TEST_PROGRAMS += test-date$X
-TEST_PROGRAMS += test-delta$X
-TEST_PROGRAMS += test-dump-cache-tree$X
-TEST_PROGRAMS += test-genrandom$X
-TEST_PROGRAMS += test-match-trees$X
-TEST_PROGRAMS += test-parse-options$X
-TEST_PROGRAMS += test-path-utils$X
-TEST_PROGRAMS += test-sha1$X
-TEST_PROGRAMS += test-sigchain$X
-
 all:: $(TEST_PROGRAMS)
 
 # GNU make supports exporting all variables by "export" without parameters.
-- 
1.6.6.rc2

^ permalink raw reply related

* [PATCH 4/5] Makefile: lazily compute header dependencies
From: Jonathan Nieder @ 2010-01-07  7:23 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>

Use the gcc -MMD -MP -MF options to generate dependency rules as
a byproduct when building .o files.

This feature has to be optional (MSVC does not seem to support
anything like this), so unfortunately it does not make the
Makefile much easier to maintain.  The feature is enabled by the
COMPUTE_HEADER_DEPENDENCIES variable, which is unset by default.

The generated Makefile fragments are saved in deps/
subdirectories of each directory containing object files.  These
directories are generated if missing at the start of each build.
A dependency on $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
avoids 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 estimate 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>
---
Timings:

for arg in YesPlease ""
do
	{
		echo NO_CURL=1
		echo NO_TCLTK=1
		echo NO_PERL=1
		echo COMPUTE_HEADER_DEPENDENCIES="$arg"
	} >config.mak
	make
	make clean

	time -p make
	touch levenshtein.h
	time -p make

	make clean
done >/dev/null

Build	COMPUTE_HEADER_DEPENDENCIES	real	user	sys
first	YesPlease			55.06	45.13	5.23
second	YesPlease			3.13	2.04	0.79
first					55.45	45.49	4.99
second					53.14	43.19	4.70

 .gitignore |    1 +
 Makefile   |   40 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index ac02a58..803247f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -170,6 +170,7 @@
 *.exe
 *.[aos]
 *.py[co]
+*.o.d
 *+
 /config.mak
 /autom4te.cache
diff --git a/Makefile b/Makefile
index 87de3c3..578843c 100644
--- a/Makefile
+++ b/Makefile
@@ -221,6 +221,9 @@ all::
 #   DEFAULT_EDITOR='~/bin/vi',
 #   DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
 #   DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
+#
+# Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
+# when an unrelated header file changes and your compiler supports it.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1653,15 +1656,38 @@ 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_dir_dep := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
+
+$(dep_dirs):
+	mkdir -p $@
+else
+dep_dirs =
+dep_dir_dep =
+endif
+
 .SUFFIXES:
 
-$(C_OBJ): %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+$(C_OBJ): %.o: %.c GIT-CFLAGS $(dep_dir_dep)
+	$(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 $(dep_dir_dep)
+	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+# Take advantage of gcc's 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
 $(GIT_OBJS): $(LIB_H)
 http.o http-walker.o http-push.o: http.h
 $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(wildcard */*.h)
@@ -1670,6 +1696,9 @@ builtin-revert.o wt-status.o: wt-status.h
 $(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
 
+dep_args =
+endif
+
 exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
@@ -1794,7 +1823,9 @@ test-delta$X: diff-delta.o patch-delta.o
 
 test-parse-options$X: parse-options.o
 
+ifndef COMPUTE_HEADER_DEPENDENCIES
 test-parse-options.o: parse-options.h
+endif
 
 .PRECIOUS: $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
 
@@ -1954,6 +1985,7 @@ clean:
 		$(LIB_FILE) $(XDIFF_LIB)
 	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
+	$(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.rc2

^ permalink raw reply related

* [PATCH/RFC 5/5] Teach Makefile to check header dependencies
From: Jonathan Nieder @ 2010-01-07  7:30 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>

Portability means we cannot completely switch over to
automatically generated dependencies on header files, since some
compilers do not support them.  This would seem to lead to a
dangerous situation in which the hand-written dependency rules
are needed for some configurations but poorly maintained because
most configurations do not use them.

Luckily, there is a way out: as part of testing git, ask the
build system to verify that the hand-written dependency rules are
consistent with the automatically generated ones.  This patch is
a start towards that goal.

The actual patch requires a few steps:

 1. Separate out a USE_COMPUTED_HEADER_DEPENDENCIES option to
    tell make to use the makefile snippets stored in deps/*
    without necessarily regenerating them;

 2. Add a PRINT_HEADER_DEPENDENCIES option to turn on
    USE_COMPUTED_HEADER_DEPENDENCIES and make the %.o: %.c rule
    print its prerequisites instead of compiling anything;

 3. Add a CHECK_HEADER_DEPENDENCIES option to turn off
    USE_COMPUTED_HEADER_DEPENDENCIES and make the %.o: %.c rule
    check that its prerequisites includes all files listed by
    'make -s PRINT_HEADER_DEPENDENCIES=YesPlease $@' instead of
    compiling anything.

With this patch applied,

	echo COMPUTE_HEADER_DEPENDENCIES=YesPlease >> config.mak
	make clean
	make
	make CHECK_HEADER_DEPENDENCIES=YesPlease

produces a useful error message:

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

Probably we should check for missing deps/%.o.d files to avoid
false negatives if some are missing.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
I look forward to your thoughts.

This is a bit clunky, but it is useful enough to detect a few problems
with the current dependency rules.  Patches should follow soon.

 Makefile |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 578843c..e642a24 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,8 @@ all::
 #
 # Define COMPUTE_HEADER_DEPENDENCIES if you want to avoid rebuilding objects
 # when an unrelated header file changes and your compiler supports it.
+#
+# Define CHECK_HEADER_DEPENDENCIES after a successful build to find problems.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1064,6 +1066,28 @@ endif
 -include config.mak.autogen
 -include config.mak
 
+ifdef PRINT_HEADER_DEPENDENCIES
+CHECK_HEADER_DEPENDENCIES = YesPlease
+endif
+
+ifdef CHECK_HEADER_DEPENDENCIES
+ifndef COMPUTE_HEADER_DEPENDENCIES
+$(error checking dependencies requires build with COMPUTE_HEADER_DEPENDENCIES)
+endif
+endif
+
+ifdef COMPUTE_HEADER_DEPENDENCIES
+ifdef CHECK_HEADER_DEPENDENCIES
+ifdef PRINT_HEADER_DEPENDENCIES
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+else
+USE_COMPUTED_HEADER_DEPENDENCIES =
+endif
+else
+USE_COMPUTED_HEADER_DEPENDENCIES = YesPlease
+endif
+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)|'
@@ -1669,14 +1693,48 @@ endif
 
 .SUFFIXES:
 
+ifdef CHECK_HEADER_DEPENDENCIES
+
+ifdef PRINT_HEADER_DEPENDENCIES
+$(C_OBJ): %.o: %.c FORCE
+	echo $^
+$(ASM_OBJ): %.o: %.S FORCE
+	echo $^
+else
+missing_deps = $(filter-out $^, \
+	$(shell $(MAKE) -s PRINT_HEADER_DEPENDENCIES=1 $@))
+
+$(C_OBJ): %.o: %.c 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 FORCE
+	@set -e; echo CHECK $@; \
+	missing_deps="$(missing_deps)"; \
+	if test "$$missing_deps"; \
+	then \
+		echo missing dependencies: $$missing_deps; \
+		false; \
+	fi
+endif
+
+else
+
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(dep_dir_dep)
 	$(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 $(dep_dir_dep)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(ALL_CFLAGS) $<
 
-ifdef COMPUTE_HEADER_DEPENDENCIES
+endif
+
+%.s: %.c GIT-CFLAGS FORCE
+	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+
+ifdef USE_COMPUTED_HEADER_DEPENDENCIES
 # Take advantage of gcc's dependency generation
 # See <http://gcc.gnu.org/gcc-3.0/features.html>.
 dep_files := $(wildcard $(foreach f,$(OBJECTS),$(dir f)deps/$(notdir $f).d))
@@ -1684,9 +1742,6 @@ 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
 $(GIT_OBJS): $(LIB_H)
 http.o http-walker.o http-push.o: http.h
@@ -1695,7 +1750,12 @@ builtin-revert.o wt-status.o: wt-status.h
 
 $(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
 
+ifdef COMPUTE_HEADER_DEPENDENCIES
+dep_file = $(dir $@)deps/$(notdir $@).d
+dep_args = -MF $(dep_file) -MMD -MP
+else
 dep_args =
 endif
 
@@ -1823,7 +1883,7 @@ test-delta$X: diff-delta.o patch-delta.o
 
 test-parse-options$X: parse-options.o
 
-ifndef COMPUTE_HEADER_DEPENDENCIES
+ifndef USE_COMPUTED_HEADER_DEPENDENCIES
 test-parse-options.o: parse-options.h
 endif
 
-- 
1.6.6.rc2

^ permalink raw reply related

* Re: [PATCH 2/5] Makefile: use target-specific variable to pass flags to cc
From: Jonathan Nieder @ 2010-01-07  7:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20100106080504.GC7298@progeny.tock>

Jonathan Nieder wrote:

> diff --git a/Makefile b/Makefile
> index ba4d071..81190a6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1467,20 +1467,19 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>  strip: $(PROGRAMS) git$X
>  	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
>  
> -git.o: git.c common-cmds.h GIT-CFLAGS
> -	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
> -		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
> -		$(ALL_CFLAGS) -o $@ -c $(filter %.c,$^)
> +git.o: common-cmds.h
> +git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
> +	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
>  
[...]

One annoying feature I wasn't thinking of: the values of
target-specific variables propagate to the dependencies of a target
(why? I can't imagine), and GIT-CFLAGS keeps on changing because of
this.

Maybe a new CMD_CFLAGS variable is needed for this, i.e. something
like the following squashed in.

diff --git a/Makefile b/Makefile
index 2580e23..d20e456 100644
--- a/Makefile
+++ b/Makefile
@@ -1468,7 +1468,7 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: common-cmds.h
-git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
+git.o: CMD_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
@@ -1476,7 +1476,7 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
 
 builtin-help.o: common-cmds.h
-builtin-help.o: ALL_CFLAGS += \
+builtin-help.o: CMD_CFLAGS += \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 	'-DGIT_INFO_PATH="$(infodir_SQ)"'
@@ -1630,28 +1630,31 @@ git.o git.spec \
 	$(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	: GIT-VERSION-FILE
 
+# This can vary by target
+CMD_CFLAGS = $(ALL_CFLAGS)
+
 %.o: %.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(CMD_CFLAGS) $<
 %.s: %.c GIT-CFLAGS .FORCE-LISTING
-	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -S $(CMD_CFLAGS) $<
 %.o: %.S GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
+	$(QUIET_CC)$(CC) -o $*.o -c $(CMD_CFLAGS) $<
 
-exec_cmd.o: ALL_CFLAGS += \
+exec_cmd.o: CMD_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
 	'-DPREFIX="$(prefix_SQ)"'
 
-builtin-init-db.o: ALL_CFLAGS += \
+builtin-init-db.o: CMD_CFLAGS += \
 	-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 
-config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+config.o: CMD_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
 
-http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
+http.o: CMD_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
 
 ifdef NO_EXPAT
 http-walker.o: http.h
-http-walker.o: ALL_CFLAGS += -DNO_EXPAT
+http-walker.o: CMD_CFLAGS += -DNO_EXPAT
 endif
 
 git-%$X: %.o $(GITLIBS)

^ permalink raw reply related

* Re: [spf:guess] Re: [spf:guess] Re: [PATCH 1/2] git-svn: ignore changeless commits when checking for a cherry-pick
From: Sam Vilain @ 2010-01-07  9:05 UTC (permalink / raw)
  To: Eric Wong; +Cc: Andrew Myrick, git
In-Reply-To: <20100107064916.GA8557@dcvr.yhbt.net>

On Wed, 2010-01-06 at 22:49 -0800, Eric Wong wrote:
> Sam Vilain <sam@vilain.net> wrote:
> > Eric Wong wrote:
> > > Andrew Myrick <amyrick@apple.com> wrote:
> > >   
> > >> diff --git a/git-svn.perl b/git-svn.perl
> > >
> > > Hi Andrew,
> > >
> > > I'll again defer to Sam for Acks on these.  Test cases would be nice to
> > > have, too.
> > >   
> > 
> > They look fine to me, agreed a test case would be nice and make sure the
> > features aren't lost later inadvertently.
> 
> Thanks Sam,
> 
> Shall I consider this an Ack and push Andrew's new changes
> up while waiting for test cases?

Sure, the intent looks fine and given that Andrew has likely been using
it a lot it's probably right.  So yes, that's an ack.  Hopefully I'll
find time to push along some tests soon.

Cheers,
Sam

^ permalink raw reply

* Re: [PATCH 1/2] git-svn: ignore changeless commits when checking for a cherry-pick
From: Eric Wong @ 2010-01-07  9:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sam, Andrew Myrick
In-Reply-To: <1262823922-3415-1-git-send-email-amyrick@apple.com>

Andrew Myrick <amyrick@apple.com> wrote:
> Update git-svn to ignore commits that do not change the tree when it is
> deciding if an svn merge ticket represents a real branch merge or just a
> cherry-pick.
> 
> Consider the following integration model in the svn repository:
> 
>    F---G  branch1
>   /     \
>  D  tag1 \   E  tag2
> /         \ /
> A---B      C  trunk
> 
> branch1 is merged to trunk in commit C.
> 
> With this patch, git-svn will correctly identify branch1 as a proper merge
> parent, instead of incorrectly ignoring it as a cherry-pick.
> 
> Signed-off-by: Andrew Myrick <amyrick@apple.com>
> ---

Thanks Andrew,

I've added Acks for Sam and myself for this series and pushed out to
git://git.bogomips.org/git-svn:

Andrew Myrick (2):
      git-svn: ignore changeless commits when checking for a cherry-pick
      git-svn: handle merge-base failures

-- 
Eric Wong

^ permalink raw reply

* Re: submodules' shortcomings, was Re: RFC: display dirty submodule working directory in git gui and gitk
From: Nanako Shiraishi @ 2010-01-07 11:04 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Heiko Voigt, Jens Lehmann, Git Mailing List, Junio C Hamano,
	Shawn O. Pearce, Paul Mackerras, Lars Hjemli, Avery Pennarun
In-Reply-To: <alpine.DEB.1.00.1001060009530.4985@pacific.mpi-cbg.de>

Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Wed, 6 Jan 2010, Nanako Shiraishi wrote:
>
>> I found this other discussion in the design area enlightening.
>> 
>> http://thread.gmane.org/gmane.comp.version-control.git/47466/focus=47621
>
> Could you be so kind and summarize the result of the thread in something 
> like 2000 characters?

Sorry, but I only said "enlightening". There wasn't a conclusion that lets you stop thinking and just go ahead implementing the design specified in the thread, if that is what you are looking for.

Instead, let me tell you an example of what I found enlightening. It isn't a summary of the result. I don't think there was a *result*; otherwise somebody already would have implemented it.

I often wonder why 'git-submodule init' copies data to .git/config file. If .gitmodules file gives the default and I can use .git/config file to override it, it seems stupid to copy entries between these files. I can just keep using data from .gitmodules file until I need to override something.

Reading the thread made me realize how wrong I was. It became very clear why .gitmodules file shouldn't even be the default that is read when no entries is in .git/config file and why .git/config file should be the only thing that is used at runtime.

Unfortunately I can't summarize the reason in '2000 characters', so you need read the thread yourself if you are interested. The key concept that I was missing was that remote repositories can move or change over time, and you may want to check out and interact with a very old version of your supermodule. The .gitmodules file checked out in such a case still records old information. Treating .gitmodules file as a hint and always looking into .git/config file is a part of the fundamental solution to that problem, but I didn't even realize that such an issue existed when I read the current discussion until I found the old thread.

I think the 'git-submodule' script is mainly based on the 'three-level thing Steven Grimm suggested', but it doesn't seem to implement all the ideas in the thread yet. It gives no interactive prompt to suggest URL from 'git-submodule init' command. Neither it records which URLs have been seen with subproject.*.seen variable. But the issues that high level design must take into account looks very well thought out already.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* [PATCH (v2) 1/2] rebase: fix --onto A...B parsing and add tests
From: Nanako Shiraishi @ 2010-01-07 11:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vocl7yxef.fsf@alter.siamese.dyndns.org>

The previous patch didn't parse "rebase --onto A...B" correctly when A
isn't an empty string. It also tried to be careful to notice a case in
which there are more than one merge bases, but forgot to give --all option
to merge-base, making the test pointless.

Fix these problems and add a test script to verify. Improvements to the
script to parse A...B syntax was taken from review comments by Johannes
Schindelin.

Signed-off-by: しらいし ななこ <nanako3@lavabit.com>
---
 git-rebase.sh                    |   33 ++++++++++-------
 t/t3415-rebase-onto-threedots.sh |   75 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 14 deletions(-)
 create mode 100755 t/t3415-rebase-onto-threedots.sh

diff --git a/git-rebase.sh b/git-rebase.sh
index 6503113..9bd8974 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -419,22 +419,27 @@ fi
 
 # Make sure the branch to rebase onto is valid.
 onto_name=${newbase-"$upstream_name"}
-if	left=$(expr "$onto_name" : '\(.*\)\.\.\.') &&
-	right=$(expr "$onto_name" : '\.\.\.\(.*\)$') &&
-	: ${left:=HEAD} ${right:=HEAD} &&
-	onto=$(git merge-base "$left" "$right")
-then
-	case "$onto" in
-	?*"$LF"?*)
-		die "$onto_name: there are more than one merge bases"
-		;;
-	'')
+case "$onto_name" in
+*...*)
+	if	left=${onto_name%...*} right=${onto_name#*...} &&
+		onto=$(git merge-base --all ${left:-HEAD} ${right:-HEAD})
+	then
+		case "$onto" in
+		?*"$LF"?*)
+			die "$onto_name: there are more than one merge bases"
+			;;
+		'')
+			die "$onto_name: there is no merge base"
+			;;
+		esac
+	else
 		die "$onto_name: there is no merge base"
-		;;
-	esac
-else
+	fi
+	;;
+*)
 	onto=$(git rev-parse --verify "${onto_name}^0") || exit
-fi
+	;;
+esac
 
 # If a hook exists, give it a chance to interrupt
 run_pre_rebase_hook "$upstream_arg" "$@"
diff --git a/t/t3415-rebase-onto-threedots.sh b/t/t3415-rebase-onto-threedots.sh
new file mode 100755
index 0000000..da378c4
--- /dev/null
+++ b/t/t3415-rebase-onto-threedots.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='git rebase --onto A...B'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-rebase.sh"
+
+# Rebase only the tip commit of "topic" on merge base between "master"
+# and "topic".  Cannot do this for "side" with "master" because there
+# is no single merge base.
+#
+#
+#	    F---G topic                             G'
+#	   /                                       /
+# A---B---C---D---E master      -->       A---B---C---D---E
+#      \   \ /
+#	\   x
+#	 \ / \ 
+#	  H---I---J---K side
+
+test_expect_success setup '
+	test_commit A &&
+	test_commit B &&
+	git branch side &&
+	test_commit C &&
+	git branch topic &&
+	git checkout side &&
+	test_commit H &&
+	git checkout master &&
+	test_tick &&
+	git merge H &&
+	git tag D &&
+	test_commit E &&
+	git checkout topic &&
+	test_commit F &&
+	test_commit G &&
+	git checkout side &&
+	test_tick &&
+	git merge C &&
+	git tag I &&
+	test_commit J &&
+	test_commit K
+'
+
+test_expect_success 'rebase --onto master...topic' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+
+	git rebase --onto master...topic F &&
+	git rev-parse HEAD^1 >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase --onto master...' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+
+	git rebase --onto master... F &&
+	git rev-parse HEAD^1 >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase --onto master...side' '
+	git reset --hard &&
+	git checkout side &&
+	git reset --hard K &&
+
+	test_must_fail git rebase --onto master...side J
+'
+
+test_done
-- 
1.6.6.53.g75f61




-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply related

* [PATCH (v2) 2/2] rebase -i: teach --onto A...B syntax
From: Nanako Shiraishi @ 2010-01-07 11:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vocl7yxef.fsf@alter.siamese.dyndns.org>

When rewriting commits on a topic branch, sometimes it is easier to
compare the version of commits before and after the rewrite if they are
based on the same commit that forked from the upstream. An earlier commit
by Junio (fixed up by the previous commit) gives "--onto A...B" syntax to
rebase command, and rebases on top of the merge base between A and B;
teach the same to the interactive version, too.

Signed-off-by: しらいし ななこ <nanako3@lavabit.com>
---
 git-rebase--interactive.sh       |   21 ++++++++++++++++++++-
 t/t3415-rebase-onto-threedots.sh |   30 ++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..f7ae02c 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -482,6 +482,25 @@ get_saved_options () {
 	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
 }
 
+LF='
+'
+parse_onto () {
+	case "$1" in
+	*...*)
+		if	left=${1%...*} right=${1#*...} &&
+			onto=$(git merge-base --all ${left:-HEAD} ${right:-HEAD})
+		then
+			case "$onto" in
+			?*"$LF"?* | '')
+				exit 1 ;;
+			esac
+			echo "$onto"
+			exit 0
+		fi
+	esac
+	git rev-parse --verify "$1^0"
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -589,7 +608,7 @@ first and then run 'git rebase --continue' again."
 		;;
 	--onto)
 		shift
-		ONTO=$(git rev-parse --verify "$1") ||
+		ONTO=$(parse_onto "$1") ||
 			die "Does not point to a valid commit: $1"
 		;;
 	--)
diff --git a/t/t3415-rebase-onto-threedots.sh b/t/t3415-rebase-onto-threedots.sh
index da378c4..5e7eb88 100755
--- a/t/t3415-rebase-onto-threedots.sh
+++ b/t/t3415-rebase-onto-threedots.sh
@@ -72,4 +72,34 @@ test_expect_success 'rebase --onto master...side' '
 	test_must_fail git rebase --onto master...side J
 '
 
+test_expect_success 'rebase -i --onto master...topic' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+	set_fake_editor &&
+	EXPECT_COUNT=1 git rebase -i --onto master...topic F &&
+	git rev-parse HEAD^1 >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase -i --onto master...' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+	set_fake_editor &&
+	EXPECT_COUNT=1 git rebase -i --onto master... F &&
+	git rev-parse HEAD^1 >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase -i --onto master...side' '
+	git reset --hard &&
+	git checkout side &&
+	git reset --hard K &&
+
+	test_must_fail git rebase -i --onto master...side J
+'
+
 test_done
-- 
1.6.6.53.g75f61




-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply related

* Difference between pull --rebase and fetch+rebase
From: martinvz @ 2010-01-07 12:23 UTC (permalink / raw)
  To: git


I have a branch configured to track a remote branch by rebasing. I excepted
that "git pull" would therefore be equivalent to fetching from the remote
repository followed by rebasing the remote branch, but it isn't. When doing
"git rebase <remote>/<branch>", it applies only the commits after the merge
base. When doing "git pull", it tries to apply two more commits (the two
commits preceding the merge base). Why is this?

I get the same result even if I do "git pull --rebase <remote> <branch>", it
doesn't seem to have anything to do with incorrect configuration of the
branch.
-- 
View this message in context: http://old.nabble.com/Difference-between-pull---rebase-and-fetch%2Brebase-tp27059158p27059158.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: Difference between pull --rebase and fetch+rebase
From: martinvz @ 2010-01-07 12:58 UTC (permalink / raw)
  To: git
In-Reply-To: <27059158.post@talk.nabble.com>


Forgot to say that I am using version 1.6.5.1.1367.gcd48 (Cygwin). I should
also say that I can see no particular reason why exactly two extra commits
were picked.


martinvz wrote:
> 
> I have a branch configured to track a remote branch by rebasing. I
> excepted that "git pull" would therefore be equivalent to fetching from
> the remote repository followed by rebasing the remote branch, but it
> isn't. When doing "git rebase <remote>/<branch>", it applies only the
> commits after the merge base. When doing "git pull", it tries to apply two
> more commits (the two commits preceding the merge base). Why is this?
> 
> I get the same result even if I do "git pull --rebase <remote> <branch>",
> it doesn't seem to have anything to do with incorrect configuration of the
> branch.
> 

-- 
View this message in context: http://old.nabble.com/Difference-between-pull---rebase-and-fetch%2Brebase-tp27059158p27059574.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH v2 0/5] Lazily generate header dependencies
From: Erik Faye-Lund @ 2010-01-07 13:22 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Nanako Shiraishi, Junio C Hamano, Johannes Sixt, Git Mailing List,
	Johannes Schindelin
In-Reply-To: <20100107071305.GA11777@progeny.tock>

On Thu, Jan 7, 2010 at 8:13 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nanako Shiraishi wrote:
>> Junio, could you tell us what happened to this thread?
>>
>> Makefile improvements.  No discussion.
>
> My bad.  The previous version was very rough because I was not sure
> yet how this could help in making the header dependency rules more
> maintainable.  If all compilers worth using support something like
> gcc's -MD option (does MSVC?), we could switch over completely;
> otherwise, we need some way to use the generated dependencies to
> check the static ones, or the static ones will go stale.

Nope, there's no support for -MD in MSVC. It does have an "/MD"
option, but it means something completely different (link with
multithreaded DLL CRT). There IS the "/showIncludes" option [1], which
should make it possible to do some build-magic to generate the correct
dependency-files, though.

[1] http://msdn.microsoft.com/en-us/library/hdkef6tk(VS.71).aspx

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* Re: [PATCH 1/2] t7002: set test prerequisite "external-grep" if  supported
From: Nguyen Thai Ngoc Duy @ 2010-01-07 13:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy6ka8rmr.fsf@alter.siamese.dyndns.org>

On 1/7/10, Junio C Hamano <gitster@pobox.com> wrote:
>  So by writing the test to check the desired outcome, instead of writing it
>  for the particular implementation of using external grep optimization, you
>  will catch both kinds of breakages.
>
>  Perhaps something like this (untested, of course)?
>
>  test_expect_success 'strings in work tree files are not found for skip-wt paths' '
>         no="no such string in the index" &&
>         test_must_fail git grep -e "$no" --cached file &&
>         git update-index --skip-worktree file &&
>         echo "$no" >file &&
>         test_must_fail git grep -e "$no" file &&
>         git update-index --no-skip-worktree file &&
>         git grep -e "$no" file
>  '
>

Very well reasoned. I'd say go for it!

Tested-by: me
-- 
Duy

^ permalink raw reply

* Re: [PATCH 1/2] t7002: set test prerequisite "external-grep" if  supported
From: Johannes Sixt @ 2010-01-07 14:04 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <fcaeb9bf1001070527ra1570a0m12182db9bb029055@mail.gmail.com>

Nguyen Thai Ngoc Duy schrieb:
> On 1/7/10, Junio C Hamano <gitster@pobox.com> wrote:
>>  So by writing the test to check the desired outcome, instead of writing it
>>  for the particular implementation of using external grep optimization, you
>>  will catch both kinds of breakages.
>>
>>  Perhaps something like this (untested, of course)?
>>
>>  test_expect_success 'strings in work tree files are not found for skip-wt paths' '
>>         no="no such string in the index" &&
>>         test_must_fail git grep -e "$no" --cached file &&
>>         git update-index --skip-worktree file &&
>>         echo "$no" >file &&
>>         test_must_fail git grep -e "$no" file &&
>>         git update-index --no-skip-worktree file &&
>>         git grep -e "$no" file
>>  '
>>
> 
> Very well reasoned. I'd say go for it!
> 
> Tested-by: me

The test is not quite complete. Not only do you want to test that the
worktree file is not looked at, but that the index version is used:


test_expect_success 'for skip-wt paths, strings are found in index, not in
worktree' '
	yes="this string is in the index" &&
	no="no such string in the index" &&
	echo "$yes" >file &&
	git update-index file &&
	echo "$no" >file &&
	git grep -e "$yes" --cached file &&
	test_must_fail git grep -e "$no" --cached file &&
	git update-index --skip-worktree file &&
	git grep -e "$yes" file &&
	test_must_fail git grep -e "$no" file &&
	git update-index --no-skip-worktree file &&
	test_must_fail git grep -e "$yes" file &&
	git grep -e "$no" file
'

Just as untested... ;)

-- Hannes

^ permalink raw reply

* Re: [PATCH 1/2] t7002: set test prerequisite "external-grep" if  supported
From: Nguyen Thai Ngoc Duy @ 2010-01-07 14:26 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4B45E9E7.1040905@viscovery.net>

On 1/7/10, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nguyen Thai Ngoc Duy schrieb:
>
> > On 1/7/10, Junio C Hamano <gitster@pobox.com> wrote:
>  >>  So by writing the test to check the desired outcome, instead of writing it
>  >>  for the particular implementation of using external grep optimization, you
>  >>  will catch both kinds of breakages.
>  >>
>  >>  Perhaps something like this (untested, of course)?
>  >>
>  >>  test_expect_success 'strings in work tree files are not found for skip-wt paths' '
>  >>         no="no such string in the index" &&
>  >>         test_must_fail git grep -e "$no" --cached file &&
>  >>         git update-index --skip-worktree file &&
>  >>         echo "$no" >file &&
>  >>         test_must_fail git grep -e "$no" file &&
>  >>         git update-index --no-skip-worktree file &&
>  >>         git grep -e "$no" file
>  >>  '
>  >>
>  >
>  > Very well reasoned. I'd say go for it!
>  >
>  > Tested-by: me
>
>
> The test is not quite complete. Not only do you want to test that the
>  worktree file is not looked at, but that the index version is used:
>
>
>  test_expect_success 'for skip-wt paths, strings are found in index, not in
>  worktree' '
>         yes="this string is in the index" &&
>
>         no="no such string in the index" &&
>
>         echo "$yes" >file &&
>         git update-index file &&
>         echo "$no" >file &&
>         git grep -e "$yes" --cached file &&
>
>         test_must_fail git grep -e "$no" --cached file &&
>         git update-index --skip-worktree file &&
>
>         git grep -e "$yes" file &&
>
>         test_must_fail git grep -e "$no" file &&
>         git update-index --no-skip-worktree file &&
>
>         test_must_fail git grep -e "$yes" file &&
>
>         git grep -e "$no" file
>  '

Can we get rid of preparing $yes and do "grep -e foo file" instead?
There are lots of foo from setup test. It's not as strict as your test
because foo is also in worktree. But we have $no for testing worktree
already.
-- 
Duy

^ permalink raw reply

* [PATCH] lockfile: show absolute filename in unable_to_lock_message.
From: Matthieu Moy @ 2010-01-07 14:54 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <4B44DB6A.2050804@kdbg.org>

When calling a git command from a subdirectory and a file locking fails,
the user will get a path relative to the root of the worktree, which is
invalid from the place where the command is ran. Make it easy for the
user to know which file it is.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
> Since these are merely informative, wouldn't it be more suitable to
> use make_nonrelative_path()?

Sensible, yes.

 lockfile.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 6851fa5..b0d74cd 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -164,9 +164,10 @@ static char *unable_to_lock_message(const char *path, int err)
 		    "If no other git process is currently running, this probably means a\n"
 		    "git process crashed in this repository earlier. Make sure no other git\n"
 		    "process is running and remove the file manually to continue.",
-		    path, strerror(err));
+			    make_nonrelative_path(path), strerror(err));
 	} else
-		strbuf_addf(&buf, "Unable to create '%s.lock': %s", path, strerror(err));
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s",
+			    make_nonrelative_path(path), strerror(err));
 	return strbuf_detach(&buf, NULL);
 }
 
-- 
1.6.6.81.gda96d0

^ permalink raw reply related

* base85: Two tiny fixes
From: Andreas Gruenbacher @ 2010-01-07 14:58 UTC (permalink / raw)
  To: git

While looking at the base85 code I found a bug in the debug code and an 
unnecessary call.  You may want to have a look at the two fixes here:

  http://www.kernel.org/pub/scm/linux/kernel/git/agruen/git.git

There is another little oddity in the way the de85 table is set up: 0 
indicates an invalid entry; to avoid this from clashing with a valid entry, 
valid entries are incremented by one and decremented again while decoding.  
This leads to slightly worse code than using a negative number to indicate 
invalid values (and avoiding to increment/decrement).

Andreas

^ permalink raw reply

* Re: [PATCH v2] Be more user-friendly when refusing to do something because of conflict.
From: Matthieu Moy @ 2010-01-07 15:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vskajq716.fsf@alter.siamese.dyndns.org>

Thanks for your detailed feedback.

Junio C Hamano <gitster@pobox.com> writes:

> "The new output looks like this, which is much better..." is missing
> here.

Added.

> One advice, "use add/rm as appropriate to mark resolution" goes only as
> far as making the index in order, without recording it in a commit.  The
> other, "commit -a", will make a commit, I suspect that "commit -a" needs
> to be matched with "commit" if the user chooses to take the first advice
> of resolving paths incrementally.  IOW
>
>     use 'git add/rm <file>' as appropriate to mark resolution and make a
>     commit, or use 'commit -a', before running me again.
>
> might be more appropriate.

Applied.

The same sentence has a slightly different meaning for commit and
merge. For commit "make a commit" means "redo what you tried to do",
and for merge, it means "make a commit before you redo what you tried
to do".

>> +static void refresh_cache_or_die(int refresh_flags)
>> +{
>> +	/*
>> +	 * refresh_flags contains REFRESH_QUIET, so the only errors
>> +	 * are for unmerged entries.
>> +	*/
>
> Mixed indentation.

fixed.

>> +	if(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {
>
> SP after "if".

fixed (together with the useless brace which you had missed ;-) ).

> What should we see upon "commit --dry-run" and what does the code after
> the patch produce?

Good remark. Just tried, it works without option, with -a, -o, -i.
They all give the usual status output, except -o which complains:

$ git commit --dry-run -o foo.txt
fatal: cannot do a partial commit during a merge.

(as before)

> Are we sure refresh_flags always lack REFRESH_UNMERGED that allows
> unmerged entries and produces the unmerged error messages when
> needed?

Argument from intimidation:
If they hadn't, we would have noticed the output (foo.txt: needs
merge) before the patch.

Rational argument:
prepare_index starts with this:

	if (is_status)
		refresh_flags |= REFRESH_UNMERGED;

and each call to refresh_cache_or_die is within prepare_index, with
these flags, untouched.

>> -	if (file_exists(git_path("MERGE_HEAD"))) [...]
>> -	if (read_cache_unmerged()) [...]
>> +	if (read_cache_unmerged()) {
>> +		die_resolve_conflict("merge");
>> +	}
>> +	if (file_exists(git_path("MERGE_HEAD"))) {
>> +		if (advice_resolve_conflict)
>> +			die("You have not concluded your merge (MERGE_HEAD exists).\n"
>> +			    "Please, commit your changes before you can merge.");
>> +		else
>> +			die("You have not concluded your merge (MERGE_HEAD exists).");
>> +	}
>
> It is not a very big deal, but why are these checked in different order
> after the patch?
[...]
> Note that the user might have already run "add -u" to mark everything
> resolved, in which case MERGE_HEAD will still exist even though the index
> is free of ummerged entries.

That's precisely the reason. You hardly have conflicts without a
MERGE_HEAD, but it's sensible to have a MERGE_HEAD without unmerged
entries in the index, which means you're one step closer to the
commit.

I've added a comment (not a commit message) to make it clear.

> Nice.  Maybe we want to handle other cases like:
>
> 	$ git rebase master
>         ... conflicted

(if it's conflicted, it's OK, same user-friendly message. If you
already git add-ed your conflicts, git pull will run normally :-( ).

>         ... called away for a 30-minutes meeting
>         ... forgot the user was in the middle of the rebase
>         $ git pull
>
> and the "pull" refused to run because the earlier "rebase" hasn't been
> concluded (I suspect an earlier "am" failure would be the same issue).

Yes, but I think that's a separate topic (and I'm getting short in
time budget). This will also apply to many commands (pull,
merge, ...), including shell and C. It would probably be good to have
a C function like can_I_start_merge() exposed as a plumbing command to
be used by git-pull.sh.

And I'm wondering whether the ability to run pull in the middle of a
rebase is a bug or a feature. Never did that, but I can imagine
someone doing a 'rebase -i' with an 'edit' command to let rebase stop,
do a merge, and then 'rebase --continue', to insert a merge commit in
the middle of a patch serie.

Patch follows. Fyi the difference with v3 are:

diff --git a/advice.c b/advice.c
index ec2bd82..3309521 100644
--- a/advice.c
+++ b/advice.c
@@ -33,9 +33,13 @@ int git_default_advice_config(const char *var, const char *value)
 void NORETURN die_resolve_conflict(const char *me)
 {
        if (advice_resolve_conflict)
+               /*
+                * Message used both when 'git commit' fails and when
+                * other commands doing a merge do.
+                */
                die("'%s' is not possible because you have unmerged files.\n"
-                   "Please, fix them up in the work tree, and then use 'git add/rm <file>'\n"
-                   "as appropriate to mark resolution, or use 'git commit -a'.", me);
+                   "Please, fix them up in the work tree, and then use 'git add/rm <file>' as\n"
+                   "appropriate to mark resolution and make a commit, or use 'git commit -a'.", me);
        else
                die("'%s' is not possible because you have unmerged files.", me);
 }
diff --git a/builtin-commit.c b/builtin-commit.c
index a4977ac..b3b37c2 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -240,8 +240,8 @@ static void refresh_cache_or_die(int refresh_flags)
        /*
         * refresh_flags contains REFRESH_QUIET, so the only errors
         * are for unmerged entries.
-       */
-       if(refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {
+        */
+       if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN)) {
                die_resolve_conflict("commit");
        }
 }
diff --git a/builtin-merge.c b/builtin-merge.c
index abe6c03..79a35c3 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -851,6 +851,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
                die_resolve_conflict("merge");
        }
        if (file_exists(git_path("MERGE_HEAD"))) {
+               /*
+                * There is no unmerged entry, don't advise 'git
+                * add/rm <file>', just 'git commit'.
+                */
                if (advice_resolve_conflict)
                        die("You have not concluded your merge (MERGE_HEAD exists).\n"
                            "Please, commit your changes before you can merge.");

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply related

* [PATCH v4] Be more user-friendly when refusing to do something because of conflict.
From: Matthieu Moy @ 2010-01-07 15:35 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpqiqbeornm.fsf@bauges.imag.fr>

Various commands refuse to run in the presence of conflicts (commit,
merge, pull, cherry-pick/revert). They all used to provide rough, and
inconsistant error messages.

A new variable advice.resolveconflict is introduced, and allows more
verbose messages, pointing the user to the appropriate solution.

For commit, the error message used to look like this:

$ git commit
foo.txt: needs merge
foo.txt: unmerged (c34a92682e0394bc0d6f4d4a67a8e2d32395c169)
foo.txt: unmerged (3afcd75de8de0bb5076942fcb17446be50451030)
foo.txt: unmerged (c9785d77b76dfe4fb038bf927ee518f6ae45ede4)
error: Error building trees

The "need merge" line is given by refresh_cache. We add the IN_PORCELAIN
option to make the output more consistant with the other porcelain
commands, and catch the error in return, to stop with a clean error
message. The next lines were displayed by a call to cache_tree_update(),
which is not reached anymore if we noticed the conflict.

The new output looks like:

U       foo.txt
fatal: 'commit' is not possible because you have unmerged files.
Please, fix them up in the work tree, and then use 'git add/rm <file>' as
appropriate to mark resolution and make a commit, or use 'git commit -a'.

Pull is slightly modified to abort immediately if $GIT_DIR/MERGE_HEAD
exists instead of waiting for merge to complain.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |    4 ++++
 advice.c                 |   16 ++++++++++++++++
 advice.h                 |    5 +++++
 builtin-commit.c         |   14 ++++++++++++--
 builtin-merge.c          |   19 ++++++++++++++-----
 builtin-revert.c         |   15 ++++++++++++++-
 git-pull.sh              |   25 +++++++++++++++++++++++--
 7 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 23a965e..5078d26 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -130,6 +130,10 @@ advice.*::
 		Advice shown when linkgit:git-merge[1] refuses to
 		merge to avoid overwritting local changes.
 		Default: true.
+	resolveConflict::
+		Advices shown by various commands when conflicts
+		prevent the operation from being performed. 
+		Default: true.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index cb666ac..3309521 100644
--- a/advice.c
+++ b/advice.c
@@ -3,6 +3,7 @@
 int advice_push_nonfastforward = 1;
 int advice_status_hints = 1;
 int advice_commit_before_merge = 1;
+int advice_resolve_conflict = 1;
 
 static struct {
 	const char *name;
@@ -11,6 +12,7 @@ static struct {
 	{ "pushnonfastforward", &advice_push_nonfastforward },
 	{ "statushints", &advice_status_hints },
 	{ "commitbeforemerge", &advice_commit_before_merge },
+	{ "resolveconflict", &advice_resolve_conflict },
 };
 
 int git_default_advice_config(const char *var, const char *value)
@@ -27,3 +29,17 @@ int git_default_advice_config(const char *var, const char *value)
 
 	return 0;
 }
+
+void NORETURN die_resolve_conflict(const char *me)
+{
+	if (advice_resolve_conflict)
+		/*
+		 * Message used both when 'git commit' fails and when
+		 * other commands doing a merge do.
+		 */
+		die("'%s' is not possible because you have unmerged files.\n"
+		    "Please, fix them up in the work tree, and then use 'git add/rm <file>' as\n"
+		    "appropriate to mark resolution and make a commit, or use 'git commit -a'.", me);
+	else
+		die("'%s' is not possible because you have unmerged files.", me);
+}
diff --git a/advice.h b/advice.h
index 3de5000..acd5fdd 100644
--- a/advice.h
+++ b/advice.h
@@ -1,10 +1,15 @@
 #ifndef ADVICE_H
 #define ADVICE_H
 
+#include "git-compat-util.h"
+
 extern int advice_push_nonfastforward;
 extern int advice_status_hints;
 extern int advice_commit_before_merge;
+extern int advice_resolve_conflict;
 
 int git_default_advice_config(const char *var, const char *value);
 
+extern void NORETURN die_resolve_conflict(const char *me);
+
 #endif /* ADVICE_H */
diff --git a/builtin-commit.c b/builtin-commit.c
index 3dfcd77..6075a59 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -235,6 +235,16 @@ static void create_base_index(void)
 		exit(128); /* We've already reported the error, finish dying */
 }
 
+static void refresh_cache_or_die(int refresh_flags)
+{
+	/*
+	 * refresh_flags contains REFRESH_QUIET, so the only errors
+	 * are for unmerged entries.
+	 */
+	if (refresh_cache(refresh_flags | REFRESH_IN_PORCELAIN))
+		die_resolve_conflict("commit");
+}
+
 static char *prepare_index(int argc, const char **argv, const char *prefix, int is_status)
 {
 	int fd;
@@ -274,7 +284,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
 	if (all || (also && pathspec && *pathspec)) {
 		int fd = hold_locked_index(&index_lock, 1);
 		add_files_to_cache(also ? prefix : NULL, pathspec, 0);
-		refresh_cache(refresh_flags);
+		refresh_cache_or_die(refresh_flags);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close_lock_file(&index_lock))
 			die("unable to write new_index file");
@@ -293,7 +303,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
 	 */
 	if (!pathspec || !*pathspec) {
 		fd = hold_locked_index(&index_lock, 1);
-		refresh_cache(refresh_flags);
+		refresh_cache_or_die(refresh_flags);
 		if (write_cache(fd, active_cache, active_nr) ||
 		    commit_locked_index(&index_lock))
 			die("unable to write new_index file");
diff --git a/builtin-merge.c b/builtin-merge.c
index f1c84d7..79a35c3 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -847,11 +847,20 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	const char *best_strategy = NULL, *wt_strategy = NULL;
 	struct commit_list **remotes = &remoteheads;
 
-	if (file_exists(git_path("MERGE_HEAD")))
-		die("You have not concluded your merge. (MERGE_HEAD exists)");
-	if (read_cache_unmerged())
-		die("You are in the middle of a conflicted merge."
-				" (index unmerged)");
+	if (read_cache_unmerged()) {
+		die_resolve_conflict("merge");
+	}
+	if (file_exists(git_path("MERGE_HEAD"))) {
+		/*
+		 * There is no unmerged entry, don't advise 'git
+		 * add/rm <file>', just 'git commit'.
+		 */
+		if (advice_resolve_conflict)
+			die("You have not concluded your merge (MERGE_HEAD exists).\n"
+			    "Please, commit your changes before you can merge.");
+		else
+			die("You have not concluded your merge (MERGE_HEAD exists).");
+	}
 
 	/*
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
diff --git a/builtin-revert.c b/builtin-revert.c
index 151aa6a..d14dde3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -233,6 +233,19 @@ static struct tree *empty_tree(void)
 	return tree;
 }
 
+static NORETURN void die_dirty_index(const char *me)
+{
+	if (read_cache_unmerged()) {
+		die_resolve_conflict(me);
+	} else {
+		if (advice_commit_before_merge)
+			die("Your local changes would be overwritten by %s.\n"
+			    "Please, commit your changes or stash them to proceed.", me);
+		else
+			die("Your local changes would be overwritten by %s.\n", me);
+	}
+}
+
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	unsigned char head[20];
@@ -269,7 +282,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		if (get_sha1("HEAD", head))
 			die ("You do not have a valid HEAD");
 		if (index_differs_from("HEAD", 0))
-			die ("Dirty index: cannot %s", me);
+			die_dirty_index(me);
 	}
 	discard_cache();
 
diff --git a/git-pull.sh b/git-pull.sh
index 9e69ada..54ce0af 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -13,8 +13,29 @@ set_reflog_action "pull $*"
 require_work_tree
 cd_to_toplevel
 
-test -z "$(git ls-files -u)" ||
-	die "You are in the middle of a conflicted merge."
+
+die_conflict () {
+    git diff-index --cached --name-status -r --ignore-submodules HEAD --
+    if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
+	die "Pull is not possible because you have unmerged files.
+Please, fix them up in the work tree, and then use 'git add/rm <file>'
+as appropriate to mark resolution, or use 'git commit -a'."
+    else
+	die "Pull is not possible because you have unmerged files."
+    fi
+}
+
+die_merge () {
+    if [ $(git config --bool --get advice.resolveConflict || echo true) = "true" ]; then
+	die "You have not concluded your merge (MERGE_HEAD exists).
+Please, commit your changes before you can merge."
+    else
+	die "You have not concluded your merge (MERGE_HEAD exists)."
+    fi
+}
+
+test -z "$(git ls-files -u)" || die_conflict
+test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity=
-- 
1.6.6.81.g872e.dirty

^ permalink raw reply related

* Re: Difference between pull --rebase and fetch+rebase
From: Santi Béjar @ 2010-01-07 16:00 UTC (permalink / raw)
  To: martinvz; +Cc: git
In-Reply-To: <27059158.post@talk.nabble.com>

On Thu, Jan 7, 2010 at 1:23 PM, martinvz
<martin.von.zweigbergk@gmail.com> wrote:
>
> I have a branch configured to track a remote branch by rebasing. I excepted
> that "git pull" would therefore be equivalent to fetching from the remote
> repository followed by rebasing the remote branch, but it isn't. When doing
> "git rebase <remote>/<branch>", it applies only the commits after the merge
> base. When doing "git pull", it tries to apply two more commits (the two
> commits preceding the merge base). Why is this?
>
> I get the same result even if I do "git pull --rebase <remote> <branch>", it
> doesn't seem to have anything to do with incorrect configuration of the
> branch.

Yes, both should do the same (at least when upstream is not rebased).
Can you provide a test case or instructions to reproduce the behavior?

Thanks,
Santi

^ permalink raw reply

* [StGit PATCH 1/2] Rename the mail --refid and --noreply options to match Git
From: Catalin Marinas @ 2010-01-07 16:09 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git

The new option names are --in-reply-to and --no-thread.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---
 stgit/commands/mail.py |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 8ff0bd4..a78c9d2 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -56,7 +56,7 @@ specified file (defaulting to '.git/covermail.tmpl' or
 
 All the subsequent e-mails appear as replies to the first e-mail sent
 (either the preamble or the first patch). E-mails can be seen as
-replies to a different e-mail by using the '--refid' option.
+replies to a different e-mail by using the '--in-reply-to' option.
 
 SMTP authentication is also possible with '--smtp-user' and
 '--smtp-password' options, also available as configuration settings:
@@ -107,7 +107,7 @@ options = [
         short = 'Add BCC to the Bcc: list'),
     opt('--auto', action = 'store_true',
         short = 'Automatically cc the patch signers'),
-    opt('--noreply', action = 'store_true',
+    opt('--no-thread', action = 'store_true',
         short = 'Do not send subsequent messages as replies'),
     opt('--unrelated', action = 'store_true',
         short = 'Send patches without sequence numbering'),
@@ -127,7 +127,7 @@ options = [
         short = 'Edit each patch before sending'),
     opt('-s', '--sleep', type = 'int', metavar = 'SECONDS',
         short = 'Sleep for SECONDS between e-mails sending'),
-    opt('--refid',
+    opt('--in-reply-to', metavar = 'REFID',
         short = 'Use REFID as the reference id'),
     opt('--smtp-server', metavar = 'HOST[:PORT] or "/path/to/sendmail -t -i"',
         short = 'SMTP server or command to use for sending mail'),
@@ -495,7 +495,7 @@ def __build_cover(tmpl, msg_id, options, patches):
 
     if not options.git:
         __build_address_headers(msg, options)
-    __build_extra_headers(msg, msg_id, options.refid)
+    __build_extra_headers(msg, msg_id, options.in_reply_to)
     __encode_message(msg)
 
     return msg
@@ -638,11 +638,11 @@ def func(parser, options, args):
     if total_nr == 0:
         raise CmdException, 'No patches to send'
 
-    if options.refid:
-        if options.noreply or options.unrelated:
+    if options.in_reply_to:
+        if options.no_thread or options.unrelated:
             raise CmdException, \
-                  '--refid option not allowed with --noreply or --unrelated'
-        ref_id = options.refid
+                  '--in-reply-to option not allowed with --no-thread or --unrelated'
+        ref_id = options.in_reply_to
     else:
         ref_id = None
 
@@ -663,7 +663,7 @@ def func(parser, options, args):
         msg_id = __send_message('cover', tmpl, options, patches)
 
         # subsequent e-mails are seen as replies to the first one
-        if not options.noreply:
+        if not options.no_thread:
             ref_id = msg_id
 
     # send the patches
@@ -681,5 +681,5 @@ def func(parser, options, args):
         msg_id = __send_message('patch', tmpl, options, p, n, total_nr, ref_id)
 
         # subsequent e-mails are seen as replies to the first one
-        if not options.noreply and not options.unrelated and not ref_id:
+        if not options.no_thread and not options.unrelated and not ref_id:
             ref_id = msg_id

^ permalink raw reply related


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