Subject: Re: [PATCH 2/2] commit: fix duplication regression in permission error output

From: Junio C Hamano <[email-masked]>
Date: Mon, 11 Oct 2021 13:56:18 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> So let's add and use that facility with a corresponding HASH_SILENT
> flag, its only user is cache-tree.c's update_one(), which will set it
> if its "WRITE_TREE_SILENT" flag is set.

OK.

> diff --git a/cache.h b/cache.h
> index 3e5658c6dda..088f274fa17 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -887,6 +887,7 @@ int ie_modified(struct index_state *, const struct cache_entry *, struct stat *,
>  #define HASH_WRITE_OBJECT 1
>  #define HASH_FORMAT_CHECK 2
>  #define HASH_RENORMALIZE  4
> +#define HASH_SILENT 8

Nice to see this done in a straight-forward way, instead of being a
single step buried in a multi-step series whose early part converts
preprocessor constants into enum and other "clean-ups" that are not
essential (and can be done as a separate series when the codebase
around the area is quiescent).

>  static int write_loose_object(const struct object_id *oid, char *hdr,
>  			      int hdrlen, const void *buf, unsigned long len,
> -			      time_t mtime)
> +			      time_t mtime, unsigned flags)
>  {
>  	int fd, ret;
>  	unsigned char compressed[4096];
> @@ -1887,7 +1887,9 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
>  
>  	fd = create_tmpfile(&tmp_file, filename.buf);
>  	if (fd < 0) {
> -		if (errno == EACCES)
> +		if (flags & HASH_SILENT)
> +			return -1;

OK.

> diff --git a/t/t0004-unwritable.sh b/t/t0004-unwritable.sh
> index 998c9d1be69..75c6d5c8b49 100755
> --- a/t/t0004-unwritable.sh
> +++ b/t/t0004-unwritable.sh
> @@ -33,7 +33,6 @@ test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository
>  
>  	cat >expect <<-\EOF &&
>  	error: insufficient permission for adding an object to repository database .git/objects
> -	error: insufficient permission for adding an object to repository database .git/objects
>  	error: Error building trees
>  	EOF

Again, I think it makes it more clear if the test starts out as
expecting a failure and gets turned into expecting a success as a
part of this fix.

Thanks.