Git development
 help / color / mirror / Atom feed
* [RFC][PATCH v2] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-09 16:45 UTC (permalink / raw)
  To: git; +Cc: tboegi

Allow git on Mac OS to store file names in the index in precomposed unicode,
while the file system used decomposed unicode.

When a file called "LATIN CAPITAL LETTER A WITH DIAERESIS"
(in utf-8 encoded as 0xc3 0x84) is created,
the filesystem converts "precomposed unicode" into "decomposed unicode",
which means that readdir() will return 0x41 0xcc 0x88.
When true, git reverts the unicode decomposition of filenames.
This is useful when pulling/pushing from repositories containing utf-8
encoded filenames using precomposed utf-8 (like Linux).

This feature is automatically switched on when "git init" is run,
and the file system is doing UTF-8 decompostion.
(Which has been observed on HFS+, SMBFS and VFAT, but not on NFS)
It can be switched off by setting core.macosforcenfc=false

It is implemented by re-defining the readdir() functions.
File names are converted into precomposed UTF-8.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 Documentation/config.txt     |    9 ++
 Makefile                     |    3 +
 builtin/init-db.c            |   22 +++++
 compat/darwin.c              |  208 ++++++++++++++++++++++++++++++++++++++++++
 compat/darwin.h              |   31 ++++++
 git-compat-util.h            |    8 ++
 git.c                        |    1 +
 t/t0050-filesystem.sh        |    1 +
 t/t3910-mac-os-precompose.sh |  117 +++++++++++++++++++++++
 9 files changed, 400 insertions(+), 0 deletions(-)
 create mode 100644 compat/darwin.c
 create mode 100644 compat/darwin.h
 create mode 100755 t/t3910-mac-os-precompose.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2959390..01b9465 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -175,6 +175,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 pulling/pushing from repositories containing utf-8
+	encoded filenames using precomposed unicode (like Linux).
+	When false, file names are handled fully transparent by git.
+	If in doubt, set core.precomposedunicode=false.
+
 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 b21d2f1..596900e 100644
--- a/Makefile
+++ b/Makefile
@@ -519,6 +519,7 @@ LIB_H += compat/bswap.h
 LIB_H += compat/cygwin.h
 LIB_H += compat/mingw.h
 LIB_H += compat/obstack.h
+LIB_H += compat/darwin.h
 LIB_H += compat/win32/pthread.h
 LIB_H += compat/win32/syslog.h
 LIB_H += compat/win32/poll.h
@@ -884,6 +885,8 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_MEMMEM = YesPlease
 	USE_ST_TIMESPEC = YesPlease
+	COMPAT_OBJS += compat/darwin.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..88c9de1 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -290,6 +290,28 @@ static int create_default_files(const char *template_path)
 		strcpy(path + len, "CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
+#if defined (PRECOMPOSED_UNICODE)
+		{
+			const static char *auml_nfc = "\xc3\xa4";
+			const static char *auml_nfd = "\x61\xcc\x88";
+			int output_fd;
+			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);
+				if (0 == access(path, R_OK))
+					git_config_set("core.precomposedunicode", "true");
+				else
+					git_config_set("core.precomposedunicode", "false");
+				path[len] = 0;
+				strcpy(path + len, auml_nfc);
+				unlink(path);
+			}
+		}
+#endif
 	}
 
 	return reinit;
diff --git a/compat/darwin.c b/compat/darwin.c
new file mode 100644
index 0000000..6cf73ca
--- /dev/null
+++ b/compat/darwin.c
@@ -0,0 +1,208 @@
+#define __DARWIN_C__
+
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+
+#include "../cache.h"
+#include "../utf8.h"
+
+#include "darwin.h"
+
+static int mac_os_precomposed_unicode;
+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);
+				iconv_close(conv);
+				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;
+}
+
+static int
+precomposed_unicode_config(const char *var, const char *value, void *cb)
+{
+	if (!strcasecmp(var, "core.precomposedunicode")) {
+		mac_os_precomposed_unicode = git_config_bool(var, value);
+		return 0;
+	}
+	return 1;
+}
+
+void
+argv_precompose(int argc, const char **argv)
+{
+	int i = 0;
+	int first_arg = 0; /* convert everything */
+	const char *oldarg;
+	char *newarg;
+	iconv_t ic_precompose;
+
+	git_config(precomposed_unicode_config, NULL);
+	if (!mac_os_precomposed_unicode)
+		return;
+
+	ic_precompose = iconv_open(repo_encoding, path_encoding);
+	if (ic_precompose == (iconv_t) -1)
+		return;
+
+	if (!strcmp("commit", argv[0])) {
+		first_arg = argc; /* default: convert nothing */
+
+		for (i = 0; i < argc; i++) {
+			if (!strcmp(argv[i], "--")) {
+				first_arg = i + 1; /* convert args after "--" */
+				i = argc;
+				break;
+			}
+		}
+		i = first_arg;
+	}
+	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);
+}
+
+
+DARWIN_DIR *
+darwin_opendir(const char *dirname)
+{
+	DARWIN_DIR *darwin_dir;
+	darwin_dir = malloc(sizeof(DARWIN_DIR));
+	if (!darwin_dir)
+		return NULL;
+
+	darwin_dir->dirp = opendir(dirname);
+	if (!darwin_dir->dirp) {
+		free(darwin_dir);
+		return NULL;
+	}
+	darwin_dir->ic_precompose = iconv_open(repo_encoding, path_encoding);
+	if (darwin_dir->ic_precompose == (iconv_t) -1) {
+		closedir(darwin_dir->dirp);
+		free(darwin_dir);
+		return NULL;
+	}
+
+	return darwin_dir;
+}
+
+struct dirent *
+darwin_readdir(DARWIN_DIR *darwin_dirp)
+{
+	struct dirent *res;
+	size_t namelen = 0;
+
+	res = readdir(darwin_dirp->dirp);
+	if (!res || !mac_os_precomposed_unicode || !has_utf8(res->d_name, (size_t)-1, &namelen))
+		return res;
+	else {
+		int olderrno = errno;
+		size_t outsz = sizeof(darwin_dirp->dirent_nfc.d_name) - 1; /* one for \0 */
+		char *outpos = darwin_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(&darwin_dirp->dirent_nfc, res,
+		       sizeof(darwin_dirp->dirent_nfc)-sizeof(darwin_dirp->dirent_nfc.d_name));
+		errno = 0;
+
+		cnt = iconv(darwin_dirp->ic_precompose, &cp, &insz, &outpos, &outsz);
+		if (cnt < sizeof(darwin_dirp->dirent_nfc.d_name) -1) {
+			*outpos = 0;
+			errno = olderrno;
+			return &darwin_dirp->dirent_nfc;
+		}
+		errno = olderrno;
+		return res;
+	}
+}
+
+
+int
+darwin_closedir(DARWIN_DIR *darwin_dirp)
+{
+	int ret_value;
+	ret_value = closedir(darwin_dirp->dirp);
+	if (darwin_dirp->ic_precompose != (iconv_t)-1)
+		iconv_close(darwin_dirp->ic_precompose);
+	free(darwin_dirp);
+	return ret_value;
+}
diff --git a/compat/darwin.h b/compat/darwin.h
new file mode 100644
index 0000000..094f930
--- /dev/null
+++ b/compat/darwin.h
@@ -0,0 +1,31 @@
+#ifndef __DARWIN_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;
+} DARWIN_DIR;
+
+char *str_precompose(const char *in, iconv_t ic_precompose);
+
+void argv_precompose(int argc, const char **argv);
+
+DARWIN_DIR *darwin_opendir(const char *dirname);
+struct dirent *darwin_readdir(DARWIN_DIR *dirp);
+int darwin_closedir(DARWIN_DIR *dirp);
+
+#ifndef __DARWIN_C__
+#define opendir(n) darwin_opendir(n)
+#define readdir(d) darwin_readdir(d)
+#define closedir(d) darwin_closedir(d)
+#define DIR DARWIN_DIR
+
+#endif  /* __DARWIN_C__ */
+
+#define  __DARWIN_H__
+#endif /* __DARWIN_H__ */
diff --git a/git-compat-util.h b/git-compat-util.h
index 230e198..859dfcf 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -90,6 +90,14 @@
 #include <windows.h>
 #endif
 
+#if defined (PRECOMPOSED_UNICODE)
+#include "compat/darwin.h"
+#else
+#define str_precompose(in,i_nfd2nfc) (NULL)
+#define argv_precompose(c,v)
+
+#endif
+
 #include <unistd.h>
 #include <stdio.h>
 #include <sys/stat.h>
diff --git a/git.c b/git.c
index 8e34903..6b2ffb7 100644
--- a/git.c
+++ b/git.c
@@ -298,6 +298,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 		    startup_info->have_repository) /* get_git_dir() may set up repo, avoid that */
 			trace_repo_setup(prefix);
 	}
+	argv_precompose(argc, argv);
 	commit_pager_choice();
 
 	if (!help && p->option & NEED_WORK_TREE)
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 1542cf6..befe39e 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -126,6 +126,7 @@ test_expect_success "setup unicode normalization tests" '
 
   test_create_repo unicode &&
   cd unicode &&
+  git config core.precomposedunicode false &&
   touch "$aumlcdiar" &&
   git add "$aumlcdiar" &&
   git commit -m initial &&
diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
new file mode 100755
index 0000000..439e266
--- /dev/null
+++ b/t/t3910-mac-os-precompose.sh
@@ -0,0 +1,117 @@
+#!/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'`
+
+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 --bool core.precomposedunicode` &&
+		test "$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
+	'
+else
+	 say "Skipping nfc/nfd tests"
+fi
+		#git commit -m "add d2.$Adiarnfd/f.$Adiarnfd" -- d2.$Adiarnfd/f.$Adiarnfd
+
+test_done
-- 
1.7.8.rc0.43.gb49a8

^ permalink raw reply related

* Re: [PATCH][RFC] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-09 16:42 UTC (permalink / raw)
  To: Miles Bader; +Cc: git
In-Reply-To: <87vcomravs.fsf@catnip.gol.com>

On 08.01.12 07:01, Miles Bader wrote:
> BTW, about the names, e.g. "darwin.c" etc -- is this code actually
> Darwin-specific, or simply Systems-that-happen-to-force-decomposed-
> unicode specific?
> 
> If the latter, maybe more generic names might be better.
> 
> Thanks,
> 
> -Miles
> 
As far as I know, Mac OS (darwin) is the only existing OS which likes
decomposed unicode so much, that forces decomposed unicode that way.
/Torsten


 

^ permalink raw reply

* Re: [PATCH][RFC] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-09 16:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Torsten Bögershausen
In-Reply-To: <7vboqehpxm.fsf@alter.siamese.dyndns.org>

On 08.01.12 03:46, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> Implementation:
>> Two files are added to the "compat" directory, darwin.h and darwin.c.
>> They implement basically 3 new functions:
>> darwin_opendir(), darwin_readdir() and darwin_closedir().
> 
> I haven't looked at the patch yet but that sounds exactly the right way to
> go about this. Nice.
> 
>> No decomposed file names in a git repository:
>> In order to prevent that ever a file name in decomposed unicode is entering
>> the index, a "brute force" attempt is taken:
>> all arguments into git (technically argv[1]..argv[n]) are converted into
>> precomposed unicode.
> 
> That also sounds sensible, but...
> 
>> This is done in git.c by calling argv_precompose() for all commands
>> except "git commit".
> 
> ... I think it generally is a bad idea to say "all except foo". There may
> be a reason why "foo" happens to be special in today's code, but who says
> there won't be another command "bar" that shares the same reason with
> "foo" to be treated specially? Or depending on the options, perhaps some
> codepath of "foo" may not want the special casing and want to go through
> the argv_precompose(), no?
> 
> After all, "git commit -- pathspec" will have to get the pathspec from the
> command line, and match them against the paths in the index, the latter of
> which you are keeping in the canonical form, so you would want the argv[]
> also be in the same form, and applying your argv_precompose() would be a
> sensible way to do so, no?

Thanks Junio for catching this.
I added a new test case as well as fixed the code.

> I would also suspect that the cleanest way to implement it is to replace
> the main() entry point (see how compat/mingw.h does this).

We only need to that argv conversion in git.c, (and not in daemon.c), so I sticked
to the old model for V1.
I send a new patch soon
/Torsten

^ permalink raw reply

* git grep doesn't follow symbolic link
From: Bertrand BENOIT @ 2012-01-09 16:54 UTC (permalink / raw)
  To: git

Hi,

I've not found information about that in documentation, so I do a report.

When using git grep, symbolic links are not followed.
Is it a wanted behavior ?

I've tested with a symbolic link:
 - 'ignored'
 - NOT staged for commit
 - to be commited
 - commited
Anytime -> no result when asking on symbolic link

Example:
# git grep foo mySrc
-> OK answer [...]

# ln -s mySrc test
# git grep foo test
-> KO: No answer

# git add test
# git grep foo test
-> KO: No answer

# git commit -m "DO NOT PUSH" test
# git grep foo test
-> KO: No answer

Best Regards,
Bertrand

^ permalink raw reply

* git-send-email: bug with sendemail.multiedit
From: Jean-Francois Dagenais @ 2012-01-09 19:09 UTC (permalink / raw)
  To: Pierre Habouzit, pierre.habouzit; +Cc: git

Bonjour Pierre! ... and all git developers!

I think there is a bug with git-send-email.perl's evaluation of the sendemail.multiedit config variable.

I was only able to make the "do_edit()" function detect it as false by setting the variable to "0" instead
of "false", like so:

git config --global sendemail.multiedit 0

otherwise do_edit evaluates it as true and invokes the editor with all files as argument.

All other git config boolean variables are set to either "true" or "false", not "0" or "1".

Not being too familiar with the perl language, I don't know how to fix this without spending an hour, which
is probably the amount of time I already spent narrowing the problem down already. So I leave this into
more capable hands.

cheers.

^ permalink raw reply

* Re: [PATCH][RFC] git on Mac OS and precomposed unicode
From: Junio C Hamano @ 2012-01-09 19:29 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git
In-Reply-To: <4F0B196B.8010904@web.de>

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

> On 08.01.12 03:46, Junio C Hamano wrote:
> ...
>> That also sounds sensible, but...
>> 
>>> This is done in git.c by calling argv_precompose() for all commands
>>> except "git commit".
>> 
>> ... I think it generally is a bad idea to say "all except foo". There may
>> be a reason why "foo" happens to be special in today's code, but who says
>> there won't be another command "bar" that shares the same reason with
>> "foo" to be treated specially? Or depending on the options, perhaps some
>> codepath of "foo" may not want the special casing and want to go through
>> the argv_precompose(), no?
>> 
>> After all, "git commit -- pathspec" will have to get the pathspec from the
>> command line,...
>
> Thanks Junio for catching this.
> I added a new test case as well as fixed the code.

I think you are sidestepping the real issue I raised, which is:

    What is the reason why you do not want to feed the precompose helper
    with some arguments to 'git commit', while it is OK to pass all
    arguments to other commands through precomposition?

I admit it was my fault that I did not spell it out clearly in my
response.

I understand that arguments other than pathspec and revs could be left in
decomposed form, but is there any harm in canonicalizing any and all
command line parameters given in decomposed form consistently into
precomposed form? What problem are you trying to solve by special casing
"git commit"? That is the real question to be answered, as there may be
other commands some of whose arguments may not want to be canonicalized
due to the same reason, but you simply overlooked them. When other people
need to fix that oversight, they need a clearly written criterion what
kind of arguments should not be fixed and why.

And the reason cannot be a desire to pass the value to "--message"
argument intact [*1*]; it is not like osx cannot handle text in
precomposed form, right?

In general, I do not want to see ugly code that says "this one potentially
names a path so we add a call to fix it from decomposed form, but that
other one is not a path and we take it intact" sprinkled all over in the
codebase, without a good reason.

It may seem that one alternative to munging argv[] is to have the
precomposition [*2*] applied inside get_pathspec() and have it take effect
only on the pathspecs, which after all ought to be the only place where
this matters, but I doubt it would result in maintainable code. The names
of branches and tags taken from the command line that are used as revision
names will also be compared with results from readdir in $GIT_DIR/refs/
and need to be canonicalized, for example. So I tend to agree with your
"brute force" approach to canonicalize argv[] before any real part of git
sees them; that is where my suggestion to wrap main() to do so came from.

Also some commands (e.g. "rev-list --stdin") take pathspecs and revs from
their standard input stream, so you would need to be careful about them.


[Footnotes]

*1* Also as other commands like "git merge" also take textual message, and
you do pass the helper to canonicalize it.  No, I am not suggesting you to
special case "git merge".

*2* By the way, this may need a better name if the patch touches anywhere
outside compat/osx --- it is about "canonicalize pathname and pathspec
given from the command line into the form used internally by git", and
from an osx person's point of view, the only difference might be
decomposed vs precomposed, but on other odd systems it might be that
pathnames on the filesystem may be using a different encoding from what is
used for pathnames in the index).

^ permalink raw reply

* Re: [PATCH v2 0/3] nd/index-pack-no-recurse
From: Junio C Hamano @ 2012-01-09 19:30 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Shawn O. Pearce
In-Reply-To: <1326081546-29320-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> Resend to incorporate the fixup commit from pu, no other changes.
>
> Nguyễn Thái Ngọc Duy (3):
>   Eliminate recursion in setting/clearing marks in commit list
>   index-pack: eliminate recursion in find_unresolved_deltas
>   index-pack: eliminate unlimited recursion in get_delta_base()
>
>  builtin/index-pack.c |  141 ++++++++++++++++++++++++++++++++------------------
>  commit.c             |   13 ++++-
>  revision.c           |   45 ++++++++++------
>  3 files changed, 131 insertions(+), 68 deletions(-)

Thanks.

^ permalink raw reply

* Re: [PATCH] gitignore: warn about pointless syntax
From: Junio C Hamano @ 2012-01-09 19:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Engelhardt, git, trast
In-Reply-To: <20120109162802.GA2374@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Jan 09, 2012 at 04:40:47PM +0100, Jan Engelhardt wrote:
>
>> +static inline void check_bogus_wildcard(const char *file, const char *p)
>> +{
>> +	if (strstr(p, "**") == NULL)
>> +		return;
>> +	warning(_("Pattern \"%s\" from file \"%s\": Double asterisk does not "
>> +		"have a special meaning and is interpreted just like a single "
>> +		"asterisk.\n"), file, p);
>
> Wouldn't this also match the meaningful "foo\**"?

Yes.

But trying to catch that false positive by checking one before "**"
against a backslash is not a way to do so as it will then turn "foo\\**"
into a false negative, and you would end up reimplementing fnmatch if you
really want to avoid false positives nor negatives. At that point, you may
be better off implementing git_fnmatch() instead that understands the
double-asterisk that works as some people may expect it to work ;-).

^ permalink raw reply

* Re: [RFC][PATCH v2] git on Mac OS and precomposed unicode
From: Junio C Hamano @ 2012-01-09 19:52 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git
In-Reply-To: <201201091745.30415.tboegi@web.de>

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

> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 0dacb8b..88c9de1 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -290,6 +290,28 @@ static int create_default_files(const char *template_path)
>  		strcpy(path + len, "CoNfIg");
>  		if (!access(path, F_OK))
>  			git_config_set("core.ignorecase", "true");
> +#if defined (PRECOMPOSED_UNICODE)
> +		{
> +...
> +#endif

I'd prefer just a single, unconditional call here:

		probe_utf8_pathname_composition(path, len);

with something like this at the top of the file:

	#ifndef PRECOMPOSED_UNICODE
	#define probe_utf8_pathname_composition(a,b) /* nothing */
        #else
        void probe_utf8_pathname_composition(char *, int);
	#endif

and implementation of the function body in compat/darwin.c (Didn't I see a
comment on the name of this file, by the way? What was the conclusion of
the discussion?).

> +void
> +argv_precompose(int argc, const char **argv)

Style.

> +{
> +	int i = 0;
> +	int first_arg = 0; /* convert everything */
> +	const char *oldarg;
> +	char *newarg;
> +	iconv_t ic_precompose;
> +
> +	git_config(precomposed_unicode_config, NULL);

Hmmmmm.  Is it safe to call git_config() this early in the program?  Have
we determined if we are in a git managed repository and where its $GIT_DIR
is?

^ permalink raw reply

* Re: [PATCH] rebase --fix: interactive fixup mode
From: Clemens Buchacher @ 2012-01-09 20:16 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <878vlh1bnm.fsf@thomas.inf.ethz.ch>

On Mon, Jan 09, 2012 at 10:13:33AM +0100, Thomas Rast wrote:
> 
> Given the name I would expect --fix to rebase far enough to make recent
> fixup!/squash! commits take effect.  Perhaps name it --recent?

Sure, I am not particular about the name.

> (And I also think that the 20 is rather arbitrary...)

Well, I need some kind of limit. Otherwise we will list the entire
history up to the root commit in case of repos with only linear history.
And that can be rather slow.

Clemens

^ permalink raw reply

* Re: [PATCH] rebase --fix: interactive fixup mode
From: Clemens Buchacher @ 2012-01-09 20:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfwfpg5st.fsf@alter.siamese.dyndns.org>

On Sun, Jan 08, 2012 at 02:58:42PM -0800, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> >
> > In order to determine a suitable range automatically, it is a reasonable
> > heuristic to rebase onto the most recent merge commit.
> 
> I understand the problem you are trying to solve, but I am not sure if
> this is a good idea from the UI point of view for two reasons.
> 
>  - "We want to limit the extent of the operation to commits since the last
>    merge" is by itself a reasonable thing to ask, and I do not think it
>    should be limited to "rebase". If we had an extended SHA-1 syntax to
>    express it, for example, you may want to say "I want to see what I did
>    since the last merge" and run "git log $last_merge_before_HEAD..".
>    Perhaps HEAD~{merge} or something?

Ok, sounds reasonable.

I am not sure what to do if the history has no merges, though.  If it's
just rev-parse HEAD~{merge} I suppose I could return nothing, or an
error. But what about the HEAD~{merge}..HEAD range? I think it would be
useful if that were not an error but the entire history.

>  - If your "rebase --fix" is to "fix" things, what is "rebase -i" about?

I would have suggested this to be the default behavior for rebase -i
without an <upstream> argument, but unfortunately we already handle this
case using @{upstream}.

^ permalink raw reply

* Re: Please support add -p with a new file, to add only part of the file
From: Jonathan Nieder @ 2012-01-09 20:47 UTC (permalink / raw)
  To: Josh Triplett; +Cc: git, Wincent Colaiuta, Thomas Rast, Jeff King
In-Reply-To: <20120109105134.1239.39047.reportbug@leaf>

(+cc: Wincent who brought us the "add -p" shortcut --- thanks!; Thomas,
 who expanded its scope to git checkout et al; and Jeff, who has done
 some hacking on it)
Hi Josh,

Josh Triplett wrote:

> I recently found myself with a new file that I needed to check in part
> of with several commits.  I wanted to use "git add -p newfile" and use
> 'e' to add and commit several times (along with corresponding bits in
> existing files).  However, "git add -p" does not work on a new file,
> only an existing file.

Yep.  A workaround is to use "git add -N newfile" before running
"git add -p newfile".

I imagine "git add -p '*.c'" should also offer to add hunks from
source files that git doesn't know about yet, too.

Here's a quick demo (untested) that might _almost_ do the right thing.
Unfortunately it leaves intent-to-add entries around even for files
the operator rejects.  Anyway, maybe it can be a good starting point
for playing around.

Hope that helps,
Jonathan

 git-add--interactive.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 8f0839d2..6e99ff1b 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1619,6 +1619,10 @@ sub main_loop {
 process_args();
 refresh();
 if ($patch_mode) {
+	if ($patch_mode eq 'stage') {
+		# NEEDSWORK: should use "git update-index --intent-to-add"
+		system(qw(git add --intent-to-add --), @ARGV);
+	}
 	patch_update_cmd();
 }
 else {
-- 
1.7.8.2

^ permalink raw reply related

* Re: [PATCH][RFC] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-09 20:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Torsten Bögershausen, git
In-Reply-To: <7vty44eksp.fsf@alter.siamese.dyndns.org>

On 01/09/2012 08:29 PM, Junio C Hamano wrote:
> Torsten Bögershausen<tboegi@web.de>  writes:
>
>> On 08.01.12 03:46, Junio C Hamano wrote:
>> ...
>>> That also sounds sensible, but...
>>>
>>>> This is done in git.c by calling argv_precompose() for all commands
>>>> except "git commit".
>>>
>>> ... I think it generally is a bad idea to say "all except foo". There may
>>> be a reason why "foo" happens to be special in today's code, but who says
>>> there won't be another command "bar" that shares the same reason with
>>> "foo" to be treated specially? Or depending on the options, perhaps some
>>> codepath of "foo" may not want the special casing and want to go through
>>> the argv_precompose(), no?
>>>
>>> After all, "git commit -- pathspec" will have to get the pathspec from the
>>> command line,...
>>
>> Thanks Junio for catching this.
>> I added a new test case as well as fixed the code.
>
> I think you are sidestepping the real issue I raised, which is:
>
>      What is the reason why you do not want to feed the precompose helper
>      with some arguments to 'git commit', while it is OK to pass all
>      arguments to other commands through precomposition?
>
> I admit it was my fault that I did not spell it out clearly in my
> response.
>
> I understand that arguments other than pathspec and revs could be left in
> decomposed form, but is there any harm in canonicalizing any and all
> command line parameters given in decomposed form consistently into
> precomposed form? What problem are you trying to solve by special casing
> "git commit"? That is the real question to be answered, as there may be
> other commands some of whose arguments may not want to be canonicalized
> due to the same reason, but you simply overlooked them. When other people
> need to fix that oversight, they need a clearly written criterion what
> kind of arguments should not be fixed and why.
>
> And the reason cannot be a desire to pass the value to "--message"
> argument intact [*1*]; it is not like osx cannot handle text in
> precomposed form, right?

The short answer for treating "git commit" special:
   The test suite didn't pass any more. (t4201-shortlog.sh)
   This seems more and more to be a bad excuse...
The long answer:
   I have to look into that more deeply.

Thanks for your replies.
/Torsten

     (And yes, Mac OS can handle precomposed unicode (at least the
      western european code points))

[snip]

^ permalink raw reply

* Re: [PATCH/RFC] gitweb: Fix actionless dispatch for non-existent objects
From: Junio C Hamano @ 2012-01-09 21:30 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <20120107104552.26867.41282.stgit@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> When gitweb URL does not provide action explicitly, e.g.
>
>   http://git.example.org/repo.git/branch
>
> dispatch() tries to guess action (view to be used) based on remaining
> parameters.  Among others it is based on the type of requested object,
> which gave problems when asking for non-existent branch or file (for
> example misspelt name).

Ok. "gave problems" is a bit unclear to see why explicitly calling
die_error() is an improvement, though. What is the nature of the
"problems"? Giving a server error 500 because later codepaths tried to
call an undefined subroutine?

> Now undefined $action from dispatch() should not result in problems.

Again, unspecified "problems" here. I'd like this sentence to end with
"should not result in X but gives an explicit '404 not found' error".

Thanks.

^ permalink raw reply

* Re: [PATCH/RFC] gitweb: Fix actionless dispatch for non-existent objects
From: Jakub Narebski @ 2012-01-09 22:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vlgef7o.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > When gitweb URL does not provide action explicitly, e.g.
> >
> >   http://git.example.org/repo.git/branch
> >
> > dispatch() tries to guess action (view to be used) based on remaining
> > parameters.  Among others it is based on the type of requested object,
> > which gave problems when asking for non-existent branch or file (for
> > example misspelt name)

                          , because git_get_type() returns undef when
requested object does not exist, and $action was left undefined.

This resulted in Perl generating the "Use of unitialized value" warnings,
which made it in error.log.  Additionally gitweb returned "400 Bad Request"
error instead of more informative "404 Not Found".
 
> 
> Ok. "gave problems" is a bit unclear to see why explicitly calling
> die_error() is an improvement, though. What is the nature of the
> "problems"? Giving a server error 500 because later codepaths tried to
> call an undefined subroutine?
> 
> > Now undefined $action from dispatch() should not result in problems.
> 
> Again, unspecified "problems" here. I'd like this sentence to end with
> "should not result in X but gives an explicit '404 not found' error".

This is about second chunk of change to gitweb/gitweb.perl, which is
responsible about silencing this warning:

   gitweb.perl: Use of uninitialized value in pattern match (m//) at ../gitweb.perl line 2397.

It was present even with the '$action or die_error(404,...)' short-circut,
as this was in the part responsible for generating page header, which is
the same for normal page and for error page.

Perhaps better solution would be to set action to 'object' if requested
object is not found (a valid action in itself).  Hmmm...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH v2 1/3] Eliminate recursion in setting/clearing marks in commit list
From: Junio C Hamano @ 2012-01-09 22:09 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Shawn O. Pearce
In-Reply-To: <1326081546-29320-2-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Recursion in a DAG is generally a bad idea because it could be very
> deep. Be defensive and avoid recursion in mark_parents_uninteresting()
> and clear_commit_marks().
>
> mark_parents_uninteresting() learns a trick from clear_commit_marks()
> to avoid malloc() in (dorminant) single-parent case.

Looks cleanly done.

This retains the depth-firstness of the (supposedly less common case)
recursion in mark_parents_uninteresting() from the original code, by
adding the already parsed parent at the beginning of the queue. I suspect
that the original went depth-first primarily because that was the most
straightforward way to code it, but now you have more flexibility, I
wonder if there is a difference if we made it width-first, and if so, if
the difference is positive or detrimental.

Thanks.

^ permalink raw reply

* Re: [PATCH v2 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Junio C Hamano @ 2012-01-09 22:10 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Shawn O. Pearce
In-Reply-To: <1326081546-29320-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

I find both the original and the updated code rather dense to read without
annotation, but from a cursory look all changes look good.

Thanks.

^ permalink raw reply

* Re: [PATCH] gitignore: warn about pointless syntax
From: Jeff King @ 2012-01-09 22:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jan Engelhardt, git, trast
In-Reply-To: <7vhb04ek6e.fsf@alter.siamese.dyndns.org>

On Mon, Jan 09, 2012 at 11:43:21AM -0800, Junio C Hamano wrote:

> >> +static inline void check_bogus_wildcard(const char *file, const char *p)
> >> +{
> >> +	if (strstr(p, "**") == NULL)
> >> +		return;
> >> +	warning(_("Pattern \"%s\" from file \"%s\": Double asterisk does not "
> >> +		"have a special meaning and is interpreted just like a single "
> >> +		"asterisk.\n"), file, p);
> >
> > Wouldn't this also match the meaningful "foo\**"?
> 
> Yes.
> 
> But trying to catch that false positive by checking one before "**"
> against a backslash is not a way to do so as it will then turn "foo\\**"
> into a false negative, and you would end up reimplementing fnmatch if you
> really want to avoid false positives nor negatives. At that point, you may
> be better off implementing git_fnmatch() instead that understands the
> double-asterisk that works as some people may expect it to work ;-).

You only have to implement proper backslash decoding, so I think it is
not as hard as reimplementing fnmatch:

  enum { NORMAL, QUOTED, WILDCARD } context = NORMAL;
  for (i = 0; p[i]; i++) {
          if (context == QUOTED)
                  context = NORMAL;
          else if (p[i] == '\\')
                  context = QUOTED;
          else if (p[i] == '*') {
                  if (context == WILDCARD) {
                        warning(...);
                        return;
                  }
                  context = WILDCARD;
          }
          else
                  context = NORMAL;
  }

That being said, if this is such a commonly-requested feature that we
need to be detecting and complaining about its absence, I would be much
more in favor of simply implementing it. Surely fnmatch is not that hard
to write, or we could lift code from glibc or even rsync.

Which perhaps was what you are getting at, but I am happy to say it more
explicitly. :)

-Peff

^ permalink raw reply

* leaky cherry-pick
From: Pete Wyckoff @ 2012-01-09 22:37 UTC (permalink / raw)
  To: git

I've got a big tree, and rebased 200 commits today using
cherry-pick.  It ran out of memory.  Both 1.7.4 and recent
master (eac2d83 (Git 1.7.9-rc0, 2012-01-06)) behave similarly.

Here's a valgrind dump for rebasing just 9 commits, if someone is
interested to track this down.  This was invoked as:

    valgrind --log-file=/tmp/vg.out --leak-check=full \
    /home/pw/src/git/git cherry-pick 8d535b6^..2cf53a0

I can re-test if you like, or provide more detail if this seems
unusual.

Scroll to the end to see the biggest leaks.

		-- Pete

==18789== Memcheck, a memory error detector
==18789== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==18789== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==18789== Command: /home/pw/src/git/git cherry-pick 8d535b6^..2cf53a0
==18789== Parent PID: 2325
==18789== 
==18789== Conditional jump or move depends on uninitialised value(s)
==18789==    at 0x4E37290: inflateReset2 (in /usr/lib/libz.so.1.2.3.4)
==18789==    by 0x4E3737F: inflateInit2_ (in /usr/lib/libz.so.1.2.3.4)
==18789==    by 0x5260FB: git_inflate_init (zlib.c:72)
==18789==    by 0x5025B2: unpack_sha1_header (sha1_file.c:1320)
==18789==    by 0x5028A1: unpack_sha1_file (sha1_file.c:1426)
==18789==    by 0x50447D: read_object (sha1_file.c:2220)
==18789==    by 0x504512: read_sha1_file_extended (sha1_file.c:2245)
==18789==    by 0x4D3880: read_sha1_file (cache.h:760)
==18789==    by 0x4D3E9F: parse_object (object.c:194)
==18789==    by 0x494A13: lookup_commit_reference_gently (commit.c:31)
==18789==    by 0x494A6C: lookup_commit_reference (commit.c:40)
==18789==    by 0x50691A: get_parent (sha1_name.c:371)
==18789== 
==18789== Conditional jump or move depends on uninitialised value(s)
==18789==    at 0x4E37290: inflateReset2 (in /usr/lib/libz.so.1.2.3.4)
==18789==    by 0x4E3737F: inflateInit2_ (in /usr/lib/libz.so.1.2.3.4)
==18789==    by 0x5260FB: git_inflate_init (zlib.c:72)
==18789==    by 0x502E9E: unpack_compressed_entry (sha1_file.c:1629)
==18789==    by 0x503810: unpack_entry (sha1_file.c:1880)
==18789==    by 0x5030B5: cache_or_unpack_entry (sha1_file.c:1690)
==18789==    by 0x5041E4: read_packed_sha1 (sha1_file.c:2162)
==18789==    by 0x50442F: read_object (sha1_file.c:2215)
==18789==    by 0x504512: read_sha1_file_extended (sha1_file.c:2245)
==18789==    by 0x494981: read_sha1_file (cache.h:760)
==18789==    by 0x49561E: parse_commit (commit.c:317)
==18789==    by 0x4F52DC: add_parents_to_list (revision.c:543)
==18789== 
==18789== Warning: set address range perms: large range [0x133a0000, 0x37dad000) (defined)
==18789== Warning: set address range perms: large range [0x4c438000, 0x7b85a000) (defined)
==18789== Conditional jump or move depends on uninitialised value(s)
==18789==    at 0x537450F: __strspn_sse42 (strspn-c.c:126)
==18789==    by 0x488910: parse_attr (attr.c:189)
==18789==    by 0x488ABC: parse_attr_line (attr.c:232)
==18789==    by 0x488CFB: handle_attr_line (attr.c:320)
==18789==    by 0x488E86: read_attr_from_file (attr.c:358)
==18789==    by 0x489130: read_attr (attr.c:428)
==18789==    by 0x489335: bootstrap_attr_stack (attr.c:525)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789== 
==18789== Conditional jump or move depends on uninitialised value(s)
==18789==    at 0x488AC6: parse_attr_line (attr.c:233)
==18789==    by 0x488CFB: handle_attr_line (attr.c:320)
==18789==    by 0x488E86: read_attr_from_file (attr.c:358)
==18789==    by 0x489130: read_attr (attr.c:428)
==18789==    by 0x489335: bootstrap_attr_stack (attr.c:525)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789==    by 0x4B69B1: write_entry (entry.c:191)
==18789==    by 0x4B6FDD: checkout_entry (entry.c:318)
==18789== 
==18789== Use of uninitialised value of size 8
==18789==    at 0x488ADA: parse_attr_line (attr.c:231)
==18789==    by 0x488CFB: handle_attr_line (attr.c:320)
==18789==    by 0x488E86: read_attr_from_file (attr.c:358)
==18789==    by 0x489130: read_attr (attr.c:428)
==18789==    by 0x489335: bootstrap_attr_stack (attr.c:525)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789==    by 0x4B69B1: write_entry (entry.c:191)
==18789==    by 0x4B6FDD: checkout_entry (entry.c:318)
==18789== 
==18789== Conditional jump or move depends on uninitialised value(s)
==18789==    at 0x537450F: __strspn_sse42 (strspn-c.c:126)
==18789==    by 0x488910: parse_attr (attr.c:189)
==18789==    by 0x488BD1: parse_attr_line (attr.c:253)
==18789==    by 0x488CFB: handle_attr_line (attr.c:320)
==18789==    by 0x488E86: read_attr_from_file (attr.c:358)
==18789==    by 0x489130: read_attr (attr.c:428)
==18789==    by 0x489335: bootstrap_attr_stack (attr.c:525)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789== 
==18789== Use of uninitialised value of size 8
==18789==    at 0x488BDE: parse_attr_line (attr.c:252)
==18789==    by 0x488CFB: handle_attr_line (attr.c:320)
==18789==    by 0x488E86: read_attr_from_file (attr.c:358)
==18789==    by 0x489130: read_attr (attr.c:428)
==18789==    by 0x489335: bootstrap_attr_stack (attr.c:525)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789==    by 0x4B69B1: write_entry (entry.c:191)
==18789==    by 0x4B6FDD: checkout_entry (entry.c:318)
==18789== 
==18789== 
==18789== HEAP SUMMARY:
==18789==     in use at exit: 511,786,999 bytes in 3,954,180 blocks
==18789==   total heap usage: 6,352,564 allocs, 2,398,384 frees, 2,610,529,433 bytes allocated
==18789== 
==18789== 1 bytes in 1 blocks are definitely lost in loss record 2 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x52DC421: strdup (strdup.c:43)
==18789==    by 0x51FDC8: xstrdup (wrapper.c:23)
==18789==    by 0x4B7181: expand_namespace (environment.c:104)
==18789==    by 0x4B7480: setup_git_env (environment.c:144)
==18789==    by 0x4B750B: get_git_dir (environment.c:162)
==18789==    by 0x4D918B: git_vsnpath (path.c:53)
==18789==    by 0x4D93D8: git_pathdup (path.c:85)
==18789==    by 0x499E96: git_config (config.c:925)
==18789==    by 0x4047A4: check_pager_config (git.c:48)
==18789==    by 0x40522D: run_builtin (git.c:293)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789== 
==18789== 5 bytes in 1 blocks are definitely lost in loss record 6 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x52DC421: strdup (strdup.c:43)
==18789==    by 0x51FDC8: xstrdup (wrapper.c:23)
==18789==    by 0x4986C9: git_config_string (config.c:475)
==18789==    by 0x4990B4: git_default_core_config (config.c:646)
==18789==    by 0x499955: git_default_config (config.c:796)
==18789==    by 0x497E22: get_value (config.c:222)
==18789==    by 0x498150: git_parse_file (config.c:330)
==18789==    by 0x499BFB: git_config_from_file (config.c:847)
==18789==    by 0x499DD5: git_config_early (config.c:896)
==18789==    by 0x499EB1: git_config (config.c:926)
==18789==    by 0x476839: cmd_cherry_pick (revert.c:1160)
==18789== 
==18789== 11 bytes in 1 blocks are definitely lost in loss record 8 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x4B73F8: setup_git_env (environment.c:136)
==18789==    by 0x4B750B: get_git_dir (environment.c:162)
==18789==    by 0x4D918B: git_vsnpath (path.c:53)
==18789==    by 0x4D93D8: git_pathdup (path.c:85)
==18789==    by 0x499E96: git_config (config.c:925)
==18789==    by 0x4047A4: check_pager_config (git.c:48)
==18789==    by 0x40522D: run_builtin (git.c:293)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789==    by 0x40571B: main (git.c:588)
==18789== 
==18789== 13 bytes in 1 blocks are definitely lost in loss record 10 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x4B737B: setup_git_env (environment.c:131)
==18789==    by 0x4B750B: get_git_dir (environment.c:162)
==18789==    by 0x4D918B: git_vsnpath (path.c:53)
==18789==    by 0x4D93D8: git_pathdup (path.c:85)
==18789==    by 0x499E96: git_config (config.c:925)
==18789==    by 0x4047A4: check_pager_config (git.c:48)
==18789==    by 0x40522D: run_builtin (git.c:293)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789==    by 0x40571B: main (git.c:588)
==18789== 
==18789== 16 bytes in 1 blocks are definitely lost in loss record 14 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x49578D: commit_list_insert (commit.c:356)
==18789==    by 0x495886: commit_list_insert_by_date (commit.c:390)
==18789==    by 0x4F51BC: commit_list_insert_by_date_cached (revision.c:508)
==18789==    by 0x4F5344: add_parents_to_list (revision.c:550)
==18789==    by 0x4F5C62: limit_list (revision.c:836)
==18789==    by 0x4FA49B: prepare_revision_walk (revision.c:2068)
==18789==    by 0x47522F: prepare_revs (revert.c:650)
==18789==    by 0x475B6D: walk_revs_populate_todo (revert.c:855)
==18789==    by 0x4766BF: pick_revisions (revert.c:1123)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789== 
==18789== 17 bytes in 1 blocks are definitely lost in loss record 17 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x52DC421: strdup (strdup.c:43)
==18789==    by 0x51FDC8: xstrdup (wrapper.c:23)
==18789==    by 0x4D93E7: git_pathdup (path.c:87)
==18789==    by 0x4B744E: setup_git_env (environment.c:141)
==18789==    by 0x4B750B: get_git_dir (environment.c:162)
==18789==    by 0x4D918B: git_vsnpath (path.c:53)
==18789==    by 0x4D93D8: git_pathdup (path.c:85)
==18789==    by 0x499E96: git_config (config.c:925)
==18789==    by 0x4047A4: check_pager_config (git.c:48)
==18789==    by 0x40522D: run_builtin (git.c:293)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789== 
==18789== 24 bytes in 1 blocks are definitely lost in loss record 21 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x4C27927: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FFCD: xrealloc (wrapper.c:82)
==18789==    by 0x509A01: strbuf_grow (strbuf.c:72)
==18789==    by 0x50A03A: strbuf_add (strbuf.c:190)
==18789==    by 0x4D9CB4: expand_user_path (path.c:255)
==18789==    by 0x49870C: git_config_pathname (config.c:483)
==18789==    by 0x499186: git_default_core_config (config.c:655)
==18789==    by 0x499955: git_default_config (config.c:796)
==18789==    by 0x497E22: get_value (config.c:222)
==18789==    by 0x498150: git_parse_file (config.c:330)
==18789==    by 0x499BFB: git_config_from_file (config.c:847)
==18789== 
==18789== 40 bytes in 8 blocks are definitely lost in loss record 23 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x52DC421: strdup (strdup.c:43)
==18789==    by 0x51FDC8: xstrdup (wrapper.c:23)
==18789==    by 0x4986C9: git_config_string (config.c:475)
==18789==    by 0x4990B4: git_default_core_config (config.c:646)
==18789==    by 0x499955: git_default_config (config.c:796)
==18789==    by 0x525EBD: git_xmerge_config (xdiff-interface.c:362)
==18789==    by 0x4CD25A: merge_recursive_config (merge-recursive.c:2032)
==18789==    by 0x497E22: get_value (config.c:222)
==18789==    by 0x498150: git_parse_file (config.c:330)
==18789==    by 0x499BFB: git_config_from_file (config.c:847)
==18789==    by 0x499DD5: git_config_early (config.c:896)
==18789== 
==18789== 65 bytes in 1 blocks are definitely lost in loss record 30 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x4C27927: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FFCD: xrealloc (wrapper.c:82)
==18789==    by 0x509A01: strbuf_grow (strbuf.c:72)
==18789==    by 0x50A1E3: strbuf_vaddf (strbuf.c:216)
==18789==    by 0x50A1AA: strbuf_addf (strbuf.c:206)
==18789==    by 0x475CAF: save_head (revert.c:884)
==18789==    by 0x476737: pick_revisions (revert.c:1131)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789== 
==18789== 144 (16 direct, 128 indirect) bytes in 1 blocks are definitely lost in loss record 41 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x47536C: commit_list_append (revert.c:692)
==18789==    by 0x475B8A: walk_revs_populate_todo (revert.c:859)
==18789==    by 0x4766BF: pick_revisions (revert.c:1123)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789==    by 0x40571B: main (git.c:588)
==18789== 
==18789== 192 bytes in 8 blocks are definitely lost in loss record 48 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x4C27927: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FFCD: xrealloc (wrapper.c:82)
==18789==    by 0x509A01: strbuf_grow (strbuf.c:72)
==18789==    by 0x50A03A: strbuf_add (strbuf.c:190)
==18789==    by 0x4D9CB4: expand_user_path (path.c:255)
==18789==    by 0x49870C: git_config_pathname (config.c:483)
==18789==    by 0x499186: git_default_core_config (config.c:655)
==18789==    by 0x499955: git_default_config (config.c:796)
==18789==    by 0x525EBD: git_xmerge_config (xdiff-interface.c:362)
==18789==    by 0x4CD25A: merge_recursive_config (merge-recursive.c:2032)
==18789==    by 0x497E22: get_value (config.c:222)
==18789== 
==18789== 281 bytes in 2 blocks are possibly lost in loss record 51 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x4E483E: create_from_disk (read-cache.c:1255)
==18789==    by 0x4E4B7F: read_index_from (read-cache.c:1328)
==18789==    by 0x4E4758: read_index (read-cache.c:1224)
==18789==    by 0x47469F: do_recursive_merge (revert.c:411)
==18789==    by 0x474FA5: do_pick_commit (revert.c:598)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789==    by 0x476756: pick_revisions (revert.c:1133)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789== 
==18789== 704 bytes in 8 blocks are definitely lost in loss record 58 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x51AFD1: create_ce_entry (unpack-trees.c:532)
==18789==    by 0x51B1AE: unpack_nondirectories (unpack-trees.c:582)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51C1F5: unpack_trees (unpack-trees.c:1063)
==18789==    by 0x4A46BD: diff_cache (diff-lib.c:474)
==18789==    by 0x4A46FC: run_diff_index (diff-lib.c:482)
==18789==    by 0x4A48DA: index_differs_from (diff-lib.c:517)
==18789==    by 0x474AC2: do_pick_commit (revert.c:503)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789== 
==18789== 1,107 bytes in 9 blocks are definitely lost in loss record 62 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x519DCD: setup_unpack_trees_porcelain (unpack-trees.c:79)
==18789==    by 0x4C8160: git_merge_trees (merge-recursive.c:235)
==18789==    by 0x4CC907: merge_trees (merge-recursive.c:1817)
==18789==    by 0x4747B8: do_recursive_merge (revert.c:425)
==18789==    by 0x474FA5: do_pick_commit (revert.c:598)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789==    by 0x476756: pick_revisions (revert.c:1133)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789== 
==18789== 1,143 bytes in 9 blocks are definitely lost in loss record 63 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x519E8F: setup_unpack_trees_porcelain (unpack-trees.c:82)
==18789==    by 0x4C8160: git_merge_trees (merge-recursive.c:235)
==18789==    by 0x4CC907: merge_trees (merge-recursive.c:1817)
==18789==    by 0x4747B8: do_recursive_merge (revert.c:425)
==18789==    by 0x474FA5: do_pick_commit (revert.c:598)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789==    by 0x476756: pick_revisions (revert.c:1133)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789== 
==18789== 1,269 bytes in 9 blocks are definitely lost in loss record 64 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x519CE0: setup_unpack_trees_porcelain (unpack-trees.c:66)
==18789==    by 0x4C8160: git_merge_trees (merge-recursive.c:235)
==18789==    by 0x4CC907: merge_trees (merge-recursive.c:1817)
==18789==    by 0x4747B8: do_recursive_merge (revert.c:425)
==18789==    by 0x474FA5: do_pick_commit (revert.c:598)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789==    by 0x476756: pick_revisions (revert.c:1133)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789== 
==18789== 1,316 bytes in 10 blocks are possibly lost in loss record 65 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x51AFD1: create_ce_entry (unpack-trees.c:532)
==18789==    by 0x51B1AE: unpack_nondirectories (unpack-trees.c:582)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789== 
==18789== 1,728 bytes in 9 blocks are definitely lost in loss record 67 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x4C27927: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FFCD: xrealloc (wrapper.c:82)
==18789==    by 0x488D5B: handle_attr_line (attr.c:325)
==18789==    by 0x488DDD: read_attr_from_array (attr.c:340)
==18789==    by 0x48924E: bootstrap_attr_stack (attr.c:501)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789==    by 0x4B69B1: write_entry (entry.c:191)
==18789== 
==18789== 2,072 (1,496 direct, 576 indirect) bytes in 1 blocks are definitely lost in loss record 70 of 97
==18789==    at 0x4C2779D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FE4E: xmalloc (wrapper.c:35)
==18789==    by 0x473F5B: parse_args (revert.c:228)
==18789==    by 0x47684E: cmd_cherry_pick (revert.c:1161)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789==    by 0x40571B: main (git.c:588)
==18789== 
==18789== 2,376 bytes in 27 blocks are definitely lost in loss record 72 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x51AFD1: create_ce_entry (unpack-trees.c:532)
==18789==    by 0x51B1AE: unpack_nondirectories (unpack-trees.c:582)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51C1F5: unpack_trees (unpack-trees.c:1063)
==18789==    by 0x4C81C2: git_merge_trees (merge-recursive.c:241)
==18789==    by 0x4CC907: merge_trees (merge-recursive.c:1817)
==18789==    by 0x4747B8: do_recursive_merge (revert.c:425)
==18789==    by 0x474FA5: do_pick_commit (revert.c:598)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789== 
==18789== 7,726 (4,320 direct, 3,406 indirect) bytes in 9 blocks are definitely lost in loss record 77 of 97
==18789==    at 0x4C27882: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FFCD: xrealloc (wrapper.c:82)
==18789==    by 0x488D5B: handle_attr_line (attr.c:325)
==18789==    by 0x488E86: read_attr_from_file (attr.c:358)
==18789==    by 0x489130: read_attr (attr.c:428)
==18789==    by 0x489335: bootstrap_attr_stack (attr.c:525)
==18789==    by 0x48940F: prepare_attr_stack (attr.c:568)
==18789==    by 0x489A27: collect_all_attrs (attr.c:725)
==18789==    by 0x489AED: git_check_attr (attr.c:739)
==18789==    by 0x49E314: convert_attrs (convert.c:730)
==18789==    by 0x49F230: get_stream_filter (convert.c:1247)
==18789==    by 0x4B69B1: write_entry (entry.c:191)
==18789== 
==18789== 744,353 bytes in 7,493 blocks are definitely lost in loss record 88 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x51AFD1: create_ce_entry (unpack-trees.c:532)
==18789==    by 0x51B1AE: unpack_nondirectories (unpack-trees.c:582)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51C1F5: unpack_trees (unpack-trees.c:1063)
==18789==    by 0x4A46BD: diff_cache (diff-lib.c:474)
==18789==    by 0x4A46FC: run_diff_index (diff-lib.c:482)
==18789== 
==18789== 1,865,981 bytes in 18,806 blocks are definitely lost in loss record 90 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x51AFD1: create_ce_entry (unpack-trees.c:532)
==18789==    by 0x51B1AE: unpack_nondirectories (unpack-trees.c:582)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51C1F5: unpack_trees (unpack-trees.c:1063)
==18789==    by 0x4C81C2: git_merge_trees (merge-recursive.c:241)
==18789==    by 0x4CC907: merge_trees (merge-recursive.c:1817)
==18789== 
==18789== 8,386,453 (1,031,576 direct, 7,354,877 indirect) bytes in 1 blocks are definitely lost in loss record 91 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x4E4B38: read_index_from (read-cache.c:1319)
==18789==    by 0x4E4758: read_index (read-cache.c:1224)
==18789==    by 0x4DB50F: read_index_preload (preload-index.c:105)
==18789==    by 0x4752A1: read_and_refresh_cache (revert.c:661)
==18789==    by 0x47657B: pick_revisions (revert.c:1081)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789==    by 0x40571B: main (git.c:588)
==18789== 
==18789== 69,782,106 (6,793,984 direct, 62,988,122 indirect) bytes in 8 blocks are definitely lost in loss record 94 of 97
==18789==    at 0x4C27882: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x51FFCD: xrealloc (wrapper.c:82)
==18789==    by 0x4E3D7C: add_index_entry (read-cache.c:976)
==18789==    by 0x519FFE: add_entry (unpack-trees.c:119)
==18789==    by 0x51D0A6: merged_entry (unpack-trees.c:1501)
==18789==    by 0x51D444: threeway_merge (unpack-trees.c:1615)
==18789==    by 0x51A645: call_unpack_fn (unpack-trees.c:289)
==18789==    by 0x51B1E2: unpack_nondirectories (unpack-trees.c:586)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789== 
==18789== 105,619,300 (9,284,184 direct, 96,335,116 indirect) bytes in 9 blocks are definitely lost in loss record 96 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x4E4B38: read_index_from (read-cache.c:1319)
==18789==    by 0x4E4758: read_index (read-cache.c:1224)
==18789==    by 0x47469F: do_recursive_merge (revert.c:411)
==18789==    by 0x474FA5: do_pick_commit (revert.c:598)
==18789==    by 0x47635D: pick_commits (revert.c:1022)
==18789==    by 0x476756: pick_revisions (revert.c:1133)
==18789==    by 0x47685A: cmd_cherry_pick (revert.c:1162)
==18789==    by 0x4052E1: run_builtin (git.c:308)
==18789==    by 0x405474: handle_internal_command (git.c:467)
==18789==    by 0x40558E: run_argv (git.c:513)
==18789== 
==18789== 313,333,787 bytes in 2,509,382 blocks are definitely lost in loss record 97 of 97
==18789==    at 0x4C25E84: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18789==    by 0x520084: xcalloc (wrapper.c:98)
==18789==    by 0x51AFD1: create_ce_entry (unpack-trees.c:532)
==18789==    by 0x51B1AE: unpack_nondirectories (unpack-trees.c:582)
==18789==    by 0x51B843: unpack_callback (unpack-trees.c:772)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789==    by 0x51AD8D: traverse_trees_recursive (unpack-trees.c:460)
==18789==    by 0x51B973: unpack_callback (unpack-trees.c:809)
==18789==    by 0x5190F8: traverse_trees (tree-walk.c:407)
==18789== 
==18789== LEAK SUMMARY:
==18789==    definitely lost: 333,068,408 bytes in 2,535,805 blocks
==18789==    indirectly lost: 166,682,225 bytes in 1,331,950 blocks
==18789==      possibly lost: 1,597 bytes in 12 blocks
==18789==    still reachable: 12,034,769 bytes in 86,413 blocks
==18789==         suppressed: 0 bytes in 0 blocks
==18789== Reachable blocks (those to which a pointer was found) are not shown.
==18789== To see them, rerun with: --leak-check=full --show-reachable=yes
==18789== 
==18789== For counts of detected and suppressed errors, rerun with: -v
==18789== Use --track-origins=yes to see where uninitialised values come from
==18789== ERROR SUMMARY: 241596 errors from 34 contexts (suppressed: 4 from 4)

^ permalink raw reply

* Re: [PATCH v2 3/3] index-pack: eliminate unlimited recursion in get_delta_base()
From: Junio C Hamano @ 2012-01-09 22:51 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Shawn O. Pearce
In-Reply-To: <1326081546-29320-4-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Revert the order of delta applying so that by the time a delta is
> applied, its base is either non-delta or already inflated.
>
> get_delta_base() is still recursive, but because base's data is always
> ready, the inner get_delta_base() call never has any chance to call
> itself again.

I suspect s/Revert/Reverse/ here, but I have a feeling that the structure
of the resulting code is a bit too complex and subtle.

In parse_pack_objects(), we have two passes. The first pass scans to
enumerate the objects that appear in the pack and sift them into base and
delta objects, and the second one starts from a base object, resolves its
immediate children with find_unresolved_daltas(), but that function recurses
many times, bound only by the number of objects in the pack, which is the
issue you are trying to address with this series.

I wonder if a cleaner approach is to change the loop in the second pass in
such a way that (1) the function it calls resolves _only_ the immediate
children of the object we know its final shape (either because the object
was recorded in the deflated form in the pack, or we have already resolved
it in earlier iteration), and (2) the loop goes over the objects[] array
not just once, but until we stopped making progress.

It would require us to be able to tell, by looking at objects[i], if the
object itself has already been handled (perhaps you can look at its
idx.sha1 field for this purpose) and if we have already handled its
immediate delta children (you may need to add a bit to struct object_entry
for this).

^ permalink raw reply

* Re: git-send-email: bug with sendemail.multiedit
From: Jeff King @ 2012-01-09 22:55 UTC (permalink / raw)
  To: Jean-Francois Dagenais
  Cc: Junio C Hamano, Pierre Habouzit, pierre.habouzit, git
In-Reply-To: <1AC16B4B-8376-4A50-A900-BB8E704FAB82@gmail.com>

On Mon, Jan 09, 2012 at 02:09:30PM -0500, Jean-Francois Dagenais wrote:

> I think there is a bug with git-send-email.perl's evaluation of the
> sendemail.multiedit config variable.
> 
> I was only able to make the "do_edit()" function detect it as false by
> setting the variable to "0" instead of "false", like so:

I think it's this:

-- >8 --
Subject: [PATCH] send-email: multiedit is a boolean config option

The sendemail.multiedit variable is meant to be a boolean.
However, it is not marked as such in the code, which means
we store its value literally. Thus in the do_edit function,
perl ends up coercing it to a boolean value according to
perl rules, not git rules. This works for "0", but "false",
"no", or "off" will erroneously be interpreted as true.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d491db9..ef30c55 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -210,6 +210,7 @@ my %config_bool_settings = (
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
     "validate" => [\$validate, 1],
+    "multiedit" => [\$multiedit, undef]
 );
 
 my %config_settings = (
@@ -227,7 +228,6 @@ my %config_settings = (
     "bcc" => \@bcclist,
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
-    "multiedit" => \$multiedit,
     "confirm"   => \$confirm,
     "from" => \$sender,
     "assume8bitencoding" => \$auto_8bit_encoding,
-- 
1.7.8

^ permalink raw reply related

* Re: git-send-email: bug with sendemail.multiedit
From: Junio C Hamano @ 2012-01-09 23:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Jean-Francois Dagenais, git
In-Reply-To: <20120109225542.GB9902@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Jan 09, 2012 at 02:09:30PM -0500, Jean-Francois Dagenais wrote:
>
>> I think there is a bug with git-send-email.perl's evaluation of the
>> sendemail.multiedit config variable.
>> 
>> I was only able to make the "do_edit()" function detect it as false by
>> setting the variable to "0" instead of "false", like so:
>
> I think it's this:
>
> -- >8 --
> Subject: [PATCH] send-email: multiedit is a boolean config option
>
> The sendemail.multiedit variable is meant to be a boolean.
> However, it is not marked as such in the code, which means
> we store its value literally. Thus in the do_edit function,
> perl ends up coercing it to a boolean value according to
> perl rules, not git rules. This works for "0", but "false",
> "no", or "off" will erroneously be interpreted as true.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Looks like it.

And in case anybody is wondering by looking at the context, confirm is not
a boolean "do you want a confirmation or not?", so it can stay where it is.

Thanks.

>  git-send-email.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d491db9..ef30c55 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -210,6 +210,7 @@ my %config_bool_settings = (
>      "signedoffbycc" => [\$signed_off_by_cc, undef],
>      "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
>      "validate" => [\$validate, 1],
> +    "multiedit" => [\$multiedit, undef]
>  );
>  
>  my %config_settings = (
> @@ -227,7 +228,6 @@ my %config_settings = (
>      "bcc" => \@bcclist,
>      "suppresscc" => \@suppress_cc,
>      "envelopesender" => \$envelope_sender,
> -    "multiedit" => \$multiedit,
>      "confirm"   => \$confirm,
>      "from" => \$sender,
>      "assume8bitencoding" => \$auto_8bit_encoding,

^ permalink raw reply

* Re: leaky cherry-pick
From: Junio C Hamano @ 2012-01-09 23:19 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: git, Pete Wyckoff
In-Reply-To: <20120109223737.GA1589@padd.com>

Pete Wyckoff <pw@padd.com> writes:

> I've got a big tree, and rebased 200 commits today using
> cherry-pick.  It ran out of memory....
>
>     valgrind --log-file=/tmp/vg.out --leak-check=full \
>     /home/pw/src/git/git cherry-pick 8d535b6^..2cf53a0

The cherry-pick and revert machinery was written to be single-shot and the
multi-pick feature was bolted on to it later; I am not so surprised if
nobody carefully looked at the memory usage when that happened.

Ram?

^ permalink raw reply

* Re: [PATCH v2 3/6] clone: factor out checkout code
From: Junio C Hamano @ 2012-01-10  0:32 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King
In-Reply-To: <1326023188-15559-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Read HEAD from disk instead of relying on local variable
> our_head_points_at, so that if earlier code fails to make HEAD
> properly, it'll be detected.

The end result might be more or less the same with your patch from the
end-user's point of view, but "if earlier code fails", shouldn't you
detect and diagnose it right there?

If you observe lack of "HEAD" in checkout(), you cannot tell if that was
because the remote did not have anything usable in the first place, or
because we knew where it should point at (and may have even attempted to
create it) but somehow failed to make it point at it.

^ permalink raw reply

* Re: [PATCH v2 4/6] clone: --branch=<branch> always means refs/heads/<branch>
From: Junio C Hamano @ 2012-01-10  0:33 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King
In-Reply-To: <1326023188-15559-4-git-send-email-pclouds@gmail.com>

Up to this point I find the series makes sense.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox