@matheusbsbs / matheus-bagliano / commits / 5c327502db

MacOS: precompose_argv_prefix()

The following sequence leads to a "BUG" assertion running under MacOS: DIR=git-test-restore-p Adiarnfd=$(printf 'A\314\210') DIRNAME=xx${Adiarnfd}yy mkdir $DIR && cd $DIR && git init && mkdir $DIRNAME && cd $DIRNAME && echo "Initial" >file && git add file && echo "One more line" >>file && echo y | git restore -p . Initialized empty Git repository in /tmp/git-test-restore-p/.git/ BUG: pathspec.c:495: error initializing pathspec_item Cannot close git diff-index --cached --numstat [snip] The command `git restore` is run from a directory inside a Git repo. Git needs to split the $CWD into 2 parts: The path to the repo and "the rest", if any. "The rest" becomes a "prefix" later used inside the pathspec code. As an example, "/path/to/repo/dir-inside-repå" would determine "/path/to/repo" as the root of the repo, the place where the configuration file .git/config is found. The rest becomes the prefix ("dir-inside-repå"), from where the pathspec machinery expands the ".", more about this later. If there is a decomposed form, (making the decomposing visible like this), "dir-inside-rep°a" doesn't match "dir-inside-repå". Git commands need to: (a) read the configuration variable "core.precomposeunicode" (b) precocompose argv[] (c) precompose the prefix, if there was any The first commit, 76759c7dff53 "git on Mac OS and precomposed unicode" addressed (a) and (b). The call to precompose_argv() was added into parse-options.c, because that seemed to be a good place when the patch was written. Commands that don't use parse-options need to do (a) and (b) themselfs. The commands `diff-files`, `diff-index`, `diff-tree` and `diff` learned (a) and (b) in commit 90a78b83e0b8 "diff: run arguments through precompose_argv" Branch names (or refs in general) using decomposed code points resulting in decomposed file names had been fixed in commit 8e712ef6fc97 "Honor core.precomposeUnicode in more places" The bug report from above shows 2 things: - more commands need to handle precomposed unicode - (c) should be implemented for all commands using pathspecs Solution: precompose_argv() now handles the prefix (if needed), and is renamed into precompose_argv_prefix(). Inside this function the config variable core.precomposeunicode is read into the global variable precomposed_unicode, as before. This reading is skipped if precomposed_unicode had been read before. The original patch for preocomposed unicode, 76759c7dff53, placed precompose_argv() into parse-options.c Now add it into git.c::run_builtin() as well. Existing precompose calls in diff-files.c and others may become redundant, and if we audit the callflows that reach these places to make sure that they can never be reached without going through the new call added to run_builtin(), we might be able to remove these existing ones. But in this commit, we do not bother to do so and leave these precompose callsites as they are. Because precompose() is idempotent and can be called on an already precomposed string safely, this is safer than removing existing calls without fully vetting the callflows. There is certainly room for cleanups - this change intends to be a bug fix. Cleanups needs more tests in e.g. t/t3910-mac-os-precompose.sh, and should be done in future commits. [1] git-bugreport-2021-01-06-1209.txt (git can't deal with special characters) [2] https://lore.kernel.org/git/A102844A-9501-4A86-854D-E3B387D378AA@icloud.com/ Reported-by: Daniel Troger <random_n0body@icloud.com> Helped-By: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Torsten Bögershausen committed Feb 3, 2021 at 17:28 UTC 5c327502dbf7a27c8784c20037851206a87857c1

11 files changed +59 -29

builtin/diff-files.c

+1 -1

@@ -35,7 +35,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)

35 */

36 rev.diffopt.ita_invisible_in_index = 1;

38 - precompose_argv(argc, argv);

38 + prefix = precompose_argv_prefix(argc, argv, prefix);

40 argc = setup_revisions(argc, argv, &rev, NULL);

41 while (1 < argc && argv[1][0] == '-') {

builtin/diff-index.c

+1 -1

@@ -25,7 +25,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)

25 git_config(git_diff_basic_config, NULL); /* no "diff" UI options */

26 repo_init_revisions(the_repository, &rev, prefix);

27 rev.abbrev = 0;

28 - precompose_argv(argc, argv);

28 + prefix = precompose_argv_prefix(argc, argv, prefix);

30 argc = setup_revisions(argc, argv, &rev, NULL);

31 for (i = 1; i < argc; i++) {

builtin/diff-tree.c

+1 -1

@@ -126,7 +126,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)

126 memset(&s_r_opt, 0, sizeof(s_r_opt));

127 s_r_opt.tweak = diff_tree_tweak_rev;

128

129 - precompose_argv(argc, argv);

129 + prefix = precompose_argv_prefix(argc, argv, prefix);

130 argc = setup_revisions(argc, argv, opt, &s_r_opt);

131

132 memset(&w, 0, sizeof(w));

builtin/diff.c

+1 -1

@@ -452,7 +452,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)

452

453 init_diff_ui_defaults();

454 git_config(git_diff_ui_config, NULL);

455 - precompose_argv(argc, argv);

455 + prefix = precompose_argv_prefix(argc, argv, prefix);

456

457 repo_init_revisions(the_repository, &rev, prefix);

458

builtin/submodule--helper.c

+1 -1

@@ -1257,7 +1257,7 @@ static int compute_summary_module_list(struct object_id *head_oid,

1257 git_config(git_diff_basic_config, NULL);

1258 init_revisions(&rev, info->prefix);

1259 rev.abbrev = 0;

1260 - precompose_argv(diff_args.nr, diff_args.v);

1260 + precompose_argv_prefix(diff_args.nr, diff_args.v, NULL);

1261 setup_revisions(diff_args.nr, diff_args.v, &rev, NULL);

1262 rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK;

1263 rev.diffopt.format_callback = submodule_summary_callback;

compat/precompose_utf8.c

+33 -19

@@ -60,32 +60,46 @@ void probe_utf8_pathname_composition(void)

60 strbuf_release(&path);

61 }

63 -

64 -void precompose_argv(int argc, const char **argv)

63 +static inline const char *precompose_string_if_needed(const char *in)

64 {

66 - int i = 0;

67 - const char *oldarg;

68 - char *newarg;

69 - iconv_t ic_precompose;

65 + size_t inlen;

66 + size_t outlen;

67 + if (has_non_ascii(in, (size_t)-1, &inlen)) {

68 + iconv_t ic_prec;

69 + char *out;

70 + if (precomposed_unicode < 0)

71 + git_config_get_bool("core.precomposeunicode", &precomposed_unicode);

72 + if (precomposed_unicode != 1)

73 + return in;

74 + ic_prec = iconv_open(repo_encoding, path_encoding);

75 + if (ic_prec == (iconv_t) -1)

76 + return in;

77 +

78 + out = reencode_string_iconv(in, inlen, ic_prec, 0, &outlen);

79 + if (out) {

80 + if (outlen == inlen && !memcmp(in, out, outlen))

81 + free(out); /* no need to return indentical */

82 + else

83 + in = out;

84 + }

85 + iconv_close(ic_prec);

71 - if (precomposed_unicode != 1)

72 - return;

87 + }

88 + return in;

89 +}

74 - ic_precompose = iconv_open(repo_encoding, path_encoding);

75 - if (ic_precompose == (iconv_t) -1)

76 - return;

91 +const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)

92 +{

93 + int i = 0;

95 while (i < argc) {

79 - size_t namelen;

80 - oldarg = argv[i];

81 - if (has_non_ascii(oldarg, (size_t)-1, &namelen)) {

82 - newarg = reencode_string_iconv(oldarg, namelen, ic_precompose, 0, NULL);

83 - if (newarg)

84 - argv[i] = newarg;

85 - }

96 + argv[i] = precompose_string_if_needed(argv[i]);

97 i++;

98 }

88 - iconv_close(ic_precompose);

99 + if (prefix) {

100 + prefix = precompose_string_if_needed(prefix);

101 + }

102 + return prefix;

103 }

104

105

compat/precompose_utf8.h

+1 -1

@@ -28,7 +28,7 @@ typedef struct {

28 struct dirent_prec_psx *dirent_nfc;

29 } PREC_DIR;

31 -void precompose_argv(int argc, const char **argv);

31 +const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix);

32 void probe_utf8_pathname_composition(void);

34 PREC_DIR *precompose_utf8_opendir(const char *dirname);

git-compat-util.h

+2 -2

@@ -252,9 +252,9 @@ typedef unsigned long uintptr_t;

252 #ifdef PRECOMPOSE_UNICODE

253 #include "compat/precompose_utf8.h"

254 #else

255 -static inline void precompose_argv(int argc, const char **argv)

255 +static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)

256 {

257 - ; /* nothing */

257 + return prefix;

258 }

259 #define probe_utf8_pathname_composition()

260 #endif

git.c

+1 -1

@@ -420,7 +420,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)

420 int nongit_ok;

421 prefix = setup_git_directory_gently(&nongit_ok);

422 }

423 -

423 + prefix = precompose_argv_prefix(argc, argv, prefix);

424 if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&

425 !(p->option & DELAY_PAGER_CONFIG))

426 use_pager = check_pager_config(p->cmd);

parse-options.c

+1 -1

@@ -869,7 +869,7 @@ int parse_options(int argc, const char **argv, const char *prefix,

869 usage_with_options(usagestr, options);

870 }

871

872 - precompose_argv(argc, argv);

872 + precompose_argv_prefix(argc, argv, NULL);

873 free(real_options);

874 free(ctx.alias_groups);

875 return parse_options_end(&ctx);

t/t3910-mac-os-precompose.sh

+16

@@ -191,6 +191,22 @@ test_expect_failure 'handle existing decomposed filenames' '

191 test_must_be_empty untracked

192 '

193

194 +test_expect_success "unicode decomposed: git restore -p . " '

195 + DIRNAMEPWD=dir.Odiarnfc &&

196 + DIRNAMEINREPO=dir.$Adiarnfc &&

197 + export DIRNAMEPWD DIRNAMEINREPO &&

198 + git init "$DIRNAMEPWD" &&

199 + (

200 + cd "$DIRNAMEPWD" &&

201 + mkdir "$DIRNAMEINREPO" &&

202 + cd "$DIRNAMEINREPO" &&

203 + echo "Initial" >file &&

204 + git add file &&

205 + echo "More stuff" >>file &&

206 + echo y | git restore -p .

207 + )

208 +'

209 +

210 # Test if the global core.precomposeunicode stops autosensing

211 # Must be the last test case

212 test_expect_success "respect git config --global core.precomposeunicode" '