Merge branch 'nd/conditional-config-in-early-config'
authorJunio C Hamano <gitster@pobox.com>
Wed, 26 Apr 2017 06:39:05 +0000 (15:39 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 26 Apr 2017 06:39:05 +0000 (15:39 +0900)
The recently introduced conditional inclusion of configuration did
not work well when early-config mechanism was involved.

* nd/conditional-config-in-early-config:
  config: correct file reading order in read_early_config()
  config: handle conditional include when $GIT_DIR is not set up
  config: prepare to pass more info in git_config_with_options()

1  2 
builtin/config.c
cache.h
config.c
t/t1305-config-include.sh

diff --combined builtin/config.c
@@@ -26,7 -26,8 +26,8 @@@ static int use_global_config, use_syste
  static struct git_config_source given_config_source;
  static int actions, types;
  static int end_null;
- static int respect_includes = -1;
+ static int respect_includes_opt = -1;
+ static struct config_options config_options;
  static int show_origin;
  
  #define ACTION_GET (1<<0)
@@@ -81,7 -82,7 +82,7 @@@ static struct option builtin_config_opt
        OPT_GROUP(N_("Other")),
        OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
        OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
-       OPT_BOOL(0, "includes", &respect_includes, N_("respect include directives on lookup")),
+       OPT_BOOL(0, "includes", &respect_includes_opt, N_("respect include directives on lookup")),
        OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")),
        OPT_END(),
  };
@@@ -242,7 -243,7 +243,7 @@@ static int get_value(const char *key_, 
        }
  
        git_config_with_options(collect_config, &values,
-                               &given_config_source, respect_includes);
+                               &given_config_source, &config_options);
  
        ret = !values.nr;
  
@@@ -320,7 -321,7 +321,7 @@@ static void get_color(const char *var, 
        get_color_found = 0;
        parsed_color[0] = '\0';
        git_config_with_options(git_get_color_config, NULL,
-                               &given_config_source, respect_includes);
+                               &given_config_source, &config_options);
  
        if (!get_color_found && def_color) {
                if (color_parse(def_color, parsed_color) < 0)
@@@ -352,7 -353,7 +353,7 @@@ static int get_colorbool(const char *va
        get_diff_color_found = -1;
        get_color_ui_found = -1;
        git_config_with_options(git_get_colorbool_config, NULL,
-                               &given_config_source, respect_includes);
+                               &given_config_source, &config_options);
  
        if (get_colorbool_found < 0) {
                if (!strcmp(get_colorbool_slot, "color.diff"))
@@@ -441,7 -442,7 +442,7 @@@ static int get_urlmatch(const char *var
        }
  
        git_config_with_options(urlmatch_config_entry, &config,
-                               &given_config_source, respect_includes);
+                               &given_config_source, &config_options);
  
        ret = !values.nr;
  
@@@ -502,7 -503,7 +503,7 @@@ int cmd_config(int argc, const char **a
        }
  
        if (use_global_config) {
 -              char *user_config = expand_user_path("~/.gitconfig");
 +              char *user_config = expand_user_path("~/.gitconfig", 0);
                char *xdg_config = xdg_config_home("config");
  
                if (!user_config)
                                prefix_filename(prefix, given_config_source.file);
        }
  
-       if (respect_includes == -1)
-               respect_includes = !given_config_source.file;
+       if (respect_includes_opt == -1)
+               config_options.respect_includes = !given_config_source.file;
+       else
+               config_options.respect_includes = respect_includes_opt;
  
        if (end_null) {
                term = '\0';
                check_argc(argc, 0, 0);
                if (git_config_with_options(show_all_config, NULL,
                                            &given_config_source,
-                                           respect_includes) < 0) {
+                                           &config_options) < 0) {
                        if (given_config_source.file)
                                die_errno("unable to read config file '%s'",
                                          given_config_source.file);
diff --combined cache.h
+++ b/cache.h
@@@ -66,12 -66,8 +66,12 @@@ unsigned long git_deflate_bound(git_zst
  #define GIT_SHA1_RAWSZ 20
  #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
  
 +/* The length in byte and in hex digits of the largest possible hash value. */
 +#define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
 +#define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
 +
  struct object_id {
 -      unsigned char hash[GIT_SHA1_RAWSZ];
 +      unsigned char hash[GIT_MAX_RAWSZ];
  };
  
  #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)
@@@ -710,8 -706,6 +710,8 @@@ extern void update_index_if_able(struc
  extern int hold_locked_index(struct lock_file *, int);
  extern void set_alternate_index_output(const char *);
  
 +extern int verify_index_checksum;
 +
  /* Environment bits from configuration mechanism */
  extern int trust_executable_bit;
  extern int trust_ctime;
@@@ -984,7 -978,7 +984,7 @@@ extern char *sha1_pack_index_name(cons
  extern const char *find_unique_abbrev(const unsigned char *sha1, int len);
  extern int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len);
  
 -extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];
 +extern const unsigned char null_sha1[GIT_MAX_RAWSZ];
  extern const struct object_id null_oid;
  
  static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
@@@ -1166,7 -1160,7 +1166,7 @@@ typedef int create_file_fn(const char *
  int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
  
  int mkdir_in_gitdir(const char *path);
 -extern char *expand_user_path(const char *path);
 +extern char *expand_user_path(const char *path, int real_home);
  const char *enter_repo(const char *path, int strict);
  static inline int is_absolute_path(const char *path)
  {
@@@ -1366,7 -1360,7 +1366,7 @@@ extern int get_sha1_with_context(const 
  
  extern int get_oid(const char *str, struct object_id *oid);
  
 -typedef int each_abbrev_fn(const unsigned char *sha1, void *);
 +typedef int each_abbrev_fn(const struct object_id *oid, void *);
  extern int for_each_abbrev(const char *prefix, each_abbrev_fn, void *);
  
  extern int set_disambiguate_hint_config(const char *var, const char *value);
@@@ -1891,6 -1885,11 +1891,11 @@@ enum config_origin_type 
        CONFIG_ORIGIN_CMDLINE
  };
  
+ struct config_options {
+       unsigned int respect_includes : 1;
+       const char *git_dir;
+ };
  typedef int (*config_fn_t)(const char *, const char *, void *);
  extern int git_default_config(const char *, const char *, void *);
  extern int git_config_from_file(config_fn_t fn, const char *, void *);
@@@ -1904,13 -1903,12 +1909,13 @@@ extern void read_early_config(config_fn
  extern void git_config(config_fn_t fn, void *);
  extern int git_config_with_options(config_fn_t fn, void *,
                                   struct git_config_source *config_source,
-                                  int respect_includes);
+                                  const struct config_options *opts);
  extern int git_parse_ulong(const char *, unsigned long *);
  extern int git_parse_maybe_bool(const char *);
  extern int git_config_int(const char *, const char *);
  extern int64_t git_config_int64(const char *, const char *);
  extern unsigned long git_config_ulong(const char *, const char *);
 +extern ssize_t git_config_ssize_t(const char *, const char *);
  extern int git_config_bool_or_int(const char *, const char *, int *);
  extern int git_config_bool(const char *, const char *);
  extern int git_config_maybe_bool(const char *, const char *);
@@@ -1957,6 -1955,7 +1962,7 @@@ struct config_include_data 
        int depth;
        config_fn_t fn;
        void *data;
+       const struct config_options *opts;
  };
  #define CONFIG_INCLUDE_INIT { 0 }
  extern int git_config_include(const char *name, const char *value, void *data);
diff --combined config.c
+++ b/config.c
@@@ -135,7 -135,7 +135,7 @@@ static int handle_path_include(const ch
        if (!path)
                return config_error_nonbool("include.path");
  
 -      expanded = expand_user_path(path);
 +      expanded = expand_user_path(path, 0);
        if (!expanded)
                return error("could not expand include path '%s'", path);
        path = expanded;
@@@ -177,7 -177,7 +177,7 @@@ static int prepare_include_condition_pa
        char *expanded;
        int prefix = 0;
  
 -      expanded = expand_user_path(pat->buf);
 +      expanded = expand_user_path(pat->buf, 1);
        if (expanded) {
                strbuf_reset(pat);
                strbuf_addstr(pat, expanded);
                        return error(_("relative config include "
                                       "conditionals must come from files"));
  
 -              strbuf_add_absolute_path(&path, cf->path);
 +              strbuf_realpath(&path, cf->path, 1);
                slash = find_last_dir_sep(path.buf);
                if (!slash)
                        die("BUG: how is this possible?");
        return prefix;
  }
  
- static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
+ static int include_by_gitdir(const struct config_options *opts,
+                            const char *cond, size_t cond_len, int icase)
  {
        struct strbuf text = STRBUF_INIT;
        struct strbuf pattern = STRBUF_INIT;
        int ret = 0, prefix;
+       const char *git_dir;
  
-       strbuf_realpath(&text, get_git_dir(), 1);
+       if (opts->git_dir)
+               git_dir = opts->git_dir;
+       else if (have_git_dir())
+               git_dir = get_git_dir();
+       else
+               goto done;
 -      strbuf_add_absolute_path(&text, git_dir);
++      strbuf_realpath(&text, git_dir, 1);
        strbuf_add(&pattern, cond, cond_len);
        prefix = prepare_include_condition_pattern(&pattern);
  
@@@ -242,13 -251,14 +251,14 @@@ done
        return ret;
  }
  
- static int include_condition_is_true(const char *cond, size_t cond_len)
+ static int include_condition_is_true(const struct config_options *opts,
+                                    const char *cond, size_t cond_len)
  {
  
        if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
-               return include_by_gitdir(cond, cond_len, 0);
+               return include_by_gitdir(opts, cond, cond_len, 0);
        else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
-               return include_by_gitdir(cond, cond_len, 1);
+               return include_by_gitdir(opts, cond, cond_len, 1);
  
        /* unknown conditionals are always false */
        return 0;
@@@ -273,7 -283,7 +283,7 @@@ int git_config_include(const char *var
                ret = handle_path_include(value, inc);
  
        if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
-           (cond && include_condition_is_true(cond, cond_len)) &&
+           (cond && include_condition_is_true(inc->opts, cond, cond_len)) &&
            !strcmp(key, "path"))
                ret = handle_path_include(value, inc);
  
@@@ -834,15 -844,6 +844,15 @@@ int git_parse_ulong(const char *value, 
        return 1;
  }
  
 +static int git_parse_ssize_t(const char *value, ssize_t *ret)
 +{
 +      intmax_t tmp;
 +      if (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_t)))
 +              return 0;
 +      *ret = tmp;
 +      return 1;
 +}
 +
  NORETURN
  static void die_bad_number(const char *name, const char *value)
  {
@@@ -901,14 -902,6 +911,14 @@@ unsigned long git_config_ulong(const ch
        return ret;
  }
  
 +ssize_t git_config_ssize_t(const char *name, const char *value)
 +{
 +      ssize_t ret;
 +      if (!git_parse_ssize_t(value, &ret))
 +              die_bad_number(name, value);
 +      return ret;
 +}
 +
  int git_parse_maybe_bool(const char *value)
  {
        if (!value)
@@@ -965,7 -958,7 +975,7 @@@ int git_config_pathname(const char **de
  {
        if (!value)
                return config_error_nonbool(var);
 -      *dest = expand_user_path(value);
 +      *dest = expand_user_path(value, 0);
        if (!*dest)
                die(_("failed to expand user dir in: '%s'"), value);
        return 0;
@@@ -1511,12 -1504,20 +1521,20 @@@ int git_config_system(void
        return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
  }
  
- static int do_git_config_sequence(config_fn_t fn, void *data)
+ static int do_git_config_sequence(const struct config_options *opts,
+                                 config_fn_t fn, void *data)
  {
        int ret = 0;
        char *xdg_config = xdg_config_home("config");
 -      char *user_config = expand_user_path("~/.gitconfig");
 +      char *user_config = expand_user_path("~/.gitconfig", 0);
-       char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
+       char *repo_config;
+       if (opts->git_dir)
+               repo_config = mkpathdup("%s/config", opts->git_dir);
+       else if (have_git_dir())
+               repo_config = git_pathdup("config");
+       else
+               repo_config = NULL;
  
        current_parsing_scope = CONFIG_SCOPE_SYSTEM;
        if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
  
  int git_config_with_options(config_fn_t fn, void *data,
                            struct git_config_source *config_source,
-                           int respect_includes)
+                           const struct config_options *opts)
  {
        struct config_include_data inc = CONFIG_INCLUDE_INIT;
  
-       if (respect_includes) {
+       if (opts->respect_includes) {
                inc.fn = fn;
                inc.data = data;
+               inc.opts = opts;
                fn = git_config_include;
                data = &inc;
        }
        else if (config_source && config_source->blob)
                return git_config_from_blob_ref(fn, config_source->blob, data);
  
-       return do_git_config_sequence(fn, data);
+       return do_git_config_sequence(opts, fn, data);
  }
  
  static void git_config_raw(config_fn_t fn, void *data)
  {
-       if (git_config_with_options(fn, data, NULL, 1) < 0)
+       struct config_options opts = {0};
+       opts.respect_includes = 1;
+       if (git_config_with_options(fn, data, NULL, &opts) < 0)
                /*
                 * git_config_with_options() normally returns only
                 * zero, as most errors are fatal, and
@@@ -1614,10 -1619,13 +1636,13 @@@ static void configset_iter(struct confi
  
  void read_early_config(config_fn_t cb, void *data)
  {
+       struct config_options opts = {0};
        struct strbuf buf = STRBUF_INIT;
  
-       git_config_with_options(cb, data, NULL, 1);
+       opts.respect_includes = 1;
  
+       if (have_git_dir())
+               opts.git_dir = get_git_dir();
        /*
         * When setup_git_directory() was not yet asked to discover the
         * GIT_DIR, we ask discover_git_directory() to figure out whether there
         * notably, the current working directory is still the same after the
         * call).
         */
-       if (!have_git_dir() && discover_git_directory(&buf)) {
-               struct git_config_source repo_config;
+       else if (discover_git_directory(&buf))
+               opts.git_dir = buf.buf;
+       git_config_with_options(cb, data, NULL, &opts);
  
-               memset(&repo_config, 0, sizeof(repo_config));
-               strbuf_addstr(&buf, "/config");
-               repo_config.file = buf.buf;
-               git_config_with_options(cb, data, &repo_config, 1);
-       }
        strbuf_release(&buf);
  }
  
@@@ -3,16 -3,6 +3,16 @@@
  test_description='test config file include directives'
  . ./test-lib.sh
  
 +# Force setup_explicit_git_dir() to run until the end. This is needed
 +# by some tests to make sure real_path() is called on $GIT_DIR. The
 +# caller needs to make sure git commands are run from a subdirectory
 +# though or real_path() will not be called.
 +force_setup_explicit_git_dir() {
 +    GIT_DIR="$(pwd)/.git"
 +    GIT_WORK_TREE="$(pwd)"
 +    export GIT_DIR GIT_WORK_TREE
 +}
 +
  test_expect_success 'include file by absolute path' '
        echo "[test]one = 1" >one &&
        echo "[include]path = \"$(pwd)/one\"" >.gitconfig &&
@@@ -218,50 -208,17 +218,61 @@@ test_expect_success 'conditional includ
        )
  '
  
+ test_expect_success 'conditional include, early config reading' '
+       (
+               cd foo &&
+               echo "[includeIf \"gitdir:foo/\"]path=bar6" >>.git/config &&
+               echo "[test]six=6" >.git/bar6 &&
+               echo 6 >expect &&
+               test-config read_early_config test.six >actual &&
+               test_cmp expect actual
+       )
+ '
 +test_expect_success SYMLINKS 'conditional include, set up symlinked $HOME' '
 +      mkdir real-home &&
 +      ln -s real-home home &&
 +      (
 +              HOME="$TRASH_DIRECTORY/home" &&
 +              export HOME &&
 +              cd "$HOME" &&
 +
 +              git init foo &&
 +              cd foo &&
 +              mkdir sub
 +      )
 +'
 +
 +test_expect_success SYMLINKS 'conditional include, $HOME expansion with symlinks' '
 +      (
 +              HOME="$TRASH_DIRECTORY/home" &&
 +              export HOME &&
 +              cd "$HOME"/foo &&
 +
 +              echo "[includeIf \"gitdir:~/foo/\"]path=bar2" >>.git/config &&
 +              echo "[test]two=2" >.git/bar2 &&
 +              echo 2 >expect &&
 +              force_setup_explicit_git_dir &&
 +              git -C sub config test.two >actual &&
 +              test_cmp expect actual
 +      )
 +'
 +
 +test_expect_success SYMLINKS 'conditional include, relative path with symlinks' '
 +      echo "[includeIf \"gitdir:./foo/.git\"]path=bar4" >home/.gitconfig &&
 +      echo "[test]four=4" >home/bar4 &&
 +      (
 +              HOME="$TRASH_DIRECTORY/home" &&
 +              export HOME &&
 +              cd "$HOME"/foo &&
 +
 +              echo 4 >expect &&
 +              force_setup_explicit_git_dir &&
 +              git -C sub config test.four >actual &&
 +              test_cmp expect actual
 +      )
 +'
 +
  test_expect_success 'include cycles are detected' '
        cat >.gitconfig <<-\EOF &&
        [test]value = gitconfig