Re: [PATCH v2 2/2] [Outreachy] stash: tolerate missing user identity
Slavica Djukic writes:
> git-stash.sh | 17 +
> t/t3903-stash.sh | 2 +-
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 94793c1a9..789ce2f41 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -55,6 +55,20 @@ untracked_files () {
> git ls-files -o $z $excl_opt -- "$@"
> }
>
> +prepare_fallback_ident () {
> + if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null
> 2>&1
> + then
> + GIT_AUTHOR_NAME="git stash"
> + GIT_AUTHOR_EMAIL=git@stash
> + GIT_COMMITTER_NAME="git stash"
> + GIT_COMMITTER_EMAIL=git@stash
> + export GIT_AUTHOR_NAME
> + export GIT_AUTHOR_EMAIL
> + export GIT_COMMITTER_NAME
> + export GIT_COMMITTER_EMAIL
> + fi
> +}
> +
> clear_stash () {
> if test $# != 0
> then
> @@ -67,6 +81,9 @@ clear_stash () {
> }
>
> create_stash () {
> +
> + prepare_fallback_ident
> +
> stash_msg=
> untracked=
> while test $# != 0
That looks like a sensible implementation to me.
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index bab8bec67..0b0814421 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -1096,7 +1096,7 @@ test_expect_success 'stash -- works with
> binary files' '
> test_path_is_file subdir/untracked
> '
>
> -test_expect_failure 'stash works when user.name and user.email are not set' '
> +test_expect_success 'stash works when user.name and user.email are not set' '
This line claims to the readers of patch that the known breakage
this known test piece demonstrated has been corrected, but they need
to refresh their memory by going back to the previous patch to see
if this "failure-to-success" flipping is done to the right test
piece, and what exactly the test piece tested to see the existing
breakage, because all the interesting part of the test are chomped
outside the post-context of this hunk.
Unless the fix is fairly complex, adding ought-to-succeed tests that
expect success that break when the code change gets omitted from the
patch in the same patch as the fix itself (i.e. squash patch 1/2 and
patch 2/2 into a single patch) would be more helpful for the readers
(it also helps cherry-picking the fix later to earlier maintenance
tracks if it becomes necessary).
> git reset &&
> git var GIT_COMMITTER_IDENT >expected &&
> >1 &&
[PATCH v2 2/2] [Outreachy] stash: tolerate missing user identity
The "git stash" command insists on having a usable user identity to
the same degree as the "git commit-tree" and "git commit" commands
do, because it uses the same codepath that creates commit objects
as these commands.
It is not strictly necesary to do so. Check if we will barf before
creating commit objects and then supply fake identity to please the
machinery that creates commits.
This is not that much of usability improvement, as the users who run
"git stash" would eventually want to record their changes that are
temporarily stored in the stashes in a more permanent history by
committing, and they must do "git config user.{name,email}" at that
point anyway, so arguably this change is only delaying a step that
is necessary to work in the repository.
Helped-by: Junio C Hamano
Signed-off-by: Slavica Djukic
---
git-stash.sh | 17 +
t/t3903-stash.sh | 2 +-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index 94793c1a9..789ce2f41 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -55,6 +55,20 @@ untracked_files () {
git ls-files -o $z $excl_opt -- "$@"
}
+prepare_fallback_ident () {
+ if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null
2>&1
+ then
+ GIT_AUTHOR_NAME="git stash"
+ GIT_AUTHOR_EMAIL=git@stash
+ GIT_COMMITTER_NAME="git stash"
+ GIT_COMMITTER_EMAIL=git@stash
+ export GIT_AUTHOR_NAME
+ export GIT_AUTHOR_EMAIL
+ export GIT_COMMITTER_NAME
+ export GIT_COMMITTER_EMAIL
+ fi
+}
+
clear_stash () {
if test $# != 0
then
@@ -67,6 +81,9 @@ clear_stash () {
}
create_stash () {
+
+ prepare_fallback_ident
+
stash_msg=
untracked=
while test $# != 0
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index bab8bec67..0b0814421 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1096,7 +1096,7 @@ test_expect_success 'stash -- works with binary
files' '
test_path_is_file subdir/untracked
'
-test_expect_failure 'stash works when user.name and user.email are not set' '
+test_expect_success 'stash works when user.name and user.email are not set' '
git reset &&
git var GIT_COMMITTER_IDENT >expected &&
>1 &&
--
2.19.1.1052.gd166e6afe
