attr: remove an implicit dependency on the_index
authorNguyễn Thái Ngọc Duy <pclouds@gmail.com>
Mon, 13 Aug 2018 16:14:20 +0000 (18:14 +0200)
committerJunio C Hamano <gitster@pobox.com>
Mon, 13 Aug 2018 21:14:42 +0000 (14:14 -0700)
Make the attr API take an index_state instead of assuming the_index in
attr code. All call sites are converted blindly to keep the patch
simple and retain current behavior. Individual call sites may receive
further updates to use the right index instead of the_index.

There is one ugly temporary workaround added in attr.c that needs some
more explanation.

Commit c24f3abace (apply: file commited with CRLF should roundtrip
diff and apply - 2017-08-19) forces one convert_to_git() call to NOT
read the index at all. But what do you know, we read it anyway by
falling back to the_index. When "istate" from convert_to_git is now
propagated down to read_attr_from_array() we will hit segfault
somewhere inside read_blob_data_from_index.

The right way of dealing with this is to kill "use_index" variable and
only follow "istate" but at this stage we are not ready for that:
while most git_attr_set_direction() calls just passes the_index to be
assigned to use_index, unpack-trees passes a different one which is
used by entry.c code, which has no way to know what index to use if we
delete use_index. So this has to be done later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
archive.c
attr.c
attr.h
builtin/check-attr.c
builtin/pack-objects.c
convert.c
dir.c
ll-merge.c
userdiff.c
ws.c
index 78b0a398a0ff74e6d6d5afb2f7f0028b41fea32e..a8397e6173f0f6460a34ecc4579af82fe2018ab1 100644 (file)
--- a/archive.c
+++ b/archive.c
@@ -109,7 +109,7 @@ static const struct attr_check *get_archive_attrs(const char *path)
        static struct attr_check *check;
        if (!check)
                check = attr_check_initl("export-ignore", "export-subst", NULL);
-       return git_check_attr(path, check) ? NULL : check;
+       return git_check_attr(&the_index, path, check) ? NULL : check;
 }
 
 static int check_attr_export_ignore(const struct attr_check *check)
diff --git a/attr.c b/attr.c
index 067fb9e0c08cefa2ba1a9b1d3444938c9bab6e3d..863fad3bd159152963b5cd91f4ef90d12769b34c 100644 (file)
--- a/attr.c
+++ b/attr.c
@@ -708,10 +708,10 @@ static struct attr_stack *read_attr_from_array(const char **list)
  * another thread could potentially be calling into the attribute system.
  */
 static enum git_attr_direction direction;
-static struct index_state *use_index;
+static const struct index_state *use_index;
 
 void git_attr_set_direction(enum git_attr_direction new_direction,
-                           struct index_state *istate)
+                           const struct index_state *istate)
 {
        if (is_bare_repository() && new_direction != GIT_ATTR_INDEX)
                BUG("non-INDEX attr direction in a bare repo");
@@ -743,13 +743,24 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
        return res;
 }
 
-static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
+static struct attr_stack *read_attr_from_index(const struct index_state *istate,
+                                              const char *path,
+                                              int macro_ok)
 {
        struct attr_stack *res;
        char *buf, *sp;
        int lineno = 0;
+       const struct index_state *to_read_from;
 
-       buf = read_blob_data_from_index(use_index ? use_index : &the_index, path, NULL);
+       /*
+        * Temporary workaround for c24f3abace (apply: file commited
+        * with CRLF should roundtrip diff and apply - 2017-08-19)
+        */
+       to_read_from = use_index ? use_index : istate;
+       if (!to_read_from)
+               return NULL;
+
+       buf = read_blob_data_from_index(to_read_from, path, NULL);
        if (!buf)
                return NULL;
 
@@ -768,15 +779,16 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
        return res;
 }
 
-static struct attr_stack *read_attr(const char *path, int macro_ok)
+static struct attr_stack *read_attr(const struct index_state *istate,
+                                   const char *path, int macro_ok)
 {
        struct attr_stack *res = NULL;
 
        if (direction == GIT_ATTR_INDEX) {
-               res = read_attr_from_index(path, macro_ok);
+               res = read_attr_from_index(istate, path, macro_ok);
        } else if (!is_bare_repository()) {
                if (direction == GIT_ATTR_CHECKOUT) {
-                       res = read_attr_from_index(path, macro_ok);
+                       res = read_attr_from_index(istate, path, macro_ok);
                        if (!res)
                                res = read_attr_from_file(path, macro_ok);
                } else if (direction == GIT_ATTR_CHECKIN) {
@@ -788,7 +800,7 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)
                                 * We allow operation in a sparsely checked out
                                 * work tree, so read from it.
                                 */
-                               res = read_attr_from_index(path, macro_ok);
+                               res = read_attr_from_index(istate, path, macro_ok);
                }
        }
 
@@ -859,7 +871,8 @@ static void push_stack(struct attr_stack **attr_stack_p,
        }
 }
 
-static void bootstrap_attr_stack(struct attr_stack **stack)
+static void bootstrap_attr_stack(const struct index_state *istate,
+                                struct attr_stack **stack)
 {
        struct attr_stack *e;
 
@@ -883,7 +896,7 @@ static void bootstrap_attr_stack(struct attr_stack **stack)
        }
 
        /* root directory */
-       e = read_attr(GITATTRIBUTES_FILE, 1);
+       e = read_attr(istate, GITATTRIBUTES_FILE, 1);
        push_stack(stack, e, xstrdup(""), 0);
 
        /* info frame */
@@ -896,7 +909,8 @@ static void bootstrap_attr_stack(struct attr_stack **stack)
        push_stack(stack, e, NULL, 0);
 }
 
-static void prepare_attr_stack(const char *path, int dirlen,
+static void prepare_attr_stack(const struct index_state *istate,
+                              const char *path, int dirlen,
                               struct attr_stack **stack)
 {
        struct attr_stack *info;
@@ -917,7 +931,7 @@ static void prepare_attr_stack(const char *path, int dirlen,
         * .gitattributes in deeper directories to shallower ones,
         * and finally use the built-in set as the default.
         */
-       bootstrap_attr_stack(stack);
+       bootstrap_attr_stack(istate, stack);
 
        /*
         * Pop the "info" one that is always at the top of the stack.
@@ -973,7 +987,7 @@ static void prepare_attr_stack(const char *path, int dirlen,
                strbuf_add(&pathbuf, path + pathbuf.len, (len - pathbuf.len));
                strbuf_addf(&pathbuf, "/%s", GITATTRIBUTES_FILE);
 
-               next = read_attr(pathbuf.buf, 0);
+               next = read_attr(istate, pathbuf.buf, 0);
 
                /* reset the pathbuf to not include "/.gitattributes" */
                strbuf_setlen(&pathbuf, len);
@@ -1095,7 +1109,9 @@ static void determine_macros(struct all_attrs_item *all_attrs,
  * If check->check_nr is non-zero, only attributes in check[] are collected.
  * Otherwise all attributes are collected.
  */
-static void collect_some_attrs(const char *path, struct attr_check *check)
+static void collect_some_attrs(const struct index_state *istate,
+                              const char *path,
+                              struct attr_check *check)
 {
        int i, pathlen, rem, dirlen;
        const char *cp, *last_slash = NULL;
@@ -1114,7 +1130,7 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
                dirlen = 0;
        }
 
-       prepare_attr_stack(path, dirlen, &check->stack);
+       prepare_attr_stack(istate, path, dirlen, &check->stack);
        all_attrs_init(&g_attr_hashmap, check);
        determine_macros(check->all_attrs, check->stack);
 
@@ -1136,11 +1152,13 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
        fill(path, pathlen, basename_offset, check->stack, check->all_attrs, rem);
 }
 
-int git_check_attr(const char *path, struct attr_check *check)
+int git_check_attr(const struct index_state *istate,
+                  const char *path,
+                  struct attr_check *check)
 {
        int i;
 
-       collect_some_attrs(path, check);
+       collect_some_attrs(istate, path, check);
 
        for (i = 0; i < check->nr; i++) {
                size_t n = check->items[i].attr->attr_nr;
@@ -1153,12 +1171,13 @@ int git_check_attr(const char *path, struct attr_check *check)
        return 0;
 }
 
-void git_all_attrs(const char *path, struct attr_check *check)
+void git_all_attrs(const struct index_state *istate,
+                  const char *path, struct attr_check *check)
 {
        int i;
 
        attr_check_reset(check);
-       collect_some_attrs(path, check);
+       collect_some_attrs(istate, path, check);
 
        for (i = 0; i < check->all_attrs_nr; i++) {
                const char *name = check->all_attrs[i].attr->name;
diff --git a/attr.h b/attr.h
index 46340010bbee263d13ebb4bee67786132c4959cb..3daca3c0cba58ad8d15258c1e2fa5eb74b1ad72b 100644 (file)
--- a/attr.h
+++ b/attr.h
@@ -1,6 +1,8 @@
 #ifndef ATTR_H
 #define ATTR_H
 
+struct index_state;
+
 /* An attribute is a pointer to this opaque structure */
 struct git_attr;
 
@@ -60,13 +62,15 @@ void attr_check_free(struct attr_check *check);
  */
 const char *git_attr_name(const struct git_attr *);
 
-int git_check_attr(const char *path, struct attr_check *check);
+int git_check_attr(const struct index_state *istate,
+                  const char *path, struct attr_check *check);
 
 /*
  * Retrieve all attributes that apply to the specified path.
  * check holds the attributes and their values.
  */
-void git_all_attrs(const char *path, struct attr_check *check);
+void git_all_attrs(const struct index_state *istate,
+                  const char *path, struct attr_check *check);
 
 enum git_attr_direction {
        GIT_ATTR_CHECKIN,
@@ -74,7 +78,7 @@ enum git_attr_direction {
        GIT_ATTR_INDEX
 };
 void git_attr_set_direction(enum git_attr_direction new_direction,
-                           struct index_state *istate);
+                           const struct index_state *istate);
 
 void attr_start(void);
 
index 91444dc0448b32e854f1923fe1e57e28c87f5b35..f7b59993d3a10952921204bbaaf4d4bf349191c5 100644 (file)
@@ -63,9 +63,9 @@ static void check_attr(const char *prefix,
                prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
 
        if (collect_all) {
-               git_all_attrs(full_path, check);
+               git_all_attrs(&the_index, full_path, check);
        } else {
-               if (git_check_attr(full_path, check))
+               if (git_check_attr(&the_index, full_path, check))
                        die("git_check_attr died");
        }
        output_attr(check, file);
index 4391504a91367bc8c4897e66921fe9a91263f0d0..3ff6da441f369dd313c645fa36a3fb334d397323 100644 (file)
@@ -945,7 +945,7 @@ static int no_try_delta(const char *path)
 
        if (!check)
                check = attr_check_initl("delta", NULL);
-       if (git_check_attr(path, check))
+       if (git_check_attr(&the_index, path, check))
                return 0;
        if (ATTR_FALSE(check->items[0].value))
                return 1;
index 7907efd16f279df81ca18f2f370bb89380ccf921..1935bde9295b7cc511cb53e2e54f4edd576e8fb8 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -1303,7 +1303,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
                git_config(read_convert_config, NULL);
        }
 
-       if (!git_check_attr(path, check)) {
+       if (!git_check_attr(&the_index, path, check)) {
                struct attr_check_item *ccheck = check->items;
                ca->crlf_action = git_path_check_crlf(ccheck + 4);
                if (ca->crlf_action == CRLF_UNDEFINED)
diff --git a/dir.c b/dir.c
index 21e6f2520a487ec9167e7d1f79101f82444af832..29fbbd48c8cfd57c79062fe39e08d426a1bc41d5 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -281,7 +281,7 @@ static int match_attrs(const char *name, int namelen,
 {
        int i;
 
-       git_check_attr(name, item->attr_check);
+       git_check_attr(&the_index, name, item->attr_check);
        for (i = 0; i < item->attr_match_nr; i++) {
                const char *value;
                int matched;
index a6ad2ec12dc9c1ece81f120196c81fd259e40eb6..0e2800f7bb46bdf9e1713d210a5dc1c02fbbaa26 100644 (file)
@@ -371,7 +371,7 @@ int ll_merge(mmbuffer_t *result_buf,
        if (!check)
                check = attr_check_initl("merge", "conflict-marker-size", NULL);
 
-       if (!git_check_attr(path, check)) {
+       if (!git_check_attr(&the_index, path, check)) {
                ll_driver_name = check->items[0].value;
                if (check->items[1].value) {
                        marker_size = atoi(check->items[1].value);
@@ -398,7 +398,7 @@ int ll_merge_marker_size(const char *path)
 
        if (!check)
                check = attr_check_initl("conflict-marker-size", NULL);
-       if (!git_check_attr(path, check) && check->items[0].value) {
+       if (!git_check_attr(&the_index, path, check) && check->items[0].value) {
                marker_size = atoi(check->items[0].value);
                if (marker_size <= 0)
                        marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
index 36af25e7f9f4693b196320f56bb0546d7dd2986a..f3f4be579c9810d0fcf94badd4bf21770b99c91b 100644 (file)
@@ -278,7 +278,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path)
                check = attr_check_initl("diff", NULL);
        if (!path)
                return NULL;
-       if (git_check_attr(path, check))
+       if (git_check_attr(&the_index, path, check))
                return NULL;
 
        if (ATTR_TRUE(check->items[0].value))
diff --git a/ws.c b/ws.c
index a07caedd5a565bbf29e4f9b3036ac60e6e9f716e..5b67b426e7b41d92f9b11bd82fd7e9bd09c89d79 100644 (file)
--- a/ws.c
+++ b/ws.c
@@ -78,7 +78,7 @@ unsigned whitespace_rule(const char *pathname)
        if (!attr_whitespace_rule)
                attr_whitespace_rule = attr_check_initl("whitespace", NULL);
 
-       if (!git_check_attr(pathname, attr_whitespace_rule)) {
+       if (!git_check_attr(&the_index, pathname, attr_whitespace_rule)) {
                const char *value;
 
                value = attr_whitespace_rule->items[0].value;