Git development
 help / color / mirror / Atom feed
* [PATCH 1/2] Clarify the gitmodules and submodules docs
From: P Baker @ 2009-04-08  7:04 UTC (permalink / raw)
  To: git
In-Reply-To: <1239172816-38966-1-git-send-email-pbaker@retrodict.com>

Added some explanation to the docs to clear up some confusing parts of
git-submodules that appeared frequently on the mailing list.

 Signed-off-by: pbaker <pbaker@retrodict.com>
 ---

 As I dug into the reasoning and structure of git-submodule as part of
GSoC preparation, I also ran across frequently asked questions on the
mailing list. From this background, I added some explanation to the
docs to  clear up some confusing parts of git-submodules.

 - pbaker

 Documentation/git-submodule.txt |    9 ++++++---
  Documentation/gitmodules.txt    |    8 +++++++-
  2 files changed, 13 insertions(+), 4 deletions(-)

 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index 3b8df44..1ca8184 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -50,9 +50,12 @@ This command will manage the tree entries and
contents of the
  gitmodules file for you, as well as inspect the status of your
  submodules and update them.
  When adding a new submodule to the tree, the 'add' subcommand
 -is to be used.  However, when pulling a tree containing submodules,
 -these will not be checked out by default;
 -the 'init' and 'update' subcommands will maintain submodules
 +is to be used. This creates a record in the gitmodules file for each
 +submodule. When the file is committed and pulled by others it serves as an
 +in-tree reference for where to obtain the submodule.
 +
 +When pulling a tree containing submodules, these will not be checked out by
 +default; the 'init' and 'update' subcommands will maintain submodules
  checked out and at appropriate revision in your working tree.
  You can briefly inspect the up-to-date status of your submodules
  using the 'status' subcommand and get a detailed overview of the
 diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
 index d1a17e2..8f03310 100644
 --- a/Documentation/gitmodules.txt
 +++ b/Documentation/gitmodules.txt
 @@ -15,7 +15,13 @@ DESCRIPTION

  The `.gitmodules` file, located in the top-level directory of a git
  working tree, is a text file with a syntax matching the requirements
 -of linkgit:git-config[1].
 +of linkgit:git-config[1]. As this file is managed by Git, it tracks the
 +records of a project's submodules. Information stored in this file is used
 +as a hint to prime the authoritative version of the record stored in the
 +project configuration file. User specific record changes (e.g. to account
 +for differences in submodule URLs due to networking situations) should be
 +made to the configuration file, while record changes to be propagated (e.g.
 +due to a relocation of the submodule source) should be made to this file.

  The file contains one subsection per submodule, and the subsection value
  is the name of the submodule. Each submodule section also contains the
 --
 1.6.2.1.316.gedbc2

^ permalink raw reply

* [PATCH 2/2] Make git submodule --quiet summary obey flag
From: P Baker @ 2009-04-08  7:05 UTC (permalink / raw)
  To: git
In-Reply-To: <1239172707-38915-2-git-send-email-pbaker@retrodict.com>

Even with flag, summary would echo output. This patch conforms
git-submodule to match user expectations.

 Signed-off-by: pbaker <pbaker@retrodict.com>
 ---
  git-submodule.sh |   16 ++++++++--------
  1 files changed, 8 insertions(+), 8 deletions(-)

 diff --git a/git-submodule.sh b/git-submodule.sh
 index 7c2e060..784c6fa 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -187,7 +187,7 @@ cmd_add()
        then
                if test -d "$path"/.git -o -f "$path"/.git
                then
 -                       echo "Adding existing repo at '$path' to the index"
 +                       say "Adding existing repo at '$path' to the index"
                else
                        die "'$path' already exists and is not a valid git repo"
                fi
 @@ -450,10 +450,10 @@ cmd_summary() {
                while read mod_src mod_dst sha1_src sha1_dst status name
                do
                        # Always show modules deleted or type-changed
(blob<->module)
 -                       test $status = D -o $status = T && echo
"$name" && continue
 +                       test $status = D -o $status = T && say
"$name" && continue
                        # Also show added or modified modules which
are checked out
                        GIT_DIR="$name/.git" git-rev-parse --git-dir
>/dev/null 2>&1 &&
 -                       echo "$name"
 +                       say "$name"
                done
        )

 @@ -522,25 +522,25 @@ cmd_summary() {
                        total_commits=" ($(($total_commits + 0)))"
                        ;;
                esac
 -
 +
                sha1_abbr_src=$(echo $sha1_src | cut -c1-7)
                sha1_abbr_dst=$(echo $sha1_dst | cut -c1-7)
                if test $status = T
                then
                        if test $mod_dst = 160000
                        then
 -                               echo "* $name
$sha1_abbr_src(blob)->$sha1_abbr_dst(submodule)$total_commits:"
 +                               say "* $name
$sha1_abbr_src(blob)->$sha1_abbr_dst(submodule)$total_commits:"
                        else
 -                               echo "* $name
$sha1_abbr_src(submodule)->$sha1_abbr_dst(blob)$total_commits:"
 +                               say "* $name
$sha1_abbr_src(submodule)->$sha1_abbr_dst(blob)$total_commits:"
                        fi
                else
 -                       echo "* $name
$sha1_abbr_src...$sha1_abbr_dst$total_commits:"
 +                       say "* $name
$sha1_abbr_src...$sha1_abbr_dst$total_commits:"
                fi
                if test -n "$errmsg"
                then
                        # Don't give error msg for modification whose
dst is not submodule
                        # i.e. deleted or changed to blob
 -                       test $mod_dst = 160000 && echo "$errmsg"
 +                       test $mod_dst = 160000 && echo >&2 "$errmsg"
                else
                        if test $mod_src = 160000 -a $mod_dst = 160000
                        then
 --
 1.6.2.1.316.gedbc2

^ permalink raw reply

* Re: [PATCH v5 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: Junio C Hamano @ 2009-04-08  7:05 UTC (permalink / raw)
  To: David Aguilar; +Cc: gitster, git, charles, markus.heidelberg
In-Reply-To: <1239172085-23389-1-git-send-email-davvid@gmail.com>

David Aguilar <davvid@gmail.com> writes:

> This consolidates the common functionality from git-mergetool and
> git-difftool--helper into a single git-mergetool--lib scriptlet.
>
> +guess_merge_tool () {
> +	tools="ecmerge"
> +	if merge_mode; then
> +		tools="tortoisemerge"
> +	else
> +		tools="kompare"
> +	fi

The first assignment of ecmerge seems a no-op.

> +	if test -n "$DISPLAY"; then
> +		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> +			tools="meld opendiff kdiff3 tkdiff xxdiff $tools \
> +				gvimdiff diffuse ecmerge"
> +		else
> +			tools="opendiff kdiff3 tkdiff xxdiff meld $tools \
> +				gvimdiff diffuse ecmerge"
> +		fi
> +	fi
> +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> +		# $EDITOR is emacs so add emerge as a candidate
> +		tools="$tools emerge vimdiff"
> +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> +		# $EDITOR is vim so add vimdiff as a candidate
> +		tools="$tools vimdiff emerge"
> +	else
> +		tools="$tools emerge vimdiff"
> +	fi
> +	tools="$(echo "$tools" | sed -e 's/ 	*/ /g')"
> +	echo >&2 "merge tool candidates: $tools"

Sorry, but I am not sure what this is doing.  Replace a SP followed by
zero or more HT with a single SP?  

Ahh, you are removing the HT in the indentation part?

I'd rather see the above written like this if that is the case:

> +	if test -n "$DISPLAY"; then
> +		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> +			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
> +		else
> +			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
> +		fi
> + 		tools="$tools gvimdiff diffuse ecmerge"
> +	fi

then you can lose the "echo | sed", no?

^ permalink raw reply

* Re: [PATCH 1/2] Clarify the gitmodules and submodules docs
From: Junio C Hamano @ 2009-04-08  7:16 UTC (permalink / raw)
  To: P Baker; +Cc: git
In-Reply-To: <526944450904080004r1249d1b8j4a44472d72502b9c@mail.gmail.com>

I think the new description is good, and the message claims to be sent by
send-email, but I am seeing a funny whitespace damage everywhere in your
patch.

 * Signed-off-by: and the three-dash lines have a leading SP

 * The first line of diffstat is indented by one SP as expected; the
   second line has one extra SP at the beginning.

 * everything after that has one extra SP at the beginning.  Also the hunk
   header for the first hunk is linewrapped.

Very curious.  I could just hand munge the patch if I wanted to, but I'd
rather not.  As you are in "GSoC preparation", I expect to receive
patches, perhaps tons of them, from you in the future, and I'd want to see
easier procedural wrinkles straightened out first.  That will allow us to
concentrate on the substance when "the real thing" starts.

    Added some explanation to the docs to clear up some confusing parts of
    git-submodules that appeared frequently on the mailing list.

We tend to say "Add some explanation..." here.  Think of it as your giving
an order to _me_, "cause this to happen", by accepting and applying your
patch.

^ permalink raw reply

* [PATCH v6 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: David Aguilar @ 2009-04-08  7:17 UTC (permalink / raw)
  To: gitster; +Cc: git, charles, markus.heidelberg, David Aguilar

This consolidates the common functionality from git-mergetool and
git-difftool--helper into a single git-mergetool--lib scriptlet.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

Simplified guess_merge_tool()'s tools= section.

 .gitignore                           |    1 +
 Documentation/git-mergetool--lib.txt |   56 +++++
 Makefile                             |    1 +
 git-difftool--helper.sh              |  186 +----------------
 git-mergetool--lib.sh                |  378 ++++++++++++++++++++++++++++++++++
 git-mergetool.sh                     |  224 ++-------------------
 6 files changed, 458 insertions(+), 388 deletions(-)
 create mode 100644 Documentation/git-mergetool--lib.txt
 create mode 100644 git-mergetool--lib.sh

diff --git a/.gitignore b/.gitignore
index a36da9d..757c7f0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ git-merge-recursive
 git-merge-resolve
 git-merge-subtree
 git-mergetool
+git-mergetool--lib
 git-mktag
 git-mktree
 git-name-rev
diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
new file mode 100644
index 0000000..3d57031
--- /dev/null
+++ b/Documentation/git-mergetool--lib.txt
@@ -0,0 +1,56 @@
+git-mergetool--lib(1)
+=====================
+
+NAME
+----
+git-mergetool--lib - Common git merge tool shell scriptlets
+
+SYNOPSIS
+--------
+'. "$(git --exec-path)/git-mergetool--lib"'
+
+DESCRIPTION
+-----------
+
+This is not a command the end user would want to run.  Ever.
+This documentation is meant for people who are studying the
+Porcelain-ish scripts and/or are writing new ones.
+
+The 'git-mergetool--lib' scriptlet is designed to be sourced (using
+`.`) by other shell scripts to set up functions for working
+with git merge tools.
+
+Before sourcing it, your script should set up a few variables;
+`TOOL_MODE` is used to define the operation mode for various
+functions.  'diff' and 'merge' are valid values.
+
+FUNCTIONS
+---------
+get_merge_tool::
+	returns a merge tool
+
+get_merge_tool_cmd::
+	returns the custom command for a merge tool.
+
+get_merge_tool_path::
+	returns the custom path for a merge tool.
+
+run_merge_tool::
+	launches a merge tool given the tool name and a true/false
+	flag to indicate whether a merge base is present.
+	'$merge_tool', '$merge_tool_path', and for custom commands,
+	'$merge_tool_cmd', must be defined prior to calling
+	run_merge_tool.  Additionally, '$MERGED', '$LOCAL', '$REMOTE',
+	and '$BASE' must be defined for use by the merge tool.
+
+Author
+------
+Written by David Aguilar <davvid@gmail.com>
+
+Documentation
+--------------
+Documentation by David Aguilar and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a80055f..3e56274 100644
--- a/Makefile
+++ b/Makefile
@@ -284,6 +284,7 @@ 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-parse-remote.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index f3c27d8..b450036 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -5,6 +5,10 @@
 #
 # Copyright (c) 2009 David Aguilar
 
+# Load common functions from git-mergetool--lib
+TOOL_MODE=diff
+. git-mergetool--lib
+
 # difftool.prompt controls the default prompt/no-prompt behavior
 # and is overridden with $GIT_DIFFTOOL*_PROMPT.
 should_prompt () {
@@ -16,8 +20,7 @@ should_prompt () {
 	fi
 }
 
-# This function prepares temporary files and launches the appropriate
-# merge tool.
+# Sets up shell variables and runs a merge tool
 launch_merge_tool () {
 	# Merged is the filename as it appears in the work tree
 	# Local is the contents of a/filename
@@ -37,187 +40,16 @@ launch_merge_tool () {
 	fi
 
 	# Run the appropriate merge tool command
-	case "$merge_tool" in
-	kdiff3)
-		basename=$(basename "$MERGED")
-		"$merge_tool_path" --auto \
-			--L1 "$basename (A)" \
-			--L2 "$basename (B)" \
-			"$LOCAL" "$REMOTE" \
-			> /dev/null 2>&1
-		;;
-
-	kompare)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	tkdiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	meld)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	diffuse)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	vimdiff)
-		"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
-		;;
-
-	gvimdiff)
-		"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
-		;;
-
-	xxdiff)
-		"$merge_tool_path" \
-			-R 'Accel.Search: "Ctrl+F"' \
-			-R 'Accel.SearchForward: "Ctrl-G"' \
-			"$LOCAL" "$REMOTE"
-		;;
-
-	opendiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	ecmerge)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" \
-			--default --mode=merge2 --to="$MERGED"
-		;;
-
-	emerge)
-		"$merge_tool_path" -f emerge-files-command \
-			"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-		;;
-
-	*)
-		if test -n "$merge_tool_cmd"; then
-			( eval $merge_tool_cmd )
-		fi
-		;;
-	esac
-}
-
-# Verifies that (difftool|mergetool).<tool>.cmd exists
-valid_custom_tool() {
-	merge_tool_cmd="$(git config difftool.$1.cmd)"
-	test -z "$merge_tool_cmd" &&
-	merge_tool_cmd="$(git config mergetool.$1.cmd)"
-	test -n "$merge_tool_cmd"
-}
-
-# Verifies that the chosen merge tool is properly setup.
-# Built-in merge tools are always valid.
-valid_tool() {
-	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
-		;; # happy
-	*)
-		if ! valid_custom_tool "$1"
-		then
-			return 1
-		fi
-		;;
-	esac
-}
-
-# Sets up the merge_tool_path variable.
-# This handles the difftool.<tool>.path configuration.
-# This also falls back to mergetool defaults.
-init_merge_tool_path() {
-	merge_tool_path=$(git config difftool."$1".path)
-	test -z "$merge_tool_path" &&
-	merge_tool_path=$(git config mergetool."$1".path)
-	if test -z "$merge_tool_path"; then
-		case "$1" in
-		vimdiff)
-			merge_tool_path=vim
-			;;
-		gvimdiff)
-			merge_tool_path=gvim
-			;;
-		emerge)
-			merge_tool_path=emacs
-			;;
-		*)
-			merge_tool_path="$1"
-			;;
-		esac
-	fi
+	run_merge_tool "$merge_tool"
 }
 
 # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values
 test -n "$GIT_MERGE_TOOL" && merge_tool="$GIT_MERGE_TOOL"
 test -n "$GIT_DIFF_TOOL" && merge_tool="$GIT_DIFF_TOOL"
 
-# If merge tool was not specified then use the diff.tool
-# configuration variable.  If that's invalid then reset merge_tool.
-# Fallback to merge.tool.
-if test -z "$merge_tool"; then
-	merge_tool=$(git config diff.tool)
-	test -z "$merge_tool" &&
-	merge_tool=$(git config merge.tool)
-	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-		echo >&2 "git config option diff.tool set to unknown tool: $merge_tool"
-		echo >&2 "Resetting to default..."
-		unset merge_tool
-	fi
-fi
-
-# Try to guess an appropriate merge tool if no tool has been set.
-if test -z "$merge_tool"; then
-	# We have a $DISPLAY so try some common UNIX merge tools
-	if test -n "$DISPLAY"; then
-		# If gnome then prefer meld, otherwise, prefer kdiff3 or kompare
-		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			merge_tool_candidates="meld kdiff3 kompare tkdiff xxdiff gvimdiff diffuse"
-		else
-			merge_tool_candidates="kdiff3 kompare tkdiff xxdiff meld gvimdiff diffuse"
-		fi
-	fi
-	if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-		# $EDITOR is emacs so add emerge as a candidate
-		merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-	elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-		# $EDITOR is vim so add vimdiff as a candidate
-		merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-	else
-		merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-	fi
-	echo "merge tool candidates: $merge_tool_candidates"
-
-	# Loop over each candidate and stop when a valid merge tool is found.
-	for i in $merge_tool_candidates
-	do
-		init_merge_tool_path $i
-		if type "$merge_tool_path" > /dev/null 2>&1; then
-			merge_tool=$i
-			break
-		fi
-	done
-
-	if test -z "$merge_tool" ; then
-		echo "No known merge resolution program available."
-		exit 1
-	fi
-
-else
-	# A merge tool has been set, so verify that it's valid.
-	if ! valid_tool "$merge_tool"; then
-		echo >&2 "Unknown merge tool $merge_tool"
-		exit 1
-	fi
-
-	init_merge_tool_path "$merge_tool"
-
-	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-		echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-		exit 1
-	fi
-fi
-
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
 
 # Launch the merge tool on each path provided by 'git diff'
 while test $# -gt 6
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
new file mode 100644
index 0000000..c5db24e
--- /dev/null
+++ b/git-mergetool--lib.sh
@@ -0,0 +1,378 @@
+# git-mergetool--lib is a library for common merge tool functions
+diff_mode() {
+	test "$TOOL_MODE" = diff
+}
+
+merge_mode() {
+	test "$TOOL_MODE" = merge
+}
+
+translate_merge_tool_path () {
+	if test -n "$2"; then
+		echo "$2"
+	else
+		case "$1" in
+		vimdiff)
+			path=vim
+			;;
+		gvimdiff)
+			path=gvim
+			;;
+		emerge)
+			path=emacs
+			;;
+		*)
+			path="$1"
+			;;
+		esac
+		echo "$path"
+	fi
+}
+
+check_unchanged () {
+	if test "$MERGED" -nt "$BACKUP"; then
+		status=0
+	else
+		while true; do
+			echo "$MERGED seems unchanged."
+			printf "Was the merge successful? [y/n] "
+			read answer < /dev/tty
+			case "$answer" in
+			y*|Y*) status=0; break ;;
+			n*|N*) status=1; break ;;
+			esac
+		done
+	fi
+}
+
+valid_tool () {
+	case "$1" in
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+	emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+		;; # happy
+	tortoisemerge)
+		if ! merge_mode; then
+			return 1
+		fi
+		;;
+	kompare)
+		if ! diff_mode; then
+			return 1
+		fi
+		;;
+	*)
+		if test -z "$(get_merge_tool_cmd "$1")"; then
+			return 1
+		fi
+		;;
+	esac
+}
+
+get_merge_tool_cmd () {
+	diff_mode &&
+	custom_cmd="$(git config difftool.$1.cmd)"
+	test -z "$custom_cmd" &&
+	custom_cmd="$(git config mergetool.$1.cmd)"
+	test -n "$custom_cmd" &&
+	echo "$custom_cmd"
+}
+
+run_merge_tool () {
+	base_present="$2"
+	status=0
+
+	case "$1" in
+	kdiff3)
+		if merge_mode; then
+			if $base_present; then
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Base)" \
+					--L2 "$MERGED (Local)" \
+					--L3 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$BASE" "$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			else
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Local)" \
+					--L2 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			fi
+			status=$?
+		else
+			("$merge_tool_path" --auto \
+			 --L1 "$MERGED (A)" \
+			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
+		fi
+		;;
+	kompare)
+		"$merge_tool_path" "$LOCAL" "$REMOTE"
+		;;
+	tkdiff)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" -a "$BASE" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			else
+				"$merge_tool_path" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	meld)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	diffuse)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" \
+					"$BASE" | cat
+			else
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	vimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	gvimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	xxdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" -X --show-merged-pane \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$BASE" "$REMOTE"
+			else
+				"$merge_tool_path" -X $extra \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$REMOTE"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	opendiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-ancestor "$BASE" \
+					-merge "$MERGED" | cat
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-merge "$MERGED" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	ecmerge)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+					--default --mode=merge3 --to="$MERGED"
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					--default --mode=merge2 --to="$MERGED"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				--default --mode=merge2 --to="$MERGED"
+		fi
+		;;
+	emerge)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" \
+					-f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" \
+					"$(basename "$MERGED")"
+			else
+				"$merge_tool_path" \
+					-f emerge-files-command \
+					"$LOCAL" "$REMOTE" \
+					"$(basename "$MERGED")"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" -f emerge-files-command \
+				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
+		fi
+		;;
+	tortoisemerge)
+		if $base_present; then
+			touch "$BACKUP"
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
+			check_unchanged
+		else
+			echo "TortoiseMerge cannot be used without a base" 1>&2
+			status=1
+		fi
+		;;
+	*)
+		if test -z "$merge_tool_cmd"; then
+			if merge_mode; then
+				status=1
+			fi
+			break
+		fi
+		if merge_mode; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
+				touch "$BACKUP"
+				( eval $merge_tool_cmd )
+				check_unchanged
+			else
+				( eval $merge_tool_cmd )
+				status=$?
+			fi
+		else
+			( eval $merge_tool_cmd )
+		fi
+		;;
+	esac
+	return $status
+}
+
+guess_merge_tool () {
+	if merge_mode; then
+		tools="tortoisemerge"
+	else
+		tools="kompare"
+	fi
+	if test -n "$DISPLAY"; then
+		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+			tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
+		else
+			tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
+		fi
+		tools="$tools gvimdiff diffuse ecmerge"
+	fi
+	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
+		# $EDITOR is emacs so add emerge as a candidate
+		tools="$tools emerge vimdiff"
+	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
+		# $EDITOR is vim so add vimdiff as a candidate
+		tools="$tools vimdiff emerge"
+	else
+		tools="$tools emerge vimdiff"
+	fi
+	echo >&2 "merge tool candidates: $tools"
+
+	# Loop over each candidate and stop when a valid merge tool is found.
+	for i in $tools
+	do
+		merge_tool_path="$(translate_merge_tool_path "$i")"
+		if type "$merge_tool_path" > /dev/null 2>&1; then
+			merge_tool="$i"
+			break
+		fi
+	done
+
+	if test -z "$merge_tool" ; then
+		echo >&2 "No known merge resolution program available."
+		return 1
+	fi
+	echo "$merge_tool"
+}
+
+get_configured_merge_tool () {
+	# Diff mode first tries diff.tool and falls back to merge.tool.
+	# Merge mode only checks merge.tool
+	if diff_mode; then
+		tool=$(git config diff.tool)
+	fi
+	if test -z "$tool"; then
+		tool=$(git config merge.tool)
+	fi
+	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
+		echo >&2 "git config option $TOOL_MODE.tool set to unknown tool: $merge_tool"
+		echo >&2 "Resetting to default..."
+		return 1
+	fi
+	echo "$tool"
+}
+
+get_merge_tool_path () {
+	# A merge tool has been set, so verify that it's valid.
+	if ! valid_tool "$merge_tool"; then
+		echo >&2 "Unknown merge tool $merge_tool"
+		exit 1
+	fi
+	if diff_mode; then
+		merge_tool_path=$(git config difftool."$merge_tool".path)
+	fi
+	if test -z "$merge_tool_path"; then
+		merge_tool_path=$(git config mergetool."$merge_tool".path)
+	fi
+	merge_tool_path="$(translate_merge_tool_path "$merge_tool" "$merge_tool_path")"
+	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
+		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as '$merge_tool_path'"
+		exit 1
+	fi
+	echo "$merge_tool_path"
+}
+
+get_merge_tool () {
+	merge_tool="$1"
+	# Check if a merge tool has been configured
+	if test -z "$merge_tool"; then
+		merge_tool=$(get_configured_merge_tool)
+	fi
+	# Try to guess an appropriate merge tool if no tool has been set.
+	if test -z "$merge_tool"; then
+		merge_tool=$(guess_merge_tool) || exit
+	fi
+	echo "$merge_tool"
+}
diff --git a/git-mergetool.sh b/git-mergetool.sh
index cceebb7..efa31a2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -11,7 +11,9 @@
 USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
+TOOL_MODE=merge
 . git-sh-setup
+. git-mergetool--lib
 require_work_tree
 
 # Returns true if the mode reflects a symlink
@@ -110,22 +112,6 @@ resolve_deleted_merge () {
 	done
 }
 
-check_unchanged () {
-    if test "$MERGED" -nt "$BACKUP" ; then
-	status=0;
-    else
-	while true; do
-	    echo "$MERGED seems unchanged."
-	    printf "Was the merge successful? [y/n] "
-	    read answer < /dev/tty
-	    case "$answer" in
-		y*|Y*) status=0; break ;;
-		n*|N*) status=1; break ;;
-	    esac
-	done
-    fi
-}
-
 checkout_staged_file () {
     tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
 
@@ -188,107 +174,11 @@ merge_file () {
 	read ans
     fi
 
-    case "$merge_tool" in
-	kdiff3)
-	    if base_present ; then
-		("$merge_tool_path" --auto --L1 "$MERGED (Base)" --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" \
-		    -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    else
-		("$merge_tool_path" --auto --L1 "$MERGED (Local)" --L2 "$MERGED (Remote)" \
-		    -o "$MERGED" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    fi
-	    status=$?
-	    ;;
-	tkdiff)
-	    if base_present ; then
-		"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    else
-		"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    status=$?
-	    ;;
-	meld)
-	    touch "$BACKUP"
-	    "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	vimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	gvimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	xxdiff)
-	    touch "$BACKUP"
-	    if base_present ; then
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
-	    else
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    check_unchanged
-	    ;;
-	opendiff)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$MERGED" | cat
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$MERGED" | cat
-	    fi
-	    check_unchanged
-	    ;;
-	ecmerge)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"
-	    fi
-	    check_unchanged
-	    ;;
-	emerge)
-	    if base_present ; then
-		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
-	    else
-		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-	    fi
-	    status=$?
-	    ;;
-	tortoisemerge)
-	    if base_present ; then
-		touch "$BACKUP"
-		"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
-		check_unchanged
-	    else
-		echo "TortoiseMerge cannot be used without a base" 1>&2
-		status=1
-	    fi
-	    ;;
-	*)
-	    if test -n "$merge_tool_cmd"; then
-		if test "$merge_tool_trust_exit_code" = "false"; then
-		    touch "$BACKUP"
-		    ( eval $merge_tool_cmd )
-		    check_unchanged
-		else
-		    ( eval $merge_tool_cmd )
-		    status=$?
-		fi
-	    fi
-	    ;;
-    esac
-    if test "$status" -ne 0; then
+    present=false
+    base_present &&
+    present=true
+
+    if ! run_merge_tool "$merge_tool" "$present"; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
 
@@ -347,44 +237,6 @@ do
     shift
 done
 
-valid_custom_tool()
-{
-    merge_tool_cmd="$(git config mergetool.$1.cmd)"
-    test -n "$merge_tool_cmd"
-}
-
-valid_tool() {
-	case "$1" in
-		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | tortoisemerge)
-			;; # happy
-		*)
-			if ! valid_custom_tool "$1"; then
-				return 1
-			fi
-			;;
-	esac
-}
-
-init_merge_tool_path() {
-	merge_tool_path=$(git config mergetool.$1.path)
-	if test -z "$merge_tool_path" ; then
-		case "$1" in
-			vimdiff)
-				merge_tool_path=vim
-				;;
-			gvimdiff)
-				merge_tool_path=gvim
-				;;
-			emerge)
-				merge_tool_path=emacs
-				;;
-			*)
-				merge_tool_path=$1
-				;;
-		esac
-	fi
-}
-
 prompt_after_failed_merge() {
     while true; do
 	printf "Continue merging other unresolved paths (y/n) ? "
@@ -402,62 +254,12 @@ prompt_after_failed_merge() {
     done
 }
 
-if test -z "$merge_tool"; then
-    merge_tool=$(git config merge.tool)
-    if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
-	    echo >&2 "Resetting to default..."
-	    unset merge_tool
-    fi
-fi
-
-if test -z "$merge_tool" ; then
-    if test -n "$DISPLAY"; then
-        if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-            merge_tool_candidates="meld kdiff3 tkdiff xxdiff tortoisemerge gvimdiff diffuse"
-        else
-            merge_tool_candidates="kdiff3 tkdiff xxdiff meld tortoisemerge gvimdiff diffuse"
-        fi
-    fi
-    if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-    elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-    else
-        merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-    fi
-    echo "merge tool candidates: $merge_tool_candidates"
-    for i in $merge_tool_candidates; do
-        init_merge_tool_path $i
-        if type "$merge_tool_path" > /dev/null 2>&1; then
-            merge_tool=$i
-            break
-        fi
-    done
-    if test -z "$merge_tool" ; then
-	echo "No known merge resolution program available."
-	exit 1
-    fi
-else
-    if ! valid_tool "$merge_tool"; then
-        echo >&2 "Unknown merge_tool $merge_tool"
-        exit 1
-    fi
-
-    init_merge_tool_path "$merge_tool"
-
-    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
-    merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-    if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-        exit 1
-    fi
-
-    if ! test -z "$merge_tool_cmd"; then
-        merge_tool_trust_exit_code="$(git config --bool mergetool.$merge_tool.trustExitCode || echo false)"
-    fi
-fi
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
+merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
+merge_tool_trust_exit_code="$(git config --bool mergetool."$merge_tool".trustExitCode || echo false)"
 
 last_status=0
 rollup_status=0
-- 
1.6.2.2.446.gfbdc

^ permalink raw reply related

* [PATCH] git-pull.sh: better warning message for "git pull" on detached head.
From: Matthieu Moy @ 2009-04-08  7:24 UTC (permalink / raw)
  To: gitster, git; +Cc: Matthieu Moy
In-Reply-To: <7vk55wvutb.fsf@gitster.siamese.dyndns.org>

Otherwise, git complains about not finding a branch to pull from in
'branch..merge', which is hardly understandable. While we're there,
reword the sentences slightly.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-pull.sh |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 8a26763..8c75027 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -90,23 +90,31 @@ error_on_no_merge_candidates () {
 
 	curr_branch=${curr_branch#refs/heads/}
 
-	echo "You asked me to pull without telling me which branch you"
-	echo "want to merge with, and 'branch.${curr_branch}.merge' in"
-	echo "your configuration file does not tell me either.  Please"
-	echo "name which branch you want to merge on the command line and"
-	echo "try again (e.g. 'git pull <repository> <refspec>')."
-	echo "See git-pull(1) for details on the refspec."
-	echo
-	echo "If you often merge with the same branch, you may want to"
-	echo "configure the following variables in your configuration"
-	echo "file:"
-	echo
-	echo "    branch.${curr_branch}.remote = <nickname>"
-	echo "    branch.${curr_branch}.merge = <remote-ref>"
-	echo "    remote.<nickname>.url = <url>"
-	echo "    remote.<nickname>.fetch = <refspec>"
-	echo
-	echo "See git-config(1) for details."
+	if [ -z "$curr_branch" ]; then
+		echo "You are not currently on a branch, so I cannot use any"
+		echo "'branch.<branchname>.merge' in your configuration file."
+		echo "Please specify which branch you want to merge on the command"
+		echo "line and try again (e.g. 'git pull <repository> <refspec>')."
+		echo "See git-pull(1) for details."
+	else
+		echo "You asked me to pull without telling me which branch you"
+		echo "want to merge with, and 'branch.${curr_branch}.merge' in"
+		echo "your configuration file does not tell me either.	Please"
+		echo "specify which branch you want to merge on the command line and"
+		echo "try again (e.g. 'git pull <repository> <refspec>')."
+		echo "See git-pull(1) for details."
+		echo
+		echo "If you often merge with the same branch, you may want to"
+		echo "configure the following variables in your configuration"
+		echo "file:"
+		echo
+		echo "    branch.${curr_branch}.remote = <nickname>"
+		echo "    branch.${curr_branch}.merge = <remote-ref>"
+		echo "    remote.<nickname>.url = <url>"
+		echo "    remote.<nickname>.fetch = <refspec>"
+		echo
+		echo "See git-config(1) for details."
+	fi
 	exit 1
 }
 
-- 
1.6.2.2.449.g92961.dirty

^ permalink raw reply related

* Re: [PATCH 2/3] rev-list: remove last static vars used in "show_commit"
From: Paolo Bonzini @ 2009-04-08  7:40 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <20090407040854.4338.40055.chriscool@tuxfamily.org>


>  	struct rev_info revs;
> +	struct rev_list_info info;
>  	int reaches = 0, all = 0;
>  
> +	memset(&info, 0, sizeof(info));
> +	info.revs = &revs;

Would it make sense to embed the struct rev_info entirely in the new
struct, without going through a pointer?

Paolo

^ permalink raw reply

* Re: [PATCH] graph API: Added logic for colored edges
From: Junio C Hamano @ 2009-04-08  7:59 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Jeff King, Nanako Shiraishi
In-Reply-To: <20090407185724.GA9996@linux.vnet>

Allan Caffee <allan.caffee@gmail.com> writes:

> @@ -72,11 +69,21 @@ struct column {
>  	 */
>  	struct commit *commit;
>  	/*
> -	 * XXX: Once we add support for colors, struct column could also
> -	 * contain the color of its branch line.
> +	 * The color to (optionally) print this column in.
>  	 */
> +	const char *color;

You already use short for git_graph.default_column_color and I suspect in
the longer term you want to make this one an index into column_colors[]
array the same way.  We may someday decide to support non-ANSI color
scheme using ncurses or something, and at that point the "hardware color"
constants like GIT_COLOR_RED and friends may change from strings to small
integers we use to call our (yet to be written) curses interface layer
with.

> @@ -312,6 +343,33 @@ static struct commit_list *first_interesting_parent(struct git_graph *graph)
>  	return next_interesting_parent(graph, parents);
>  }
>  
> +static const char* graph_get_current_column_color(const struct git_graph* graph)

Style.  Asterisk comes next to identifiers, not types (the parameter to
graph_increment_column_color has the same issue).

	static const char *graph_...color(const struct git_graph *graph)

> @@ -596,7 +661,7 @@ static void graph_output_padding_line(struct git_graph *graph,
>  	 * Output a padding row, that leaves all branch lines unchanged
>  	 */
>  	for (i = 0; i < graph->num_new_columns; i++) {
> -		strbuf_addstr(sb, "| ");
> +		strbuf_write_column(sb, &graph->new_columns[i], "| ");

Hmmm, this forbids us to use reverse color in the color palette because
that would highlight the trailing whitespace.  Is that something we care
about, or a reversed "|", "/" and "\" are already too ugly that we won't
want to support them?

> @@ -648,8 +713,11 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
>  	for (i = 0; i < graph->num_columns; i++) {
>  		struct column *col = &graph->columns[i];
>  		if (col->commit == graph->commit) {
> +			struct strbuf tmp = STRBUF_INIT;
>  			seen_this = 1;
> -			strbuf_addf(sb, "| %*s", graph->expansion_row, "");
> +			strbuf_addf(&tmp, "| %*s", graph->expansion_row, "");
> +			strbuf_write_column(sb, col, tmp.buf);
> +			strbuf_release(&tmp);

Same here.

> @@ -662,13 +730,13 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
>  			 */
>  			if (graph->prev_state == GRAPH_POST_MERGE &&
>  			    graph->prev_commit_index < i)
> -				strbuf_addstr(sb, "\\ ");
> +				strbuf_write_column(sb, col, "\\ ");
>  			else
> -				strbuf_addstr(sb, "| ");
> +				strbuf_write_column(sb, col, "| ");
>  		} else if (seen_this && (graph->expansion_row > 0)) {
> -			strbuf_addstr(sb, "\\ ");
> +			strbuf_write_column(sb, col, "\\ ");
>  		} else {
> -			strbuf_addstr(sb, "| ");
> +			strbuf_write_column(sb, col, "| ");
>  		}
>  	}

Likewise.

> @@ -744,14 +813,25 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
>  			if (graph->num_parents < 3)
>  				strbuf_addch(sb, ' ');
>  			else {
> +				/*
> +				 * Here dashless_commits represents the
> +				 * number of parents which don't need
> +				 * to have dashes (because their edges
> +				 * fit neatly under the commit).
> +				 */
> +				const int dashless_commits = 2;
>  				int num_dashes =
> -					((graph->num_parents - 2) * 2) - 1;
> +					((graph->num_parents - dashless_commits) * 2) - 1;
>  				for (j = 0; j < num_dashes; j++)
> -					strbuf_addch(sb, '-');
> -				strbuf_addstr(sb, ". ");
> +					strbuf_write_column(sb,
> +							    &graph->new_columns[(j / 2) + dashless_commits],
> +							    "-");
> +				strbuf_write_column(sb,
> +						    &graph->new_columns[(j / 2) + dashless_commits],
> +						    ". ");

The nesting seems to be becoming too deep and the body of the for loop is
getting too long.  Time to make it a helper function that handles only one
column, perhaps?

^ permalink raw reply

* Re: [PATCH 1/3] git remote update: Report error for non-existing groups
From: Finn Arne Gangstad @ 2009-04-08  8:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vprfnubyi.fsf@gitster.siamese.dyndns.org>

On Tue, Apr 07, 2009 at 07:16:21PM -0700, Junio C Hamano wrote:
> Finn Arne Gangstad <finnag@pvv.org> writes:
> 
> > @@ -1227,8 +1229,11 @@ static int update(int argc, const char **argv)
> >  
> >  	remote_group.list = &list;
> >  	for (i = 1; i < argc; i++) {
> > +		int groups_found = 0;
> >  		remote_group.name = argv[i];
> > -		result = git_config(get_remote_group, NULL);
> > +		result = git_config(get_remote_group, &groups_found);
> > +		if (!groups_found && (i != 1 || strcmp(argv[1], "default")))
> > +			die("No such remote group: '%s'", argv[i]);
> 
> I think you are trying to be silent about the case where the caller feeds
> you the default_argv[] array with this, but do we want to be more explicit
> about this so that we do die when the end user explicitly says "default"
> from the command line?

Are you thinking that "git remote update default" should only be allowed
if you have configured a group named default?

The old code would allow "git remote update default" and actually do the
same as "git remote update", so I wanted to keep the (possibly unwanted?)
behaviour. If we want to disallow it, we can just do
if (!groups_found && argv != default_argv) instead.

- Finn Arne

^ permalink raw reply

* Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
From: Petr Baudis @ 2009-04-08  8:12 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git
In-Reply-To: <1238975176-14354-1-git-send-email-sam.vilain@catalyst.net.nz>

On Mon, Apr 06, 2009 at 11:46:15AM +1200, Sam Vilain wrote:
> Add a new module, Git::Config, for a better Git configuration API.
> 
> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>

I'm really sorry that I probably won't have time soon to properly review
this patch. :-(

> +			throw Error::Simple (
> +				"'$item' is specified multiple times",
> +			       );

So just one comment - in general people seem to be unhappy with this way
of exception handling, preferring die-eval to throw-catch. We might
seize the opportunity here and start using die in all new modules,
keeping Error::Simple only in legacy Git.pm (and its wrappers for the
Git::Config stuff).

-- 
				Petr "Pasky" Baudis
The average, healthy, well-adjusted adult gets up at seven-thirty
in the morning feeling just terrible. -- Jean Kerr

^ permalink raw reply

* Re: [PATCH 1/3] git remote update: Report error for non-existing groups
From: Junio C Hamano @ 2009-04-08  8:20 UTC (permalink / raw)
  To: Finn Arne Gangstad; +Cc: git
In-Reply-To: <20090408080738.GA24386@pvv.org>

Finn Arne Gangstad <finnag@pvv.org> writes:

> On Tue, Apr 07, 2009 at 07:16:21PM -0700, Junio C Hamano wrote:
>> Finn Arne Gangstad <finnag@pvv.org> writes:
>> 
>> > @@ -1227,8 +1229,11 @@ static int update(int argc, const char **argv)
>> >  
>> >  	remote_group.list = &list;
>> >  	for (i = 1; i < argc; i++) {
>> > +		int groups_found = 0;
>> >  		remote_group.name = argv[i];
>> > -		result = git_config(get_remote_group, NULL);
>> > +		result = git_config(get_remote_group, &groups_found);
>> > +		if (!groups_found && (i != 1 || strcmp(argv[1], "default")))
>> > +			die("No such remote group: '%s'", argv[i]);
>> 
>> I think you are trying to be silent about the case where the caller feeds
>> you the default_argv[] array with this, but do we want to be more explicit
>> about this so that we do die when the end user explicitly says "default"
>> from the command line?
>
> Are you thinking that "git remote update default" should only be allowed
> if you have configured a group named default?

I have no preference either way, and that is why I asked.

"git remote update" without explicit "default" is obviously what your code
try not to say "No such remote group" to, and that probably is a sane
thing to do.

I don't know what users want to see when they say "default" explicitly
without having an explicit configuration.  Should it do the same thing as
"git remote update"?

^ permalink raw reply

* Alles wird Git, was Re: [PATCH] mailmap: resurrect lower-casing of email addresses
From: Johannes Schindelin @ 2009-04-08  9:36 UTC (permalink / raw)
  To: Markus Heidelberg; +Cc: git
In-Reply-To: <200904072158.41927.markus.heidelberg@web.de>

Hi,

[following ALASCM's example, I am culling the Cc: list]

On Tue, 7 Apr 2009, Markus Heidelberg wrote:

> Johannes Schindelin, 07.04.2009:
> 
> > On Mon, 6 Apr 2009, A Large Angry SCM wrote:
> > 
> > > s/Germany and Munich/Berlin and Munich/
> > 
> > Cool!  Berlin is only 2 hours from my home, so let's meet there!
> > 
> > We could even use the opportunity for a little informal German 
> > GitTogether... "Alles wird Git!"?
> 
> Damn, why only is Berlin on the other end of Germany compared to where I 
> live?

There are planes ;-)

> Nice slogan, BTW.

Thanks!

Ciao,
Dscho

^ permalink raw reply

* Re: [ANNOUNCE] git_fast_filter
From: Johannes Schindelin @ 2009-04-08  9:45 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Git Mailing List
In-Reply-To: <51419b2c0904072035u1182b507o836a67ac308d32b9@mail.gmail.com>

Hi,

On Tue, 7 Apr 2009, Elijah Newren wrote:

> Just thought I'd make this available, in case there's others with niche 
> needs that find it useful...

Have you seen

	http://thread.gmane.org/gmane.comp.version-control.git/52323

I was rather disappointed that skimo left the patch series in a rather 
half-useful state.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
From: Sam Vilain @ 2009-04-08  9:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, Frank Lichtenheld, git, Petr Baudis
In-Reply-To: <7vbpr7r72w.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>   
>> Errr... wouldn't it be better to simply use 
>>
>> +		my ($item, $value) = split("\n", $_, 2)
>>
>> here? Have you tested Git::Config with a "null" value, i.e. something
>> like
>>
>>     [section]
>>         noval
>>
>> in the config file (which evaluates to 'true' with '--bool' option)?
>> Because from what I remember from the discussion on the 
>> "git config --null --list" format the lack of "\n" is used to
>> distinguish between noval (which is equivalent to 'true'), and empty
>> value (which is equivalent to 'false')
>>
>>     [boolean
>>         noval        # equivalent to 'true'
>>         empty1 =     # equivalent to 'false'
>>         empty2 = ""  # equivalent to 'false'
>>     
>
> I do not mind if the _write method always wrote out
>
> 	[core]
>         	autocrlf = true
>
> for a variable that is true, but it should be able to read existing
>
> 	[core]
>         	autocrlf
>
> correctly.
>   

Yep - that's what I thought was reasonable behaviour as well and what my 
submission does.

> Sam, I think you meant to make me squash the "Oops, for no good reason,
> here is a fix-up" into the previous one, but for this case, I'd appreciate
> a re-roll of the series, that includes a test to read from an existing
> configuration file that contains such "presense of the name alone means
> boolean true" variables.
>   

Sure, I rebased the series to have the fix-ups at the right places, but 
didn't think it was an interesting enough change to rate a full 
re-submission. The series at git://github.com/samv/git branch 
perl-Config has the minor change put into the place it was introduced. I 
put a little note to this effect after the --- line.

I'm not quite sure what you want squashed where, maybe just edit the 
below list to be how you'd like it,

pick d43238e perl: add new module Git::Config for cached 'git config' access
pick 5ea135d perl: make Git.pm use new Git::Config module
pick b2865bc perl: make Git::Config use --null
pick 28eecdc perl: fix no value items in Git::Config

:-)

Sam

^ permalink raw reply

* [PATCH 0/8] support "in-tree attributes" for git-archive
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This series attempts to fix "git archive" sometimes looking into
.gitattributes in working directory and may cause undesired effects.

Nguyễn Thái Ngọc Duy (8):
  archive: add shortcuts for --format and --prefix

This one is unrelated. Just a convenient patch.

  move prune_cache() to git lib
  archive: add a failure test wrt .gitattributes  misreading
  archive: add tests for directory selection
  attr: add GIT_ATTR_INDEX "direction"
  archive: use index instead of parsing tree directly
  archive: disregard .gitattributes on working directory

Main part, which converts archive.c to load index first, then export from there.

  archive: support creating archives from index

Well, show off patch ;-)

 Documentation/git-archive.txt |    4 +-
 archive.c                     |  106 +++++++++++++++++++++++++++++++----------
 archive.h                     |    1 +
 attr.c                        |    4 +-
 attr.h                        |    3 +-
 builtin-ls-files.c            |   29 +-----------
 cache.h                       |    2 +
 read-cache.c                  |   27 ++++++++++
 t/t0024-crlf-archive.sh       |   35 +++++++++++++-
 t/t5000-tar-tree.sh           |    6 ++-
 10 files changed, 158 insertions(+), 59 deletions(-)

^ permalink raw reply

* [PATCH 1/8] archive: add shortcuts for --format and --prefix
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-archive.txt |    4 +++-
 archive.c                     |    4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index c1adf59..c9fff54 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -9,7 +9,7 @@ git-archive - Create an archive of files from a named tree
 SYNOPSIS
 --------
 [verse]
-'git archive' --format=<fmt> [--list] [--prefix=<prefix>/] [<extra>]
+'git archive' [-f|--format=<fmt>] [--list] [-p|--prefix=<prefix>/] [<extra>]
 	      [--output=<file>]
 	      [--remote=<repo> [--exec=<git-upload-archive>]] <tree-ish>
 	      [path...]
@@ -33,6 +33,7 @@ comment.
 OPTIONS
 -------
 
+-f=<fmt>::
 --format=<fmt>::
 	Format of the resulting archive: 'tar' or 'zip'.  The default
 	is 'tar'.
@@ -45,6 +46,7 @@ OPTIONS
 --verbose::
 	Report progress to stderr.
 
+-p=<prefix>/::
 --prefix=<prefix>/::
 	Prepend <prefix>/ to each filename in the archive.
 
diff --git a/archive.c b/archive.c
index 96b62d4..e87fed7 100644
--- a/archive.c
+++ b/archive.c
@@ -260,8 +260,8 @@ static int parse_archive_args(int argc, const char **argv,
 	int list = 0;
 	struct option opts[] = {
 		OPT_GROUP(""),
-		OPT_STRING(0, "format", &format, "fmt", "archive format"),
-		OPT_STRING(0, "prefix", &base, "prefix",
+		OPT_STRING('f', "format", &format, "fmt", "archive format"),
+		OPT_STRING('p', "prefix", &base, "prefix",
 			"prepend prefix to each pathname in the archive"),
 		OPT_STRING(0, "output", &output, "file",
 			"write the archive to this file"),
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 2/8] move prune_cache() to git lib
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-2-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin-ls-files.c |   29 +----------------------------
 cache.h            |    2 ++
 read-cache.c       |   27 +++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 88e2697..0151647 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -206,33 +206,6 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 	}
 }
 
-/*
- * Prune the index to only contain stuff starting with "prefix"
- */
-static void prune_cache(const char *prefix)
-{
-	int pos = cache_name_pos(prefix, prefix_len);
-	unsigned int first, last;
-
-	if (pos < 0)
-		pos = -pos-1;
-	memmove(active_cache, active_cache + pos,
-		(active_nr - pos) * sizeof(struct cache_entry *));
-	active_nr -= pos;
-	first = 0;
-	last = active_nr;
-	while (last > first) {
-		int next = (last + first) >> 1;
-		struct cache_entry *ce = active_cache[next];
-		if (!strncmp(ce->name, prefix, prefix_len)) {
-			first = next+1;
-			continue;
-		}
-		last = next;
-	}
-	active_nr = last;
-}
-
 static const char *verify_pathspec(const char *prefix)
 {
 	const char **p, *n, *prev;
@@ -541,7 +514,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		show_cached = 1;
 
 	if (prefix)
-		prune_cache(prefix);
+		prune_cache(prefix, prefix_len);
 	if (with_tree) {
 		/*
 		 * Basic sanity check; show-stages and show-unmerged
diff --git a/cache.h b/cache.h
index ab1294d..54660a5 100644
--- a/cache.h
+++ b/cache.h
@@ -335,6 +335,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 #define ce_modified(ce, st, options) ie_modified(&the_index, (ce), (st), (options))
 #define cache_name_exists(name, namelen, igncase) index_name_exists(&the_index, (name), (namelen), (igncase))
 #define cache_name_is_other(name, namelen) index_name_is_other(&the_index, (name), (namelen))
+#define prune_cache(prefix, prefix_len) prune_index((prefix), (prefix_len), &the_index);
 #endif
 
 enum object_type {
@@ -455,6 +456,7 @@ extern int add_file_to_index(struct index_state *, const char *path, int flags);
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);
 extern int ce_same_name(struct cache_entry *a, struct cache_entry *b);
 extern int index_name_is_other(const struct index_state *, const char *, int);
+extern void prune_index(const char *prefix, int prefix_len, struct index_state *istate);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index 3f58711..74f889e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1706,3 +1706,30 @@ int index_name_is_other(const struct index_state *istate, const char *name,
 	}
 	return 1;
 }
+
+/*
+ * Prune the index to only contain stuff starting with "prefix"
+ */
+void prune_index(const char *prefix, int prefix_len, struct index_state *istate)
+{
+	int pos = index_name_pos(istate, prefix, prefix_len);
+	unsigned int first, last;
+
+	if (pos < 0)
+		pos = -pos-1;
+	memmove(istate->cache, istate->cache + pos,
+		(istate->cache_nr - pos) * sizeof(struct cache_entry *));
+	istate->cache_nr -= pos;
+	first = 0;
+	last = istate->cache_nr;
+	while (last > first) {
+		int next = (last + first) >> 1;
+		struct cache_entry *ce = istate->cache[next];
+		if (!strncmp(ce->name, prefix, prefix_len)) {
+			first = next+1;
+			continue;
+		}
+		last = next;
+	}
+	istate->cache_nr = last;
+}
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 3/8] archive: add a failure test wrt .gitattributes  misreading
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-3-git-send-email-pclouds@gmail.com>

"git archive" is used to create archives from a tree (or commit), so
it should not consult any .gitattributes files on working directory.

It currently does, so this patch records the failure.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t0024-crlf-archive.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index c7d0324..9c56e0e 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -43,4 +43,11 @@ test_expect_success UNZIP 'zip archive' '
 
 '
 
+test_expect_failure '.gitattributes in workdir should not be consulted' '
+	echo "sample -crlf" > .gitattributes &&
+	git archive --format=tar HEAD |
+	( mkdir untarred2 && cd untarred2 && "$TAR" -xf - )
+	test_cmp sample untarred2/sample
+'
+
 test_done
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 4/8] archive: add tests for directory selection
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-4-git-send-email-pclouds@gmail.com>

While in archive.c, it says "pathspec". It's not really pathspec.
Make a few tests to catch this point.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c               |   17 +++++++++++++++++
 t/t0024-crlf-archive.sh |   30 +++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/archive.c b/archive.c
index e87fed7..5b54d95 100644
--- a/archive.c
+++ b/archive.c
@@ -15,6 +15,8 @@ static char const * const archive_usage[] = {
 
 #define USES_ZLIB_COMPRESSION 1
 
+static int list_archive(struct archiver_args *args);
+
 static const struct archiver {
 	const char *name;
 	write_archive_fn_t write_archive;
@@ -22,8 +24,23 @@ static const struct archiver {
 } archivers[] = {
 	{ "tar", write_tar_archive },
 	{ "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
+	{ "dump-file-list", list_archive },
 };
 
+static int list_archive_entry(struct archiver_args *args,
+		const unsigned char *sha1, const char *path, size_t pathlen,
+		unsigned int mode, void *buffer, unsigned long size)
+{
+	printf("%s\n", path);
+	return 0;
+}
+
+static int list_archive(struct archiver_args *args)
+{
+	write_archive_entries(args, list_archive_entry);
+	return 0;
+}
+
 static void format_subst(const struct commit *commit,
                          const char *src, size_t len,
                          struct strbuf *buf)
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index 9c56e0e..695ad5a 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -9,14 +9,42 @@ test_expect_success setup '
 
 	git config core.autocrlf true
 
+	mkdir sub &&
 	printf "CRLF line ending\r\nAnd another\r\n" > sample &&
-	git add sample &&
+	cp sample sub/sample &&
+	git add sample sub/sample &&
 
 	test_tick &&
 	git commit -m Initial
 
 '
 
+cat <<\EOF > expected
+sample
+sub/
+sub/sample
+EOF
+
+test_expect_success 'archive without subdir' '
+	git archive --format=dump-file-list HEAD > output &&
+	test_cmp output expected
+'
+
+cat <<\EOF > expected
+sub/
+sub/sample
+EOF
+
+test_expect_success 'archive with subdir' '
+	git archive --format=dump-file-list HEAD sub > output &&
+	test_cmp output expected
+'
+
+test_expect_success 'subdir is not pathspec' '
+	test "$(git archive --format=dump-file-list HEAD sub/)" = ""
+	test "$(git archive --format=dump-file-list HEAD s\*)" = ""
+'
+
 test_expect_success 'tar archive' '
 
 	git archive --format=tar HEAD |
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 5/8] attr: add GIT_ATTR_INDEX "direction"
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-5-git-send-email-pclouds@gmail.com>

This instructs attr mechanism not to look into working .gitattributes
at all. Needed by tools that does not handle working directory, such
as "git archive".

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 attr.c |    4 +++-
 attr.h |    3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/attr.c b/attr.c
index 43259e5..37ca288 100644
--- a/attr.c
+++ b/attr.c
@@ -405,7 +405,7 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)
 		if (!res)
 			res = read_attr_from_file(path, macro_ok);
 	}
-	else {
+	else if (direction == GIT_ATTR_CHECKIN) {
 		res = read_attr_from_file(path, macro_ok);
 		if (!res)
 			/*
@@ -415,6 +415,8 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)
 			 */
 			res = read_attr_from_index(path, macro_ok);
 	}
+	else
+		res = read_attr_from_index(path, macro_ok);
 	if (!res)
 		res = xcalloc(1, sizeof(*res));
 	return res;
diff --git a/attr.h b/attr.h
index 3a2f4ec..69b5767 100644
--- a/attr.h
+++ b/attr.h
@@ -33,7 +33,8 @@ int git_checkattr(const char *path, int, struct git_attr_check *);
 
 enum git_attr_direction {
 	GIT_ATTR_CHECKIN,
-	GIT_ATTR_CHECKOUT
+	GIT_ATTR_CHECKOUT,
+	GIT_ATTR_INDEX,
 };
 void git_attr_set_direction(enum git_attr_direction, struct index_state *);
 
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 6/8] archive: use index instead of parsing tree directly
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-6-git-send-email-pclouds@gmail.com>

Currently:

 - "git archive" parses trees directly
 - attr mechanism does not support in-tree attributes

Making attr.c support in-tree attributes is quite a non-trivial task,
IMHO. Instead this patch makes "git archive" read tree to index first,
then start exporting from there.

One minor regression: "git archive" now will no longer generate
directory entries, only files.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c               |   66 ++++++++++++++++++++++++++++++++---------------
 t/t0024-crlf-archive.sh |    2 -
 2 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/archive.c b/archive.c
index 5b54d95..402f6d4 100644
--- a/archive.c
+++ b/archive.c
@@ -4,6 +4,8 @@
 #include "attr.h"
 #include "archive.h"
 #include "parse-options.h"
+#include "unpack-trees.h"
+#include "dir.h"
 
 static char const * const archive_usage[] = {
 	"git archive [options] <tree-ish> [path...]",
@@ -167,7 +169,9 @@ int write_archive_entries(struct archiver_args *args,
 		write_archive_entry_fn_t write_entry)
 {
 	struct archiver_context context;
-	int err;
+	struct unpack_trees_options opts;
+	struct tree_desc t;
+	int i, prefix_len, err;
 
 	if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
 		size_t len = args->baselen;
@@ -185,11 +189,42 @@ int write_archive_entries(struct archiver_args *args,
 	context.args = args;
 	context.write_entry = write_entry;
 
-	err =  read_tree_recursive(args->tree, args->base, args->baselen, 0,
-			args->pathspec, write_archive_entry, &context);
-	if (err == READ_TREE_RECURSIVE)
-		err = 0;
-	return err;
+	memset(&opts, 0, sizeof(opts));
+	opts.index_only = 1;
+	opts.head_idx = -1;
+	opts.src_index = &the_index;
+	opts.dst_index = &the_index;
+	opts.fn = oneway_merge;
+	init_tree_desc(&t, args->tree->buffer, args->tree->size);
+	if (unpack_trees(1, &t, &opts))
+		return -1;
+	prefix_len = common_prefix(args->pathspec);
+	if (prefix_len)
+		prune_cache(args->pathspec[0], prefix_len);
+
+	for (i = 0;i < active_nr;i++) {
+		struct cache_entry *ce = active_cache[i];
+		const char **path;
+		const char *str, *prefix;
+		int match = 0;
+
+		if (!args->pathspec || !args->pathspec[0] || !*args->pathspec[0])
+			match = 1;
+
+		/* It's not really pathspec, so match_pathspec can't be used */
+		for (path = args->pathspec;!match && *path;path++) {
+			for (str = ce->name, prefix = *path;*prefix && *str == *prefix; str++, prefix++)
+				;
+			if (!*prefix && *str == '/')
+				match = 1;
+		}
+
+		if (match)
+			write_archive_entry(ce->sha1, args->base, args->baselen,
+					    ce->name, ce->ce_mode, ce_stage(ce),
+					    &context);
+	}
+	return 0;
 }
 
 static const struct archiver *lookup_archiver(const char *name)
@@ -207,9 +242,10 @@ static const struct archiver *lookup_archiver(const char *name)
 }
 
 static void parse_pathspec_arg(const char **pathspec,
-		struct archiver_args *ar_args)
+			       struct archiver_args *ar_args,
+			       const char *prefix)
 {
-	ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
+	ar_args->pathspec = get_pathspec(prefix, pathspec);
 }
 
 static void parse_treeish_arg(const char **argv,
@@ -238,18 +274,6 @@ static void parse_treeish_arg(const char **argv,
 	if (tree == NULL)
 		die("not a tree object");
 
-	if (prefix) {
-		unsigned char tree_sha1[20];
-		unsigned int mode;
-		int err;
-
-		err = get_tree_entry(tree->object.sha1, prefix,
-				     tree_sha1, &mode);
-		if (err || !S_ISDIR(mode))
-			die("current working directory is untracked");
-
-		tree = parse_tree_indirect(tree_sha1);
-	}
 	ar_args->tree = tree;
 	ar_args->commit_sha1 = commit_sha1;
 	ar_args->commit = commit;
@@ -356,7 +380,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 		prefix = setup_git_directory();
 
 	parse_treeish_arg(argv, &args, prefix);
-	parse_pathspec_arg(argv + 1, &args);
+	parse_pathspec_arg(argv + 1, &args, prefix);
 
 	git_config(git_default_config, NULL);
 
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index 695ad5a..1cbe511 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -21,7 +21,6 @@ test_expect_success setup '
 
 cat <<\EOF > expected
 sample
-sub/
 sub/sample
 EOF
 
@@ -31,7 +30,6 @@ test_expect_success 'archive without subdir' '
 '
 
 cat <<\EOF > expected
-sub/
 sub/sample
 EOF
 
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 7/8] archive: disregard .gitattributes on working directory
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-7-git-send-email-pclouds@gmail.com>

Some tests in t500-tar-tree.sh depends on in-worktree
.gitattributes. Fix them too.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 We could make an option to keep old behaviour. But is it worth it?

 archive.c               |    1 +
 t/t0024-crlf-archive.sh |    2 +-
 t/t5000-tar-tree.sh     |    6 +++++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/archive.c b/archive.c
index 402f6d4..1ce644e 100644
--- a/archive.c
+++ b/archive.c
@@ -138,6 +138,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 	path_without_prefix = path.buf + args->baselen;
 
 	setup_archive_check(check);
+	git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
 	if (!git_checkattr(path_without_prefix, ARRAY_SIZE(check), check)) {
 		if (ATTR_TRUE(check[0].value))
 			return 0;
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index 1cbe511..bf531fc 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -69,7 +69,7 @@ test_expect_success UNZIP 'zip archive' '
 
 '
 
-test_expect_failure '.gitattributes in workdir should not be consulted' '
+test_expect_success '.gitattributes in workdir should not be consulted' '
 	echo "sample -crlf" > .gitattributes &&
 	git archive --format=tar HEAD |
 	( mkdir untarred2 && cd untarred2 && "$TAR" -xf - )
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 7641e0d..420e7ad 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -56,6 +56,7 @@ test_expect_success \
     'add files to repository' \
     'find a -type f | xargs git update-index --add &&
      find a -type l | xargs git update-index --add &&
+     git update-index --add .gitattributes &&
      treeid=`git write-tree` &&
      echo $treeid >treeid &&
      git update-ref HEAD $(TZ=GMT GIT_COMMITTER_DATE="2005-05-27 22:00:00" \
@@ -140,6 +141,8 @@ test_expect_success \
 test_expect_success \
     'create archives with substfiles' \
     'echo "substfile?" export-subst >a/.gitattributes &&
+     git add a/.gitattributes &&
+     git commit -q -m new &&
      git archive HEAD >f.tar &&
      git archive --prefix=prefix/ HEAD >g.tar &&
      rm a/.gitattributes'
@@ -165,7 +168,8 @@ test_expect_success \
      'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
       >g/prefix/a/substfile1.expected &&
       test_cmp g/prefix/a/substfile1.expected g/prefix/a/substfile1 &&
-      test_cmp a/substfile2 g/prefix/a/substfile2
+      test_cmp a/substfile2 g/prefix/a/substfile2 &&
+      git reset HEAD^
 '
 
 test_expect_success \
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* [PATCH 8/8] archive: support creating archives from index
From: Nguyễn Thái Ngọc Duy @ 2009-04-08 10:05 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1239185133-4181-8-git-send-email-pclouds@gmail.com>

This is more or less for fun. Does anyone really want to create archives
from index?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c |   38 +++++++++++++++++++++++++-------------
 archive.h |    1 +
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/archive.c b/archive.c
index 1ce644e..bb0a0f1 100644
--- a/archive.c
+++ b/archive.c
@@ -181,7 +181,7 @@ int write_archive_entries(struct archiver_args *args,
 			len--;
 		if (args->verbose)
 			fprintf(stderr, "%.*s\n", (int)len, args->base);
-		err = write_entry(args, args->tree->object.sha1, args->base,
+		err = write_entry(args, args->tree ? args->tree->object.sha1 : null_sha1, args->base,
 				len, 040777, NULL, 0);
 		if (err)
 			return err;
@@ -190,15 +190,19 @@ int write_archive_entries(struct archiver_args *args,
 	context.args = args;
 	context.write_entry = write_entry;
 
-	memset(&opts, 0, sizeof(opts));
-	opts.index_only = 1;
-	opts.head_idx = -1;
-	opts.src_index = &the_index;
-	opts.dst_index = &the_index;
-	opts.fn = oneway_merge;
-	init_tree_desc(&t, args->tree->buffer, args->tree->size);
-	if (unpack_trees(1, &t, &opts))
-		return -1;
+	if (args->tree) {
+		memset(&opts, 0, sizeof(opts));
+		opts.index_only = 1;
+		opts.head_idx = -1;
+		opts.src_index = &the_index;
+		opts.dst_index = &the_index;
+		opts.fn = oneway_merge;
+		init_tree_desc(&t, args->tree->buffer, args->tree->size);
+		if (unpack_trees(1, &t, &opts))
+			return -1;
+	}
+	else
+		read_cache();
 	prefix_len = common_prefix(args->pathspec);
 	if (prefix_len)
 		prune_cache(args->pathspec[0], prefix_len);
@@ -300,6 +304,7 @@ static int parse_archive_args(int argc, const char **argv,
 	int verbose = 0;
 	int i;
 	int list = 0;
+	int use_index = 0;
 	struct option opts[] = {
 		OPT_GROUP(""),
 		OPT_STRING('f', "format", &format, "fmt", "archive format"),
@@ -307,6 +312,8 @@ static int parse_archive_args(int argc, const char **argv,
 			"prepend prefix to each pathname in the archive"),
 		OPT_STRING(0, "output", &output, "file",
 			"write the archive to this file"),
+		OPT_BOOLEAN('i', "index", &use_index,
+			"generate from index instead"),
 		OPT__VERBOSE(&verbose),
 		OPT__COMPR('0', &compression_level, "store only", 0),
 		OPT__COMPR('1', &compression_level, "compress faster", 1),
@@ -348,7 +355,7 @@ static int parse_archive_args(int argc, const char **argv,
 	}
 
 	/* We need at least one parameter -- tree-ish */
-	if (argc < 1)
+	if (argc < 1 && !use_index)
 		usage_with_options(archive_usage, opts);
 	*ar = lookup_archiver(format);
 	if (!*ar)
@@ -366,6 +373,7 @@ static int parse_archive_args(int argc, const char **argv,
 	args->verbose = verbose;
 	args->base = base;
 	args->baselen = strlen(base);
+	args->use_index = use_index;
 
 	return argc;
 }
@@ -376,12 +384,16 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	const struct archiver *ar = NULL;
 	struct archiver_args args;
 
+	memset(&args, 0, sizeof(args));
 	argc = parse_archive_args(argc, argv, &ar, &args);
 	if (setup_prefix && prefix == NULL)
 		prefix = setup_git_directory();
 
-	parse_treeish_arg(argv, &args, prefix);
-	parse_pathspec_arg(argv + 1, &args, prefix);
+	if (!args.use_index) {
+		parse_treeish_arg(argv, &args, prefix);
+		argv++;
+	}
+	parse_pathspec_arg(argv, &args, prefix);
 
 	git_config(git_default_config, NULL);
 
diff --git a/archive.h b/archive.h
index 0b15b35..fb94d47 100644
--- a/archive.h
+++ b/archive.h
@@ -10,6 +10,7 @@ struct archiver_args {
 	time_t time;
 	const char **pathspec;
 	unsigned int verbose : 1;
+	unsigned int use_index : 1;
 	int compression_level;
 };
 
-- 
1.6.2.2.602.g83ee9f

^ permalink raw reply related

* Re: [msysGit] Git for Windows 1.6.2.2
From: Janos Laube @ 2009-04-08 10:11 UTC (permalink / raw)
  To: Johannes.Schindelin; +Cc: msysgit, git
In-Reply-To: <alpine.DEB.1.00.0904080440170.10279@pacific.mpi-cbg.de>

>
> Hi,
>
> this email tries to inform you that the Windows installer was updated to
> the newest version of Git.
>
> Changes since Git-1.6.2.1-preview20090322
>
> New Features
>
> - Comes with official git 1.6.2.2.
> - Upgraded Tcl/Tk to 8.5.5.
> - TortoiseMerge is supported by mergetool now (Thanks Markus Heidelberg).
> - Uses pthreads (faster garbage collection on multi-core machines).
> - The test suite passes!
>
> Bugfixes
>
> - Renaming was made more robust (due to Explorer or some virus scanners,
>  files could not be renamed at the first try, so we have to try multiple
>  times).
> - Johannes Sixt made lots of changes to the test-suite to identify
>  properly which tests should pass, and which ones cannot pass due to
>  limitations of the platform.
> - Support PAGERs with spaces in their filename.
> - Quite a few changes were undone which we needed in the olden days of
>  msysGit.
> - Fall back to / when HOME cannot be set to the real home directory due to
>  locale issues (works around Issue 108 for the moment).
>
> Ciao,
> Dscho "who really needs some sleep"

thank you very much! awesome work :-)

^ permalink raw reply

* Re: [PATCH] perl: add new module Git::Config for cached 'git config' access
From: Jakub Narebski @ 2009-04-08 10:18 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Frank Lichtenheld, git, Petr Baudis
In-Reply-To: <49DC3ADD.5000902@catalyst.net.nz>

By the way, did you take a look how cached 'git config' access and
typecasting is done in gitweb?  See commit b201927 (gitweb: Read
repo config using 'git config -z -l') and following similar commits.

On Wed, 8 April 2009, Sam Vilain wrote:
> Jakub Narebski wrote:

>>> -		my ($item, $value) = m{(.*?)=(.*)};
>>> +		my ($item, $value) = m{(.*?)\n((?s:.*))\0}
>>> +			or die "failed to parse it; \$_='$_'";
>> 
>> Errr... wouldn't it be better to simply use 
>> 
>> +		my ($item, $value) = split("\n", $_, 2)
>> 
>> here?
> 
> Yeah, I guess that's easier to read and possibly faster; both are
> using the regexp engine and using COW strings though, so it's probably
> not as bad as one might think.

The version using 'split' has the advantage that for config variable
with no value (e.g. "[section] noval") it sets $item (why this variable
is called $item and not $var, $variable or $key, BTW.?) to fully 
qualified variable name (e.g. "section.noval"), and sets $value to 
undef, instead of failing like your original version using regexp.

And I also think that this version is easier to understand, and might be 
a bit faster as well; but it is more important to be easier to 
understand.

>> Have you tested Git::Config with a "null" value, i.e. something
>> like
>> 
>>     [section]
>>         noval
>> 
>> in the config file (which evaluates to 'true' with '--bool' option)?
>> Because from what I remember from the discussion on the 
>> "git config --null --list" format the lack of "\n" is used to
>> distinguish between noval (which is equivalent to 'true'), and empty
>> value (which is equivalent to 'false')
>> 
>>     [boolean]
>>         noval        # equivalent to 'true'
>>         empty1 =     # equivalent to 'false'
>>         empty2 = ""  # equivalent to 'false'
> 
> That I didn't consider.  Below is a patch for this.  Any more
> gremlins? 

I have nor examined your patch in detail; I'll try to do it soon,
but with git config file parsing there lies following traps.

1. In fully qualified variable name section name and variable name
   have to be compared case insensitive (or normalized, i.e.
   lowercased), while subsection part (if it exists) is case sensitive.

2. When coercing type to bool, you need to remember (and test) that
   there are values which are truish (no value, 'true', 'yes', non-zero
   integer usually 1), values which are falsish (empry, 'false', 'no',
   0); other values IIRC are truish too.

3. When coercing type to int, you need to remember about optional
   value suffixes: 'k', 'm' or 'g'.

4. I don't know if you remembered about 'colorbool' and 'color'; the
   latter would probably require some extra CPAN module for ANSI color
   escapes... or copying color codes from the C version.

> 
> Subject: perl: fix no value items in Git::Config
> 
> When interpreted as boolean, items in the configuration which do not
> have an '=' are interpreted as true.  Parse for this situation, and
> represent it with an object in the state hash which works a bit like
> undef, but isn't.

Why not represent it simply as an 'undef'? You can always distinguish 
between not defined and not existing by using 'exists'...

> Sneak a couple of vim footer changes in too.

Hmmm...

> 
> Signed-off-by: Sam Vilain <sam@vilain.net>

[...]
-- 
Jakub Narebski
Poland

^ 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