Re: [PATCH v3] setup: fail if .git is not a file or directory

Hi Junio,

I admit the code logic is indeed full of flaws - the vast majority of your suggestions make more sense than the current code. I do lack experience handling this kind of call chain, but I'll keep learning #^#

Given the extensive changes required, I won't address each point you raised individually. Instead, I'll rewrite the code directly based on your proposed solution. Please reserve your review for the next patch.

 >> I have verified this with a test script covering:
 >> 1. Normal .git file
 >> 2. .git as a symlink to a directory
 >> 3. .git as a FIFO
 >> 4. .git as a symlink to a FIFO
 >> 5. .git with garbage content
 >>
 >>  setup.c | 39 +++++++++++++++++++++++++++++----------
 >>  setup.h |  3 +++
 >>  2 files changed, 32 insertions(+), 10 deletions(-)

 >There is no test addition here, though?  What am I missing?

Sorry, I didn't express myself clearly. I meant I tested it myself but never add a test script. Test script will be included in the next patch.

On the other hand, if I understand correctly, state flows should be categorized as follows:

  1. Nothing there (ENOENT) ---> ignore and go up one level
  2. Directory (IS_A_DIR) ---> check is_git_directory
  3. NOT_A_FILE ---> die
  4. *REAL* error (READ_FAILED, INVALID_FORMAT) ---> die

And I mixed 1 and 2 and covered 4 in an obscure way (!= STAT_FAILED). I don't think this code is "unrunable" but indeed the logic flow is GARBAGE. I'll fix it.

Thank you for your advice,

Regards,

Yuchen