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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 11, 2021, 20:56 UTC
Message-ID: <xmqq1r4rb7rh.fsf@gitster.g>
In-Reply-To: <patch-2.2-56b20f6024b-20211009T133354Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> 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.

Show 9 quoted lines

> 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).

Show 14 quoted lines

>  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.

Show 11 quoted lines

> 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.