While the first part is more of preventing faults. This part fixes the
remaining faults (or introduces more faults, who knows). I think I
have got it to a readable/testable state.
On top of nd/setup, obviously.
Nguyễn Thái Ngọc Duy (16):
Move enter_repo() to setup.c
enter_repo(): initialize other variables as setup_git_directory_gently() does
rev-parse --git-dir: print relative gitdir correctly
worktree setup: call set_git_dir explicitly
Add git_config_early()
Preparation
Use git_config_early() instead of git_config() during repo setup
worktree setup: restore original state when things go wrong
init/clone: turn on startup->have_repository properly
Improve setup stuff
git_config(): do not read .git/config if there is no repository
Do not read .git/info/exclude if there is no repository
Do not read .git/info/attributes if there is no repository
apply: do not check sha1 when repository has not been found
config: do not read .git/config if there is no repository
Stop improper access to repo (and incorrectly set git_dir
along the way)
Allow to undo setup_git_directory_gently() gracefully (and fix alias code)
alias: keep repository found while collecting aliases as long as possible
Alias fix/improvement. I think I can add some tests for this.
Guard unallowed access to repository when it's not set up
The original patch [1] that has grown up to a 37-patch, 2-part series
[1] http://mid.gmane.org/1265370468-6147-1-git-send-email-pclouds@gmail.com
attr.c | 5 +-
builtin/apply.c | 2 +-
builtin/clone.c | 3 +-
builtin/config.c | 9 ++-
builtin/init-db.c | 10 ++-
builtin/rev-parse.c | 8 ++
cache.h | 6 ++-
config.c | 22 +++++--
dir.c | 8 ++-
environment.c | 33 ++++++++-
git.c | 22 ++++---
path.c | 91 -----------------------
setup.c | 184 ++++++++++++++++++++++++++++++++++++++++++++---
t/t1300-repo-config.sh | 14 ++++
t/t1302-repo-version.sh | 2 +-
t/t7002-grep.sh | 24 ++++++
16 files changed, 309 insertions(+), 134 deletions(-)
When git_dir is relative, it is relative to Git's current working
directory, which is worktree top directory. "git rev-parse --git-dir"
is expected to output relative to user's current working directory.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/rev-parse.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
@@ -646,6 +646,14 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)staticcharcwd[PATH_MAX];intlen;if(gitdir){+if(prefix&&!is_absolute_path(gitdir)){+intlen;+if(!getcwd(cwd,PATH_MAX))+die_errno("unable to get current working directory");+len=strlen(cwd);+printf("%s%s%s\n",cwd,len&&cwd[len-1]!='/'?"/":"",gitdir);+continue;+}puts(gitdir);continue;}
This version of git_config() will be used during repository setup.
As a repository is being set up, $GIT_DIR is not nailed down yet,
git_pathdup() should not be used to get $GIT_DIR/config.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 1 +
config.c | 19 ++++++++++++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
@@ -699,10 +699,9 @@ int git_config_global(void)return!git_env_bool("GIT_CONFIG_NOGLOBAL",0);}-intgit_config(config_fn_tfn,void*data)+intgit_config_early(config_fn_tfn,void*data,constchar*repo_config){intret=0,found=0;-char*repo_config=NULL;constchar*home=NULL;/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
@@ -724,17 +723,27 @@ int git_config(config_fn_t fn, void *data)free(user_config);}-repo_config=git_pathdup("config");-if(!access(repo_config,R_OK)){+if(repo_config&&!access(repo_config,R_OK)){ret+=git_config_from_file(fn,repo_config,data);found+=1;}-free(repo_config);if(found==0)return-1;returnret;}+intgit_config(config_fn_tfn,void*data)+{+char*repo_config=NULL;+intret;++repo_config=git_pathdup("config");+ret=git_config_early(fn,data,repo_config);+if(repo_config)+free(repo_config);+returnret;+}+/**Findallthestuffforgit_config_set()below.*/
If no repository is found, do not bother calling git_pathdup(). If a
command forgets to call setup_git_directory*() or enter_repo(),
$GIT_DIR/config will be missed, though.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
config.c | 3 ++-
t/t1300-repo-config.sh | 13 +++++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
@@ -824,4 +824,17 @@ test_expect_success 'check split_cmdline return' "test_must_failgitmergemaster"+test_expect_success'skip .git/config if there is no repository''+(+mkdir-pa/b/.git&&+cda&&+GIT_CEILING_DIRECTORIES="`pwd`"&&+exportGIT_CEILING_DIRECTORIES&&+cdb&&+echo"[core]">.git/config&&+echo"wrong = true">>.git/config&&+test-z"$(gitvar-l|grepcore.wrong)"+)+'+ test_done
With startup_info != NULL, many code path may be disabled, depending
on repo setup. Also move set_git_dir() closer to have_repository
assignment to make it clear about repo setup.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/clone.c | 3 +--
builtin/init-db.c | 9 +++++----
cache.h | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
@@ -460,9 +460,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)if(safe_create_leading_directories_const(git_dir)<0)die("could not create leading directories of '%s'",git_dir);-set_git_dir(make_absolute_path(git_dir));-init_db(option_template,option_quiet?INIT_DB_QUIET:0);+init_db(git_dir,option_template,option_quiet?INIT_DB_QUIET:0);/**Atthispoint,theconfigexists,sowedonotneedthe
This one is difficult to demonstrate by tests. So I'll describe the
call flow instead.
Say we do "git grep --no-index --show-function foo -- bar" in no
repository. The call flow would be:
- ...
- cmd_grep
- grep_directory
- grep_file
- grep_buffer
- grep_buffer_1
- userdiff_find_by_path
- git_checkattr("diff")
- bootstrap_attr_stack
- git_path("info/attributes")
Because no repository is found, git_dir in environment.c would be
NULL. When it reaches git_path("info/attributes"), it will
automatically set git_dir to ".git". If we have $(cwd)/.git/info/attributes
then that file will be read, no matter $(cwd)/.git is a valid
repository.
This bug is hard to be exposed, because after git_checkattr("diff")
finds something (wrong). It will look up for diff drivers from
.git/config. If we do thing correctly, .git/config will not be read,
thus no diff drivers, no visible impact. If .git/config is read,
git_dir must be set already, all this would not happen.
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
attr.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
@@ -833,7 +833,8 @@ test_expect_success 'skip .git/config if there is no repository' 'cdb&&echo"[core]">.git/config&&echo"wrong = true">>.git/config&&-test-z"$(gitvar-l|grepcore.wrong)"+test-z"$(gitvar-l|grepcore.wrong)"&&+test-z"$(gitconfig--boolcore.wrong)")'
@@ -29,7 +29,7 @@ test_expect_success 'gitdir selection on normal repos' '# Make sure it would stop at test2, not trash test_expect_success'gitdir selection on unsupported repo''(cdtest2&&-test"$(gitconfigcore.repositoryformatversion)"=99)'+test"$(gitconfig--file=.git/configcore.repositoryformatversion)"=99)' test_expect_success'gitdir not required mode''(gitapply--stattest.patch&&
unset_git_directory() can only clean up things as long as
set_git_dir() has not been called because set_git_dir() keeps internal
state itself. Even worse, set_git_dir() may override $GIT_DIR env
variable. All those are now handled by unset_git_env().
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
cache.h | 1 +
environment.c | 20 ++++++++++++++++++++
git.c | 11 +++++------
setup.c | 2 ++
4 files changed, 28 insertions(+), 6 deletions(-)
@@ -184,6 +199,11 @@ char *get_graft_file(void)intset_git_dir(constchar*path){+staticintoriginal_git_dir_set=0;+if(!original_git_dir_set){+original_git_dir=getenv(GIT_DIR_ENVIRONMENT);+original_git_dir_set=1;+}if(setenv(GIT_DIR_ENVIRONMENT,path,1))returnerror("Could not set GIT_DIR to '%s'",path);setup_git_env();
@@ -146,14 +146,13 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)staticinthandle_alias(int*argcp,constchar***argv){intenvchanged=0,ret=0,saved_errno=errno;-constchar*subdir;intcount,option_count;constchar**new_argv;constchar*alias_command;char*alias_string;intunused_nongit;-subdir=setup_git_directory_gently(&unused_nongit);+setup_git_directory_gently(&unused_nongit);alias_command=(*argv)[0];alias_string=alias_lookup(alias_command);
@@ -210,8 +209,7 @@ static int handle_alias(int *argcp, const char ***argv)ret=1;}-if(subdir&&chdir(subdir))-die_errno("Cannot change to '%s'",subdir);+unset_git_directory(startup_info->prefix);errno=saved_errno;
@@ -240,8 +238,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)intstatus,help;structstatst;-memset(&git_startup_info,0,sizeof(git_startup_info));-startup_info=&git_startup_info;help=argc==2&&!strcmp(argv[1],"-h");if(!help){if(p->option&RUN_SETUP)
@@ -486,6 +482,9 @@ int main(int argc, const char **argv){constchar*cmd;+memset(&git_startup_info,0,sizeof(git_startup_info));+startup_info=&git_startup_info;+cmd=git_extract_argv0_path(argv[0]);if(!cmd)cmd="git-help";
In order to read aliases, $GIT_CONFIG/config must be read (if found).
Currently, after alias handling is done, we chdir() back to original
cwd, pretending no repository setup is done.
Given plain chdir() is not a proper way to undo
setup_git_directory_gently(), and 80% builtin commands will need to
search for a repository, we could keep the repository found in alias
handling code. Until we are clear, there's no need for a repository,
then we can undo the setup with unset_git_directory().
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
git.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
Many code path will skip repo access if startup_info->have_repository
is false. This may be a fault if startup_info->have_repository has not
been properly initialized.
So the rule is one of the following commands must be run before any
repo access. And none of them can be called twice.
- setup_git_directory*
- enter_repo
- init_db
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
builtin/init-db.c | 1 +
cache.h | 1 +
config.c | 2 ++
environment.c | 13 +++++++++++--
setup.c | 13 +++++++++++++
5 files changed, 28 insertions(+), 2 deletions(-)
@@ -237,7 +237,17 @@ void setup_work_tree(void)git_dir=make_absolute_path(git_dir);if(!work_tree||chdir(work_tree))die("This operation must be run in a work tree");++/*+*have_run_setup_gitdirisunsetinordertoavoiddie()ing+*insideset_git_env().Wedon'tactuallyinitialize+*repotwice,we'rejustrelative-izinggitdir+*/+if(startup_info)+startup_info->have_run_setup_gitdir=0;set_git_dir(make_relative_path(git_dir,work_tree));+if(startup_info)+startup_info->have_run_setup_gitdir=1;initialized=1;}
Something went wrong in rebases and conflicts. gitfile_dir in the
above call should be replaced by "."
if (check_repository_format_gently(".", nongit_ok))
--
Duy