Junio C Hamano <gitster@xxxxxxxxx> writes:
> ... I am not sure about this part. Do we have any other codepath to
> show "to debug, run the program with this" suggestion? Adding it in
> the documentation is probably good, but this is an extra message
> that is much larger than the "owned by X but you are Y" message that
> would be shown. With or without the environment set, the output
> will become noisier with this patch. I wonder if we are better off
> giving the information that is given in the warning (in compat/ part
> of the patch) _unconditionally_ in the message, which would make it
> less noisy overall.
I am wondering if passing a struct to allow is_path_owned_by*()
helper(s) to report the detail of the failures they discover a
cleaner way to do this. To illustrate what I meant, along the
lines of the following patch, with any additional code to actually
stuff messages in the strbuf report, in the is_path_owned_by*()
implementation.
I am perfectly OK if we make it a debug-only option by hiding this
behind an environment variable, but if we were to do so, I do not
want to see us advertize the environment variable in the die()
message (a debug-only option can be documented, but not worth
surfacing in and contaminating the usual UI output).
Comments?
compat/mingw.c | 2 +-
git-compat-util.h | 3 ++-
setup.c | 12 +++++++++---
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git c/compat/mingw.c w/compat/mingw.c
index 2607de93af..f12b7df16d 100644
--- c/compat/mingw.c
+++ w/compat/mingw.c
@@ -2673,7 +2673,7 @@ static PSID get_current_user_sid(void)
return result;
}
-int is_path_owned_by_current_sid(const char *path)
+int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
{
WCHAR wpath[MAX_PATH];
PSID sid = NULL;
diff --git c/git-compat-util.h w/git-compat-util.h
index 58d7708296..de34b0ea7e 100644
--- c/git-compat-util.h
+++ w/git-compat-util.h
@@ -487,7 +487,8 @@ static inline void extract_id_from_env(const char *env, uid_t *id)
}
}
-static inline int is_path_owned_by_current_uid(const char *path)
+struct strbuf;
+static inline int is_path_owned_by_current_uid(const char *path, struct strbuf *report)
{
struct stat st;
uid_t euid;
diff --git c/setup.c w/setup.c
index 09b6549ba9..ed823585f7 100644
--- c/setup.c
+++ w/setup.c
@@ -1143,12 +1143,18 @@ static int ensure_valid_ownership(const char *gitfile,
struct safe_directory_data data = {
.path = worktree ? worktree : gitdir
};
+ struct strbuf report = STRBUF_INIT;
if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
- (!gitfile || is_path_owned_by_current_user(gitfile)) &&
- (!worktree || is_path_owned_by_current_user(worktree)) &&
- (!gitdir || is_path_owned_by_current_user(gitdir)))
+ (!gitfile || is_path_owned_by_current_user(gitfile, &report)) &&
+ (!worktree || is_path_owned_by_current_user(worktree, &report)) &&
+ (!gitdir || is_path_owned_by_current_user(gitdir, &report))) {
+ if (report.len) {
+ fputs(report.buf, stderr);
+ strbuf_release(&report);
+ }
return 1;
+ }
/*
* data.path is the "path" that identifies the repository and it is
