From: Tian Yuchen <a3205153416@gmail.com>
Date: Wed, 18 Feb 2026 11:11:39 GMT
Subject: Re: [PATCH v5 1/2] setup: distingush ENOENT from other stat errors
Message-ID: <85978d52-7bdc-4c10-8f8e-8c4c2a804cfd@gmail.com>
In-Reply-To: <CAOLa=ZR_tH6A6JEj7NwziwYaVtezkHMez_cZNYyU1TQi5D8=XQ@mail.gmail.com>

Hi Karthik,

Thanks for the review!

> Nit: For this function it should be okay to return early. But I was
> expecting a break here, since it was using 'break' before, ideally we
> shouldn't change it unless there is a reason to.

> Would it make more sense to do 'not a regular file: %s'?

Points taken.

> So why didn't we add the tests here for the changes made?

I think the reason is that this commit is a refactoring of the internel 
error handling. The actual logic change that triggers these new paths 
happens in the next commit (2/2). Without the logic change in the next 
patch, the system behaves identically to before, doesn't it?

> Nit: I would even go further to even separate this into two commits:
> 1. Split 'stat()' error into  ERR_STAT_FAILED and ERR_STAT_ENOENT.
> 2. Introduce 'READ_GITFILE_ERR_IS_A_DIR'.
> 
> But I'll leave that to you.
I appreciate the suggestion. But since the changes are relatively small 
and closely related, I'll stick to keeping them in this single patch to 
avoid excessive fragmentation.

Regards,

Yuchen