[PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

These config variables are parsed to substitute ~ and ~user with getpw
entries.

user_path() refactored into new function expand_user_path(), to allow
dynamically allocating the return buffer.

Original patch by Karl Chen, modified by Matthieu Moy.

Signed-off-by: Karl Chen <quarl@quarl.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
The original thread was here:

http://thread.gmane.org/gmane.comp.version-control.git/93250/focus=94276

My version is made a bit simpler by using strbuf for string
manipulation in expand_user_path.

I'm not sure I fully adressed Junio's point here:
http://thread.gmane.org/gmane.comp.version-control.git/93250/focus=94276

> I do not see any strong reason why the single caller of user_path() has to
> keep using the static allocation.  Would it help to reduce the complexity
> of your expand_user_path() implementation, if we fixed the caller along
> the lines of this patch (untested, but just to illustrate the point)?
> 
> [...]
> --- i/path.c
> +++ w/path.c
> @@ -221,19 +221,22 @@ char *enter_repo(char *path, int strict)
>  		if (PATH_MAX <= len)
>  			return NULL;
>  		if (path[0] == '~') {
> -			if (!user_path(used_path, path, PATH_MAX))
> +			char *newpath = expand_user_path(path);
> [...]

I'm just copying back into the static buffer to let enter_repo() do
the same string manipulation as it used to do (concatenate with .git
suffixes). I think the whole enter_repo could use strbuf instead of
static buffers, but that's a different point (probably made easier by
my patch).

 Documentation/config.txt |    4 ++-
 builtin-commit.c         |    2 +-
 cache.h                  |    2 +
 config.c                 |   11 +++++-
 path.c                   |   84 +++++++++++++++++++++++++++------------------
 5 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d1e2120..c37b51d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,7 +380,8 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  See
+	of files which are not meant to be tracked.  "~" and "~user"
+	are expanded to the user's home directory.  See
 	linkgit:gitignore[5].
 
 core.editor::
@@ -670,6 +671,7 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
+	"~" and "~user" are expanded to the user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/builtin-commit.c b/builtin-commit.c
index d525b89..09d2840 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -999,7 +999,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!strcmp(k, "commit.template"))
-		return git_config_string(&template_file, k, v);
+		return git_config_pathname(&template_file, k, v);
 
 	return git_status_config(k, v, s);
 }
diff --git a/cache.h b/cache.h
index 71a731d..42f7cd8 100644
--- a/cache.h
+++ b/cache.h
@@ -645,6 +645,7 @@ int set_shared_perm(const char *path, int mode);
 #define adjust_shared_perm(path) set_shared_perm((path), 0)
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
+extern char *expand_user_path(const char *path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
@@ -903,6 +904,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index c644061..0fcc4ce 100644
--- a/config.c
+++ b/config.c
@@ -351,6 +351,15 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_pathname(const char **dest, const char *var, const char *value) {
+	if (!value)
+		return config_error_nonbool(var);
+	*dest = expand_user_path(value);
+	if (!*dest)
+		die("Failed to expand user dir in: '%s'", value);
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -474,7 +483,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return git_config_string(&editor_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile"))
-		return git_config_string(&excludes_file, var, value);
+		return git_config_pathname(&excludes_file, var, value);
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git a/path.c b/path.c
index 047fdb0..009c8e0 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "strbuf.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -207,43 +208,49 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static char *user_path(char *buf, char *path, int sz)
+static inline struct passwd *getpw_str(const char *username, size_t len)
 {
+	if (len == 0)
+		return getpwuid(getuid());
+
 	struct passwd *pw;
-	char *slash;
-	int len, baselen;
+	char *username_z = xmalloc(len + 1);
+	memcpy(username_z, username, len);
+	username_z[len] = '\0';
+	pw = getpwnam(username_z);
+	free(username_z);
+	return pw;
+}
 
-	if (!path || path[0] != '~')
-		return NULL;
-	path++;
-	slash = strchr(path, '/');
-	if (path[0] == '/' || !path[0]) {
-		pw = getpwuid(getuid());
-	}
-	else {
-		if (slash) {
-			*slash = 0;
-			pw = getpwnam(path);
-			*slash = '/';
-		}
-		else
-			pw = getpwnam(path);
-	}
-	if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
-		return NULL;
-	baselen = strlen(pw->pw_dir);
-	memcpy(buf, pw->pw_dir, baselen);
-	while ((1 < baselen) && (buf[baselen-1] == '/')) {
-		buf[baselen-1] = 0;
-		baselen--;
-	}
-	if (slash && slash[1]) {
-		len = strlen(slash);
-		if (sz <= baselen + len)
-			return NULL;
-		memcpy(buf + baselen, slash, len + 1);
+/*
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL, then
+ * it is a newly allocated string. Returns NULL on getpw failure or if
+ * path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+	struct strbuf user_path = STRBUF_INIT;
+	char * first_slash = strchrnul(path, '/');
+	char * to_copy;
+	if (path == NULL)
+		goto return_null;
+
+	if (path[0] == '~') {
+		const char *username = path + 1;
+		size_t username_len = first_slash - username;
+		struct passwd *pw = getpw_str(username, username_len);
+		if (!pw)
+			goto return_null;
+		strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
+		to_copy = first_slash;
+	} else if (path[0] != '/') {
+		to_copy = path;
 	}
-	return buf;
+	strbuf_add(&user_path, to_copy, strlen(to_copy));
+	return strbuf_detach(&user_path, NULL);
+return_null:
+	strbuf_release(&user_path);
+	return NULL;
 }
 
 /*
@@ -291,8 +298,17 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				if (path != newpath)
+					free(newpath);
 				return NULL;
+			}
+			/* Copy back into the static buffer. A pity
+			   since newpath was not bounded, but other
+			   branches of the if are limited by PATH_MAX
+			   anyway. */
+			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-- 
1.6.5.2.152.gbbe9e

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> My version is made a bit simpler by using strbuf for string
> manipulation in expand_user_path.

Nice to see people keeping track of issues that we tried to address but
didn't complete.

> I'm not sure I fully adressed Junio's point here:

We'll see ;-)

> I'm just copying back into the static buffer to let enter_repo() do
> the same string manipulation as it used to do (concatenate with .git
> suffixes). I think the whole enter_repo could use strbuf instead of
> static buffers, but that's a different point (probably made easier by
> my patch).

> diff --git a/config.c b/config.c
> index c644061..0fcc4ce 100644
> --- a/config.c
> +++ b/config.c
> @@ -351,6 +351,15 @@ int git_config_string(const char **dest, const char *var, const char *value)
>  	return 0;
>  }
>  
> +int git_config_pathname(const char **dest, const char *var, const char *value) {
> +	if (!value)
> +		return config_error_nonbool(var);
> +	*dest = expand_user_path(value);
> +	if (!*dest)
> +		die("Failed to expand user dir in: '%s'", value);
> +	return 0;
> +}
> +

> @@ -207,43 +208,49 @@ int validate_headref(const char *path)
>  	return -1;
>  }
>  
> -static char *user_path(char *buf, char *path, int sz)
> +static inline struct passwd *getpw_str(const char *username, size_t len)
>  {
> +	if (len == 0)
> +		return getpwuid(getuid());
> +
>  	struct passwd *pw;

Decl-after-statement.

Do you need this special case of passing a zero-length string (not NULL
pointer as a string) to getpw_str() to grab the current user?  Which
codepath is this needed?

> +	char *username_z = xmalloc(len + 1);
> +	memcpy(username_z, username, len);
> +	username_z[len] = '\0';
> +	pw = getpwnam(username_z);
> +	free(username_z);
> +	return pw;
> +}

> +/*
> + * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL, then
> + * it is a newly allocated string. Returns NULL on getpw failure or if
> + * path is NULL.
> + */
> +char *expand_user_path(const char *path)
> +{
> +	struct strbuf user_path = STRBUF_INIT;
> +	char * first_slash = strchrnul(path, '/');
> +	char * to_copy;

Style: "char *first_slash"

Should't "to_copy" be initialized to "path" here?  What do you copy when
path[0] is '/'?

> +	if (path == NULL)
> +		goto return_null;
> +
> +	if (path[0] == '~') {
> +		const char *username = path + 1;
> +		size_t username_len = first_slash - username;
> +		struct passwd *pw = getpw_str(username, username_len);
> +		if (!pw)
> +			goto return_null;
> +		strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
> +		to_copy = first_slash;
> +	} else if (path[0] != '/') {
> +		to_copy = path;
>  	}
> -	return buf;
> +	strbuf_add(&user_path, to_copy, strlen(to_copy));
> +	return strbuf_detach(&user_path, NULL);
> +return_null:
> +	strbuf_release(&user_path);
> +	return NULL;
>  }



>  /*
> @@ -291,8 +298,17 @@ char *enter_repo(char *path, int strict)
>  		if (PATH_MAX <= len)
>  			return NULL;
>  		if (path[0] == '~') {
> -			if (!user_path(used_path, path, PATH_MAX))
> +			char *newpath = expand_user_path(path);
> +			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
> +				if (path != newpath)
> +					free(newpath);

Your expand_user_path() never returns the original, no?

>  				return NULL;
> +			}
> +			/* Copy back into the static buffer. A pity
> +			   since newpath was not bounded, but other
> +			   branches of the if are limited by PATH_MAX
> +			   anyway. */
> +			strcpy(used_path, newpath); free(newpath);
>  			strcpy(validated_path, path);
>  			path = used_path;
>  		}

Heh, in a sense you _did_ address my point by punting, but that is Ok.  As
you said earlier that would be a good topic of a separate patch.

	/*
         * By the way, we write our
         * multi-line comments like
         * this.
         */

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Jakub Narebski]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> These config variables are parsed to substitute ~ and ~user with getpw
> entries.
> 
> user_path() refactored into new function expand_user_path(), to allow
> dynamically allocating the return buffer.
> 
> Original patch by Karl Chen, modified by Matthieu Moy.
> 
> Signed-off-by: Karl Chen <quarl@quarl.org>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index d1e2120..c37b51d 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -380,7 +380,8 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
>  core.excludesfile::
>  	In addition to '.gitignore' (per-directory) and
>  	'.git/info/exclude', git looks into this file for patterns
> -	of files which are not meant to be tracked.  See
> +	of files which are not meant to be tracked.  "~" and "~user"
> +	are expanded to the user's home directory.  See
>  	linkgit:gitignore[5].
>  
>  core.editor::

It would be nice to have an option to git-config which would do such
expansion, as a separate type similar to --int and --bool, e.g.:

  git config --path section.key

so that not only core.excludesfile could use this new feature, but for
example also core.worktree, commit.template, gitcvs.logfile,
mailmap.file, and perhaps also *.receivepack and *.uploadpack

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index d1e2120..c37b51d 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -380,7 +380,8 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
>>  core.excludesfile::
>>  	In addition to '.gitignore' (per-directory) and
>>  	'.git/info/exclude', git looks into this file for patterns
>> -	of files which are not meant to be tracked.  See
>> +	of files which are not meant to be tracked.  "~" and "~user"
>> +	are expanded to the user's home directory.  See
>>  	linkgit:gitignore[5].
>>  
>>  core.editor::
>
> It would be nice to have an option to git-config which would do such
> expansion, as a separate type similar to --int and --bool, e.g.:
>
>   git config --path section.key
>
> so that not only core.excludesfile could use this new feature, but for
> example also core.worktree, commit.template, gitcvs.logfile,
> mailmap.file, and perhaps also *.receivepack and *.uploadpack

What should "git config -l" do for these (and core.excludesfile)?

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Junio C Hamano]

I'd like to squash this to your patch, based on the earlier review
comments.

I didn't mention about the first hunk; it is just a style.  An opening
brace that begins a function body comes at column 1.

Also first_slash and to_copy are made const pointers, as they do not have
to touch the region of memory they point to (otherwise you cannot assign
path to to_copy without getting warned).

 config.c |    3 ++-
 path.c   |   32 ++++++++++++++------------------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/config.c b/config.c
index 0fcc4ce..b3d1ff4 100644
--- a/config.c
+++ b/config.c
@@ -351,7 +351,8 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
-int git_config_pathname(const char **dest, const char *var, const char *value) {
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
 	if (!value)
 		return config_error_nonbool(var);
 	*dest = expand_user_path(value);
diff --git a/path.c b/path.c
index 009c8e0..2470f78 100644
--- a/path.c
+++ b/path.c
@@ -208,11 +208,8 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static inline struct passwd *getpw_str(const char *username, size_t len)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
-	if (len == 0)
-		return getpwuid(getuid());
-
 	struct passwd *pw;
 	char *username_z = xmalloc(len + 1);
 	memcpy(username_z, username, len);
@@ -223,18 +220,18 @@ static inline struct passwd *getpw_str(const char *username, size_t len)
 }
 
 /*
- * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL, then
- * it is a newly allocated string. Returns NULL on getpw failure or if
- * path is NULL.
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
  */
 char *expand_user_path(const char *path)
 {
 	struct strbuf user_path = STRBUF_INIT;
-	char * first_slash = strchrnul(path, '/');
-	char * to_copy;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
 	if (path == NULL)
 		goto return_null;
-
 	if (path[0] == '~') {
 		const char *username = path + 1;
 		size_t username_len = first_slash - username;
@@ -243,8 +240,6 @@ char *expand_user_path(const char *path)
 			goto return_null;
 		strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
 		to_copy = first_slash;
-	} else if (path[0] != '/') {
-		to_copy = path;
 	}
 	strbuf_add(&user_path, to_copy, strlen(to_copy));
 	return strbuf_detach(&user_path, NULL);
@@ -300,14 +295,15 @@ char *enter_repo(char *path, int strict)
 		if (path[0] == '~') {
 			char *newpath = expand_user_path(path);
 			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
-				if (path != newpath)
-					free(newpath);
+				free(newpath);
 				return NULL;
 			}
-			/* Copy back into the static buffer. A pity
-			   since newpath was not bounded, but other
-			   branches of the if are limited by PATH_MAX
-			   anyway. */
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
 			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
-- 
1.6.5.3.283.g4b054

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Jeff King]

On Mon, Nov 16, 2009 at 11:07:40AM +0100, Matthieu Moy wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index d1e2120..c37b51d 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -380,7 +380,8 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
>  core.excludesfile::
>  	In addition to '.gitignore' (per-directory) and
>  	'.git/info/exclude', git looks into this file for patterns
> -	of files which are not meant to be tracked.  See
> +	of files which are not meant to be tracked.  "~" and "~user"
> +	are expanded to the user's home directory.  See
>  	linkgit:gitignore[5].

Reading this, it is not clear to me if:

  1. "~" and "~user" are expanded to the home directory of "user", where
     "user" is the user running git

or

  2. "~" is expanded to the home directory of the user running git, and
     an arbitrary "~user" is expanded to that user's home directory.

I would expect (2), since that is how everything else works. And it
seems from the code that is what you do. But something about the way it
is written makes me think of (1). I also recall having this same
question during the last round, so at the very least it is not me just
mis-reading it once. ;)

Maybe:

  A leading path component of "~user" is expanded to the home directory
  of "user"; "~" is expanded to the home directory of the user running
  git.

would be more clear?

-Peff

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Mike Hommey]

On Tue, Nov 17, 2009 at 02:34:26AM -0500, Jeff King wrote:
> Maybe:
> 
>   A leading path component of "~user" is expanded to the home directory
>   of "user"; "~" is expanded to the home directory of the user running
>   git.
> 
> would be more clear?

Add "real" before "user running git" and you have my vote. Or maybe
using the effective user would be better, and the patch should use
geteuid() instead of getuid(), I don't know. ident.c uses getuid(), but
I'm wondering if that's what it should use (although it doesn't seem to
have been a problem to anyone)

Mike

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Mon, Nov 16, 2009 at 11:07:40AM +0100, Matthieu Moy wrote:
>
>> +	of files which are not meant to be tracked.  "~" and "~user"
>> +	are expanded to the user's home directory.  See
>>  	linkgit:gitignore[5].
>
> Reading this, it is not clear to me if:
>
>   1. "~" and "~user" are expanded to the home directory of "user", where
>      "user" is the user running git
>
> or
>
>   2. "~" is expanded to the home directory of the user running git, and
>      an arbitrary "~user" is expanded to that user's home directory.
>
> I would expect (2), since that is how everything else works.

Yes. "user" in the sentence is either the user running Git or the same
string as "user" in "~user". I'm not against your proposal, but I'm
afraid we're making the sentence uselessly heavy, since, as you say,
this ~ and ~user convention is widely spread, and I hardly imagine
someone interpreting the sentence as "if you say ~foo, it will expand
to the home directory of bar".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Jeff King]

On Tue, Nov 17, 2009 at 09:53:52AM +0100, Matthieu Moy wrote:

> Yes. "user" in the sentence is either the user running Git or the same
> string as "user" in "~user". I'm not against your proposal, but I'm
> afraid we're making the sentence uselessly heavy, since, as you say,
> this ~ and ~user convention is widely spread, and I hardly imagine
> someone interpreting the sentence as "if you say ~foo, it will expand
> to the home directory of bar".

I didn't think it would expand ~foo to the home directory of bar. I
thought that it might _only_ accept ~bar, and not ~foo.

If I'm the only one confused, then we can drop it. But if not, I don't
think it is much more work to be precise.

-Peff

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Jakub Narebski <jnareb@gmail.com> writes:
>
>> It would be nice to have an option to git-config which would do such
>> expansion, as a separate type similar to --int and --bool, e.g.:
>>
>>   git config --path section.key
>>
>> so that not only core.excludesfile could use this new feature, but for
>> example also core.worktree, commit.template, gitcvs.logfile,
>> mailmap.file, and perhaps also *.receivepack and *.uploadpack
>
> What should "git config -l" do for these (and core.excludesfile)?

I don't know what it "should", but it "does" not do the expansion. I
had the same questionning when testing the patch, I'd have liked to be
able to write a simple test-case like

$ git config core.excludesfile '~/foo'
$ git config --i-dont-know-what core.excludesfile

to go through this codepath. Maybe we can just say

$ git config --default core.excludesfile

to say "call git_default_config(...) on this before printing it". My
understanding is that this is what the C code is doing, we should
allow the shell scripts to do the same.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> I'd like to squash this to your patch, based on the earlier review
> comments.

Sure. And apologize for the style issues, I knew it, but one hardly
changes his (bad) habits ;-).

I'll resend soon.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Jakub Narebski]

Dnia wtorek 17. listopada 2009 07:22, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:

> >
> > It would be nice to have an option to git-config which would do such
> > expansion, as a separate type similar to --int and --bool, e.g.:
> >
> >   git config --path section.key
> >
> > so that not only core.excludesfile could use this new feature, but for
> > example also core.worktree, commit.template, gitcvs.logfile,
> > mailmap.file, and perhaps also *.receivepack and *.uploadpack
> 
> What should "git config -l" do for these (and core.excludesfile)?

Nothing (i.e. show unexpanded / not converted), just like for boolean
variables "git config -l" doesn't convert 1/on/yes to true, and 0/off/no
to false, just like for integer variables "git config -l" doesn't convert
to simple decimal number taking into account optional 'k', 'm' or 'g'
suffix.


BTW. the suffix part of integer conversion should really be described
in the paragraph about --int and --bool options (which should be made
into proper description list, and not a prose paragraph).

P.S. I am a bit missing --local / --repository option to git-config to
complement --global (which should probably be named --user) and --system
options.

-- 
Jakub Narebski
Poland

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Jakub Narebski]

Dnia wtorek 17. listopada 2009 09:57, Matthieu Moy napisał:
> Junio C Hamano <gitster@pobox.com> writes:
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> It would be nice to have an option to git-config which would do such
>>> expansion, as a separate type similar to --int and --bool, e.g.:
>>>
>>>   git config --path section.key
>>>
>>> so that not only core.excludesfile could use this new feature, but for
>>> example also core.worktree, commit.template, gitcvs.logfile,
>>> mailmap.file, and perhaps also *.receivepack and *.uploadpack
>>
>> What should "git config -l" do for these (and core.excludesfile)?
> 
> I don't know what it "should", but it "does" not do the expansion. I
> had the same questionning when testing the patch, I'd have liked to be
> able to write a simple test-case like
> 
> $ git config core.excludesfile '~/foo'
> $ git config --i-dont-know-what core.excludesfile
> 
> to go through this codepath. Maybe we can just say
> 
> $ git config --default core.excludesfile
> 
> to say "call git_default_config(...) on this before printing it". My
> understanding is that this is what the C code is doing, we should
> allow the shell scripts to do the same.

I think it is a very good idea.  Nevertheless it can apply only to
config variables git core knows about, and not for example for git-gui,
or gitk, or qgit, or tig, or StGIT, etc. configuration.  Therefore
"git config --path" would be still needed.

-- 
Jakub Narębski
Poland

[PATCH v2] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

These config variables are parsed to substitute ~ and ~user with getpw
entries.

user_path() refactored into new function expand_user_path(), to allow
dynamically allocating the return buffer.

Original patch by Karl Chen, modified by Matthieu Moy, and further
amended by Junio C Hamano.

Signed-off-by: Karl Chen <quarl@quarl.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Here's the new version, with Junio's modifications squashed in.

I just replaced "user" with "specified user" to make it clear one can
specify a user with ~foo/, but since the documentation appears in two
places (possibly more in the future), I prefered to Keep It Simple.

 Documentation/config.txt |    4 ++-
 builtin-commit.c         |    2 +-
 cache.h                  |    2 +
 config.c                 |   12 ++++++-
 path.c                   |   80 ++++++++++++++++++++++++++-------------------
 5 files changed, 63 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index d1e2120..475d544 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,7 +380,8 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  See
+	of files which are not meant to be tracked.  "~/" and "~user/"
+	are expanded to the specified user's home directory.  See
 	linkgit:gitignore[5].
 
 core.editor::
@@ -670,6 +671,7 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
+	"~/" and "~user/" are expanded to the specified user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/builtin-commit.c b/builtin-commit.c
index d525b89..09d2840 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -999,7 +999,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!strcmp(k, "commit.template"))
-		return git_config_string(&template_file, k, v);
+		return git_config_pathname(&template_file, k, v);
 
 	return git_status_config(k, v, s);
 }
diff --git a/cache.h b/cache.h
index 71a731d..42f7cd8 100644
--- a/cache.h
+++ b/cache.h
@@ -645,6 +645,7 @@ int set_shared_perm(const char *path, int mode);
 #define adjust_shared_perm(path) set_shared_perm((path), 0)
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
+extern char *expand_user_path(const char *path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
@@ -903,6 +904,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index c644061..b3d1ff4 100644
--- a/config.c
+++ b/config.c
@@ -351,6 +351,16 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	*dest = expand_user_path(value);
+	if (!*dest)
+		die("Failed to expand user dir in: '%s'", value);
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -474,7 +484,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return git_config_string(&editor_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile"))
-		return git_config_string(&excludes_file, var, value);
+		return git_config_pathname(&excludes_file, var, value);
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git a/path.c b/path.c
index 047fdb0..2470f78 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "strbuf.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -207,43 +208,44 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static char *user_path(char *buf, char *path, int sz)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
 	struct passwd *pw;
-	char *slash;
-	int len, baselen;
+	char *username_z = xmalloc(len + 1);
+	memcpy(username_z, username, len);
+	username_z[len] = '\0';
+	pw = getpwnam(username_z);
+	free(username_z);
+	return pw;
+}
 
-	if (!path || path[0] != '~')
-		return NULL;
-	path++;
-	slash = strchr(path, '/');
-	if (path[0] == '/' || !path[0]) {
-		pw = getpwuid(getuid());
-	}
-	else {
-		if (slash) {
-			*slash = 0;
-			pw = getpwnam(path);
-			*slash = '/';
-		}
-		else
-			pw = getpwnam(path);
-	}
-	if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
-		return NULL;
-	baselen = strlen(pw->pw_dir);
-	memcpy(buf, pw->pw_dir, baselen);
-	while ((1 < baselen) && (buf[baselen-1] == '/')) {
-		buf[baselen-1] = 0;
-		baselen--;
-	}
-	if (slash && slash[1]) {
-		len = strlen(slash);
-		if (sz <= baselen + len)
-			return NULL;
-		memcpy(buf + baselen, slash, len + 1);
+/*
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+	struct strbuf user_path = STRBUF_INIT;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
+	if (path == NULL)
+		goto return_null;
+	if (path[0] == '~') {
+		const char *username = path + 1;
+		size_t username_len = first_slash - username;
+		struct passwd *pw = getpw_str(username, username_len);
+		if (!pw)
+			goto return_null;
+		strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
+		to_copy = first_slash;
 	}
-	return buf;
+	strbuf_add(&user_path, to_copy, strlen(to_copy));
+	return strbuf_detach(&user_path, NULL);
+return_null:
+	strbuf_release(&user_path);
+	return NULL;
 }
 
 /*
@@ -291,8 +293,18 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				free(newpath);
 				return NULL;
+			}
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
+			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-- 
1.6.5.2.152.gbbe9e

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Andreas Schwab]

Mike Hommey <mh@glandium.org> writes:

> On Tue, Nov 17, 2009 at 02:34:26AM -0500, Jeff King wrote:
>> Maybe:
>> 
>>   A leading path component of "~user" is expanded to the home directory
>>   of "user"; "~" is expanded to the home directory of the user running
>>   git.
>> 
>> would be more clear?
>
> Add "real" before "user running git" and you have my vote. Or maybe
> using the effective user would be better, and the patch should use
> geteuid() instead of getuid(), I don't know. ident.c uses getuid(), but
> I'm wondering if that's what it should use (although it doesn't seem to
> have been a problem to anyone)

"~" should just expand to the value of $HOME, like in the shell,
independent of the real home directory of the real or effective user.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Junio C Hamano]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Mike Hommey <mh@glandium.org> writes:
>
>> On Tue, Nov 17, 2009 at 02:34:26AM -0500, Jeff King wrote:
>>> Maybe:
>>> 
>>>   A leading path component of "~user" is expanded to the home directory
>>>   of "user"; "~" is expanded to the home directory of the user running
>>>   git.
>>> 
>>> would be more clear?
>>
>> Add "real" before "user running git" and you have my vote. Or maybe
>> using the effective user would be better, and the patch should use
>> geteuid() instead of getuid(), I don't know. ident.c uses getuid(), but
>> I'm wondering if that's what it should use (although it doesn't seem to
>> have been a problem to anyone)
>
> "~" should just expand to the value of $HOME, like in the shell,
> independent of the real home directory of the real or effective user.

How should this interact with installations that run gitosis/gitlite that
use shared account, giving user identity via the ssh key?

Note that the question is not "how would this...", but "how _should_
this...".

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Andreas Schwab]

Junio C Hamano <gitster@pobox.com> writes:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Mike Hommey <mh@glandium.org> writes:
>>
>>> On Tue, Nov 17, 2009 at 02:34:26AM -0500, Jeff King wrote:
>>>> Maybe:
>>>> 
>>>>   A leading path component of "~user" is expanded to the home directory
>>>>   of "user"; "~" is expanded to the home directory of the user running
>>>>   git.
>>>> 
>>>> would be more clear?
>>>
>>> Add "real" before "user running git" and you have my vote. Or maybe
>>> using the effective user would be better, and the patch should use
>>> geteuid() instead of getuid(), I don't know. ident.c uses getuid(), but
>>> I'm wondering if that's what it should use (although it doesn't seem to
>>> have been a problem to anyone)
>>
>> "~" should just expand to the value of $HOME, like in the shell,
>> independent of the real home directory of the real or effective user.
>
> How should this interact with installations that run gitosis/gitlite that
> use shared account, giving user identity via the ssh key?

Sorry, I don't know enough about such an installation to be able to
answer that.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Mike Hommey <mh@glandium.org> writes:
>>
>> "~" should just expand to the value of $HOME, like in the shell,
>> independent of the real home directory of the real or effective user.

Right, I'll resend in a minute.

Junio C Hamano <gitster@pobox.com> writes:

> How should this interact with installations that run gitosis/gitlite that
> use shared account, giving user identity via the ssh key?

I guess there's some kind of rhetorical question behind that I missed,
but if a gitosis-like installation uses the user git, and the user foo
connects to it identifier by its SSH key, I don't see what you could
do other than expanding ~ to the homedir of git, which is $HOME at the
time your run git on the server.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

[PATCH v3] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

These config variables are parsed to substitute ~ and ~user with getpw
entries.

user_path() refactored into new function expand_user_path(), to allow
dynamically allocating the return buffer.

Original patch by Karl Chen, modified by Matthieu Moy, and further
amended by Junio C Hamano.

Signed-off-by: Karl Chen <quarl@quarl.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

Since v2, I changed the code to use $HOME instead of the actual home
to expand ~.

This way, if you run "HOME=/else/where/ git status", then both
.gitconfig and ~/.gitignore will be taken from /else/where.

 Documentation/config.txt |    7 +++-
 builtin-commit.c         |    2 +-
 cache.h                  |    2 +
 config.c                 |   12 ++++++-
 path.c                   |   83 +++++++++++++++++++++++++++------------------
 5 files changed, 69 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cb73d75..fb1740c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,8 +380,9 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  See
-	linkgit:gitignore[5].
+	of files which are not meant to be tracked.  "~/" is expanded
+	to the value of `$HOME` and "~user/" to the specified user's
+	home directory.  See linkgit:gitignore[5].
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
@@ -670,6 +671,8 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
+	"~/" is expanded to the value of `$HOME` and "~user/" to the
+	specified user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/builtin-commit.c b/builtin-commit.c
index d525b89..09d2840 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -999,7 +999,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!strcmp(k, "commit.template"))
-		return git_config_string(&template_file, k, v);
+		return git_config_pathname(&template_file, k, v);
 
 	return git_status_config(k, v, s);
 }
diff --git a/cache.h b/cache.h
index 71a731d..42f7cd8 100644
--- a/cache.h
+++ b/cache.h
@@ -645,6 +645,7 @@ int set_shared_perm(const char *path, int mode);
 #define adjust_shared_perm(path) set_shared_perm((path), 0)
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
+extern char *expand_user_path(const char *path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
@@ -903,6 +904,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index c644061..b3d1ff4 100644
--- a/config.c
+++ b/config.c
@@ -351,6 +351,16 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	*dest = expand_user_path(value);
+	if (!*dest)
+		die("Failed to expand user dir in: '%s'", value);
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -474,7 +484,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return git_config_string(&editor_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile"))
-		return git_config_string(&excludes_file, var, value);
+		return git_config_pathname(&excludes_file, var, value);
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git a/path.c b/path.c
index 047fdb0..6351b57 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "strbuf.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -207,43 +208,49 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static char *user_path(char *buf, char *path, int sz)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
 	struct passwd *pw;
-	char *slash;
-	int len, baselen;
+	char *username_z = xmalloc(len + 1);
+	memcpy(username_z, username, len);
+	username_z[len] = '\0';
+	pw = getpwnam(username_z);
+	free(username_z);
+	return pw;
+}
 
-	if (!path || path[0] != '~')
-		return NULL;
-	path++;
-	slash = strchr(path, '/');
-	if (path[0] == '/' || !path[0]) {
-		pw = getpwuid(getuid());
-	}
-	else {
-		if (slash) {
-			*slash = 0;
-			pw = getpwnam(path);
-			*slash = '/';
+/*
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+	struct strbuf user_path = STRBUF_INIT;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
+	if (path == NULL)
+		goto return_null;
+	if (path[0] == '~') {
+		const char *username = path + 1;
+		size_t username_len = first_slash - username;
+		if (username_len == 0) {
+			const char * home = getenv("HOME");
+			strbuf_add(&user_path, home, strlen(home));
+		} else {
+			struct passwd *pw = getpw_str(username, username_len);
+			if (!pw)
+				goto return_null;
+			strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
 		}
-		else
-			pw = getpwnam(path);
+		to_copy = first_slash;
 	}
-	if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
-		return NULL;
-	baselen = strlen(pw->pw_dir);
-	memcpy(buf, pw->pw_dir, baselen);
-	while ((1 < baselen) && (buf[baselen-1] == '/')) {
-		buf[baselen-1] = 0;
-		baselen--;
-	}
-	if (slash && slash[1]) {
-		len = strlen(slash);
-		if (sz <= baselen + len)
-			return NULL;
-		memcpy(buf + baselen, slash, len + 1);
-	}
-	return buf;
+	strbuf_add(&user_path, to_copy, strlen(to_copy));
+	return strbuf_detach(&user_path, NULL);
+return_null:
+	strbuf_release(&user_path);
+	return NULL;
 }
 
 /*
@@ -291,8 +298,18 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				free(newpath);
 				return NULL;
+			}
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
+			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-- 
1.6.5.2.152.gbbe9e

[PATCH v4] Expand ~ and ~user in core.excludesfile, commit.template

[Matthieu Moy]

These config variables are parsed to substitute ~ and ~user with getpw
entries.

user_path() refactored into new function expand_user_path(), to allow
dynamically allocating the return buffer.

Original patch by Karl Chen, modified by Matthieu Moy, and further
amended by Junio C Hamano.

Signed-off-by: Karl Chen <quarl@quarl.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
"Oops, I did it again :-("

Just a style issue since v3 (char * home -> char *home), sorry for the noise.

 Documentation/config.txt |    7 +++-
 builtin-commit.c         |    2 +-
 cache.h                  |    2 +
 config.c                 |   12 ++++++-
 path.c                   |   83 +++++++++++++++++++++++++++------------------
 5 files changed, 69 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 78ee906..b4fe843 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,8 +380,9 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  See
-	linkgit:gitignore[5].
+	of files which are not meant to be tracked.  "~/" is expanded
+	to the value of `$HOME` and "~user/" to the specified user's
+	home directory.  See linkgit:gitignore[5].
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
@@ -681,6 +682,8 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
+	"~/" is expanded to the value of `$HOME` and "~user/" to the
+	specified user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/builtin-commit.c b/builtin-commit.c
index 0fd7af6..87d1b90 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1175,7 +1175,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!strcmp(k, "commit.template"))
-		return git_config_string(&template_file, k, v);
+		return git_config_pathname(&template_file, k, v);
 
 	return git_status_config(k, v, s);
 }
diff --git a/cache.h b/cache.h
index f7533ff..2ba9690 100644
--- a/cache.h
+++ b/cache.h
@@ -649,6 +649,7 @@ int set_shared_perm(const char *path, int mode);
 #define adjust_shared_perm(path) set_shared_perm((path), 0)
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
+extern char *expand_user_path(const char *path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
@@ -909,6 +910,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 51f2208..37385ce 100644
--- a/config.c
+++ b/config.c
@@ -351,6 +351,16 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	*dest = expand_user_path(value);
+	if (!*dest)
+		die("Failed to expand user dir in: '%s'", value);
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -479,7 +489,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return git_config_string(&editor_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile"))
-		return git_config_string(&excludes_file, var, value);
+		return git_config_pathname(&excludes_file, var, value);
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git a/path.c b/path.c
index c7679be..2ec950b 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "strbuf.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -207,43 +208,49 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static char *user_path(char *buf, char *path, int sz)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
 	struct passwd *pw;
-	char *slash;
-	int len, baselen;
+	char *username_z = xmalloc(len + 1);
+	memcpy(username_z, username, len);
+	username_z[len] = '\0';
+	pw = getpwnam(username_z);
+	free(username_z);
+	return pw;
+}
 
-	if (!path || path[0] != '~')
-		return NULL;
-	path++;
-	slash = strchr(path, '/');
-	if (path[0] == '/' || !path[0]) {
-		pw = getpwuid(getuid());
-	}
-	else {
-		if (slash) {
-			*slash = 0;
-			pw = getpwnam(path);
-			*slash = '/';
+/*
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+	struct strbuf user_path = STRBUF_INIT;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
+	if (path == NULL)
+		goto return_null;
+	if (path[0] == '~') {
+		const char *username = path + 1;
+		size_t username_len = first_slash - username;
+		if (username_len == 0) {
+			const char *home = getenv("HOME");
+			strbuf_add(&user_path, home, strlen(home));
+		} else {
+			struct passwd *pw = getpw_str(username, username_len);
+			if (!pw)
+				goto return_null;
+			strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
 		}
-		else
-			pw = getpwnam(path);
+		to_copy = first_slash;
 	}
-	if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
-		return NULL;
-	baselen = strlen(pw->pw_dir);
-	memcpy(buf, pw->pw_dir, baselen);
-	while ((1 < baselen) && (buf[baselen-1] == '/')) {
-		buf[baselen-1] = 0;
-		baselen--;
-	}
-	if (slash && slash[1]) {
-		len = strlen(slash);
-		if (sz <= baselen + len)
-			return NULL;
-		memcpy(buf + baselen, slash, len + 1);
-	}
-	return buf;
+	strbuf_add(&user_path, to_copy, strlen(to_copy));
+	return strbuf_detach(&user_path, NULL);
+return_null:
+	strbuf_release(&user_path);
+	return NULL;
 }
 
 /*
@@ -291,8 +298,18 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				free(newpath);
 				return NULL;
+			}
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
+			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-- 
1.6.5.2.152.gbbe9e

[PATCH] expand_user_path: expand ~ to $HOME, not to the actual homedir.

[Matthieu Moy]

In 395de250d (Expand ~ and ~user in core.excludesfile, commit.template),
we introduced the mechanism. But expanding ~ using getpw is not what
people overriding $HOME would usually expect. In particular, git looks
for the user's .gitconfig using $HOME, so it's better to be consistent.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
This is basically my patch v4, but since an earlier version is already
in next, I resend the last bits of it, based on next.

 Documentation/config.txt |    9 +++++----
 path.c                   |   13 +++++++++----
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 468e285..39d1226 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -381,9 +381,9 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  "~/" and "~user/"
-	are expanded to the specified user's home directory.  See
-	linkgit:gitignore[5].
+	of files which are not meant to be tracked.  "~/" is expanded
+	to the value of `$HOME` and "~user/" to the specified user's
+	home directory.  See linkgit:gitignore[5].
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
@@ -683,7 +683,8 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
-	"~/" and "~user/" are expanded to the specified user's home directory.
+	"~/" is expanded to the value of `$HOME` and "~user/" to the
+	specified user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/path.c b/path.c
index b4a8075..2ec950b 100644
--- a/path.c
+++ b/path.c
@@ -235,10 +235,15 @@ char *expand_user_path(const char *path)
 	if (path[0] == '~') {
 		const char *username = path + 1;
 		size_t username_len = first_slash - username;
-		struct passwd *pw = getpw_str(username, username_len);
-		if (!pw)
-			goto return_null;
-		strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
+		if (username_len == 0) {
+			const char *home = getenv("HOME");
+			strbuf_add(&user_path, home, strlen(home));
+		} else {
+			struct passwd *pw = getpw_str(username, username_len);
+			if (!pw)
+				goto return_null;
+			strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
+		}
 		to_copy = first_slash;
 	}
 	strbuf_add(&user_path, to_copy, strlen(to_copy));
-- 
1.6.5.2.152.gbbe9e

Re: [PATCH] expand_user_path: expand ~ to $HOME, not to the actual homedir.

[Jeff King]

On Thu, Nov 19, 2009 at 04:21:15PM +0100, Matthieu Moy wrote:

> -	of files which are not meant to be tracked.  "~/" and "~user/"
> -	are expanded to the specified user's home directory.  See
> -	linkgit:gitignore[5].
> +	of files which are not meant to be tracked.  "~/" is expanded
> +	to the value of `$HOME` and "~user/" to the specified user's
> +	home directory.  See linkgit:gitignore[5].

Thanks. As a side effect, this wording change addresses my original
ambiguity concern (and I also think using $HOME is the right thing to
do).

-Peff

Re: [PATCH] expand_user_path: expand ~ to $HOME, not to the actual homedir.

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> On Thu, Nov 19, 2009 at 04:21:15PM +0100, Matthieu Moy wrote:
>
>> -	of files which are not meant to be tracked.  "~/" and "~user/"
>> -	are expanded to the specified user's home directory.  See
>> -	linkgit:gitignore[5].
>> +	of files which are not meant to be tracked.  "~/" is expanded
>> +	to the value of `$HOME` and "~user/" to the specified user's
>> +	home directory.  See linkgit:gitignore[5].
>
> Thanks. As a side effect, this wording change addresses my original
> ambiguity concern (and I also think using $HOME is the right thing to
> do).

Not a pure coincidence indeed ;-). In this new version, ~ and ~foo do
actually different things so they definitely deserve the heaviness of
a few extra words.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v4] Expand ~ and ~user in core.excludesfile, commit.template

[Junio C Hamano]

Oops; last night I wrote

    Thanks; v3 was already in next so I'll make an interdiff into a separate
    "fix-up" commit.

but didn't send it out.  Thanks for rerolling the patch.