Re: [PATCH] stash: Fix multiple error messages on create if no HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sebastian Morr <sebastian@xxxxxxx> writes:

> This bugged me: In a new, empty repository:
>
>     $ git stash
>     fatal: bad revision 'HEAD'
>     fatal: bad revision 'HEAD'
>     fatal: Needed a single revision
>     You do not have the initial commit yet
>
> With this patch:
>
>     $ git stash
>     You do not have the initial commit yet

Yeah, that looks tidier.

> With the --quiet option, I wouldn't expect diff-index to print error
> messages.

Even with --quiet, an error is an error.

Asking for a diff with the current HEAD when you do not have one yet _is_
an error and "diff-index" is correct to report the error. The bug
(i.e. "git stash" shows that error message) is in the program that calls
diff-index when the operation does not make sense.

"Hiding error" is not desirable, especially if it is only for giving a
cleaner error message for a narrow corner case that nobody cares
(i.e. running "stash" when you do not have any commit to go back to), with
the side effect that it hides _real_ errors that may help users notice
unusual situation (e.g. corrupted index file).

Probably the right way to fix it is to check if HEAD is a valid commit
before running any command that requires to have a valid HEAD (i.e.
create and save would need one; list, show, etc. would not) and give that
"You do not have the initial commit yet" message, before passing the
control to the implementations of these individual subcommands, without
touching the places you touched in this patch.

>  no_changes () {
> -	git diff-index --quiet --cached HEAD --ignore-submodules -- &&
> +	git diff-index --quiet --cached HEAD --ignore-submodules -- 2>/dev/null &&
>  	git diff-files --quiet --ignore-submodules &&
>  	(test -z "$untracked" || test -z "$(untracked_files)")
>  }
> @@ -67,7 +67,7 @@ create_stash () {
>  	fi
>  
>  	# state of the base commit
> -	if b_commit=$(git rev-parse --verify HEAD)
> +	if b_commit=$(git rev-parse --quiet --verify HEAD)
>  	then
>  		head=$(git rev-list --oneline -n 1 HEAD --)
>  	else
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]