All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC/PATCH v5] git on Mac OS and precomposed unicode
@ 2012-06-24 15:47 Torsten Bögershausen
  2012-06-25 21:24 ` Junio C Hamano
  2012-06-25 22:33 ` Junio C Hamano
  0 siblings, 2 replies; 4+ messages in thread
From: Torsten Bögershausen @ 2012-06-24 15:47 UTC (permalink / raw)
  To: git; +Cc: tboegi

The problem:
Mac OS X may manipulate file names containing unicode on file systems
HFS+, VFAT or SAMBA.

When a file using unicode code points outside ASCII is created on a HFS+ drive,
the file name is converted into decomposed unicode and written to disk.
No conversion is done if the file name is already decomposed unicode.

Calling open("\xc3\x84", ...) whith a precomposed "Ä" yields the same
result as open("\x41\xcc\x88",...) with a decomposed "Ä".

This precomposition is done for all file system calls like
creat(), open(), fopen(), stat/lstat() etc.

The result is that readdir() will return the file names in decomposed unicode,
even if the user expects precomposed unicode.

In opposite to HFS+ stores Mac OS X  files on a VFAT drive (e.g. an USB drive),
in precomposed unicode, but readdir() returns file names in decomposed unicode.

When a git repository is stored on a network share using SAMBA, file names are
send over the wire and written to disk on the remote system in precomposed
unicode.
To be compatible with HFS+ and VFAT returns Mac OS X readdir()
decomposed unicode.

NFS mounted from Linux is fully transparent and does not need all this.

Summary:
filenames which are unicode equivalent in their precomposed or decomposed
form are folded onto the same file system object.
Calling readdir() returns always the decomposed form.

The unicode decomposition creates some problems:
- "git add" needs the decomposed form on the command line,
  so that the file name is picked up when readdir() is called
  to build a list of files on disk.
- The decomposed form is not (easily) available on the keyboard.
  To work around this, a wildcard could be used in "git add":
  Instead of using "git add Märchen.txt" the user needs to enter
  "git add M*rchen", "git add M<TAB>" or "git add *".
- "git log", "git mv" and all other commands needs the decomposed form
  to find the file name which is stored as decomposed in the index.
- The file names are stored in decomposed unicode in the index, but
  precomposed on disk.
    This makes it impossible to use this repository under e.g.
    Linux or Windows:
    All files appear to be deleted in the decomposed form and
    untracked in the precomposed form.

Knowing that Mac OS X writes file names as precomposed to disk,
and treats precomposed and decomposed file names as equal, git under Mac OS X
can be improved to revert the unicode decomposition of file names.

A new confguration variable is added: "core.precomposedunicode"

If set to false, git behaves exactly as older versions of git.
When a new git version is installed and there is a repository
where the configuration "core.precomposedunicode" is not present,
the new git is backward compatible.

If set to true, git allows forces the index to only use precomposed unicode.

The code in compat/precomposed_utf8.c implements basically 4 new functions:
precomposed_utf8_opendir(), precomposed_utf8_readdir(),
precomposed_utf8_closedir() precompose_argv()

As theere are only precomposed unicode file names in the index,
and precomposed and decomposed are treated as equal by the OS,
all arguments into git (argv[1]..argv[n]) are converted into
precomposed unicode.
This is done in parse-options.c by calling precompose_argv().
This function is actually a #define, and it is only defined under Mac OS.
Nothing is converted on any other OS.

The argv[] conversion allows to use the TAB filename completion done by the
shell on command line.
It tolerates other tools which use readdir() to feed decomposed file names
into git.

Auto sensing:
When creating a new git repository with "git init" or "git clone",
"core.precomposedunicode" will be set "false".

The user needs to activate this feature manually.
She typically sets core.precomposedunicode to "true" on HFS and VFAT,
or file systems mounted via SAMBA onto a Linux box.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 Documentation/config.txt     |   9 ++
 Makefile                     |   3 +
 builtin/init-db.c            |   2 +
 cache.h                      |   1 +
 compat/precomposed_utf8.c    | 200 +++++++++++++++++++++++++++++++++++++++++++
 compat/precomposed_utf8.h    |  30 +++++++
 config.c                     |   5 ++
 environment.c                |   1 +
 git-compat-util.h            |   9 ++
 parse-options.c              |   1 +
 t/t3910-mac-os-precompose.sh | 128 +++++++++++++++++++++++++++
 11 files changed, 389 insertions(+)
 create mode 100644 compat/precomposed_utf8.c
 create mode 100644 compat/precomposed_utf8.h
 create mode 100755 t/t3910-mac-os-precompose.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..8e4a6a8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -210,6 +210,15 @@ The default is false, except linkgit:git-clone[1] or linkgit:git-init[1]
 will probe and set core.ignorecase true if appropriate when the repository
 is created.
 
+core.precomposedunicode::
+	This option is only used by Mac OS implementation of git.
+	When core.precomposedunicode=true, git reverts the unicode decomposition
+	of filenames done by Mac OS. This is useful when sharing a repository
+	between Mac OS and Linux or	Windows.
+	(Git for Windows/msysGit 1.7.10 is needed, or git under cygwin 1.7).
+	When false, file names are handled fully transparent by git, which means
+	that file names are stored as decomposed unicode in the	repository.
+
 core.trustctime::
 	If false, the ctime differences between the index and the
 	working tree are ignored; useful when the inode change time
diff --git a/Makefile b/Makefile
index f62ca2a..55ceb10 100644
--- a/Makefile
+++ b/Makefile
@@ -607,6 +607,7 @@ LIB_H += compat/bswap.h
 LIB_H += compat/cygwin.h
 LIB_H += compat/mingw.h
 LIB_H += compat/obstack.h
+LIB_H += compat/precomposed_utf8.h
 LIB_H += compat/terminal.h
 LIB_H += compat/win32/dirent.h
 LIB_H += compat/win32/poll.h
@@ -1000,6 +1001,8 @@ ifeq ($(uname_S),Darwin)
 	NO_MEMMEM = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	HAVE_DEV_TTY = YesPlease
+	COMPAT_OBJS += compat/precomposed_utf8.o
+	BASIC_CFLAGS += -DPRECOMPOSED_UNICODE
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 0dacb8b..06953df 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -290,6 +290,8 @@ static int create_default_files(const char *template_path)
 		strcpy(path + len, "CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
+
+		probe_utf8_pathname_composition(path, len);
 	}
 
 	return reinit;
diff --git a/cache.h b/cache.h
index 06413e1..bc69166 100644
--- a/cache.h
+++ b/cache.h
@@ -560,6 +560,7 @@ extern int read_replace_refs;
 extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
+extern int mac_os_precomposed_unicode;
 
 enum branch_track {
 	BRANCH_TRACK_UNSPECIFIED = -1,
diff --git a/compat/precomposed_utf8.c b/compat/precomposed_utf8.c
new file mode 100644
index 0000000..f510f21
--- /dev/null
+++ b/compat/precomposed_utf8.c
@@ -0,0 +1,200 @@
+#define __PRECOMPOSED_UNICODE_C__
+
+#include "../cache.h"
+#include "../utf8.h"
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+#include "precomposed_utf8.h"
+
+const static char *repo_encoding = "UTF-8";
+const static char *path_encoding = "UTF-8-MAC";
+
+
+/* Code borrowed from utf8.c */
+#if defined(OLD_ICONV) || (defined(__sun__) && !defined(_XPG6))
+	typedef const char * iconv_ibp;
+#else
+	typedef char * iconv_ibp;
+#endif
+
+static char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv)
+{
+	size_t outsz, outalloc;
+	char *out, *outpos;
+	iconv_ibp cp;
+
+	outsz = insz;
+	outalloc = outsz + 1; /* for terminating NUL */
+	out = xmalloc(outalloc);
+	outpos = out;
+	cp = (iconv_ibp)in;
+
+	while (1) {
+		size_t cnt = iconv(conv, &cp, &insz, &outpos, &outsz);
+
+		if (cnt == -1) {
+			size_t sofar;
+			if (errno != E2BIG) {
+				free(out);
+				return NULL;
+			}
+			/* insz has remaining number of bytes.
+			 * since we started outsz the same as insz,
+			 * it is likely that insz is not enough for
+			 * converting the rest.
+			 */
+			sofar = outpos - out;
+			outalloc = sofar + insz * 2 + 32;
+			out = xrealloc(out, outalloc);
+			outpos = out + sofar;
+			outsz = outalloc - sofar - 1;
+		}
+		else {
+			*outpos = '\0';
+			break;
+		}
+	}
+	return out;
+}
+
+static size_t has_utf8(const char *s, size_t maxlen, size_t *strlen_c)
+{
+	const uint8_t *utf8p = (const uint8_t*) s;
+	size_t strlen_chars = 0;
+	size_t ret = 0;
+
+	if ((!utf8p) || (!*utf8p))
+		return 0;
+
+	while((*utf8p) && maxlen) {
+		if (*utf8p & 0x80)
+			ret++;
+		strlen_chars++;
+		utf8p++;
+		maxlen--;
+	}
+	if (strlen_c)
+		*strlen_c = strlen_chars;
+
+	return ret;
+}
+
+
+void probe_utf8_pathname_composition(char *path, int len)
+{
+	const static char *auml_nfc = "\xc3\xa4";
+	const static char *auml_nfd = "\x61\xcc\x88";
+	int output_fd;
+	if (mac_os_precomposed_unicode != -1)
+		return; /* We found it defined in the global config, respect it */
+	path[len] = 0;
+	strcpy(path + len, auml_nfc);
+	output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600);
+	if (output_fd >=0) {
+		close(output_fd);
+		path[len] = 0;
+		strcpy(path + len, auml_nfd);
+		/* Indicate the the user, that we can configure it to true */
+		if (0 == access(path, R_OK))
+			git_config_set("core.precomposedunicode", "false");
+		path[len] = 0;
+		strcpy(path + len, auml_nfc);
+		unlink(path);
+	}
+}
+
+
+void precompose_argv(int argc, const char **argv)
+{
+	int i = 0;
+	const char *oldarg;
+	char *newarg;
+	iconv_t ic_precompose;
+
+	if (mac_os_precomposed_unicode != 1)
+		return;
+
+	ic_precompose = iconv_open(repo_encoding, path_encoding);
+	if (ic_precompose == (iconv_t) -1)
+		return;
+
+	while (i < argc) {
+		size_t namelen;
+		oldarg = argv[i];
+		if (has_utf8(oldarg, (size_t)-1, &namelen)) {
+			newarg = reencode_string_iconv(oldarg, namelen, ic_precompose);
+			if (newarg)
+				argv[i] = newarg;
+		}
+		i++;
+	}
+	iconv_close(ic_precompose);
+}
+
+
+PRECOMPOSED_UTF_DIR * precomposed_utf8_opendir(const char *dirname)
+{
+	PRECOMPOSED_UTF_DIR *precomposed_utf8_dir;
+	precomposed_utf8_dir = xmalloc(sizeof(PRECOMPOSED_UTF_DIR));
+
+	precomposed_utf8_dir->dirp = opendir(dirname);
+	if (!precomposed_utf8_dir->dirp) {
+		free(precomposed_utf8_dir);
+		return NULL;
+	}
+	precomposed_utf8_dir->ic_precompose = iconv_open(repo_encoding, path_encoding);
+	if (precomposed_utf8_dir->ic_precompose == (iconv_t) -1) {
+		closedir(precomposed_utf8_dir->dirp);
+		free(precomposed_utf8_dir);
+		return NULL;
+	}
+
+	return precomposed_utf8_dir;
+}
+
+struct dirent * precomposed_utf8_readdir(PRECOMPOSED_UTF_DIR *precomposed_utf8_dirp)
+{
+	struct dirent *res;
+	size_t namelen = 0;
+
+	res = readdir(precomposed_utf8_dirp->dirp);
+	if (res && (mac_os_precomposed_unicode == 1) && has_utf8(res->d_name, (size_t)-1, &namelen)) {
+		int ret_errno = errno;
+		size_t outsz = sizeof(precomposed_utf8_dirp->dirent_nfc.d_name) - 1; /* one for \0 */
+		char *outpos = precomposed_utf8_dirp->dirent_nfc.d_name;
+		iconv_ibp cp;
+		size_t cnt;
+		size_t insz = namelen;
+		cp = (iconv_ibp)res->d_name;
+
+		/* Copy all data except the name */
+		memcpy(&precomposed_utf8_dirp->dirent_nfc, res,
+		       sizeof(precomposed_utf8_dirp->dirent_nfc)-sizeof(precomposed_utf8_dirp->dirent_nfc.d_name));
+		errno = 0;
+
+		cnt = iconv(precomposed_utf8_dirp->ic_precompose, &cp, &insz, &outpos, &outsz);
+		if (cnt < sizeof(precomposed_utf8_dirp->dirent_nfc.d_name) -1) {
+			*outpos = 0;
+			errno = ret_errno;
+			return &precomposed_utf8_dirp->dirent_nfc;
+		}
+		errno = ret_errno;
+	}
+	return res;
+}
+
+
+int precomposed_utf8_closedir(PRECOMPOSED_UTF_DIR *precomposed_utf8_dirp)
+{
+	int ret_value;
+	int ret_errno;
+	ret_value = closedir(precomposed_utf8_dirp->dirp);
+	ret_errno = errno;
+	if (precomposed_utf8_dirp->ic_precompose != (iconv_t)-1)
+		iconv_close(precomposed_utf8_dirp->ic_precompose);
+	free(precomposed_utf8_dirp);
+	errno = ret_errno;
+	return ret_value;
+}
diff --git a/compat/precomposed_utf8.h b/compat/precomposed_utf8.h
new file mode 100644
index 0000000..79e65e7
--- /dev/null
+++ b/compat/precomposed_utf8.h
@@ -0,0 +1,30 @@
+#ifndef __PRECOMPOSED_UNICODE_H__
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <iconv.h>
+
+
+typedef struct {
+	iconv_t ic_precompose;
+	DIR *dirp;
+	struct dirent dirent_nfc;
+} PRECOMPOSED_UTF_DIR;
+
+char *precompose_str(const char *in, iconv_t ic_precompose);
+void precompose_argv(int argc, const char **argv);
+void probe_utf8_pathname_composition(char *, int);
+
+PRECOMPOSED_UTF_DIR *precomposed_utf8_opendir(const char *dirname);
+struct dirent *precomposed_utf8_readdir(PRECOMPOSED_UTF_DIR *dirp);
+int precomposed_utf8_closedir(PRECOMPOSED_UTF_DIR *dirp);
+
+#ifndef __PRECOMPOSED_UNICODE_C__
+#define opendir(n) precomposed_utf8_opendir(n)
+#define readdir(d) precomposed_utf8_readdir(d)
+#define closedir(d) precomposed_utf8_closedir(d)
+#define DIR PRECOMPOSED_UTF_DIR
+#endif /* __PRECOMPOSED_UNICODE_C__ */
+
+#define  __PRECOMPOSED_UNICODE_H__
+#endif /* __PRECOMPOSED_UNICODE_H__ */
diff --git a/config.c b/config.c
index 71ef171..50d4c0e 100644
--- a/config.c
+++ b/config.c
@@ -758,6 +758,11 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.precomposedunicode")) {
+		mac_os_precomposed_unicode = git_config_bool(var, value);
+		return 0;
+	}
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/environment.c b/environment.c
index 669e498..200a04f 100644
--- a/environment.c
+++ b/environment.c
@@ -58,6 +58,7 @@ char *notes_ref_name;
 int grafts_replace_parents = 1;
 int core_apply_sparse_checkout;
 int merge_log_config = -1;
+int mac_os_precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 struct startup_info *startup_info;
 unsigned long pack_size_limit_cfg;
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 5bd9ad7..600fa9e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -153,6 +153,15 @@
 #endif
 #endif
 
+/* used on Mac OS X */
+#ifdef PRECOMPOSED_UNICODE
+#include "compat/precomposed_utf8.h"
+#else
+#define precompose_str(in,i_nfd2nfc)
+#define precompose_argv(c,v)
+#define probe_utf8_pathname_composition(a,b)
+#endif
+
 #ifndef NO_LIBGEN_H
 #include <libgen.h>
 #else
diff --git a/parse-options.c b/parse-options.c
index ab70c29..c1c66bd 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -476,6 +476,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
 		usage_with_options(usagestr, options);
 	}
 
+	precompose_argv(argc, argv);
 	return parse_options_end(&ctx);
 }
 
diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
new file mode 100755
index 0000000..b40f1c1
--- /dev/null
+++ b/t/t3910-mac-os-precompose.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Torsten Bögershausen
+#
+
+test_description='utf-8 decomposed (nfd) converted to precomposed (nfc)'
+
+. ./test-lib.sh
+
+Adiarnfc=`printf '\303\204'`
+Odiarnfc=`printf '\303\226'`
+Adiarnfd=`printf 'A\314\210'`
+Odiarnfd=`printf 'O\314\210'`
+
+# check if the feature is compiled in
+mkdir junk &&
+>junk/"$Adiarnfc" &&
+case "$(cd junk && echo *)" in
+	"$Adiarnfd")
+	test_nfd=1
+	;;
+	*)	;;
+esac
+rm -rf junk
+
+
+if test "$test_nfd"
+then
+	test_expect_success "detect if nfd needed" '
+		precomposedunicode=`git config core.precomposedunicode` &&
+		test "$precomposedunicode" = false &&
+		git config core.precomposedunicode true
+	'
+	test_expect_success "setup" '
+		>x &&
+		git add x &&
+		git commit -m "1st commit" &&
+		git rm x &&
+		git commit -m "rm x"
+	'
+	test_expect_success "setup case mac" '
+		git checkout -b mac_os
+	'
+	# This will test nfd2nfc in readdir()
+	test_expect_success "add file Adiarnfc" '
+		echo f.Adiarnfc >f.$Adiarnfc &&
+		git add f.$Adiarnfc &&
+		git commit -m "add f.$Adiarnfc"
+	'
+	# This will test nfd2nfc in git stage()
+	test_expect_success "stage file d.Adiarnfd/f.Adiarnfd" '
+		mkdir d.$Adiarnfd &&
+		echo d.$Adiarnfd/f.$Adiarnfd >d.$Adiarnfd/f.$Adiarnfd &&
+		git stage d.$Adiarnfd/f.$Adiarnfd &&
+		git commit -m "add d.$Adiarnfd/f.$Adiarnfd"
+	'
+	test_expect_success "add link Adiarnfc" '
+		ln -s d.$Adiarnfd/f.$Adiarnfd l.$Adiarnfc &&
+		git add l.$Adiarnfc &&
+		git commit -m "add l.Adiarnfc"
+	'
+	# This will test git log
+	test_expect_success "git log f.Adiar" '
+		git log f.$Adiarnfc > f.Adiarnfc.log &&
+		git log f.$Adiarnfd > f.Adiarnfd.log &&
+		test -s f.Adiarnfc.log &&
+		test -s f.Adiarnfd.log &&
+		test_cmp f.Adiarnfc.log f.Adiarnfd.log &&
+		rm f.Adiarnfc.log f.Adiarnfd.log
+	'
+	# This will test git ls-files
+	test_expect_success "git lsfiles f.Adiar" '
+		git ls-files f.$Adiarnfc > f.Adiarnfc.log &&
+		git ls-files f.$Adiarnfd > f.Adiarnfd.log &&
+		test -s f.Adiarnfc.log &&
+		test -s f.Adiarnfd.log &&
+		test_cmp f.Adiarnfc.log f.Adiarnfd.log &&
+		rm f.Adiarnfc.log f.Adiarnfd.log
+	'
+	# This will test git mv
+	test_expect_success "git mv" '
+		git mv f.$Adiarnfd f.$Odiarnfc &&
+		git mv d.$Adiarnfd d.$Odiarnfc &&
+		git mv l.$Adiarnfd l.$Odiarnfc &&
+		git commit -m "mv Adiarnfd Odiarnfc"
+	'
+	# Files can be checked out as nfc
+	# And the link has been corrected from nfd to nfc
+	test_expect_success "git checkout nfc" '
+		rm f.$Odiarnfc &&
+		git checkout f.$Odiarnfc
+	'
+	# Make it possible to checkout files with their NFD names
+	test_expect_success "git checkout file nfd" '
+		rm -f f.* &&
+		git checkout f.$Odiarnfd
+	'
+	# Make it possible to checkout links with their NFD names
+	test_expect_success "git checkout link nfd" '
+		rm l.* &&
+		git checkout l.$Odiarnfd
+	'
+	test_expect_success "setup case mac2" '
+		git checkout master &&
+		git reset --hard &&
+		git checkout -b mac_os_2
+	'
+	# This will test nfd2nfc in git commit
+	test_expect_success "commit file d2.Adiarnfd/f.Adiarnfd" '
+		mkdir d2.$Adiarnfd &&
+		echo d2.$Adiarnfd/f.$Adiarnfd >d2.$Adiarnfd/f.$Adiarnfd &&
+		git add d2.$Adiarnfd/f.$Adiarnfd &&
+		git commit -m "add d2.$Adiarnfd/f.$Adiarnfd" -- d2.$Adiarnfd/f.$Adiarnfd
+	'
+	# Test if the global core.precomposedunicode stops autosensing
+	# Must be the last test case
+	test_expect_success "respect git config --global core.precomposedunicode" '
+		git config --global core.precomposedunicode true &&
+		rm -rf .git &&
+		git init &&
+		precomposedunicode=`git config core.precomposedunicode` &&
+		test "$precomposedunicode" = "true"
+	'
+else
+	 say "Skipping nfc/nfd tests"
+fi
+
+test_done
-- 
1.7.11.1.30.g00c4fee

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC/PATCH v5] git on Mac OS and precomposed unicode
  2012-06-24 15:47 [RFC/PATCH v5] git on Mac OS and precomposed unicode Torsten Bögershausen
@ 2012-06-25 21:24 ` Junio C Hamano
  2012-06-25 22:33 ` Junio C Hamano
  1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2012-06-25 21:24 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git

Torsten Bögershausen <tboegi@web.de> writes:

> Knowing that Mac OS X writes file names as precomposed to disk,
> and treats precomposed and decomposed file names as equal, git under Mac OS X
> can be improved to revert the unicode decomposition of file names.

The basic idea looks really good, but there are a few nits.

Thanks.

> The user needs to activate this feature manually.
> She typically sets core.precomposedunicode to "true" on HFS and VFAT,
> or file systems mounted via SAMBA onto a Linux box.

What do you mean by the last part?  A Linux box that mounts a
filesystem from MacOS X box via cifs?  Or MacOS X mounts a
filesystem from a Linux box over cifs?

> +core.precomposedunicode::
> +	This option is only used by Mac OS implementation of git.
> +	When core.precomposedunicode=true, git reverts the unicode decomposition
> +	of filenames done by Mac OS. This is useful when sharing a repository
> +	between Mac OS and Linux or	Windows.

Why this funny inter-word spacing?

> +	(Git for Windows/msysGit 1.7.10 is needed, or git under cygwin 1.7).
> +	When false, file names are handled fully transparent by git, which means
> +	that file names are stored as decomposed unicode in the	repository.

I am assuming, after reading from this section, that the answer to
my earlier question is "MacOS X that mounts over cifs from whatever
file server" (i.e. the latter).  

It often is a good idea, after writing "X , which means that Y", if
you can just drop X and go strait to Y.  The result often becomes
much more clear.

As this section is clearly labeled as "Mac OS only", I think in this
case it is OK to say "when false, file names are always stored as
decomposed unicode..." for now.

Given that your compat/precomposed_utf8.[ch] looks like it does not
use anything MacOS X specific (other than "UTF-8-MAC" given to
iconv(3) API), do you foresee that this might become useful on non
Mac build of Git in the future?

> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 0dacb8b..06953df 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -290,6 +290,8 @@ static int create_default_files(const char *template_path)
>  		strcpy(path + len, "CoNfIg");
>  		if (!access(path, F_OK))
>  			git_config_set("core.ignorecase", "true");
> +
> +		probe_utf8_pathname_composition(path, len);
>  	}

Probing for case-insensitiveness and probing for UTF-8 mangling are
logically related and do not deserve a separation with the extra
blank line.

> diff --git a/compat/precomposed_utf8.c b/compat/precomposed_utf8.c
> new file mode 100644
> index 0000000..f510f21
> --- /dev/null
> +++ b/compat/precomposed_utf8.c
> @@ -0,0 +1,200 @@
> +#define __PRECOMPOSED_UNICODE_C__
> +
> +#include "../cache.h"
> +#include "../utf8.h"

Do you really need "../"?  I thought we compiled with -I.. from the
Makefile.

> +#include <stdlib.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdint.h>

And as you are including "cache.h" (which in turn includes
"git-compat-util.h"), I doubt you would need to include these
yourself, and if you do not need them, I would prefer not to see
them.

> +#include "precomposed_utf8.h"
> +
> +const static char *repo_encoding = "UTF-8";
> +const static char *path_encoding = "UTF-8-MAC";
> +
> +
> +/* Code borrowed from utf8.c */
> +#if defined(OLD_ICONV) || (defined(__sun__) && !defined(_XPG6))
> +	typedef const char * iconv_ibp;
> +#else
> +	typedef char * iconv_ibp;
> +#endif

Seeing "defined(__sun__)" here, is this not for Mac OS X-only after
all?  If so, I'd have to ask you to come up with a better name than
"mac_os_precomposed_unicode". It is primarily about sanitizing
decomposed utf-8 and it only is a happenstance that Mac OS X is the
most likely platform people may need this feature on (iow, "mac_os"
is not the fundamental part of this issue; having to sanitize paths
of decomposed UTF-8 is).

> +static char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv)
> +{
> +	size_t outsz, outalloc;
> +	char *out, *outpos;
> +	iconv_ibp cp;
> +
> +	outsz = insz;
> +	outalloc = outsz + 1; /* for terminating NUL */
> +	out = xmalloc(outalloc);
> +	outpos = out;
> +	cp = (iconv_ibp)in;
> +
> +	while (1) {
> +	...
> +	}
> +	return out;
> +}

Shouldn't this part be using a new helper function refactored out of
the utf8.c::reencode_string() function, instead of cutting and
pasting?

> +static size_t has_utf8(const char *s, size_t maxlen, size_t *strlen_c)
> +{
> +	const uint8_t *utf8p = (const uint8_t*) s;
> +	size_t strlen_chars = 0;
> +	size_t ret = 0;
> +
> +	if ((!utf8p) || (!*utf8p))
> +		return 0;
> +
> +	while((*utf8p) && maxlen) {
> +		if (*utf8p & 0x80)
> +			ret++;
> +		strlen_chars++;
> +		utf8p++;
> +		maxlen--;
> +	}
> +	if (strlen_c)
> +		*strlen_c = strlen_chars;
> +
> +	return ret;
> +}
> +
> +
> +void probe_utf8_pathname_composition(char *path, int len)
> +{
> +	const static char *auml_nfc = "\xc3\xa4";
> +	const static char *auml_nfd = "\x61\xcc\x88";
> +	int output_fd;
> +	if (mac_os_precomposed_unicode != -1)
> +		return; /* We found it defined in the global config, respect it */
> +	path[len] = 0;
> +	strcpy(path + len, auml_nfc);
> +	output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600);
> +	if (output_fd >=0) {
> +		close(output_fd);
> +		path[len] = 0;
> +		strcpy(path + len, auml_nfd);
> +		/* Indicate the the user, that we can configure it to true */
> +		if (0 == access(path, R_OK))
> +			git_config_set("core.precomposedunicode", "false");
> +		path[len] = 0;
> +		strcpy(path + len, auml_nfc);
> +		unlink(path);
> +	}

Now this function has figured out if the filesystem mangles composed
UTF-8 pathnames, shouldn't it flip mac_os_precomposed_unicode to
either 0 or 1 before it returns?

> +}
> +
> +
> +void precompose_argv(int argc, const char **argv)
> +{
> +	int i = 0;
> +	const char *oldarg;
> +	char *newarg;
> +	iconv_t ic_precompose;
> +
> +	if (mac_os_precomposed_unicode != 1)
> +		return;

Could the mac_os_precomposed_unicode flag be -1 (unknown) here?
Otherwise just write

	if (!mac_os_precomposed_unicode)
        	return;

> +PRECOMPOSED_UTF_DIR * precomposed_utf8_opendir(const char *dirname)

Asterisk sticks to the identifier, i.e.

    PRECOMPOSED_UTF_DIR *precomposed_utf8_opendir(const char *dirname)

> +{
> +	PRECOMPOSED_UTF_DIR *precomposed_utf8_dir;
> +	precomposed_utf8_dir = xmalloc(sizeof(PRECOMPOSED_UTF_DIR));
> +
> +	precomposed_utf8_dir->dirp = opendir(dirname);
> +	if (!precomposed_utf8_dir->dirp) {
> +		free(precomposed_utf8_dir);
> +		return NULL;
> +	}
> +	precomposed_utf8_dir->ic_precompose = iconv_open(repo_encoding, path_encoding);
> +	if (precomposed_utf8_dir->ic_precompose == (iconv_t) -1) {
> +		closedir(precomposed_utf8_dir->dirp);
> +		free(precomposed_utf8_dir);
> +		return NULL;

Hrm, I wonder what value the "errno" variable should be set to when
this happens.

> +	}
> +
> +	return precomposed_utf8_dir;
> +}
> +
> +struct dirent * precomposed_utf8_readdir(PRECOMPOSED_UTF_DIR *precomposed_utf8_dirp)

Likewise.

> +{
> +	struct dirent *res;
> +	size_t namelen = 0;
> +
> +	res = readdir(precomposed_utf8_dirp->dirp);
> +	if (res && (mac_os_precomposed_unicode == 1) && has_utf8(res->d_name, (size_t)-1, &namelen)) {

Likewise.

> +		int ret_errno = errno;
> +		size_t outsz = sizeof(precomposed_utf8_dirp->dirent_nfc.d_name) - 1; /* one for \0 */

A few issues:

 - Why (-1)?  Don't you need terminating NUL at the end of the
   output anyway?

 - sizeof(d_name) is most likely incorrect (Cf.
   http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/dirent.h.html)

 - Is the reason why it is appropriate to update "res" in-place
   because turning a decomposed name into precomposed form will
   always yield a shorter result?  Is that guaranteed?


> +		/* Copy all data except the name */
> +		memcpy(&precomposed_utf8_dirp->dirent_nfc, res,
> +		       sizeof(precomposed_utf8_dirp->dirent_nfc)-sizeof(precomposed_utf8_dirp->dirent_nfc.d_name));
> +		errno = 0;
> +
> +		cnt = iconv(precomposed_utf8_dirp->ic_precompose, &cp, &insz, &outpos, &outsz);
> +		if (cnt < sizeof(precomposed_utf8_dirp->dirent_nfc.d_name) -1) {

s/-1)/- 1)/;

Can't this iconv() fail and return -1 here, which would be smaller
than the size of the structure minus one?

> +			*outpos = 0;
> +			errno = ret_errno;
> +			return &precomposed_utf8_dirp->dirent_nfc;
> +		}
> +		errno = ret_errno;
> +	}
> +	return res;
> +}

> diff --git a/compat/precomposed_utf8.h b/compat/precomposed_utf8.h
> new file mode 100644
> index 0000000..79e65e7
> --- /dev/null
> +++ b/compat/precomposed_utf8.h
> @@ -0,0 +1,30 @@
> +#ifndef __PRECOMPOSED_UNICODE_H__
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <dirent.h>
> +#include <iconv.h>

The same comments for #include applies here.

> +typedef struct {
> +	iconv_t ic_precompose;
> +	DIR *dirp;
> +	struct dirent dirent_nfc;
> +} PRECOMPOSED_UTF_DIR;

Note that "struct dirent" can be defined with d_name[1]; aren't you
risking the memory after this structure to be overwritten with a
longer name?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC/PATCH v5] git on Mac OS and precomposed unicode
  2012-06-24 15:47 [RFC/PATCH v5] git on Mac OS and precomposed unicode Torsten Bögershausen
  2012-06-25 21:24 ` Junio C Hamano
@ 2012-06-25 22:33 ` Junio C Hamano
  2012-06-26  6:27   ` Torsten Bögershausen
  1 sibling, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2012-06-25 22:33 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git

Torsten Bögershausen <tboegi@web.de> writes:

> The problem:

As to the log message, I've rewritten it a bit by reordering
paragraphs and cutting redundant sentences. For exact wording nits,
please check 'pu' when I finish today's integration cycle and push
the results out, but I'll justify the reasoning behind my rewrite
here.

> Mac OS X may manipulate file names containing unicode on file systems
> HFS+, VFAT or SAMBA.
>
> When a file using unicode code points outside ASCII is created on a HFS+ drive,
> the file name is converted into decomposed unicode and written to disk.
> No conversion is done if the file name is already decomposed unicode.

I do not think it matters very much if it is written decomposed
(HFS+) or precomposed (VFAT). The important glitch that affects us
is that readdir(3) on Mac OS X gives the readers decomposed form,
unless over NFS, and the important saving grace that your patch
exploits is that stat/open/etc. will take either form and name the
same file.  So I tried to minimize the description on how it is
written to disk in my rewrite.

> The unicode decomposition creates some problems:
> - "git add" needs the decomposed form on the command line,
>   so that the file name is picked up when readdir() is called
>   to build a list of files on disk.
> - The decomposed form is not (easily) available on the keyboard.
>   To work around this, a wildcard could be used in "git add":
>   Instead of using "git add Märchen.txt" the user needs to enter
>   "git add M*rchen", "git add M<TAB>" or "git add *".
> - "git log", "git mv" and all other commands needs the decomposed form
>   to find the file name which is stored as decomposed in the index.
> - The file names are stored in decomposed unicode in the index, but
>   precomposed on disk.
>     This makes it impossible to use this repository under e.g.
>     Linux or Windows:
>     All files appear to be deleted in the decomposed form and
>     untracked in the precomposed form.

I do not think "workaround" deserves a mention; presense of mixture
of precomposed and decomposed forms is the root cause of the
problem, and even if we prefer to use precomposed form (for
interoperability if nothing else), the "workaround" to force more
decomposed input will make the problem worse, not better.

> Knowing that Mac OS X writes file names as precomposed to disk,

Again, how it writes is not important; readdir(3) giving us what is
different from what we used for creat(2) is.

> The argv[] conversion allows to use the TAB filename completion done by the
> shell on command line.

Yes, this is exactly why "workaround" is not a workaround, but is
yet another problem.

> When creating a new git repository with "git init" or "git clone",
> "core.precomposedunicode" will be set "false".
>
> The user needs to activate this feature manually.
> She typically sets core.precomposedunicode to "true" on HFS and VFAT,
> or file systems mounted via SAMBA onto a Linux box.

This we might want to change it in a couple of cycles after this
feature hits 'next' and people gain experience with it.

I think the reason to choose the safer "false" default is to keep
the behaviours between an old repository on Mac OS X and a new
repository cloned from it also on Mac OS X the same, but if we can
detect that the filesystem is broken, and have a code to work around
the breakage, I think the longer term direction would be to set it
to ensure that the resulting history records paths consistently in
precomposed form (another choice might be to normalize to decomposed
form, but my understanding is that it would not help anybody, as
nobody other than Mac OS X uses it).

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC/PATCH v5] git on Mac OS and precomposed unicode
  2012-06-25 22:33 ` Junio C Hamano
@ 2012-06-26  6:27   ` Torsten Bögershausen
  0 siblings, 0 replies; 4+ messages in thread
From: Torsten Bögershausen @ 2012-06-26  6:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git

>On 26.06.12 00:33, Junio C Hamano wrote:
[...]
Thanks Junio for the long replies.
I'll come back with better suggestion (within days, weeks or months)
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-06-26  6:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-24 15:47 [RFC/PATCH v5] git on Mac OS and precomposed unicode Torsten Bögershausen
2012-06-25 21:24 ` Junio C Hamano
2012-06-25 22:33 ` Junio C Hamano
2012-06-26  6:27   ` Torsten Bögershausen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.