Re: [PATCH] worktree repair: detect relative path in .git file correctly

"Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com> writes:

This is ugly. What problem is this really fixing? What "conversion from a relative path to an absolute path" does the above refer to? What "dead code"? Where in what file and what function? Why does the caller even care if it is absolute or relative? Shouldn't they work equally well as long as they point at the right location?

The proposed log message hides so many details to evaluate the claim that this is a good change, and raises many unanswered questions.

Yes, read_gitfile_gently() always turns the gitfile it reads into an absolute form. Is there a caller A that wants the underlying relative form, and if so why? Is it to compare with some other path that is relative? How did the code B obtained the other path to be compared that is relative? If that code B used the helper that is different from read_gitfile_gently() to obtain the other path that is relative, perhaps the caller A can be changed to call it instead of calling read_gitfile_gently() and the fix can be done without churning so many existing call sites?

Stepping back a bit, why does "repair" even care if it is relative? Is it considered a semi-error when a gitfile records its target as a relative path? If so, I wonder if a cleaner way may be to add a new READ_GITFILE_ERR_RELATIVE_PATH constant that is treated as non-fatal error by the read_gitfile_error_die() function? If that approach works, that may be the cleanest, as I suspect that "was it recorded as an absolute path?" will not stay to be the only special case in niche applications like "repair", but we need to audit callers of the _gently() function and make sure they do not barf with the new return code.

If not, perhaps introduce a separate function that returns the path it read without any conversion, i.e.,

    char *read_raw_gitfile(const char *path);

that "repair" thing can use, and have it do the relateve-to-absolute converaion itself, perhaps? That function would be created by moving most of the code from read_gitfile_gently() and read_gitfile_gently() would become a very thin wrapper around that function. Wouldn't that be the least invasive and cleanest solution, if it works?

Thanks.