attr: remove an implicit dependency on the_index

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>

Nguyễn Thái Ngọc Duy committed Aug 13, 2018 at 18:14 UTC 7a400a2c0270f2085b70690e4ddbfd8d141e69ca
10 files changed +55 -32
archive.c
+1 -1
@@ -109,7 +109,7 @@ static const struct attr_check *get_archive_attrs(const char *path)
109 static struct attr_check *check;
110 if (!check)
111 check = attr_check_initl("export-ignore", "export-subst", NULL);
112 - return git_check_attr(path, check) ? NULL : check;
112 + return git_check_attr(&the_index, path, check) ? NULL : check;
113 }
114
115 static int check_attr_export_ignore(const struct attr_check *check)
attr.c
+38 -19
@@ -708,10 +708,10 @@ static struct attr_stack *read_attr_from_array(const char **list)
708 * another thread could potentially be calling into the attribute system.
709 */
710 static enum git_attr_direction direction;
711 -static struct index_state *use_index;
711 +static const struct index_state *use_index;
712
713 void git_attr_set_direction(enum git_attr_direction new_direction,
714 - struct index_state *istate)
714 + const struct index_state *istate)
715 {
716 if (is_bare_repository() && new_direction != GIT_ATTR_INDEX)
717 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)
743 return res;
744 }
745
746 -static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
746 +static struct attr_stack *read_attr_from_index(const struct index_state *istate,
747 + const char *path,
748 + int macro_ok)
749 {
750 struct attr_stack *res;
751 char *buf, *sp;
752 int lineno = 0;
753 + const struct index_state *to_read_from;
754
752 - buf = read_blob_data_from_index(use_index ? use_index : &the_index, path, NULL);
755 + /*
756 + * Temporary workaround for c24f3abace (apply: file commited
757 + * with CRLF should roundtrip diff and apply - 2017-08-19)
758 + */
759 + to_read_from = use_index ? use_index : istate;
760 + if (!to_read_from)
761 + return NULL;
762 +
763 + buf = read_blob_data_from_index(to_read_from, path, NULL);
764 if (!buf)
765 return NULL;
766
@@ -768,15 +779,16 @@ static struct attr_stack *read_attr_from_index(const char *path, int macro_ok)
779 return res;
780 }
781
771 -static struct attr_stack *read_attr(const char *path, int macro_ok)
782 +static struct attr_stack *read_attr(const struct index_state *istate,
783 + const char *path, int macro_ok)
784 {
785 struct attr_stack *res = NULL;
786
787 if (direction == GIT_ATTR_INDEX) {
776 - res = read_attr_from_index(path, macro_ok);
788 + res = read_attr_from_index(istate, path, macro_ok);
789 } else if (!is_bare_repository()) {
790 if (direction == GIT_ATTR_CHECKOUT) {
779 - res = read_attr_from_index(path, macro_ok);
791 + res = read_attr_from_index(istate, path, macro_ok);
792 if (!res)
793 res = read_attr_from_file(path, macro_ok);
794 } else if (direction == GIT_ATTR_CHECKIN) {
@@ -788,7 +800,7 @@ static struct attr_stack *read_attr(const char *path, int macro_ok)
800 * We allow operation in a sparsely checked out
801 * work tree, so read from it.
802 */
791 - res = read_attr_from_index(path, macro_ok);
803 + res = read_attr_from_index(istate, path, macro_ok);
804 }
805 }
806
@@ -859,7 +871,8 @@ static void push_stack(struct attr_stack **attr_stack_p,
871 }
872 }
873
862 -static void bootstrap_attr_stack(struct attr_stack **stack)
874 +static void bootstrap_attr_stack(const struct index_state *istate,
875 + struct attr_stack **stack)
876 {
877 struct attr_stack *e;
878
@@ -883,7 +896,7 @@ static void bootstrap_attr_stack(struct attr_stack **stack)
896 }
897
898 /* root directory */
886 - e = read_attr(GITATTRIBUTES_FILE, 1);
899 + e = read_attr(istate, GITATTRIBUTES_FILE, 1);
900 push_stack(stack, e, xstrdup(""), 0);
901
902 /* info frame */
@@ -896,7 +909,8 @@ static void bootstrap_attr_stack(struct attr_stack **stack)
909 push_stack(stack, e, NULL, 0);
910 }
911
899 -static void prepare_attr_stack(const char *path, int dirlen,
912 +static void prepare_attr_stack(const struct index_state *istate,
913 + const char *path, int dirlen,
914 struct attr_stack **stack)
915 {
916 struct attr_stack *info;
@@ -917,7 +931,7 @@ static void prepare_attr_stack(const char *path, int dirlen,
931 * .gitattributes in deeper directories to shallower ones,
932 * and finally use the built-in set as the default.
933 */
920 - bootstrap_attr_stack(stack);
934 + bootstrap_attr_stack(istate, stack);
935
936 /*
937 * 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,
987 strbuf_add(&pathbuf, path + pathbuf.len, (len - pathbuf.len));
988 strbuf_addf(&pathbuf, "/%s", GITATTRIBUTES_FILE);
989
976 - next = read_attr(pathbuf.buf, 0);
990 + next = read_attr(istate, pathbuf.buf, 0);
991
992 /* reset the pathbuf to not include "/.gitattributes" */
993 strbuf_setlen(&pathbuf, len);
@@ -1095,7 +1109,9 @@ static void determine_macros(struct all_attrs_item *all_attrs,
1109 * If check->check_nr is non-zero, only attributes in check[] are collected.
1110 * Otherwise all attributes are collected.
1111 */
1098 -static void collect_some_attrs(const char *path, struct attr_check *check)
1112 +static void collect_some_attrs(const struct index_state *istate,
1113 + const char *path,
1114 + struct attr_check *check)
1115 {
1116 int i, pathlen, rem, dirlen;
1117 const char *cp, *last_slash = NULL;
@@ -1114,7 +1130,7 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
1130 dirlen = 0;
1131 }
1132
1117 - prepare_attr_stack(path, dirlen, &check->stack);
1133 + prepare_attr_stack(istate, path, dirlen, &check->stack);
1134 all_attrs_init(&g_attr_hashmap, check);
1135 determine_macros(check->all_attrs, check->stack);
1136
@@ -1136,11 +1152,13 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
1152 fill(path, pathlen, basename_offset, check->stack, check->all_attrs, rem);
1153 }
1154
1139 -int git_check_attr(const char *path, struct attr_check *check)
1155 +int git_check_attr(const struct index_state *istate,
1156 + const char *path,
1157 + struct attr_check *check)
1158 {
1159 int i;
1160
1143 - collect_some_attrs(path, check);
1161 + collect_some_attrs(istate, path, check);
1162
1163 for (i = 0; i < check->nr; i++) {
1164 size_t n = check->items[i].attr->attr_nr;
@@ -1153,12 +1171,13 @@ int git_check_attr(const char *path, struct attr_check *check)
1171 return 0;
1172 }
1173
1156 -void git_all_attrs(const char *path, struct attr_check *check)
1174 +void git_all_attrs(const struct index_state *istate,
1175 + const char *path, struct attr_check *check)
1176 {
1177 int i;
1178
1179 attr_check_reset(check);
1161 - collect_some_attrs(path, check);
1180 + collect_some_attrs(istate, path, check);
1181
1182 for (i = 0; i < check->all_attrs_nr; i++) {
1183 const char *name = check->all_attrs[i].attr->name;
attr.h
+7 -3
@@ -1,6 +1,8 @@
1 #ifndef ATTR_H
2 #define ATTR_H
3
4 +struct index_state;
5 +
6 /* An attribute is a pointer to this opaque structure */
7 struct git_attr;
8
@@ -60,13 +62,15 @@ void attr_check_free(struct attr_check *check);
62 */
63 const char *git_attr_name(const struct git_attr *);
64
63 -int git_check_attr(const char *path, struct attr_check *check);
65 +int git_check_attr(const struct index_state *istate,
66 + const char *path, struct attr_check *check);
67
68 /*
69 * Retrieve all attributes that apply to the specified path.
70 * check holds the attributes and their values.
71 */
69 -void git_all_attrs(const char *path, struct attr_check *check);
72 +void git_all_attrs(const struct index_state *istate,
73 + const char *path, struct attr_check *check);
74
75 enum git_attr_direction {
76 GIT_ATTR_CHECKIN,
@@ -74,7 +78,7 @@ enum git_attr_direction {
78 GIT_ATTR_INDEX
79 };
80 void git_attr_set_direction(enum git_attr_direction new_direction,
77 - struct index_state *istate);
81 + const struct index_state *istate);
82
83 void attr_start(void);
84
builtin/check-attr.c
+2 -2
@@ -63,9 +63,9 @@ static void check_attr(const char *prefix,
63 prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
64
65 if (collect_all) {
66 - git_all_attrs(full_path, check);
66 + git_all_attrs(&the_index, full_path, check);
67 } else {
68 - if (git_check_attr(full_path, check))
68 + if (git_check_attr(&the_index, full_path, check))
69 die("git_check_attr died");
70 }
71 output_attr(check, file);
builtin/pack-objects.c
+1 -1
@@ -945,7 +945,7 @@ static int no_try_delta(const char *path)
945
946 if (!check)
947 check = attr_check_initl("delta", NULL);
948 - if (git_check_attr(path, check))
948 + if (git_check_attr(&the_index, path, check))
949 return 0;
950 if (ATTR_FALSE(check->items[0].value))
951 return 1;
convert.c
+1 -1
@@ -1303,7 +1303,7 @@ static void convert_attrs(struct conv_attrs *ca, const char *path)
1303 git_config(read_convert_config, NULL);
1304 }
1305
1306 - if (!git_check_attr(path, check)) {
1306 + if (!git_check_attr(&the_index, path, check)) {
1307 struct attr_check_item *ccheck = check->items;
1308 ca->crlf_action = git_path_check_crlf(ccheck + 4);
1309 if (ca->crlf_action == CRLF_UNDEFINED)
dir.c
+1 -1
@@ -281,7 +281,7 @@ static int match_attrs(const char *name, int namelen,
281 {
282 int i;
283
284 - git_check_attr(name, item->attr_check);
284 + git_check_attr(&the_index, name, item->attr_check);
285 for (i = 0; i < item->attr_match_nr; i++) {
286 const char *value;
287 int matched;
ll-merge.c
+2 -2
@@ -371,7 +371,7 @@ int ll_merge(mmbuffer_t *result_buf,
371 if (!check)
372 check = attr_check_initl("merge", "conflict-marker-size", NULL);
373
374 - if (!git_check_attr(path, check)) {
374 + if (!git_check_attr(&the_index, path, check)) {
375 ll_driver_name = check->items[0].value;
376 if (check->items[1].value) {
377 marker_size = atoi(check->items[1].value);
@@ -398,7 +398,7 @@ int ll_merge_marker_size(const char *path)
398
399 if (!check)
400 check = attr_check_initl("conflict-marker-size", NULL);
401 - if (!git_check_attr(path, check) && check->items[0].value) {
401 + if (!git_check_attr(&the_index, path, check) && check->items[0].value) {
402 marker_size = atoi(check->items[0].value);
403 if (marker_size <= 0)
404 marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
userdiff.c
+1 -1
@@ -278,7 +278,7 @@ struct userdiff_driver *userdiff_find_by_path(const char *path)
278 check = attr_check_initl("diff", NULL);
279 if (!path)
280 return NULL;
281 - if (git_check_attr(path, check))
281 + if (git_check_attr(&the_index, path, check))
282 return NULL;
283
284 if (ATTR_TRUE(check->items[0].value))
ws.c
+1 -1
@@ -78,7 +78,7 @@ unsigned whitespace_rule(const char *pathname)
78 if (!attr_whitespace_rule)
79 attr_whitespace_rule = attr_check_initl("whitespace", NULL);
80
81 - if (!git_check_attr(pathname, attr_whitespace_rule)) {
81 + if (!git_check_attr(&the_index, pathname, attr_whitespace_rule)) {
82 const char *value;
83
84 value = attr_whitespace_rule->items[0].value;