Git development
 help / color / mirror / Atom feed
* Re: dotfiles in git template dir are not copied
From: Junio C Hamano @ 2017-02-17 22:44 UTC (permalink / raw)
  To: Grégoire PARIS; +Cc: Jeff King, git
In-Reply-To: <9894f34a-d362-7741-b5b8-3743fe8f4e0b@greg0ire.fr>

Grégoire PARIS <postmaster@greg0ire.fr> writes:

>> I do not think we should change the behaviour
>> to copy files whose names begin with a dot.
>
> So bug turned feature it is :)

There was no 'bug' either.  It's just the way it is ;-)

^ permalink raw reply

* Re: dotfiles in git template dir are not copied
From: Grégoire PARIS @ 2017-02-17 22:49 UTC (permalink / raw)
  To: Philip Oakley, Jeff King; +Cc: git
In-Reply-To: <0C79F8D069F240A6A2C5BF7D7243122F@PhilipOakley>

Le 17/02/2017 à 23:39, Philip Oakley a écrit :
> From: "Grégoire PARIS" <postmaster@greg0ire.fr>
>> > You could, for example, have your template directory itself be a git
>> repository.
>>
>> I can and I do and indeed, that might be the reason behind this.
>> I made a PR to document this : https://github.com/git/git/pull/325
>>
> While the PR is a simple one line change to the documention, the patch 
> should be submitted to the list.
> See Documenation/SubmittingPatches
>
> Don't forget to Sign Off your patch (see the Developer's Certificate 
> of Origin section).
> -- 
> Philip
>
Ok sorry, I'll fix this rightaway!
--
greg0ire

^ permalink raw reply

* [PATCH v5 3/6] stash: refactor stash_create
From: Thomas Gummerer @ 2017-02-17 22:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Matthieu Moy,
	Thomas Gummerer
In-Reply-To: <20170217224141.19183-1-t.gummerer@gmail.com>

Refactor the internal stash_create function to use a -m flag for
specifying the message and -u flag to indicate whether untracked files
should be added to the stash.

This makes it easier to pass a pathspec argument to stash_create in the
next patch.

The user interface for git stash create stays the same.

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

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..d93c47446a 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..e76741d37d 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -58,8 +58,23 @@ 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-"BUG: create_stash () -m requires an argument"}
+			;;
+		-u|--include-untracked)
+			shift
+			untracked=${1-"BUG: create_stash () -u requires an argument"}
+			;;
+		esac
+		shift
+	done
 
 	git update-index -q --refresh
 	if no_changes
@@ -268,7 +283,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 +682,7 @@ clear)
 	;;
 create)
 	shift
-	create_stash "$*" && echo "$w_commit"
+	create_stash -m "$*" && echo "$w_commit"
 	;;
 store)
 	shift
-- 
2.11.0.301.g27b9849079.dirty


^ permalink raw reply related

* [PATCH v5 1/6] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-17 22:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Matthieu Moy,
	Thomas Gummerer
In-Reply-To: <20170217224141.19183-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.g27b9849079.dirty


^ permalink raw reply related

* [PATCH v5 6/6] stash: allow pathspecs in the no verb form
From: Thomas Gummerer @ 2017-02-17 22:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Matthieu Moy,
	Thomas Gummerer
In-Reply-To: <20170217224141.19183-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.

Also make git stash -p an alias for git stash push -p.  This allows
users to use git stash -p <pathspec>.

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

diff --git a/git-stash.sh b/git-stash.sh
index 2a33614cb7..1446fbe2e8 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -667,12 +667,15 @@ apply_to_branch () {
 	}
 }
 
+test "$1" = "-p" && set "push" "$@"
+
 PARSE_CACHE='--not-parsed'
 # The default command is "push" if nothing but options are given
 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 7f90a247b4..c0ae41e724 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -872,4 +872,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.g27b9849079.dirty


^ permalink raw reply related

* [PATCH v5 5/6] stash: use stash_push for no verb form
From: Thomas Gummerer @ 2017-02-17 22:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Matthieu Moy,
	Thomas Gummerer
In-Reply-To: <20170217224141.19183-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.

Previously we allowed git stash -- -message, which is no longer allowed
after this patch.  Messages starting with a hyphen was allowed since
3c2eb80f, ("stash: simplify defaulting to "save" and reject unknown
options").  However it was never the intent to allow that, but rather it
was allowed accidentally.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 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 086c229db0..97194576ef 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 6b56f84d81..2a33614cb7 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
@@ -668,7 +668,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
@@ -678,7 +678,7 @@ do
 	esac
 done
 
-test -n "$seen_non_option" || set "save" "$@"
+test -n "$seen_non_option" || set "push" "$@"
 
 # Main command set
 case "$1" in
@@ -729,7 +729,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 4fb800eec8..7f90a247b4 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.g27b9849079.dirty


^ permalink raw reply related

* [PATCH v5 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
From: Thomas Gummerer @ 2017-02-17 22:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Matthieu Moy,
	Thomas Gummerer
In-Reply-To: <20170217224141.19183-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                       | 48 +++++++++++++++++++------
 t/t3903-stash.sh                   | 72 ++++++++++++++++++++++++++++++++++++++
 t/t3905-stash-include-untracked.sh | 26 ++++++++++++++
 4 files changed, 146 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index d93c47446a..086c229db0 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 run `git reset
 	--hard` to revert them.  The <message> part is optional and gives
@@ -55,6 +60,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 e76741d37d..6b56f84d81 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 () {
@@ -72,12 +73,16 @@ create_stash () {
 			shift
 			untracked=${1-"BUG: create_stash () -u requires an argument"}
 			;;
+		--)
+			shift
+			break
+			;;
 		esac
 		shift
 	done
 
 	git update-index -q --refresh
-	if no_changes
+	if no_changes "$@"
 	then
 		exit 0
 	fi
@@ -109,7 +114,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" &&
@@ -132,7 +137,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"
@@ -146,7 +151,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) ||
@@ -275,7 +280,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
@@ -283,18 +288,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 ffe3549ea5..4fb800eec8 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -802,4 +802,76 @@ test_expect_success 'create with multiple arguments for the 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.g27b9849079.dirty


^ permalink raw reply related

* [PATCH v5 2/6] stash: add test for the create command line arguments
From: Thomas Gummerer @ 2017-02-17 22:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski, Matthieu Moy,
	Thomas Gummerer
In-Reply-To: <20170217224141.19183-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.g27b9849079.dirty


^ permalink raw reply related

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

Thanks Matthieu, Peff and Junio for the discussion on v3 and v4.

Changes since v4:
Dropped patch 1 from the series, as it's already in master

Instead of changing the external interface to git stash create, only
refactor the internal create_stash() function to take -m and -u
arguments.  This also simplifies the internal option parsing.

Make git stash -p an alias for git stash push -p, so git stash -p
<pathspec> is allowed.

Interdiff below:

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index b0825f4aca..97194576ef 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -53,9 +53,8 @@ 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).
-	The <message> part is optional and gives
+	Save your local modifications to a new 'stash', and run `git reset
+	--hard` to revert them.  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
diff --git a/git-stash.sh b/git-stash.sh
index a184b1e274..1446fbe2e8 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -67,51 +67,20 @@ create_stash () {
 		case "$1" in
 		-m|--message)
 			shift
-			test -z ${1+x} && usage
-			stash_msg="$1"
-			new_style=t
+			stash_msg=${1-"BUG: create_stash () -m requires an argument"}
 			;;
 		-u|--include-untracked)
 			shift
-			test -z ${1+x} && usage
-			untracked="$1"
-			new_style=t
+			untracked=${1-"BUG: create_stash () -u requires an argument"}
 			;;
 		--)
 			shift
-			new_style=t
-			break
-			;;
-		*)
-			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="$*"
-		set --
-	fi
-
 	git update-index -q --refresh
 	if no_changes "$@"
 	then
@@ -698,6 +667,8 @@ apply_to_branch () {
 	}
 }
 
+test "$1" = "-p" && set "push" "$@"
+
 PARSE_CACHE='--not-parsed'
 # The default command is "push" if nothing but options are given
 seen_non_option=
@@ -740,7 +711,7 @@ clear)
 	;;
 create)
 	shift
-	create_stash "$@" && echo "$w_commit"
+	create_stash -m "$*" && echo "$w_commit"
 	;;
 store)
 	shift
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 22ac75377b..c0ae41e724 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -800,15 +800,6 @@ 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_expect_success 'stash -- <filename> stashes and restores the file' '
 	>foo &&
 	>bar &&

Thomas Gummerer (6):
  stash: introduce push verb
  stash: add test for the create command line arguments
  stash: refactor stash_create
  stash: teach 'push' (and 'create_stash') to honor pathspec
  stash: use stash_push for no verb form
  stash: allow pathspecs in the no verb form

 Documentation/git-stash.txt        |  16 ++++-
 git-stash.sh                       | 128 ++++++++++++++++++++++++++++++-------
 t/t3903-stash.sh                   | 118 +++++++++++++++++++++++++++++++++-
 t/t3905-stash-include-untracked.sh |  26 ++++++++
 4 files changed, 261 insertions(+), 27 deletions(-)

-- 
2.11.0.301.g27b9849079.dirty


^ permalink raw reply related

* Re: [PATCH] tempfile: avoid "ferror | fclose" trick
From: Junio C Hamano @ 2017-02-17 22:40 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, Andreas Schwab, Jáchym Barvínek, git
In-Reply-To: <20170217221019.wjuaxmaatqtx2olt@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> If we are trying to make sure that the caller would not say "failed
>> to close tempfile: ERRNO" with an ERRNO that is unrelated to any
>> stdio opration, I am not sure if the patch improves things.  The
>> caller did not fail to close (most likely we successfully closed
>> it), and no matter what futzing we do to errno, the message supplied
>> by such a caller will not be improved.
>
> Right. EIO is almost certainly _not_ the error we saw. But I would
> rather consistently say "I/O error" and have the user scratch their
> head, look up this thread, and say "ah, it was probably a deferred
> error", as opposed to the alternative: the user sees something
> nonsensical like ENOMEM or EBADF. Those are more misleading, and worse,
> may change from run to run based on what other code runs or fails in
> between.

My point was actually not what errno we feed to strerror().  In that
example, what is more misleading is the fixed part of the error
message the caller of close_tempfile() used after seeing the funcion
fail, i.e. "failed to close".  strerror() part is used to explain
why we "failed to close", and of course any nonsensical errno that
we did not get from the failed stdio call would not explain it, but
a more misleading part is that we did not even "failed to close" it.

We just noticed an earlier error while attempting to close it.
strerror() in the message does not even have to be related to the
closing of the file handle.

>> If the caller used "noticed an earlier error while closing tempfile:
>> ERRNO", such a message would describe the situation more correctly,
>> but then ERRNO that is not about stdio is probably acceptable in the
>> context of that message (the original ERRNO might be ENOSPC that is
>> even more specific than EIO, FWIW).  So I am not sure if the things
>> will improve from the status quo.
>
> Yes, that's I suggested that xfclose() is probably not a good direction.
> The _best_ thing we can do is have the caller not report errno at all
> (or even say "there was an earlier error, I have no idea what errno
> was"). And xfclose() works in the opposite direction.

I think we are in agreement on this point ;-)

> The only reason I do not think we should do so for close_tempfile() is
> that the fclose is typically far away from the code that actually calls
> error(). We'd have to pass the tristate (success, fail, fail-with-errno)
> state up through the stack (most of the calls indirectly come from
> commit_lock_file(), I would think).

We _could_ clear errno to allow caller to tell them apart, though,
if we wanted to ;-)

^ permalink raw reply

* Re: Re: dotfiles in git template dir are not copied
From: Philip Oakley @ 2017-02-17 22:39 UTC (permalink / raw)
  To: Jeff King, Grégoire PARIS; +Cc: git
In-Reply-To: <2bae8d8a-f0bf-fa8b-8ce4-6880d3490b43@greg0ire.fr>

From: "Grégoire PARIS" <postmaster@greg0ire.fr>
> > You could, for example, have your template directory itself be a git
> repository.
>
> I can and I do and indeed, that might be the reason behind this.
> I made a PR to document this : https://github.com/git/git/pull/325
>
While the PR is a simple one line change to the documention, the patch 
should be submitted to the list.
See Documenation/SubmittingPatches

Don't forget to Sign Off your patch (see the Developer's Certificate of 
Origin section).
--
Philip 


^ permalink raw reply

* Re: dotfiles in git template dir are not copied
From: Grégoire PARIS @ 2017-02-17 22:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqmvdkqxfd.fsf@gitster.mtv.corp.google.com>

 > I do not think we should change the behaviour
 > to copy files whose names begin with a dot.

So bug turned feature it is :)

I amended my commit message accordingly.

In my case, my template dir is not at the root of the repository where 
it is versioned, so it would not be a problem for me, but I think some 
people might use this feature to add tests/docs without having them copied.

my template dir repository : https://github.com/greg0ire/git_template

^ permalink raw reply

* Re: git alias for options
From: hIpPy @ 2017-02-17 22:34 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Ævar Arnfjörð Bjarmason,
	Git Mailing List
In-Reply-To: <xmqq4lzsqwjy.fsf@gitster.mtv.corp.google.com>

I think the conversation has drifted away from what I am asking / hoping for.

I apologize but I do not feel in the position of submitting patches
yet. I'll first need to read some code first before that.

I'm coming from the angle where currently I can alias just the command
(ex: l for log) not considering the 'command with options'.

[alias]
  l = log

Currently, the order is as follows:

If built-in git command then it wins.
If custom git command (script as git-l) then it wins.
If alias for git command then it wins.
Else throw as unknown command.

In short, built-in git command > custom git command > git alias.

So I think, the same could also be used for options.

Say I want an alias for option --name-status as -s, so I can type:
$ git log -s

But there is already a -s option and that wins so the built-in option
alias wins.

However, I think I should be able alias it as --ns.
$ git log --ns


Thanks,
hippy

^ permalink raw reply

* Re: git alias for options
From: Junio C Hamano @ 2017-02-17 22:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, hIpPy, Git Mailing List
In-Reply-To: <20170217221317.e5kby2jwutdznnlk@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Feb 17, 2017 at 11:10:08PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> The most gentle first step would be to try to turn the existing config
>> options where you can override cli-options into some declarative thing
>> from the current ad-hoc code we have for each one.
>> 
>> That would be no change in behavior, but would make it easier to
>> migrate more things in the future.
>
> Yeah, I'd agree with that. It does not change anything for the users,
> but it makes the implementation less annoying.

Yup, as long as that declarative thing (presumably it would hook
into parse-options that is already sort of declarative) allows some
command line options not overridable with configuration, I think it
would be OK.

I do not think anybody wants to see "reset --hard" and turn it into
"[reset] hard = yes" configuration, and we should not even allow for
that.


^ permalink raw reply

* Git bisect does not find commit introducing the bug
From: Alex Hoffman @ 2017-02-17 22:29 UTC (permalink / raw)
  To: git

Hi there,

According to the documentation "git bisect" is designed "to find the
commit that introduced a bug" .
I have found a situation in which it does not returns the commit I expected.
In order to reproduce the problem:

1. mkdir test; cd test;
git clone https://github.com/entbugger/git-bisect-issue.git
cd git-bisect-issue

The tag "v.bad" is one bad version and tag "v.good" is the first good
version. A good version is one having the line "FEATURE2" in
file1.txt, which was introduced in "v.good".

2. Copy test scripts to another folder to make sure they are not
overridden by 'git bisect'
cp *.sh ../
cd ..
./search-bug-git.sh

3. Run ./search-bug-git.sh to search for the commit introducing the
bug. It finds that the commit 04c6f4b9ed with the message "Feature 1"
is the first one introducing the bug.

First of all this is confusing, as this commit cannot be reached
starting from "v.good". Then I expected the commit with the message
"Introduce bug" to be returned by 'git bisect', as it is the first
commit  between "v.good" and "v.bad" that does not contain the line
"FEATURE2" in file1.txt, which is what I understand by the commit
"that introduced a bug" (cited from the manpage of git bisect).

Thanks for looking to this,

Best regards,
VG

^ permalink raw reply

* Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Mike Crowe @ 2017-02-17 22:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqr32wqxr6.fsf@gitster.mtv.corp.google.com>

On Friday 17 February 2017 at 14:05:17 -0800, Junio C Hamano wrote:
> Mike Crowe <mac@mcrowe.com> writes:
> 
> > If "git diff --quiet" finds it necessary to compare actual file contents,
> > and a file requires CRLF conversion, then it incorrectly exits with an exit
> > code of 1 even if there have been no changes.
> >
> > The patch below adds a test file that shows the problem.
> 
> If "git diff" does not show any output and "git diff --exit-code" or
> "git diff --quiet" says there are differences, then it is a bug.
> 
> I would however have expected that any culprit would involve a code
> that says "under QUICK option, we do not have to bother doing
> this".  The part you quoted:
> 
> > 	if (!DIFF_FILE_VALID(p->one) || /* (1) */
> > 	    !DIFF_FILE_VALID(p->two) ||
> > 	    (p->one->oid_valid && p->two->oid_valid) ||
> > 	    (p->one->mode != p->two->mode) ||
> > 	    diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||
> > 	    diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||
> > 	    (p->one->size != p->two->size) ||
> > 	    !diff_filespec_is_identical(p->one, p->two)) /* (2) */
> > 		p->skip_stat_unmatch_result = 1;
> 
> is used by "git diff" with and without "--quiet", afacr, so I
> suspect that the bug lies somewhere else.

I can't say that I really understand the code fully, but it appears that
the first pass generates a queue of files that contain differences. The
result of the quiet diff comes from the size of that queue,
diff_queued_diff.nr, being non-zero in diffcore_std. I'm assuming that the
result of the noisy diff comes from actually comparing the files.

But, I've only spent a short while looking so I may have got the wrong end
of the stick.

Thanks.

Mike.

^ permalink raw reply

* Re: body-CC-comment regression
From: Junio C Hamano @ 2017-02-17 22:22 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Johan Hovold, git, Jeff King, Kevin Daudt, Larry Finger
In-Reply-To: <vpq1suwvab9.fsf@anie.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> That approach may still constrain what those in the former camp can
>> write in the "cruft" part, like they cannot write comma or semicolon
>> as part of the "cruft", no?
>
> Right. Indeed, this may be a problem since the use of "#" for stable
> seem to include commit message, and they may contain commas.
>
> So, maybe Johan's patch is better indeed.

OK, so I'll queue that one with your Ack for now so that we won't
forget.  I guess we still want a few tests?

Thanks.

^ permalink raw reply

* Re: git alias for options
From: Ævar Arnfjörð Bjarmason @ 2017-02-17 22:10 UTC (permalink / raw)
  To: Jeff King; +Cc: hIpPy, Git Mailing List
In-Reply-To: <20170217204227.kreormjoo5lr6zu4@sigill.intra.peff.net>

On Fri, Feb 17, 2017 at 9:42 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Feb 17, 2017 at 02:50:23PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> On Fri, Feb 17, 2017 at 9:23 AM, hIpPy <hippy2981@gmail.com> wrote:
>> > Git has aliases for git commands. Is there a (an inbuilt) way to alias
>> > options? If not, what is the reason?
>>
>> This has long been on my  wishlist, because there's a lot of
>> copy/pasted logic all over Git to make git foo --whatever aliased to
>> foo.whatever in the config, but only for some options.
>>
>> It should ideally be part of something every option just supports, via
>> the getopts struct.
>>
>> However, it can't allow you to modify whatever option you want,
>> because some things like git-commit-tree should not be customized
>> based on config, it would break things that rely on the plumbing
>> commands.
>>
>> So it would have to be a whitelist for each option we allow to be
>> configured like this via the getopts struct.
>>
>> Also there are surely other edge cases, like maybe the config option
>> now doesn't 1=1 map to the name for the option in some cases, or the
>> flag should be config-able but is has no long form (which we'd like
>> for the config), then we'd want to add that etc.
>
> I think your idea is roughly equivalent in functionality to just
> allowing aliases to override command names. E.g., anything you could do
> with:
>
>   [log]
>   follow = true
>   decorate = false
>
> could be done with:
>
>   [alias]
>   log = "log --follow --no-decorate"

Indeed, exact same thing, different syntax. Mostly I like this
suggestion better, although the bad side of it is that it's not as
easy to introspect with a dump of git-config -l.

> The reason we have historically not allowed that is for the
> "commit-tree" plumbing reason you gave above. One option would be to
> relax it for a whitelist of porcelain commands. Then your whitelist at
> least only has to cover commands, and not each individual option.
>
> I think there are a lot of corner cases in that whitelist, though. A lot
> of commands serve dual porcelain/plumbing purposes. E.g., "log" and
> "tag" are commonly used by both humans and by scripts.
>
> A first step in that direction would probably be an environment variable
> to tell Git to suppress command-aliases. Scripts would set that to
> ensure a more vanilla experience. It doesn't fix _existing_ scripts, but
> if that option were introduced, then over time scripts would start to
> use it. Then eventually it would be safe(r) to introduce something like
> command aliases.

The most gentle first step would be to try to turn the existing config
options where you can override cli-options into some declarative thing
from the current ad-hoc code we have for each one.

That would be no change in behavior, but would make it easier to
migrate more things in the future.

Anyway, words are cheap. Just replied because to the extent that hIpPy
wants to work on this I thought I'd sprinkle some historical caveats
from memory. Other than that no point in keeping talking about this
without patches.

^ permalink raw reply

* Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log
From: Junio C Hamano @ 2017-02-17 22:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170217195549.z6uyy7hbbhj5avh7@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Thinking on it more, we probably _do_ want two entries. Because the
> operations are not atomic, it's possible that we may end up in a
> half-way state after the first entry is written. And when debugging such
> a case, I'd much rather see the first half of the operation logged than
> nothing at all.

Yes, that sounds right.

^ permalink raw reply

* Re: [PATCH] tempfile: avoid "ferror | fclose" trick
From: Jeff King @ 2017-02-17 22:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Michael Haggerty, Andreas Schwab, Jáchym Barvínek, git
In-Reply-To: <xmqqy3x4qyte.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 17, 2017 at 01:42:21PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Fri, Feb 17, 2017 at 01:17:06PM -0800, Junio C Hamano wrote:
> >
> >> Stepping back a bit, would this be really needed?  Even if the ferror()
> >> does not update errno, the original stdio operation that failed
> >> would have, no?
> >
> > Sure, but we have no clue what happened in between.
> 
> Hmm, so we are protecting against somebody who does "errno = 0"
> explicitly, because she knows that she's dealt with the error from
> stdio earlier?  Such a careful person would have called clearerr()
> as well, I would guess.

I'm not sure I understand what you are saying here. If somebody calls
clearerr(), our ferror() handling does not trigger at all, and do not
care what is in errno either way. They can reset errno or not when they
clearerr(), but it is not relevant.

If you are asking about somebody who sets errno to "0" and _doesn't_
call clearerr(), then I don't know what that person is trying to
accomplish. Setting errno to "0" is not the right way to clear an error.
And they certainly should not be relying on it not to get overwritten
before we make it to the final ferror()/fclose().

> So let's assume we only care about the case where some other error
> was detected and errno was updated by a system library call.

Right.

> > I think our emails crossed, but our patches are obviously quite similar.
> > My commit message maybe explains a bit more of my thinking.
> 
> Yes, but ;-)
> 
> If we are trying to make sure that the caller would not say "failed
> to close tempfile: ERRNO" with an ERRNO that is unrelated to any
> stdio opration, I am not sure if the patch improves things.  The
> caller did not fail to close (most likely we successfully closed
> it), and no matter what futzing we do to errno, the message supplied
> by such a caller will not be improved.

Right. EIO is almost certainly _not_ the error we saw. But I would
rather consistently say "I/O error" and have the user scratch their
head, look up this thread, and say "ah, it was probably a deferred
error", as opposed to the alternative: the user sees something
nonsensical like ENOMEM or EBADF. Those are more misleading, and worse,
may change from run to run based on what other code runs or fails in
between.

> If the caller used "noticed an earlier error while closing tempfile:
> ERRNO", such a message would describe the situation more correctly,
> but then ERRNO that is not about stdio is probably acceptable in the
> context of that message (the original ERRNO might be ENOSPC that is
> even more specific than EIO, FWIW).  So I am not sure if the things
> will improve from the status quo.

Yes, that's I suggested that xfclose() is probably not a good direction.
The _best_ thing we can do is have the caller not report errno at all
(or even say "there was an earlier error, I have no idea what errno
was"). And xfclose() works in the opposite direction.

The only reason I do not think we should do so for close_tempfile() is
that the fclose is typically far away from the code that actually calls
error(). We'd have to pass the tristate (success, fail, fail-with-errno)
state up through the stack (most of the calls indirectly come from
commit_lock_file(), I would think).

-Peff

^ permalink raw reply

* Re: git alias for options
From: Jeff King @ 2017-02-17 22:13 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: hIpPy, Git Mailing List
In-Reply-To: <CACBZZX7nJkRmSxTdvhgXz5Zuk0KeovSZM1D_eGqbBn7i20+ePQ@mail.gmail.com>

On Fri, Feb 17, 2017 at 11:10:08PM +0100, Ævar Arnfjörð Bjarmason wrote:

> > A first step in that direction would probably be an environment variable
> > to tell Git to suppress command-aliases. Scripts would set that to
> > ensure a more vanilla experience. It doesn't fix _existing_ scripts, but
> > if that option were introduced, then over time scripts would start to
> > use it. Then eventually it would be safe(r) to introduce something like
> > command aliases.
> 
> The most gentle first step would be to try to turn the existing config
> options where you can override cli-options into some declarative thing
> from the current ad-hoc code we have for each one.
> 
> That would be no change in behavior, but would make it easier to
> migrate more things in the future.

Yeah, I'd agree with that. It does not change anything for the users,
but it makes the implementation less annoying.

-Peff

^ permalink raw reply

* Re: dotfiles in git template dir are not copied
From: Junio C Hamano @ 2017-02-17 22:12 UTC (permalink / raw)
  To: Grégoire PARIS; +Cc: Jeff King, git
In-Reply-To: <2bae8d8a-f0bf-fa8b-8ce4-6880d3490b43@greg0ire.fr>

Grégoire PARIS <postmaster@greg0ire.fr> writes:

>> You could, for example, have your template directory itself be a git 
> repository.
>
> I can and I do and indeed, that might be the reason behind this.

An embedded .git was _not_ the reason behind the current behaviour
when we wrote it.  The primary reason is because we did not ship and
did not plan to ship anything .dot in our standard template set and
there wasn't any reason to copy what we were not going to ever ship
;-).

As a justification after the fact, "you wouldn't want to copy .git
do you?" is a good one, though, and I do not think we should change
the behaviour to copy files whose names begin with a dot.



^ permalink raw reply

* Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Junio C Hamano @ 2017-02-17 22:05 UTC (permalink / raw)
  To: Mike Crowe; +Cc: git
In-Reply-To: <20170217212633.GA24937@mcrowe.com>

Mike Crowe <mac@mcrowe.com> writes:

> If "git diff --quiet" finds it necessary to compare actual file contents,
> and a file requires CRLF conversion, then it incorrectly exits with an exit
> code of 1 even if there have been no changes.
>
> The patch below adds a test file that shows the problem.

If "git diff" does not show any output and "git diff --exit-code" or
"git diff --quiet" says there are differences, then it is a bug.

I would however have expected that any culprit would involve a code
that says "under QUICK option, we do not have to bother doing
this".  The part you quoted:

> 	if (!DIFF_FILE_VALID(p->one) || /* (1) */
> 	    !DIFF_FILE_VALID(p->two) ||
> 	    (p->one->oid_valid && p->two->oid_valid) ||
> 	    (p->one->mode != p->two->mode) ||
> 	    diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||
> 	    diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||
> 	    (p->one->size != p->two->size) ||
> 	    !diff_filespec_is_identical(p->one, p->two)) /* (2) */
> 		p->skip_stat_unmatch_result = 1;

is used by "git diff" with and without "--quiet", afacr, so I
suspect that the bug lies somewhere else.

^ permalink raw reply

* Re: [PATCH] l10n: de.po: translate 241 messages
From: Phillip Sz @ 2017-02-17 21:56 UTC (permalink / raw)
  To: Ralf Thielow, git
  Cc: Thomas Rast, Jan Krüger, Christian Stimming,
	Matthias Rüster, Magnus Görlitz
In-Reply-To: <20170217174132.8816-1-ralf.thielow@gmail.com>

Hey,

> @@ -916,17 +916,17 @@ msgstr ""
>  msgid ""
>  "The merge base %s is %s.\n"
>  "This means the first '%s' commit is between %s and [%s].\n"
>  msgstr ""
>  "Die Merge-Basis %s ist %s.\n"
>  "Das bedeutet, der erste '%s' Commit befindet sich zwischen %s und [%s]\n"

"Das bedeutet, der erste '%s' Commit befindet sich zwischen %s und [%s].\n"

Period is missing.


>  #: remote.c:2092
>  #, c-format
>  msgid "Your branch is ahead of '%s' by %d commit.\n"
>  msgid_plural "Your branch is ahead of '%s' by %d commits.\n"
> -msgstr[0] "Ihr Branch ist vor '%s' um %d Commit.\n"
> -msgstr[1] "Ihr Branch ist vor '%s' um %d Commits.\n"
> +msgstr[0] "Ihr Branch ist %2$d Commit vor '%1$s'.\n"
> +msgstr[1] "Ihr Branch ist %2$d Commits vor '%1$s'.\n"
>  

Does this "%2$d" works and why not use '%s'?

>  #: sequencer.c:840
> -#, fuzzy
>  msgid "could not read HEAD's commit message"
> -msgstr "Konnte Commit-Beschreibung nicht lesen: %s"
> +msgstr "Konnte Commit-Beschreibung von HEAD nicht lesen"
>

>  #: sequencer.c:846
> -#, fuzzy, c-format
> +#, c-format
>  msgid "cannot write '%s'"
> -msgstr "kann '%s' nicht erstellen"
> +msgstr "kann '%s' nicht schreiben"

I think we should either use "kann" or "konnte".
We have used both and maybe we should unify it? What do you think?

>  #: sequencer.c:1341
> -#, fuzzy
>  msgid "please fix this using 'git rebase --edit-todo'."
>  msgstr "Bitte beheben Sie das, indem Sie 'git rebase --edit-todo' ausführen."

Maybe: "Bitte beheben Sie dieses, indem Sie 'git rebase --edit-todo'
ausführen."

>  #: git-add--interactive.perl:1074
>  #, perl-format
>  msgid ""
>  "---\n"
>  "To remove '%s' lines, make them ' ' lines (context).\n"
>  "To remove '%s' lines, delete them.\n"
>  "Lines starting with %s will be removed.\n"
>  msgstr ""
> +"---\n"
> +"Um '%s' Zeilen zu entfernen, machen Sie aus diesen ' ' Zeilen (Kontext).\n"
> +"Um '%s' Zeilen zu entferenen, löschen Sie diese.\n"
> +"Zeilen, die mit %s beginnen, werden entfernt.\n"
>  

"Um '%s' Zeilen zu entfernen, löschen Sie diese.\n"

Anyway thanks a lot for your awesome work!

	Phillip

^ permalink raw reply

* Re: Re: dotfiles in git template dir are not copied
From: Grégoire PARIS @ 2017-02-17 21:42 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20170217204411.2yixhuazgczxmmxa@sigill.intra.peff.net>

 > You could, for example, have your template directory itself be a git 
repository.

I can and I do and indeed, that might be the reason behind this.
I made a PR to document this : https://github.com/git/git/pull/325

--
greg0ire

^ 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