Git development
 help / color / mirror / Atom feed
* [PATCH v4 1/7] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-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.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* [PATCH v4 2/7] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-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..8365ebba2a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -9,6 +9,8 @@ USAGE="list [<options>]
    or: $dashless branch <branchname> [<stash>]
    or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
 		       [-u|--include-untracked] [-a|--all] [<message>]]
+   or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+		      [-u|--include-untracked] [-a|--all] [-m <message>]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -189,10 +191,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 +219,11 @@ save_stash () {
 		-a|--all)
 			untracked=all
 			;;
+		-m|--message)
+			shift
+			test -z ${1+x} && usage
+			stash_msg=$1
+			;;
 		--help)
 			show_help
 			;;
@@ -251,8 +259,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 +297,36 @@ save_stash () {
 	fi
 }
 
+save_stash () {
+	push_options=
+	while test $# != 0
+	do
+		case "$1" in
+		--)
+			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..3577115807 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 -1 >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* [PATCH v4 6/7] stash: use stash_push for no verb form
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-1-t.gummerer@gmail.com>

Now that we have stash_push, which accepts pathspec arguments, use
it instead of stash_save in git stash without any additional verbs.

This does introduce a small regression.  Previously we allowed
git stash -- -message, with a hyphen in front of the message.  However
git stash -- message without the hyphen was not allowed.  After this
change adding a message to the stash, with or without hyphen is no
longer allowed.

While this now allows specifying a message with the -m flag, it also
disallows messages without a hyphen.  This is to prevent user errors,
where a user tries to stash something with a message, but changes their
mind, and now would like to apply a stash, but forgets to remove the -m
flag.  E.g. git stash -m mes^H^H^Happly would result in a stash with the
message apply, while the user might have intended to apply a previous
stash.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---

If this kind of regression introduced here is not acceptable, I think
we could hide this change between some kind of config option, and
transition users over later after a warning period.

 Documentation/git-stash.txt |  8 ++++----
 git-stash.sh                | 16 ++++++++--------
 t/t3903-stash.sh            |  4 +---
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index f462f393b0..b0825f4aca 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,11 +13,11 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 '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]
+'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>...]
+	     [--] [<pathspec>...]]
 'git stash' clear
 'git stash' create [<message>]
 'git stash' create [-m <message>] [-u|--include-untracked <untracked|all>]
diff --git a/git-stash.sh b/git-stash.sh
index db56cf2c66..769cee9fd8 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,11 +7,11 @@ USAGE="list [<options>]
    or: $dashless drop [-q|--quiet] [<stash>]
    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    or: $dashless branch <branchname> [<stash>]
-   or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		       [-u|--include-untracked] [-a|--all] [<message>]]
-   or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		      [-u|--include-untracked] [-a|--all] [-m <message>]
-		      [-- <pathspec>...]
+   or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+		      [-u|--include-untracked] [-a|--all] [<message>]
+   or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+		       [-u|--include-untracked] [-a|--all] [-m <message>]
+		       [-- <pathspec>...]]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -699,7 +699,7 @@ apply_to_branch () {
 }
 
 PARSE_CACHE='--not-parsed'
-# The default command is "save" if nothing but options are given
+# The default command is "push" if nothing but options are given
 seen_non_option=
 for opt
 do
@@ -709,7 +709,7 @@ do
 	esac
 done
 
-test -n "$seen_non_option" || set "save" "$@"
+test -n "$seen_non_option" || set "push" "$@"
 
 # Main command set
 case "$1" in
@@ -760,7 +760,7 @@ branch)
 *)
 	case $# in
 	0)
-		save_stash &&
+		push_stash &&
 		say "$(gettext "(To restore them type \"git stash apply\")")"
 		;;
 	*)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 8b372c35fb..d568799da9 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -274,9 +274,7 @@ test_expect_success 'stash --invalid-option' '
 	git add file2 &&
 	test_must_fail git stash --invalid-option &&
 	test_must_fail git stash save --invalid-option &&
-	test bar5,bar6 = $(cat file),$(cat file2) &&
-	git stash -- -message-starting-with-dash &&
-	test bar,bar2 = $(cat file),$(cat file2)
+	test bar5,bar6 = $(cat file),$(cat file2)
 '
 
 test_expect_success 'stash an added file' '
-- 
2.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* [PATCH v4 5/7] stash: teach 'push' (and 'create') to honor pathspec
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-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                       | 50 ++++++++++++++++++++------
 t/t3903-stash.sh                   | 72 ++++++++++++++++++++++++++++++++++++++
 t/t3905-stash-include-untracked.sh | 26 ++++++++++++++
 4 files changed, 148 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index a138ed6a24..f462f393b0 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>]
+	     [-- <pathspec>...]
 '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 6d629fc43f..db56cf2c66 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -11,6 +11,7 @@ USAGE="list [<options>]
 		       [-u|--include-untracked] [-a|--all] [<message>]]
    or: $dashless push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
 		      [-u|--include-untracked] [-a|--all] [-m <message>]
+		      [-- <pathspec>...]
    or: $dashless clear"
 
 SUBDIRECTORY_OK=Yes
@@ -35,15 +36,15 @@ else
 fi
 
 no_changes () {
-	git diff-index --quiet --cached HEAD --ignore-submodules -- &&
-	git diff-files --quiet --ignore-submodules &&
+	git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
+	git diff-files --quiet --ignore-submodules -- "$@" &&
 	(test -z "$untracked" || test -z "$(untracked_files)")
 }
 
 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 () {
@@ -76,6 +77,11 @@ create_stash () {
 			untracked="$1"
 			new_style=t
 			;;
+		--)
+			shift
+			new_style=t
+			break
+			;;
 		*)
 			if test -n "$new_style"
 			then
@@ -103,10 +109,11 @@ create_stash () {
 	if test -z "$new_style"
 	then
 		stash_msg="$*"
+		set --
 	fi
 
 	git update-index -q --refresh
-	if no_changes
+	if no_changes "$@"
 	then
 		exit 0
 	fi
@@ -138,7 +145,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" &&
@@ -161,7 +168,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"
@@ -175,7 +182,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) ||
@@ -304,7 +311,7 @@ push_stash () {
 	fi
 
 	git update-index -q --refresh
-	if no_changes
+	if no_changes "$@"
 	then
 		say "$(gettext "No local changes to save")"
 		exit 0
@@ -312,18 +319,39 @@ 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
+			saved_untracked=
+			if test -n "$(git ls-files --others -- "$@")"
+			then
+				saved_untracked=$(
+					git ls-files -z --others -- "$@" |
+					    xargs -0 git stash create -u all --)
+			fi
+			git ls-files -z -- "$@" | xargs -0 git reset ${GIT_QUIET:+-q} --
+			git ls-files -z --modified -- "$@" | xargs -0 git checkout ${GIT_QUIET:+-q} HEAD --
+			if test -n "$(git ls-files -z --others -- "$@")"
+			then
+				git ls-files -z --others -- "$@" | xargs -0 git clean --force -d ${GIT_QUIET:+-q} --
+			fi
+			if test -n "$saved_untracked"
+			then
+				git stash pop -q $saved_untracked
+			fi
+		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 812d0f7a40..8b372c35fb 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -811,4 +811,76 @@ 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 &&
+	>bar &&
+	git add foo* &&
+	git stash push -- "foo b*" &&
+	test_path_is_missing "foo bar" &&
+	test_path_is_file 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_expect_success 'stash push -p with pathspec shows no changes only onece' '
+	>file &&
+	git add file &&
+	git stash push -p not-file >actual &&
+	echo "No local changes to save" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash push with pathspec shows no changes when there are none' '
+	>file &&
+	git add file &&
+	git stash push not-file >actual &&
+	echo "No local changes to save" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'untracked file is not removed when using pathspecs' '
+	>untracked &&
+	git stash push untracked &&
+	test_path_is_file untracked
+'
+
+test_expect_success 'untracked files are left in place when -u is not given' '
+	>file &&
+	git add file &&
+	>untracked &&
+	git stash push file &&
+	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..193adc7b68 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 b*" &&
+	test_path_is_missing "foo bar" &&
+	test_path_is_file 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.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* [PATCH v4 4/7] stash: introduce new format create
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-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.

This introduces a slight regression, when git stash create -m works is
used.  Before this change, it created a stash with the message
"-m works", but now it creates a stash with the message "-m".

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt |  1 +
 git-stash.sh                | 52 +++++++++++++++++++++++++++++++++++++++++----
 t/t3903-stash.sh            |  9 ++++++++
 3 files changed, 58 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 8365ebba2a..6d629fc43f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,52 @@ clear_stash () {
 }
 
 create_stash () {
-	stash_msg="$1"
-	untracked="$2"
+	stash_msg=
+	untracked=
+	new_style=
+	while test $# != 0
+	do
+		case "$1" in
+		-m|--message)
+			shift
+			test -z ${1+x} && usage
+			stash_msg="$1"
+			new_style=t
+			;;
+		-u|--include-untracked)
+			shift
+			test -z ${1+x} && usage
+			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
@@ -268,7 +312,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 +711,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 ffe3549ea5..812d0f7a40 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 -s ${STASH_ID} >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* [PATCH v4 3/7] stash: add test for the create command line arguments
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-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 3577115807..ffe3549ea5 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 -s ${STASH_ID} >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 -s ${STASH_ID} >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* [PATCH v4 7/7] stash: allow pathspecs in the no verb form
From: Thomas Gummerer @ 2017-02-12 21:54 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Thomas Gummerer
In-Reply-To: <20170212215420.16701-1-t.gummerer@gmail.com>

Now that stash_push is used in the no verb form of stash, allow
specifying the command line for this form as well.  Always use -- to
disambiguate pathspecs from other non-option arguments.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 git-stash.sh     |  1 +
 t/t3903-stash.sh | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 769cee9fd8..a184b1e274 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -704,6 +704,7 @@ seen_non_option=
 for opt
 do
 	case "$opt" in
+	--) break ;;
 	-*) ;;
 	*) seen_non_option=t; break ;;
 	esac
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index d568799da9..22ac75377b 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -881,4 +881,19 @@ test_expect_success 'untracked files are left in place when -u is not given' '
 	test_path_is_file untracked
 '
 
+test_expect_success 'stash without verb with pathspec' '
+	>"foo bar" &&
+	>foo &&
+	>bar &&
+	git add foo* &&
+	git stash -- "foo b*" &&
+	test_path_is_missing "foo bar" &&
+	test_path_is_file 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.11.0.301.g86e6ecc671.dirty


^ permalink raw reply related

* Re: [PATCH v4 0/7] stash: support pathspec argument
From: Thomas Gummerer @ 2017-02-12 22:07 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170212215420.16701-1-t.gummerer@gmail.com>

On 02/12, Thomas Gummerer wrote:
> Thanks Peff and Junio for the review of the last round.
>

Sorry it seems like I messed up the In-Reply-To header.  Previous rounds were at:
v1: http://public-inbox.org/git/20170121200804.19009-1-t.gummerer@gmail.com/
v2: http://public-inbox.org/git/20170129201604.30445-1-t.gummerer@gmail.com/
v3: http://public-inbox.org/git/20170205202642.14216-1-t.gummerer@gmail.com/

^ permalink raw reply

* Re: What's cooking in git.git (Feb 2017, #03; Fri, 10)
From: René Scharfe @ 2017-02-12 23:01 UTC (permalink / raw)
  To: Vegard Nossum, Junio C Hamano; +Cc: git
In-Reply-To: <36fdf8c9-f82d-4996-09ef-7d718ab3ac41@oracle.com>

Am 12.02.2017 um 19:32 schrieb Vegard Nossum:
> I said I would resubmit the patches with more config options and more
> command-line arguments (to avoid potentially breaking backwards
> compatibility), but IIRC the response seemed to be "preceding blank line
> heuristic is good enough" and "why bother", so I ended up not not
> resubmitting anything.

I was (and still am) looking forward to your patches.  The current 
heuristic is simplistic, the patches you already sent improve the output 
in certain scenarios, my proposed changes on top aimed to limit 
drawbacks in other scenarios, but together they still have shortcomings.

Avoiding new switches would be nice, though (if possible).  I feel we 
need a lot more tests to nail down our expectations.

René

^ permalink raw reply

* Re: [git-gui] Amending doesn't preserve timestamp
From: Igor Djordjevic BugA @ 2017-02-12 23:06 UTC (permalink / raw)
  To: Juraj; +Cc: git@vger.kernel.org
In-Reply-To: <CAEPqvozCvFqOTNPw0c1ErKoOd+Mn1WCyJr9hj6CXiWWtL93Tqw@mail.gmail.com>

On 12/02/2017 22:40, Juraj wrote:
> Hi Igor,
> 
> I forgot to write the version I'm using. It's on Ubuntu 16.04, git-gui
> package version 1:2.7.4-0ubuntu1 (--version: git-gui version 0.20.0),
> git version 2.7.4, tcl and tk 8.6.0+9. Perhaps it got fixed in a newer
> version, in that case, my bad for not checking before posting.
> 
> Thanks,
> Juraj

Hi Juraj,

Indeed, if I`m reading it correctly, it seems to be addressed in git-gui
version 0.21.0[1], introduced in git version 2.11.0[2] on 2016-11-29
("git-gui: Do not reset author details on amend", 2016-04-11[3],
referencing an old bug report[4]).

Regards,
BugA

[1] https://public-inbox.org/git/878ttji701.fsf@red.patthoyts.tk/
[2]
https://public-inbox.org/git/xmqqmvgidlsg.fsf@gitster.mtv.corp.google.com/
[3]
https://public-inbox.org/git/1462458182-4488-1-git-send-email-orgads@gmail.com/
[4]
https://public-inbox.org/git/CAGHpTB+35j0njmCZ0uCgBVroe=Ma7HLnn6fDty8yebKWgEmECg@mail.gmail.com/

^ permalink raw reply

* Re: [PATCH] fetch: print an error when declining to request an unadvertised object
From: Junio C Hamano @ 2017-02-12 23:49 UTC (permalink / raw)
  To: Matt McCutchen; +Cc: git
In-Reply-To: <1486934007.8517.10.camel@mattmccutchen.net>

Matt McCutchen <matt@mattmccutchen.net> writes:

> What do you think?  Do you not care about having a more specific error,
> in which case I can copy the code from builtin/fetch-pack.c to
> fetch_refs_via_pack?  Or shall I add code to filter_refs to set a flag
> and add code to builtin/fetch-pack.c and fetch_refs_via_pack to check
> the flag?  Or what?

The fact that we have the above two choices tells me that a two-step
approach may be an appropriate approach.

The first step is to teach fetch_refs_via_pack() that it should not
ignore the information returned in sought[].  It would add new code
similar to what cmd_fetch_pack() uses to notice and report errors
[*1*] to the function.  It would be a sensible first step, but would
not let the user know which of multiple causes of "not matched" we
noticed.

By "a more specific error", I think you are envisioning that the
current boolean "matched" is made into an enum that allows the
caller to tell how each request did not match [*2*].  That can be
the topic of the second patch and would have to touch filter_refs()
[*3*], cmd_fetch_pack() and fetch_refs_via_pack().

I do not have strong preference myself offhand between stopping at
the first step or completing both.

Even if you did only the first step, as long as the second step can
be done without reverting what the first step did [*4*] by somebody
who cares the "specific error" deeply enough, I am OK with that.  Of
course if you did both steps, that is fine by me as well ;-)


[Footnote]

*1* While I know that it is not right to die() in filter_refs(), and
    fetch_refs_via_pack() is a better place to notice errors, I do
    not offhand know if it is the right place to report errors, or a
    caller higher in the callchain may want the callee to be silent
    and wants to show its own error message (in which case the error
    may have to percolate upwards in the callchain).

*2* e.g. "was it a ref but they did not advertise?  Did it request
    an explicit object name and they did not allow it?"  We may want
    to support other "more specific" errors that can be detected in
    the future.

*3* The current code flips the sought[i]->matched bit on for matched
    ones (relying on the initial state of the bit being false), but
    it now needs to stuff different kind of "not matched" to the
    field to allow the caller to act on it.

*4* IOW, I am OK with an initial "small" improvement, but I'd want
    to make sure that such an initial step does not make future
    enhancements by others harder.

^ permalink raw reply

* [PATCH] completion: teach to complete git-subtree
From: cornelius.weig @ 2017-02-13  1:07 UTC (permalink / raw)
  To: git
  Cc: bitte.keine.werbung.einwerfen, Cornelius Weig, szeder.dev, s6t,
	greened, davvid

From: Cornelius Weig <cornelius.weig@tngtech.com>

Git-subtree is a contrib-command without completion, making it
cumbersome to use.

Teach bash completion to complete the subcommands of subtree (add,
merge, pull, push, split) and options thereof. For subcommands
supporting it (add, push, pull) also complete remote names and refspec.

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
 contrib/completion/git-completion.bash | 35 ++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6c6e1c7..430bfed 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -535,9 +535,9 @@ __git_complete_remote_or_refspec ()
 {
 	local cur_="$cur" cmd="${words[1]}"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
-	if [ "$cmd" = "remote" ]; then
-		((c++))
-	fi
+	case "$cmd" in
+	remote|subtree) ((c++)) ;;
+	esac
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -586,7 +586,7 @@ __git_complete_remote_or_refspec ()
 			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
-	pull|remote)
+	pull|remote|subtree)
 		if [ $lhs = 1 ]; then
 			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
 		else
@@ -2569,6 +2569,33 @@ _git_submodule ()
 	fi
 }
 
+_git_subtree ()
+{
+	local subcommands="add merge pull push split"
+	local subcommand="$(__git_find_on_cmdline "$subcommands")"
+	if [ -z "$subcommand" ]; then
+		__gitcomp "$subcommands"
+		return
+	fi
+	case "$subcommand,$cur" in
+	add,--*|merge,--*|pull,--*)
+		__gitcomp "--prefix= --squash --message="
+		;;
+	push,--*)
+		__gitcomp "--prefix="
+		;;
+	split,--*)
+		__gitcomp "
+			--annotate= --branch= --ignore-joins
+			--onto= --prefix= --rejoin
+			"
+		;;
+	add,*|push,*|pull,*)
+		__git_complete_remote_or_refspec
+		;;
+	esac
+}
+
 _git_svn ()
 {
 	local subcommands="
-- 
2.10.2


^ permalink raw reply related

* Re: [PATCH 0/5] Store submodules in a hash, not a linked list
From: Duy Nguyen @ 2017-02-13  2:39 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, Junio C Hamano, Stefan Beller,
	Johannes Schindelin, David Turner, git
In-Reply-To: <20170210004033.cgqmovhvoylad5cf@sigill.intra.peff.net>

On Thu, Feb 09, 2017 at 07:40:33PM -0500, Jeff King wrote:
> On Thu, Feb 09, 2017 at 10:23:35PM +0100, Michael Haggerty wrote:
> 
> > >> So push the submodule attribute down to the `files_ref_store` class
> > >> (but continue to let the `ref_store`s be looked up by submodule).
> > > 
> > > I'm not sure I understand all of the ramifications here. It _sounds_ like
> > > pushing this down into the files-backend code would make it harder to
> > > have mixed ref-backends for different submodules. Or is this just
> > > pushing down an implementation detail of the files backend, and future
> > > code is free to have as many different ref_stores as it likes?
> > 
> > I don't understand how this would make it harder, aside from the fact
> > that a new backend class might also need a path member and have to
> > maintain its own copy rather than using one that the base class provides.
> 
> Probably the answer is "I'm really confused".
> 
> But here's how my line of reasoning went:
> 
>   Right now we have a main ref-store that points to the submodule
>   ref-stores. I don't know the current state of it, but in theory those
>   could all use different backends.
> 
>   This seems like it's pushing that submodule linkage down into the
>   backend.
> 
> But I think from your response that the answer is no, the thing that is
> being pushed down is not the right way for the main ref store and the
> submodules to be linked.

I think it's more about "pushing out" than "pushing down". Once files
backend takes a path to .git directory, we could have a submodule
ref_store that resolves submodule path to that .git directory,
files-backend will not need to know anything about submodules.

I imagine in future lookup_ref_store() will take a .git path instead
of a submodule path, then iterate through all backends and call the
backend-specific "probe" function to let the backend figure out if
it's the right backend and whatever parameters it needs (e.g. IP
address or path). There would be submodule_lookup_ref_store() wrapper
that converts submodule path to .git path for lookup_ref_store() to
consume.
--
Duy

^ permalink raw reply

* Incorrect pipe for git log graph when using --name-status option
From: hIpPy @ 2017-02-13  2:49 UTC (permalink / raw)
  To: git

The `git log` command with `graph` and pretty format works correctly
as expected.

$ git log --graph --pretty=format:'%h' -2
* 714a14e
* 87dce5f


However, with `--name-status` option added, there is a pipe
incorrectly placed after the commit hash (example below).

$ git log --graph --pretty=format:'%h' -2 --name-status
* 714a14e|
| M README.md
| A rm.Extensions/BitSet.cs
| M rm.Extensions/Properties/AssemblyInfo.cs
| M rm.Extensions/rm.Extensions.csproj
| A rm.ExtensionsTest/BitSetTest.cs
| M rm.ExtensionsTest/rm.ExtensionsTest.csproj

* 87dce5f|
| M rm.Extensions/GraphExtension.cs
| M rm.Extensions/Wrapped.cs
| M rm.Extensions/WrappedExtension.cs
| M rm.Extensions/rm.Extensions.csproj


IMHO, I think this is a bug. I think the correct output should be
below.

$ git log --graph --pretty=format:'%h' -2 --name-status
* 714a14e
| M README.md
| A rm.Extensions/BitSet.cs
| M rm.Extensions/Properties/AssemblyInfo.cs
| M rm.Extensions/rm.Extensions.csproj

| A rm.ExtensionsTest/BitSetTest.cs
| M rm.ExtensionsTest/rm.ExtensionsTest.csproj
|
* 87dce5f
| M rm.Extensions/GraphExtension.cs
| M rm.Extensions/Wrapped.cs
| M rm.Extensions/WrappedExtension.cs

| M rm.Extensions/rm.Extensions.csproj

I'm using this:

git version 2.11.0.windows.1
GNU bash, version 4.3.46(2)-release (x86_64-pc-msys)
Windows 8.1 64-bit


Thanks,
RM

^ permalink raw reply

* Re: [PATCH v2 0/9] Store submodules in a hash, not a linked list
From: David Turner @ 2017-02-13  3:09 UTC (permalink / raw)
  To: Michael Haggerty, Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
	Johannes Schindelin, Jeff King, git
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>



On Fri, 2017-02-10 at 12:16 +0100, Michael Haggerty wrote:
> This is v2 of the patch series, considerably reorganized but not that
> different codewise. Thanks to Stefan, Junio, and Peff for their
> feedback about v1 [1]. I think I have addressed all of your comments.

LGTM.


^ permalink raw reply

* Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]
From: Arif Khokar @ 2017-02-13  5:52 UTC (permalink / raw)
  To: Johannes Schindelin, Arif Khokar
  Cc: Jakub Narębski, Jeff King, Stefan Beller,
	meta@public-inbox.org, git@vger.kernel.org, Eric Wong
In-Reply-To: <alpine.DEB.2.20.1702101707060.3496@virtualbox>

On 02/10/2017 11:10 AM, Johannes Schindelin wrote:
> Hi Arif,
>
> On Wed, 24 Aug 2016, Johannes Schindelin wrote:

>> I recently adapted an old script I had to apply an entire patch series
>> given the GMane link to its cover letter:
>>
>> https://github.com/git-for-windows/build-extra/blob/master/apply-from-gmane.sh
>>
>> Maybe you find it in you to adapt that to work with public-inbox.org?
>
> Oh well. That would have been too easy a task, right?
>
> As it happens, I needed this functionality myself (when reworking my
> git-path-in-subdir patch to include Mike Rappazzo's previous patch series
> that tried to fix the same bug).
>
> I copy-edited the script to work with public-inbox.org, it accepts a
> Message-ID or URL or GMane URL and will try to apply the patch (or patch
> series) on top of the current revision:
>
> https://github.com/git-for-windows/build-extra/blob/2268850552c7/apply-from-public-inbox.sh

Thanks for the link.  One thing that comes to mind that is that it may 
be better to just download the patches and then manually apply them 
afterwords rather than doing it in the script itself.  Or at least add 
an option to the script to not automatically invoke git am.

Getting back to the point I made when this thread was still active, I 
still think it would be better to be able to list the message-id values 
in the header or body of the cover letter message of a patch series 
(preferably the former) in order to facilitate downloading the patches 
via NNTP from gmane or public-inbox.org.  That would make it easier 
compared to the different, ad-hoc, methods that exist for each email client.

Alternatively, or perhaps in addition to the list of message-ids, a list 
of URLs to public-inbox.org or gmane messages could also be provided for 
those who prefer to download patches via HTTP.

^ permalink raw reply

* Re: Git status performance on PS (command prompt)
From: Christian Couder @ 2017-02-13  6:12 UTC (permalink / raw)
  To: brian m. carlson, Mark Gaiser, git
In-Reply-To: <20170212164650.u7frv234kmu5hm7r@genre.crustytoothpaste.net>

On Sun, Feb 12, 2017 at 5:46 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Sun, Feb 12, 2017 at 04:53:47PM +0100, Mark Gaiser wrote:

[...]

>> I did a bit of profiling in git to figure out where this slowdown comes from.
>> Callgrind tells me that "read_directory_recursive" takes up ~62% of the time.
>> Within that call the function "last_exclude_matching_from_list" takes
>> up 49% of the time it takes to run "git status --porcelain -b".

The core.untrackedCache config option may help.

^ permalink raw reply

* Re: [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
From: Michael Haggerty @ 2017-02-13  6:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
	Johannes Schindelin, David Turner, Jeff King, git
In-Reply-To: <xmqqbmu9lul6.fsf@gitster.mtv.corp.google.com>

On 02/10/2017 08:22 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> [...]
> 
> OK, but one thing puzzles me...
> 
>> @@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store;
>>  static struct hashmap submodule_ref_stores;
>>  
>>  /*
>> - * Return the ref_store instance for the specified submodule (or the
>> - * main repository if submodule is NULL). If that ref_store hasn't
>> - * been initialized yet, return NULL.
>> - */
>> -static struct ref_store *lookup_ref_store(const char *submodule)
>> -{
>> -	struct submodule_hash_entry *entry;
>> -
>> -	if (!submodule)
>> -		return main_ref_store;
>> -
>> -	if (!submodule_ref_stores.tablesize)
>> -		/* It's initialized on demand in register_ref_store(). */
>> -		return NULL;
>> -
>> -	entry = hashmap_get_from_hash(&submodule_ref_stores,
>> -				      strhash(submodule), submodule);
>> -	return entry ? entry->refs : NULL;
>> -}
>> -
>> -/*
>>   * Register the specified ref_store to be the one that should be used
>>   * for submodule (or the main repository if submodule is NULL). It is
>>   * a fatal error to call this function twice for the same submodule.
>> @@ -1451,6 +1430,27 @@ static struct ref_store *ref_store_init(const char *submodule)
>>  	return refs;
>>  }
>>  
>> +/*
>> + * Return the ref_store instance for the specified submodule (or the
>> + * main repository if submodule is NULL). If that ref_store hasn't
>> + * been initialized yet, return NULL.
>> + */
>> +static struct ref_store *lookup_ref_store(const char *submodule)
>> +{
>> +	struct submodule_hash_entry *entry;
>> +
>> +	if (!submodule)
>> +		return main_ref_store;
>> +
>> +	if (!submodule_ref_stores.tablesize)
>> +		/* It's initialized on demand in register_ref_store(). */
>> +		return NULL;
>> +
>> +	entry = hashmap_get_from_hash(&submodule_ref_stores,
>> +				      strhash(submodule), submodule);
>> +	return entry ? entry->refs : NULL;
>> +}
>> +
> 
> I somehow thought that we had an early "reorder the code" step to
> avoid hunks like these?  Am I missing some subtle changes made while
> moving the function down?

You are quite right; thanks for noticing. I forgot to un-move this
function when re-rolling. These two hunks can be discarded (the function
text is unchanged).

I pushed the fixed commit to branch `submodule-hash` in my fork [1]. If
you'd like me to send it to the mailing list again, please let me know.

Michael

[1] https://github.com/mhagger/git


^ permalink raw reply

* Re: [RFC PATCH] show decorations at the end of the line
From: Junio C Hamano @ 2017-02-13  8:30 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFw2S14a4_4YK0b6PNK4TH_AUo_+2JN+PTyBTufNeB5t6A@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Sat, Feb 11, 2017 at 10:02 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> I've signed off on this, because I think it's an "obvious" improvement,
>> but I'm putting the "RFC" in the subject line because this is clearly a
>> subjective thing.
>
> Side note: the one downside of showing the decorations at the end of
> the line is that now they are obviously at the end of the line - and
> thus likely to be more hidden by things like line truncation.

Side note: I refrained from commenting on this patch because
everybody knows that the what I would say anyway ;-) and I didn't
want to speak first to discourage others from raising their opinion.

An obvious downside is that people (against all recommendations) are
likely to have written a loose script expecting the --oneline format
is cast in stone.  I personally think it is OK to break them as long
as "workaround" (aka kosher way to do what they have been doing) is
obvious and easily doable, and in this case their script can switch
to use --format to keep using the order of fields and format they
have been relying on.

It would be nice if we can have that --format string they can use
somewhere in the log message, so that I can cut & paste it into the
release notes that contains this change (i.e. "those who want to
keep using the traditional --oneline --decorate can use this string
as pretty.my1line configuration variable and use --pretty=my1line
instead").

^ permalink raw reply

* [PATCH] clean: use warning_errno() when appropriate
From: Nguyễn Thái Ngọc Duy @ 2017-02-13  9:27 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

All these warning() calls are preceded by a system call. Report the
actual error to help the user understand why we fail to remove
something.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clean.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index d6bc3aaaea..dc1168747e 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -175,7 +175,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		res = dry_run ? 0 : rmdir(path->buf);
 		if (res) {
 			quote_path_relative(path->buf, prefix, &quoted);
-			warning(_(msg_warn_remove_failed), quoted.buf);
+			warning_errno(_(msg_warn_remove_failed), quoted.buf);
 			*dir_gone = 0;
 		}
 		return res;
@@ -209,7 +209,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 				string_list_append(&dels, quoted.buf);
 			} else {
 				quote_path_relative(path->buf, prefix, &quoted);
-				warning(_(msg_warn_remove_failed), quoted.buf);
+				warning_errno(_(msg_warn_remove_failed), quoted.buf);
 				*dir_gone = 0;
 				ret = 1;
 			}
@@ -231,7 +231,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 			*dir_gone = 1;
 		else {
 			quote_path_relative(path->buf, prefix, &quoted);
-			warning(_(msg_warn_remove_failed), quoted.buf);
+			warning_errno(_(msg_warn_remove_failed), quoted.buf);
 			*dir_gone = 0;
 			ret = 1;
 		}
@@ -982,7 +982,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			res = dry_run ? 0 : unlink(abs_path.buf);
 			if (res) {
 				qname = quote_path_relative(item->string, NULL, &buf);
-				warning(_(msg_warn_remove_failed), qname);
+				warning_errno(_(msg_warn_remove_failed), qname);
 				errors++;
 			} else if (!quiet) {
 				qname = quote_path_relative(item->string, NULL, &buf);
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]
From: Johannes Schindelin @ 2017-02-13 14:37 UTC (permalink / raw)
  To: Arif Khokar
  Cc: Arif Khokar, Jakub Narębski, Jeff King, Stefan Beller,
	meta@public-inbox.org, git@vger.kernel.org, Eric Wong
In-Reply-To: <d16546a4-25be-7b85-3191-e9393fda1164@hotmail.com>

Hi Arif,

On Mon, 13 Feb 2017, Arif Khokar wrote:

> On 02/10/2017 11:10 AM, Johannes Schindelin wrote:
> >
> > On Wed, 24 Aug 2016, Johannes Schindelin wrote:
> 
> > > I recently adapted an old script I had to apply an entire patch
> > > series given the GMane link to its cover letter:
> > >
> > > https://github.com/git-for-windows/build-extra/blob/master/apply-from-gmane.sh
> > >
> > > Maybe you find it in you to adapt that to work with
> > > public-inbox.org?
> >
> > Oh well. That would have been too easy a task, right?
> >
> > As it happens, I needed this functionality myself (when reworking my
> > git-path-in-subdir patch to include Mike Rappazzo's previous patch
> > series that tried to fix the same bug).
> >
> > I copy-edited the script to work with public-inbox.org, it accepts a
> > Message-ID or URL or GMane URL and will try to apply the patch (or
> > patch series) on top of the current revision:
> >
> > https://github.com/git-for-windows/build-extra/blob/2268850552c7/apply-from-public-inbox.sh
> 
> Thanks for the link.  One thing that comes to mind that is that it may
> be better to just download the patches and then manually apply them
> afterwords rather than doing it in the script itself.  Or at least add
> an option to the script to not automatically invoke git am.

I actually had expected *you* to put in a little bit of an effort, too. In
fact, I was very disappointed that you did not even look into porting that
script to use public-inbox instead of GMane.

> Getting back to the point I made when this thread was still active, I
> still think it would be better to be able to list the message-id values
> in the header or body of the cover letter message of a patch series
> (preferably the former) in order to facilitate downloading the patches
> via NNTP from gmane or public-inbox.org.  That would make it easier
> compared to the different, ad-hoc, methods that exist for each email
> client.

You can always do that yourself: you can modify your cover letter to
include that information.

Note that doing this automatically in format-patch may not be appropriate,
as 1) the Message-ID could be modified depending on the mail client used
to send the mails, and 2) it is not unheard of that a developer
finds a bug in the middle of sending a patch series, fixes that bug, and
regenerates the remainder of the patch series, completely rewriting those
Message-IDs.

> Alternatively, or perhaps in addition to the list of message-ids, a list
> of URLs to public-inbox.org or gmane messages could also be provided for
> those who prefer to download patches via HTTP.

At this point, I am a little disinterested in a discussion without code. I
brought some code to the table, after all.

Ciao,
Johannes

^ permalink raw reply

* [PATCH/RFC 00/11] Remove submodule from files-backend.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-13 15:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
	novalis, Nguyễn Thái Ngọc Duy

This is on top of mh/submodule-hash, it:

 - centralizes path manipulation around submodule, $GIT_DIR... in
   files-backend.c to a new function files_path() with the intention
   to make it easier to change later. files_path() is destined to
   become

      strbuf_addbuf(&sb, refs->gitdir);
      strbuf_complete(&sb, '/');
      strbuf_vaddf(&sb, fmt, vap);

   but that can only be achieved once we get compound ref store.

 - removes git_path() and submodule_git_path() from files-backend.c. No
   more magic path translation as far as ref backend is concerned.

 - moves submodule path resolution code outside the backend.
   files-backend is now oblivious of submodules and in theory a
   submodule ref-store supports all operations (but I could be wrong,
   I didn't stare hard)

 - exposes get_submodule_ref_store() and get_main_ref_store() as public
   API. A new set of API around ref-store will be added. And
   get_worktree_ref_store() of course. The *_submodule() API might be
   removed, we'll see.

The problem with set_worktree_head_symref() (which peeks into another
gitdir) should be solved once we have get_worktree_ref_store(). That's
my next step.

Compound ref store will have to wait until I'm done with my
gc-in-worktree problem as I think I can live without it for now. I
think after compound ref store is in place, adding lmdb backend back
should be much cleaner because it does not care about either
submodules or worktrees.

Nguyễn Thái Ngọc Duy (11):
  refs-internal.c: make files_log_ref_write() static
  files-backend: convert git_path() to strbuf_git_path()
  files-backend: add files_path()
  files-backend: replace *git_path*() with files_path()
  refs.c: share is_per_worktree_ref() to files-backend.c
  refs-internal.h: correct is_per_worktree_ref()
  files-backend: remove the use of git_path()
  refs.c: factor submodule code out of get_ref_store()
  refs: move submodule code out of files-backend.c
  files-backend: remove submodule_allowed from files_downcast()
  refs: split and make get_*_ref_store() public API

 refs.c               | 125 ++++++++++++------
 refs.h               |  13 ++
 refs/files-backend.c | 350 ++++++++++++++++++++++++++++++---------------------
 refs/refs-internal.h |  28 ++---
 4 files changed, 316 insertions(+), 200 deletions(-)

-- 
2.11.0.157.gd943d85


^ permalink raw reply

* [PATCH 03/11] files-backend: add files_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-13 15:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
	novalis, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170213152011.12050-1-pclouds@gmail.com>

This will be the replacement for both git_path() and git_path_submodule()
in this file. The idea is backend takes a git path and use that,
oblivious of submodule, linked worktrees and such.

This is the middle step towards that. Eventually the "submodule" field
in 'struct files_ref_store' should be replace by "gitdir". And a
compound ref_store is created to combine two files backends together,
one represents the shared refs in $GIT_COMMON_DIR, one per-worktree. At
that point, files_path() becomes a wrapper of strbuf_vaddf().

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6582c9b2d..39217a2ca 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -169,6 +169,9 @@ static int files_log_ref_write(const char *refname, const unsigned char *old_sha
 			       const unsigned char *new_sha1, const char *msg,
 			       int flags, struct strbuf *err);
 
+void files_path(struct files_ref_store *refs, struct strbuf *sb,
+		const char *fmt, ...) __attribute__((format (printf, 3, 4)));
+
 static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 {
 	struct ref_dir *dir;
@@ -930,6 +933,23 @@ struct files_ref_store {
 /* Lock used for the main packed-refs file: */
 static struct lock_file packlock;
 
+void files_path(struct files_ref_store *refs, struct strbuf *sb,
+		const char *fmt, ...)
+{
+	struct strbuf tmp = STRBUF_INIT;
+	va_list vap;
+
+	va_start(vap, fmt);
+	strbuf_vaddf(&tmp, fmt, vap);
+	va_end(vap);
+	if (refs->submodule)
+		strbuf_git_path_submodule(sb, refs->submodule,
+					  "%s", tmp.buf);
+	else
+		strbuf_git_path(sb, "%s", tmp.buf);
+	strbuf_release(&tmp);
+}
+
 /*
  * Increment the reference count of *packed_refs.
  */
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 02/11] files-backend: convert git_path() to strbuf_git_path()
From: Nguyễn Thái Ngọc Duy @ 2017-02-13 15:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
	novalis, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170213152011.12050-1-pclouds@gmail.com>

git_path() and friends are going to be killed in files-backend.c in near
future. And because there's a risk with overwriting buffer in
git_path(), let's convert them all to strbuf_git_path(). We'll have
easier time killing/converting strbuf_git_path() then because we won't
have to worry about memory management again.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 114 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 90 insertions(+), 24 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 75565c3aa..6582c9b2d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2169,6 +2169,8 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 	static int timeout_configured = 0;
 	static int timeout_value = 1000;
 	struct packed_ref_cache *packed_ref_cache;
+	struct strbuf sb = STRBUF_INIT;
+	int ret;
 
 	files_assert_main_repository(refs, "lock_packed_refs");
 
@@ -2177,10 +2179,13 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 		timeout_configured = 1;
 	}
 
-	if (hold_lock_file_for_update_timeout(
-			    &packlock, git_path("packed-refs"),
-			    flags, timeout_value) < 0)
+	strbuf_git_path(&sb, "packed-refs");
+	ret = hold_lock_file_for_update_timeout(&packlock, sb.buf,
+						flags, timeout_value);
+	strbuf_release(&sb);
+	if (ret < 0)
 		return -1;
+
 	/*
 	 * Get the current packed-refs while holding the lock.  If the
 	 * packed-refs file has been modified since we last read it,
@@ -2335,6 +2340,9 @@ static void try_remove_empty_parents(char *name)
 	for (q = p; *q; q++)
 		;
 	while (1) {
+		struct strbuf sb = STRBUF_INIT;
+		int ret;
+
 		while (q > p && *q != '/')
 			q--;
 		while (q > p && *(q-1) == '/')
@@ -2342,7 +2350,10 @@ static void try_remove_empty_parents(char *name)
 		if (q == p)
 			break;
 		*q = '\0';
-		if (rmdir(git_path("%s", name)))
+		strbuf_git_path(&sb, "%s", name);
+		ret = rmdir(sb.buf);
+		strbuf_release(&sb);
+		if (ret)
 			break;
 	}
 }
@@ -2431,7 +2442,11 @@ static int repack_without_refs(struct files_ref_store *refs,
 		return 0; /* no refname exists in packed refs */
 
 	if (lock_packed_refs(refs, 0)) {
-		unable_to_lock_message(git_path("packed-refs"), errno, err);
+		struct strbuf sb = STRBUF_INIT;
+
+		strbuf_git_path(&sb, "packed-refs");
+		unable_to_lock_message(sb.buf, errno, err);
+		strbuf_release(&sb);
 		return -1;
 	}
 	packed = get_packed_refs(refs);
@@ -2529,11 +2544,12 @@ static int rename_tmp_log(const char *newrefname)
 {
 	int attempts_remaining = 4;
 	struct strbuf path = STRBUF_INIT;
+	struct strbuf tmp_renamed_log = STRBUF_INIT;
 	int ret = -1;
 
- retry:
-	strbuf_reset(&path);
 	strbuf_git_path(&path, "logs/%s", newrefname);
+	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
+ retry:
 	switch (safe_create_leading_directories_const(path.buf)) {
 	case SCLD_OK:
 		break; /* success */
@@ -2546,7 +2562,7 @@ static int rename_tmp_log(const char *newrefname)
 		goto out;
 	}
 
-	if (rename(git_path(TMP_RENAMED_LOG), path.buf)) {
+	if (rename(tmp_renamed_log.buf, path.buf)) {
 		if ((errno==EISDIR || errno==ENOTDIR) && --attempts_remaining > 0) {
 			/*
 			 * rename(a, b) when b is an existing
@@ -2574,6 +2590,7 @@ static int rename_tmp_log(const char *newrefname)
 	ret = 0;
 out:
 	strbuf_release(&path);
+	strbuf_release(&tmp_renamed_log);
 	return ret;
 }
 
@@ -2614,9 +2631,15 @@ static int files_rename_ref(struct ref_store *ref_store,
 	int flag = 0, logmoved = 0;
 	struct ref_lock *lock;
 	struct stat loginfo;
-	int log = !lstat(git_path("logs/%s", oldrefname), &loginfo);
+	struct strbuf sb_oldref = STRBUF_INIT;
+	struct strbuf sb_newref = STRBUF_INIT;
+	struct strbuf tmp_renamed_log = STRBUF_INIT;
+	int log, ret;
 	struct strbuf err = STRBUF_INIT;
 
+	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	log = !lstat(sb_oldref.buf, &loginfo);
+	strbuf_release(&sb_oldref);
 	if (log && S_ISLNK(loginfo.st_mode))
 		return error("reflog for %s is a symlink", oldrefname);
 
@@ -2630,7 +2653,12 @@ static int files_rename_ref(struct ref_store *ref_store,
 	if (!rename_ref_available(oldrefname, newrefname))
 		return 1;
 
-	if (log && rename(git_path("logs/%s", oldrefname), git_path(TMP_RENAMED_LOG)))
+	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
+	ret = log && rename(sb_oldref.buf, tmp_renamed_log.buf);
+	strbuf_release(&sb_oldref);
+	strbuf_release(&tmp_renamed_log);
+	if (ret)
 		return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s",
 			oldrefname, strerror(errno));
 
@@ -2709,13 +2737,19 @@ static int files_rename_ref(struct ref_store *ref_store,
 	log_all_ref_updates = flag;
 
  rollbacklog:
-	if (logmoved && rename(git_path("logs/%s", newrefname), git_path("logs/%s", oldrefname)))
+	strbuf_git_path(&sb_newref, "logs/%s", newrefname);
+	strbuf_git_path(&sb_oldref, "logs/%s", oldrefname);
+	if (logmoved && rename(sb_newref.buf, sb_oldref.buf))
 		error("unable to restore logfile %s from %s: %s",
 			oldrefname, newrefname, strerror(errno));
+	strbuf_git_path(&tmp_renamed_log, TMP_RENAMED_LOG);
 	if (!logmoved && log &&
-	    rename(git_path(TMP_RENAMED_LOG), git_path("logs/%s", oldrefname)))
+	    rename(tmp_renamed_log.buf, sb_oldref.buf))
 		error("unable to restore logfile %s from "TMP_RENAMED_LOG": %s",
 			oldrefname, strerror(errno));
+	strbuf_release(&sb_newref);
+	strbuf_release(&sb_oldref);
+	strbuf_release(&tmp_renamed_log);
 
 	return 1;
 }
@@ -3111,22 +3145,32 @@ int set_worktree_head_symref(const char *gitdir, const char *target)
 static int files_reflog_exists(struct ref_store *ref_store,
 			       const char *refname)
 {
+	struct strbuf sb = STRBUF_INIT;
 	struct stat st;
+	int ret;
 
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "reflog_exists");
 
-	return !lstat(git_path("logs/%s", refname), &st) &&
-		S_ISREG(st.st_mode);
+	strbuf_git_path(&sb, "logs/%s", refname);
+	ret = !lstat(sb.buf, &st) && S_ISREG(st.st_mode);
+	strbuf_release(&sb);
+	return ret;
 }
 
 static int files_delete_reflog(struct ref_store *ref_store,
 			       const char *refname)
 {
+	struct strbuf sb = STRBUF_INIT;
+	int ret;
+
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "delete_reflog");
 
-	return remove_path(git_path("logs/%s", refname));
+	strbuf_git_path(&sb, "logs/%s", refname);
+	ret = remove_path(sb.buf);
+	strbuf_release(&sb);
+	return ret;
 }
 
 static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
@@ -3181,7 +3225,9 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "for_each_reflog_ent_reverse");
 
-	logfp = fopen(git_path("logs/%s", refname), "r");
+	strbuf_git_path(&sb, "logs/%s", refname);
+	logfp = fopen(sb.buf, "r");
+	strbuf_release(&sb);
 	if (!logfp)
 		return -1;
 
@@ -3287,7 +3333,9 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "for_each_reflog_ent");
 
-	logfp = fopen(git_path("logs/%s", refname), "r");
+	strbuf_git_path(&sb, "logs/%s", refname);
+	logfp = fopen(sb.buf, "r");
+	strbuf_release(&sb);
 	if (!logfp)
 		return -1;
 
@@ -3369,12 +3417,15 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
 {
 	struct files_reflog_iterator *iter = xcalloc(1, sizeof(*iter));
 	struct ref_iterator *ref_iterator = &iter->base;
+	struct strbuf sb = STRBUF_INIT;
 
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "reflog_iterator_begin");
 
 	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);
-	iter->dir_iterator = dir_iterator_begin(git_path("logs"));
+	strbuf_git_path(&sb, "logs");
+	iter->dir_iterator = dir_iterator_begin(sb.buf);
+	strbuf_release(&sb);
 	return ref_iterator;
 }
 
@@ -3843,8 +3894,13 @@ static int files_transaction_commit(struct ref_store *ref_store,
 		ret = TRANSACTION_GENERIC_ERROR;
 		goto cleanup;
 	}
-	for_each_string_list_item(ref_to_delete, &refs_to_delete)
-		unlink_or_warn(git_path("logs/%s", ref_to_delete->string));
+	for_each_string_list_item(ref_to_delete, &refs_to_delete) {
+		struct strbuf sb = STRBUF_INIT;
+
+		strbuf_git_path(&sb, "logs/%s", ref_to_delete->string);
+		unlink_or_warn(sb.buf);
+		strbuf_release(&sb);
+	}
 	clear_loose_ref_cache(refs);
 
 cleanup:
@@ -4098,18 +4154,28 @@ static int files_reflog_expire(struct ref_store *ref_store,
 
 static int files_init_db(struct ref_store *ref_store, struct strbuf *err)
 {
+	struct strbuf sb = STRBUF_INIT;
+
 	/* Check validity (but we don't need the result): */
 	files_downcast(ref_store, 0, "init_db");
 
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
-	safe_create_dir(git_path("refs/heads"), 1);
-	safe_create_dir(git_path("refs/tags"), 1);
+	strbuf_git_path(&sb, "refs/heads");
+	safe_create_dir(sb.buf, 1);
+	strbuf_reset(&sb);
+	strbuf_git_path(&sb, "refs/tags");
+	safe_create_dir(sb.buf, 1);
+	strbuf_reset(&sb);
 	if (get_shared_repository()) {
-		adjust_shared_perm(git_path("refs/heads"));
-		adjust_shared_perm(git_path("refs/tags"));
+		strbuf_git_path(&sb, "refs/heads");
+		adjust_shared_perm(sb.buf);
+		strbuf_reset(&sb);
+		strbuf_git_path(&sb, "refs/tags");
+		adjust_shared_perm(sb.buf);
 	}
+	strbuf_release(&sb);
 	return 0;
 }
 
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 01/11] refs-internal.c: make files_log_ref_write() static
From: Nguyễn Thái Ngọc Duy @ 2017-02-13 15:20 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Michael Haggerty, Johannes Schindelin, sbeller,
	novalis, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170213152011.12050-1-pclouds@gmail.com>

Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10)
but probably never used outside refs-internal.c

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index cdb6b8ff5..75565c3aa 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -165,6 +165,9 @@ static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
 					  const char *dirname, size_t len,
 					  int incomplete);
 static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
+static int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+			       const unsigned char *new_sha1, const char *msg,
+			       int flags, struct strbuf *err);
 
 static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 33adbf93b..59e65958a 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -222,10 +222,6 @@ struct ref_transaction {
 	enum ref_transaction_state state;
 };
 
-int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
-			const unsigned char *new_sha1, const char *msg,
-			int flags, struct strbuf *err);
-
 /*
  * Check for entries in extras that are within the specified
  * directory, where dirname is a reference directory name including
-- 
2.11.0.157.gd943d85


^ permalink raw reply related


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