Re: [PATCH] replace: forbid replacing an object with one of a different type

[Thomas Rast <trast@inf.ethz.ch>]

Christian Couder <chriscool@tuxfamily.org> writes:

> Users replacing an object with one of a different type were not
> prevented to do so, even if it was obvious, and stated in the doc,
> that bad things would result from doing that.
>
> To avoid mistakes, it is better to just forbid that though.
>
> The doc will be updated in a later patch.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Feel free to steal some of my other email for the commit message, to
write down for posterity that reverting would not really be a useful
step.

The patch looks good to me.

> If this patch is considered useful, I will update the doc and
> maybe add tests.
>
>  builtin/replace.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/builtin/replace.c b/builtin/replace.c
> index 59d3115..0246ab3 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
>  			  int force)
>  {
>  	unsigned char object[20], prev[20], repl[20];
> +	enum object_type obj_type, repl_type;
>  	char ref[PATH_MAX];
>  	struct ref_lock *lock;
>  
> @@ -100,6 +101,14 @@ static int replace_object(const char *object_ref, const char *replace_ref,
>  	if (check_refname_format(ref, 0))
>  		die("'%s' is not a valid ref name.", ref);
>  
> +	obj_type = sha1_object_info(object, NULL);
> +	repl_type = sha1_object_info(repl, NULL);
> +	if (obj_type != repl_type)
> +		die("Object ref '%s' is of type '%s'\n"
> +		    "while replace ref '%s' is of type '%s'.",
> +		    object_ref, typename(obj_type),
> +		    replace_ref, typename(repl_type));
> +
>  	if (read_ref(ref, prev))
>  		hashclr(prev);
>  	else if (!force)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch