Re: [PATCH] shallow.c: use 'reset_repository_shallow' when appropriate

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 21, 2020, 20:52 UTC
Message-ID: <xmqq5zdsmuht.fsf@gitster.c.googlers.com>
In-Reply-To: <20200421204516.GA69777@syl.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 12 quoted lines

>> > @@ -414,6 +414,7 @@ void prune_shallow(unsigned options)
>> >  	} else {
>> >  		unlink(git_path_shallow(the_repository));
>> >  		rollback_lock_file(&shallow_lock);
>> > +		reset_repository_shallow(the_repository);
>> >  	}
>>
>> Here, we reset only after we realize we cannot write the updated
>> shallow file.  Intended?
>
> Yes, see this earlier discussion I had about it with Jonathan:
> https://lore.kernel.org/git/20200416020509.225014-1-jonathantanmy@google.com/.

I did, and then I asked the question, because I couldn't quite get if JTan was asking a question similar to the one he asked earlier in the message ("do you need a reset in the "else" branch as well?"), or if he was saying what he sees there, "the opposite case", was good.

Also, I was sort-of reacting to """In any case, I think the commit message should discuss why reset_repository_shallow() is added only on the unlink+rollback side in one "if" statement, but only on the opposite "commit" side in another "if" statement.""" in that message.

Thanks.