On Sat, Oct 05, 2024 at 03:30:41AM +0000, John Cai via GitGitGadget wrote: > From: John Cai <johncai86@xxxxxxxxx> > > The current code in run_builtin() passes in a repository to the builtin > based on whether cmd_struct's option flag has RUN_SETUP. > > This is incorrect, however, since some builtins that only have > RUN_SETUP_GENTLY can potentially take a repository. > setup_git_directory_gently() tells us whether or not a command is being > run inside of a repository. > > Use the output of setup_git_directory_gently() to help determine whether > or not there is a repository to pass to the builtin. If not, then we > just pass NULL. > > As part of this patch, we need to modify add to check for a NULL repo > before calling repo_git_config(), since add -h can be run outside of a > repository. > > Signed-off-by: John Cai <johncai86@xxxxxxxxx> > --- > builtin/add.c | 3 ++- > git.c | 7 ++++--- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/builtin/add.c b/builtin/add.c > index 773b7224a49..7d353077921 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -385,7 +385,8 @@ int cmd_add(int argc, > char *ps_matched = NULL; > struct lock_file lock_file = LOCK_INIT; > > - repo_config(repo, add_config, NULL); > + if (repo) > + repo_config(repo, add_config, NULL); The reason why we need to check whether the `repo` is NULL is that when using "git add -h", the RUN_SETUP flag would be converted to RUN_SETUP_GENTLY. I think this change is OK. But I wonder whether we should encapsulate the logic into the "repo_config" function. It's a little cumbersome to check whether the "repo" exists. I also feel it's a bad idea to check in the "repo_config" function because in the most time, we run commands inside the repository. So, in my view, at current, this is enough. Thanks, Jialuo
