Git development
 help / color / mirror / Atom feed
* [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec
From: Thomas Gummerer @ 2017-02-05 20:26 UTC (permalink / raw)
  To: git
  Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
	Thomas Gummerer
In-Reply-To: <20170205202642.14216-1-t.gummerer@gmail.com>

While working on a repository, it's often helpful to stash the changes
of a single or multiple files, and leave others alone.  Unfortunately
git currently offers no such option.  git stash -p can be used to work
around this, but it's often impractical when there are a lot of changes
over multiple files.

Allow 'git stash push' to take pathspec to specify which paths to stash.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt        | 11 ++++++++++
 git-stash.sh                       | 30 ++++++++++++++++++++-------
 t/t3903-stash.sh                   | 42 ++++++++++++++++++++++++++++++++++++++
 t/t3905-stash-include-untracked.sh | 26 +++++++++++++++++++++++
 4 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index a138ed6a24..be55e456fa 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -15,9 +15,13 @@ SYNOPSIS
 'git stash' branch <branchname> [<stash>]
 'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 	     [-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+	     [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
+	     [--] [<pathspec>...]
 'git stash' clear
 'git stash' create [<message>]
 'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
+	     [-- <paths>...]
 'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
 
 DESCRIPTION
@@ -47,6 +51,7 @@ OPTIONS
 -------
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
 
 	Save your local modifications to a new 'stash' and roll them
 	back to HEAD (in the working tree and in the index).
@@ -56,6 +61,12 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
 	only <message> does not trigger this action to prevent a misspelled
 	subcommand from making an unwanted stash.
 +
+When pathspec is given to 'git stash push', the new stash records the
+modified states only for the files that match the pathspec.  The index
+entries and working tree files are then rolled back to the state in
+HEAD only for these files, too, leaving files that do not match the
+pathspec intact.
++
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
 +
diff --git a/git-stash.sh b/git-stash.sh
index 33b2d0384c..46367d40aa 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -41,7 +41,7 @@ no_changes () {
 untracked_files () {
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt
+	git ls-files -o -z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -72,6 +72,11 @@ create_stash () {
 			untracked="$1"
 			new_style=t
 			;;
+		--)
+			shift
+			new_style=t
+			break
+			;;
 		*)
 			if test -n "$new_style"
 			then
@@ -99,6 +104,10 @@ create_stash () {
 	if test -z "$new_style"
 	then
 		stash_msg="$*"
+		while test $# != 0
+		do
+			shift
+		done
 	fi
 
 	git update-index -q --refresh
@@ -134,7 +143,7 @@ create_stash () {
 		# Untracked files are stored by themselves in a parentless commit, for
 		# ease of unpacking later.
 		u_commit=$(
-			untracked_files | (
+			untracked_files "$@" | (
 				GIT_INDEX_FILE="$TMPindex" &&
 				export GIT_INDEX_FILE &&
 				rm -f "$TMPindex" &&
@@ -157,7 +166,7 @@ create_stash () {
 			git read-tree --index-output="$TMPindex" -m $i_tree &&
 			GIT_INDEX_FILE="$TMPindex" &&
 			export GIT_INDEX_FILE &&
-			git diff-index --name-only -z HEAD -- >"$TMP-stagenames" &&
+			git diff-index --name-only -z HEAD -- "$@" >"$TMP-stagenames" &&
 			git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
 			git write-tree &&
 			rm -f "$TMPindex"
@@ -171,7 +180,7 @@ create_stash () {
 
 		# find out what the user wants
 		GIT_INDEX_FILE="$TMP-index" \
-			git add--interactive --patch=stash -- &&
+			git add--interactive --patch=stash -- "$@" &&
 
 		# state of the working tree
 		w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
@@ -307,18 +316,25 @@ push_stash () {
 	git reflog exists $ref_stash ||
 		clear_stash || die "$(gettext "Cannot initialize stash")"
 
-	create_stash -m "$stash_msg" -u "$untracked"
+	create_stash -m "$stash_msg" -u "$untracked" -- "$@"
 	store_stash -m "$stash_msg" -q $w_commit ||
 	die "$(gettext "Cannot save the current status")"
 	say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
 
 	if test -z "$patch_mode"
 	then
-		git reset --hard ${GIT_QUIET:+-q}
+		if test $# != 0
+		then
+			git ls-files -z -- "$@" | xargs -0 git reset --
+			git ls-files -z --modified -- "$@" | xargs -0 git checkout HEAD --
+			git ls-files -z --others -- "$@" | xargs -0 git clean --force --
+		else
+			git reset --hard ${GIT_QUIET:+-q}
+		fi
 		test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
 		if test -n "$untracked"
 		then
-			git clean --force --quiet -d $CLEAN_X_OPTION
+			git clean --force --quiet -d $CLEAN_X_OPTION -- "$@"
 		fi
 
 		if test "$keep_index" = "t" && test -n "$i_tree"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index b859e51086..461fe46045 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -811,4 +811,46 @@ test_expect_success 'new style stash create stores correct message' '
 	test_cmp expect actual
 '
 
+test_expect_success 'stash -- <filename> stashes and restores the file' '
+	>foo &&
+	>bar &&
+	git add foo bar &&
+	git stash push -- foo &&
+	test_path_is_file bar &&
+	test_path_is_missing foo &&
+	git stash pop &&
+	test_path_is_file foo &&
+	test_path_is_file bar
+'
+
+test_expect_success 'stash with multiple filename arguments' '
+	>foo &&
+	>bar &&
+	>extra &&
+	git add foo bar extra &&
+	git stash push -- foo bar &&
+	test_path_is_missing bar &&
+	test_path_is_missing foo &&
+	test_path_is_file extra &&
+	git stash pop &&
+	test_path_is_file foo &&
+	test_path_is_file bar &&
+	test_path_is_file extra
+'
+
+test_expect_success 'stash with file including $IFS character' '
+	>"foo	bar" &&
+	>foo &&
+	>untracked &&
+	git add foo* &&
+	git stash push -- foo* &&
+	test_path_is_missing "foo	bar" &&
+	test_path_is_missing foo &&
+	test_path_is_file untracked &&
+	git stash pop &&
+	test_path_is_file "foo	bar" &&
+	test_path_is_file foo &&
+	test_path_is_file untracked
+'
+
 test_done
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f372fc8ca8..9a98e31a3e 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '
 	test -s .gitignore
 '
 
+test_expect_success 'stash push --include-untracked with pathspec' '
+	>foo &&
+	>bar &&
+	git stash push --include-untracked -- foo &&
+	test_path_is_file bar &&
+	test_path_is_missing foo &&
+	git stash pop &&
+	test_path_is_file bar &&
+	test_path_is_file foo
+'
+
+test_expect_success 'stash push with $IFS character' '
+	>"foo	bar" &&
+	>foo &&
+	>bar &&
+	git add foo* &&
+	git stash push --include-untracked -- foo* &&
+	test_path_is_missing "foo	bar" &&
+	test_path_is_missing foo &&
+	test_path_is_file bar &&
+	git stash pop &&
+	test_path_is_file "foo	bar" &&
+	test_path_is_file foo &&
+	test_path_is_file bar
+'
+
 test_done
-- 
2.12.0.rc0.208.g81c5d00b20.dirty


^ permalink raw reply related

* [PATCH v3 1/5] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-02-05 20:26 UTC (permalink / raw)
  To: git
  Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
	Thomas Gummerer
In-Reply-To: <20170205202642.14216-1-t.gummerer@gmail.com>

Don't mention git reset --hard in the documentation for git stash save.
It's an implementation detail that doesn't matter to the end user and
thus shouldn't be exposed to them.  In addition it's not quite true for
git stash -p, and will not be true when a filename argument to limit the
stash to a few files is introduced.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..2e9e344cd7 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -47,8 +47,9 @@ OPTIONS
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
 
-	Save your local modifications to a new 'stash', and run `git reset
-	--hard` to revert them.  The <message> part is optional and gives
+	Save your local modifications to a new 'stash' and roll them
+	back to HEAD (in the working tree and in the index).
+	The <message> part is optional and gives
 	the description along with the stashed state.  For quickly making
 	a snapshot, you can omit _both_ "save" and <message>, but giving
 	only <message> does not trigger this action to prevent a misspelled
-- 
2.12.0.rc0.208.g81c5d00b20.dirty


^ permalink raw reply related

* [PATCH v3 3/5] stash: add test for the create command line arguments
From: Thomas Gummerer @ 2017-02-05 20:26 UTC (permalink / raw)
  To: git
  Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
	Thomas Gummerer
In-Reply-To: <20170205202642.14216-1-t.gummerer@gmail.com>

Currently there is no test showing the expected behaviour of git stash
create's command line arguments.  Add a test for that to show the
current expected behaviour and to make sure future refactorings don't
break those expectations.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 t/t3903-stash.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 0171b824c9..21cb70dc74 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
 	test_cmp expect actual
 '
 
+test_expect_success 'create stores correct message' '
+	>foo &&
+	git add foo &&
+	STASH_ID=$(git stash create "create test message") &&
+	echo "On master: create test message" >expect &&
+	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'create with multiple arguments for the message' '
+	>foo &&
+	git add foo &&
+	STASH_ID=$(git stash create test untracked) &&
+	echo "On master: test untracked" >expect &&
+	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.12.0.rc0.208.g81c5d00b20.dirty


^ permalink raw reply related

* [PATCH v3 0/5] stash: support pathspec argument
From: Thomas Gummerer @ 2017-02-05 20:26 UTC (permalink / raw)
  To: git
  Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
	Thomas Gummerer
In-Reply-To: <20170129201604.30445-1-t.gummerer@gmail.com>

Thanks Junio for the review in the previous round.

Changes since v2:

- $IFS should now be supported by using "$@" everywhere instead of using
  a $files variable.
- Added a new patch showing the old behaviour of git stash create is
  preserved.
- Rephrased the documentation
- Simplified the option parsing in save_stash, by leaving the
  actual parsing to push_stash instead.

Interdiff below:

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8306bac397..be55e456fa 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 	     [-u|--include-untracked] [-a|--all] [<message>]]
 'git stash' push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 	     [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
-	     [--] [<paths>...]
+	     [--] [<pathspec>...]
 'git stash' clear
 'git stash' create [<message>]
 'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
@@ -51,19 +51,21 @@ OPTIONS
 -------
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<paths>...]::
+push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
 
 	Save your local modifications to a new 'stash' and roll them
-	back both in the working tree and in the index.
+	back to HEAD (in the working tree and in the index).
 	The <message> part is optional and gives
 	the description along with the stashed state.  For quickly making
 	a snapshot, you can omit _both_ "save" and <message>, but giving
 	only <message> does not trigger this action to prevent a misspelled
 	subcommand from making an unwanted stash.
 +
-If the paths argument is given in 'git stash push', only these files
-are put in the new 'stash'.  In addition only the indicated files are
-changed in the working tree to match the index.
+When pathspec is given to 'git stash push', the new stash records the
+modified states only for the files that match the pathspec.  The index
+entries and working tree files are then rolled back to the state in
+HEAD only for these files, too, leaving files that do not match the
+pathspec intact.
 +
 If the `--keep-index` option is used, all changes already added to the
 index are left intact.
diff --git a/git-stash.sh b/git-stash.sh
index 0072a38b4c..46367d40aa 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -41,7 +41,7 @@ no_changes () {
 untracked_files () {
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt -- $1
+	git ls-files -o -z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -59,13 +59,12 @@ create_stash () {
 	stash_msg=
 	untracked=
 	new_style=
-	files=
 	while test $# != 0
 	do
 		case "$1" in
 		-m|--message)
 			shift
-			stash_msg="$1"
+			stash_msg=${1?"-m needs an argument"}
 			new_style=t
 			;;
 		-u|--include-untracked)
@@ -75,7 +74,6 @@ create_stash () {
 			;;
 		--)
 			shift
-			files="$@"
 			new_style=t
 			break
 			;;
@@ -106,6 +104,10 @@ create_stash () {
 	if test -z "$new_style"
 	then
 		stash_msg="$*"
+		while test $# != 0
+		do
+			shift
+		done
 	fi
 
 	git update-index -q --refresh
@@ -141,7 +143,7 @@ create_stash () {
 		# Untracked files are stored by themselves in a parentless commit, for
 		# ease of unpacking later.
 		u_commit=$(
-			untracked_files $files | (
+			untracked_files "$@" | (
 				GIT_INDEX_FILE="$TMPindex" &&
 				export GIT_INDEX_FILE &&
 				rm -f "$TMPindex" &&
@@ -164,7 +166,7 @@ create_stash () {
 			git read-tree --index-output="$TMPindex" -m $i_tree &&
 			GIT_INDEX_FILE="$TMPindex" &&
 			export GIT_INDEX_FILE &&
-			git diff-index --name-only -z HEAD -- $files >"$TMP-stagenames" &&
+			git diff-index --name-only -z HEAD -- "$@" >"$TMP-stagenames" &&
 			git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
 			git write-tree &&
 			rm -f "$TMPindex"
@@ -178,7 +180,7 @@ create_stash () {
 
 		# find out what the user wants
 		GIT_INDEX_FILE="$TMP-index" \
-			git add--interactive --patch=stash -- $files &&
+			git add--interactive --patch=stash -- "$@" &&
 
 		# state of the working tree
 		w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
@@ -268,7 +270,7 @@ push_stash () {
 			;;
 		-m|--message)
 			shift
-			stash_msg=$1
+			stash_msg=${1?"-m needs an argument"}
 			;;
 		--help)
 			show_help
@@ -300,8 +302,6 @@ push_stash () {
 		shift
 	done
 
-	files="$*"
-
 	if test -n "$patch_mode" && test -n "$untracked"
 	then
 		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
@@ -316,14 +316,14 @@ push_stash () {
 	git reflog exists $ref_stash ||
 		clear_stash || die "$(gettext "Cannot initialize stash")"
 
-	create_stash -m "$stash_msg" -u "$untracked" -- $files
+	create_stash -m "$stash_msg" -u "$untracked" -- "$@"
 	store_stash -m "$stash_msg" -q $w_commit ||
 	die "$(gettext "Cannot save the current status")"
 	say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
 
 	if test -z "$patch_mode"
 	then
-		if test -n "$files"
+		if test $# != 0
 		then
 			git ls-files -z -- "$@" | xargs -0 git reset --
 			git ls-files -z --modified -- "$@" | xargs -0 git checkout HEAD --
@@ -334,7 +334,7 @@ push_stash () {
 		test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
 		if test -n "$untracked"
 		then
-			git clean --force --quiet -d $CLEAN_X_OPTION -- $files
+			git clean --force --quiet -d $CLEAN_X_OPTION -- "$@"
 		fi
 
 		if test "$keep_index" = "t" && test -n "$i_tree"
@@ -357,24 +357,6 @@ save_stash () {
 	while test $# != 0
 	do
 		case "$1" in
-		-k|--keep-index)
-			push_options="-k $push_options"
-			;;
-		--no-keep-index)
-			push_options="--no-keep-index $push_options"
-			;;
-		-p|--patch)
-			push_options="-p $push_options"
-			;;
-		-q|--quiet)
-			push_options="-q $push_options"
-			;;
-		-u|--include-untracked)
-			push_options="-u $push_options"
-			;;
-		-a|--all)
-			push_options="-a $push_options"
-			;;
 		--help)
 			show_help
 			;;
@@ -383,20 +365,8 @@ save_stash () {
 			break
 			;;
 		-*)
-			option="$1"
-			# TRANSLATORS: $option is an invalid option, like
-			# `--blah-blah'. The 7 spaces at the beginning of the
-			# second line correspond to "error: ". So you should line
-			# up the second line with however many characters the
-			# translation of "error: " takes in your language. E.g. in
-			# English this is:
-			#
-			#    $ git stash save --blah-blah 2>&1 | head -n 2
-			#    error: unknown option for 'stash save': --blah-blah
-			#           To provide a message, use git stash save -- '--blah-blah'
-			eval_gettextln "error: unknown option for 'stash save': \$option
-       To provide a message, use git stash save -- '\$option'"
-			usage
+			# pass all options through to push_stash
+			push_options="$push_options $1"
 			;;
 		*)
 			break
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ca4c44aa9c..461fe46045 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -784,7 +784,7 @@ test_expect_success 'push -m shows right message' '
 	test_cmp expect actual
 '
 
-test_expect_success 'deprecated version of stash create stores correct message' '
+test_expect_success 'create stores correct message' '
 	>foo &&
 	git add foo &&
 	STASH_ID=$(git stash create "create test message") &&
@@ -793,6 +793,15 @@ test_expect_success 'deprecated version of stash create stores correct message'
 	test_cmp expect actual
 '
 
+test_expect_success 'create with multiple arguments for the message' '
+	>foo &&
+	git add foo &&
+	STASH_ID=$(git stash create test untracked) &&
+	echo "On master: test untracked" >expect &&
+	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'new style stash create stores correct message' '
 	>foo &&
 	git add foo &&
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f372fc8ca8..9a98e31a3e 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -185,4 +185,30 @@ test_expect_success 'stash save --all is stash poppable' '
 	test -s .gitignore
 '
 
+test_expect_success 'stash push --include-untracked with pathspec' '
+	>foo &&
+	>bar &&
+	git stash push --include-untracked -- foo &&
+	test_path_is_file bar &&
+	test_path_is_missing foo &&
+	git stash pop &&
+	test_path_is_file bar &&
+	test_path_is_file foo
+'
+
+test_expect_success 'stash push with $IFS character' '
+	>"foo	bar" &&
+	>foo &&
+	>bar &&
+	git add foo* &&
+	git stash push --include-untracked -- foo* &&
+	test_path_is_missing "foo	bar" &&
+	test_path_is_missing foo &&
+	test_path_is_file bar &&
+	git stash pop &&
+	test_path_is_file "foo	bar" &&
+	test_path_is_file foo &&
+	test_path_is_file bar
+'
+
 test_done

Thomas Gummerer (5):
  Documentation/stash: remove mention of git reset --hard
  stash: introduce push verb
  stash: add test for the create command line arguments
  stash: introduce new format create
  stash: teach 'push' (and 'create') to honor pathspec

 Documentation/git-stash.txt        |  17 ++++-
 git-stash.sh                       | 124 +++++++++++++++++++++++++++++++++----
 t/t3903-stash.sh                   |  78 +++++++++++++++++++++++
 t/t3905-stash-include-untracked.sh |  26 ++++++++
 4 files changed, 230 insertions(+), 15 deletions(-)

-- 
2.12.0.rc0.208.g81c5d00b20.dirty


^ permalink raw reply related

* [PATCH v3 4/5] stash: introduce new format create
From: Thomas Gummerer @ 2017-02-05 20:26 UTC (permalink / raw)
  To: git
  Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
	Thomas Gummerer
In-Reply-To: <20170205202642.14216-1-t.gummerer@gmail.com>

git stash create currently supports a positional argument for adding a
message.  This is not quite in line with how git commands usually take
comments (using a -m flag).

Add a new syntax for adding a message to git stash create using a -m
flag.  This is with the goal of deprecating the old style git stash
create with positional arguments.

This also adds a -u argument, for untracked files.  This is already used
internally as another positional argument, but can now be used from the
command line.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt |  1 +
 git-stash.sh                | 50 +++++++++++++++++++++++++++++++++++++++++----
 t/t3903-stash.sh            |  9 ++++++++
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9e344cd7..a138ed6a24 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 	     [-u|--include-untracked] [-a|--all] [<message>]]
 'git stash' clear
 'git stash' create [<message>]
+'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
 'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
 
 DESCRIPTION
diff --git a/git-stash.sh b/git-stash.sh
index bf7863a846..33b2d0384c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -56,8 +56,50 @@ clear_stash () {
 }
 
 create_stash () {
-	stash_msg="$1"
-	untracked="$2"
+	stash_msg=
+	untracked=
+	new_style=
+	while test $# != 0
+	do
+		case "$1" in
+		-m|--message)
+			shift
+			stash_msg=${1?"-m needs an argument"}
+			new_style=t
+			;;
+		-u|--include-untracked)
+			shift
+			untracked="$1"
+			new_style=t
+			;;
+		*)
+			if test -n "$new_style"
+			then
+				echo "invalid argument"
+				option="$1"
+				# TRANSLATORS: $option is an invalid option, like
+				# `--blah-blah'. The 7 spaces at the beginning of the
+				# second line correspond to "error: ". So you should line
+				# up the second line with however many characters the
+				# translation of "error: " takes in your language. E.g. in
+				# English this is:
+				#
+				#    $ git stash save --blah-blah 2>&1 | head -n 2
+				#    error: unknown option for 'stash save': --blah-blah
+				#           To provide a message, use git stash save -- '--blah-blah'
+				eval_gettextln "error: unknown option for 'stash create': \$option"
+				usage
+			fi
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test -z "$new_style"
+	then
+		stash_msg="$*"
+	fi
 
 	git update-index -q --refresh
 	if no_changes
@@ -265,7 +307,7 @@ push_stash () {
 	git reflog exists $ref_stash ||
 		clear_stash || die "$(gettext "Cannot initialize stash")"
 
-	create_stash "$stash_msg" $untracked
+	create_stash -m "$stash_msg" -u "$untracked"
 	store_stash -m "$stash_msg" -q $w_commit ||
 	die "$(gettext "Cannot save the current status")"
 	say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
@@ -667,7 +709,7 @@ clear)
 	;;
 create)
 	shift
-	create_stash "$*" && echo "$w_commit"
+	create_stash "$@" && echo "$w_commit"
 	;;
 store)
 	shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 21cb70dc74..b859e51086 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,13 @@ test_expect_success 'create with multiple arguments for the message' '
 	test_cmp expect actual
 '
 
+test_expect_success 'new style stash create stores correct message' '
+	>foo &&
+	git add foo &&
+	STASH_ID=$(git stash create -m "create test message new style") &&
+	echo "On master: create test message new style" >expect &&
+	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.12.0.rc0.208.g81c5d00b20.dirty


^ permalink raw reply related

* [PATCH v3 2/5] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-05 20:26 UTC (permalink / raw)
  To: git
  Cc: Stephan Beyer, Junio C Hamano, Marc Strapetz, Jeff King,
	Johannes Schindelin, Øyvind A . Holm, Jakub Narębski,
	Thomas Gummerer
In-Reply-To: <20170205202642.14216-1-t.gummerer@gmail.com>

Introduce a new git stash push verb in addition to git stash save.  The
push verb is used to transition from the current command line arguments
to a more conventional way, in which the message is given as an argument
to the -m option.

This allows us to have pathspecs at the end of the command line
arguments like other Git commands do, so that the user can say which
subset of paths to stash (and leave others behind).

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 git-stash.sh     | 46 +++++++++++++++++++++++++++++++++++++++++++---
 t/t3903-stash.sh |  9 +++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 10c284d1aa..bf7863a846 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -189,10 +189,11 @@ store_stash () {
 	return $ret
 }
 
-save_stash () {
+push_stash () {
 	keep_index=
 	patch_mode=
 	untracked=
+	stash_msg=
 	while test $# != 0
 	do
 		case "$1" in
@@ -216,6 +217,10 @@ save_stash () {
 		-a|--all)
 			untracked=all
 			;;
+		-m|--message)
+			shift
+			stash_msg=${1?"-m needs an argument"}
+			;;
 		--help)
 			show_help
 			;;
@@ -251,8 +256,6 @@ save_stash () {
 		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
 	fi
 
-	stash_msg="$*"
-
 	git update-index -q --refresh
 	if no_changes
 	then
@@ -291,6 +294,39 @@ save_stash () {
 	fi
 }
 
+save_stash () {
+	push_options=
+	while test $# != 0
+	do
+		case "$1" in
+		--help)
+			show_help
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			# pass all options through to push_stash
+			push_options="$push_options $1"
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	stash_msg="$*"
+
+	if test -z "$stash_msg"
+	then
+		push_stash $push_options
+	else
+		push_stash $push_options -m "$stash_msg"
+	fi
+}
+
 have_stash () {
 	git rev-parse --verify --quiet $ref_stash >/dev/null
 }
@@ -617,6 +653,10 @@ save)
 	shift
 	save_stash "$@"
 	;;
+push)
+	shift
+	push_stash "$@"
+	;;
 apply)
 	shift
 	apply_stash "$@"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 2de3e18ce6..0171b824c9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -775,4 +775,13 @@ test_expect_success 'stash is not confused by partial renames' '
 	test_path_is_missing file
 '
 
+test_expect_success 'push -m shows right message' '
+	>foo &&
+	git add foo &&
+	git stash push -m "test message" &&
+	echo "stash@{0}: On master: test message" >expect &&
+	git stash list | head -n 1 >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.12.0.rc0.208.g81c5d00b20.dirty


^ permalink raw reply related

* [BUG] was: Re: [PATCH] Remove --no-gui option from difftool usage string
From: David Aguilar @ 2017-02-05 20:17 UTC (permalink / raw)
  To: Denton Liu, Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <20170204025617.GA9221@arch-attack.localdomain>


On Fri, Feb 03, 2017 at 06:56:17PM -0800, Denton Liu wrote:
> The --no-gui option not documented in the manpage, nor is it actually
> used in the source code. This change removes it from the usage help
> that's printed.
> 
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  git-difftool.perl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

[Dscho, I found a bug, see below]

I forgot to mention that the scripted difftool has been
rewritten in C and will be going away soon.
builtin/difftool.c is already in "next".

New patches against difftool should target the builtin.

Regarding removing it from the usage string, IMO that can be
considered a good change if the rationale were instead that
we never expect users to ever type "--no-gui" in practice.

Wasting the short usage string screen space with a useless to
99.99% users option is bad for usability.  From that perspective
we shouldn't mention it there, so reframing the commit message
towards that argument would make for a better motivation.

Removing the mention from the usage string and adding it to the
manpage would be the a good change from that perspective as well.


BTW, in "next", it seems that the builtin difftool crashes when
doing "git difftool -h", so we should probably add a test
for that too...

From the tip of next:

$ git difftool -h
fatal: BUG: setup_git_env called without repository
-- 
David

^ permalink raw reply

* Re: git-scm.com status report
From: Pranit Bauva @ 2017-02-05 20:11 UTC (permalink / raw)
  To: Jeff King; +Cc: Git List
In-Reply-To: <20170202023349.7fopb3a6pc6dkcmd@sigill.intra.peff.net>

Hey Peff,

On Thu, Feb 2, 2017 at 8:03 AM, Jeff King <peff@peff.net> wrote:
> ## What's on the site
>
> We have the domains git-scm.com and git-scm.org (the latter we've had
> for a while). They both point to the same website, which has general
> information about Git, including:

Since we have an "official" control over the website, shouldn't we be
using the .org domain more because we are more of an organization?
What I mean is that in many places, we have referred to git-scm.com,
which was perfectly fine because it was done by github which is a
company but now I think it would be more appropriate to use
git-scm.org domain. We can forward all .com requests to .org and try
to move all reference we know about, to .org. What do you all think?

Regards,
Pranit Bauva

^ permalink raw reply

* Re: Git clonebundles
From: Christian Couder @ 2017-02-05 16:37 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Stefan Saasen, Git Mailing List
In-Reply-To: <CAJo=hJsS6FmL9iNScaXqkWJumALfGr8Od5QkbfZ+ZG3osxkp7Q@mail.gmail.com>

On Sat, Feb 4, 2017 at 6:39 PM, Shawn Pearce <spearce@spearce.org> wrote:
> On Mon, Jan 30, 2017 at 11:00 PM, Stefan Saasen <ssaasen@atlassian.com> wrote:
>>
>> Bitbucket recently added support for Mercurial’s clonebundle extension
>> (http://gregoryszorc.com/blog/2015/10/22/cloning-improvements-in-mercurial-3.6/).
>> Mercurial’s clone bundles allow the Mercurial client to seed a repository using
>> a bundle file instead of dynamically generating a bundle for the client.
> ...
>> Prior art
>> ~~~~~~~~~
>>
>> Our proof-of-concept is built on top of ideas that have been
>> circulating for a while. We are aware of a number of proposed changes
>> in this space:
>>
>>
>> * Jeff King's work on network bundles:
>> https://github.com/peff/git/commit/17e2409df37edd0c49ef7d35f47a7695f9608900
>> * Nguyễn Thái Ngọc Duy's work on "[PATCH 0/8] Resumable clone
>> revisited, proof of concept":
>> https://www.spinics.net/lists/git/msg267260.html
>> * Resumable clone work by Kevin Wern:
>> https://public-inbox.org/git/1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com/
>
> I think you missed the most common deployment of prior art, which is
> Android using the git-repo tool[1]. The git-repo tool has had
> clone.bundle support since Sep 2011[2] and the Android Git servers
> have been answering /clone.bundle requests[3] since just before that.
> The bundle files are generated with `git bundle create` on a regular
> schedule by cron.
>
> [1] https://gerrit.googlesource.com/git-repo/+/04071c1c72437a930db017bd4c562ad06087986a/project.py#2091
> [2] https://gerrit.googlesource.com/git-repo/+/f322b9abb4cadc67b991baf6ba1b9f2fbd5d7812
> [3] https://android.googlesource.com/platform/frameworks/base/clone.bundle

There is also Junio's work on Bundle v3 that was unfortunately
recently discarded.
Look for "jc/bundle" in:

http://public-inbox.org/git/xmqq4m0cry60.fsf@gitster.mtv.corp.google.com/

and previous "What's cooking in git.git" emails.

I am also working on adding external object database support using
previous work by Peff:

http://public-inbox.org/git/20161130210420.15982-1-chriscool@tuxfamily.org/

that could be extended to support clone bundles.

[...]

^ permalink raw reply

* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Pranit Bauva @ 2017-02-05 14:55 UTC (permalink / raw)
  To: Siddharth Kannan
  Cc: Git List, Matthieu.Moy, Jeff King, Junio C Hamano, Duy Nguyen,
	Brian M. Carlson
In-Reply-To: <1486299439-2859-1-git-send-email-kannan.siddharth12@gmail.com>

Hey Siddharth,

On Sun, Feb 5, 2017 at 6:27 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> Search and replace "-" (written in the context of a branch name) in the argument
> list with "@{-1}". As per the help text of git rev-list, this includes the following four
> cases:
>
>   a. "-"
>   b. "^-"
>   c. "-..other-branch-name" or "other-branch-name..-"
>   d. "-...other-branch-name" or "other-branch-name...-"
>
> (a) and (b) have been implemented as in the previous implementations of
> this abbreviation. Namely, 696acf45 (checkout: implement "-" abbreviation, add
> docs and tests, 2009-01-17), 182d7dc4 (cherry-pick: allow "-" as
> abbreviation of '@{-1}', 2013-09-05) and 4e8115ff (merge: allow "-" as a
> short-hand for "previous branch", 2011-04-07)
>
> (c) and (d) have been implemented by using the strbuf API, growing it to the
> right size and placing "@{-1}" instead of "-"
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---
> This is a patch for one of the microprojects of SoC 2017. [1]
>
> I have implemented this using multiple methods, that I have re-written again and
> again for better versions ([2]). The present version I feel is the closest that
> I could get to the existing code in the repository. This patch only uses
> functions that are commonly used in the rest of the codebase.
>
> I still have to write tests, as well as update documentation as done in 696acf45
> (checkout: implement "-" abbreviation, add docs and tests, 2009-01-17).
>
> I request your comments on this patch. Also, I have the following questions
> regarding this patch:
>
> 1. Is the approach that I have used to solve this problem fine?
> 2. Is the code I am writing in the right function? (I have put it right
> before the revisions data structure is setup, thus these changes affect only
> git-log)
>
> [1]: https://git.github.io/SoC-2017-Microprojects/
> [2]: https://github.com/git/git/compare/6e3a7b3...icyflame:7e286c9.patch (Uses
> strbufs for the starting 4 characters, and last 4 characters and compares those
> to the appropriate strings for case (c) and case (d). I edited this patch to use
> strstr instead, which avoids all the strbuf declarations)
>
>  builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 55d20cc..a5aac99 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -132,7 +132,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>                          struct rev_info *rev, struct setup_revision_opt *opt)
>  {
>         struct userformat_want w;
> -       int quiet = 0, source = 0, mailmap = 0;
> +       int quiet = 0, source = 0, mailmap = 0, i = 0;
>         static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
>
>         const struct option builtin_log_options[] = {
> @@ -158,6 +158,51 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>
>         if (quiet)
>                 rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> +
> +       /*
> +        * Check if any argument has a "-" in it, which has been referred to as a
> +        * shorthand for @{-1}.  Handles methods that might be used to list commits
> +        * as mentioned in git rev-list --help
> +        */
> +
> +       for(i = 0; i < argc; ++i) {
> +               if (!strcmp(argv[i], "-")) {
> +                       argv[i] = "@{-1}";
> +               } else if (!strcmp(argv[i], "^-")) {
> +                       argv[i] = "^@{-1}";
> +               } else if (strlen(argv[i]) >= 4) {
> +
> +                       if (strstr(argv[i], "-...") == argv[i] || strstr(argv[i], "-..") == argv[i]) {
> +                               struct strbuf changed_argument = STRBUF_INIT;
> +
> +                               strbuf_addstr(&changed_argument, "@{-1}");
> +                               strbuf_addstr(&changed_argument, argv[i] + 1);
> +
> +                               strbuf_setlen(&changed_argument, strlen(argv[i]) + 4);
> +
> +                               argv[i] = strbuf_detach(&changed_argument, NULL);
> +                       }
> +
> +                       /*
> +                        * Find the first occurence, and add the size to it and proceed if
> +                        * the resulting value is NULL
> +                        */
> +                       if (!(strstr(argv[i], "...-") + 4)  ||
> +                                       !(strstr(argv[i], "..-") + 3)) {
> +                               struct strbuf changed_argument = STRBUF_INIT;
> +
> +                               strbuf_addstr(&changed_argument, argv[i]);
> +
> +                               strbuf_grow(&changed_argument, strlen(argv[i]) + 4);
> +                               strbuf_setlen(&changed_argument, strlen(argv[i]) + 4);
> +
> +                               strbuf_splice(&changed_argument, strlen(argv[i]) - 1, 5, "@{-1}", 5);
> +
> +                               argv[i] = strbuf_detach(&changed_argument, NULL);
> +                       }
> +               }
> +       }
> +
>         argc = setup_revisions(argc, argv, rev, opt);
>
>         /* Any arguments at this point are not recognized */
> --


It is highly recommended to follow the pre existing style of code and
commits. In the micro project list, I think it is mentioned that this
similar thing is implemented in git-merge so you should try and dig
the commit history of that file to find the similar change.

If you do this, then you will find out that there is a very short and
sweet way to do it. I won't directly point out the commit.

strbuf API should be used when you need to modify the contents of the
string. I think you have a little confusion.

If you declare the string as,

const char *str = "foo";

then, you can also do,

str = "bar";

But you can't do,

str[1] = 'z';

I hope you get what I am saying, if not, search for it.

Regards,
Pranit Bauva

^ permalink raw reply

* Re: [RFC] Add support for downloading blobs on demand
From: Christian Couder @ 2017-02-05 14:03 UTC (permalink / raw)
  To: Ben Peart; +Cc: Jeff King, git, Johannes Schindelin
In-Reply-To: <002601d2710b$c3715890$4a5409b0$@gmail.com>

(Sorry for the late reply and thanks to Dscho for pointing me to this thread.)

On Tue, Jan 17, 2017 at 10:50 PM, Ben Peart <peartben@gmail.com> wrote:
>> From: Jeff King [mailto:peff@peff.net]
>> On Fri, Jan 13, 2017 at 10:52:53AM -0500, Ben Peart wrote:
>>
>> > Clone and fetch will pass a  --lazy-clone  flag (open to a better name
>> > here) similar to  --depth  that instructs the server to only return
>> > commits and trees and to ignore blobs.
>> >
>> > Later during git operations like checkout, when a blob cannot be found
>> > after checking all the regular places (loose, pack, alternates, etc),
>> > git will download the missing object and place it into the local
>> > object store (currently as a loose object) then resume the operation.
>>
>> Have you looked at the "external odb" patches I wrote a while ago, and
>> which Christian has been trying to resurrect?
>>
>>   https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpublic-
>> inbox.org%2Fgit%2F20161130210420.15982-1-
>> chriscool%40tuxfamily.org%2F&data=02%7C01%7CBen.Peart%40microsoft.c
>> om%7C9596d3bf32564f123e0c08d43f08a9e1%7C72f988bf86f141af91ab2d7c
>> d011db47%7C1%7C0%7C636202753822020527&sdata=a6%2BGOAQoRhjFoxS
>> vftY8JZAVUssmrXuDZ9OBy3xqNZk%3D&reserved=0
>>
>> This is a similar approach, though I pushed the policy for "how do you get the
>> objects" out into an external script. One advantage there is that large objects
>> could easily be fetched from another source entirely (e.g., S3 or equivalent)
>> rather than the repo itself.
>>
>> The downside is that it makes things more complicated, because a push or a
>> fetch now involves three parties (server, client, and the alternate object
>> store). So questions like "do I have all the objects I need" are hard to reason
>> about.
>>
>> If you assume that there's going to be _some_ central Git repo which has all
>> of the objects, you might as well fetch from there (and do it over normal git
>> protocols). And that simplifies things a bit, at the cost of being less flexible.
>
> We looked quite a bit at the external odb patches, as well as lfs and
> even using alternates.  They all share a common downside that you must
> maintain a separate service that contains _some_ of the files.

Pushing the policy for "how do you get the objects" out into an
external helper doesn't mean that the external helper cannot use the
main service.
The external helper is still free to do whatever it wants including
calling the main service if it thinks it's better.

> These
> files must also be versioned, replicated, backed up and the service
> itself scaled out to handle the load.  As you mentioned, having multiple
> services involved increases flexability but it also increases the
> complexity and decreases the reliability of the overall version control
> service.

About reliability, I think it depends a lot on the use case. If you
want to get very big files over an unreliable connection, it can
better if you send those big files over a restartable protocol and
service like HTTP/S on a regular web server.

> For operational simplicity, we opted to go with a design that uses a
> single, central git repo which has _all_ the objects and to focus on
> enhancing it to handle large numbers of files efficiently.  This allows
> us to focus our efforts on a great git service and to avoid having to
> build out these other services.

Ok, but I don't think it prevents you from using at least some of the
same mechanisms that the external odb series is using.
And reducing the number of mechanisms in Git itself is great for its
maintainability and simplicity.

>> > To prevent git from accidentally downloading all missing blobs, some
>> > git operations are updated to be aware of the potential for missing blobs.
>> > The most obvious being check_connected which will return success as if
>> > everything in the requested commits is available locally.
>>
>> Actually, Git is pretty good about trying not to access blobs when it doesn't
>> need to. The important thing is that you know enough about the blobs to
>> fulfill has_sha1_file() and sha1_object_info() requests without actually
>> fetching the data.
>>
>> So the client definitely needs to have some list of which objects exist, and
>> which it _could_ get if it needed to.

Yeah, and the external odb series handles that already, thanks to
Peff's initial work.

>> The one place you'd probably want to tweak things is in the diff code, as a
>> single "git log -Sfoo" would fault in all of the blobs.
>
> It is an interesting idea to explore how we could be smarter about
> preventing blobs from faulting in if we had enough info to fulfill
> has_sha1_file() and sha1_object_info().  Given we also heavily prune the
> working directory using sparse-checkout, this hasn't been our top focus
> but it is certainly something worth looking into.

The external odb series doesn't handle preventing blobs from faulting
in yet, so this could be a common problem.

[...]

>> One big hurdle to this approach, no matter the protocol, is how you are
>> going to handle deltas. Right now, a git client tells the server "I have this
>> commit, but I want this other one". And the server knows which objects the
>> client has from the first, and which it needs from the second. Moreover, it
>> knows that it can send objects in delta form directly from disk if the other
>> side has the delta base.
>>
>> So what happens in this system? We know we don't need to send any blobs
>> in a regular fetch, because the whole idea is that we only send blobs on
>> demand. So we wait for the client to ask us for blob A. But then what do we
>> send? If we send the whole blob without deltas, we're going to waste a lot of
>> bandwidth.
>>
>> The on-disk size of all of the blobs in linux.git is ~500MB. The actual data size
>> is ~48GB. Some of that is from zlib, which you get even for non-deltas. But
>> the rest of it is from the delta compression. I don't think it's feasible to give
>> that up, at least not for "normal" source repos like linux.git (more on that in
>> a minute).
>>
>> So ideally you do want to send deltas. But how do you know which objects
>> the other side already has, which you can use as a delta base? Sending the
>> list of "here are the blobs I have" doesn't scale. Just the sha1s start to add
>> up, especially when you are doing incremental fetches.

To initialize some paths that the client wants, it could perhaps just
ask for some pack files, or maybe bundle files, related to these
paths.
Those packs or bundles could be downloaded either directly from the
main server or from other web or proxy servers.

>> I think this sort of things performs a lot better when you just focus on large
>> objects. Because they don't tend to delta well anyway, and the savings are
>> much bigger by avoiding ones you don't want. So a directive like "don't
>> bother sending blobs larger than 1MB" avoids a lot of these issues. In other
>> words, you have some quick shorthand to communicate between the client
>> and server: this what I have, and what I don't.
>> Normal git relies on commit reachability for that, but there are obviously
>> other dimensions. The key thing is that both sides be able to express the
>> filters succinctly, and apply them efficiently.
>
> Our challenge has been more the sheer _number_ of files that exist in
> the repo rather than the _size_ of the files in the repo.  With >3M
> source files and any typical developer only needing a small percentage
> of those files to do their job, our focus has been pruning the tree as
> much as possible such that they only pay the cost for the files they
> actually need.  With typical text source files being 10K - 20K in size,
> the overhead of the round trip is a significant part of the overall
> transfer time so deltas don't help as much.  I agree that large files
> are also a problem but it isn't my top focus at this point in time.

Ok, but it would be nice if both problems could be solved using some
common mechanisms.
This way it could probably work better in situations where there are
both a large number of files _and_ some big files.
And from what I am seeing, there could be no real downside from using
some common mechanisms.

>> If most of your benefits are not from avoiding blobs in general, but rather
>> just from sparsely populating the tree, then it sounds like sparse clone might
>> be an easier path forward. The general idea is to restrict not just the
>> checkout, but the actual object transfer and reachability (in the tree
>> dimension, the way shallow clone limits it in the time dimension, which will
>> require cooperation between the client and server).
>>
>> So that's another dimension of filtering, which should be expressed pretty
>> succinctly: "I'm interested in these paths, and not these other ones." It's
>> pretty easy to compute on the server side during graph traversal (though it
>> interacts badly with reachability bitmaps, so there would need to be some
>> hacks there).
>>
>> It's an idea that's been talked about many times, but I don't recall that there
>> were ever working patches. You might dig around in the list archive under
>> the name "sparse clone" or possibly "narrow clone".
>
> While a sparse/narrow clone would work with this proposal, it isn't
> required.  You'd still probably want all the commits and trees but the
> clone would also bring down the specified blobs.  Combined with using
> "depth" you could further limit it to those blobs at tip.
>
> We did run into problems with this model however as our usage patterns
> are such that our working directories often contain very sparse trees
> and as a result, we can end up with thousands of entries in the sparse
> checkout file.  This makes it difficult for users to manually specify a
> sparse-checkout before they even do a clone.  We have implemented a
> hashmap based sparse-checkout to deal with the performance issues of
> having that many entries but that's a different RFC/PATCH.  In short, we
> found that a "lazy-clone" and downloading blobs on demand provided a
> better developer experience.

I think both ways are possible using the external odb mechanism.

>> > Future Work
>> > ~~~~~~~~~~~
>> >
>> > The current prototype calls a new hook proc in
>> > sha1_object_info_extended and read_object, to download each missing
>> > blob.  A better solution would be to implement this via a long running
>> > process that is spawned on the first download and listens for requests
>> > to download additional objects until it terminates when the parent git
>> > operation exits (similar to the recent long running smudge and clean filter
>> work).
>>
>> Yeah, see the external-odb discussion. Those prototypes use a process per
>> object, but I think we all agree after seeing how the git-lfs interface has
>> scaled that this is a non-starter. Recent versions of git-lfs do the single-
>> process thing, and I think any sort of external-odb hook should be modeled
>> on that protocol.

I agree that the git-lfs scaling work is great, but I think it's not
necessary in the external odb work to have the same kind of
single-process protocol from the beginning (though it should be
possible and easy to add it).
For example if the external odb work can be used or extended to handle
restartable clone by downloading a single bundle when cloning, this
would not need that kind of protocol.

> I'm looking into this now and plan to re-implement it this way before
> sending out the first patch series.  Glad to hear you think it is a good
> protocol to model it on.

Yeah, for your use case on Windows, it looks really worth it to use
this kind of protocol.

>> > Need to investigate an alternate batching scheme where we can make a
>> > single request for a set of "related" blobs and receive single a
>> > packfile (especially during checkout).
>>
>> I think this sort of batching is going to be the really hard part to retrofit onto
>> git. Because you're throwing out the procedural notion that you can loop
>> over a set of objects and ask for each individually.
>> You have to start deferring computation until answers are ready. Some
>> operations can do that reasonably well (e.g., checkout), but something like
>> "git log -p" is constantly digging down into history. I suppose you could just
>> perform the skeleton of the operation _twice_, once to find the list of objects
>> to fault in, and the second time to actually do it.

In my opinion, perhaps we can just prevent "git log -p" from faulting
in blobs and have it show a warning saying that it was performed only
on a subset of all the blobs.

[...]

^ permalink raw reply

* [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Siddharth Kannan @ 2017-02-05 12:57 UTC (permalink / raw)
  To: git
  Cc: pranit.bauva, Matthieu.Moy, peff, gitster, pclouds, sandals,
	Siddharth Kannan

Search and replace "-" (written in the context of a branch name) in the argument
list with "@{-1}". As per the help text of git rev-list, this includes the following four
cases:

  a. "-"
  b. "^-"
  c. "-..other-branch-name" or "other-branch-name..-"
  d. "-...other-branch-name" or "other-branch-name...-"

(a) and (b) have been implemented as in the previous implementations of
this abbreviation. Namely, 696acf45 (checkout: implement "-" abbreviation, add
docs and tests, 2009-01-17), 182d7dc4 (cherry-pick: allow "-" as
abbreviation of '@{-1}', 2013-09-05) and 4e8115ff (merge: allow "-" as a
short-hand for "previous branch", 2011-04-07)

(c) and (d) have been implemented by using the strbuf API, growing it to the
right size and placing "@{-1}" instead of "-"

Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
---
This is a patch for one of the microprojects of SoC 2017. [1]

I have implemented this using multiple methods, that I have re-written again and
again for better versions ([2]). The present version I feel is the closest that
I could get to the existing code in the repository. This patch only uses
functions that are commonly used in the rest of the codebase.

I still have to write tests, as well as update documentation as done in 696acf45
(checkout: implement "-" abbreviation, add docs and tests, 2009-01-17).

I request your comments on this patch. Also, I have the following questions
regarding this patch:

1. Is the approach that I have used to solve this problem fine?
2. Is the code I am writing in the right function? (I have put it right
before the revisions data structure is setup, thus these changes affect only
git-log)

[1]: https://git.github.io/SoC-2017-Microprojects/
[2]: https://github.com/git/git/compare/6e3a7b3...icyflame:7e286c9.patch (Uses
strbufs for the starting 4 characters, and last 4 characters and compares those
to the appropriate strings for case (c) and case (d). I edited this patch to use
strstr instead, which avoids all the strbuf declarations)

 builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc..a5aac99 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -132,7 +132,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
 	struct userformat_want w;
-	int quiet = 0, source = 0, mailmap = 0;
+	int quiet = 0, source = 0, mailmap = 0, i = 0;
 	static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};

 	const struct option builtin_log_options[] = {
@@ -158,6 +158,51 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,

 	if (quiet)
 		rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
+
+	/*
+	 * Check if any argument has a "-" in it, which has been referred to as a
+	 * shorthand for @{-1}.  Handles methods that might be used to list commits
+	 * as mentioned in git rev-list --help
+	 */
+
+	for(i = 0; i < argc; ++i) {
+		if (!strcmp(argv[i], "-")) {
+			argv[i] = "@{-1}";
+		} else if (!strcmp(argv[i], "^-")) {
+			argv[i] = "^@{-1}";
+		} else if (strlen(argv[i]) >= 4) {
+
+			if (strstr(argv[i], "-...") == argv[i] || strstr(argv[i], "-..") == argv[i]) {
+				struct strbuf changed_argument = STRBUF_INIT;
+
+				strbuf_addstr(&changed_argument, "@{-1}");
+				strbuf_addstr(&changed_argument, argv[i] + 1);
+
+				strbuf_setlen(&changed_argument, strlen(argv[i]) + 4);
+
+				argv[i] = strbuf_detach(&changed_argument, NULL);
+			}
+
+			/*
+			 * Find the first occurence, and add the size to it and proceed if
+			 * the resulting value is NULL
+			 */
+			if (!(strstr(argv[i], "...-") + 4)  ||
+					!(strstr(argv[i], "..-") + 3)) {
+				struct strbuf changed_argument = STRBUF_INIT;
+
+				strbuf_addstr(&changed_argument, argv[i]);
+
+				strbuf_grow(&changed_argument, strlen(argv[i]) + 4);
+				strbuf_setlen(&changed_argument, strlen(argv[i]) + 4);
+
+				strbuf_splice(&changed_argument, strlen(argv[i]) - 1, 5, "@{-1}", 5);
+
+				argv[i] = strbuf_detach(&changed_argument, NULL);
+			}
+		}
+	}
+
 	argc = setup_revisions(argc, argv, rev, opt);

 	/* Any arguments at this point are not recognized */
--
2.1.4


^ permalink raw reply related

* Re: [PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-02-05 12:13 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A. Holm, Jakub Narębski
In-Reply-To: <xmqq7f5cuu8l.fsf@gitster.mtv.corp.google.com>

On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Don't mention git reset --hard in the documentation for git stash save.
> > It's an implementation detail that doesn't matter to the end user and
> > thus shouldn't be exposed to them.
> 
> Everybody understands what "reset --hard" does; it can be an
> easier-to-read "short-hand" description to say "reset --hard"
> instead of giving a lengthy description of what happens.

While this is definitely true for experienced git users, it might not
be for some people relatively new to git, which are probably the ones
that need the description most.

> Because of that, I do not necessarily agree with the above
> justification.  I'll read the remainder of the series before
> commenting further on the above.
> 
> > diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> > index 2e9cef06e6..0fc23c25ee 100644
> > --- a/Documentation/git-stash.txt
> > +++ b/Documentation/git-stash.txt
> > @@ -47,8 +47,9 @@ OPTIONS
> >  
> >  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
> >  
> > -	Save your local modifications to a new 'stash', and run `git reset
> > -	--hard` to revert them.  The <message> part is optional and gives
> > +	Save your local modifications to a new 'stash' and roll them
> > +	back both in the working tree and in the index.
> 
> "... roll them back both ..." is unclear where they are rolled back
> to.  
> 
> Perhaps "roll them back ... to HEAD" or something?

Yeah that makes sense, thanks.

-- 
Thomas

^ permalink raw reply

* [PATCH] p5302: create repositories for index-pack results explicitly
From: René Scharfe @ 2017-02-05 11:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git List

Before 7176a314 (index-pack: complain when --stdin is used outside of a
repo) index-pack silently created a non-existing target directory; now
the command refuses to work unless it's used against a valid repository.
That causes p5302 to fail, which relies on the former behavior.  Fix it
by setting up the destinations for its performance tests using git init.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 t/perf/p5302-pack-index.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh
index 5ee9211f98..99bdb16c85 100755
--- a/t/perf/p5302-pack-index.sh
+++ b/t/perf/p5302-pack-index.sh
@@ -13,6 +13,13 @@ test_expect_success 'repack' '
 	export PACK
 '
 
+test_expect_success 'create target repositories' '
+	for repo in t1 t2 t3 t4 t5 t6
+	do
+		git init --bare $repo
+	done
+'
+
 test_perf 'index-pack 0 threads' '
 	GIT_DIR=t1 git index-pack --threads=1 --stdin < $PACK
 '
-- 
2.11.1


^ permalink raw reply related

* Re: [PATCH v2 4/4] stash: support filename argument
From: Thomas Gummerer @ 2017-02-05 11:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A. Holm, Jakub Narębski
In-Reply-To: <xmqqa8a8uuc9.fsf@gitster.mtv.corp.google.com>

On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Add an optional filename argument to git stash push, which allows for
> > stashing a single (or multiple) files.
> 
> You can give pathspec with one or more elements, so "an optional
> argument" sounds too limiting.  
> 
>     Allow 'git stash push' to take pathspec to specify which paths
>     to stash.
> 
> Also retitle
> 
> 	stash: teach 'push' (and 'create') to honor pathspec
> 
> or something.
> 
> > @@ -56,6 +61,10 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
> >  	only <message> does not trigger this action to prevent a misspelled
> >  	subcommand from making an unwanted stash.
> >  +
> > +If the paths argument is given in 'git stash push', only these files
> > +are put in the new 'stash'.  In addition only the indicated files are
> > +changed in the working tree to match the index.
> 
> Actually the stash contains "all paths".  You could say that you are
> placing _modifications_ to these paths in stash, even though that is
> not how Git's world model works (i.e. everything is a snapshot, and
> modifications are merely difference between two successive
> snapshots).  A technically correct version may be:
> 
> 	When pathspec is given to 'git stash push', the new stash
> 	records the modified states only for the files that match
> 	the pathspec.  The index entries and working tree files are
> 	then rolled back to the state in HEAD only for these files,
> 	too, leaving files that do not match the pathspec intact.
> 
> > diff --git a/git-stash.sh b/git-stash.sh
> > index 5f08b43967..0072a38b4c 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -41,7 +41,7 @@ no_changes () {
> >  untracked_files () {
> >  	excl_opt=--exclude-standard
> >  	test "$untracked" = "all" && excl_opt=
> > -	git ls-files -o -z $excl_opt
> > +	git ls-files -o -z $excl_opt -- $1
> 
> Hmph, why "$1" is spelled without dq, implying that it is split at
> $IFS boundary?  This line alone makes me suspect that this is not
> prepared to deal correctly with $IFS.  Let's read on...
> 
> > @@ -59,6 +59,7 @@ create_stash () {
> >  	stash_msg=
> >  	untracked=
> >  	new_style=
> > +	files=
> >  	while test $# != 0
> >  	do
> >  		case "$1" in
> > @@ -72,6 +73,12 @@ create_stash () {
> >  			untracked="$1"
> >  			new_style=t
> >  			;;
> > +		--)
> > +			shift
> > +			files="$@"
> 
> Isn't this the same as writing files="$*", i.e. concatenate the
> multiple arguments with the first whitespace in $IFS in between?
> 
> > @@ -134,7 +141,7 @@ create_stash () {
> >  		# Untracked files are stored by themselves in a parentless commit, for
> >  		# ease of unpacking later.
> >  		u_commit=$(
> > -			untracked_files | (
> > +			untracked_files $files | (
> 
> ... and this lets it split at $IFS again when passing it down to the
> helper.  But the helper looks only at $1 so the second and subsequent
> ones will be ignored altogether.
> 
> This cannot be correct, and any hunk in the remainder of the patch
> that mentions $files will be incorrect for the same reason.
> 
> Is it possible to carry what the caller (and the end user) gave you
> in "$@" without molesting it at all?  That would mean you do not
> need to introduce $files variable at all, and then the places that
> do things like this:
> 
> > -	create_stash -m "$stash_msg" -u "$untracked"
> > +	create_stash -m "$stash_msg" -u "$untracked" -- $files
> 
> can instead do
> 
> 	create_stash -m "$stash_msg" -u "$untracked" -- "$@"
> 
> That would allow you to work correctly with pathspec with $IFS
> whitespaces in them.

Thanks for taking the time for this explanation!  It cleared quite a
few things up in my head.  I'll make these fixes in the re-roll.

-- 
Thomas

^ permalink raw reply

* Re: [PATCH/TOY] Shortcuts to quickly refer to a commit name with keyboard
From: Duy Nguyen @ 2017-02-05 10:39 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Jeff King, Git mailing list
In-Reply-To: <CA+P7+xoMTX5n_h+5MytZwVqKjqa0wdNKCeDtH29A_+WSfr6gTQ@mail.gmail.com>

On Sat, Jan 21, 2017 at 2:16 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> I would be interested in the code for this.. I'm curious if I can
> adapt it to my use of tmux.

I stumbled upon this which does mention about git SHA-1. Maybe you'll
find it useful. Haven't tried it out though.

https://github.com/morantron/tmux-fingers

-- 
Duy

^ permalink raw reply

* Re: [PATCH] Remove --no-gui option from difftool usage string
From: David Aguilar @ 2017-02-05 10:22 UTC (permalink / raw)
  To: Denton Liu; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <20170204062351.GA11277@arch-attack.localdomain>

On Fri, Feb 03, 2017 at 10:23:51PM -0800, Denton Liu wrote:
> On Fri, Feb 03, 2017 at 09:58:09PM -0800, Jacob Keller wrote:
> > On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.denton@gmail.com> wrote:
> > > The --no-gui option not documented in the manpage, nor is it actually
> > > used in the source code. This change removes it from the usage help
> > > that's printed.
> > >
> > > Signed-off-by: Denton Liu <liu.denton@gmail.com>
> > > ---
> > >  git-difftool.perl | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/git-difftool.perl b/git-difftool.perl
> > > index a5790d03a..657d5622d 100755
> > > --- a/git-difftool.perl
> > > +++ b/git-difftool.perl
> > > @@ -29,8 +29,8 @@ sub usage
> > >         print << 'USAGE';
> > >  usage: git difftool [-t|--tool=<tool>] [--tool-help]
> > >                      [-x|--extcmd=<cmd>]
> > > -                    [-g|--gui] [--no-gui]
> > > -                    [--prompt] [-y|--no-prompt]
> > > +                    [-g|--gui] [--prompt]
> > > +                    [-y|--no-prompt]
> > >                      [-d|--dir-diff]
> > >                      ['git diff' options]
> > >  USAGE
> > > --
> > > 2.11.0
> > >
> > 
> > Aren't "--no-foo" options automatically created for booleans when
> > using our option parsing code?
> > 
> > Thanks,
> > Jake
> 
> Sorry, I guess I didn't notice that. Would it make sense to document it
> in the manpage then?

The general "--no-*" convention is mentioned in "git help cli",
but the manpages and usage strings across all commands are
inconsistent about mentioning the "--no-*" variants; some do,
some don't.

IMO it probably wouldn't hurt to document --no-gui in the manpage.

cheers,
-- 
David

^ permalink raw reply

* Re: [PATCH] Add --gui option to mergetool
From: David Aguilar @ 2017-02-05 10:18 UTC (permalink / raw)
  To: Denton Liu; +Cc: git
In-Reply-To: <20170204064303.GA14990@arch-attack.localdomain>

On Fri, Feb 03, 2017 at 10:43:03PM -0800, Denton Liu wrote:
> * fix the discrepancy between difftool and mergetool where
>   the former has the --gui flag and the latter does not by adding the
>   functionality to mergetool

Please avoid bullet points in commit messages when a simple
paragraph will suffice.

The implementation of this feature seems ok, but tests are
needed in t/t7610-mergetool.sh.

> * make difftool read 'merge.guitool' as a fallback, in accordance to the
>   manpage for difftool: "git difftool falls back to git mergetool
>   config variables when the difftool equivalents have not been defined"

I did not spot this change in the code.

Nonetheless, this should be split off as a separate patch, and
tests should be added.

> * add guitool-related completions

This should be split off as a separate patch as well.

Generally, 3 bullet points suggests there should be 3 patches in
this series.

Thanks,
-- 
David

^ permalink raw reply

* Re: release notes/ change number discrepancy ? - Documentation/RelNotes/2.10.0.txt "merge b738396..."
From: Junio C Hamano @ 2017-02-05  7:44 UTC (permalink / raw)
  To: Zenaan Harkness; +Cc: git
In-Reply-To: <20170205022156.GA19612@x220-a02>

Zenaan Harkness <zen@freedbms.net> writes:

> Am I missing something in the following:
>
> looking at Documentation/RelNotes/2.10.0.txt I see the following release
> note (~line 35):
>
>  * "upload-pack" allows a custom "git pack-objects" replacement when
>    responding to "fetch/clone" via the uploadpack.packObjectsHook.
>    (merge b738396 jk/upload-pack-hook later to maint).
>
>
> but when I run git show b738396 , I get the following:
>
> commit b738396cfdcc276c0cde0c1a6462c5cc74ba7b76
> Author: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date:   Thu Jul 14 15:58:59 2016 +0200
>
>     mingw: fix regression in t1308-config-set
>
> which seems to be completely unrelated. What am I missing please?

I think the commit is an "oops, we found a regression on a different
platform than the one used when developing the series after its
development completed, and here is a fix on top" commit that is
queued as the tip of a series.  You shouldn't be using "git show" on
it to look ONLY the tip of the series.

Let me show a better way to ask Git what you want to know, with the
excellent "git when-merged" script (google for it).

$ git when-merged b738396 master
refs/heads/master                      75676c8c8b6cbeec7ccb68d97c17db230d9f2659

We merged that commit to 'master' at 75676c8c8.  What does the merge
log say?

$ git show 75676c8c
commit 75676c8c8b6cbeec7ccb68d97c17db230d9f2659
Merge: 79ed43c28f b738396cfd
Author: Junio C Hamano <gitster@pobox.com>
Date:   Thu Jul 14 10:38:57 2016 -0700

    Merge branch 'jk/upload-pack-hook'

    A hot-fix to make a test working in mingw again.

    * jk/upload-pack-hook:
      mingw: fix regression in t1308-config-set

OK, so it was a hot-fix that consists of a single commit.  What did
we need to hot-fix?  A hot-fix is typically queued as a direct
follow-up to what is needed to be fixed.  When did we merge the
parent of the fix?

$ git when-merged b738396^ master
refs/heads/master                      1e4bf907890e094f1c1c8c5086387e7d5fdb0655

And that merge commit on 'master' shows us the series that needed to
be fixed up.

$ git show 1e4bf907890
commit 1e4bf907890e094f1c1c8c5086387e7d5fdb0655
Merge: 7a738b40f6 20b20a22f8
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Jul 6 13:38:11 2016 -0700

    Merge branch 'jk/upload-pack-hook'

    "upload-pack" allows a custom "git pack-objects" replacement when
    responding to "fetch/clone" via the uploadpack.packObjectsHook.

    * jk/upload-pack-hook:
      upload-pack: provide a hook for running pack-objects
      t1308: do not get fooled by symbolic links to the source tree
      config: add a notion of "scope"
      config: return configset value for current_config_ functions
      config: set up config_source for command-line config
      git_config_parse_parameter: refactor cleanup code
      git_config_with_options: drop "found" counting

You learned from the above that the jk/upload-pack-hook topic was
developed as a 7-patch series, reviewed, tested and got merged to
'master' on Jul 6th.  Unfortunately a test in the series had a
portability issue that wasn't discovered while it was being reviewed
and tested, and a hot-fix was queued on top and merged to 'master'
about a week later.  If we wanted to merge the topic to the
maintenance track, we cannot just merge the original 7-patch series.
We need to merge the whole thing, including the 8th one that is the
hot-fix.

^ permalink raw reply

* Re: [PATCH] tag: add a config option for setting --annotate by default
From: David Aguilar @ 2017-02-05  4:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqlgtm8s5k.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 03, 2017 at 09:02:47PM -0800, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> 
> > Make it easier for users to remember to annotate their tags.
> > Allow setting the default value for "--annotate" via the "tag.annotate"
> > configuration variable.
> >
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> 
> I do not care too strongly about this, but I need to point out that
> this will have fallout to tools and scripts.  E.g. if you have this
> configured and try to create a new tag in gitk, wouldn't this part
> 
> 	if {$msg != {}} {
> 	    exec git tag -a -m $msg $tag $id
> 	} else {
> 	    exec git tag $tag $id
> 	}
> 
> try to open an editor somehow to get the message even when $msg is
> an empty string?  I think the same problem already exists for the
> tag.forceSignAnnotated variable we already have added, though.

That's true.  I should have put "RFC" in the subject line.
Let's drop this patch unless there's others that find it useful.

How do you feel about a patch to add "git merge --signoff", for
consistency with "git commit"?

The rationale is that there might be situations (evil merges, or
even regular merges depending on the project) where someone
might want to signoff on their merges.
-- 
David

^ permalink raw reply

* release notes/ change number discrepancy ? - Documentation/RelNotes/2.10.0.txt "merge b738396..."
From: Zenaan Harkness @ 2017-02-05  2:21 UTC (permalink / raw)
  To: git

Am I missing something in the following:

looking at Documentation/RelNotes/2.10.0.txt I see the following release
note (~line 35):

 * "upload-pack" allows a custom "git pack-objects" replacement when
   responding to "fetch/clone" via the uploadpack.packObjectsHook.
   (merge b738396 jk/upload-pack-hook later to maint).


but when I run git show b738396 , I get the following:

commit b738396cfdcc276c0cde0c1a6462c5cc74ba7b76
Author: Johannes Schindelin <johannes.schindelin@gmx.de>
Date:   Thu Jul 14 15:58:59 2016 +0200

    mingw: fix regression in t1308-config-set


which seems to be completely unrelated. What am I missing please?

Thanks
Zenaan

^ permalink raw reply

* [PATCH] completion: fix git svn authorship switches
From: Eric Wong @ 2017-02-05  2:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

--add-author-from and --use-log-author are for "git svn dcommit",
not "git svn (init|clone)"

Signed-off-by: Eric Wong <e@80x24.org>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

 Though, I now wonder if allowing those switches to write changes
 in $GIT_CONFIG at init/clone time makes sense...

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 97d73ad88f..6990e98c44 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2587,14 +2587,14 @@ _git_svn ()
 			--no-metadata --use-svm-props --use-svnsync-props
 			--log-window-size= --no-checkout --quiet
 			--repack-flags --use-log-author --localtime
+			--add-author-from
 			--ignore-paths= --include-paths= $remote_opts
 			"
 		local init_opts="
 			--template= --shared= --trunk= --tags=
 			--branches= --stdlayout --minimize-url
 			--no-metadata --use-svm-props --use-svnsync-props
-			--rewrite-root= --prefix= --use-log-author
-			--add-author-from $remote_opts
+			--rewrite-root= --prefix= $remote_opts
 			"
 		local cmt_opts="
 			--edit --rmdir --find-copies-harder --copy-similarity=
-- 
EW

^ permalink raw reply related

* Re: init --separate-git-dir does not set core.worktree
From: Kyle Meyer @ 2017-02-04 23:34 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Stefan Beller
In-Reply-To: <CACsJy8C=owNPpND4Ab7bFE24kpWBr5fQdob21DEDCckCXu0Mng@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Feb 2, 2017 at 7:37 PM, Kyle Meyer <kyle@kyleam.com> wrote:

[...]

>>  * COMMIT_EDITMSG in .git/modules/<module>/: set working tree to the
>>    output of "git rev-parse --show-toplevel"
>>
>>  * COMMIT_EDITMSG in .git/worktrees/<wtree>/: set working tree to the
>>    path in .git/worktrees/<wtree>/gitdir, minus the trailing "/.git"
>>
>>  * COMMIT_EDITMSG in .git: set working tree to the parent directory
>>
>> This fails for a repo set up with --separate-git-dir [*2*], where the
>> last step will go out into an unrelated repo.  If core.worktree was set
>> and "git rev-parse --show-toplevel" returned the working tree like it
>> did for submodules, things would work.
>
> OK. If I read this right, given a path of any text file somewhere
> within ".git" directory. you are tasked to find out where the
> associated worktree is?

Right

> I.e. this is not an emacsclient executed as part of "git commit",
> correct?

... but it is from a "git commit" call.  I think you're asking because,
if we know where "git commit" was called from, we know what the working
tree is.  This is true, but with the current Magit design, the function
for determining the top-level of the working tree, magit-toplevel,
doesn't have access to this information.  From Emacs, Magit calls "git
commit", setting GIT_EDITOR for that process so that git invokes the
current Emacs instance for editing the commit message.  From
COMMIT_EDITMSG, we want the magit-toplevel to be able to determine the
working tree location so that commands can use magit-toplevel to set
their pwd.

The only Magit command that currently relies on magit-toplevel to
determine the working tree from ".git/COMMIT_EDITMSG" is a command that
shows a diff with the staged changes in a separate buffer.  (Even though
"git diff --cached" would work from within ".git/", some functionality
in this buffer depends on its pwd being set top-level of the working
tree.)

So, whatever solution we come up with on the Magit end will likely
involve recording where "git commit" was called so that the diff command
can figure out the working tree.

> So you need some sort of back-link to ".git" location. And
> unfortunately there's no such thing for .git file (unless it points to
> .git/worktrees/...). I'm hesitant to set core.worktree unless it's
> absolutely needed since it may have unexpected interaction with
> $GIT_WORK_TREE and others (not sure if it has any interaction with
> submodules too). I think adding a new file "gitdir" would be a safer
> option.
>
> I'm not entirely sure if enforcing "one worktree - one repository" is
> safe though. The first two bullet points are very specific and we can
> assume that, but ".git" files alone can be used for anything. In
> theory you can always create a secondary worktree (that's not managed
> by "git worktree") by setting GIT_WORK_TREE and "git checkout -f"
> somewhere. But I guess those would be temporary and nobody would want
> magic to point back to them.

Right, makes sense.

Unfortunately, magit-toplevel was designed thinking that the
--separate-git-dir / core.worktree behavior in Git 2.8.4 and lower was
intentional [*].  We'll need to rework this on our end.

Thanks for your input and for confirming that "git init
--separate-git-dir" does not set core.worktree by design.

[*] https://github.com/magit/magit/issues/460#issuecomment-36035787.

-- 
Kyle

^ permalink raw reply

* Re: feature request: add -q to "git branch"
From: Pranit Bauva @ 2017-02-04 21:17 UTC (permalink / raw)
  To: Kevin Layer; +Cc: Git List
In-Reply-To: <CAFZEwPOFDT7=1qhg4ygJpVUnfQo3XUjDoNtZ4LJvG5V9+RDNwA@mail.gmail.com>

Hey Kevin,

Sorry for the previous message.

On Sun, Feb 5, 2017 at 2:47 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> Hey Kevin,
>
> On Fri, Feb 3, 2017 at 11:59 PM, Kevin Layer <layer@known.net> wrote:
>> It should be possible to quietly create a branch.

I think `git branch` is already quiet. Are you seeing something else?

Regards,
Pranit Bauva

^ permalink raw reply

* Re: feature request: add -q to "git branch"
From: Pranit Bauva @ 2017-02-04 21:17 UTC (permalink / raw)
  To: Kevin Layer; +Cc: Git List
In-Reply-To: <CAGSZTjLmYCyKZ1BBRv+JVYq4oX7EQcNzyxAnS_3NBUPjr3g8zQ@mail.gmail.com>

Hey Kevin,

On Fri, Feb 3, 2017 at 11:59 PM, Kevin Layer <layer@known.net> wrote:
> It should be possible to quietly create a branch.
>
> Thanks.
>
> Kevin

^ 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