]> git.ipfire.org Git - thirdparty/git.git/commitdiff
MacOS: precompose_argv_prefix()
authorTorsten Bögershausen <tboegi@web.de>
Wed, 3 Feb 2021 16:28:23 +0000 (17:28 +0100)
committerJunio C Hamano <gitster@pobox.com>
Wed, 3 Feb 2021 22:09:37 +0000 (14:09 -0800)
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>
builtin/diff-files.c
builtin/diff-index.c
builtin/diff-tree.c
builtin/diff.c
builtin/submodule--helper.c
compat/precompose_utf8.c
compat/precompose_utf8.h
git-compat-util.h
git.c
parse-options.c
t/t3910-mac-os-precompose.sh

index 1e352dd8f77c281e41d702af81378cc9e213aaad..e3851dd1c02ed6c3830bac5816f81b5c33e1e4c7 100644 (file)
@@ -35,7 +35,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
         */
        rev.diffopt.ita_invisible_in_index = 1;
 
-       precompose_argv(argc, argv);
+       prefix = precompose_argv_prefix(argc, argv, prefix);
 
        argc = setup_revisions(argc, argv, &rev, NULL);
        while (1 < argc && argv[1][0] == '-') {
index 7f5281c46168ed72505fe9ee3cfa200c200b4cb1..c33d7af47859cc1a42ad3c11046812e6a5dd53eb 100644 (file)
@@ -25,7 +25,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
        git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
        repo_init_revisions(the_repository, &rev, prefix);
        rev.abbrev = 0;
-       precompose_argv(argc, argv);
+       prefix = precompose_argv_prefix(argc, argv, prefix);
 
        argc = setup_revisions(argc, argv, &rev, NULL);
        for (i = 1; i < argc; i++) {
index 9fc95e959f0e0dba69cc1f96597bb77f30997d1e..178d12f07f95936f53ef3cf800346130b3bb6a37 100644 (file)
@@ -126,7 +126,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
        memset(&s_r_opt, 0, sizeof(s_r_opt));
        s_r_opt.tweak = diff_tree_tweak_rev;
 
-       precompose_argv(argc, argv);
+       prefix = precompose_argv_prefix(argc, argv, prefix);
        argc = setup_revisions(argc, argv, opt, &s_r_opt);
 
        memset(&w, 0, sizeof(w));
index 780c33877f85b63c8ecf6bffbe6f7e2c5e91f5aa..3c87c959672bacebd8a660269424d0235431779a 100644 (file)
@@ -452,7 +452,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 
        init_diff_ui_defaults();
        git_config(git_diff_ui_config, NULL);
-       precompose_argv(argc, argv);
+       prefix = precompose_argv_prefix(argc, argv, prefix);
 
        repo_init_revisions(the_repository, &rev, prefix);
 
index c30896c8978873c6fe8f3dbcc5897c111c271d7d..7fbad8a20611d0fd82029ef0ab821364ecbe5c8b 100644 (file)
@@ -1257,7 +1257,7 @@ static int compute_summary_module_list(struct object_id *head_oid,
        git_config(git_diff_basic_config, NULL);
        init_revisions(&rev, info->prefix);
        rev.abbrev = 0;
-       precompose_argv(diff_args.nr, diff_args.v);
+       precompose_argv_prefix(diff_args.nr, diff_args.v, NULL);
        setup_revisions(diff_args.nr, diff_args.v, &rev, NULL);
        rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK;
        rev.diffopt.format_callback = submodule_summary_callback;
index 136250fbf6c4f7e707c25dba936e2effebd847f2..ec560565a862ccf091d68cf34bf2bb985e4faf9c 100644 (file)
@@ -60,32 +60,46 @@ void probe_utf8_pathname_composition(void)
        strbuf_release(&path);
 }
 
-
-void precompose_argv(int argc, const char **argv)
+static inline const char *precompose_string_if_needed(const char *in)
 {
-       int i = 0;
-       const char *oldarg;
-       char *newarg;
-       iconv_t ic_precompose;
+       size_t inlen;
+       size_t outlen;
+       if (has_non_ascii(in, (size_t)-1, &inlen)) {
+               iconv_t ic_prec;
+               char *out;
+               if (precomposed_unicode < 0)
+                       git_config_get_bool("core.precomposeunicode", &precomposed_unicode);
+               if (precomposed_unicode != 1)
+                       return in;
+               ic_prec = iconv_open(repo_encoding, path_encoding);
+               if (ic_prec == (iconv_t) -1)
+                       return in;
+
+               out = reencode_string_iconv(in, inlen, ic_prec, 0, &outlen);
+               if (out) {
+                       if (outlen == inlen && !memcmp(in, out, outlen))
+                               free(out); /* no need to return indentical */
+                       else
+                               in = out;
+               }
+               iconv_close(ic_prec);
 
-       if (precomposed_unicode != 1)
-               return;
+       }
+       return in;
+}
 
-       ic_precompose = iconv_open(repo_encoding, path_encoding);
-       if (ic_precompose == (iconv_t) -1)
-               return;
+const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
+{
+       int i = 0;
 
        while (i < argc) {
-               size_t namelen;
-               oldarg = argv[i];
-               if (has_non_ascii(oldarg, (size_t)-1, &namelen)) {
-                       newarg = reencode_string_iconv(oldarg, namelen, ic_precompose, 0, NULL);
-                       if (newarg)
-                               argv[i] = newarg;
-               }
+               argv[i] = precompose_string_if_needed(argv[i]);
                i++;
        }
-       iconv_close(ic_precompose);
+       if (prefix) {
+               prefix = precompose_string_if_needed(prefix);
+       }
+       return prefix;
 }
 
 
index 6f843d3e1a121cf8f73409bc66898f74a9f14f2c..d70b84665c61faee9e1fe1197c1fe61788d2afb2 100644 (file)
@@ -28,7 +28,7 @@ typedef struct {
        struct dirent_prec_psx *dirent_nfc;
 } PREC_DIR;
 
-void precompose_argv(int argc, const char **argv);
+const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix);
 void probe_utf8_pathname_composition(void);
 
 PREC_DIR *precompose_utf8_opendir(const char *dirname);
index 104993b97550c8df8f3834929e945dee43af2742..93d9b4b7afd29b9209ba01728cda85a356a35c56 100644 (file)
@@ -252,9 +252,9 @@ typedef unsigned long uintptr_t;
 #ifdef PRECOMPOSE_UNICODE
 #include "compat/precompose_utf8.h"
 #else
-static inline void precompose_argv(int argc, const char **argv)
+static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
 {
-       ; /* nothing */
+       return prefix;
 }
 #define probe_utf8_pathname_composition()
 #endif
diff --git a/git.c b/git.c
index a00a0a4d941261b8f7d39390d6f518197f38be15..16a485fbe747be3db6d895f1e6faa43cd63c608c 100644 (file)
--- a/git.c
+++ b/git.c
@@ -420,7 +420,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
                        int nongit_ok;
                        prefix = setup_git_directory_gently(&nongit_ok);
                }
-
+               prefix = precompose_argv_prefix(argc, argv, prefix);
                if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY) &&
                    !(p->option & DELAY_PAGER_CONFIG))
                        use_pager = check_pager_config(p->cmd);
index f0507432ee35ba8646a510d1a28ec3561f6bcb30..fbea16eaf5c2051f062bcf8b34ab99c3a4313075 100644 (file)
@@ -869,7 +869,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
                usage_with_options(usagestr, options);
        }
 
-       precompose_argv(argc, argv);
+       precompose_argv_prefix(argc, argv, NULL);
        free(real_options);
        free(ctx.alias_groups);
        return parse_options_end(&ctx);
index 54ce19e353d924de08d4242f519fef7a44ce4787..bc632a03ed23d42499e74ef606b3fa2727618a6f 100755 (executable)
@@ -191,6 +191,22 @@ test_expect_failure 'handle existing decomposed filenames' '
        test_must_be_empty untracked
 '
 
+test_expect_success "unicode decomposed: git restore -p . " '
+       DIRNAMEPWD=dir.Odiarnfc &&
+       DIRNAMEINREPO=dir.$Adiarnfc &&
+       export DIRNAMEPWD DIRNAMEINREPO &&
+       git init "$DIRNAMEPWD" &&
+       (
+               cd "$DIRNAMEPWD" &&
+               mkdir "$DIRNAMEINREPO" &&
+               cd "$DIRNAMEINREPO" &&
+               echo "Initial" >file &&
+               git add file &&
+               echo "More stuff" >>file &&
+               echo y | git restore -p .
+       )
+'
+
 # Test if the global core.precomposeunicode stops autosensing
 # Must be the last test case
 test_expect_success "respect git config --global core.precomposeunicode" '