Skip to content

[2.54.0] fixup! gvfs: add global command pre and post hook procs #955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dscho merged 2 commits into from
Jul 15, 2026
+37 −17
Merged

[2.54.0] fixup! gvfs: add global command pre and post hook procs #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f78ea6
fixup! gvfs: add global command pre and post hook procs
derrickstolee Jul 13, 2026
3b682fb
trace2: tolerate failed timestamp formatting
derrickstolee Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions git.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,9 +509,12 @@ static int run_post_command_hook(struct repository *r)

/*
* Only run post_command if pre_command succeeded in this process
* and we haven't attempted post_command yet.
*/
if (!run_post_hook)
return 0;
run_post_hook = 0;

@dscho dscho Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derrickstolee I think that this cannot recurse in the same process, as the caller in the atexit() handler should only be called well after the three other callers (all on the main thread):

So I wonder whether this change on its own can do anything to help the issue.

I rather suspect that the infinite loop might happen due to the post-command hook calling another Git command? If that is the case, then we'd also have to move the setenv("COMMAND_HOOK_LOCK", "false", 1); call right after the COMMAND_HOOK_LOCK guard, too, not just the run_post_hook = 0; assignment?

@derrickstolee derrickstolee Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still trying to figure out a reproducer or at least a trace of the issue, but if we get a die() between this point and the child process kickoff it may start recursing here. At least, that's my theory.

@derrickstolee derrickstolee Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repeated fatal: recursion detected in die handler wouldn't happen from just a recursion within the post-command hook calling another git process. And the user output looks like

fatal: recursion detected in die handler
fatal: recursion detected in die handler
fatal: recursion detected in die handler
fatal: recursion detected in die handler
fatal: recursion detected in die handler
fatal: recursion detected in die handler
fatal: recursion detected in die handler
fatal: recursion detected in die handler

and continues until they kill the process.

@derrickstolee derrickstolee Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dscho I did end up finding a trace that could be helpful, leading to a change in the trace2 code. I'll polish that up for upstream soon, but I'd like your feedback here.

@dscho dscho Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so the problem is a die() in the atexit() path? That does make sense to me.

@derrickstolee derrickstolee Jul 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And things are finally coming together: this happens when the atexit event is traced as part of GIT_TRACE2_EVENT, which is set for these users to an instance of the trace2collector.


lock = getenv("COMMAND_HOOK_LOCK");
if (!lock || strcmp(lock, "true"))
return 0;
Expand All @@ -520,8 +523,6 @@ static int run_post_command_hook(struct repository *r)
strvec_pushf(&opt.args, "--exit_code=%u", exit_code);
ret = run_hooks_opt(r, "post-command", &opt);

run_post_hook = 0;

errno = saved_errno;
strvec_clear(&sargv);
strvec_clear(&opt.args);
Expand Down
49 changes: 34 additions & 15 deletions trace2/tr2_tbuf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,64 @@

void tr2_tbuf_local_time(struct tr2_tbuf *tb)
{
struct timeval tv;
struct tm tm;
struct timeval tv = { 0 };
struct tm tm = { 0 };
time_t secs;
int len;

gettimeofday(&tv, NULL);
secs = tv.tv_sec;
localtime_r(&secs, &tm);

xsnprintf(tb->buf, sizeof(tb->buf), "%02d:%02d:%02d.%06ld", tm.tm_hour,
tm.tm_min, tm.tm_sec, (long)tv.tv_usec);
len = snprintf(tb->buf, sizeof(tb->buf), "%02d:%02d:%02d.%06ld",
tm.tm_hour, tm.tm_min, tm.tm_sec, (long)tv.tv_usec);

if (len < 0 || (size_t)len >= sizeof(tb->buf)) {
const char *blank = "00:00:00.000000";
strlcpy(tb->buf, blank, sizeof(tb->buf));
}
}

void tr2_tbuf_utc_datetime_extended(struct tr2_tbuf *tb)
{
struct timeval tv;
struct tm tm;
struct timeval tv = { 0 };
struct tm tm = { 0 };
time_t secs;
int len;

gettimeofday(&tv, NULL);
secs = tv.tv_sec;
gmtime_r(&secs, &tm);

xsnprintf(tb->buf, sizeof(tb->buf),
"%4d-%02d-%02dT%02d:%02d:%02d.%06ldZ", tm.tm_year + 1900,
tm.tm_mon + 1, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec,
(long)tv.tv_usec);
len = snprintf(tb->buf, sizeof(tb->buf),
"%4d-%02d-%02dT%02d:%02d:%02d.%06ldZ",
tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
tm.tm_hour, tm.tm_min, tm.tm_sec, (long)tv.tv_usec);

if (len < 0 || (size_t)len >= sizeof(tb->buf)) {
const char *blank = "1900-00-00T00:00:00.000000Z";
strlcpy(tb->buf, blank, sizeof(tb->buf));
}
}

void tr2_tbuf_utc_datetime(struct tr2_tbuf *tb)
{
struct timeval tv;
struct tm tm;
struct timeval tv = { 0 };
struct tm tm = { 0 };
time_t secs;
int len;

gettimeofday(&tv, NULL);
secs = tv.tv_sec;
gmtime_r(&secs, &tm);

xsnprintf(tb->buf, sizeof(tb->buf), "%4d%02d%02dT%02d%02d%02d.%06ldZ",
tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour,
tm.tm_min, tm.tm_sec, (long)tv.tv_usec);
len = snprintf(tb->buf, sizeof(tb->buf),
"%4d%02d%02dT%02d%02d%02d.%06ldZ",
tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
tm.tm_hour, tm.tm_min, tm.tm_sec, (long)tv.tv_usec);

if (len < 0 || (size_t)len >= sizeof(tb->buf)) {
const char *blank = "19000000T000000.000000Z";
strlcpy(tb->buf, blank, sizeof(tb->buf));
}
}
Loading