@matheusbsbs / matheus-bagliano / commits / 21e5ad50fc

Fork 0

safecrlf: Add mechanism to warn about irreversible crlf conversions

CRLF conversion bears a slight chance of corrupting data. autocrlf=true will convert CRLF to LF during commit and LF to CRLF during checkout. A file that contains a mixture of LF and CRLF before the commit cannot be recreated by git. For text files this is the right thing to do: it corrects line endings such that we have only LF line endings in the repository. But for binary files that are accidentally classified as text the conversion can corrupt data. If you recognize such corruption early you can easily fix it by setting the conversion type explicitly in .gitattributes. Right after committing you still have the original file in your work tree and this file is not yet corrupted. You can explicitly tell git that this file is binary and git will handle the file appropriately. Unfortunately, the desired effect of cleaning up text files with mixed line endings and the undesired effect of corrupting binary files cannot be distinguished. In both cases CRLFs are removed in an irreversible way. For text files this is the right thing to do because CRLFs are line endings, while for binary files converting CRLFs corrupts data. This patch adds a mechanism that can either warn the user about an irreversible conversion or can even refuse to convert. The mechanism is controlled by the variable core.safecrlf, with the following values: - false: disable safecrlf mechanism - warn: warn about irreversible conversions - true: refuse irreversible conversions The default is to warn. Users are only affected by this default if core.autocrlf is set. But the current default of git is to leave core.autocrlf unset, so users will not see warnings unless they deliberately chose to activate the autocrlf mechanism. The safecrlf mechanism's details depend on the git command. The general principles when safecrlf is active (not false) are: - we warn/error out if files in the work tree can modified in an irreversible way without giving the user a chance to backup the original file. - for read-only operations that do not modify files in the work tree we do not not print annoying warnings. There are exceptions. Even though... - "git add" itself does not touch the files in the work tree, the next checkout would, so the safety triggers; - "git apply" to update a text file with a patch does touch the files in the work tree, but the operation is about text files and CRLF conversion is about fixing the line ending inconsistencies, so the safety does not trigger; - "git diff" itself does not touch the files in the work tree, it is often run to inspect the changes you intend to next "git add". To catch potential problems early, safety triggers. The concept of a safety check was originally proposed in a similar way by Linus Torvalds. Thanks to Dimitry Potapov for insisting on getting the naked LF/autocrlf=true case right. Signed-off-by: Steffen Prohaska <prohaska@zib.de>

Steffen Prohaska committed Feb 6, 2008 at 12:25 UTC 21e5ad50fc5e7277c74cfbb3cf6502468e840f86

11 files changed +189 -11

Documentation/config.txt

+45

@@ -139,6 +139,51 @@ core.autocrlf::

139 "text" (i.e. be subjected to the autocrlf mechanism) is

140 decided purely based on the contents.

141

142 +core.safecrlf::

143 + If true, makes git check if converting `CRLF` as controlled by

144 + `core.autocrlf` is reversible. Git will verify if a command

145 + modifies a file in the work tree either directly or indirectly.

146 + For example, committing a file followed by checking out the

147 + same file should yield the original file in the work tree. If

148 + this is not the case for the current setting of

149 + `core.autocrlf`, git will reject the file. The variable can

150 + be set to "warn", in which case git will only warn about an

151 + irreversible conversion but continue the operation.

152 ++

153 +CRLF conversion bears a slight chance of corrupting data.

154 +autocrlf=true will convert CRLF to LF during commit and LF to

155 +CRLF during checkout. A file that contains a mixture of LF and

156 +CRLF before the commit cannot be recreated by git. For text

157 +files this is the right thing to do: it corrects line endings

158 +such that we have only LF line endings in the repository.

159 +But for binary files that are accidentally classified as text the

160 +conversion can corrupt data.

161 ++

162 +If you recognize such corruption early you can easily fix it by

163 +setting the conversion type explicitly in .gitattributes. Right

164 +after committing you still have the original file in your work

165 +tree and this file is not yet corrupted. You can explicitly tell

166 +git that this file is binary and git will handle the file

167 +appropriately.

168 ++

169 +Unfortunately, the desired effect of cleaning up text files with

170 +mixed line endings and the undesired effect of corrupting binary

171 +files cannot be distinguished. In both cases CRLFs are removed

172 +in an irreversible way. For text files this is the right thing

173 +to do because CRLFs are line endings, while for binary files

174 +converting CRLFs corrupts data.

175 ++

176 +Note, this safety check does not mean that a checkout will generate a

177 +file identical to the original file for a different setting of

178 +`core.autocrlf`, but only for the current one. For example, a text

179 +file with `LF` would be accepted with `core.autocrlf=input` and could

180 +later be checked out with `core.autocrlf=true`, in which case the

181 +resulting file would contain `CRLF`, although the original file

182 +contained `LF`. However, in both work trees the line endings would be

183 +consistent, that is either all `LF` or all `CRLF`, but never mixed. A

184 +file with mixed line endings would be reported by the `core.safecrlf`

185 +mechanism.

186 +

187 core.symlinks::

188 If false, symbolic links are checked out as small plain files that

189 contain the link text. linkgit:git-update-index[1] and

Documentation/gitattributes.txt

+20

@@ -133,6 +133,26 @@ When `core.autocrlf` is set to "input", line endings are

133 converted to LF upon checkin, but there is no conversion done

134 upon checkout.

135

136 +If `core.safecrlf` is set to "true" or "warn", git verifies if

137 +the conversion is reversible for the current setting of

138 +`core.autocrlf`. For "true", git rejects irreversible

139 +conversions; for "warn", git only prints a warning but accepts

140 +an irreversible conversion. The safety triggers to prevent such

141 +a conversion done to the files in the work tree, but there are a

142 +few exceptions. Even though...

143 +

144 +- "git add" itself does not touch the files in the work tree, the

145 + next checkout would, so the safety triggers;

146 +

147 +- "git apply" to update a text file with a patch does touch the files

148 + in the work tree, but the operation is about text files and CRLF

149 + conversion is about fixing the line ending inconsistencies, so the

150 + safety does not trigger;

151 +

152 +- "git diff" itself does not touch the files in the work tree, it is

153 + often run to inspect the changes you intend to next "git add". To

154 + catch potential problems early, safety triggers.

155 +

156

157 `ident`

158 ^^^^^^^

builtin-apply.c

+1 -1

@@ -1430,7 +1430,7 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)

1430 case S_IFREG:

1431 if (strbuf_read_file(buf, path, st->st_size) != st->st_size)

1432 return error("unable to open or read %s", path);

1433 - convert_to_git(path, buf->buf, buf->len, buf);

1433 + convert_to_git(path, buf->buf, buf->len, buf, 0);

1434 return 0;

1435 default:

1436 return -1;

builtin-blame.c

+1 -1

@@ -2073,7 +2073,7 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con

2073 if (strbuf_read(&buf, 0, 0) < 0)

2074 die("read error %s from stdin", strerror(errno));

2075 }

2076 - convert_to_git(path, buf.buf, buf.len, &buf);

2076 + convert_to_git(path, buf.buf, buf.len, &buf, 0);

2077 origin->file.ptr = buf.buf;

2078 origin->file.size = buf.len;

2079 pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);

cache.h

+10 -1

@@ -330,6 +330,14 @@ extern size_t packed_git_limit;

330 extern size_t delta_base_cache_limit;

331 extern int auto_crlf;

332

333 +enum safe_crlf {

334 + SAFE_CRLF_FALSE = 0,

335 + SAFE_CRLF_FAIL = 1,

336 + SAFE_CRLF_WARN = 2,

337 +};

338 +

339 +extern enum safe_crlf safe_crlf;

340 +

341 #define GIT_REPO_VERSION 0

342 extern int repository_format_version;

343 extern int check_repository_format(void);

@@ -633,7 +641,8 @@ extern void trace_argv_printf(const char **argv, const char *format, ...);

641

642 /* convert.c */

643 /* returns 1 if *dst was used */

636 -extern int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst);

644 +extern int convert_to_git(const char *path, const char *src, size_t len,

645 + struct strbuf *dst, enum safe_crlf checksafe);

646 extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);

647

648 /* add */

config.c

@@ -407,6 +407,15 @@ int git_default_config(const char *var, const char *value)

407 return 0;

408 }

409

410 + if (!strcmp(var, "core.safecrlf")) {

411 + if (value && !strcasecmp(value, "warn")) {

412 + safe_crlf = SAFE_CRLF_WARN;

413 + return 0;

414 + }

415 + safe_crlf = git_config_bool(var, value);

416 + return 0;

417 + }

418 +

419 if (!strcmp(var, "user.name")) {

420 strlcpy(git_default_name, value, sizeof(git_default_name));

421 return 0;

convert.c

+41 -6

@@ -85,8 +85,39 @@ static int is_binary(unsigned long size, struct text_stat *stats)

85 return 0;

86 }

88 +static void check_safe_crlf(const char *path, int action,

89 + struct text_stat *stats, enum safe_crlf checksafe)

90 +{

91 + if (!checksafe)

92 + return;

93 +

94 + if (action == CRLF_INPUT || auto_crlf <= 0) {

95 + /*

96 + * CRLFs would not be restored by checkout:

97 + * check if we'd remove CRLFs

98 + */

99 + if (stats->crlf) {

100 + if (checksafe == SAFE_CRLF_WARN)

101 + warning("CRLF will be replaced by LF in %s.", path);

102 + else /* i.e. SAFE_CRLF_FAIL */

103 + die("CRLF would be replaced by LF in %s.", path);

104 + }

105 + } else if (auto_crlf > 0) {

106 + /*

107 + * CRLFs would be added by checkout:

108 + * check if we have "naked" LFs

109 + */

110 + if (stats->lf != stats->crlf) {

111 + if (checksafe == SAFE_CRLF_WARN)

112 + warning("LF will be replaced by CRLF in %s", path);

113 + else /* i.e. SAFE_CRLF_FAIL */

114 + die("LF would be replaced by CRLF in %s", path);

115 + }

116 + }

117 +}

118 +

119 static int crlf_to_git(const char *path, const char *src, size_t len,

89 - struct strbuf *buf, int action)

120 + struct strbuf *buf, int action, enum safe_crlf checksafe)

121 {

122 struct text_stat stats;

123 char *dst;

@@ -95,9 +126,6 @@ static int crlf_to_git(const char *path, const char *src, size_t len,

126 return 0;

127

128 gather_stats(src, len, &stats);

98 - /* No CR? Nothing to convert, regardless. */

99 - if (!stats.cr)

100 - return 0;

129

130 if (action == CRLF_GUESS) {

131 /*

@@ -115,6 +143,12 @@ static int crlf_to_git(const char *path, const char *src, size_t len,

143 return 0;

144 }

145

146 + check_safe_crlf(path, action, &stats, checksafe);

147 +

148 + /* Optimization: No CR? Nothing to convert, regardless. */

149 + if (!stats.cr)

150 + return 0;

151 +

152 /* only grow if not in place */

153 if (strbuf_avail(buf) + buf->len < len)

154 strbuf_grow(buf, len - buf->len);

@@ -536,7 +570,8 @@ static int git_path_check_ident(const char *path, struct git_attr_check *check)

570 return !!ATTR_TRUE(value);

571 }

572

539 -int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst)

573 +int convert_to_git(const char *path, const char *src, size_t len,

574 + struct strbuf *dst, enum safe_crlf checksafe)

575 {

576 struct git_attr_check check[3];

577 int crlf = CRLF_GUESS;

@@ -558,7 +593,7 @@ int convert_to_git(const char *path, const char *src, size_t len, struct strbuf

593 src = dst->buf;

594 len = dst->len;

595 }

561 - ret |= crlf_to_git(path, src, len, dst, crlf);

596 + ret |= crlf_to_git(path, src, len, dst, crlf, checksafe);

597 if (ret) {

598 src = dst->buf;

599 len = dst->len;

diff.c

+1 -1

@@ -1624,7 +1624,7 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)

1624 * Convert from working tree format to canonical git format

1625 */

1626 strbuf_init(&buf, 0);

1627 - if (convert_to_git(s->path, s->data, s->size, &buf)) {

1627 + if (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf)) {

1628 size_t size = 0;

1629 munmap(s->data, s->size);

1630 s->should_munmap = 0;

environment.c

@@ -35,6 +35,7 @@ int pager_use_color = 1;

35 char *editor_program;

36 char *excludes_file;

37 int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */

38 +enum safe_crlf safe_crlf = SAFE_CRLF_WARN;

39 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;

41 /* This is set by setup_git_dir_gently() and/or git_default_config() */

sha1_file.c

+2 -1

@@ -2358,7 +2358,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,

2358 if ((type == OBJ_BLOB) && S_ISREG(st->st_mode)) {

2359 struct strbuf nbuf;

2360 strbuf_init(&nbuf, 0);

2361 - if (convert_to_git(path, buf, size, &nbuf)) {

2361 + if (convert_to_git(path, buf, size, &nbuf,

2362 + write_object ? safe_crlf : 0)) {

2363 munmap(buf, size);

2364 buf = strbuf_detach(&nbuf, &size);

2365 re_allocated = 1;

t/t0020-crlf.sh

+58

@@ -8,6 +8,10 @@ q_to_nul () {

8 tr Q '\000'

9 }

11 +q_to_cr () {

12 + tr Q '\015'

13 +}

14 +

15 append_cr () {

16 sed -e 's/$/Q/' | tr Q '\015'

17 }

@@ -42,6 +46,60 @@ test_expect_success setup '

46 echo happy.

47 '

49 +test_expect_success 'safecrlf: autocrlf=input, all CRLF' '

50 +

51 + git config core.autocrlf input &&

52 + git config core.safecrlf true &&

53 +

54 + for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&

55 + ! git add allcrlf

56 +'

57 +

58 +test_expect_success 'safecrlf: autocrlf=input, mixed LF/CRLF' '

59 +

60 + git config core.autocrlf input &&

61 + git config core.safecrlf true &&

62 +

63 + for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >mixed &&

64 + ! git add mixed

65 +'

66 +

67 +test_expect_success 'safecrlf: autocrlf=true, all LF' '

68 +

69 + git config core.autocrlf true &&

70 + git config core.safecrlf true &&

71 +

72 + for w in I am all LF; do echo $w; done >alllf &&

73 + ! git add alllf

74 +'

75 +

76 +test_expect_success 'safecrlf: autocrlf=true mixed LF/CRLF' '

77 +

78 + git config core.autocrlf true &&

79 + git config core.safecrlf true &&

80 +

81 + for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >mixed &&

82 + ! git add mixed

83 +'

84 +

85 +test_expect_success 'safecrlf: print warning only once' '

86 +

87 + git config core.autocrlf input &&

88 + git config core.safecrlf warn &&

89 +

90 + for w in I am all LF; do echo $w; done >doublewarn &&

91 + git add doublewarn &&

92 + git commit -m "nowarn" &&

93 + for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >doublewarn &&

94 + test $(git add doublewarn 2>&1 | grep "CRLF will be replaced by LF" | wc -l) = 1

95 +'

96 +

97 +test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '

98 + git config core.autocrlf false &&

99 + git config core.safecrlf false &&

100 + git reset --hard HEAD^

101 +'

102 +

103 test_expect_success 'update with autocrlf=input' '

104

105 rm -f tmp one dir/two three &&