Re: [PATCH v2 1/2] sh-setup: Write a new require_clean_work_tree function

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

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:

> Write a new require_clean_work_tree function to error out when
> unstaged changes are present in the working tree and (optionally)
> uncommitted changes in the index.
>
> Cc: Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>
> Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx>

Please don't do this in-body "Cc:"; it is meaningless.

> ---
>  git-sh-setup.sh |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 6131670..215ec33 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -145,6 +145,34 @@ require_work_tree () {
>  	die "fatal: $0 cannot be used without a working tree."
>  }
>  
> +require_clean_work_tree () {
> +	# Update the index
> +	git update-index -q --ignore-submodules --refresh
> +	err=0
> +
> +	# Disallow unstaged changes in the working tree
> +	if ! git diff-files --quiet --ignore-submodules --

What is that trailing double-dash about?

> +	then
> +		echo >&2 "cannot $1: you have unstaged changes."
> +		git diff-files --name-status -r --ignore-submodules -- >&2
> +		err=1
> +	fi
> +
> +	# Disallow uncommitted changes in the index
> +	if ! git diff-index --cached --quiet HEAD --ignore-submodules --

Do not write HEAD there that sets a wrong example; the command line
arguments are flag-options, revs, double-dash and pathspec.

Contrary to what your proposed log message says, I do not see anything
"optional" in the way how this check is done here...  What is going on?

Unfortunately we cannot judge if unconditional check is the right thing to
do without looking at the callers; why did you make this into two-patch
series?

Mental note before reviewing the second patch: do all callers want the
same "both working tree and index are spiffy clean" check?

> +	then
> +		echo >&2 "cannot $1: your index contains uncommitted changes."
> +		git diff-index --cached --name-status -r --ignore-submodules HEAD -- >&2
> +		err=1
> +	fi
> +
> +	if [ $err = 1 ]
> +	then
> +	    echo >&2 "Please commit or stash them."
> +	    exit 1
> +	fi
> +}

Mental note before reviewing the second patch: warning/error messages from
this codepath are all written without warning: or error: prefixes.
--
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]