From ec70f52f6fb6d3e08c7b24f8b5bf25502d8ee59b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 May 2011 12:52:12 -0700 Subject: [PATCH 1/4] convert: rename the "eol" global variable to "core_eol" Yes, it is clear that "eol" wants to mean some sort of end-of-line thing, but as the name of a global variable, it is way too short to describe what kind of end-of-line thing it wants to represent. Besides, there are many codepaths that want to use their own local "char *eol" variable to point at the end of the current line they are processing. This global variable holds what we read from core.eol configuration variable. Name it as such. Signed-off-by: Junio C Hamano --- cache.h | 2 +- config.c | 12 ++++++------ convert.c | 4 ++-- environment.c | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cache.h b/cache.h index 2b34116624..4e9123b77b 100644 --- a/cache.h +++ b/cache.h @@ -606,7 +606,7 @@ enum eol { #endif }; -extern enum eol eol; +extern enum eol core_eol; enum branch_track { BRANCH_TRACK_UNSPECIFIED = -1, diff --git a/config.c b/config.c index 5f9ec28945..671c8df2cc 100644 --- a/config.c +++ b/config.c @@ -583,7 +583,7 @@ static int git_default_core_config(const char *var, const char *value) if (!strcmp(var, "core.autocrlf")) { if (value && !strcasecmp(value, "input")) { - if (eol == EOL_CRLF) + if (core_eol == EOL_CRLF) return error("core.autocrlf=input conflicts with core.eol=crlf"); auto_crlf = AUTO_CRLF_INPUT; return 0; @@ -603,14 +603,14 @@ static int git_default_core_config(const char *var, const char *value) if (!strcmp(var, "core.eol")) { if (value && !strcasecmp(value, "lf")) - eol = EOL_LF; + core_eol = EOL_LF; else if (value && !strcasecmp(value, "crlf")) - eol = EOL_CRLF; + core_eol = EOL_CRLF; else if (value && !strcasecmp(value, "native")) - eol = EOL_NATIVE; + core_eol = EOL_NATIVE; else - eol = EOL_UNSET; - if (eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT) + core_eol = EOL_UNSET; + if (core_eol == EOL_CRLF && auto_crlf == AUTO_CRLF_INPUT) return error("core.autocrlf=input conflicts with core.eol=crlf"); return 0; } diff --git a/convert.c b/convert.c index 7eb51b16ed..4dba329e50 100644 --- a/convert.c +++ b/convert.c @@ -113,10 +113,10 @@ static enum eol determine_output_conversion(enum action action) return EOL_CRLF; else if (auto_crlf == AUTO_CRLF_INPUT) return EOL_LF; - else if (eol == EOL_UNSET) + else if (core_eol == EOL_UNSET) return EOL_NATIVE; } - return eol; + return core_eol; } static void check_safe_crlf(const char *path, enum action action, diff --git a/environment.c b/environment.c index 40185bc854..7fe9f10124 100644 --- a/environment.c +++ b/environment.c @@ -43,7 +43,7 @@ const char *askpass_program; const char *excludes_file; enum auto_crlf auto_crlf = AUTO_CRLF_FALSE; int read_replace_refs = 1; -enum eol eol = EOL_UNSET; +enum eol core_eol = EOL_UNSET; enum safe_crlf safe_crlf = SAFE_CRLF_WARN; unsigned whitespace_rule_cfg = WS_DEFAULT_RULE; enum branch_track git_branch_track = BRANCH_TRACK_REMOTE; From c61dcff9d6944eb35abf7fc7faa36f23a49fabf6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 May 2011 13:12:57 -0700 Subject: [PATCH 2/4] convert: give saner names to crlf/eol variables, types and functions Back when the conversion was only about the end-of-line convention, it might have made sense to call what we do upon seeing CR/LF simply an "action", but these days the conversion routines do a lot more than just tweaking the line ending. Raname "action" to "crlf_action". The function that decides what end of line conversion to use on the output codepath was called "determine_output_conversion", as if there is no other kind of output conversion. Rename it to "output_eol"; it is a function that returns what EOL convention is to be used. A function that decides what "crlf_action" needs to be used on the input codepath, given what conversion attribute is set to the path and global end-of-line convention, was called "determine_action". Rename it to "input_crlf_action". Signed-off-by: Junio C Hamano --- convert.c | 61 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/convert.c b/convert.c index 4dba329e50..e0ee245153 100644 --- a/convert.c +++ b/convert.c @@ -12,7 +12,7 @@ * translation when the "text" attribute or "auto_crlf" option is set. */ -enum action { +enum crlf_action { CRLF_GUESS = -1, CRLF_BINARY = 0, CRLF_TEXT, @@ -94,9 +94,9 @@ static int is_binary(unsigned long size, struct text_stat *stats) return 0; } -static enum eol determine_output_conversion(enum action action) +static enum eol output_eol(enum crlf_action crlf_action) { - switch (action) { + switch (crlf_action) { case CRLF_BINARY: return EOL_UNSET; case CRLF_CRLF: @@ -119,13 +119,13 @@ static enum eol determine_output_conversion(enum action action) return core_eol; } -static void check_safe_crlf(const char *path, enum action action, +static void check_safe_crlf(const char *path, enum crlf_action crlf_action, struct text_stat *stats, enum safe_crlf checksafe) { if (!checksafe) return; - if (determine_output_conversion(action) == EOL_LF) { + if (output_eol(crlf_action) == EOL_LF) { /* * CRLFs would not be restored by checkout: * check if we'd remove CRLFs @@ -136,7 +136,7 @@ static void check_safe_crlf(const char *path, enum action action, else /* i.e. SAFE_CRLF_FAIL */ die("CRLF would be replaced by LF in %s.", path); } - } else if (determine_output_conversion(action) == EOL_CRLF) { + } else if (output_eol(crlf_action) == EOL_CRLF) { /* * CRLFs would be added by checkout: * check if we have "naked" LFs @@ -188,18 +188,19 @@ static int has_cr_in_index(const char *path) } static int crlf_to_git(const char *path, const char *src, size_t len, - struct strbuf *buf, enum action action, enum safe_crlf checksafe) + struct strbuf *buf, + enum crlf_action crlf_action, enum safe_crlf checksafe) { struct text_stat stats; char *dst; - if (action == CRLF_BINARY || - (action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) || !len) + if (crlf_action == CRLF_BINARY || + (crlf_action == CRLF_GUESS && auto_crlf == AUTO_CRLF_FALSE) || !len) return 0; gather_stats(src, len, &stats); - if (action == CRLF_AUTO || action == CRLF_GUESS) { + if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) { /* * We're currently not going to even try to convert stuff * that has bare CR characters. Does anybody do that crazy @@ -214,7 +215,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len, if (is_binary(len, &stats)) return 0; - if (action == CRLF_GUESS) { + if (crlf_action == CRLF_GUESS) { /* * If the file in the index has any CR in it, do not convert. * This is the new safer autocrlf handling. @@ -224,7 +225,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len, } } - check_safe_crlf(path, action, &stats, checksafe); + check_safe_crlf(path, crlf_action, &stats, checksafe); /* Optimization: No CR? Nothing to convert, regardless. */ if (!stats.cr) @@ -234,7 +235,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len, if (strbuf_avail(buf) + buf->len < len) strbuf_grow(buf, len - buf->len); dst = buf->buf; - if (action == CRLF_AUTO || action == CRLF_GUESS) { + if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) { /* * If we guessed, we already know we rejected a file with * lone CR, and we can strip a CR without looking at what @@ -257,12 +258,12 @@ static int crlf_to_git(const char *path, const char *src, size_t len, } static int crlf_to_worktree(const char *path, const char *src, size_t len, - struct strbuf *buf, enum action action) + struct strbuf *buf, enum crlf_action crlf_action) { char *to_free = NULL; struct text_stat stats; - if (!len || determine_output_conversion(action) != EOL_CRLF) + if (!len || output_eol(crlf_action) != EOL_CRLF) return 0; gather_stats(src, len, &stats); @@ -275,8 +276,8 @@ static int crlf_to_worktree(const char *path, const char *src, size_t len, if (stats.lf == stats.crlf) return 0; - if (action == CRLF_AUTO || action == CRLF_GUESS) { - if (action == CRLF_GUESS) { + if (crlf_action == CRLF_AUTO || crlf_action == CRLF_GUESS) { + if (crlf_action == CRLF_GUESS) { /* If we have any CR or CRLF line endings, we do not touch it */ /* This is the new safer autocrlf-handling */ if (stats.cr > 0 || stats.crlf > 0) @@ -715,7 +716,7 @@ static int git_path_check_ident(const char *path, struct git_attr_check *check) return !!ATTR_TRUE(value); } -static enum action determine_action(enum action text_attr, enum eol eol_attr) +static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol eol_attr) { if (text_attr == CRLF_BINARY) return CRLF_BINARY; @@ -730,7 +731,7 @@ int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst, enum safe_crlf checksafe) { struct git_attr_check check[5]; - enum action action = CRLF_GUESS; + enum crlf_action crlf_action = CRLF_GUESS; enum eol eol_attr = EOL_UNSET; int ident = 0, ret = 0; const char *filter = NULL; @@ -738,9 +739,9 @@ int convert_to_git(const char *path, const char *src, size_t len, setup_convert_check(check); if (!git_checkattr(path, ARRAY_SIZE(check), check)) { struct convert_driver *drv; - action = git_path_check_crlf(path, check + 4); - if (action == CRLF_GUESS) - action = git_path_check_crlf(path, check + 0); + crlf_action = git_path_check_crlf(path, check + 4); + if (crlf_action == CRLF_GUESS) + crlf_action = git_path_check_crlf(path, check + 0); ident = git_path_check_ident(path, check + 1); drv = git_path_check_convert(path, check + 2); eol_attr = git_path_check_eol(path, check + 3); @@ -753,8 +754,8 @@ int convert_to_git(const char *path, const char *src, size_t len, src = dst->buf; len = dst->len; } - action = determine_action(action, eol_attr); - ret |= crlf_to_git(path, src, len, dst, action, checksafe); + crlf_action = input_crlf_action(crlf_action, eol_attr); + ret |= crlf_to_git(path, src, len, dst, crlf_action, checksafe); if (ret) { src = dst->buf; len = dst->len; @@ -767,7 +768,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src, int normalizing) { struct git_attr_check check[5]; - enum action action = CRLF_GUESS; + enum crlf_action crlf_action = CRLF_GUESS; enum eol eol_attr = EOL_UNSET; int ident = 0, ret = 0; const char *filter = NULL; @@ -775,9 +776,9 @@ static int convert_to_working_tree_internal(const char *path, const char *src, setup_convert_check(check); if (!git_checkattr(path, ARRAY_SIZE(check), check)) { struct convert_driver *drv; - action = git_path_check_crlf(path, check + 4); - if (action == CRLF_GUESS) - action = git_path_check_crlf(path, check + 0); + crlf_action = git_path_check_crlf(path, check + 4); + if (crlf_action == CRLF_GUESS) + crlf_action = git_path_check_crlf(path, check + 0); ident = git_path_check_ident(path, check + 1); drv = git_path_check_convert(path, check + 2); eol_attr = git_path_check_eol(path, check + 3); @@ -795,8 +796,8 @@ static int convert_to_working_tree_internal(const char *path, const char *src, * is a smudge filter. The filter might expect CRLFs. */ if (filter || !normalizing) { - action = determine_action(action, eol_attr); - ret |= crlf_to_worktree(path, src, len, dst, action); + crlf_action = input_crlf_action(crlf_action, eol_attr); + ret |= crlf_to_worktree(path, src, len, dst, crlf_action); if (ret) { src = dst->buf; len = dst->len; From 83295964b3289e957d028960f14a2b71348c39ed Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 May 2011 11:23:04 -0700 Subject: [PATCH 3/4] convert: make it safer to add conversion attributes The places that need to pass an array of "struct git_attr_check" needed to be careful to pass a large enough array and know what index each element lied. Make it safer and easier to code these. Besides, the hard-coded sequence of initializing various attributes was too ugly after we gained more than a few attributes. Signed-off-by: Junio C Hamano --- convert.c | 48 ++++++++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/convert.c b/convert.c index e0ee245153..a05820ba63 100644 --- a/convert.c +++ b/convert.c @@ -475,30 +475,6 @@ static int read_convert_config(const char *var, const char *value, void *cb) return 0; } -static void setup_convert_check(struct git_attr_check *check) -{ - static struct git_attr *attr_text; - static struct git_attr *attr_crlf; - static struct git_attr *attr_eol; - static struct git_attr *attr_ident; - static struct git_attr *attr_filter; - - if (!attr_text) { - attr_text = git_attr("text"); - attr_crlf = git_attr("crlf"); - attr_eol = git_attr("eol"); - attr_ident = git_attr("ident"); - attr_filter = git_attr("filter"); - user_convert_tail = &user_convert; - git_config(read_convert_config, NULL); - } - check[0].attr = attr_crlf; - check[1].attr = attr_ident; - check[2].attr = attr_filter; - check[3].attr = attr_eol; - check[4].attr = attr_text; -} - static int count_ident(const char *cp, unsigned long size) { /* @@ -727,10 +703,30 @@ static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol e return text_attr; } +static const char *conv_attr_name[] = { + "crlf", "ident", "filter", "eol", "text", +}; +#define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name) + +static void setup_convert_check(struct git_attr_check *check) +{ + int i; + static struct git_attr_check ccheck[NUM_CONV_ATTRS]; + + if (!ccheck[0].attr) { + for (i = 0; i < NUM_CONV_ATTRS; i++) + ccheck[i].attr = git_attr(conv_attr_name[i]); + user_convert_tail = &user_convert; + git_config(read_convert_config, NULL); + } + for (i = 0; i < NUM_CONV_ATTRS; i++) + check[i].attr = ccheck[i].attr; +} + int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst, enum safe_crlf checksafe) { - struct git_attr_check check[5]; + struct git_attr_check check[NUM_CONV_ATTRS]; enum crlf_action crlf_action = CRLF_GUESS; enum eol eol_attr = EOL_UNSET; int ident = 0, ret = 0; @@ -767,7 +763,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src, size_t len, struct strbuf *dst, int normalizing) { - struct git_attr_check check[5]; + struct git_attr_check check[NUM_CONV_ATTRS]; enum crlf_action crlf_action = CRLF_GUESS; enum eol eol_attr = EOL_UNSET; int ident = 0, ret = 0; From 3bfba20dae16384cb7112268462bd01d30d4a698 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 9 May 2011 13:58:31 -0700 Subject: [PATCH 4/4] convert: make it harder to screw up adding a conversion attribute The current internal API requires the callers of setup_convert_check() to supply the git_attr_check structures (hence they need to know how many to allocate), but they grab the same set of attributes for given path. Define a new convert_attrs() API that fills a higher level information that the callers (convert_to_git and convert_to_working_tree) really want, and move the common code to interact with the attributes system to it. Signed-off-by: Junio C Hamano --- convert.c | 79 ++++++++++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/convert.c b/convert.c index a05820ba63..efc7e07d47 100644 --- a/convert.c +++ b/convert.c @@ -703,12 +703,19 @@ static enum crlf_action input_crlf_action(enum crlf_action text_attr, enum eol e return text_attr; } +struct conv_attrs { + struct convert_driver *drv; + enum crlf_action crlf_action; + enum eol eol_attr; + int ident; +}; + static const char *conv_attr_name[] = { "crlf", "ident", "filter", "eol", "text", }; #define NUM_CONV_ATTRS ARRAY_SIZE(conv_attr_name) -static void setup_convert_check(struct git_attr_check *check) +static void convert_attrs(struct conv_attrs *ca, const char *path) { int i; static struct git_attr_check ccheck[NUM_CONV_ATTRS]; @@ -719,70 +726,60 @@ static void setup_convert_check(struct git_attr_check *check) user_convert_tail = &user_convert; git_config(read_convert_config, NULL); } - for (i = 0; i < NUM_CONV_ATTRS; i++) - check[i].attr = ccheck[i].attr; + + if (!git_checkattr(path, NUM_CONV_ATTRS, ccheck)) { + ca->crlf_action = git_path_check_crlf(path, ccheck + 4); + if (ca->crlf_action == CRLF_GUESS) + ca->crlf_action = git_path_check_crlf(path, ccheck + 0); + ca->ident = git_path_check_ident(path, ccheck + 1); + ca->drv = git_path_check_convert(path, ccheck + 2); + ca->eol_attr = git_path_check_eol(path, ccheck + 3); + } else { + ca->drv = NULL; + ca->crlf_action = CRLF_GUESS; + ca->eol_attr = EOL_UNSET; + ca->ident = 0; + } } int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst, enum safe_crlf checksafe) { - struct git_attr_check check[NUM_CONV_ATTRS]; - enum crlf_action crlf_action = CRLF_GUESS; - enum eol eol_attr = EOL_UNSET; - int ident = 0, ret = 0; + int ret = 0; const char *filter = NULL; + struct conv_attrs ca; - setup_convert_check(check); - if (!git_checkattr(path, ARRAY_SIZE(check), check)) { - struct convert_driver *drv; - crlf_action = git_path_check_crlf(path, check + 4); - if (crlf_action == CRLF_GUESS) - crlf_action = git_path_check_crlf(path, check + 0); - ident = git_path_check_ident(path, check + 1); - drv = git_path_check_convert(path, check + 2); - eol_attr = git_path_check_eol(path, check + 3); - if (drv && drv->clean) - filter = drv->clean; - } + convert_attrs(&ca, path); + if (ca.drv) + filter = ca.drv->clean; ret |= apply_filter(path, src, len, dst, filter); if (ret) { src = dst->buf; len = dst->len; } - crlf_action = input_crlf_action(crlf_action, eol_attr); - ret |= crlf_to_git(path, src, len, dst, crlf_action, checksafe); + ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr); + ret |= crlf_to_git(path, src, len, dst, ca.crlf_action, checksafe); if (ret) { src = dst->buf; len = dst->len; } - return ret | ident_to_git(path, src, len, dst, ident); + return ret | ident_to_git(path, src, len, dst, ca.ident); } static int convert_to_working_tree_internal(const char *path, const char *src, size_t len, struct strbuf *dst, int normalizing) { - struct git_attr_check check[NUM_CONV_ATTRS]; - enum crlf_action crlf_action = CRLF_GUESS; - enum eol eol_attr = EOL_UNSET; - int ident = 0, ret = 0; + int ret = 0; const char *filter = NULL; + struct conv_attrs ca; - setup_convert_check(check); - if (!git_checkattr(path, ARRAY_SIZE(check), check)) { - struct convert_driver *drv; - crlf_action = git_path_check_crlf(path, check + 4); - if (crlf_action == CRLF_GUESS) - crlf_action = git_path_check_crlf(path, check + 0); - ident = git_path_check_ident(path, check + 1); - drv = git_path_check_convert(path, check + 2); - eol_attr = git_path_check_eol(path, check + 3); - if (drv && drv->smudge) - filter = drv->smudge; - } + convert_attrs(&ca, path); + if (ca.drv) + filter = ca.drv->smudge; - ret |= ident_to_worktree(path, src, len, dst, ident); + ret |= ident_to_worktree(path, src, len, dst, ca.ident); if (ret) { src = dst->buf; len = dst->len; @@ -792,8 +789,8 @@ static int convert_to_working_tree_internal(const char *path, const char *src, * is a smudge filter. The filter might expect CRLFs. */ if (filter || !normalizing) { - crlf_action = input_crlf_action(crlf_action, eol_attr); - ret |= crlf_to_worktree(path, src, len, dst, crlf_action); + ca.crlf_action = input_crlf_action(ca.crlf_action, ca.eol_attr); + ret |= crlf_to_worktree(path, src, len, dst, ca.crlf_action); if (ret) { src = dst->buf; len = dst->len;