Merge branch 'jk/die-on-bogus-worktree-late'
authorJunio C Hamano <gitster@pobox.com>
Tue, 16 Jun 2015 21:27:06 +0000 (14:27 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 16 Jun 2015 21:27:06 +0000 (14:27 -0700)
The setup code used to die when core.bare and core.worktree are set
inconsistently, even for commands that do not need working tree.

* jk/die-on-bogus-worktree-late:
setup_git_directory: delay core.bare/core.worktree errors

1  2 
setup.c
t/t1510-repo-setup.sh
diff --combined setup.c
index 863ddfd938d29e58341bd2be5dd3b7ecd4db7df2,9c32aae3020174523e9638cb605b71840c60f71d..82c0cc2a13bfeae3ec364dce2438fb4c67e46a35
+++ b/setup.c
@@@ -4,6 -4,7 +4,7 @@@
  
  static int inside_git_dir = -1;
  static int inside_work_tree = -1;
+ static int work_tree_config_is_bogus;
  
  /*
   * The input parameter must contain an absolute path, and it must already be
@@@ -140,9 -141,7 +141,9 @@@ int check_filename(const char *prefix, 
                if (arg[2] == '\0') /* ":/" is root dir, always exists */
                        return 1;
                name = arg + 2;
 -      } else if (prefix)
 +      } else if (!no_wildcard(arg))
 +              return 1;
 +      else if (prefix)
                name = prefix_filename(prefix, strlen(prefix), arg);
        else
                name = arg;
@@@ -226,36 -225,6 +227,36 @@@ void verify_non_filename(const char *pr
            "'git <command> [<revision>...] -- [<file>...]'", arg);
  }
  
 +int get_common_dir(struct strbuf *sb, const char *gitdir)
 +{
 +      struct strbuf data = STRBUF_INIT;
 +      struct strbuf path = STRBUF_INIT;
 +      const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
 +      int ret = 0;
 +      if (git_common_dir) {
 +              strbuf_addstr(sb, git_common_dir);
 +              return 1;
 +      }
 +      strbuf_addf(&path, "%s/commondir", gitdir);
 +      if (file_exists(path.buf)) {
 +              if (strbuf_read_file(&data, path.buf, 0) <= 0)
 +                      die_errno(_("failed to read %s"), path.buf);
 +              while (data.len && (data.buf[data.len - 1] == '\n' ||
 +                                  data.buf[data.len - 1] == '\r'))
 +                      data.len--;
 +              data.buf[data.len] = '\0';
 +              strbuf_reset(&path);
 +              if (!is_absolute_path(data.buf))
 +                      strbuf_addf(&path, "%s/", gitdir);
 +              strbuf_addbuf(&path, &data);
 +              strbuf_addstr(sb, real_path(path.buf));
 +              ret = 1;
 +      } else
 +              strbuf_addstr(sb, gitdir);
 +      strbuf_release(&data);
 +      strbuf_release(&path);
 +      return ret;
 +}
  
  /*
   * Test if it looks like we're at a git directory.
   */
  int is_git_directory(const char *suspect)
  {
 -      char path[PATH_MAX];
 -      size_t len = strlen(suspect);
 +      struct strbuf path = STRBUF_INIT;
 +      int ret = 0;
 +      size_t len;
 +
 +      /* Check worktree-related signatures */
 +      strbuf_addf(&path, "%s/HEAD", suspect);
 +      if (validate_headref(path.buf))
 +              goto done;
  
 -      if (PATH_MAX <= len + strlen("/objects"))
 -              die("Too long path: %.*s", 60, suspect);
 -      strcpy(path, suspect);
 +      strbuf_reset(&path);
 +      get_common_dir(&path, suspect);
 +      len = path.len;
 +
 +      /* Check non-worktree-related signatures */
        if (getenv(DB_ENVIRONMENT)) {
                if (access(getenv(DB_ENVIRONMENT), X_OK))
 -                      return 0;
 +                      goto done;
        }
        else {
 -              strcpy(path + len, "/objects");
 -              if (access(path, X_OK))
 -                      return 0;
 +              strbuf_setlen(&path, len);
 +              strbuf_addstr(&path, "/objects");
 +              if (access(path.buf, X_OK))
 +                      goto done;
        }
  
 -      strcpy(path + len, "/refs");
 -      if (access(path, X_OK))
 -              return 0;
 +      strbuf_setlen(&path, len);
 +      strbuf_addstr(&path, "/refs");
 +      if (access(path.buf, X_OK))
 +              goto done;
  
 -      strcpy(path + len, "/HEAD");
 -      if (validate_headref(path))
 -              return 0;
 -
 -      return 1;
 +      ret = 1;
 +done:
 +      strbuf_release(&path);
 +      return ret;
  }
  
  int is_inside_git_dir(void)
@@@ -327,6 -287,10 +328,10 @@@ void setup_work_tree(void
  
        if (initialized)
                return;
+       if (work_tree_config_is_bogus)
+               die("unable to set up work tree using invalid config");
        work_tree = get_git_work_tree();
        git_dir = get_git_dir();
        if (!is_absolute_path(git_dir))
        initialized = 1;
  }
  
 +static int check_repo_format(const char *var, const char *value, void *cb)
 +{
 +      if (strcmp(var, "core.repositoryformatversion") == 0)
 +              repository_format_version = git_config_int(var, value);
 +      else if (strcmp(var, "core.sharedrepository") == 0)
 +              shared_repository = git_config_perm(var, value);
 +      return 0;
 +}
 +
  static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
  {
 -      char repo_config[PATH_MAX+1];
 +      struct strbuf sb = STRBUF_INIT;
 +      const char *repo_config;
 +      config_fn_t fn;
 +      int ret = 0;
 +
 +      if (get_common_dir(&sb, gitdir))
 +              fn = check_repo_format;
 +      else
 +              fn = check_repository_format_version;
 +      strbuf_addstr(&sb, "/config");
 +      repo_config = sb.buf;
  
        /*
         * git_config() can't be used here because it calls git_pathdup()
         * Use a gentler version of git_config() to check if this repo
         * is a good one.
         */
 -      snprintf(repo_config, PATH_MAX, "%s/config", gitdir);
 -      git_config_early(check_repository_format_version, NULL, repo_config);
 +      git_config_early(fn, NULL, repo_config);
        if (GIT_REPO_VERSION < repository_format_version) {
                if (!nongit_ok)
                        die ("Expected git repo version <= %d, found %d",
                        GIT_REPO_VERSION, repository_format_version);
                warning("Please upgrade Git");
                *nongit_ok = -1;
 -              return -1;
 +              ret = -1;
        }
 -      return 0;
 +      strbuf_release(&sb);
 +      return ret;
 +}
 +
 +static void update_linked_gitdir(const char *gitfile, const char *gitdir)
 +{
 +      struct strbuf path = STRBUF_INIT;
 +      struct stat st;
 +
 +      strbuf_addf(&path, "%s/gitfile", gitdir);
 +      if (stat(path.buf, &st) || st.st_mtime + 24 * 3600 < time(NULL))
 +              write_file(path.buf, 0, "%s\n", gitfile);
 +      strbuf_release(&path);
  }
  
  /*
@@@ -451,8 -385,6 +456,8 @@@ const char *read_gitfile(const char *pa
  
        if (!is_git_directory(dir))
                die("Not a git repository: %s", dir);
 +
 +      update_linked_gitdir(path, dir);
        path = real_path(dir);
  
        free(buf);
@@@ -495,8 -427,11 +500,11 @@@ static const char *setup_explicit_git_d
        if (work_tree_env)
                set_git_work_tree(work_tree_env);
        else if (is_bare_repository_cfg > 0) {
-               if (git_work_tree_cfg) /* #22.2, #30 */
-                       die("core.bare and core.worktree do not make sense");
+               if (git_work_tree_cfg) {
+                       /* #22.2, #30 */
+                       warning("core.bare and core.worktree do not make sense");
+                       work_tree_config_is_bogus = 1;
+               }
  
                /* #18, #26 */
                set_git_dir(gitdirenv);
@@@ -872,10 -807,11 +880,10 @@@ int git_config_perm(const char *var, co
  
  int check_repository_format_version(const char *var, const char *value, void *cb)
  {
 -      if (strcmp(var, "core.repositoryformatversion") == 0)
 -              repository_format_version = git_config_int(var, value);
 -      else if (strcmp(var, "core.sharedrepository") == 0)
 -              shared_repository = git_config_perm(var, value);
 -      else if (strcmp(var, "core.bare") == 0) {
 +      int ret = check_repo_format(var, value, cb);
 +      if (ret)
 +              return ret;
 +      if (strcmp(var, "core.bare") == 0) {
                is_bare_repository_cfg = git_config_bool(var, value);
                if (is_bare_repository_cfg == 1)
                        inside_work_tree = -1;
diff --combined t/t1510-repo-setup.sh
index 33c1a587b3b9f28ecc824a17650467bdc6011bc6,f6aa3c70f8ade06b94d28e340d7060a81718dcca..13ae12dfa7d494f50fc2e728d3e7ee36ff5534bc
@@@ -106,7 -106,6 +106,7 @@@ setup_env () 
  expect () {
        cat >"$1/expected" <<-EOF
        setup: git_dir: $2
 +      setup: git_common_dir: $2
        setup: worktree: $3
        setup: cwd: $4
        setup: prefix: $5
@@@ -599,11 -598,20 +599,20 @@@ test_expect_success '#20b/c: core.workt
        mkdir -p 20b/.git/wt/sub &&
        (
                cd 20b/.git &&
-               test_must_fail git symbolic-ref HEAD >/dev/null
+               test_must_fail git status >/dev/null
        ) 2>message &&
        grep "core.bare and core.worktree" message
  '
  
+ test_expect_success '#20d: core.worktree and core.bare OK when working tree not needed' '
+       setup_repo 20d non-existent "" true &&
+       mkdir -p 20d/.git/wt/sub &&
+       (
+               cd 20d/.git &&
+               git config foo.bar value
+       )
+ '
  # Case #21: core.worktree/GIT_WORK_TREE overrides core.bare' '
  test_expect_success '#21: setup, core.worktree warns before overriding core.bare' '
        setup_repo 21 non-existent "" unset &&
                cd 21/.git &&
                GIT_WORK_TREE="$here/21" &&
                export GIT_WORK_TREE &&
-               git symbolic-ref HEAD >/dev/null
+               git status >/dev/null
        ) 2>message &&
        ! test -s message
  
@@@ -701,13 -709,13 +710,13 @@@ test_expect_success '#22.2: core.worktr
                cd 22/.git &&
                GIT_DIR=. &&
                export GIT_DIR &&
-               test_must_fail git symbolic-ref HEAD 2>result
+               test_must_fail git status 2>result
        ) &&
        (
                cd 22 &&
                GIT_DIR=.git &&
                export GIT_DIR &&
-               test_must_fail git symbolic-ref HEAD 2>result
+               test_must_fail git status 2>result
        ) &&
        grep "core.bare and core.worktree" 22/.git/result &&
        grep "core.bare and core.worktree" 22/result
@@@ -753,9 -761,8 +762,8 @@@ test_expect_success '#28: core.worktre
        setup_repo 28 "$here/28" gitfile true &&
        (
                cd 28 &&
-               test_must_fail git symbolic-ref HEAD
+               test_must_fail git status
        ) 2>message &&
-       ! grep "^warning:" message &&
        grep "core.bare and core.worktree" message
  '
  
@@@ -767,7 -774,7 +775,7 @@@ test_expect_success '#29: setup' 
                cd 29 &&
                GIT_WORK_TREE="$here/29" &&
                export GIT_WORK_TREE &&
-               git symbolic-ref HEAD >/dev/null
+               git status
        ) 2>message &&
        ! test -s message
  '
@@@ -778,7 -785,7 +786,7 @@@ test_expect_success '#30: core.worktre
        setup_repo 30 "$here/30" gitfile true &&
        (
                cd 30 &&
-               test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result
+               test_must_fail env GIT_DIR=.git git status 2>result
        ) &&
        grep "core.bare and core.worktree" 30/result
  '