Git development
 help / color / mirror / Atom feed
* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
From: SZEDER Gábor @ 2010-01-31 19:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, David Rhodes Clymer, git
In-Reply-To: <20100129200431.GE22101@spearce.org>

On Fri, Jan 29, 2010 at 12:04:31PM -0800, Shawn O. Pearce wrote:
> SZEDER G?bor <szeder@ira.uka.de> wrote:
> > 
> > _git_lgm () {
> >         _git_log
> > }
> > 
> > Unfortunately, it doesn't work at all.
> > 
> > In _git() first we have 'lgm' in $command, which is ok, but then comes
> > this alias handling thing
> > 
> >         local expansion=$(__git_aliased_command "$command")
> >         [ "$expansion" ] && command="$expansion"
> > 
> > which writes '!sh' into $command, and that doesn't look quite right
> 
> __git_aliased_command is returning the first word out of the alias.

Actually, it returns the first word from the alias which does not
start with a dash.  It behaves this way since its introduction in
367dce2a (Bash completion support for aliases, 2006-10-28).  I'm not
sure what the original intent was behind ignoring words starting with
a dash, but it gave me some ideas.

> I think we need to change this block here to:
> 
>   case "$expansion" of
>   \!*) : leave command as alias ;;
>   '')  : leave command alone ;;
>   *)   command="$expansion" ;;
>   esac
> 
> Or something like that.  Because an alias whose value starts with
> ! is a shell command to be executed, so we want to use _git_$command
> for completion, but other aliases are builtin commands and we should
> use their first word token (what __git_aliased_command returns)
> as the name of the completion function.

After pondering about it for a while, I think that in this case the
real issue is not _git() not handling __git_aliased_command()'s return
value corretly, but rather __git_aliased_command() returning junk in
case of a more advanced alias.  And while fixing it up, we can also
improve on it to return the right command in some more cases, too.

Let's have an other look at Junio's alias:

    [alias]
        lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"

While it's clear that full parsing of something like that in the
completion code is unfeasible, we can easily get rid of stuff that is
definitely not a git command: !sh shell commands, options, and
environment variables.


diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 45a393f..faddbdf 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -625,10 +625,15 @@ __git_aliased_command ()
 	local word cmdline=$(git --git-dir="$(__gitdir)" \
 		config --get "alias.$1")
 	for word in $cmdline; do
-		if [ "${word##-*}" ]; then
-			echo $word
+		case "$word" in
+		\!*)	: shell command alias ;;
+		-*)	: option ;;
+		*=*)	: setting env ;;
+		git)	: git itself ;;
+		*)
+			echo "$word"
 			return
-		fi
+		esac
 	done
 }

 
and this way it would correctly return 'log' for Junio's 'lgm' alias.
With a bit tweaking we could also extend it to handle !gitk aliases,
too.

Of course, it isn't perfect either, and could be fooled easily.  It's
not hard to construct an alias, in which a word does not match any of
these filter patterns, but is still not a git command (e.g.  by
setting an environment variable to a value which contains spaces).  It
may even return false positives, when the output of a git command is
piped into an other git command, and the second gets the command line
options via $@, but the first command will be returned.  However, such
problematic cases could be handled by a custom completion function
provided by the user.

What do you think?


Best,
Gábor

^ permalink raw reply related

* Re: v1.7.0-rc0 shows lots of "unable to find <sha1>" on git-stash
From: Junio C Hamano @ 2010-01-31 19:21 UTC (permalink / raw)
  To: Jonathan del Strother; +Cc: Junio C Hamano, Jens Lehmann, Git Mailing List
In-Reply-To: <57518fd11001311116t1bde882dub1dd3ca4af201b2e@mail.gmail.com>

Jonathan del Strother <maillist@steelskies.com> writes:

>>> +     strbuf_addf(&buf, "GIT_INDEX_FILE=");
>>
>> This should be:
>>
>>        strbuf_addf(&buf, "GIT_INDEX_FILE");
>
> Sorry, I'm not sure I follow your amendment - it looks exactly like
> the patch you originally supplied?

Lack of the last "=" tells: "Instead of exporting GIT_INDEX_FILE with an
empty string as its value, unexport it."

^ permalink raw reply

* Re: v1.7.0-rc0 shows lots of "unable to find <sha1>" on git-stash
From: Jonathan del Strother @ 2010-01-31 19:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, Git Mailing List
In-Reply-To: <57518fd11001311116t1bde882dub1dd3ca4af201b2e@mail.gmail.com>

On 31 January 2010 19:16, Jonathan del Strother <maillist@steelskies.com> wrote:
> On 30 January 2010 20:25, Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Please try this.
>>
>> oops, but with this, too.
>>
>>> diff --git a/submodule.c b/submodule.c
>>> index ca0527f..8bd0a30 100644
>>> --- a/submodule.c
>>> +++ b/submodule.c
>>> ...
>>> @@ -142,7 +142,9 @@ int is_submodule_modified(const char *path)
>>>       env[0] = strbuf_detach(&buf, NULL);
>>>       strbuf_addf(&buf, "GIT_DIR=%s/.git", path);
>>>       env[1] = strbuf_detach(&buf, NULL);
>>> -     env[2] = NULL;
>>> +     strbuf_addf(&buf, "GIT_INDEX_FILE=");
>>
>> This should be:
>>
>>        strbuf_addf(&buf, "GIT_INDEX_FILE");
>>
>>> +     env[2] = strbuf_detach(&buf, NULL);
>>> +     env[3] = NULL;
>>>
>>>       memset(&cp, 0, sizeof(cp));
>>>       cp.argv = argv;
>>
>
> Sorry, I'm not sure I follow your amendment - it looks exactly like
> the patch you originally supplied?
>
> With that original patch applied, I no longer get the 'unable to find'
> errors on stashing.  However, git-status shows all my submodules as
> being modified, but there appear to be no local changes :
>
> [jon@gir:Developer/AudioBooWeb]$ git status
> # On branch giterror
> # Changed but not updated:
> #   (use "git add <file>..." to update what will be committed)
> #   (use "git checkout -- <file>..." to discard changes in working directory)
> #
> #       modified:   shared/vendor/plugins/acts_as_list
> #       modified:   shared/vendor/plugins/cucumber
> #       modified:   shared/vendor/plugins/delayed_job
> #       modified:   shared/vendor/plugins/haml
> #       modified:   shared/vendor/plugins/hoptoad_notifier
> #       modified:   shared/vendor/plugins/machinist
> #       modified:   shared/vendor/plugins/newrelic_rpm
> #       modified:   shared/vendor/plugins/rspec
> #       modified:   shared/vendor/plugins/rspec-rails
> #       modified:   shared/vendor/rails
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> [jon@gir:Developer/AudioBooWeb]$ git diff
> diff --git a/shared/vendor/plugins/acts_as_list
> b/shared/vendor/plugins/acts_as_list
> diff --git a/shared/vendor/plugins/cucumber b/shared/vendor/plugins/cucumber
> diff --git a/shared/vendor/plugins/delayed_job
> b/shared/vendor/plugins/delayed_job
> diff --git a/shared/vendor/plugins/haml b/shared/vendor/plugins/haml
> diff --git a/shared/vendor/plugins/hoptoad_notifier
> b/shared/vendor/plugins/hoptoad_notifier
> diff --git a/shared/vendor/plugins/machinist b/shared/vendor/plugins/machinist
> diff --git a/shared/vendor/plugins/newrelic_rpm
> b/shared/vendor/plugins/newrelic_rpm
> diff --git a/shared/vendor/plugins/rspec b/shared/vendor/plugins/rspec
> diff --git a/shared/vendor/plugins/rspec-rails
> b/shared/vendor/plugins/rspec-rails
> diff --git a/shared/vendor/rails b/shared/vendor/rails
>

...  however, the latest version of master
(b0883aa6c77111e88496bd0afe073caf68ab9f99) gives me no errors, and
(correctly) doesn't show my submodules as being modified.  I still
don't quite follow exactly what was wrong, but thanks :)

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.7.0.rc1
From: Junio C Hamano @ 2010-01-31 19:23 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20100131091446.GA25704@coredump.intra.peff.net>

Thanks.

^ permalink raw reply

* Re: [PATCH] Fix memory leak in submodule.c
From: Junio C Hamano @ 2010-01-31 19:23 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Git Mailing List
In-Reply-To: <4B65B345.1090907@web.de>

Thanks.

^ permalink raw reply

* Re: [PATCH v2] Do not install shell libraries executable
From: Junio C Hamano @ 2010-01-31 19:46 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jeff King, Johannes Schindelin, David Aguilar, git
In-Reply-To: <20100131083459.GA18561@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Last time I also missed git-parse-remote, so while at it, I am taking
> the opportunity to add that to SCRIPT_LIB_SH, too.

Your patch says it was generated by 1.7.0-rc1, but the change itself
seems to be based on an older version.  Curious.

How much would it hurt the distro packagers, if we don't take this patch
before 1.7.0?  If this would help a lot, let's give it a bit higher
priority and make sure 1.7.0 ships with (a corrected version of) it;
otherwise I'd say we should not merge this before 1.7.0.

> +SCRIPT_LIB_SH += git-mergetool--lib.sh
> +SCRIPT_LIB_SH += git-parse-remote.sh
> +SCRIPT_LIB_SH += git-sh-setup.sh
> + ...
> @@ -1792,6 +1802,7 @@ install: all
>  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
>  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
>  	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> +	$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'

I can understand that you didn't want to include the "included scriptlets"
as part of ALL_PROGRAMS because $(INSTALL) may flip 'x' bit on.

But you should then be installing %(patsubst %.sh,%,$(SCRIPT_LIB_SH));
otherwise, you are installing git-sh-setup.sh and

    . git-sh-setup

would not work.  Have you ever tested this?

> @@ -1901,7 +1912,7 @@ distclean: clean
>  clean:
>  	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
>  		$(LIB_FILE) $(XDIFF_LIB)
> -	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
> +	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git$X

And this is even worse.  You are removing the _source_ here.  I can see
you didn't even test the very basic: "make clean && make".

> @@ -1930,7 +1941,7 @@ endif
>  ### Check documentation
>  #
>  check-docs::
> -	@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \
> +	@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk; \

Likewise.

>  	do \
>  		case "$$v" in \
>  		git-merge-octopus | git-merge-ours | git-merge-recursive | \
> @@ -1975,7 +1986,7 @@ check-docs::
>  		documented,gittutorial-2 | \
>  		sentinel,not,matching,is,ok ) continue ;; \
>  		esac; \
> -		case " $(ALL_PROGRAMS) $(BUILT_INS) git gitk " in \
> +		case " $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk " in \

Likewise.

Wouldn't it make a bit more sense to do it like this instead?  I at least
did "make clean && make" ;-)

-- >8 --
From: Jonathan Nieder <jrnieder@gmail.com>
Subject: [PATCH] Do not install shell libraries executable

Some scripts are expected to be sourced instead of executed on their own.
Avoid some confusion by not marking them executable.

The executable bit was confusing the valgrind support of our test scripts,
which assumed that any executable without a #!-line should be intercepted
and run through valgrind.  So during valgrind-enabled tests, any script
sourcing these files actually sourced the valgrind interception script
instead.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile            |   42 ++++++++++++++++++++++++++++--------------
 1 files changed, 28 insertions(+), 14 deletions(-)
 mode change 100755 => 100644 git-parse-remote.sh
 mode change 100755 => 100644 git-sh-setup.sh

diff --git a/Makefile b/Makefile
index af08c8f..6bbeb24 100644
--- a/Makefile
+++ b/Makefile
@@ -341,6 +341,7 @@ PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
 SCRIPT_SH =
+SCRIPT_LIB =
 TEST_PROGRAMS =
 
 SCRIPT_SH += git-am.sh
@@ -352,20 +353,21 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
-SCRIPT_SH += git-mergetool--lib.sh
 SCRIPT_SH += git-notes.sh
-SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase--interactive.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-repack.sh
 SCRIPT_SH += git-request-pull.sh
-SCRIPT_SH += git-sh-setup.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
+SCRIPT_LIB += git-mergetool--lib
+SCRIPT_LIB += git-parse-remote
+SCRIPT_LIB += git-sh-setup
+
 SCRIPT_PERL += git-add--interactive.perl
 SCRIPT_PERL += git-difftool.perl
 SCRIPT_PERL += git-archimport.perl
@@ -1454,7 +1456,7 @@ export TAR INSTALL DESTDIR SHELL_PATH
 
 SHELL = $(SHELL_PATH)
 
-all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
+all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
 ifneq (,$X)
 	$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)
 endif
@@ -1505,17 +1507,25 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt
 common-cmds.h: $(wildcard Documentation/git-*.txt)
 	$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@
 
+define cmd_munge_script
+$(RM) $@ $@+ && \
+sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+    -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
+    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
+    -e $(BROKEN_PATH_FIX) \
+    $@.sh >$@+
+endef
+
 $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
-	$(QUIET_GEN)$(RM) $@ $@+ && \
-	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-	    -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \
-	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-	    -e 's/@@NO_CURL@@/$(NO_CURL)/g' \
-	    -e $(BROKEN_PATH_FIX) \
-	    $@.sh >$@+ && \
+	$(QUIET_GEN)$(cmd_munge_script) && \
 	chmod +x $@+ && \
 	mv $@+ $@
 
+$(SCRIPT_LIB) : % : %.sh
+	$(QUIET_GEN)$(cmd_munge_script) && \
+	mv $@+ $@
+
 ifndef NO_PERL
 $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
 
@@ -1866,6 +1876,7 @@ install: all
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+	$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 	$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
 	$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
 ifndef NO_PERL
@@ -1985,7 +1996,7 @@ distclean: clean
 clean:
 	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)
-	$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X
+	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) -r bin-wrappers
 	$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*
@@ -2017,7 +2028,7 @@ endif
 ### Check documentation
 #
 check-docs::
-	@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \
+	@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git gitk; \
 	do \
 		case "$$v" in \
 		git-merge-octopus | git-merge-ours | git-merge-recursive | \
@@ -2060,9 +2071,12 @@ check-docs::
 		documented,gitrepository-layout | \
 		documented,gittutorial | \
 		documented,gittutorial-2 | \
+		documented,git-bisect-lk2009 | \
+		documented.git-remote-helpers | \
+		documented,gitworkflows | \
 		sentinel,not,matching,is,ok ) continue ;; \
 		esac; \
-		case " $(ALL_PROGRAMS) $(BUILT_INS) git gitk " in \
+		case " $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git gitk " in \
 		*" $$cmd "*)	;; \
 		*) echo "removed but $$how: $$cmd" ;; \
 		esac; \
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
old mode 100755
new mode 100644
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
old mode 100755
new mode 100644
-- 
1.7.0.rc1.141.gd3fd2

^ permalink raw reply related

* Re: [PATCH] Make NO_PTHREADS the sole thread configuration variable
From: Junio C Hamano @ 2010-01-31 19:52 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Dan McGee, git
In-Reply-To: <alpine.LFD.2.00.1001302015120.1681@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> Simply mentioning in the 1.7.0 
> release notes that THREADED_DELTA_SEARCH is no more should be fine.  

Ok; here is what I'll queue.

Thanks, both.

-- >8 --
From: Dan McGee <dpmcgee@gmail.com>
Date: Fri, 29 Jan 2010 19:22:19 -0600
Subject: [PATCH] Make NO_PTHREADS the sole thread configuration variable

When the first piece of threaded code was introduced in commit 8ecce684, it
came with its own THREADED_DELTA_SEARCH Makefile option. Since this time,
more threaded code has come into the codebase and a NO_PTHREADS option has
also been added. Get rid of the original option as the newer, more generic
option covers everything we need.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/RelNotes-1.7.0.txt |    3 +++
 Makefile                         |   24 ++----------------------
 builtin-pack-objects.c           |   12 ++++++------
 config.mak.in                    |    1 -
 configure.ac                     |    9 ++-------
 5 files changed, 13 insertions(+), 36 deletions(-)

diff --git a/Documentation/RelNotes-1.7.0.txt b/Documentation/RelNotes-1.7.0.txt
index 997b026..45ac906 100644
--- a/Documentation/RelNotes-1.7.0.txt
+++ b/Documentation/RelNotes-1.7.0.txt
@@ -175,6 +175,9 @@ Updates since v1.6.6
 
  * Many more commands are now built-in.
 
+ * THREADED_DELTA_SEARCH is no more.  If you build with threads, delta
+   compression will always take advantage of it.
+
 Fixes since v1.6.6
 ------------------
 
diff --git a/Makefile b/Makefile
index af08c8f..83383c3 100644
--- a/Makefile
+++ b/Makefile
@@ -180,9 +180,6 @@ all::
 # If not set it defaults to the bare 'wish'. If it is set to the empty
 # string then NO_TCLTK will be forced (this is used by configure script).
 #
-# Define THREADED_DELTA_SEARCH if you have pthreads and wish to exploit
-# parallel delta searching when packing objects.
-#
 # Define INTERNAL_QSORT to use Git's implementation of qsort(), which
 # is a simplified version of the merge sort used in glibc. This is
 # recommended if Git triggers O(n^2) behavior in your platform's qsort().
@@ -722,12 +719,10 @@ EXTLIBS =
 ifeq ($(uname_S),Linux)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
 	NO_STRLCPY = YesPlease
 	NO_MKSTEMPS = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),UnixWare)
 	CC = cc
@@ -781,7 +776,6 @@ ifeq ($(uname_S),Darwin)
 		NO_STRLCPY = YesPlease
 	endif
 	NO_MEMMEM = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
@@ -794,7 +788,6 @@ ifeq ($(uname_S),SunOS)
 	NO_MKDTEMP = YesPlease
 	NO_MKSTEMPS = YesPlease
 	NO_REGEX = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 	ifeq ($(uname_R),5.7)
 		NEEDS_RESOLV = YesPlease
 		NO_IPV6 = YesPlease
@@ -850,7 +843,6 @@ ifeq ($(uname_S),FreeBSD)
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	USE_ST_TIMESPEC = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 	ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
 		PTHREAD_LIBS = -pthread
 		NO_UINTMAX_T = YesPlease
@@ -864,7 +856,6 @@ ifeq ($(uname_S),OpenBSD)
 	NEEDS_LIBICONV = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
-	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),NetBSD)
 	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
@@ -872,7 +863,6 @@ ifeq ($(uname_S),NetBSD)
 	endif
 	BASIC_CFLAGS += -I/usr/pkg/include
 	BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib
-	THREADED_DELTA_SEARCH = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	NO_MKSTEMPS = YesPlease
 endif
@@ -887,9 +877,7 @@ ifeq ($(uname_S),AIX)
 	INTERNAL_QSORT = UnfortunatelyYes
 	NEEDS_LIBICONV=YesPlease
 	BASIC_CFLAGS += -D_LARGE_FILES
-	ifneq ($(shell expr "$(uname_V)" : '[1234]'),1)
-		THREADED_DELTA_SEARCH = YesPlease
-	else
+	ifeq ($(shell expr "$(uname_V)" : '[1234]'),1)
 		NO_PTHREADS = YesPlease
 	endif
 endif
@@ -915,7 +903,6 @@ ifeq ($(uname_S),IRIX)
 	SNPRINTF_RETURNS_BOGUS = YesPlease
 	SHELL_PATH = /usr/gnu/bin/bash
 	NEEDS_LIBGEN = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),IRIX64)
 	NO_SETENV=YesPlease
@@ -934,7 +921,6 @@ ifeq ($(uname_S),IRIX64)
 	SNPRINTF_RETURNS_BOGUS = YesPlease
 	SHELL_PATH=/usr/gnu/bin/bash
 	NEEDS_LIBGEN = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 endif
 ifeq ($(uname_S),HP-UX)
 	NO_IPV6=YesPlease
@@ -984,7 +970,6 @@ ifeq ($(uname_S),Windows)
 	NO_CURL = YesPlease
 	NO_PYTHON = YesPlease
 	BLK_SHA1 = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 
 	CC = compat/vcbuild/scripts/clink.pl
 	AR = compat/vcbuild/scripts/lib.pl
@@ -1036,7 +1021,6 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_REGEX = YesPlease
 	NO_PYTHON = YesPlease
 	BLK_SHA1 = YesPlease
-	THREADED_DELTA_SEARCH = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch -Icompat/win32
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	COMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o \
@@ -1334,16 +1318,12 @@ ifdef RUNTIME_PREFIX
 endif
 
 ifdef NO_PTHREADS
-	THREADED_DELTA_SEARCH =
 	BASIC_CFLAGS += -DNO_PTHREADS
 else
 	EXTLIBS += $(PTHREAD_LIBS)
-endif
-
-ifdef THREADED_DELTA_SEARCH
-	BASIC_CFLAGS += -DTHREADED_DELTA_SEARCH
 	LIB_OBJS += thread-utils.o
 endif
+
 ifdef DIR_HAS_BSD_GROUP_SEMANTICS
 	COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
 endif
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index b0887d7..4a41547 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -17,7 +17,7 @@
 #include "progress.h"
 #include "refs.h"
 
-#ifdef THREADED_DELTA_SEARCH
+#ifndef NO_PTHREADS
 #include "thread-utils.h"
 #include <pthread.h>
 #endif
@@ -1255,7 +1255,7 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,
 	return 0;
 }
 
-#ifdef THREADED_DELTA_SEARCH
+#ifndef NO_PTHREADS
 
 static pthread_mutex_t read_mutex;
 #define read_lock()		pthread_mutex_lock(&read_mutex)
@@ -1380,7 +1380,7 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
 	/*
 	 * Handle memory allocation outside of the cache
 	 * accounting lock.  Compiler will optimize the strangeness
-	 * away when THREADED_DELTA_SEARCH is not defined.
+	 * away when NO_PTHREADS is defined.
 	 */
 	free(trg_entry->delta_data);
 	cache_lock();
@@ -1567,7 +1567,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
 	free(array);
 }
 
-#ifdef THREADED_DELTA_SEARCH
+#ifndef NO_PTHREADS
 
 /*
  * The main thread waits on the condition that (at least) one of the workers
@@ -1899,7 +1899,7 @@ static int git_pack_config(const char *k, const char *v, void *cb)
 		if (delta_search_threads < 0)
 			die("invalid number of threads specified (%d)",
 			    delta_search_threads);
-#ifndef THREADED_DELTA_SEARCH
+#ifdef NO_PTHREADS
 		if (delta_search_threads != 1)
 			warning("no threads support, ignoring %s", k);
 #endif
@@ -2227,7 +2227,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			delta_search_threads = strtoul(arg+10, &end, 0);
 			if (!arg[10] || *end || delta_search_threads < 0)
 				usage(pack_usage);
-#ifndef THREADED_DELTA_SEARCH
+#ifdef NO_PTHREADS
 			if (delta_search_threads != 1)
 				warning("no threads support, "
 					"ignoring %s", arg);
diff --git a/config.mak.in b/config.mak.in
index 67b12f7..6008ac9 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -56,5 +56,4 @@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
 FREAD_READS_DIRECTORIES=@FREAD_READS_DIRECTORIES@
 SNPRINTF_RETURNS_BOGUS=@SNPRINTF_RETURNS_BOGUS@
 NO_PTHREADS=@NO_PTHREADS@
-THREADED_DELTA_SEARCH=@THREADED_DELTA_SEARCH@
 PTHREAD_LIBS=@PTHREAD_LIBS@
diff --git a/configure.ac b/configure.ac
index 78345eb..229140e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -762,10 +762,9 @@ AC_SUBST(NO_MKSTEMPS)
 # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link.
 # Enable it on Windows.  By default, symrefs are still used.
 #
-# Define NO_PTHREADS if we do not have pthreads
+# Define NO_PTHREADS if we do not have pthreads.
 #
-# Define PTHREAD_LIBS to the linker flag used for Pthread support and define
-# THREADED_DELTA_SEARCH if Pthreads are available.
+# Define PTHREAD_LIBS to the linker flag used for Pthread support.
 AC_DEFUN([PTHREADTEST_SRC], [
 #include <pthread.h>
 
@@ -782,7 +781,6 @@ dnl   [[pthread_mutex_t test_mutex;]]
 dnl )])
 
 NO_PTHREADS=UnfortunatelyYes
-THREADED_DELTA_SEARCH=
 PTHREAD_LIBS=
 
 if test -n "$USER_NOPTHREAD"; then
@@ -798,7 +796,6 @@ elif test -z "$PTHREAD_CFLAGS"; then
 	[AC_MSG_RESULT([yes])
 		NO_PTHREADS=
 		PTHREAD_LIBS="$opt"
-		THREADED_DELTA_SEARCH=YesPlease
 		break
 	],
 	[AC_MSG_RESULT([no])])
@@ -812,7 +809,6 @@ else
 	[AC_MSG_RESULT([yes])
 		NO_PTHREADS=
 		PTHREAD_LIBS="$PTHREAD_CFLAGS"
-		THREADED_DELTA_SEARCH=YesPlease
 	],
 	[AC_MSG_RESULT([no])])
 
@@ -823,7 +819,6 @@ CFLAGS="$old_CFLAGS"
 
 AC_SUBST(PTHREAD_LIBS)
 AC_SUBST(NO_PTHREADS)
-AC_SUBST(THREADED_DELTA_SEARCH)
 
 ## Output files
 AC_CONFIG_FILES(["${config_file}":"${config_in}":"${config_append}"])
-- 
1.7.0.rc1.141.gd3fd2

^ permalink raw reply related

* Re: [PATCH v2] Do not install shell libraries executable
From: Jonathan Nieder @ 2010-01-31 20:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, David Aguilar, git
In-Reply-To: <7vy6jecb0y.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> How much would it hurt the distro packagers, if we don't take this patch
> before 1.7.0?  If this would help a lot, let's give it a bit higher
> priority and make sure 1.7.0 ships with (a corrected version of) it;
> otherwise I'd say we should not merge this before 1.7.0.

Given that Peff’s fix is in, I don’t think it is needed at all.  So I
would say, better to let it wait.

> > +SCRIPT_LIB_SH += git-mergetool--lib.sh
> > +SCRIPT_LIB_SH += git-parse-remote.sh
> > +SCRIPT_LIB_SH += git-sh-setup.sh
> > + ...
> > @@ -1792,6 +1802,7 @@ install: all
> >  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
> >  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> >  	$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> > +	$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
> 
> I can understand that you didn't want to include the "included scriptlets"
> as part of ALL_PROGRAMS because $(INSTALL) may flip 'x' bit on.
> 
> But you should then be installing %(patsubst %.sh,%,$(SCRIPT_LIB_SH));
> otherwise, you are installing git-sh-setup.sh and
> 
>     . git-sh-setup
> 
> would not work.  Have you ever tested this?

Good catches, sorry. :(  I did test but clearly I was not thinking very
well when I looked at the result...

> Wouldn't it make a bit more sense to do it like this instead?

Yes, that looks right.  Thanks for cleaning up my mess!

Sorry for the trouble,
Jonathan

^ permalink raw reply

* Re: [PATCH v2] Do not install shell libraries executable
From: Junio C Hamano @ 2010-01-31 20:05 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jeff King, Johannes Schindelin, David Aguilar, git
In-Reply-To: <20100131200028.GA4045@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
>
>> How much would it hurt the distro packagers, if we don't take this patch
>> before 1.7.0?  If this would help a lot, let's give it a bit higher
>> priority and make sure 1.7.0 ships with (a corrected version of) it;
>> otherwise I'd say we should not merge this before 1.7.0.
>
> Given that Peff’s fix is in, I don’t think it is needed at all.  So I
> would say, better to let it wait.

I was referring to this from your original:

    It was also confusing dpkg-shlibdeps, so I recently came up with
    this fix.  Both fixes seem like good changes to me, and both
    could be applied.  Your fix has the virtue of being shorter,
    hence safer.

Is Jeff's mergetool-lib change enough to address this issue as well?

^ permalink raw reply

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

Jonathan Nieder <jrnieder@gmail.com> writes:

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

Thanks.

Three observations:

 * "make distclean" should remove $(dep_files);

 * "deps/" should probably be ".depend/" or something less distracting;

 * I wish CHECK_HEADER_DEPENDENCIES pointed out unnecessary dependencies
   hardcoded in the Makefile.

-- >8 --
Subject: [PATCH] Makefile: "make distclean" should remove dependency files

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 45b1f54..8578b31 100644
--- a/Makefile
+++ b/Makefile
@@ -2110,7 +2110,7 @@ dist-doc:
 ### Cleaning rules
 
 distclean: clean
-	$(RM) configure
+	$(RM) configure $(dep_files)
 
 clean:
 	$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \
-- 
1.7.0.rc1.141.gd3fd2

^ permalink raw reply related

* Re: [PATCH v2] Do not install shell libraries executable
From: Jonathan Nieder @ 2010-01-31 21:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, David Aguilar, git
In-Reply-To: <7vockaca5w.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>
>>> How much would it hurt the distro packagers, if we don't take this patch
>>> before 1.7.0?  If this would help a lot, let's give it a bit higher
>>> priority and make sure 1.7.0 ships with (a corrected version of) it;
>>> otherwise I'd say we should not merge this before 1.7.0.
>>
>> Given that Peff’s fix is in, I don’t think it is needed at all.  So I
>> would say, better to let it wait.
>
> I was referring to this from your original:
> 
>     It was also confusing dpkg-shlibdeps, so I recently came up with
>     this fix.  Both fixes seem like good changes to me, and both
>     could be applied.  Your fix has the virtue of being shorter,
>     hence safer.
> 
> Is Jeff's mergetool-lib change enough to address this issue as well?

I just checked; looks like I was confusing a few issues.

 - dpkg-shlibdeps does not like to be fed scripts, period.  That
   has nothing to do with this.

 - debian/rules in the git-core package feeds every file in
   /usr/bin and gitexecdir that doesn’t start with #! to 'strip'.
   Jeff's change helps that; my fix has nothing to do with it.

 - some other tool must have been happier with these files not
   being executable, but I cannot reproduce this or find it now.

I wrote that patch late at night, and unfortunately, I cannot justify it
to myself now.  With Jeff’s change applied, there is no obvious breakage
that it fixes.  If a problem comes up again, I will let you know.

Embarrassed,
Jonathan

^ permalink raw reply

* Re: [PATCH v2] Do not install shell libraries executable
From: Junio C Hamano @ 2010-01-31 21:08 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jeff King, Johannes Schindelin, David Aguilar, git
In-Reply-To: <20100131210052.GA4435@progeny.tock>

Jonathan Nieder <jrnieder@gmail.com> writes:

> I wrote that patch late at night, and unfortunately, I cannot justify it
> to myself now.  With Jeff’s change applied, there is no obvious breakage
> that it fixes.  If a problem comes up again, I will let you know.

Thanks for a thorough write-up.  Really appreciated.

^ permalink raw reply

* Re: [PATCH 12/12] Teach Makefile to check header dependencies
From: Jonathan Nieder @ 2010-01-31 21:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <7viqaic8gs.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> Thanks.
> 
> Three observations:
> 
>  * "make distclean" should remove $(dep_files);

It removes $(dep_dirs) in "make clean".  Do you think they should be
kept around?  Or is this about when COMPUTE_HEADER_DEPENDENCIES is
turned off but .o.d files remain?

>  * "deps/" should probably be ".depend/" or something less distracting;

Good idea.

>  * I wish CHECK_HEADER_DEPENDENCIES pointed out unnecessary dependencies
>    hardcoded in the Makefile.

That would not be hard to do, but wouldn’t the $(GIT_OBJS): $(LIB_H)
rule create a lot of noise?

How about if it checks for duplicate dependencies and unnecessary
dependencies that are not in LIB_H?

> -- >8 --
> Subject: [PATCH] Makefile: "make distclean" should remove dependency files
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Makefile |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 45b1f54..8578b31 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2110,7 +2110,7 @@ dist-doc:
>  ### Cleaning rules
>  
>  distclean: clean
> -	$(RM) configure
> +	$(RM) configure $(dep_files)

This couldn’t hurt, but I wonder whether it is necessary; see above.

Thanks for the feedback,
Jonathan

^ permalink raw reply

* [PATCH 13/12] Makefile: tuck away generated makefile fragments in .depend
From: Jonathan Nieder @ 2010-01-31 21:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <7viqaic8gs.fsf@alter.siamese.dyndns.org>

When building with COMPUTE_HEADER_DEPENDENCIES on, save
dependency information to .depend/ instead of deps/ so it does
not show up in ‘ls’ output.  Otherwise, the extra directories can
be distracting.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

>  * "deps/" should probably be ".depend/" or something less distracting;

Good idea.  Here goes...

 Makefile |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 45b1f54..c939139 100644
--- a/Makefile
+++ b/Makefile
@@ -1692,15 +1692,15 @@ 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)
 
-dep_files := $(foreach f,$(OBJECTS),$(dir $f)deps/$(notdir $f).d)
+dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)
 
 ifdef COMPUTE_HEADER_DEPENDENCIES
-dep_dirs := $(addsuffix deps,$(sort $(dir $(OBJECTS))))
+dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))
 $(dep_dirs):
 	mkdir -p $@
 
 missing_dep_dirs := $(filter-out $(wildcard $(dep_dirs)),$(dep_dirs))
-dep_file = $(dir $@)deps/$(notdir $@).d
+dep_file = $(dir $@).depend/$(notdir $@).d
 dep_args = -MF $(dep_file) -MMD -MP
 ifdef CHECK_HEADER_DEPENDENCIES
 $(error cannot compute header dependencies outside a normal build. \
-- 
1.7.0.rc1

^ permalink raw reply related

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

Jonathan Nieder <jrnieder@gmail.com> writes:

> Junio C Hamano wrote:
> ...
>> Three observations:
>> 
>>  * "make distclean" should remove $(dep_files);
>
> It removes $(dep_dirs) in "make clean".  Do you think they should be
> kept around?  Or is this about when COMPUTE_HEADER_DEPENDENCIES is
> turned off but .o.d files remain?

Yes, the last one.  After playing around, with or without COMPUTE/CHECK,
and then making a final build, it would be nice if "make distclean"
removed all the intermediate build products without the same special
"debug build" options (and without COMPUTE, dep_dirs is empty so nothing
is removed).  It is also fine if "make clean" did so.

>>  * I wish CHECK_HEADER_DEPENDENCIES pointed out unnecessary dependencies
>>    hardcoded in the Makefile.
>
> That would not be hard to do, but wouldn’t the $(GIT_OBJS): $(LIB_H)
> rule create a lot of noise?
>
> How about if it checks for duplicate dependencies and unnecessary
> dependencies that are not in LIB_H?

That would be ideal, I think.

^ permalink raw reply

* [PATCH 14/12] Makefile: always remove .depend directories on 'make clean'
From: Jonathan Nieder @ 2010-01-31 21:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Johannes Schindelin, Erik Faye-Lund,
	Sverre Rabbelier, Andreas Schwab
In-Reply-To: <7vaavuc6f4.fsf@alter.siamese.dyndns.org>

Even if COMPUTE_HEADER_DEPENDENCIES is not set, some .o.d files
might be lying around from previous builds when it was.  This
is especially likely because using the CHECK_HEADER_DEPENDENCIES
feature requires building sometimes with COMPUTE... on and
sometimes with it off.  At the end of such an exercise, to get
a blank slate, the user ought to be able to just run 'make clean'.
Make it so.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> Yes, the last one.  After playing around, with or without COMPUTE/CHECK,
> and then making a final build, it would be nice if "make distclean"
> removed all the intermediate build products without the same special
> "debug build" options (and without COMPUTE, dep_dirs is empty so nothing
> is removed).

Another good idea. :)

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

diff --git a/Makefile b/Makefile
index c939139..93e1a92 100644
--- a/Makefile
+++ b/Makefile
@@ -1693,9 +1693,9 @@ XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
 OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS)
 
 dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)
+dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))
 
 ifdef COMPUTE_HEADER_DEPENDENCIES
-dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))
 $(dep_dirs):
 	mkdir -p $@
 
-- 
1.7.0.rc1

^ permalink raw reply related

* Re: [RFH] rpm packaging failure
From: David Aguilar @ 2010-01-31 23:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Todd Zullinger, git, Johan Herland, Sverre Rabbelier
In-Reply-To: <7vd40rjks4.fsf@alter.siamese.dyndns.org>

On Sat, Jan 30, 2010 at 02:22:03PM -0800, Junio C Hamano wrote:
> Todd Zullinger <tmz@pobox.com> writes:
> 
> > Not shipping them if they aren't yet used/needed for anything other
> > than folks working on remote helpers seems reasonable and the rm in
> > the spec file should do the trick nicely.
> 
> Thanks.
> 
> To allow us to go forward a bit easier, I am planning to use the attached,
> as we would need some parts of it when we do start generating a separate
> package, I think.
> 
> -- >8 --
> Subject: [PATCH] RPM packaging: don't include foreign-scm-helper bits yet
> 
> The files in /usr/lib/python* are only the support infrastructure for
> foreign scm interface yet to be written and/or shipped with git.  Don't
> include them in the binary package (this will also free us from Python
> dependency).
> 
> When we ship with foreign scm interface, we will need to package these
> files with it in a separate subpackage, but we are not there yet.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  git.spec.in |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)


Apologies for the late response to this thread.

Would it be simpler to define NO_PYTHON when running make?
We can always resurrect %python_sitelib when it is needed.

Here's what that looks like relative to master.
I've tested this on fedora11.


On a slightly related note, I'm seeing this as well:

error: Installed (but unpackaged) file(s) found:
   /usr/lib/Git.pm

Maybe my setup is wanky?

-- >8 --
From 7fb05607633bc424fc29b645e6f4ee7e48371f94 Mon Sep 17 00:00:00 2001
From: David Aguilar <davvid@gmail.com>
Date: Sun, 31 Jan 2010 14:52:13 -0800
Subject: [PATCH] RPM packaging: Define NO_PYTHON to avoid Python dependency

We don't use the Python bits yet so we shouldn't require
python to build.  The %define used python to calculate
%python_sitelib and thus incurred a build-time python
dependency without specifying python in BuildRequires.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git.spec.in |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/git.spec.in b/git.spec.in
index 0a03108..ae7edc6 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -98,7 +98,6 @@ BuildRequires:  perl(Error)
 Perl interface to Git
 
 %define path_settings ETC_GITCONFIG=/etc/gitconfig prefix=%{_prefix} mandir=%{_mandir} htmldir=%{_docdir}/%{name}-%{version}
-%{!?python_sitelib: %define python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
 
 %prep
 %setup -q
@@ -106,14 +105,15 @@ Perl interface to Git
 %build
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" \
      %{path_settings} \
+     NO_PYTHON=NotYet \
      all %{!?_without_docs: doc}
 
 %install
 rm -rf $RPM_BUILD_ROOT
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" DESTDIR=$RPM_BUILD_ROOT \
      %{path_settings} \
+     NO_PYTHON=NotYet \
      INSTALLDIRS=vendor install %{!?_without_docs: install-doc}
-test ! -d $RPM_BUILD_ROOT%{python_sitelib} || rm -fr $RPM_BUILD_ROOT%{python_sitelib}
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name '*.bs' -empty -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name perllocal.pod -exec rm -f {} ';'
@@ -192,6 +192,9 @@ rm -rf $RPM_BUILD_ROOT
 # No files for you!
 
 %changelog
+* Sun Jan 31 2010 David Aguilar <davvid@gmail.com>
+- Define NO_PYTHON until we actually need the Python bits
+
 * Sat Jan 30 2010 Junio C Hamano <gitster@pobox.com>
 - We don't ship Python bits until a real foreign scm interface comes.
 
-- 
1.6.2.5

^ permalink raw reply related

* Re: [RFH] rpm packaging failure
From: David Aguilar @ 2010-01-31 23:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Todd Zullinger, git, Johan Herland, Sverre Rabbelier
In-Reply-To: <20100131231430.GA10914@gmail.com>

On Sun, Jan 31, 2010 at 03:14:32PM -0800, David Aguilar wrote:
> On Sat, Jan 30, 2010 at 02:22:03PM -0800, Junio C Hamano wrote:
> > Todd Zullinger <tmz@pobox.com> writes:
> > 
> > > Not shipping them if they aren't yet used/needed for anything other
> > > than folks working on remote helpers seems reasonable and the rm in
> > > the spec file should do the trick nicely.
> > 
> > Thanks.
> > 
> > To allow us to go forward a bit easier, I am planning to use the attached,
> > as we would need some parts of it when we do start generating a separate
> > package, I think.
> > 
> > -- >8 --
> > Subject: [PATCH] RPM packaging: don't include foreign-scm-helper bits yet
> > 
> > The files in /usr/lib/python* are only the support infrastructure for
> > foreign scm interface yet to be written and/or shipped with git.  Don't
> > include them in the binary package (this will also free us from Python
> > dependency).
> > 
> > When we ship with foreign scm interface, we will need to package these
> > files with it in a separate subpackage, but we are not there yet.
> > 
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > ---
> >  git.spec.in |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> 
> Apologies for the late response to this thread.
> 
> Would it be simpler to define NO_PYTHON when running make?
> We can always resurrect %python_sitelib when it is needed.
> 
> Here's what that looks like relative to master.


I see you just pushed out a new master with the %define changes.
Here's a rebased patch just in case.

-- >8 --
From ac09b94c332564cc7dde3c2f8961990098b5d4a0 Mon Sep 17 00:00:00 2001
From: David Aguilar <davvid@gmail.com>
Date: Sun, 31 Jan 2010 14:52:13 -0800
Subject: [PATCH] RPM packaging: Define NO_PYTHON to avoid Python dependency

We don't use the Python bits yet so we shouldn't require python
to build.  The %define used python to calculate %python_sitelib
and thus incurred a build-time python dependency without
specifying python in BuildRequires.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git.spec.in |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/git.spec.in b/git.spec.in
index ee74a5e..320829a 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -98,7 +98,6 @@ BuildRequires:  perl(Error)
 Perl interface to Git
 
 %define path_settings ETC_GITCONFIG=/etc/gitconfig prefix=%{_prefix} mandir=%{_mandir} htmldir=%{_docdir}/%{name}-%{version}
-%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print get_python_lib()")}
 
 %prep
 %setup -q
@@ -106,14 +105,15 @@ Perl interface to Git
 %build
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" \
      %{path_settings} \
+     NO_PYTHON=NotYet \
      all %{!?_without_docs: doc}
 
 %install
 rm -rf $RPM_BUILD_ROOT
 make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" DESTDIR=$RPM_BUILD_ROOT \
      %{path_settings} \
+     NO_PYTHON=NotYet \
      INSTALLDIRS=vendor install %{!?_without_docs: install-doc}
-test ! -d $RPM_BUILD_ROOT%{python_sitelib} || rm -fr $RPM_BUILD_ROOT%{python_sitelib}
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name '*.bs' -empty -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -type f -name perllocal.pod -exec rm -f {} ';'
@@ -192,6 +192,9 @@ rm -rf $RPM_BUILD_ROOT
 # No files for you!
 
 %changelog
+* Sun Jan 31 2010 David Aguilar <davvid@gmail.com>
+- Define NO_PYTHON until we actually need the Python bits.
+
 * Sun Jan 31 2010 Junio C Hamano <gitster@pobox.com>
 - Do not use %define inside %{!?...} construct.
 
-- 
1.7.0.rc1.7.gc0da5

^ permalink raw reply related

* Re: [PATCH v6] add --summary option to git-push and git-fetch
From: Daniel Barkalow @ 2010-02-01  0:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Larry D'Anna, git, Shawn O. Pearce, Ilari Liusvaara
In-Reply-To: <7vsk9oysds.fsf@alter.siamese.dyndns.org>

On Fri, 29 Jan 2010, Junio C Hamano wrote:

> As I said in my review during the earlier rounds, I do not know if it is
> safe to use the flags and do the traversal inside this same process.  You
> may be clearing the flags to protect your traversal (one per branch) from
> stepping on each other, but how would this affect the use of object flags
> in existing parts of the "push" machinery?  Is the reasoning that even if
> push calls into traversal code and after it walked the commit ancestry for
> its own purpose, your addition will clear the flags and existing code will
> never look at object flags again, so this new code is free to use them and
> all is Ok?  As long as you made sure that nobody looks at object flags you
> modified, then I am fine with that---I just don't know if that is what is
> happening here, and that is why I am asking.
> 
> I'd need help from the usual "transport" suspects for this patch.

I'm pretty sure that the built-in transport implementations all clear the 
flags themselves before using them. The fetch side has to be able to fetch 
twice in order to handle tags, and the push side has to be able to push to 
multiple destinations. So both parts should be defending themselves 
against flags that are specificly confusing to that part.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH v6] add --summary option to git-push and git-fetch
From: Larry D'Anna @ 2010-02-01  0:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <7vsk9oysds.fsf@alter.siamese.dyndns.org>

* Junio C Hamano (gitster@pobox.com) [100130 02:16]:
> Larry D'Anna <larry@elder-gods.org> writes:
> > +
> > +	object = parse_object(sha1); 
> > +	if (!object)
> > +	    die("bad object %s", arg);
> > +	
> > +	object_deref = deref_tag(object, NULL, 0); 
> > +	if (object_deref && object_deref->type == OBJ_COMMIT)
> > +	    if (flags_to_clear)
> > +		clear_commit_marks((struct commit *) object_deref, flags_to_clear); 
> > +
> > +	object->flags |= flags ^ local_flags; 
> 
> This smells somewhat fishy---what is the reason this "peel and mark" needs
> to be done only in this codepath, and none of the other callers of
> get_reference() need a similar logic, for example?
> 
> In general, why do you need to sprinkle clear-commit-marks all over the
> place?  

My idea was to call call clear_commit_marks on the "roots" of the revision arg,
and since handle_revision_arg looks up those roots in several different places,
i had to put clear_commit_marks in each of those places.  the reason the patch
is particularly ugly in this spot is that the other places where i put
clear_commit_marks, I already had a struct commit *, but here i just had a
object that might be a tag.

> This is not a rhetorical question (I haven't reviewed all the
> codepath involved for quite some time), but naïvely it appears it would be
> a lot simpler if you can let the existing code to do all the revision
> parsing and preparation to add to the pending object array as usual, and
> clear the flags from them before you let prepare_revision_walk() to start
> traversing the commit, but you probably had some reason why that simpler
> approach would not work and did it this way.  What am I missing?

The "existing code" being the caller of print_summary_for_push_or_fetch?  I
suppose I just wanted to keep the patches interference with update_local_ref to
a minimum, so I had it just grab the existing variable "quickref" out of that
function, because that was all the info I really needed to print the summary.

So i guess you're saying that it would be better for update_local_ref and
print_summary_for_push_or_fetch to clear the flags, and just pass a rev_info for
print_summary_for_push_or_fetch instead of quickref?

   --larry

^ permalink raw reply

* Wishlist for branch management
From: H. Peter Anvin @ 2010-02-01  1:18 UTC (permalink / raw)
  To: Git Mailing List

A wishlist for better handling of branches:

git clone --branches

... git clone, with the additional step of setting up local branches for
each one of the remote branches.

git branch --current

... list the current branch name, for use in scripts.  Equivalent to:
	"git branch | grep '^\*' | cut -c3-"

git push --current

... push the current branch, and only the current branch...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

^ permalink raw reply

* Re: Wishlist for branch management
From: Björn Steinbrink @ 2010-02-01  1:31 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Git Mailing List
In-Reply-To: <4B662BEF.7040503@zytor.com>

On 2010.01.31 17:18:39 -0800, H. Peter Anvin wrote:
> git branch --current
> 
> ... list the current branch name, for use in scripts.  Equivalent to:
> 	"git branch | grep '^\*' | cut -c3-"

In scripts, plumbing should be used. I use:
	git rev-parse --symbolic-full-name HEAD

This gives either the full refname of the checked out branch head, e.g.
refs/heads/master, or HEAD in case of a detached HEAD.

> git push --current
> 
> ... push the current branch, and only the current branch...

Unless you want to push to a different ref remotely, e.g. pushing
refs/heads/master-public to refs/heads/master, you can use:
	git push <remote> HEAD

For example, when refs/heads/master is checked out, then:
	git push origin HEAD
acts the same as:
	git push origin refs/heads/master

Björn

^ permalink raw reply

* GIT_WORK_TREE environment variable not working
From: Ron Garret @ 2010-02-01  1:33 UTC (permalink / raw)
  To: git

What am I doing wrong here?

[ron@mickey:~/devel/gittest]$ pwd
/Users/ron/devel/gittest
[ron@mickey:~/devel/gittest]$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#  git/
nothing added to commit but untracked files present (use "git add" to 
track)
[ron@mickey:~/devel/gittest]$ cd
[ron@mickey:~]$ export GIT_WORK_TREE=/Users/ron/devel/gittest
[ron@mickey:~]$ git status
fatal: Not a git repository (or any of the parent directories): .git
[ron@mickey:~]$ git status --work-tree=/Users/ron/devel/gittest
fatal: Not a git repository (or any of the parent directories): .git
[ron@mickey:~]$

^ permalink raw reply

* [PATCH 1/2] configure: Allow GIT_ARG_SET_PATH to handle --without-PROGRAM
From: Ben Walton @ 2010-02-01  2:15 UTC (permalink / raw)
  To: git, gitster; +Cc: Ben Walton
In-Reply-To: <1264990505-29578-1-git-send-email-bwalton@artsci.utoronto.ca>

Add an optional second argument to both GIT_ARG_SET_PATH and
GIT_CONF_APPEND_PATH such that any value of the second argument will
enable configure to set NO_$PROGRAM in addition to an empty
$PROGRAM_PATH.  This is initially useful for allowing configure to
disable the use of python, as the remote helper code has nothing
leveraging it yet.

The Makefile already recognizes NO_PYTHON, but configure provided no
way to set it appropriately.

Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
 configure.ac |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 229140e..9eaae7d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -23,21 +23,32 @@ AC_DEFUN([GIT_CONF_APPEND_LINE],
 # GIT_ARG_SET_PATH(PROGRAM)
 # -------------------------
 # Provide --with-PROGRAM=PATH option to set PATH to PROGRAM
+# Optional second argument allows setting NO_PROGRAM=YesPlease if
+# --without-PROGRAM version used.
 AC_DEFUN([GIT_ARG_SET_PATH],
 [AC_ARG_WITH([$1],
  [AS_HELP_STRING([--with-$1=PATH],
                  [provide PATH to $1])],
- [GIT_CONF_APPEND_PATH($1)],[])
+ [GIT_CONF_APPEND_PATH($1,$2)],[])
 ])# GIT_ARG_SET_PATH
 #
 # GIT_CONF_APPEND_PATH(PROGRAM)
 # ------------------------------
 # Parse --with-PROGRAM=PATH option to set PROGRAM_PATH=PATH
 # Used by GIT_ARG_SET_PATH(PROGRAM)
+# Optional second argument allows setting NO_PROGRAM=YesPlease if
+# --without-PROGRAM is used.
 AC_DEFUN([GIT_CONF_APPEND_PATH],
 [PROGRAM=m4_toupper($1); \
 if test "$withval" = "no"; then \
-	AC_MSG_ERROR([You cannot use git without $1]); \
+	if test -n "$2"; then \
+		m4_toupper($1)_PATH=$withval; \
+		AC_MSG_NOTICE([Disabling use of ${PROGRAM}]); \
+		GIT_CONF_APPEND_LINE(NO_${PROGRAM}=YesPlease); \
+		GIT_CONF_APPEND_LINE(${PROGRAM}_PATH=); \
+	else \
+		AC_MSG_ERROR([You cannot use git without $1]); \
+	fi; \
 else \
 	if test "$withval" = "yes"; then \
 		AC_MSG_WARN([You should provide path for --with-$1=PATH]); \
-- 
1.6.5.3

^ permalink raw reply related

* [PATCH 0/2] configure tweaks for NO_PYTHON
From: Ben Walton @ 2010-02-01  2:15 UTC (permalink / raw)
  To: git, gitster; +Cc: Ben Walton

The following patches enable a user to ./configure git with python
disabled.  There is nothing using the remote helper libraries yet.  I
noticed this missing capability after the recent rpm .spec thread.

Ben Walton (2):
  configure: Allow GIT_ARG_SET_PATH to handle --without-PROGRAM
  configure: Allow --without-python

 configure.ac |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

^ 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