Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Jakub Narebski @ 2012-02-16 13:40 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1329395775-18294-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Changes since v2:
> 
[...]
> - Shell style issues (thanks to Jakub)
[...]

> +verbose "Auto-detecting PDF viewer"
> +candidates="xdg-open evince okular xpdf acroread"
> +if [ "$(uname)" = Darwin ]; then
> +    # open exists on GNU/Linux, but does not open PDFs
> +    candidates="open $candidates"
> +fi
> +
> +for command in $candidates; do
> +    if [ "$PDFVIEWER" = "" ]; then
> +	if command -v "$command" >/dev/null 2>&1; then
> +	    PDFVIEWER="$command"
> +	else
> +	    verbose_progress
> +	fi
> +    fi
> +done
> +verbose_done "$PDFVIEWER"

Eh?  I don't see shell style issues fixed (loop inside conditional
instead of vice-versa, "test ..." instead of "[ ... ]").

Nb. I think it would be good to put detecting PDF viewer in its own
function, don't you?

-- 
Jakub Narebski

^ permalink raw reply

* Re: git status: small difference between stating whole repository and small subdirectory
From: Piotr Krukowiecki @ 2012-02-16 13:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Rast, Git Mailing List, Nguyen Thai Ngoc Duy
In-Reply-To: <20120215190318.GA5992@sigill.intra.peff.net>

On Wed, Feb 15, 2012 at 8:03 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 15, 2012 at 09:57:29AM +0100, Piotr Krukowiecki wrote:
>>
> I notice that you're still I/O bound even after the repack:
>
>> $ time git status  -- .
>> real    0m2.503s
>> user    0m0.160s
>> sys     0m0.096s
>>
>> $ time git status
>> real    0m9.663s
>> user    0m0.232s
>> sys     0m0.556s
>
> Did you drop caches here, too?

Yes I did - with cache the status takes something like 0.1-0.3s on whole repo.


>  Usually that would not be the case on a
> warm cache. If it is, then it sounds like you are short on memory to
> actually hold the directory tree and object db in cache. If not, what do
> the warm cache numbers look like?

I've got 4GB of ram and I did not hit the swap when doing last
performance tests AFAIK.
Please see my previous posts for performance results with warm cache
and profile results:

http://article.gmane.org/gmane.comp.version-control.git/190397
http://article.gmane.org/gmane.comp.version-control.git/190638


>> - can it be faster without repacking?
>
> Not really. You're showing an I/O problem, and repacking is git's way of
> reducing I/O.

So if I understand correctly, the reason is because git must compare
workspace files with packed objects - and the problem is
reading/seeking/searching in the packs?

Is there a way to make packs better? I think most operations are on
workdir files - so maybe it'd be possible to tell gc/repack/whatever
to optimize access to files which I currently have in workdir?


>> - even with packed repo, the time on small subdirectory is much higher
>> than I'd expect given time on whole repo and subdirectory size - why?
>
> Hard to say without profiling.  It may be that we reduced the object db
> lookups, saving some time, but still end up stat()ing the whole tree.
> The optimization to stat only the directories of interest was in 688cd6d
> (status: only touch path we may need to check, 2010-01-14), which went
> into v1.7.0. What version of git are you using?

For latest tests I've used 1.7.9.rc0.10.gbeecc, for profiling - 1.7.9.188.g12766

Is there anything else I could do?


-- 
Piotr Krukowiecki

^ permalink raw reply

* Re: git status: small difference between stating whole repository and small subdirectory
From: Piotr Krukowiecki @ 2012-02-16 13:22 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Thomas Rast, Git Mailing List, Jeff King
In-Reply-To: <CAA01CsoW-32kOtD2g==2FDcYaxfSVD4Xdy1bYG_wJBWDNept4A@mail.gmail.com>

On Wed, Feb 15, 2012 at 4:14 PM, Piotr Krukowiecki
<piotr.krukowiecki@gmail.com> wrote:
> On Wed, Feb 15, 2012 at 12:01 PM, Nguyen Thai Ngoc Duy
> <pclouds@gmail.com> wrote:
>> On Wed, Feb 15, 2012 at 3:57 PM, Piotr Krukowiecki
>> <piotr.krukowiecki@gmail.com> wrote:
>>> Indeed, after gc the times went down:
>>> 10s -> 2.3s (subdirectory)
>>> 17s -> 9.5s (whole repo)
>>>
>>> 2 seconds is much better and I'd say acceptable for me. But my questions are:
>>> - why is it so slow with not packed repo?
>>> - can it be faster without repacking?
>>
>> gc does more than just repacking. If you still have the un-gc'd repo,
>> Try these commands one by one, and time "git status" after each:
>>
>>  - git pack-refs --all --prune
>>  - git reflog expire --all
>>  - git repack -d -l
>>  - git prune --expire
>>  - git rerere gc
>
> It will take some time but hopefully I'll have the stats for tomorrow.

Here they are. I did 'status' three times to get reliable results and
before each run have dropped caches. Backed up repository was copied
before each 'status'. Full log is at http://pastebin.com/VmB7J9CJ

git version 1.7.9.rc0.10.gbeecc

Results after each command:

status on whole repo:
18.5s - after git count-objects -v
16.0s - after git pack-refs --all --prune
20.2s - after git reflog expire --all
13.0s - after git repack -d -l
16.8s - after git prune --expire now
19.7s - after git rerere gc

status on subdir:
9.7s - after git count-objects -v
9.2s - after git pack-refs --all --prune
9.3s - after git reflog expire --all
4.4s - after git repack -d -l
9.2s - after git prune --expire now
9.0s - after git rerere gc


-- 
Piotr Krukowiecki

^ permalink raw reply

* [PATCH v3] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Matthieu Moy @ 2012-02-16 12:36 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpq39abrxav.fsf@bauges.imag.fr>

git-latexdiff is a wrapper around latexdiff
(http://www.ctan.org/pkg/latexdiff) that allows using it to diff two
revisions of a LaTeX file.

git-latexdiff is made to work on documents split accross multiple .tex
files (plus possibly figures and other non-diffable files), hence could
not be implemented as a per-file diff driver.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Changes since v2:

- Try 'open' on MacOS to view PDF file.
- Shell style issues (thanks to Jakub)

 contrib/latex/Makefile      |   22 ++++
 contrib/latex/README        |   12 ++
 contrib/latex/git-latexdiff |  255 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 289 insertions(+), 0 deletions(-)
 create mode 100644 contrib/latex/Makefile
 create mode 100644 contrib/latex/README
 create mode 100755 contrib/latex/git-latexdiff

diff --git a/contrib/latex/Makefile b/contrib/latex/Makefile
new file mode 100644
index 0000000..4617906
--- /dev/null
+++ b/contrib/latex/Makefile
@@ -0,0 +1,22 @@
+-include ../../config.mak
+-include ../../config.mak.autogen
+
+ifndef SHELL_PATH
+	SHELL_PATH = /bin/sh
+endif
+
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+
+SCRIPT=git-latexdiff
+
+.PHONY: install help
+help:
+	@echo 'This is the help target of the Makefile. Current configuration:'
+	@echo '  gitexecdir = $(gitexecdir_SQ)'
+	@echo '  SHELL_PATH = $(SHELL_PATH_SQ)'
+	@echo 'Run "$(MAKE) install" to install $(SCRIPT) in gitexecdir.'
+
+install:
+	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' $(SCRIPT) > '$(gitexecdir_SQ)/$(SCRIPT)'
+	chmod 755 '$(gitexecdir)/$(SCRIPT)'
diff --git a/contrib/latex/README b/contrib/latex/README
new file mode 100644
index 0000000..2d7fdd6
--- /dev/null
+++ b/contrib/latex/README
@@ -0,0 +1,12 @@
+git-latexdiff is a wrapper around latexdiff
+(http://www.ctan.org/pkg/latexdiff) that allows using it to diff two
+revisions of a LaTeX file.
+
+The script internally checks out the full tree for the specified
+revisions, and calls latexdiff with the --flatten option, hence this
+works if the document is split into multiple .tex files.
+
+Try "git latexdiff -h" for more information.
+
+To install, either drop git-latexdiff in your $PATH, or run "make
+install".
diff --git a/contrib/latex/git-latexdiff b/contrib/latex/git-latexdiff
new file mode 100755
index 0000000..8466f52
--- /dev/null
+++ b/contrib/latex/git-latexdiff
@@ -0,0 +1,255 @@
+#! /bin/sh
+
+# Author: Matthieu Moy <Matthieu.Moy@imag.fr> (2012)
+
+# Missing features (patches welcome ;-) :
+# - diff the index or the current worktree 
+# - checkout only a subdirectory of the repo
+# - hardlink temporary checkouts as much as possible
+
+usage () {
+            cat << EOF
+Usage: $(basename $0) [options] OLD [NEW]
+Call latexdiff on two Git revisions of a file.
+
+OLD and NEW are Git revision identifiers. NEW defaults to HEAD.
+
+Options:
+    --help                this help message
+    --main <file.tex>     name of the main LaTeX file
+    --no-view             don't display the resulting PDF file
+    --view                view the resulting PDF file
+                            (default if -o is not used)
+    --pdf-viewer <cmd>    use <cmd> to view the PDF file (default: \$PDFVIEWER)
+    --no-cleanup          don't cleanup temp dir after running
+    -o <file>, --output <file>
+                          copy resulting PDF into <file>
+                             (usually ending with .pdf)
+EOF
+}
+
+die () {
+    echo "fatal: $@"
+    exit 1
+}
+
+verbose () {
+    if [ "$verbose" = 1 ]; then
+	printf "%s ..." "$@"
+    fi
+}
+
+verbose_progress () {
+    if [ "$verbose" = 1 ]; then
+	printf "." "$@"
+    fi
+}
+
+verbose_done () {
+    if [ "$verbose" = 1 ]; then
+	echo " ${1:-done}."
+    fi
+}
+
+old=
+new=
+main=
+view=maybe
+cleanup=1
+verbose=0
+output=
+initial_dir=$PWD
+
+while test $# -ne 0; do
+    case "$1" in
+        "--help"|"-h")
+            usage
+            exit 0
+            ;;
+	"--main")
+	    shift
+	    main=$1
+	    ;;
+	"--no-view")
+	    view=0
+	    ;;
+	"--view")
+	    view=1
+	    ;;
+	"--pdf-viewer")
+	    shift
+	    PDFVIEWER="$1"
+	    ;;
+	"--no-cleanup")
+	    cleanup=0
+	    ;;
+	"-o"|"--output")
+	    shift
+	    output=$1
+	    ;;
+	"--verbose"|"-v")
+	    verbose=1
+	    ;;
+        *)
+	    if [ "$1" = "" ]; then
+		echo "Empty string not allowed as argument"
+		usage
+		exit 1
+	    elif [ "$old" = "" ]; then
+		old=$1
+	    elif [ "$new" = "" ]; then
+		new=$1
+	    else
+		echo "Bad argument $1"
+		usage
+		exit 1
+	    fi
+            ;;
+    esac
+    shift
+done
+
+if [ "$new" = "" ]; then
+    new=HEAD
+fi
+
+if [ "$old" = "" ]; then
+    echo "fatal: Please, provide at least one revision to diff with."
+    usage
+    exit 1
+fi
+
+verbose "Auto-detecting PDF viewer"
+candidates="xdg-open evince okular xpdf acroread"
+if [ "$(uname)" = Darwin ]; then
+    # open exists on GNU/Linux, but does not open PDFs
+    candidates="open $candidates"
+fi
+
+for command in $candidates; do
+    if [ "$PDFVIEWER" = "" ]; then
+	if command -v "$command" >/dev/null 2>&1; then
+	    PDFVIEWER="$command"
+	else
+	    verbose_progress
+	fi
+    fi
+done
+verbose_done "$PDFVIEWER"
+
+case "$view" in
+    maybe|1)
+	if [ "$PDFVIEWER" = "" ]; then
+	    echo "warning: could not find a PDF viewer on your system."
+	    echo "warning: Please set \$PDFVIEWER or use --pdf-viewer CMD."
+	    PDFVIEWER=false
+	fi
+	;;
+esac
+
+if [ "$main" = "" ]; then
+    printf "%s" "No --main provided, trying to guess ... "
+    main=$(git grep -l '^[ \t]*\\documentclass')
+    # May return multiple results, but if so the result won't be a file.
+    if [ -r "$main" ]; then
+	echo "Using $main as the main file."
+    else
+	if [ "$main" = "" ]; then
+	    echo "No candidate for main file."
+	else
+	    echo "Multiple candidates for main file:"
+	    printf "%s\n" "$main" | sed 's/^/\t/'
+	fi
+	die "Please, provide a main file with --main FILE.tex."
+    fi
+fi
+
+if [ ! -r "$main" ]; then
+    die "Cannot read $main."
+fi
+
+verbose "Creating temporary directories"
+
+git_prefix=$(git rev-parse --show-prefix)
+cd "$(git rev-parse --show-cdup)" || die "Can't cd back to repository root"
+git_dir="$(git rev-parse --git-dir)" || die "Not a git repository?"
+git_dir=$(cd "$git_dir"; pwd)
+
+main=$git_prefix/$main
+
+tmpdir=$initial_dir/git-latexdiff.$$
+mkdir "$tmpdir" || die "Cannot create temporary directory."
+
+cd "$tmpdir" || die "Cannot cd to $tmpdir"
+
+mkdir old new diff || die "Cannot create old, new and diff directories."
+
+verbose_done
+verbose "Checking out old and new version"
+
+cd old || die "Cannot cd to old/"
+git --git-dir="$git_dir" --work-tree=. checkout "$old" -- . || die "checkout failed for old/"
+verbose_progress
+cd ../new || die "Cannot cd to new/"
+git --git-dir="$git_dir" --work-tree=. checkout "$new" -- . || die "checkout failed for new/"
+verbose_progress
+cd ..
+
+verbose_done
+verbose "Running latexdiff --flatten old/$main new/$main > $main"
+
+latexdiff --flatten old/"$main" new/"$main" > diff.tex || die "latexdiff failed"
+
+mv -f diff.tex new/"$main"
+
+verbose_done
+
+mainbase=$(basename "$main" .tex)
+maindir=$(dirname "$main")
+
+verbose "Compiling result"
+
+compile_error=0
+cd new/"$maindir" || die "Can't cd to new/$maindir"
+if [ -f Makefile ]; then
+    make || compile_error=1
+else
+    pdflatex --interaction errorstopmode "$mainbase" || compile_error=1
+fi
+
+verbose_done
+
+pdffile="$mainbase".pdf
+if [ ! -r "$pdffile" ]; then
+    echo "No PDF file generated."
+    compile_error=1
+fi
+
+if [ ! -s "$pdffile" ]; then
+    echo "PDF file generated is empty."
+    compile_error=1
+fi
+
+if [ "$compile_error" = "1" ]; then
+    echo "Error during compilation. Please examine and cleanup if needed:"
+    echo "Directory: $tmpdir/new/$maindir/"
+    echo "     File: $mainbase.tex"
+    # Don't clean up to let the user diagnose.
+    exit 1
+fi
+
+if [ "$output" != "" ]; then
+    abs_pdffile="$PWD/$pdffile"
+    (cd "$initial_dir" && cp "$abs_pdffile" "$output")
+    echo "Output written on $output"
+fi
+
+if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
+    "$PDFVIEWER" "$pdffile"
+fi
+
+if [ "$cleanup" = 1 ]; then
+    verbose "Cleaning-up result"
+    rm -fr "$tmpdir"
+    verbose_done
+fi
-- 
1.7.9.111.gf3fb0.dirty

^ permalink raw reply related

* Re: [PATCH 1/3 v5] diff --stat: tests for long filenames and big change counts
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-16  9:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael J Gruber, pclouds
In-Reply-To: <7vvcn810ml.fsf@alter.siamese.dyndns.org>

On 02/15/2012 06:12 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>
>> Eleven tests for various combinations of a long filename and/or big
>> change count and ways to specify widths for diff --stat.
>> ---
>
> Sign-off?

> Hrm, this does not seem to pass, making the result of applying [1/3] fail;
> I see that the elided name is shown much shorter than the above expects.

Hi,

I'm sorry for not properly testing the patch with tests. I somehow 
convinced myself that the tests pass. This whole series needs more work, 
even after squashing in your two patches. I'll send a new series, but 
probably not today. (I'll try to remember the sign-off too, next time).
--
Zbyszek

^ permalink raw reply

* Re: [PATCH] completion: --list option for git-branch
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-16  9:28 UTC (permalink / raw)
  To: Ralf Thielow; +Cc: spearce, git, gitster
In-Reply-To: <1329338218-9822-1-git-send-email-ralf.thielow@googlemail.com>

On 02/15/2012 09:36 PM, Ralf Thielow wrote:
> Signed-off-by: Ralf Thielow<ralf.thielow@googlemail.com>
> ---
>   contrib/completion/git-completion.bash |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d7367e9..1505cff 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1137,7 +1137,7 @@ _git_branch ()
>   		__gitcomp "
>   			--color --no-color --verbose --abbrev= --no-abbrev
>   			--track --no-track --contains --merged --no-merged
> -			--set-upstream --edit-description
> +			--set-upstream --edit-description --list
>   			"
>   		;;
>   	*)
Normally one would use just a bare 'git branch' to list branches.
Why would you want to use --list in an interactive environment (as 
opposed to a script)? Isn't it better not to clutter the completion
options with something that the user actually has not need for?

Zbyszek

^ permalink raw reply

* Re: [PATCH v2] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Jakub Narebski @ 2012-02-16  9:15 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1329381560-15853-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> +verbose "Auto-detecting PDF viewer"
> +for command in xdg-open evince okular xpdf acroread; do
> +    if [ "$PDFVIEWER" = "" ]; then
> +	if command -v "$command" >/dev/null 2>&1; then
> +	    PDFVIEWER="$command"
> +	else
> +	    verbose_progress
> +	fi
> +    fi
> +done
> +verbose_done "$PDFVIEWER"

Why we autodetect PDF viewer unconditionally?  Why we do not stop on
first detected viewer rather than last?  Why not

  +if [ "$PDFVIEWER" = "" ]; then
  +	verbose "Auto-detecting PDF viewer"
  +	for command in xdg-open evince okular xpdf acroread; do
  +		if command -v "$command" >/dev/null 2>&1; then
  +			PDFVIEWER=$command
  +			break
  +		else
  +			verbose_progress
  +		fi
  +	done
  +	verbose_done "$PDFVIEWER"
  +fi

Nb. Documentation/CodingGuidelines says:

  For shell scripts specifically (not exhaustive):
  
  [...]
  
   - We prefer "test" over "[ ... ]".

I know that 'contrib/' is more relaxed...
-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Matthieu Moy @ 2012-02-16  8:59 UTC (permalink / raw)
  To: Steven Michalske; +Cc: Tim Haga, git, gitster
In-Reply-To: <94614DF7-9EE3-47CB-BC6E-97069597557E@gmail.com>

Steven Michalske <smichalske@gmail.com> writes:

> On Mac OS X use the open command and the OS will use the correct viewer chosen by the user
>
> open "$pdffile"

Unfortunately, open also exists on my system, but does not do that at
all. I'm squashing this into the next version:

--- a/contrib/latex/git-latexdiff
+++ b/contrib/latex/git-latexdiff
@@ -120,7 +120,13 @@ if [ "$old" = "" ]; then
 fi
 
 verbose "Auto-detecting PDF viewer"
-for command in xdg-open evince okular xpdf acroread; do
+candidates="xdg-open evince okular xpdf acroread"
+if [ "$(uname)" = Darwin ]; then
+    # open exists on GNU/Linux, but does not open PDFs
+    candidates="open $candidates"
+fi
+
+for command in $candidates; do
     if [ "$PDFVIEWER" = "" ]; then
        if command -v "$command" >/dev/null 2>&1; then
            PDFVIEWER="$command"

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

^ permalink raw reply

* Re: [PATCH] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Steven Michalske @ 2012-02-16  8:47 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Tim Haga, git, gitster
In-Reply-To: <vpq39abrxav.fsf@bauges.imag.fr>

On Mac OS X use the open command and the OS will use the correct viewer chosen by the user

open "$pdffile"


On Feb 16, 2012, at 12:34 AM, Matthieu Moy wrote:

> Tim Haga <timhaga@ebene6.org> writes:
> 
>> While testing your script on my office machine i discovered that the
>> following might be a problem:
>> 
>>> +if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
>>> +    xpdf "$pdffile"
>>> +fi
>> 
>> Xpdf is not installed on all machines (e.g. it's not installed on my
>> office machine), so maybe it would be a good idea to use a environment
>> variable instead?
> 
> Right. I'm squashing this into the next version to allow configuration
> (environment variable or --pdf-viewer) and sensible auto-detection:
> 
> diff --git a/contrib/latex/git-latexdiff b/contrib/latex/git-latexdiff
> index 13aeb9a..85aafda 100755
> --- a/contrib/latex/git-latexdiff
> +++ b/contrib/latex/git-latexdiff
> @@ -20,6 +20,8 @@ Options:
> 	--no-view	Don't display the resulting PDF file
> 	--view		View the resulting PDF file
> 			(default if -o is not used)
> +	--pdf-viewer CMD
> +			Use CMD to view the PDF file (default: \$PDFVIEWER)
> 	--no-cleanup	Don't cleanup temp dir after running
> 	-o FILE, --output FILE
> 			Copy resulting PDF into FILE
> @@ -46,7 +48,7 @@ verbose_progress () {
> 
> verbose_done () {
>     if [ "$verbose" = 1 ]; then
> -	echo " done."
> +	echo " ${1:-done}."
>     fi
> }
> 
> @@ -75,6 +77,10 @@ while test $# -ne 0; do
> 	"--view")
> 	    view=1
> 	    ;;
> +	"--pdf-viewer")
> +	    shift
> +	    PDFVIEWER="$1"
> +	    ;;
> 	"--no-cleanup")
> 	    cleanup=0
> 	    ;;
> @@ -114,6 +120,28 @@ if [ "$old" = "" ]; then
>     exit 1
> fi
> 
> +verbose "Auto-detecting PDF viewer"
> +for command in xdg-open evince okular xpdf acroread; do
> +    if [ "$PDFVIEWER" = "" ]; then
> +	if command -v "$command" >/dev/null 2>&1; then
> +	    PDFVIEWER="$command"
> +	else
> +	    verbose_progress
> +	fi
> +    fi
> +done
> +verbose_done "$PDFVIEWER"
> +
> +case "$view" in
> +    maybe|1)
> +	if [ "$PDFVIEWER" = "" ]; then
> +	    echo "warning: could not find a PDF viewer on your system."
> +	    echo "warning: Please set \$PDFVIEWER or use --pdf-viewer CMD."
> +	    PDFVIEWER=false
> +	fi
> +	;;
> +esac
> +
> if [ "$main" = "" ]; then
>     printf "%s" "No --main provided, trying to guess ... "
>     main=$(git grep -l '^[ \t]*\\documentclass')
> @@ -212,7 +240,7 @@ if [ "$output" != "" ]; then
> fi
> 
> if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
> -    xpdf "$pdffile"
> +    "$PDFVIEWER" "$pdffile"
> fi
> 
> if [ "$cleanup" = 1 ]; then
> 
> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v2] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Matthieu Moy @ 2012-02-16  8:39 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpq39abrxav.fsf@bauges.imag.fr>

git-latexdiff is a wrapper around latexdiff
(http://www.ctan.org/pkg/latexdiff) that allows using it to diff two
revisions of a LaTeX file.

git-latexdiff is made to work on documents split accross multiple .tex
files (plus possibly figures and other non-diffable files), hence could
not be implemented as a per-file diff driver.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Changes since v1:

- Configuration/autodetection of PDF viewer
- Reformat output of -h to match what PARSE_OPTION does for other commands

 contrib/latex/Makefile      |   22 ++++
 contrib/latex/README        |   12 ++
 contrib/latex/git-latexdiff |  249 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 283 insertions(+), 0 deletions(-)
 create mode 100644 contrib/latex/Makefile
 create mode 100644 contrib/latex/README
 create mode 100755 contrib/latex/git-latexdiff

diff --git a/contrib/latex/Makefile b/contrib/latex/Makefile
new file mode 100644
index 0000000..4617906
--- /dev/null
+++ b/contrib/latex/Makefile
@@ -0,0 +1,22 @@
+-include ../../config.mak
+-include ../../config.mak.autogen
+
+ifndef SHELL_PATH
+	SHELL_PATH = /bin/sh
+endif
+
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+
+SCRIPT=git-latexdiff
+
+.PHONY: install help
+help:
+	@echo 'This is the help target of the Makefile. Current configuration:'
+	@echo '  gitexecdir = $(gitexecdir_SQ)'
+	@echo '  SHELL_PATH = $(SHELL_PATH_SQ)'
+	@echo 'Run "$(MAKE) install" to install $(SCRIPT) in gitexecdir.'
+
+install:
+	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' $(SCRIPT) > '$(gitexecdir_SQ)/$(SCRIPT)'
+	chmod 755 '$(gitexecdir)/$(SCRIPT)'
diff --git a/contrib/latex/README b/contrib/latex/README
new file mode 100644
index 0000000..2d7fdd6
--- /dev/null
+++ b/contrib/latex/README
@@ -0,0 +1,12 @@
+git-latexdiff is a wrapper around latexdiff
+(http://www.ctan.org/pkg/latexdiff) that allows using it to diff two
+revisions of a LaTeX file.
+
+The script internally checks out the full tree for the specified
+revisions, and calls latexdiff with the --flatten option, hence this
+works if the document is split into multiple .tex files.
+
+Try "git latexdiff -h" for more information.
+
+To install, either drop git-latexdiff in your $PATH, or run "make
+install".
diff --git a/contrib/latex/git-latexdiff b/contrib/latex/git-latexdiff
new file mode 100755
index 0000000..57c2237
--- /dev/null
+++ b/contrib/latex/git-latexdiff
@@ -0,0 +1,249 @@
+#! /bin/sh
+
+# Author: Matthieu Moy <Matthieu.Moy@imag.fr> (2012)
+
+# Missing features (patches welcome ;-) :
+# - diff the index or the current worktree 
+# - checkout only a subdirectory of the repo
+# - hardlink temporary checkouts as much as possible
+
+usage () {
+            cat << EOF
+Usage: $(basename $0) [options] OLD [NEW]
+Call latexdiff on two Git revisions of a file.
+
+OLD and NEW are Git revision identifiers. NEW defaults to HEAD.
+
+Options:
+    --help                this help message
+    --main <file.tex>     name of the main LaTeX file
+    --no-view             don't display the resulting PDF file
+    --view                view the resulting PDF file
+                            (default if -o is not used)
+    --pdf-viewer <cmd>    use <cmd> to view the PDF file (default: \$PDFVIEWER)
+    --no-cleanup          don't cleanup temp dir after running
+    -o <file>, --output <file>
+                          copy resulting PDF into <file>
+                             (usually ending with .pdf)
+EOF
+}
+
+die () {
+    echo "fatal: $@"
+    exit 1
+}
+
+verbose () {
+    if [ "$verbose" = 1 ]; then
+	printf "%s ..." "$@"
+    fi
+}
+
+verbose_progress () {
+    if [ "$verbose" = 1 ]; then
+	printf "." "$@"
+    fi
+}
+
+verbose_done () {
+    if [ "$verbose" = 1 ]; then
+	echo " ${1:-done}."
+    fi
+}
+
+old=
+new=
+main=
+view=maybe
+cleanup=1
+verbose=0
+output=
+initial_dir=$PWD
+
+while test $# -ne 0; do
+    case "$1" in
+        "--help"|"-h")
+            usage
+            exit 0
+            ;;
+	"--main")
+	    shift
+	    main=$1
+	    ;;
+	"--no-view")
+	    view=0
+	    ;;
+	"--view")
+	    view=1
+	    ;;
+	"--pdf-viewer")
+	    shift
+	    PDFVIEWER="$1"
+	    ;;
+	"--no-cleanup")
+	    cleanup=0
+	    ;;
+	"-o"|"--output")
+	    shift
+	    output=$1
+	    ;;
+	"--verbose"|"-v")
+	    verbose=1
+	    ;;
+        *)
+	    if [ "$1" = "" ]; then
+		echo "Empty string not allowed as argument"
+		usage
+		exit 1
+	    elif [ "$old" = "" ]; then
+		old=$1
+	    elif [ "$new" = "" ]; then
+		new=$1
+	    else
+		echo "Bad argument $1"
+		usage
+		exit 1
+	    fi
+            ;;
+    esac
+    shift
+done
+
+if [ "$new" = "" ]; then
+    new=HEAD
+fi
+
+if [ "$old" = "" ]; then
+    echo "fatal: Please, provide at least one revision to diff with."
+    usage
+    exit 1
+fi
+
+verbose "Auto-detecting PDF viewer"
+for command in xdg-open evince okular xpdf acroread; do
+    if [ "$PDFVIEWER" = "" ]; then
+	if command -v "$command" >/dev/null 2>&1; then
+	    PDFVIEWER="$command"
+	else
+	    verbose_progress
+	fi
+    fi
+done
+verbose_done "$PDFVIEWER"
+
+case "$view" in
+    maybe|1)
+	if [ "$PDFVIEWER" = "" ]; then
+	    echo "warning: could not find a PDF viewer on your system."
+	    echo "warning: Please set \$PDFVIEWER or use --pdf-viewer CMD."
+	    PDFVIEWER=false
+	fi
+	;;
+esac
+
+if [ "$main" = "" ]; then
+    printf "%s" "No --main provided, trying to guess ... "
+    main=$(git grep -l '^[ \t]*\\documentclass')
+    # May return multiple results, but if so the result won't be a file.
+    if [ -r "$main" ]; then
+	echo "Using $main as the main file."
+    else
+	if [ "$main" = "" ]; then
+	    echo "No candidate for main file."
+	else
+	    echo "Multiple candidates for main file:"
+	    printf "%s\n" "$main" | sed 's/^/\t/'
+	fi
+	die "Please, provide a main file with --main FILE.tex."
+    fi
+fi
+
+if [ ! -r "$main" ]; then
+    die "Cannot read $main."
+fi
+
+verbose "Creating temporary directories"
+
+git_prefix=$(git rev-parse --show-prefix)
+cd "$(git rev-parse --show-cdup)" || die "Can't cd back to repository root"
+git_dir="$(git rev-parse --git-dir)" || die "Not a git repository?"
+git_dir=$(cd "$git_dir"; pwd)
+
+main=$git_prefix/$main
+
+tmpdir=$initial_dir/git-latexdiff.$$
+mkdir "$tmpdir" || die "Cannot create temporary directory."
+
+cd "$tmpdir" || die "Cannot cd to $tmpdir"
+
+mkdir old new diff || die "Cannot create old, new and diff directories."
+
+verbose_done
+verbose "Checking out old and new version"
+
+cd old || die "Cannot cd to old/"
+git --git-dir="$git_dir" --work-tree=. checkout "$old" -- . || die "checkout failed for old/"
+verbose_progress
+cd ../new || die "Cannot cd to new/"
+git --git-dir="$git_dir" --work-tree=. checkout "$new" -- . || die "checkout failed for new/"
+verbose_progress
+cd ..
+
+verbose_done
+verbose "Running latexdiff --flatten old/$main new/$main > $main"
+
+latexdiff --flatten old/"$main" new/"$main" > diff.tex || die "latexdiff failed"
+
+mv -f diff.tex new/"$main"
+
+verbose_done
+
+mainbase=$(basename "$main" .tex)
+maindir=$(dirname "$main")
+
+verbose "Compiling result"
+
+compile_error=0
+cd new/"$maindir" || die "Can't cd to new/$maindir"
+if [ -f Makefile ]; then
+    make || compile_error=1
+else
+    pdflatex --interaction errorstopmode "$mainbase" || compile_error=1
+fi
+
+verbose_done
+
+pdffile="$mainbase".pdf
+if [ ! -r "$pdffile" ]; then
+    echo "No PDF file generated."
+    compile_error=1
+fi
+
+if [ ! -s "$pdffile" ]; then
+    echo "PDF file generated is empty."
+    compile_error=1
+fi
+
+if [ "$compile_error" = "1" ]; then
+    echo "Error during compilation. Please examine and cleanup if needed:"
+    echo "Directory: $tmpdir/new/$maindir/"
+    echo "     File: $mainbase.tex"
+    # Don't clean up to let the user diagnose.
+    exit 1
+fi
+
+if [ "$output" != "" ]; then
+    abs_pdffile="$PWD/$pdffile"
+    (cd "$initial_dir" && cp "$abs_pdffile" "$output")
+    echo "Output written on $output"
+fi
+
+if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
+    "$PDFVIEWER" "$pdffile"
+fi
+
+if [ "$cleanup" = 1 ]; then
+    verbose "Cleaning-up result"
+    rm -fr "$tmpdir"
+    verbose_done
+fi
-- 
1.7.9.111.gf3fb0.dirty

^ permalink raw reply related

* Re: [PATCH] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Matthieu Moy @ 2012-02-16  8:34 UTC (permalink / raw)
  To: Tim Haga; +Cc: git, gitster
In-Reply-To: <20120216003300.17228570@sirion>

Tim Haga <timhaga@ebene6.org> writes:

> While testing your script on my office machine i discovered that the
> following might be a problem:
>
>> +if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
>> +    xpdf "$pdffile"
>> +fi
>
> Xpdf is not installed on all machines (e.g. it's not installed on my
> office machine), so maybe it would be a good idea to use a environment
> variable instead?

Right. I'm squashing this into the next version to allow configuration
(environment variable or --pdf-viewer) and sensible auto-detection:

diff --git a/contrib/latex/git-latexdiff b/contrib/latex/git-latexdiff
index 13aeb9a..85aafda 100755
--- a/contrib/latex/git-latexdiff
+++ b/contrib/latex/git-latexdiff
@@ -20,6 +20,8 @@ Options:
 	--no-view	Don't display the resulting PDF file
 	--view		View the resulting PDF file
 			(default if -o is not used)
+	--pdf-viewer CMD
+			Use CMD to view the PDF file (default: \$PDFVIEWER)
 	--no-cleanup	Don't cleanup temp dir after running
 	-o FILE, --output FILE
 			Copy resulting PDF into FILE
@@ -46,7 +48,7 @@ verbose_progress () {
 
 verbose_done () {
     if [ "$verbose" = 1 ]; then
-	echo " done."
+	echo " ${1:-done}."
     fi
 }
 
@@ -75,6 +77,10 @@ while test $# -ne 0; do
 	"--view")
 	    view=1
 	    ;;
+	"--pdf-viewer")
+	    shift
+	    PDFVIEWER="$1"
+	    ;;
 	"--no-cleanup")
 	    cleanup=0
 	    ;;
@@ -114,6 +120,28 @@ if [ "$old" = "" ]; then
     exit 1
 fi
 
+verbose "Auto-detecting PDF viewer"
+for command in xdg-open evince okular xpdf acroread; do
+    if [ "$PDFVIEWER" = "" ]; then
+	if command -v "$command" >/dev/null 2>&1; then
+	    PDFVIEWER="$command"
+	else
+	    verbose_progress
+	fi
+    fi
+done
+verbose_done "$PDFVIEWER"
+
+case "$view" in
+    maybe|1)
+	if [ "$PDFVIEWER" = "" ]; then
+	    echo "warning: could not find a PDF viewer on your system."
+	    echo "warning: Please set \$PDFVIEWER or use --pdf-viewer CMD."
+	    PDFVIEWER=false
+	fi
+	;;
+esac
+
 if [ "$main" = "" ]; then
     printf "%s" "No --main provided, trying to guess ... "
     main=$(git grep -l '^[ \t]*\\documentclass')
@@ -212,7 +240,7 @@ if [ "$output" != "" ]; then
 fi
 
 if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
-    xpdf "$pdffile"
+    "$PDFVIEWER" "$pdffile"
 fi
 
 if [ "$cleanup" = 1 ]; then

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

^ permalink raw reply related

* [PATCH 8/8] config: do not respect includes for single-file --list
From: Jeff King @ 2012-02-16  8:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

The original include implementation tried not to impact
calls to "git config" that look at a single file. However,
since we called into git_config in a few places (e.g.,
--list), our respect_includes flag was not supported.

This patch teaches git_config_with_options a flag to
respect includes (instead of doing so by default), and
teaches git-config to use it.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/config.c          |    7 ++++---
 cache.h                   |    3 ++-
 config.c                  |   14 ++++++++------
 t/t1305-config-include.sh |    8 ++++++++
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 8901dd9..d41a9bf 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -316,7 +316,7 @@ static void get_color(const char *def_color)
 	get_color_found = 0;
 	parsed_color[0] = '\0';
 	git_config_with_options(git_get_color_config, NULL,
-				given_config_file);
+				given_config_file, respect_includes);
 
 	if (!get_color_found && def_color)
 		color_parse(def_color, "command line", parsed_color);
@@ -344,7 +344,7 @@ static int get_colorbool(int print)
 	get_colorbool_found = -1;
 	get_diff_color_found = -1;
 	git_config_with_options(git_get_colorbool_config, NULL,
-				given_config_file);
+				given_config_file, respect_includes);
 
 	if (get_colorbool_found < 0) {
 		if (!strcmp(get_colorbool_slot, "color.diff"))
@@ -441,7 +441,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
 		if (git_config_with_options(show_all_config, NULL,
-					    given_config_file) < 0) {
+					    given_config_file,
+					    respect_includes) < 0) {
 			if (given_config_file)
 				die_errno("unable to read config file '%s'",
 					  given_config_file);
diff --git a/cache.h b/cache.h
index 411c60d..ff54d6f 100644
--- a/cache.h
+++ b/cache.h
@@ -1115,7 +1115,8 @@ extern int git_config_from_file(config_fn_t fn, const char *, void *);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
 extern int git_config(config_fn_t fn, void *);
-extern int git_config_with_options(config_fn_t fn, void *, const char *filename);
+extern int git_config_with_options(config_fn_t fn, void *,
+				   const char *filename, int respect_includes);
 extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
 extern int git_parse_ulong(const char *, unsigned long *);
 extern int git_config_int(const char *, const char *);
diff --git a/config.c b/config.c
index e1d6857..ad03908 100644
--- a/config.c
+++ b/config.c
@@ -976,16 +976,18 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 }
 
 int git_config_with_options(config_fn_t fn, void *data,
-			    const char *filename)
+			    const char *filename, int respect_includes)
 {
 	char *repo_config = NULL;
 	int ret;
 	struct config_include_data inc = CONFIG_INCLUDE_INIT;
 
-	inc.fn = fn;
-	inc.data = data;
-	fn = git_config_include;
-	data = &inc;
+	if (respect_includes) {
+		inc.fn = fn;
+		inc.data = data;
+		fn = git_config_include;
+		data = &inc;
+	}
 
 	/*
 	 * If we have a specific filename, use it. Otherwise, follow the
@@ -1003,7 +1005,7 @@ int git_config_with_options(config_fn_t fn, void *data,
 
 int git_config(config_fn_t fn, void *data)
 {
-	return git_config_with_options(fn, data, NULL);
+	return git_config_with_options(fn, data, NULL, 1);
 }
 
 /*
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 0a27ec4..f3e03a0 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -59,6 +59,14 @@ test_expect_success 'single file lookup does not expand includes by default' '
 	test_cmp expect actual
 '
 
+test_expect_success 'single file list does not expand includes by default' '
+	echo "[test]one = 1" >one &&
+	echo "[include]path = one" >.gitconfig &&
+	echo "include.path=one" >expect &&
+	git config -f .gitconfig --list >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'writing config file does not expand includes' '
 	echo "[test]one = 1" >one &&
 	echo "[include]path = one" >.gitconfig &&
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 7/8] config: eliminate config_exclusive_filename
From: Jeff King @ 2012-02-16  8:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

This is a magic global variable that was intended as an
override to the usual git-config lookup process. Once upon a
time, you could specify GIT_CONFIG to any git program, and
it would look only at that file. This turned out to be
confusing and cause a lot of bugs for little gain. As a
result, dc87183 (Only use GIT_CONFIG in "git config", not
other programs, 2008-06-30) took this away for all callers
except git-config.

Since git-config no longer uses it either, the variable can
just go away. As the diff shows, nobody was setting to
anything except NULL, so we can just replace any sites where
it was read with NULL.

Signed-off-by: Jeff King <peff@peff.net>
---
This could be squashed into the last patch (really, all of the last few
patches could be squashed). But I was able to "git grep
config_exclusive_filename" at this state and see that yes, indeed, the
variable is now totally useless.

 cache.h  |    2 --
 config.c |   10 +++-------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 7cb8874..411c60d 100644
--- a/cache.h
+++ b/cache.h
@@ -1150,8 +1150,6 @@ struct config_include_data {
 #define CONFIG_INCLUDE_INIT { 0 }
 extern int git_config_include(const char *name, const char *value, void *data);
 
-extern const char *config_exclusive_filename;
-
 #define MAX_GITNAME (1000)
 extern char git_default_email[MAX_GITNAME];
 extern char git_default_name[MAX_GITNAME];
diff --git a/config.c b/config.c
index fbf883d..e1d6857 100644
--- a/config.c
+++ b/config.c
@@ -26,8 +26,6 @@ static config_file *cf;
 
 static int zlib_compression_seen;
 
-const char *config_exclusive_filename = NULL;
-
 #define MAX_INCLUDE_DEPTH 10
 static const char include_depth_advice[] =
 "exceeded maximum include depth (%d) while including\n"
@@ -1005,7 +1003,7 @@ int git_config_with_options(config_fn_t fn, void *data,
 
 int git_config(config_fn_t fn, void *data)
 {
-	return git_config_with_options(fn, data, config_exclusive_filename);
+	return git_config_with_options(fn, data, NULL);
 }
 
 /*
@@ -1504,8 +1502,7 @@ write_err_out:
 int git_config_set_multivar(const char *key, const char *value,
 			const char *value_regex, int multi_replace)
 {
-	return git_config_set_multivar_in_file(config_exclusive_filename,
-					       key, value, value_regex,
+	return git_config_set_multivar_in_file(NULL, key, value, value_regex,
 					       multi_replace);
 }
 
@@ -1631,8 +1628,7 @@ out:
 
 int git_config_rename_section(const char *old_name, const char *new_name)
 {
-	return git_config_rename_section_in_file(config_exclusive_filename,
-						 old_name, new_name);
+	return git_config_rename_section_in_file(NULL, old_name, new_name);
 }
 
 /*
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 6/8] config: stop using config_exclusive_filename
From: Jeff King @ 2012-02-16  8:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

The git-config command sometimes operates on the default set
of config files (either reading from all, or writing to repo
config), and sometimes operates on a specific file. In the
latter case, we set the magic global config_exclusive_filename,
and the code in config.c does the right thing.

Instead, let's have git-config use the "advanced" variants
of config.c's functions which let it specify an individual
filename (or NULL for the default). This makes the code a
lot more obvious, and fixes two small bugs:

  1. A relative path specified by GIT_CONFIG=foo will look
     in the wrong directory if we have to chdir as part of
     repository setup. We already handle this properly for
     "git config -f foo", but the GIT_CONFIG lookup used
     config_exclusive_filename directly. By dropping to a
     single magic variable, the GIT_CONFIG case now just
     works.

  2. Calling "git config -f foo --edit" would not respect
     core.editor. This is because just before editing, we
     called git_config, which would respect the
     config_exclusive_filename setting, even though this
     particular git_config call was not about looking in the
     user's specified file, but rather about loading actual
     git config, just as any other git program would.

Signed-off-by: Jeff King <peff@peff.net>
---
The diffstat is slightly disappointing. It really is a one-for-one
conversion in almost every case, but the functions and variable names
are so bloody long that I had to wrap the calls to stay under 80
characters.

 builtin/config.c       |   61 ++++++++++++++++++++++++++++-------------------
 t/t1300-repo-config.sh |   25 +++++++++++++++++++
 2 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 5a43a3c..8901dd9 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -167,7 +167,7 @@ static int get_value(const char *key_, const char *regex_)
 	config_fn_t fn;
 	void *data;
 
-	local = config_exclusive_filename;
+	local = given_config_file;
 	if (!local) {
 		const char *home = getenv("HOME");
 		local = repo_config = git_pathdup("config");
@@ -315,7 +315,8 @@ static void get_color(const char *def_color)
 {
 	get_color_found = 0;
 	parsed_color[0] = '\0';
-	git_config(git_get_color_config, NULL);
+	git_config_with_options(git_get_color_config, NULL,
+				given_config_file);
 
 	if (!get_color_found && def_color)
 		color_parse(def_color, "command line", parsed_color);
@@ -342,7 +343,8 @@ static int get_colorbool(int print)
 {
 	get_colorbool_found = -1;
 	get_diff_color_found = -1;
-	git_config(git_get_colorbool_config, NULL);
+	git_config_with_options(git_get_colorbool_config, NULL,
+				given_config_file);
 
 	if (get_colorbool_found < 0) {
 		if (!strcmp(get_colorbool_slot, "color.diff"))
@@ -365,7 +367,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	int nongit = !startup_info->have_repository;
 	char *value;
 
-	config_exclusive_filename = getenv(CONFIG_ENVIRONMENT);
+	given_config_file = getenv(CONFIG_ENVIRONMENT);
 
 	argc = parse_options(argc, argv, prefix, builtin_config_options,
 			     builtin_config_usage,
@@ -380,27 +382,27 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		char *home = getenv("HOME");
 		if (home) {
 			char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
-			config_exclusive_filename = user_config;
+			given_config_file = user_config;
 		} else {
 			die("$HOME not set");
 		}
 	}
 	else if (use_system_config)
-		config_exclusive_filename = git_etc_gitconfig();
+		given_config_file = git_etc_gitconfig();
 	else if (use_local_config)
-		config_exclusive_filename = git_pathdup("config");
+		given_config_file = git_pathdup("config");
 	else if (given_config_file) {
 		if (!is_absolute_path(given_config_file) && prefix)
-			config_exclusive_filename =
+			given_config_file =
 				xstrdup(prefix_filename(prefix,
 							strlen(prefix),
 							given_config_file));
 		else
-			config_exclusive_filename = given_config_file;
+			given_config_file = given_config_file;
 	}
 
 	if (respect_includes == -1)
-		respect_includes = !config_exclusive_filename;
+		respect_includes = !given_config_file;
 
 	if (end_null) {
 		term = '\0';
@@ -438,28 +440,29 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 	if (actions == ACTION_LIST) {
 		check_argc(argc, 0, 0);
-		if (git_config(show_all_config, NULL) < 0) {
-			if (config_exclusive_filename)
+		if (git_config_with_options(show_all_config, NULL,
+					    given_config_file) < 0) {
+			if (given_config_file)
 				die_errno("unable to read config file '%s'",
-					  config_exclusive_filename);
+					  given_config_file);
 			else
 				die("error processing config file(s)");
 		}
 	}
 	else if (actions == ACTION_EDIT) {
 		check_argc(argc, 0, 0);
-		if (!config_exclusive_filename && nongit)
+		if (!given_config_file && nongit)
 			die("not in a git directory");
 		git_config(git_default_config, NULL);
-		launch_editor(config_exclusive_filename ?
-			      config_exclusive_filename : git_path("config"),
+		launch_editor(given_config_file ?
+			      given_config_file : git_path("config"),
 			      NULL, NULL);
 	}
 	else if (actions == ACTION_SET) {
 		int ret;
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
-		ret = git_config_set(argv[0], value);
+		ret = git_config_set_in_file(given_config_file, argv[0], value);
 		if (ret == CONFIG_NOTHING_SET)
 			error("cannot overwrite multiple values with a single value\n"
 			"       Use a regexp, --add or --replace-all to change %s.", argv[0]);
@@ -468,17 +471,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	else if (actions == ACTION_SET_ALL) {
 		check_argc(argc, 2, 3);
 		value = normalize_value(argv[0], argv[1]);
-		return git_config_set_multivar(argv[0], value, argv[2], 0);
+		return git_config_set_multivar_in_file(given_config_file,
+						       argv[0], value, argv[2], 0);
 	}
 	else if (actions == ACTION_ADD) {
 		check_argc(argc, 2, 2);
 		value = normalize_value(argv[0], argv[1]);
-		return git_config_set_multivar(argv[0], value, "^$", 0);
+		return git_config_set_multivar_in_file(given_config_file,
+						       argv[0], value, "^$", 0);
 	}
 	else if (actions == ACTION_REPLACE_ALL) {
 		check_argc(argc, 2, 3);
 		value = normalize_value(argv[0], argv[1]);
-		return git_config_set_multivar(argv[0], value, argv[2], 1);
+		return git_config_set_multivar_in_file(given_config_file,
+						       argv[0], value, argv[2], 1);
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);
@@ -499,18 +505,22 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	else if (actions == ACTION_UNSET) {
 		check_argc(argc, 1, 2);
 		if (argc == 2)
-			return git_config_set_multivar(argv[0], NULL, argv[1], 0);
+			return git_config_set_multivar_in_file(given_config_file,
+							       argv[0], NULL, argv[1], 0);
 		else
-			return git_config_set(argv[0], NULL);
+			return git_config_set_in_file(given_config_file,
+						      argv[0], NULL);
 	}
 	else if (actions == ACTION_UNSET_ALL) {
 		check_argc(argc, 1, 2);
-		return git_config_set_multivar(argv[0], NULL, argv[1], 1);
+		return git_config_set_multivar_in_file(given_config_file,
+						       argv[0], NULL, argv[1], 1);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;
 		check_argc(argc, 2, 2);
-		ret = git_config_rename_section(argv[0], argv[1]);
+		ret = git_config_rename_section_in_file(given_config_file,
+							argv[0], argv[1]);
 		if (ret < 0)
 			return ret;
 		if (ret == 0)
@@ -519,7 +529,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	else if (actions == ACTION_REMOVE_SECTION) {
 		int ret;
 		check_argc(argc, 1, 1);
-		ret = git_config_rename_section(argv[0], NULL);
+		ret = git_config_rename_section_in_file(given_config_file,
+							argv[0], NULL);
 		if (ret < 0)
 			return ret;
 		if (ret == 0)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6de46bb..5f249f6 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -458,6 +458,14 @@ test_expect_success 'refer config from subdirectory' '
 
 '
 
+test_expect_success 'refer config from subdirectory via GIT_CONFIG' '
+	(
+		cd x &&
+		GIT_CONFIG=../other-config git config --get ein.bahn >actual &&
+		test_cmp expect actual
+	)
+'
+
 cat > expect << EOF
 [ein]
 	bahn = strasse
@@ -960,4 +968,21 @@ test_expect_success 'git -c complains about empty key and value' '
 	test_must_fail git -c "" rev-parse
 '
 
+test_expect_success 'git config --edit works' '
+	git config -f tmp test.value no &&
+	echo test.value=yes >expect &&
+	GIT_EDITOR="echo [test]value=yes >" git config -f tmp --edit &&
+	git config -f tmp --list >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git config --edit respects core.editor' '
+	git config -f tmp test.value no &&
+	echo test.value=yes >expect &&
+	test_config core.editor "echo [test]value=yes >" &&
+	git config -f tmp --edit &&
+	git config -f tmp --list >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 5/8] config: provide a version of git_config with more options
From: Jeff King @ 2012-02-16  8:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

Callers may want to provide a specific version of a file in
which to look for config. Right now this can be done by
setting the magic global config_exclusive_filename variable.
By providing a version of git_config that takes a filename,
we can take a step towards making this magic global go away.

Furthermore, by providing a more "advanced" interface, we
now have a a natural place to add new options for callers
like git-config, which care about tweaking the specifics of
config lookup, without disturbing the large number of
"simple" users (i.e., every other part of git).

The astute reader of this patch may notice that the logic
for handling config_exclusive_filename was taken out of
git_config_early, but added into git_config. This means that
git_config_early will no longer respect config_exclusive_filename.
That's OK, because the only other caller of git_config_early
is check_repository_format_gently, but the only function
which sets config_exclusive_filename is cmd_config, which
does not call check_repository_format_gently (and if it did,
it would have been a bug, anyway, as we would be checking
the repository format in the wrong file).

Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this could also learn about a respect_includes flag, and it in
fact does in patch 8. I didn't include it in the initial version because
this part of the series is really about refactoring the
config_exclusive_filename stuff (and is semantically independent of the
notion of included files).

 cache.h  |    1 +
 config.c |   22 +++++++++++++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 7e375ce..7cb8874 100644
--- a/cache.h
+++ b/cache.h
@@ -1115,6 +1115,7 @@ extern int git_config_from_file(config_fn_t fn, const char *, void *);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
 extern int git_config(config_fn_t fn, void *);
+extern int git_config_with_options(config_fn_t fn, void *, const char *filename);
 extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
 extern int git_parse_ulong(const char *, unsigned long *);
 extern int git_config_int(const char *, const char *);
diff --git a/config.c b/config.c
index c456600..fbf883d 100644
--- a/config.c
+++ b/config.c
@@ -942,9 +942,6 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 	int ret = 0, found = 0;
 	const char *home = NULL;
 
-	/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
-	if (config_exclusive_filename)
-		return git_config_from_file(fn, config_exclusive_filename, data);
 	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
 					    data);
@@ -980,7 +977,8 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 	return ret == 0 ? found : ret;
 }
 
-int git_config(config_fn_t fn, void *data)
+int git_config_with_options(config_fn_t fn, void *data,
+			    const char *filename)
 {
 	char *repo_config = NULL;
 	int ret;
@@ -988,14 +986,28 @@ int git_config(config_fn_t fn, void *data)
 
 	inc.fn = fn;
 	inc.data = data;
+	fn = git_config_include;
+	data = &inc;
+
+	/*
+	 * If we have a specific filename, use it. Otherwise, follow the
+	 * regular lookup sequence.
+	 */
+	if (filename)
+		return git_config_from_file(fn, filename, data);
 
 	repo_config = git_pathdup("config");
-	ret = git_config_early(git_config_include, &inc, repo_config);
+	ret = git_config_early(fn, data, repo_config);
 	if (repo_config)
 		free(repo_config);
 	return ret;
 }
 
+int git_config(config_fn_t fn, void *data)
+{
+	return git_config_with_options(fn, data, config_exclusive_filename);
+}
+
 /*
  * Find all the stuff for git_config_set() below.
  */
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 4/8] config: teach git_config_rename_section a file argument
From: Jeff King @ 2012-02-16  8:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

The other config-writing functions (git_config_set and
git_config_set_multivar) each have an -"in_file" version to
write a specific file. Let's add one for rename_section,
with the eventual goal of moving away from the magic
config_exclusive_filename global.

Signed-off-by: Jeff King <peff@peff.net>
---
 cache.h  |    1 +
 config.c |   20 +++++++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 94f3eaf..7e375ce 100644
--- a/cache.h
+++ b/cache.h
@@ -1130,6 +1130,7 @@ extern int git_config_parse_key(const char *, char **, int *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
+extern int git_config_rename_section_in_file(const char *, const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int check_repository_format_version(const char *var, const char *value, void *cb);
 extern int git_env_bool(const char *, int);
diff --git a/config.c b/config.c
index a9eec58..c456600 100644
--- a/config.c
+++ b/config.c
@@ -1537,19 +1537,19 @@ static int section_name_match (const char *buf, const char *name)
 }
 
 /* if new_name == NULL, the section is removed instead */
-int git_config_rename_section(const char *old_name, const char *new_name)
+int git_config_rename_section_in_file(const char *config_filename,
+				      const char *old_name, const char *new_name)
 {
 	int ret = 0, remove = 0;
-	char *config_filename;
+	char *filename_buf = NULL;
 	struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
 	int out_fd;
 	char buf[1024];
 	FILE *config_file;
 
-	if (config_exclusive_filename)
-		config_filename = xstrdup(config_exclusive_filename);
-	else
-		config_filename = git_pathdup("config");
+	if (!config_filename)
+		config_filename = filename_buf = git_pathdup("config");
+
 	out_fd = hold_lock_file_for_update(lock, config_filename, 0);
 	if (out_fd < 0) {
 		ret = error("could not lock config file %s", config_filename);
@@ -1613,10 +1613,16 @@ unlock_and_out:
 	if (commit_lock_file(lock) < 0)
 		ret = error("could not commit config file %s", config_filename);
 out:
-	free(config_filename);
+	free(filename_buf);
 	return ret;
 }
 
+int git_config_rename_section(const char *old_name, const char *new_name)
+{
+	return git_config_rename_section_in_file(config_exclusive_filename,
+						 old_name, new_name);
+}
+
 /*
  * Call this to report error for your variable that should not
  * get a boolean value (i.e. "[my] var" means "true").
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 3/8] config: teach git_config_set_multivar_in_file a default path
From: Jeff King @ 2012-02-16  8:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

The git_config_set_multivar_in_file function takes a
filename argument to specify the file into which the values
should be written. Currently, this value must be non-NULL.
Callers which want to write to the default location must use
the regular, non-"in_file" version, which will either write
to config_exclusive_filename, or to the repo config if the
exclusive filename is NULL.

Let's migrate the "default to using repo config" logic into
the "in_file" form. That will let callers get the same
default-if-NULL behavior as one gets with
config_exclusive_filename, but without having to use the
global variable.

Signed-off-by: Jeff King <peff@peff.net>
---
 config.c |   20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/config.c b/config.c
index e3fcf75..a9eec58 100644
--- a/config.c
+++ b/config.c
@@ -1300,6 +1300,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
 	int fd = -1, in_fd;
 	int ret;
 	struct lock_file *lock = NULL;
+	char *filename_buf = NULL;
 
 	/* parse-key returns negative; flip the sign to feed exit(3) */
 	ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
@@ -1308,6 +1309,8 @@ int git_config_set_multivar_in_file(const char *config_filename,
 
 	store.multi_replace = multi_replace;
 
+	if (!config_filename)
+		config_filename = filename_buf = git_pathdup("config");
 
 	/*
 	 * The lock serves a purpose in addition to locking: the new
@@ -1477,6 +1480,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
 out_free:
 	if (lock)
 		rollback_lock_file(lock);
+	free(filename_buf);
 	return ret;
 
 write_err_out:
@@ -1488,19 +1492,9 @@ write_err_out:
 int git_config_set_multivar(const char *key, const char *value,
 			const char *value_regex, int multi_replace)
 {
-	const char *config_filename;
-	char *buf = NULL;
-	int ret;
-
-	if (config_exclusive_filename)
-		config_filename = config_exclusive_filename;
-	else
-		config_filename = buf = git_pathdup("config");
-
-	ret = git_config_set_multivar_in_file(config_filename, key, value,
-					value_regex, multi_replace);
-	free(buf);
-	return ret;
+	return git_config_set_multivar_in_file(config_exclusive_filename,
+					       key, value, value_regex,
+					       multi_replace);
 }
 
 static int section_name_match (const char *buf, const char *name)
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 2/8] config: copy the return value of prefix_filename
From: Jeff King @ 2012-02-16  8:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

The prefix_filename function returns a pointer to a static
buffer which may be overwritten by subsequent calls. Since
we are going to keep the result around for a while, let's be
sure to duplicate it for safety.

I don't think this can be triggered as a bug in the current
code, but it's a good idea to be defensive, as any resulting
bug would be quite subtle.

Signed-off-by: Jeff King <peff@peff.net>
---
I was tempted to simply use OPT_FILENAME to get the argument to "-f",
which would handle the prefixing for us. However, we can also get the
value from $GIT_CONFIG. That value doesn't actually run through this
code currently, but I think it should (and I fix this in patch 6).

 builtin/config.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 09bf778..5a43a3c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -391,9 +391,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		config_exclusive_filename = git_pathdup("config");
 	else if (given_config_file) {
 		if (!is_absolute_path(given_config_file) && prefix)
-			config_exclusive_filename = prefix_filename(prefix,
-								    strlen(prefix),
-								    given_config_file);
+			config_exclusive_filename =
+				xstrdup(prefix_filename(prefix,
+							strlen(prefix),
+							given_config_file));
 		else
 			config_exclusive_filename = given_config_file;
 	}
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 1/8] t1300: add missing &&-chaining
From: Jeff King @ 2012-02-16  8:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1300-repo-config.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 0690e0e..6de46bb 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -451,7 +451,7 @@ test_expect_success 'refer config from subdirectory' '
 	mkdir x &&
 	(
 		cd x &&
-		echo strasse >expect
+		echo strasse >expect &&
 		git config --get --file ../other-config ein.bahn >actual &&
 		test_cmp expect actual
 	)
-- 
1.7.9.1.4.g8ffed

^ permalink raw reply related

* [PATCH 0/8] config-include fixes
From: Jeff King @ 2012-02-16  8:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20120214220953.GC24802@sigill.intra.peff.net>

On Tue, Feb 14, 2012 at 05:09:53PM -0500, Jeff King wrote:

> > Hmm, I thought t1305 covered "config --list", and ... oops, it makes sure
> > the output contains the inclusion.
> 
> Yes. It should include it (and does correctly) when not using any
> per-file options, but does not correctly turn it off for the per-file
> case (because we bail to regular git_config instead of custom lookup
> code).
> 
> > > Do you want to revert and re-do to make master pretty, or should I just
> > > build on top?
> > 
> > Do you mean 'next'?
> 
> I meant "revert from next and re-reroll, so that when the re-roll gets
> merged to master, the result there will look pretty".

The fix for this is pretty straightforward. However, I took the
opportunity to do some cleanup and fix some minor bugs while I was in
the area, and ended up with quite a big series.

I prepared this on top of what you have queued in jk/config-include.
However, all of the cleanup is semantically independent of the topic
(though there are a few minor textual conflicts). If I were re-rolling,
I would put it all at the front, then squash patch 8 into my prior
"implement config includes" patch.

The patches are:

  [1/8]: t1300: add missing &&-chaining

Obvious cleanup.

  [2/8]: config: copy the return value of prefix_filename

Very minor bug.

  [3/8]: config: teach git_config_set_multivar_in_file a default path
  [4/8]: config: teach git_config_rename_section a file argument
  [5/8]: config: provide a version of git_config with more options
  [6/8]: config: stop using config_exclusive_filename
  [7/8]: config: eliminate config_exclusive_filename

This is all cleanup which makes config_exclusive_filename go away. It's
not strictly necessary for this series, but it's something I've been
wanting to clean up for a while. And it does fix a few minor bugs (see
patch 6/8). And the refactoring in 5/8 lays the groundwork for 8/8.

  [8/8]: config: do not respect includes for single-file --list

The actual fix for the regression in my config-include patch.

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] Document format of basic Git objects
From: Junio C Hamano @ 2012-02-16  7:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v4nur2806.fsf@alter.siamese.dyndns.org>

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

>> +- POSIX file mode encoded in octal ascii
>
> Add ", no 0 padding to the right" at the end, as I heard that every
> imitation of Git gets this wrong in its first version.

Ehh, of course no 0 padding on the LEFT hand side.

Rice-bowl with left hand, chopsticks with right hand. I always mix these
up.  Sorry.

^ permalink raw reply

* Re: git-subtree Ready #2
From: David A. Greene @ 2012-02-16  4:07 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Avery Pennarun
In-Reply-To: <87sjicpsr1.fsf@smith.obbligato.org>

greened@obbligato.org (David A. Greene) writes:

>> But more important than the physical layout is the maintenance plan
>> going forward.  Is Avery going to keep maintaining git-subtree, and we
>> will just occasionally pull? Are you maintaining it? Where will patches
>> go? To a github repo? To git@vger?
>
> I am still waiting to hear from Avery on that.  I will ping him again.
> My intention is to certainly participate in maintenance.  I use
> git-subtree daily so it's in my interest to keep it working.

I've attached Avery's response below.  The short summary is that he
thinks maintaining it in the vger git repository is the way to go and
that he's fine moving patches to/from GitHub as necessary.

So again, I will certainly be part of the maintenance team.  There are a
few other people currently helping out with maintenance and I will help
Avery to get the word out about the switch as he requires.

                            -Dave

--8<-----------------------------------------------------------------------

Thanks for putting work into this.  You can feel free to cc: me on any
git-list discussions if you want.

I haven't done any significant amount of git-subtree maintenance lately,
but even if I did, since it's only one file it should be easy to put
move patches between github and git whenever we want.  I would suggest
that just maintaining it as part of git is the best way to go, since
having diverging versions doesn't really help anyone.

I'm sure the potential benefit of putting git-subtree in the contrib/
directory is that we could then use git-subtree to maintain the
git-subtree git subtree, which is a fun wordplay, but perhaps
ironically, as a single rarely-changing file, git-subtree is probably
not the right tool for these purposes :)

Have fun,

Avery

^ permalink raw reply

* Re: importance of .git extension for bare repos
From: Jeff King @ 2012-02-16  2:30 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Neal Kreitzinger, git
In-Reply-To: <CALUzUxqEirjnCBHt28dwcgTBqC+u0wydNsnVR1rgyGG0-R_M1w@mail.gmail.com>

On Thu, Feb 16, 2012 at 09:38:08AM +0800, Tay Ray Chuan wrote:

> I don't believe its significant to git itself, it's just a convention
> for us humans.

That's not entirely true. If you try to access a repository by name
(e.g., "git clone foo", "git fetch foo"), git will look for "foo.git".
This magic lookup works for local repositories, and for remote
repositories served over ssh or by git-daemon. I think it should also
work for smart-http, but I didn't check. It doesn't work for dumb http
(because it would involve making several exploratory http requests).

-Peff

^ permalink raw reply

* Re: importance of .git extension for bare repos
From: Tay Ray Chuan @ 2012-02-16  1:38 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <jhhkn8$a0t$1@dough.gmane.org>

I don't believe its significant to git itself, it's just a convention
for us humans.

-- 
Cheers,
Ray Chuan


On Thu, Feb 16, 2012 at 9:04 AM, Neal Kreitzinger <neal@rsss.com> wrote:
> I have a user who has created bare repos without naming them with the .git
> extension.  As a result, the file paths are MYBARE/config instead of
> MYBARE.git/config, etc.  Is this a supported naming convention for bare
> repos in git.git, or is it going to have some problems?
>
> v/r,
> neal
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* importance of .git extension for bare repos
From: Neal Kreitzinger @ 2012-02-16  1:04 UTC (permalink / raw)
  To: git

I have a user who has created bare repos without naming them with the .git 
extension.  As a result, the file paths are MYBARE/config instead of 
MYBARE.git/config, etc.  Is this a supported naming convention for bare 
repos in git.git, or is it going to have some problems?

v/r,
neal 

^ 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