Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Torsten Bögershausen



There seems to be some other trouble under Mac OS, not yet fully tracked down,
(may be related to the diff -r)

Torsten sees failures of this kind under Mac OS:

diff -r .git/modules/sub1/config sub1/.git/config
6d5
 worktree = ../../../sub1
8a8

 worktree = ../../../sub1

So the config contains the same content, but the worktree setting moved
to a different line. This seems to be the result of setting core.worktree
in the test_git_directory_is_unchanged function just before the diff -r,
but only under Mac OS.

So I was suspecting diff -r beinng non-portable, but that doesn't seem 
to be the problem here.
(But I wouldn't be surprised if there where problems with diff -r on 
some Unix systems)
Anyway, checking all the files in the working tree seems to be a good 
thing to do,

but that does not necessarily work for .git/config.
A brute force approach could be to simply run the config file(s) 
through sort and compare them:


sort .git/modules/sub1/config expect 
sort sub1/.git/config actual 
test_cmp expect actual 
rm expect actual 
cp git/modules/sub1/config sub1/.git/config


[end of scriptlet]
And here the dumps of the 2 config files:
...
Branch remove_sub1 set up to track remote branch remove_sub1 from origin.
warning: unable to rmdir sub1: Directory not empty
Updating 68c8810..81b9f6a
Fast-forward
 .gitmodules | 4 
 1 file changed, 4 deletions(-)
 delete mode 100644 .gitmodules
-
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
worktree = ../../../sub1
ignorecase = true
precomposeunicode = true
[remote origin]
url = /Users/tb/projects/git/tb.140704_JensLehman/t/trash 
directory.t7613-merge-submodule/submodule_update_repo/.

fetch = +refs/heads/*:refs/remotes/origin/*
[branch master]
remote = origin
merge = refs/heads/master

[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
ignorecase = true
precomposeunicode = true
worktree = ../../../sub1
[remote origin]
url = /Users/tb/projects/git/tb.140704_JensLehman/t/trash 
directory.t7613-merge-submodule/submodule_update_repo/.

fetch = +refs/heads/*:refs/remotes/origin/*
[branch master]
remote = origin
merge = refs/heads/master
=
diff -r .git/modules/sub1/config sub1/.git/config
6d5
   worktree = ../../../sub1
8a8
   worktree = ../../../sub1
not ok 7 - git merge: removed submodule leaves submodule containing a 
.git directory alone


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] http: Add Accept-Language header if possible

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 02:46:35PM +0900, Yi, EungJun wrote:

 I agree with you. In fact, I tried to get user's preferred language in
 the same way as gettext. It has guess_category_value() to do that and
 the function is good enough because it considers $LANGUAGE, $LC_ALL,
 $LANG, and also system-dependent preferences. But the function does
 not seem a public API and I don't know how can I use the function in
 Git. So I chose to use $LANGUAGE only.

I did some digging, and I think the public API is setlocale with a NULL
parameter, like:

  printf(%s\n, setlocale(LC_MESSAGES, NULL));

That still will end up like en_US.UTF-8, though; I couldn't find any
standard functions for parsing that. It seems like it would be pretty
straightforward to do so, though.

From my brief reading of rfc2616, that should probably become en-us,
and any time we add x-y, we may want to add x as a fallback (that is
certainly true for English; I don't know about other languages with
dialects).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


No fchmod() under msygit - Was: Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Torsten Bögershausen

On 07/08/2014 10:25 PM, Ramsay Jones wrote:

On 08/07/14 20:34, Jens Lehmann wrote:

Am 07.07.2014 21:40, schrieb Torsten Bögershausen:

On 2014-07-07 19.05, Junio C Hamano wrote:

Jens Lehmann jens.lehm...@web.de writes:


Junio, do you want me to resend 02/14 without the non-portable echo -n
or could you just squash the following diff in?

Amended locally here already; thanks, both.

There seems to be some other trouble under Mac OS, not yet fully tracked down,
(may be related to the diff -r)

Torsten sees failures of this kind under Mac OS:

diff -r .git/modules/sub1/config sub1/.git/config
6d5
 worktree = ../../../sub1
8a8

 worktree = ../../../sub1

So the config contains the same content, but the worktree setting moved
to a different line. This seems to be the result of setting core.worktree
in the test_git_directory_is_unchanged function just before the diff -r,
but only under Mac OS.


And Msysgit complains
error: fchmod on c:/xxxt/trash 
directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
 failed: Function not implemented

I'm not sure what this is about, seems to happen during the cp -R of
the repo under .git/modules into the submodule.

I haven't looked into this at all, but from the above message, and
noting that fchmod() is not implemented in mingw (see compat/mingw.h
line 91), and the following:

 $ git grep -n fchmodcommit daa22c6f8da466bd7a438f1bc27375fd737ffcf3
Author: Eric Wong normalper...@yhbt.net
Date:   Tue May 6 00:17:14 2014 +

 config: preserve config file permissions on edits
 


 compat/mingw.h:91:static inline int fchmod(int fildes, mode_t mode)
 config.c:1639:  if (fchmod(fd, st.st_mode  0)  0) {
 config.c:1640:  error(fchmod on %s failed: %s,
 config.c:1818:  if (fchmod(out_fd, st.st_mode  0)  0) {
 config.c:1819:  ret = error(fchmod on %s failed: %s,
 $

[I happen to be on the pu branch at the moment, so YMMV!]

Both calls to fchmod() above are on config lock files, one
in git_config_set_multivar_in_file() and the other in
git_config_rename_section_in_file().




commit daa22c6f8da466bd7a438f1bc27375fd737ffcf3
Author: Eric Wong normalper...@yhbt.net
Date:   Tue May 6 00:17:14 2014 +

config: preserve config file permissions on edits

(And why is it   0 and not   0777)
Can we avoid the fchmod()  all together ?


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: move detection doesnt take filename into account

2014-07-09 Thread Jeff King
On Tue, Jul 01, 2014 at 10:08:15AM -0700, Junio C Hamano wrote:

 I didn't think it through but my gut feeling is that we could change
 the name similarity score to be the length of the tail part that
 matches (e.g. 1.a to a/2.a that has the same two bytes at the tail
 is a better match than to a/2.b that does not share any tail, and to
 a/1.a that shares the three bytes at the tail is an even better
 match).

The delta heuristics in pack-objects use pack_name_hash, which claims:

/*
 * This effectively just creates a sortable number from the
 * last sixteen non-whitespace characters. Last characters
 * count most, so things that end in .c sort together.
 */

which might be another option (and seems like a superset of the basename
check, short of basenames that are longer than 16 characters).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: `git log --graph` with multiple roots is confusing

2014-07-09 Thread Jeff King
On Mon, Jun 30, 2014 at 03:04:19AM -0700, Gary Fixler wrote:

 I just made a new test repo, added and fetched two unrelated repos,
 and then did the log view (all/graph/decorate/oneline), and tacked on
 --boundary, but saw no change. The root commits looked the same.

There was some discussion a while back on making root commits more
apparent in the graph view:

  http://article.gmane.org/gmane.comp.version-control.git/239580

That topic has stalled, but perhaps you can help push it forward.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Error running 'git status' with Git version of current 'next' branch

2014-07-09 Thread Ralf Thielow
Hi,

I'm getting the following error when calling 'git status' on one of
my projects.

git: dir.c:739: last_exclude_matching_from_list: Assertion `x-baselen
== 0 || x-base[x-baselen - 1] == '/'' failed.
Aborted (core dumped)

I'm using the current 'next', git version 2.0.1.664.g35b839c
I have bisected it to commit d3ccb7d (dir: remove PATH_MAX limitation).

Here's a simple script to reproduce it:

mkdir tmp
cd tmp
git init
mkdir -p 
foo/foo-bar/fbar/foo/bar/foo/foobar/f/baar/fo
echo /foo foo/.gitignore
git status

Ralf
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: `git log --graph` with multiple roots is confusing

2014-07-09 Thread Duy Nguyen
On Wed, Jul 9, 2014 at 1:51 PM, Jeff King p...@peff.net wrote:
 On Mon, Jun 30, 2014 at 03:04:19AM -0700, Gary Fixler wrote:

 I just made a new test repo, added and fetched two unrelated repos,
 and then did the log view (all/graph/decorate/oneline), and tacked on
 --boundary, but saw no change. The root commits looked the same.

 There was some discussion a while back on making root commits more
 apparent in the graph view:

   http://article.gmane.org/gmane.comp.version-control.git/239580

 That topic has stalled, but perhaps you can help push it forward.

Grafted commits are marked --decorate, so perhaps another (simpler)
route to make root commits stand out is decorate them.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 02/32] path.c: make get_pathname() call sites return const char *

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Before the previous commit, get_pathname returns an array of PATH_MAX
length. Even if git_path() and similar functions does not use the
whole array, git_path() caller can, in theory.

After the commit, get_pathname() may return a buffer that has just
enough room for the returned string and git_path() caller should never
write beyond that.

Make git_path(), mkpath() and git_path_submodule() return a const
buffer to make sure callers do not write in it at all.

This could have been part of the previous commit, but the const
conversion is too much distraction from the core changes in path.c.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 2 +-
 builtin/clone.c| 9 +
 builtin/fetch.c| 5 +++--
 builtin/fsck.c | 4 ++--
 builtin/receive-pack.c | 2 +-
 builtin/remote.c   | 2 +-
 builtin/repack.c   | 8 +---
 cache.h| 6 +++---
 fast-import.c  | 2 +-
 notes-merge.c  | 6 +++---
 path.c | 6 +++---
 refs.c | 8 
 run-command.c  | 4 ++--
 run-command.h  | 2 +-
 sha1_file.c| 2 +-
 15 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f1dc56e..6f74cfb 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -585,7 +585,7 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (opts-new_branch_log  !log_all_ref_updates) {
int temp;
char log_file[PATH_MAX];
-   char *ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
+   const char *ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
 
temp = log_all_ref_updates;
log_all_ref_updates = 1;
diff --git a/builtin/clone.c b/builtin/clone.c
index b12989d..0878e73 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -289,16 +289,17 @@ static void copy_alternates(struct strbuf *src, struct 
strbuf *dst,
struct strbuf line = STRBUF_INIT;
 
while (strbuf_getline(line, in, '\n') != EOF) {
-   char *abs_path, abs_buf[PATH_MAX];
+   char *abs_path;
if (!line.len || line.buf[0] == '#')
continue;
if (is_absolute_path(line.buf)) {
add_to_alternates_file(line.buf);
continue;
}
-   abs_path = mkpath(%s/objects/%s, src_repo, line.buf);
-   normalize_path_copy(abs_buf, abs_path);
-   add_to_alternates_file(abs_buf);
+   abs_path = mkpathdup(%s/objects/%s, src_repo, line.buf);
+   normalize_path_copy(abs_path, abs_path);
+   add_to_alternates_file(abs_path);
+   free(abs_path);
}
strbuf_release(line);
fclose(in);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index dd46b61..eb3180d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -573,7 +573,8 @@ static int store_updated_refs(const char *raw_url, const 
char *remote_name,
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
-   char *url, *filename = dry_run ? /dev/null : git_path(FETCH_HEAD);
+   char *url;
+   const char *filename = dry_run ? /dev/null : git_path(FETCH_HEAD);
int want_status;
 
fp = fopen(filename, a);
@@ -807,7 +808,7 @@ static void check_not_current_branch(struct ref *ref_map)
 
 static int truncate_fetch_head(void)
 {
-   char *filename = git_path(FETCH_HEAD);
+   const char *filename = git_path(FETCH_HEAD);
FILE *fp = fopen(filename, w);
 
if (!fp)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8aadca1..d414962 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -225,12 +225,12 @@ static void check_unreachable_object(struct object *obj)
printf(dangling %s %s\n, typename(obj-type),
   sha1_to_hex(obj-sha1));
if (write_lost_and_found) {
-   char *filename = git_path(lost-found/%s/%s,
+   const char *filename = git_path(lost-found/%s/%s,
obj-type == OBJ_COMMIT ? commit : other,
sha1_to_hex(obj-sha1));
FILE *f;
 
-   if (safe_create_leading_directories(filename)) {
+   if (safe_create_leading_directories_const(filename)) {
error(Could not create lost-found);
return;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..3b64fef 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -599,7 +599,7 @@ static void 

[PATCH v6 03/32] git_snpath(): retire and replace with strbuf_git_path()

2014-07-09 Thread Nguyễn Thái Ngọc Duy
In the previous patch, git_snpath() is modified to allocate a new
strbuf buffer because vsnpath() needs that. But that makes it
awkward because git_snpath() receives a pre-allocated buffer from
outside and has to copy data back. Rename it to strbuf_git_path()
and make it receive strbuf directly.

Using git_path() in update_refs_for_switch() which used to call
git_snpath() is safe because that function and all of its callers do
not keep any pointer to the round-robin buffer pool allocated by
get_pathname().

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 13 +
 cache.h|  4 +--
 path.c | 11 ++--
 refs.c | 78 +-
 refs.h |  2 +-
 5 files changed, 61 insertions(+), 47 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6f74cfb..7356799 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -584,18 +584,21 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (opts-new_orphan_branch) {
if (opts-new_branch_log  !log_all_ref_updates) {
int temp;
-   char log_file[PATH_MAX];
-   const char *ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
+   struct strbuf log_file = STRBUF_INIT;
+   int ret;
+   const char *ref_name;
 
+   ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
temp = log_all_ref_updates;
log_all_ref_updates = 1;
-   if (log_ref_setup(ref_name, log_file, 
sizeof(log_file))) {
+   ret = log_ref_setup(ref_name, log_file);
+   log_all_ref_updates = temp;
+   strbuf_release(log_file);
+   if (ret) {
fprintf(stderr, _(Can not do reflog 
for '%s'\n),
opts-new_orphan_branch);
-   log_all_ref_updates = temp;
return;
}
-   log_all_ref_updates = temp;
}
}
else
diff --git a/cache.h b/cache.h
index 822017c..8faf947 100644
--- a/cache.h
+++ b/cache.h
@@ -674,8 +674,8 @@ extern int check_repository_format(void);
 
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
-extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
-   __attribute__((format (printf, 3, 4)));
+extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
+   __attribute__((format (printf, 2, 3)));
 extern char *git_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
 extern char *mkpathdup(const char *fmt, ...)
diff --git a/path.c b/path.c
index a3f8826..e545064 100644
--- a/path.c
+++ b/path.c
@@ -70,19 +70,12 @@ static void vsnpath(struct strbuf *buf, const char *fmt, 
va_list args)
strbuf_cleanup_path(buf);
 }
 
-char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
 {
-   struct strbuf sb = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   vsnpath(sb, fmt, args);
+   vsnpath(sb, fmt, args);
va_end(args);
-   if (sb.len = n)
-   strlcpy(buf, bad_path, n);
-   else
-   memcpy(buf, sb.buf, sb.len + 1);
-   strbuf_release(sb);
-   return buf;
 }
 
 char *git_pathdup(const char *fmt, ...)
diff --git a/refs.c b/refs.c
index ce5968a..45ab86d 100644
--- a/refs.c
+++ b/refs.c
@@ -1534,10 +1534,12 @@ static const char *handle_missing_loose_ref(const char 
*refname,
 
 const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int 
reading, int *flag)
 {
+   struct strbuf sb_path = STRBUF_INIT;
int depth = MAXDEPTH;
ssize_t len;
char buffer[256];
static char refname_buffer[256];
+   const char *ret;
 
if (flag)
*flag = 0;
@@ -1546,15 +1548,17 @@ const char *resolve_ref_unsafe(const char *refname, 
unsigned char *sha1, int rea
return NULL;
 
for (;;) {
-   char path[PATH_MAX];
+   const char *path;
struct stat st;
char *buf;
int fd;
 
if (--depth  0)
-   return NULL;
+   goto fail;
 
-   git_snpath(path, sizeof(path), %s, refname);
+   strbuf_reset(sb_path);
+   strbuf_git_path(sb_path, %s, refname);
+  

[PATCH v6 06/32] setup_git_env: use git_pathdup instead of xmalloc + sprintf

2014-07-09 Thread Nguyễn Thái Ngọc Duy
From: Jeff King p...@peff.net

This is shorter, harder to get wrong, and more clearly
captures the intent.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 environment.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/environment.c b/environment.c
index 4dac5e9..4de7b81 100644
--- a/environment.c
+++ b/environment.c
@@ -135,15 +135,11 @@ static void setup_git_env(void)
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
git_object_dir = getenv(DB_ENVIRONMENT);
-   if (!git_object_dir) {
-   git_object_dir = xmalloc(strlen(git_dir) + 9);
-   sprintf(git_object_dir, %s/objects, git_dir);
-   }
+   if (!git_object_dir)
+   git_object_dir = git_pathdup(objects);
git_index_file = getenv(INDEX_ENVIRONMENT);
-   if (!git_index_file) {
-   git_index_file = xmalloc(strlen(git_dir) + 7);
-   sprintf(git_index_file, %s/index, git_dir);
-   }
+   if (!git_index_file)
+   git_index_file = git_pathdup(index);
git_graft_file = getenv(GRAFT_ENVIRONMENT);
if (!git_graft_file)
git_graft_file = git_pathdup(info/grafts);
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 07/32] setup_git_env(): introduce git_path_from_env() helper

2014-07-09 Thread Nguyễn Thái Ngọc Duy
From: Jeff King p...@peff.net

Check the value of an environment and fall back to a known path
inside $GIT_DIR is repeated a few times to determine the location
of the data store, the index and the graft file, but the return
value of getenv is not guaranteed to survive across further
invocations of setenv or even getenv.

Make sure to xstrdup() the value we receive from getenv(3), and
encapsulate the pattern into a helper function.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 environment.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/environment.c b/environment.c
index 4de7b81..565f652 100644
--- a/environment.c
+++ b/environment.c
@@ -124,6 +124,12 @@ static char *expand_namespace(const char *raw_namespace)
return strbuf_detach(buf, NULL);
 }
 
+static char *git_path_from_env(const char *envvar, const char *path)
+{
+   const char *value = getenv(envvar);
+   return value ? xstrdup(value) : git_pathdup(%s, path);
+}
+
 static void setup_git_env(void)
 {
const char *gitfile;
@@ -134,15 +140,9 @@ static void setup_git_env(void)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
-   git_object_dir = getenv(DB_ENVIRONMENT);
-   if (!git_object_dir)
-   git_object_dir = git_pathdup(objects);
-   git_index_file = getenv(INDEX_ENVIRONMENT);
-   if (!git_index_file)
-   git_index_file = git_pathdup(index);
-   git_graft_file = getenv(GRAFT_ENVIRONMENT);
-   if (!git_graft_file)
-   git_graft_file = git_pathdup(info/grafts);
+   git_object_dir = git_path_from_env(DB_ENVIRONMENT, objects);
+   git_index_file = git_path_from_env(INDEX_ENVIRONMENT, index);
+   git_graft_file = git_path_from_env(GRAFT_ENVIRONMENT, info/grafts);
if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
check_replace_refs = 0;
namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 01/32] path.c: make get_pathname() return strbuf instead of static buffer

2014-07-09 Thread Nguyễn Thái Ngọc Duy
We've been avoiding PATH_MAX whenever possible. This patch makes
get_pathname() return a strbuf and updates the callers to take
advantage of this. The code is simplified as we no longer need to
worry about buffer overflow.

vsnpath() behavior is changed slightly: previously it always clears
the buffer before writing, now it just appends. Fortunately this is a
static function and all of its callers prepare the buffer properly:
git_path() gets the buffer from get_pathname() which resets the
buffer, the remaining call sites start with STRBUF_INIT'd buffer.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c | 120 -
 1 file changed, 51 insertions(+), 69 deletions(-)

diff --git a/path.c b/path.c
index bc804a3..42ef3af 100644
--- a/path.c
+++ b/path.c
@@ -16,11 +16,15 @@ static int get_st_mode_bits(const char *path, int *mode)
 
 static char bad_path[] = /bad-path/;
 
-static char *get_pathname(void)
+static struct strbuf *get_pathname(void)
 {
-   static char pathname_array[4][PATH_MAX];
+   static struct strbuf pathname_array[4] = {
+   STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+   };
static int index;
-   return pathname_array[3  ++index];
+   struct strbuf *sb = pathname_array[3  ++index];
+   strbuf_reset(sb);
+   return sb;
 }
 
 static char *cleanup_path(char *path)
@@ -34,6 +38,13 @@ static char *cleanup_path(char *path)
return path;
 }
 
+static void strbuf_cleanup_path(struct strbuf *sb)
+{
+   char *path = cleanup_path(sb-buf);
+   if (path  sb-buf)
+   strbuf_remove(sb, 0, path - sb-buf);
+}
+
 char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 {
va_list args;
@@ -49,85 +60,70 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
return cleanup_path(buf);
 }
 
-static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args)
+static void vsnpath(struct strbuf *buf, const char *fmt, va_list args)
 {
const char *git_dir = get_git_dir();
-   size_t len;
-
-   len = strlen(git_dir);
-   if (n  len + 1)
-   goto bad;
-   memcpy(buf, git_dir, len);
-   if (len  !is_dir_sep(git_dir[len-1]))
-   buf[len++] = '/';
-   len += vsnprintf(buf + len, n - len, fmt, args);
-   if (len = n)
-   goto bad;
-   return cleanup_path(buf);
-bad:
-   strlcpy(buf, bad_path, n);
-   return buf;
+   strbuf_addstr(buf, git_dir);
+   if (buf-len  !is_dir_sep(buf-buf[buf-len - 1]))
+   strbuf_addch(buf, '/');
+   strbuf_vaddf(buf, fmt, args);
+   strbuf_cleanup_path(buf);
 }
 
 char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 {
-   char *ret;
+   struct strbuf sb = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   ret = vsnpath(buf, n, fmt, args);
+   vsnpath(sb, fmt, args);
va_end(args);
-   return ret;
+   if (sb.len = n)
+   strlcpy(buf, bad_path, n);
+   else
+   memcpy(buf, sb.buf, sb.len + 1);
+   strbuf_release(sb);
+   return buf;
 }
 
 char *git_pathdup(const char *fmt, ...)
 {
-   char path[PATH_MAX], *ret;
+   struct strbuf path = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   ret = vsnpath(path, sizeof(path), fmt, args);
+   vsnpath(path, fmt, args);
va_end(args);
-   return xstrdup(ret);
+   return strbuf_detach(path, NULL);
 }
 
 char *mkpathdup(const char *fmt, ...)
 {
-   char *path;
struct strbuf sb = STRBUF_INIT;
va_list args;
-
va_start(args, fmt);
strbuf_vaddf(sb, fmt, args);
va_end(args);
-   path = xstrdup(cleanup_path(sb.buf));
-
-   strbuf_release(sb);
-   return path;
+   strbuf_cleanup_path(sb);
+   return strbuf_detach(sb, NULL);
 }
 
 char *mkpath(const char *fmt, ...)
 {
va_list args;
-   unsigned len;
-   char *pathname = get_pathname();
-
+   struct strbuf *pathname = get_pathname();
va_start(args, fmt);
-   len = vsnprintf(pathname, PATH_MAX, fmt, args);
+   strbuf_vaddf(pathname, fmt, args);
va_end(args);
-   if (len = PATH_MAX)
-   return bad_path;
-   return cleanup_path(pathname);
+   return cleanup_path(pathname-buf);
 }
 
 char *git_path(const char *fmt, ...)
 {
-   char *pathname = get_pathname();
+   struct strbuf *pathname = get_pathname();
va_list args;
-   char *ret;
-
va_start(args, fmt);
-   ret = vsnpath(pathname, PATH_MAX, fmt, args);
+   vsnpath(pathname, fmt, args);
va_end(args);
-   return ret;
+   return pathname-buf;
 }
 
 void home_config_paths(char **global, char **xdg, char *file)
@@ -158,41 +154,27 @@ void home_config_paths(char **global, char **xdg, char 
*file)
 
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
-  

[PATCH v6 04/32] path.c: rename vsnpath() to do_git_path()

2014-07-09 Thread Nguyễn Thái Ngọc Duy
The name vsnpath() gives an impression that this is general path
handling function. It's not. This is the underlying implementation of
git_path(), git_pathdup() and strbuf_git_path() which will prefix
$GIT_DIR in the result string.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/path.c b/path.c
index e545064..2cb2e61 100644
--- a/path.c
+++ b/path.c
@@ -60,7 +60,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
return cleanup_path(buf);
 }
 
-static void vsnpath(struct strbuf *buf, const char *fmt, va_list args)
+static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
 {
const char *git_dir = get_git_dir();
strbuf_addstr(buf, git_dir);
@@ -74,7 +74,7 @@ void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
 {
va_list args;
va_start(args, fmt);
-   vsnpath(sb, fmt, args);
+   do_git_path(sb, fmt, args);
va_end(args);
 }
 
@@ -83,7 +83,7 @@ char *git_pathdup(const char *fmt, ...)
struct strbuf path = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   vsnpath(path, fmt, args);
+   do_git_path(path, fmt, args);
va_end(args);
return strbuf_detach(path, NULL);
 }
@@ -114,7 +114,7 @@ const char *git_path(const char *fmt, ...)
struct strbuf *pathname = get_pathname();
va_list args;
va_start(args, fmt);
-   vsnpath(pathname, fmt, args);
+   do_git_path(pathname, fmt, args);
va_end(args);
return pathname-buf;
 }
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 00/32] Support multiple checkouts

2014-07-09 Thread Nguyễn Thái Ngọc Duy
This is basically a reroll from what was parked on 'pu' with two
new patches at the end: one to not share info/sparse-checkout
across worktrees, and one to allow 'checkout --to` from a bare
repository.

I cherry-picked two patches from jk/xstrfmt (on next) because they
result in non-trivial conflicts. When this series is merged with
jk/xstrfmt, you still get conflicts in environment.c, but you can just
pick up my changes and drop Jeff's.

Dennis Kaarsemaker (1):
  checkout: don't require a work tree when checking out into a new one

Jeff King (2):
  setup_git_env: use git_pathdup instead of xmalloc + sprintf
  setup_git_env(): introduce git_path_from_env() helper

Nguyễn Thái Ngọc Duy (29):
  path.c: make get_pathname() return strbuf instead of static buffer
  path.c: make get_pathname() call sites return const char *
  git_snpath(): retire and replace with strbuf_git_path()
  path.c: rename vsnpath() to do_git_path()
  path.c: group git_path(), git_pathdup() and strbuf_git_path() together
  git_path(): be aware of file relocation in $GIT_DIR
  *.sh: respect $GIT_INDEX_FILE
  reflog: avoid constructing .lock path with git_path
  fast-import: use git_path() for accessing .git dir instead of get_git_dir()
  commit: use SEQ_DIR instead of hardcoding sequencer
  $GIT_COMMON_DIR: a new environment variable
  git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects
  *.sh: avoid hardcoding $GIT_DIR/hooks/...
  git-stash: avoid hardcoding $GIT_DIR/logs/
  setup.c: convert is_git_directory() to use strbuf
  setup.c: detect $GIT_COMMON_DIR in is_git_directory()
  setup.c: convert check_repository_format_gently to use strbuf
  setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()
  setup.c: support multi-checkout repo setup
  wrapper.c: wrapper to open a file, fprintf then close
  use new wrapper write_file() for simple file writing
  checkout: support checking out into a new working directory
  checkout: clean up half-prepared directories in --to mode
  checkout: detach if the branch is already checked out elsewhere
  prune: strategies for linked checkouts
  gc: style change -- no SP before closing bracket
  gc: support prune --repos
  count-objects: report unused files in $GIT_DIR/repos/...
  git_path(): keep info/sparse-checkout per work-tree

 Documentation/config.txt   |   9 ++
 Documentation/git-checkout.txt |  34 +
 Documentation/git-prune.txt|   3 +
 Documentation/git-rev-parse.txt|  10 ++
 Documentation/git.txt  |   9 ++
 Documentation/gitrepository-layout.txt |  75 --
 builtin/branch.c   |   4 +-
 builtin/checkout.c | 248 -
 builtin/clone.c|   9 +-
 builtin/commit.c   |   2 +-
 builtin/count-objects.c|   4 +-
 builtin/fetch.c|   5 +-
 builtin/fsck.c |   4 +-
 builtin/gc.c   |  21 ++-
 builtin/init-db.c  |   7 +-
 builtin/prune.c|  74 ++
 builtin/receive-pack.c |   2 +-
 builtin/reflog.c   |   2 +-
 builtin/remote.c   |   2 +-
 builtin/repack.c   |   8 +-
 builtin/rev-parse.c|  11 ++
 cache.h|  17 ++-
 daemon.c   |  11 +-
 environment.c  |  45 --
 fast-import.c  |   7 +-
 git-am.sh  |  22 +--
 git-pull.sh|   2 +-
 git-rebase--interactive.sh |   6 +-
 git-rebase--merge.sh   |   6 +-
 git-rebase.sh  |   4 +-
 git-sh-setup.sh|   2 +-
 git-stash.sh   |   6 +-
 git.c  |   2 +-
 notes-merge.c  |   6 +-
 path.c | 234 +--
 refs.c |  86 +++-
 refs.h |   2 +-
 run-command.c  |   4 +-
 run-command.h  |   2 +-
 setup.c| 124 +
 sha1_file.c|   2 +-
 submodule.c|   9 +-
 t/t0060-path-utils.sh  |  35 +
 t/t1501-worktree.sh|  76 ++
 t/t1510-repo-setup.sh  |   1 +
 t/t2025-checkout-to.sh (new +x)|  72 ++
 templates/hooks--applypatch-msg.sample |   4 +-
 templates/hooks--pre-applypatch.sample |   4 +-
 trace.c|   1 +
 transport.c|   8 +-
 wrapper.c  |  31 +
 51 files changed, 1109 insertions(+), 265 deletions(-)
 create mode 

[PATCH v6 05/32] path.c: group git_path(), git_pathdup() and strbuf_git_path() together

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/path.c b/path.c
index 2cb2e61..65881aa 100644
--- a/path.c
+++ b/path.c
@@ -78,6 +78,16 @@ void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
va_end(args);
 }
 
+const char *git_path(const char *fmt, ...)
+{
+   struct strbuf *pathname = get_pathname();
+   va_list args;
+   va_start(args, fmt);
+   do_git_path(pathname, fmt, args);
+   va_end(args);
+   return pathname-buf;
+}
+
 char *git_pathdup(const char *fmt, ...)
 {
struct strbuf path = STRBUF_INIT;
@@ -109,16 +119,6 @@ const char *mkpath(const char *fmt, ...)
return cleanup_path(pathname-buf);
 }
 
-const char *git_path(const char *fmt, ...)
-{
-   struct strbuf *pathname = get_pathname();
-   va_list args;
-   va_start(args, fmt);
-   do_git_path(pathname, fmt, args);
-   va_end(args);
-   return pathname-buf;
-}
-
 void home_config_paths(char **global, char **xdg, char *file)
 {
char *xdg_home = getenv(XDG_CONFIG_HOME);
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 14/32] git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects

2014-07-09 Thread Nguyễn Thái Ngọc Duy
If $GIT_COMMON_DIR is set, $GIT_OBJECT_DIRECTORY should be
$GIT_COMMON_DIR/objects, not $GIT_DIR/objects. Just let rev-parse
--git-path handle it.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-sh-setup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 9447980..d3dbb2f 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -345,7 +345,7 @@ then
echo 2 Unable to determine absolute path of git directory
exit 1
}
-   : ${GIT_OBJECT_DIRECTORY=$GIT_DIR/objects}
+   : ${GIT_OBJECT_DIRECTORY=$(git rev-parse --git-path objects)}
 fi
 
 peel_committish () {
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 08/32] git_path(): be aware of file relocation in $GIT_DIR

2014-07-09 Thread Nguyễn Thái Ngọc Duy
We allow the user to relocate certain paths out of $GIT_DIR via
environment variables, e.g. GIT_OBJECT_DIRECTORY, GIT_INDEX_FILE and
GIT_GRAFT_FILE. Callers are not supposed to use git_path() or
git_pathdup() to get those paths. Instead they must use
get_object_directory(), get_index_file() and get_graft_file()
respectively. This is inconvenient and could be missed in review (for
example, there's git_path(objects/info/alternates) somewhere in
sha1_file.c).

This patch makes git_path() and git_pathdup() understand those
environment variables. So if you set GIT_OBJECT_DIRECTORY to /foo/bar,
git_path(objects/abc) should return /foo/bar/abc. The same is done
for the two remaining env variables.

git rev-parse --git-path is the wrapper for script use.

This patch kinda reverts a0279e1 (setup_git_env: use git_pathdup
instead of xmalloc + sprintf - 2014-06-19) because using git_pathdup
here would result in infinite recursion:

  setup_git_env() - git_pathdup(objects) - .. - adjust_git_path()
  - get_object_directory() - oops, git_object_directory is NOT set
  yet - setup_git_env()

I wanted to make git_pathdup_literal() that skips adjust_git_path().
But that won't work because later on when $GIT_COMMON_DIR is
introduced, git_pathdup_literal(objects) needs adjust_git_path() to
replace $GIT_DIR with $GIT_COMMON_DIR.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-rev-parse.txt |  7 ++
 builtin/rev-parse.c |  7 ++
 cache.h |  1 +
 environment.c   | 19 +++-
 path.c  | 49 +++--
 t/t0060-path-utils.sh   | 19 
 6 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 987395d..9465399 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -232,6 +232,13 @@ print a message to stderr and exit with nonzero status.
repository.  If path is a gitfile then the resolved path
to the real repository is printed.
 
+--git-path path::
+   Resolve $GIT_DIR/path and takes other path relocation
+   variables such as $GIT_OBJECT_DIRECTORY,
+   $GIT_INDEX_FILE... into account. For example, if
+   $GIT_OBJECT_DIRECTORY is set to /foo/bar then git rev-parse
+   --git-path objects/abc returns /foo/bar/abc.
+
 --show-cdup::
When the command is invoked from a subdirectory, show the
path of the top-level directory relative to the current
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 1a6122d..7606d43 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -529,6 +529,13 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
for (i = 1; i  argc; i++) {
const char *arg = argv[i];
 
+   if (!strcmp(arg, --git-path)) {
+   if (!argv[i + 1])
+   die(--git-path requires an argument);
+   puts(git_path(%s, argv[i + 1]));
+   i++;
+   continue;
+   }
if (as_is) {
if (show_file(arg, output_prefix)  as_is  2)
verify_filename(prefix, arg, 0);
diff --git a/cache.h b/cache.h
index 8faf947..961f93d 100644
--- a/cache.h
+++ b/cache.h
@@ -612,6 +612,7 @@ extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
+extern int git_db_env, git_index_env, git_graft_env;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/environment.c b/environment.c
index 565f652..06bc8cc 100644
--- a/environment.c
+++ b/environment.c
@@ -83,6 +83,7 @@ static size_t namespace_len;
 
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_graft_file;
+int git_db_env, git_index_env, git_graft_env;
 
 /*
  * Repository-local GIT_* environment variables; see cache.h for details.
@@ -124,10 +125,18 @@ static char *expand_namespace(const char *raw_namespace)
return strbuf_detach(buf, NULL);
 }
 
-static char *git_path_from_env(const char *envvar, const char *path)
+static char *git_path_from_env(const char *envvar, const char *path,
+  int* fromenv)
 {
const char *value = getenv(envvar);
-   return value ? xstrdup(value) : git_pathdup(%s, path);
+   if (!value) {
+   char *buf = xmalloc(strlen(git_dir) + strlen(path) + 2);
+   sprintf(buf, %s/%s, git_dir, path);
+   return buf;
+   }
+   if (fromenv)
+   *fromenv = 1;
+   return xstrdup(value);
 }
 
 static void setup_git_env(void)
@@ -140,9 +149,9 @@ static void setup_git_env(void)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
gitfile = read_gitfile(git_dir);

[PATCH v6 15/32] *.sh: avoid hardcoding $GIT_DIR/hooks/...

2014-07-09 Thread Nguyễn Thái Ngọc Duy
If $GIT_COMMON_DIR is set, it should be $GIT_COMMON_DIR/hooks/, not
$GIT_DIR/hooks/. Just let rev-parse --git-path handle it.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-am.sh  | 22 +++---
 git-rebase--interactive.sh |  6 +++---
 git-rebase--merge.sh   |  6 ++
 git-rebase.sh  |  4 ++--
 templates/hooks--applypatch-msg.sample |  4 ++--
 templates/hooks--pre-applypatch.sample |  4 ++--
 6 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index ee61a77..66803d1 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -810,10 +810,10 @@ To restore the original branch and stop patching run 
\\$cmdline --abort\.
continue
fi
 
-   if test -x $GIT_DIR/hooks/applypatch-msg
+   hook=$(git rev-parse --git-path hooks/applypatch-msg)
+   if test -x $hook
then
-   $GIT_DIR/hooks/applypatch-msg $dotest/final-commit ||
-   stop_here $this
+   $hook $dotest/final-commit || stop_here $this
fi
 
if test -f $dotest/final-commit
@@ -887,9 +887,10 @@ did you forget to use 'git add'?
stop_here_user_resolve $this
fi
 
-   if test -x $GIT_DIR/hooks/pre-applypatch
+   hook=$(git rev-parse --git-path hooks/pre-applypatch)
+   if test -x $hook
then
-   $GIT_DIR/hooks/pre-applypatch || stop_here $this
+   $hook || stop_here $this
fi
 
tree=$(git write-tree) 
@@ -916,18 +917,17 @@ did you forget to use 'git add'?
echo $(cat $dotest/original-commit) $commit  
$dotest/rewritten
fi
 
-   if test -x $GIT_DIR/hooks/post-applypatch
-   then
-   $GIT_DIR/hooks/post-applypatch
-   fi
+   hook=$(git rev-parse --git-path hooks/post-applypatch)
+   test -x $hook  $hook
 
go_next
 done
 
 if test -s $dotest/rewritten; then
 git notes copy --for-rewrite=rebase  $dotest/rewritten
-if test -x $GIT_DIR/hooks/post-rewrite; then
-   $GIT_DIR/hooks/post-rewrite rebase  $dotest/rewritten
+hook=$(git rev-parse --git-path hooks/post-rewrite)
+if test -x $hook; then
+   $hook rebase  $dotest/rewritten
 fi
 fi
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7e1eda0..e8995f9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -642,9 +642,9 @@ do_next () {
git notes copy --for-rewrite=rebase  $rewritten_list ||
true # we don't care if this copying failed
} 
-   if test -x $GIT_DIR/hooks/post-rewrite 
-   test -s $rewritten_list; then
-   $GIT_DIR/hooks/post-rewrite rebase  $rewritten_list
+   hook=$(git rev-parse --git-path hooks/post-rewrite)
+   if test -x $hook  test -s $rewritten_list; then
+   $hook rebase  $rewritten_list
true # we don't care if this hook failed
fi 
warn Successfully rebased and updated $head_name.
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index d3fb67d..2cc2a6d 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -94,10 +94,8 @@ finish_rb_merge () {
if test -s $state_dir/rewritten
then
git notes copy --for-rewrite=rebase $state_dir/rewritten
-   if test -x $GIT_DIR/hooks/post-rewrite
-   then
-   $GIT_DIR/hooks/post-rewrite rebase 
$state_dir/rewritten
-   fi
+   hook=$(git rev-parse --git-path hooks/post-rewrite)
+   test -x $hook  $hook rebase $state_dir/rewritten
fi
say All done.
 }
diff --git a/git-rebase.sh b/git-rebase.sh
index 06c810b..d60e710 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -201,9 +201,9 @@ run_specific_rebase () {
 
 run_pre_rebase_hook () {
if test -z $ok_to_skip_pre_rebase 
-  test -x $GIT_DIR/hooks/pre-rebase
+  test -x $(git rev-parse --git-path hooks/pre-rebase)
then
-   $GIT_DIR/hooks/pre-rebase ${1+$@} ||
+   $(git rev-parse --git-path hooks/pre-rebase) ${1+$@} ||
die $(gettext The pre-rebase hook refused to rebase.)
fi
 }
diff --git a/templates/hooks--applypatch-msg.sample 
b/templates/hooks--applypatch-msg.sample
index 8b2a2fe..a5d7b84 100755
--- a/templates/hooks--applypatch-msg.sample
+++ b/templates/hooks--applypatch-msg.sample
@@ -10,6 +10,6 @@
 # To enable this hook, rename this file to applypatch-msg.
 
 . git-sh-setup
-test -x $GIT_DIR/hooks/commit-msg 
-   exec $GIT_DIR/hooks/commit-msg ${1+$@}
+commitmsg=$(git rev-parse --git-path hooks/commit-msg)
+test -x $commitmsg  exec $commitmsg ${1+$@}
 :
diff --git a/templates/hooks--pre-applypatch.sample 
b/templates/hooks--pre-applypatch.sample
index b1f187c..4142082 100755
--- a/templates/hooks--pre-applypatch.sample
+++ 

[PATCH v6 12/32] commit: use SEQ_DIR instead of hardcoding sequencer

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 84cec9a..9f2aba3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -156,7 +156,7 @@ static void determine_whence(struct wt_status *s)
whence = FROM_MERGE;
else if (file_exists(git_path(CHERRY_PICK_HEAD))) {
whence = FROM_CHERRY_PICK;
-   if (file_exists(git_path(sequencer)))
+   if (file_exists(git_path(SEQ_DIR)))
sequencer_in_use = 1;
}
else
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 13/32] $GIT_COMMON_DIR: a new environment variable

2014-07-09 Thread Nguyễn Thái Ngọc Duy
This variable is intended to support multiple working directories
attached to a repository. Such a repository may have a main working
directory, created by either git init or git clone and one or more
linked working directories. These working directories and the main
repository share the same repository directory.

In linked working directories, $GIT_COMMON_DIR must be defined to point
to the real repository directory and $GIT_DIR points to an unused
subdirectory inside $GIT_COMMON_DIR. File locations inside the
repository are reorganized from the linked worktree view point:

 - worktree-specific such as HEAD, logs/HEAD, index, other top-level
   refs and unrecognized files are from $GIT_DIR.

 - the rest like objects, refs, info, hooks, packed-refs, shallow...
   are from $GIT_COMMON_DIR (except info/sparse-checkout, but that's
   a separate patch)

Scripts are supposed to retrieve paths in $GIT_DIR with git rev-parse
--git-path, which will take care of $GIT_DIR vs $GIT_COMMON_DIR
business.

The redirection is done by git_path(), git_pathdup() and
strbuf_git_path(). The selected list of paths goes to $GIT_COMMON_DIR,
not the other way around in case a developer adds a new
worktree-specific file and it's accidentally promoted to be shared
across repositories (this includes unknown files added by third party
commands)

The list of known files that belong to $GIT_DIR are:

ADD_EDIT.patch BISECT_ANCESTORS_OK BISECT_EXPECTED_REV BISECT_LOG
BISECT_NAMES CHERRY_PICK_HEAD COMMIT_MSG FETCH_HEAD HEAD MERGE_HEAD
MERGE_MODE MERGE_RR NOTES_EDITMSG NOTES_MERGE_WORKTREE ORIG_HEAD
REVERT_HEAD SQUASH_MSG TAG_EDITMSG fast_import_crash_* logs/HEAD
next-index-* rebase-apply rebase-merge rsync-refs-* sequencer/*
shallow_*

Path mapping is NOT done for git_path_submodule(). Multi-checkouts are
not supported as submodules.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git.txt  |  8 +++
 Documentation/gitrepository-layout.txt | 42 ++
 cache.h|  4 +++-
 environment.c  | 28 +--
 path.c | 34 +++
 t/t0060-path-utils.sh  | 15 
 6 files changed, 114 insertions(+), 17 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7924209..749052f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -788,6 +788,14 @@ Git so take care if using Cogito etc.
an explicit repository directory set via 'GIT_DIR' or on the
command line.
 
+'GIT_COMMON_DIR'::
+   If this variable is set to a path, non-worktree files that are
+   normally in $GIT_DIR will be taken from this path
+   instead. Worktree-specific files such as HEAD or index are
+   taken from $GIT_DIR. See linkgit:gitrepository-layout[5] for
+   details. This variable has lower precedence than other path
+   variables such as GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY...
+
 Git Commits
 ~~~
 'GIT_AUTHOR_NAME'::
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 17d2ea6..7629e38 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -46,6 +46,9 @@ of incomplete object store is not suitable to be published for
 use with dumb transports but otherwise is OK as long as
 `objects/info/alternates` points at the object stores it
 borrows from.
++
+This directory is ignored if $GIT_COMMON_DIR is set and
+$GIT_COMMON_DIR/objects will be used instead.
 
 objects/[0-9a-f][0-9a-f]::
A newly created object is stored in its own file.
@@ -92,7 +95,8 @@ refs::
References are stored in subdirectories of this
directory.  The 'git prune' command knows to preserve
objects reachable from refs found in this directory and
-   its subdirectories.
+   its subdirectories. This directory is ignored if $GIT_COMMON_DIR
+   is set and $GIT_COMMON_DIR/refs will be used instead.
 
 refs/heads/`name`::
records tip-of-the-tree commit objects of branch `name`
@@ -114,7 +118,8 @@ refs/replace/`obj-sha1`::
 packed-refs::
records the same information as refs/heads/, refs/tags/,
and friends record in a more efficient way.  See
-   linkgit:git-pack-refs[1].
+   linkgit:git-pack-refs[1]. This file is ignored if $GIT_COMMON_DIR
+   is set and $GIT_COMMON_DIR/packed-refs will be used instead.
 
 HEAD::
A symref (see glossary) to the `refs/heads/` namespace
@@ -133,6 +138,11 @@ being a symref to point at the current branch.  Such a 
state
 is often called 'detached HEAD.'  See linkgit:git-checkout[1]
 for details.
 
+config::
+   Repository specific configuration file. This file is ignored
+   $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/config will be
+   used instead.
+
 branches::
A slightly deprecated way to store 

[PATCH v6 11/32] fast-import: use git_path() for accessing .git dir instead of get_git_dir()

2014-07-09 Thread Nguyễn Thái Ngọc Duy
This allows git_path() to redirect info/fast-import to another place
if needed

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 fast-import.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index c74ea15..e8ec34d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3123,12 +3123,9 @@ static void parse_progress(void)
 
 static char* make_fast_import_path(const char *path)
 {
-   struct strbuf abs_path = STRBUF_INIT;
-
if (!relative_marks_paths || is_absolute_path(path))
return xstrdup(path);
-   strbuf_addf(abs_path, %s/info/fast-import/%s, get_git_dir(), path);
-   return strbuf_detach(abs_path, NULL);
+   return xstrdup(git_path(info/fast-import/%s, path));
 }
 
 static void option_import_marks(const char *marks,
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 09/32] *.sh: respect $GIT_INDEX_FILE

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-pull.sh  | 2 +-
 git-stash.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 18a394f..6ab0c31 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -240,7 +240,7 @@ test true = $rebase  {
if ! git rev-parse -q --verify HEAD /dev/null
then
# On an unborn branch
-   if test -f $GIT_DIR/index
+   if test -f $(git rev-parse --git-path index)
then
die $(gettext updating an unborn branch with changes 
added to the index)
fi
diff --git a/git-stash.sh b/git-stash.sh
index bcc757b..393e1ec 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -20,7 +20,7 @@ require_work_tree
 cd_to_toplevel
 
 TMP=$GIT_DIR/.git-stash.$$
-TMPindex=${GIT_INDEX_FILE-$GIT_DIR/index}.stash.$$
+TMPindex=${GIT_INDEX_FILE-$(git rev-parse --git-path index)}.stash.$$
 trap 'rm -f $TMP-* $TMPindex' 0
 
 ref_stash=refs/stash
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 10/32] reflog: avoid constructing .lock path with git_path

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Among pathnames in $GIT_DIR, e.g. index or packed-refs, we want to
automatically and silently map some of them to the $GIT_DIR of the
repository we are borrowing from via $GIT_COMMON_DIR mechanism.  When
we formulate the pathname for its lockfile, we want it to be in the
same location as its final destination.  index is not shared and
needs to remain in the borrowing repository, while packed-refs is
shared and needs to go to the borrowed repository.

git_path() could be taught about the .lock suffix and map
index.lock and packed-refs.lock the same way their basenames are
mapped, but instead the caller can help by asking where the basename
(e.g. index) is mapped to git_path() and then appending .lock
after the mapping is done.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/reflog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index e8a8fb1..9bd874d 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -372,7 +372,7 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
if (!reflog_exists(ref))
goto finish;
if (!cmd-dry_run) {
-   newlog_path = git_pathdup(logs/%s.lock, ref);
+   newlog_path = mkpathdup(%s.lock, log_file);
cb.newlog = fopen(newlog_path, w);
}
 
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 20/32] setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 setup.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index a17389f..79f79f2 100644
--- a/setup.c
+++ b/setup.c
@@ -346,6 +346,10 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
const char *repo_config;
int ret = 0;
 
+   get_common_dir(sb, gitdir);
+   strbuf_addstr(sb, /config);
+   repo_config = sb.buf;
+
/*
 * git_config() can't be used here because it calls git_pathdup()
 * to get $GIT_CONFIG/config. That call will make setup_git_env()
@@ -355,8 +359,6 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
 * Use a gentler version of git_config() to check if this repo
 * is a good one.
 */
-   strbuf_addf(sb, %s/config, gitdir);
-   repo_config = sb.buf;
git_config_early(check_repository_format_version, NULL, repo_config);
if (GIT_REPO_VERSION  repository_format_version) {
if (!nongit_ok)
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 16/32] git-stash: avoid hardcoding $GIT_DIR/logs/....

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-stash.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 393e1ec..41f8f6b 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -184,7 +184,7 @@ store_stash () {
fi
 
# Make sure the reflog for stash is kept.
-   : $GIT_DIR/logs/$ref_stash
+   : $(git rev-parse --git-path logs/$ref_stash)
git update-ref -m $stash_msg $ref_stash $w_commit
ret=$?
test $ret != 0  test -z $quiet 
@@ -259,7 +259,7 @@ save_stash () {
say $(gettext No local changes to save)
exit 0
fi
-   test -f $GIT_DIR/logs/$ref_stash ||
+   test -f $(git rev-parse --git-path logs/$ref_stash) ||
clear_stash || die $(gettext Cannot initialize stash)
 
create_stash $stash_msg $untracked
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 17/32] setup.c: convert is_git_directory() to use strbuf

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 setup.c | 37 +
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/setup.c b/setup.c
index 0a22f8b..425fd79 100644
--- a/setup.c
+++ b/setup.c
@@ -238,31 +238,36 @@ void verify_non_filename(const char *prefix, const char 
*arg)
  */
 int is_git_directory(const char *suspect)
 {
-   char path[PATH_MAX];
-   size_t len = strlen(suspect);
+   struct strbuf path = STRBUF_INIT;
+   int ret = 0;
+   size_t len;
 
-   if (PATH_MAX = len + strlen(/objects))
-   die(Too long path: %.*s, 60, suspect);
-   strcpy(path, suspect);
+   strbuf_addstr(path, suspect);
+   len = path.len;
if (getenv(DB_ENVIRONMENT)) {
if (access(getenv(DB_ENVIRONMENT), X_OK))
-   return 0;
+   goto done;
}
else {
-   strcpy(path + len, /objects);
-   if (access(path, X_OK))
-   return 0;
+   strbuf_addstr(path, /objects);
+   if (access(path.buf, X_OK))
+   goto done;
}
 
-   strcpy(path + len, /refs);
-   if (access(path, X_OK))
-   return 0;
+   strbuf_setlen(path, len);
+   strbuf_addstr(path, /refs);
+   if (access(path.buf, X_OK))
+   goto done;
 
-   strcpy(path + len, /HEAD);
-   if (validate_headref(path))
-   return 0;
+   strbuf_setlen(path, len);
+   strbuf_addstr(path, /HEAD);
+   if (validate_headref(path.buf))
+   goto done;
 
-   return 1;
+   ret = 1;
+done:
+   strbuf_release(path);
+   return ret;
 }
 
 int is_inside_git_dir(void)
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 18/32] setup.c: detect $GIT_COMMON_DIR in is_git_directory()

2014-07-09 Thread Nguyễn Thái Ngọc Duy
If the file $GIT_DIR/commondir exists, it contains the value of
$GIT_COMMON_DIR.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/gitrepository-layout.txt |  7 ++
 setup.c| 43 +-
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 7629e38..0f341fc 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -237,6 +237,13 @@ shallow::
file is ignored if $GIT_COMMON_DIR is set and
$GIT_COMMON_DIR/shallow will be used instead.
 
+commondir::
+   If this file exists, $GIT_COMMON_DIR (see linkgit:git[1]) will
+   be set to the path specified in this file if it is not
+   explicitly set. If the specified path is relative, it is
+   relative to $GIT_DIR. The repository with commondir is
+   incomplete without the repository pointed by commondir.
+
 modules::
Contains the git-repositories of the submodules. This
directory is ignored if $GIT_COMMON_DIR is set and
diff --git a/setup.c b/setup.c
index 425fd79..176d505 100644
--- a/setup.c
+++ b/setup.c
@@ -224,6 +224,33 @@ void verify_non_filename(const char *prefix, const char 
*arg)
'git command [revision...] -- [file...]', arg);
 }
 
+static void get_common_dir(struct strbuf *sb, const char *gitdir)
+{
+   struct strbuf data = STRBUF_INIT;
+   struct strbuf path = STRBUF_INIT;
+   const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+   if (git_common_dir) {
+   strbuf_addstr(sb, git_common_dir);
+   return;
+   }
+   strbuf_addf(path, %s/commondir, gitdir);
+   if (file_exists(path.buf)) {
+   if (strbuf_read_file(data, path.buf, 0) = 0)
+   die_errno(_(failed to read %s), path.buf);
+   while (data.len  (data.buf[data.len - 1] == '\n' ||
+   data.buf[data.len - 1] == '\r'))
+   data.len--;
+   data.buf[data.len] = '\0';
+   strbuf_reset(path);
+   if (!is_absolute_path(data.buf))
+   strbuf_addf(path, %s/, gitdir);
+   strbuf_addbuf(path, data);
+   strbuf_addstr(sb, real_path(path.buf));
+   } else
+   strbuf_addstr(sb, gitdir);
+   strbuf_release(data);
+   strbuf_release(path);
+}
 
 /*
  * Test if it looks like we're at a git directory.
@@ -242,13 +269,22 @@ int is_git_directory(const char *suspect)
int ret = 0;
size_t len;
 
-   strbuf_addstr(path, suspect);
+   /* Check worktree-related signatures */
+   strbuf_addf(path, %s/HEAD, suspect);
+   if (validate_headref(path.buf))
+   goto done;
+
+   strbuf_reset(path);
+   get_common_dir(path, suspect);
len = path.len;
+
+   /* Check non-worktree-related signatures */
if (getenv(DB_ENVIRONMENT)) {
if (access(getenv(DB_ENVIRONMENT), X_OK))
goto done;
}
else {
+   strbuf_setlen(path, len);
strbuf_addstr(path, /objects);
if (access(path.buf, X_OK))
goto done;
@@ -259,11 +295,6 @@ int is_git_directory(const char *suspect)
if (access(path.buf, X_OK))
goto done;
 
-   strbuf_setlen(path, len);
-   strbuf_addstr(path, /HEAD);
-   if (validate_headref(path.buf))
-   goto done;
-
ret = 1;
 done:
strbuf_release(path);
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 19/32] setup.c: convert check_repository_format_gently to use strbuf

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 setup.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 176d505..a17389f 100644
--- a/setup.c
+++ b/setup.c
@@ -342,7 +342,9 @@ void setup_work_tree(void)
 
 static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 {
-   char repo_config[PATH_MAX+1];
+   struct strbuf sb = STRBUF_INIT;
+   const char *repo_config;
+   int ret = 0;
 
/*
 * git_config() can't be used here because it calls git_pathdup()
@@ -353,7 +355,8 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
 * Use a gentler version of git_config() to check if this repo
 * is a good one.
 */
-   snprintf(repo_config, PATH_MAX, %s/config, gitdir);
+   strbuf_addf(sb, %s/config, gitdir);
+   repo_config = sb.buf;
git_config_early(check_repository_format_version, NULL, repo_config);
if (GIT_REPO_VERSION  repository_format_version) {
if (!nongit_ok)
@@ -363,9 +366,10 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
GIT_REPO_VERSION, repository_format_version);
warning(Please upgrade Git);
*nongit_ok = -1;
-   return -1;
+   ret = -1;
}
-   return 0;
+   strbuf_release(sb);
+   return ret;
 }
 
 /*
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 26/32] checkout: detach if the branch is already checked out elsewhere

2014-07-09 Thread Nguyễn Thái Ngọc Duy
The normal rule is anything outside refs/heads/ is detached. This
increases strictness of the rule a bit more: if the branch is checked
out (either in $GIT_COMMON_DIR/HEAD or any $GIT_DIR/repos/.../HEAD)
then it's detached as well.

A hint is given so the user knows where to go and do something there
if they still want to checkout undetached here.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 80 ++
 t/t2025-checkout-to.sh | 15 --
 2 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5a52da4..069e803 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -432,6 +432,11 @@ struct branch_info {
const char *name; /* The short name used */
const char *path; /* The full name of a real branch */
struct commit *commit; /* The named commit */
+   /*
+* if not null the branch is detached because it's already
+* checked out in this checkout
+*/
+   char *checkout;
 };
 
 static void setup_branch_path(struct branch_info *branch)
@@ -640,6 +645,11 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (old-path  advice_detached_head)
detach_advice(new-name);
describe_detached_head(_(HEAD is now at), 
new-commit);
+   if (new-checkout  !*new-checkout)
+   fprintf(stderr, _(hint: the main checkout is 
holding this branch\n));
+   else if (new-checkout)
+   fprintf(stderr, _(hint: the linked checkout %s 
is holding this branch\n),
+   new-checkout);
}
} else if (new-path) { /* Switch branches. */
create_symref(HEAD, new-path, msg.buf);
@@ -982,6 +992,73 @@ static const char *unique_tracking_name(const char *name, 
unsigned char *sha1)
return NULL;
 }
 
+static int check_linked_checkout(struct branch_info *new,
+ const char *name, const char *path)
+{
+   struct strbuf sb = STRBUF_INIT;
+   char *start, *end;
+   if (strbuf_read_file(sb, path, 0)  0)
+   return 0;
+   if (!starts_with(sb.buf, ref:)) {
+   strbuf_release(sb);
+   return 0;
+   }
+
+   start = sb.buf + 4;
+   while (isspace(*start))
+   start++;
+   end = start;
+   while (*end  !isspace(*end))
+   end++;
+   if (!strncmp(start, new-path, end - start) 
+   new-path[end - start] == '\0') {
+   strbuf_release(sb);
+   new-path = NULL; /* detach */
+   new-checkout = xstrdup(name); /* reason */
+   return 1;
+   }
+   strbuf_release(sb);
+   return 0;
+}
+
+static void check_linked_checkouts(struct branch_info *new)
+{
+   struct strbuf path = STRBUF_INIT;
+   DIR *dir;
+   struct dirent *d;
+
+   strbuf_addf(path, %s/repos, get_git_common_dir());
+   if ((dir = opendir(path.buf)) == NULL) {
+   strbuf_release(path);
+   return;
+   }
+
+   strbuf_reset(path);
+   strbuf_addf(path, %s/HEAD, get_git_common_dir());
+   /*
+* $GIT_COMMON_DIR/HEAD is practically outside
+* $GIT_DIR so resolve_ref_unsafe() won't work (it
+* uses git_path). Parse the ref ourselves.
+*/
+   if (check_linked_checkout(new, , path.buf)) {
+   strbuf_release(path);
+   closedir(dir);
+   return;
+   }
+
+   while ((d = readdir(dir)) != NULL) {
+   if (!strcmp(d-d_name, .) || !strcmp(d-d_name, ..))
+   continue;
+   strbuf_reset(path);
+   strbuf_addf(path, %s/repos/%s/HEAD,
+   get_git_common_dir(), d-d_name);
+   if (check_linked_checkout(new, d-d_name, path.buf))
+   break;
+   }
+   strbuf_release(path);
+   closedir(dir);
+}
+
 static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
@@ -1109,6 +1186,9 @@ static int parse_branchname_arg(int argc, const char 
**argv,
else
new-path = NULL; /* not an existing branch */
 
+   if (new-path)
+   check_linked_checkouts(new);
+
new-commit = lookup_commit_reference_gently(rev, 1);
if (!new-commit) {
/* not a commit */
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index 5ec49e2..2d35a9b 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh
@@ -13,13 +13,14 @@ test_expect_success 'checkout --to not updating paths' '
 '
 
 test_expect_success 'checkout --to a new worktree' '
+   git 

[PATCH v6 25/32] checkout: clean up half-prepared directories in --to mode

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 49 +++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7df586a..5a52da4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -20,6 +20,7 @@
 #include resolve-undo.h
 #include submodule.h
 #include argv-array.h
+#include sigchain.h
 
 static const char * const checkout_usage[] = {
N_(git checkout [options] branch),
@@ -814,6 +815,35 @@ static int switch_branches(const struct checkout_opts 
*opts,
return ret || writeout_error;
 }
 
+static const char *junk_work_tree;
+static const char *junk_git_dir;
+static int is_junk;
+static pid_t junk_pid;
+
+static void remove_junk(void)
+{
+   struct strbuf sb = STRBUF_INIT;
+   if (!is_junk || getpid() != junk_pid)
+   return;
+   if (junk_git_dir) {
+   strbuf_addstr(sb, junk_git_dir);
+   remove_dir_recursively(sb, 0);
+   strbuf_reset(sb);
+   }
+   if (junk_work_tree) {
+   strbuf_addstr(sb, junk_work_tree);
+   remove_dir_recursively(sb, 0);
+   }
+   strbuf_release(sb);
+}
+
+static void remove_junk_on_signal(int signo)
+{
+   remove_junk();
+   sigchain_pop(signo);
+   raise(signo);
+}
+
 static int prepare_linked_checkout(const struct checkout_opts *opts,
   struct branch_info *new)
 {
@@ -822,7 +852,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
const char *path = opts-new_worktree, *name;
struct stat st;
struct child_process cp;
-   int counter = 0, len;
+   int counter = 0, len, ret;
 
if (!new-commit)
die(_(no branch specified));
@@ -848,13 +878,21 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
strbuf_addf(sb_repo, %d, counter);
}
name = strrchr(sb_repo.buf, '/') + 1;
+
+   junk_pid = getpid();
+   atexit(remove_junk);
+   sigchain_push_common(remove_junk_on_signal);
+
if (mkdir(sb_repo.buf, 0777))
die_errno(_(could not create directory of '%s'), sb_repo.buf);
+   junk_git_dir = sb_repo.buf;
+   is_junk = 1;
 
strbuf_addf(sb_git, %s/.git, path);
if (safe_create_leading_directories_const(sb_git.buf))
die_errno(_(could not create leading directories of '%s'),
  sb_git.buf);
+   junk_work_tree = path;
 
write_file(sb_git.buf, 1, gitdir: %s/repos/%s\n,
   real_path(get_git_common_dir()), name);
@@ -879,7 +917,14 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
memset(cp, 0, sizeof(cp));
cp.git_cmd = 1;
cp.argv = opts-saved_argv;
-   return run_command(cp);
+   ret = run_command(cp);
+   if (!ret)
+   is_junk = 0;
+   strbuf_release(sb);
+   strbuf_release(sb_repo);
+   strbuf_release(sb_git);
+   return ret;
+
 }
 
 static int git_checkout_config(const char *var, const char *value, void *cb)
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 28/32] gc: style change -- no SP before closing bracket

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8d219d8..53f1302 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -285,7 +285,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
OPT__QUIET(quiet, N_(suppress progress reporting)),
{ OPTION_STRING, 0, prune, prune_expire, N_(date),
N_(prune unreferenced objects),
-   PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
+   PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire},
OPT_BOOL(0, aggressive, aggressive, N_(be more thorough 
(increased runtime))),
OPT_BOOL(0, auto, auto_gc, N_(enable auto-gc mode)),
OPT_BOOL(0, force, force, N_(force running gc even if there 
may be another gc running)),
@@ -298,7 +298,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
argv_array_pushl(pack_refs_cmd, pack-refs, --all, --prune, NULL);
argv_array_pushl(reflog, reflog, expire, --all, NULL);
argv_array_pushl(repack, repack, -d, -l, NULL);
-   argv_array_pushl(prune, prune, --expire, NULL );
+   argv_array_pushl(prune, prune, --expire, NULL);
argv_array_pushl(rerere, rerere, gc, NULL);
 
git_config(gc_config, NULL);
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 22/32] wrapper.c: wrapper to open a file, fprintf then close

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h   |  2 ++
 wrapper.c | 31 +++
 2 files changed, 33 insertions(+)

diff --git a/cache.h b/cache.h
index e7dd5ce..b363c00 100644
--- a/cache.h
+++ b/cache.h
@@ -1355,6 +1355,8 @@ static inline ssize_t write_str_in_full(int fd, const 
char *str)
 {
return write_in_full(fd, str, strlen(str));
 }
+__attribute__((format (printf,3,4)))
+extern int write_file(const char *path, int fatal, const char *fmt, ...);
 
 /* pager.c */
 extern void setup_pager(void);
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..9d7b9ac 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -493,3 +493,34 @@ struct passwd *xgetpwuid_self(void)
errno ? strerror(errno) : _(no such user));
return pw;
 }
+
+int write_file(const char *path, int fatal, const char *fmt, ...)
+{
+   struct strbuf sb = STRBUF_INIT;
+   va_list params;
+   int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0666);
+   if (fd  0) {
+   if (fatal)
+   die_errno(_(could not open %s for writing), path);
+   return -1;
+   }
+   va_start(params, fmt);
+   strbuf_vaddf(sb, fmt, params);
+   va_end(params);
+   if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
+   int err = errno;
+   close(fd);
+   strbuf_release(sb);
+   errno = err;
+   if (fatal)
+   die_errno(_(could not write to %s), path);
+   return -1;
+   }
+   strbuf_release(sb);
+   if (close(fd)) {
+   if (fatal)
+   die_errno(_(could not close %s), path);
+   return -1;
+   }
+   return 0;
+}
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 21/32] setup.c: support multi-checkout repo setup

2014-07-09 Thread Nguyễn Thái Ngọc Duy
The repo setup procedure is updated to detect $GIT_DIR/commondir and
set $GIT_COMMON_DIR properly.

The core.worktree is ignored when $GIT_COMMON_DIR is set. This is
because the config file is shared in multi-checkout setup, but
checkout directories _are_ different. Making core.worktree effective
in all checkouts mean it's back to a single checkout.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt|  2 ++
 Documentation/git-rev-parse.txt |  3 ++
 builtin/rev-parse.c |  4 +++
 cache.h |  1 +
 environment.c   |  8 ++---
 setup.c | 33 +-
 t/t1501-worktree.sh | 76 +
 t/t1510-repo-setup.sh   |  1 +
 trace.c |  1 +
 9 files changed, 115 insertions(+), 14 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bd..286e539 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,6 +380,8 @@ false), while all other repositories are assumed to be bare 
(bare
 
 core.worktree::
Set the path to the root of the working tree.
+   If GIT_COMMON_DIR environment variable is set, core.worktree
+   is ignored and not used for determining the root of working tree.
This can be overridden by the GIT_WORK_TREE environment
variable and the '--work-tree' command-line option.
The value can be an absolute path or relative to the path to
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 9465399..f1867d3 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -215,6 +215,9 @@ If `$GIT_DIR` is not defined and the current directory
 is not detected to lie in a Git repository or work tree
 print a message to stderr and exit with nonzero status.
 
+--git-common-dir::
+   Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
+
 --is-inside-git-dir::
When the current working directory is below the repository
directory print true, otherwise false.
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 7606d43..29475c5 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -757,6 +757,10 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
printf(%s%s.git\n, cwd, len  cwd[len-1] != 
'/' ? / : );
continue;
}
+   if (!strcmp(arg, --git-common-dir)) {
+   puts(get_git_common_dir());
+   continue;
+   }
if (!strcmp(arg, --resolve-git-dir)) {
const char *gitdir = argv[++i];
if (!gitdir)
diff --git a/cache.h b/cache.h
index 8052ddc..e7dd5ce 100644
--- a/cache.h
+++ b/cache.h
@@ -422,6 +422,7 @@ extern char *get_object_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
+extern int get_common_dir(struct strbuf *sb, const char *gitdir);
 extern const char *get_git_namespace(void);
 extern const char *strip_namespace(const char *namespaced_ref);
 extern const char *get_git_work_tree(void);
diff --git a/environment.c b/environment.c
index 76eb38c..8865765 100644
--- a/environment.c
+++ b/environment.c
@@ -141,6 +141,7 @@ static char *git_path_from_env(const char *envvar, const 
char *git_dir,
 
 static void setup_git_env(void)
 {
+   struct strbuf sb = STRBUF_INIT;
const char *gitfile;
const char *shallow_file;
 
@@ -149,12 +150,9 @@ static void setup_git_env(void)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
-   git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
-   if (git_common_dir) {
+   if (get_common_dir(sb, git_dir))
git_common_dir_env = 1;
-   git_common_dir = xstrdup(git_common_dir);
-   } else
-   git_common_dir = git_dir;
+   git_common_dir = strbuf_detach(sb, NULL);
git_object_dir = git_path_from_env(DB_ENVIRONMENT, git_common_dir,
   objects, git_db_env);
git_index_file = git_path_from_env(INDEX_ENVIRONMENT, git_dir,
diff --git a/setup.c b/setup.c
index 79f79f2..8f90bc3 100644
--- a/setup.c
+++ b/setup.c
@@ -224,14 +224,15 @@ void verify_non_filename(const char *prefix, const char 
*arg)
'git command [revision...] -- [file...]', arg);
 }
 
-static void get_common_dir(struct strbuf *sb, const char *gitdir)
+int get_common_dir(struct strbuf *sb, const char *gitdir)
 {
struct strbuf data = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+ 

[PATCH v6 27/32] prune: strategies for linked checkouts

2014-07-09 Thread Nguyễn Thái Ngọc Duy
(alias R=$GIT_COMMON_DIR/repos/id)

 - linked checkouts are supposed to keep its location in $R/gitdir up
   to date. The use case is auto fixup after a manual checkout move.

 - linked checkouts are supposed to update mtime of $R/gitdir. If
   $R/gitdir's mtime is older than a limit, and it points to nowhere,
   repos/id is to be pruned.

 - If $R/locked exists, repos/id is not supposed to be pruned. If
   $R/locked exists and $R/gitdir's mtime is older than a really long
   limit, warn about old unused repo.

 - git checkout --to is supposed to make a hard link named $R/link
   pointing to the .git file on supported file systems to help detect
   the user manually deleting the checkout. If $R/link exists and its
   link count is greated than 1, the repo is kept.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-prune.txt|  3 ++
 Documentation/gitrepository-layout.txt | 19 +
 builtin/checkout.c | 14 +++
 builtin/prune.c| 74 ++
 setup.c| 13 ++
 5 files changed, 123 insertions(+)

diff --git a/Documentation/git-prune.txt b/Documentation/git-prune.txt
index 7a493c8..50e39ec 100644
--- a/Documentation/git-prune.txt
+++ b/Documentation/git-prune.txt
@@ -48,6 +48,9 @@ OPTIONS
 --expire time::
Only expire loose objects older than time.
 
+--repos::
+   Prune directories in $GIT_DIR/repos.
+
 head...::
In addition to objects
reachable from any of our references, keep objects
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 543d874..bed4f1a 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -256,6 +256,25 @@ repos::
$GIT_COMMON_DIR is set and $GIT_COMMON_DIR/repos will be
used instead.
 
+repos/id/gitdir::
+   A text file containing the absolute path back to the .git file
+   that points to here. This is used to check if the linked
+   repository has been manually removed and there is no need to
+   keep this directory any more. mtime of this file should be
+   updated every time the linked repository is accessed.
+
+repos/id/locked::
+   If this file exists, the linked repository may be on a
+   portable device and not available. It does not mean that the
+   linked repository is gone and `repos/id` could be
+   removed. The file's content contains a reason string on why
+   the repository is locked.
+
+repos/id/link::
+   If this file exists, it is a hard link to the linked .git
+   file. It is used to detect if the linked repository is
+   manually removed.
+
 SEE ALSO
 
 linkgit:git-init[1],
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 069e803..98a2f5f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -898,12 +898,22 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
junk_git_dir = sb_repo.buf;
is_junk = 1;
 
+   /*
+* lock the incomplete repo so prune won't delete it, unlock
+* after the preparation is over.
+*/
+   strbuf_addf(sb, %s/locked, sb_repo.buf);
+   write_file(sb.buf, 1, initializing\n);
+
strbuf_addf(sb_git, %s/.git, path);
if (safe_create_leading_directories_const(sb_git.buf))
die_errno(_(could not create leading directories of '%s'),
  sb_git.buf);
junk_work_tree = path;
 
+   strbuf_reset(sb);
+   strbuf_addf(sb, %s/gitdir, sb_repo.buf);
+   write_file(sb.buf, 1, %s\n, real_path(sb_git.buf));
write_file(sb_git.buf, 1, gitdir: %s/repos/%s\n,
   real_path(get_git_common_dir()), name);
/*
@@ -912,6 +922,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
 * value would do because this value will be ignored and
 * replaced at the next (real) checkout.
 */
+   strbuf_reset(sb);
strbuf_addf(sb, %s/HEAD, sb_repo.buf);
write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1));
strbuf_reset(sb);
@@ -930,6 +941,9 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
ret = run_command(cp);
if (!ret)
is_junk = 0;
+   strbuf_reset(sb);
+   strbuf_addf(sb, %s/locked, sb_repo.buf);
+   unlink_or_warn(sb.buf);
strbuf_release(sb);
strbuf_release(sb_repo);
strbuf_release(sb_git);
diff --git a/builtin/prune.c b/builtin/prune.c
index 144a3bd..6db6bcc 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -112,6 +112,70 @@ static void prune_object_dir(const char *path)
}
 }
 
+static const char *prune_repo_dir(const char *id, struct stat *st)
+{
+   char *path;
+   int fd, len;
+   if (file_exists(git_path(repos/%s/locked, id)))
+   return NULL;
+   

[PATCH v6 24/32] checkout: support checking out into a new working directory

2014-07-09 Thread Nguyễn Thái Ngọc Duy
git checkout --to sets up a new working directory with a .git file
pointing to $GIT_DIR/repos/id. It then executes git checkout again
on the new worktree with the same arguments except --to is taken
out. The second checkout execution, which is not contaminated with any
info from the current repository, will actually check out and
everything that normal git checkout does.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-checkout.txt | 34 +
 Documentation/git.txt  |  3 +-
 Documentation/gitrepository-layout.txt |  7 +++
 builtin/checkout.c | 93 +-
 path.c |  2 +-
 t/t2025-checkout-to.sh (new +x)| 48 ++
 6 files changed, 183 insertions(+), 4 deletions(-)
 create mode 100755 t/t2025-checkout-to.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 33ad2ad..fcf73b2 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -225,6 +225,13 @@ This means that you can use `git checkout -p` to 
selectively discard
 edits from your current working tree. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
 
+--to=path::
+   Check out a new branch in a separate working directory at
+   `path`. A new working directory is linked to the current
+   repository, sharing everything except working directory
+   specific files such as HEAD, index... See MULTIPLE CHECKOUT
+   MODE section for more information.
+
 branch::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with refs/heads/, is a valid ref), then that
@@ -388,6 +395,33 @@ $ git reflog -2 HEAD # or
 $ git log -g -2 HEAD
 
 
+MULTIPLE CHECKOUT MODE
+---
+Normally a working directory is attached to repository. When git
+checkout --to is used, a new working directory is attached to the
+current repository. This new working directory is called linked
+checkout as compared to the main checkout prepared by git init or
+git clone. A repository has one main checkout and zero or more
+linked checkouts.
+
+All checkouts share the same repository. Linked checkouts see the
+repository a bit different from the main checkout. When the checkout
+new reads the path $GIT_DIR/HEAD for example, the actual path
+returned could be $GIT_DIR/repos/new/HEAD. This ensures checkouts
+won't step on each other.
+
+Each linked checkout has a private space in $GIT_DIR/repos, usually
+named after the base name of the working directory with a number added
+to make it unique. The linked checkout's $GIT_DIR points to this
+private space while $GIT_COMMON_DIR points to the main checkout's
+$GIT_DIR. These settings are done by git checkout --to.
+
+Because in this mode $GIT_DIR becomes a lightweight virtual file
+system where a path could be rewritten to some place else, accessing
+$GIT_DIR from scripts should use `git rev-parse --git-path` to resolve
+a path instead of using it directly unless the path is known to be
+private to the working directory.
+
 EXAMPLES
 
 
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 749052f..c0a4940 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -792,7 +792,8 @@ Git so take care if using Cogito etc.
If this variable is set to a path, non-worktree files that are
normally in $GIT_DIR will be taken from this path
instead. Worktree-specific files such as HEAD or index are
-   taken from $GIT_DIR. See linkgit:gitrepository-layout[5] for
+   taken from $GIT_DIR. See linkgit:gitrepository-layout[5] and
+   the section 'MULTIPLE CHECKOUT MODE' in linkgit:checkout[1]
details. This variable has lower precedence than other path
variables such as GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY...
 
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 0f341fc..543d874 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -249,6 +249,13 @@ modules::
directory is ignored if $GIT_COMMON_DIR is set and
$GIT_COMMON_DIR/modules will be used instead.
 
+repos::
+   Contains worktree specific information of linked
+   checkouts. Each subdirectory contains the worktree-related
+   part of a linked checkout. This directory is ignored
+   $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/repos will be
+   used instead.
+
 SEE ALSO
 
 linkgit:git-init[1],
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7356799..7df586a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -48,6 +48,10 @@ struct checkout_opts {
const char *prefix;
struct pathspec pathspec;
struct tree *source_tree;
+
+   const char *new_worktree;
+   const char **saved_argv;
+   

[PATCH v6 23/32] use new wrapper write_file() for simple file writing

2014-07-09 Thread Nguyễn Thái Ngọc Duy
This fixes common problems in these code about error handling,
forgetting to close the file handle after fprintf() fails, or not
printing out the error string..

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/branch.c  |  4 +---
 builtin/init-db.c |  7 +--
 daemon.c  | 11 +--
 submodule.c   |  9 ++---
 transport.c   |  8 +++-
 5 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 652b1d2..2d1c57c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -754,7 +754,6 @@ static const char edit_description[] = BRANCH_DESCRIPTION;
 
 static int edit_branch_description(const char *branch_name)
 {
-   FILE *fp;
int status;
struct strbuf buf = STRBUF_INIT;
struct strbuf name = STRBUF_INIT;
@@ -767,8 +766,7 @@ static int edit_branch_description(const char *branch_name)
  %s\n
Lines starting with '%c' will be stripped.\n,
branch_name, comment_line_char);
-   fp = fopen(git_path(edit_description), w);
-   if ((fwrite(buf.buf, 1, buf.len, fp)  buf.len) || fclose(fp)) {
+   if (write_file(git_path(edit_description), 0, %s, buf.buf)) {
strbuf_release(buf);
return error(_(could not write branch description template: 
%s),
 strerror(errno));
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 56f85e2..ce8416a 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -342,7 +342,6 @@ int set_git_dir_init(const char *git_dir, const char 
*real_git_dir,
 static void separate_git_dir(const char *git_dir)
 {
struct stat st;
-   FILE *fp;
 
if (!stat(git_link, st)) {
const char *src;
@@ -358,11 +357,7 @@ static void separate_git_dir(const char *git_dir)
die_errno(_(unable to move %s to %s), src, git_dir);
}
 
-   fp = fopen(git_link, w);
-   if (!fp)
-   die(_(Could not create git link %s), git_link);
-   fprintf(fp, gitdir: %s\n, git_dir);
-   fclose(fp);
+   write_file(git_link, 1, gitdir: %s\n, git_dir);
 }
 
 int init_db(const char *template_dir, unsigned int flags)
diff --git a/daemon.c b/daemon.c
index f9c63e9..40d0297 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1091,15 +1091,6 @@ static struct credentials *prepare_credentials(const 
char *user_name,
 }
 #endif
 
-static void store_pid(const char *path)
-{
-   FILE *f = fopen(path, w);
-   if (!f)
-   die_errno(cannot open pid file '%s', path);
-   if (fprintf(f, %PRIuMAX\n, (uintmax_t) getpid())  0 || fclose(f) 
!= 0)
-   die_errno(failed to write pid file '%s', path);
-}
-
 static int serve(struct string_list *listen_addr, int listen_port,
 struct credentials *cred)
 {
@@ -1309,7 +1300,7 @@ int main(int argc, char **argv)
sanitize_stdfds();
 
if (pid_file)
-   store_pid(pid_file);
+   write_file(pid_file, 1, %PRIuMAX\n, (uintmax_t) getpid());
 
/* prepare argv for serving-processes */
cld_argv = xmalloc(sizeof (char *) * (argc + 2));
diff --git a/submodule.c b/submodule.c
index b80ecac..b7b6059 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1112,16 +1112,11 @@ void connect_work_tree_and_git_dir(const char 
*work_tree, const char *git_dir)
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
const char *real_work_tree = xstrdup(real_path(work_tree));
-   FILE *fp;
 
/* Update gitfile */
strbuf_addf(file_name, %s/.git, work_tree);
-   fp = fopen(file_name.buf, w);
-   if (!fp)
-   die(_(Could not create git link %s), file_name.buf);
-   fprintf(fp, gitdir: %s\n, relative_path(git_dir, real_work_tree,
- rel_path));
-   fclose(fp);
+   write_file(file_name.buf, 1, gitdir: %s\n,
+  relative_path(git_dir, real_work_tree, rel_path));
 
/* Update core.worktree setting */
strbuf_reset(file_name);
diff --git a/transport.c b/transport.c
index 325f03e..172b3d8 100644
--- a/transport.c
+++ b/transport.c
@@ -294,7 +294,6 @@ static int write_one_ref(const char *name, const unsigned 
char *sha1,
 {
struct strbuf *buf = data;
int len = buf-len;
-   FILE *f;
 
/* when called via for_each_ref(), flags is non-zero */
if (flags  !starts_with(name, refs/heads/) 
@@ -303,10 +302,9 @@ static int write_one_ref(const char *name, const unsigned 
char *sha1,
 
strbuf_addstr(buf, name);
if (safe_create_leading_directories(buf-buf) ||
-   !(f = fopen(buf-buf, w)) ||
-   fprintf(f, %s\n, sha1_to_hex(sha1))  0 ||
-   fclose(f))
-   return error(problems writing temporary file %s, buf-buf);
+   write_file(buf-buf, 0, %s\n, 

[PATCH v6 30/32] count-objects: report unused files in $GIT_DIR/repos/...

2014-07-09 Thread Nguyễn Thái Ngọc Duy
In linked checkouts, borrowed parts like config is taken from
$GIT_COMMON_DIR. $GIT_DIR/config is never used. Report them as
garbage.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/count-objects.c |  4 +++-
 cache.h |  1 +
 path.c  | 29 +++--
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb..d3a1620 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -102,8 +102,10 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
-   if (verbose)
+   if (verbose) {
report_garbage = real_report_garbage;
+   report_linked_checkout_garbage();
+   }
memcpy(path, objdir, len);
if (len  objdir[len-1] != '/')
path[len++] = '/';
diff --git a/cache.h b/cache.h
index b363c00..39cf3f0 100644
--- a/cache.h
+++ b/cache.h
@@ -690,6 +690,7 @@ extern const char *mkpath(const char *fmt, ...) 
__attribute__((format (printf, 1
 extern const char *git_path(const char *fmt, ...) __attribute__((format 
(printf, 1, 2)));
 extern const char *git_path_submodule(const char *path, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+extern void report_linked_checkout_garbage(void);
 
 /*
  * Return the name of the file in the local object database that would
diff --git a/path.c b/path.c
index e41d6b3..b5af137 100644
--- a/path.c
+++ b/path.c
@@ -4,6 +4,7 @@
 #include cache.h
 #include strbuf.h
 #include string-list.h
+#include dir.h
 
 static int get_st_mode_bits(const char *path, int *mode)
 {
@@ -91,9 +92,9 @@ static void replace_dir(struct strbuf *buf, int len, const 
char *newdir)
 }
 
 static const char *common_list[] = {
-   /branches, /hooks, /info, /logs, /lost-found, /modules,
+   /branches, /hooks, /info, !/logs, /lost-found, /modules,
/objects, /refs, /remotes, /repos, /rr-cache, /svn,
-   config, gc.pid, packed-refs, shallow,
+   config, !gc.pid, packed-refs, shallow,
NULL
 };
 
@@ -107,6 +108,8 @@ static void update_common_dir(struct strbuf *buf, int 
git_dir_len)
for (p = common_list; *p; p++) {
const char *path = *p;
int is_dir = 0;
+   if (*path == '!')
+   path++;
if (*path == '/') {
path++;
is_dir = 1;
@@ -122,6 +125,28 @@ static void update_common_dir(struct strbuf *buf, int 
git_dir_len)
}
 }
 
+void report_linked_checkout_garbage(void)
+{
+   struct strbuf sb = STRBUF_INIT;
+   const char **p;
+   int len;
+
+   if (!git_common_dir_env)
+   return;
+   strbuf_addf(sb, %s/, get_git_dir());
+   len = sb.len;
+   for (p = common_list; *p; p++) {
+   const char *path = *p;
+   if (*path == '!')
+   continue;
+   strbuf_setlen(sb, len);
+   strbuf_addstr(sb, path);
+   if (file_exists(sb.buf))
+   report_garbage(unused in linked checkout, sb.buf);
+   }
+   strbuf_release(sb);
+}
+
 static void adjust_git_path(struct strbuf *buf, int git_dir_len)
 {
const char *base = buf-buf + git_dir_len;
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 32/32] checkout: don't require a work tree when checking out into a new one

2014-07-09 Thread Nguyễn Thái Ngọc Duy
From: Dennis Kaarsemaker den...@kaarsemaker.net

For normal use cases, it does not make sense for 'checkout' to work on
a bare repository, without a worktree. But checkout --to is an
exception because it _creates_ a new worktree. Allow this option to
run on bare repositories.

People who check out from a bare repository should remember that
core.logallrefupdates is off by default and it should be turned back
on. `--to` cannot do this automatically behind the user's back because
some user may deliberately want no reflog.

For people interested in repository setup/discovery code,
is_bare_repository_cfg (aka core.bare) is unchanged by this patch,
which means 'true' by default for bare repos. Fortunately when we get
the repo through a linked checkout, is_bare_repository_cfg is never
used. So all is still good.

[nd: commit message]

Signed-off-by: Dennis Kaarsemaker den...@kaarsemaker.net
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c |  3 +++
 git.c  |  2 +-
 t/t2025-checkout-to.sh | 15 +++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 98a2f5f..fd89d93 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1353,6 +1353,9 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
if (opts.new_worktree_mode)
opts.new_worktree = NULL;
 
+   if (!opts.new_worktree)
+   setup_work_tree();
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config(merge.conflictstyle, conflict_style, NULL);
diff --git a/git.c b/git.c
index d6b4a55..04d6c88 100644
--- a/git.c
+++ b/git.c
@@ -384,7 +384,7 @@ static struct cmd_struct commands[] = {
{ check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
{ check-mailmap, cmd_check_mailmap, RUN_SETUP },
{ check-ref-format, cmd_check_ref_format },
-   { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
+   { checkout, cmd_checkout, RUN_SETUP },
{ checkout-index, cmd_checkout_index,
RUN_SETUP | NEED_WORK_TREE},
{ cherry, cmd_cherry, RUN_SETUP },
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index 2d35a9b..a219851 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh
@@ -54,4 +54,19 @@ test_expect_success 'detach if the same branch is already 
checked out' '
)
 '
 
+test_expect_success 'checkout --to from a bare repo' '
+   (
+   git clone --bare . bare 
+   cd bare 
+   git checkout --to ../there2 master
+   )
+'
+
+test_expect_success 'checkout from a bare repo without --to' '
+   (
+   cd bare 
+   test_must_fail git checkout master
+   )
+'
+
 test_done
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 29/32] gc: support prune --repos

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt |  7 +++
 builtin/gc.c | 17 +
 2 files changed, 24 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 286e539..470f979 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1211,6 +1211,13 @@ gc.pruneexpire::
now may be used to disable this  grace period and always prune
unreachable objects immediately.
 
+gc.prunereposexpire::
+   When 'git gc' is run, it will call
+   'prune --repos --expire 3.months.ago'.
+   Override the grace period with this config variable. The value
+   now may be used to disable the grace period and always prune
+   $GIT_DIR/repos immediately.
+
 gc.reflogexpire::
 gc.pattern.reflogexpire::
'git reflog expire' removes reflog entries older than
diff --git a/builtin/gc.c b/builtin/gc.c
index 53f1302..1190183 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -33,11 +33,13 @@ static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
 static int detach_auto = 1;
 static const char *prune_expire = 2.weeks.ago;
+static const char *prune_repos_expire = 3.months.ago;
 
 static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
 static struct argv_array reflog = ARGV_ARRAY_INIT;
 static struct argv_array repack = ARGV_ARRAY_INIT;
 static struct argv_array prune = ARGV_ARRAY_INIT;
+static struct argv_array prune_repos = ARGV_ARRAY_INIT;
 static struct argv_array rerere = ARGV_ARRAY_INIT;
 
 static char *pidfile;
@@ -92,6 +94,14 @@ static int gc_config(const char *var, const char *value, 
void *cb)
}
return git_config_string(prune_expire, var, value);
}
+   if (!strcmp(var, gc.prunereposexpire)) {
+   if (value  strcmp(value, now)) {
+   unsigned long now = approxidate(now);
+   if (approxidate(value) = now)
+   return error(_(Invalid %s: '%s'), var, value);
+   }
+   return git_config_string(prune_repos_expire, var, value);
+   }
return git_default_config(var, value, cb);
 }
 
@@ -299,6 +309,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
argv_array_pushl(reflog, reflog, expire, --all, NULL);
argv_array_pushl(repack, repack, -d, -l, NULL);
argv_array_pushl(prune, prune, --expire, NULL);
+   argv_array_pushl(prune_repos, prune, --repos, --expire, NULL);
argv_array_pushl(rerere, rerere, gc, NULL);
 
git_config(gc_config, NULL);
@@ -368,6 +379,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
return error(FAILED_RUN, prune.argv[0]);
}
 
+   if (prune_repos_expire) {
+   argv_array_push(prune_repos, prune_repos_expire);
+   if (run_command_v_opt(prune_repos.argv, RUN_GIT_CMD))
+   return error(FAILED_RUN, prune_repos.argv[0]);
+   }
+
if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
return error(FAILED_RUN, rerere.argv[0]);
 
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 31/32] git_path(): keep info/sparse-checkout per work-tree

2014-07-09 Thread Nguyễn Thái Ngọc Duy
Currently git_path(info/sparse-checkout) resolves to
$GIT_COMMON_DIR/info/sparse-checkout in multiple worktree mode. It
makes more sense for the sparse checkout patterns to be per worktree,
so you can have multiple checkouts with different parts of the tree.

With this, git checkout --to new on a sparse checkout will create
new as a full checkout. Which is expected, it's how a new checkout
is made. The user can reshape the worktree afterwards.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c| 3 ++-
 t/t0060-path-utils.sh | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/path.c b/path.c
index b5af137..b1dd2bc 100644
--- a/path.c
+++ b/path.c
@@ -103,7 +103,8 @@ static void update_common_dir(struct strbuf *buf, int 
git_dir_len)
char *base = buf-buf + git_dir_len;
const char **p;
 
-   if (is_dir_file(base, logs, HEAD))
+   if (is_dir_file(base, logs, HEAD) ||
+   is_dir_file(base, info, sparse-checkout))
return; /* keep this in $GIT_DIR */
for (p = common_list; *p; p++) {
const char *path = *p;
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 2dabcef..da82aab 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -269,6 +269,7 @@ test_git_path GIT_COMMON_DIR=bar logs/HEAD
.git/logs/HEAD
 test_git_path GIT_COMMON_DIR=bar objects  bar/objects
 test_git_path GIT_COMMON_DIR=bar objects/bar  bar/objects/bar
 test_git_path GIT_COMMON_DIR=bar info/exclude bar/info/exclude
+test_git_path GIT_COMMON_DIR=bar info/sparse-checkout 
.git/info/sparse-checkout
 test_git_path GIT_COMMON_DIR=bar remotes/bar  bar/remotes/bar
 test_git_path GIT_COMMON_DIR=bar branches/bar bar/branches/bar
 test_git_path GIT_COMMON_DIR=bar logs/refs/heads/master   
bar/logs/refs/heads/master
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Unexpected end of command stream message looks irrelevant when I try to pull a non-existing branch

2014-07-09 Thread Dmitry
Hi,

I'm using Git 1.8.1 and when I run the following command:

git pull origin matser

I get the following output:

fatal: couldn't find remote ref matser
Unexpected end of command stream

The first line in the output is right on the money but the second one looks 
completely irrelevant - the command is well formed except I perhaps mistyped 
the branch name. It looks like there's some bug that prevents the program from 
just exiting after printing the first line and so the second line is being 
output.

Could you please fix this?

Thank you.
Dmitry.
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 28/32] gc: style change -- no SP before closing bracket

2014-07-09 Thread Eric Sunshine
On Wed, Jul 9, 2014 at 3:33 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  builtin/gc.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/builtin/gc.c b/builtin/gc.c
 index 8d219d8..53f1302 100644
 --- a/builtin/gc.c
 +++ b/builtin/gc.c
 @@ -285,7 +285,7 @@ int cmd_gc(int argc, const char **argv, const char 
 *prefix)
 OPT__QUIET(quiet, N_(suppress progress reporting)),
 { OPTION_STRING, 0, prune, prune_expire, N_(date),
 N_(prune unreferenced objects),
 -   PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
 +   PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire},

Yet, there is a space after the opening '{'. So, this is now
inconsistently formatted as:

{ foo, bar}

 OPT_BOOL(0, aggressive, aggressive, N_(be more thorough 
 (increased runtime))),
 OPT_BOOL(0, auto, auto_gc, N_(enable auto-gc mode)),
 OPT_BOOL(0, force, force, N_(force running gc even if 
 there may be another gc running)),
 @@ -298,7 +298,7 @@ int cmd_gc(int argc, const char **argv, const char 
 *prefix)
 argv_array_pushl(pack_refs_cmd, pack-refs, --all, --prune, 
 NULL);
 argv_array_pushl(reflog, reflog, expire, --all, NULL);
 argv_array_pushl(repack, repack, -d, -l, NULL);
 -   argv_array_pushl(prune, prune, --expire, NULL );
 +   argv_array_pushl(prune, prune, --expire, NULL);
 argv_array_pushl(rerere, rerere, gc, NULL);

 git_config(gc_config, NULL);
 --
 1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 29/32] gc: support prune --repos

2014-07-09 Thread Eric Sunshine
On Wed, Jul 9, 2014 at 3:33 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Documentation/config.txt |  7 +++
  builtin/gc.c | 17 +
  2 files changed, 24 insertions(+)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 286e539..470f979 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1211,6 +1211,13 @@ gc.pruneexpire::
 now may be used to disable this  grace period and always prune
 unreachable objects immediately.

 +gc.prunereposexpire::
 +   When 'git gc' is run, it will call
 +   'prune --repos --expire 3.months.ago'.
 +   Override the grace period with this config variable. The value
 +   now may be used to disable the grace period and always prune
 +   $GIT_DIR/repos immediately.

nit: This reads very slightly better without always:

... disable the grace period and prune [...] immediately.

More below.

 +
  gc.reflogexpire::
  gc.pattern.reflogexpire::
 'git reflog expire' removes reflog entries older than
 diff --git a/builtin/gc.c b/builtin/gc.c
 index 53f1302..1190183 100644
 --- a/builtin/gc.c
 +++ b/builtin/gc.c
 @@ -33,11 +33,13 @@ static int gc_auto_threshold = 6700;
  static int gc_auto_pack_limit = 50;
  static int detach_auto = 1;
  static const char *prune_expire = 2.weeks.ago;
 +static const char *prune_repos_expire = 3.months.ago;

  static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
  static struct argv_array reflog = ARGV_ARRAY_INIT;
  static struct argv_array repack = ARGV_ARRAY_INIT;
  static struct argv_array prune = ARGV_ARRAY_INIT;
 +static struct argv_array prune_repos = ARGV_ARRAY_INIT;
  static struct argv_array rerere = ARGV_ARRAY_INIT;

  static char *pidfile;
 @@ -92,6 +94,14 @@ static int gc_config(const char *var, const char *value, 
 void *cb)
 }
 return git_config_string(prune_expire, var, value);
 }
 +   if (!strcmp(var, gc.prunereposexpire)) {
 +   if (value  strcmp(value, now)) {
 +   unsigned long now = approxidate(now);
 +   if (approxidate(value) = now)
 +   return error(_(Invalid %s: '%s'), var, 
 value);
 +   }
 +   return git_config_string(prune_repos_expire, var, value);

This logic is copied from the gc.pruneexpire case immediately above
it. Would it make sense to factor it out into a helper function (or is
it not worth the bother for just two cases)?

 +   }
 return git_default_config(var, value, cb);
  }

 @@ -299,6 +309,7 @@ int cmd_gc(int argc, const char **argv, const char 
 *prefix)
 argv_array_pushl(reflog, reflog, expire, --all, NULL);
 argv_array_pushl(repack, repack, -d, -l, NULL);
 argv_array_pushl(prune, prune, --expire, NULL);
 +   argv_array_pushl(prune_repos, prune, --repos, --expire, NULL);
 argv_array_pushl(rerere, rerere, gc, NULL);

 git_config(gc_config, NULL);
 @@ -368,6 +379,12 @@ int cmd_gc(int argc, const char **argv, const char 
 *prefix)
 return error(FAILED_RUN, prune.argv[0]);
 }

 +   if (prune_repos_expire) {
 +   argv_array_push(prune_repos, prune_repos_expire);
 +   if (run_command_v_opt(prune_repos.argv, RUN_GIT_CMD))
 +   return error(FAILED_RUN, prune_repos.argv[0]);
 +   }
 +
 if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
 return error(FAILED_RUN, rerere.argv[0]);

 --
 1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] http: Add Accept-Language header if possible

2014-07-09 Thread Peter Krefting

Yi EungJun:


Example:
 LANGUAGE= - 
 LANGUAGE=ko - Accept-Language: ko; q=1.000, *; q=0.001
 LANGUAGE=ko:en - Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001


Avoid adding q=1.000. It is redundant (the default for any 
unqualified language names is 1.0, and additionally there has 
historically been some buggy servers that failed if it was included.



+   p1 = getenv(LANGUAGE);


You need a fallback mechanism here to parse all the possible language 
variables. I would use the first one I find of these:


 1. LANGUAGE
 2. LC_ALL
 3. LC_MESSAGES
 4. LANG

Only LANGUAGE holds a colon-separated list, but the same code can 
parse all of them, just yielding a single entry for the others.



+   strbuf_add(buf, p1, p2 - p1);


The tokens are on the form language_COUNTRY.encoding@identifier, whereas 
Accept-Language wants language-COUNTRY, so you need to a) replace _ 
with -, and b) chop off anything following a . or @.



+   strbuf_addf(buf, ; q=%.3f, q);
+   q -= 0.001;


Three decimals seems a bit overkill, but some experimentation might be 
necessary.



+   strbuf_addstr(buf, *; q=0.001\r\n);


You should probably also add an explicit en here, if none was 
already included. I've seen some servers break horribly if en isn't 
included.




For reference, I have my LANGUAGE variable set to 
sv_SE.utf8:sv:nb_NO.utf8:nb:da_DK.utf8:da:nn_NO.utf8:nn:en_GB.utf8:en_US.utf8:en


--
\\// Peter - http://www.softwolves.pp.se/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 0/3] git config cache special querying api utilizing the cache

2014-07-09 Thread Tanay Abhra
Hi,

[PATCH V7]: Style nits and a broken  chain corrected in 
`t/t1308-config-set.sh`. See
[9] for the nits.

[PATCH V6]: Style nits and mistakes corrected. Diff between v6 and v5[8] is at 
the bottom.
Thanks to Matthieu, Ramsay and Ram for their suggestions.

[PATCH V5]: `config_set` now uses a single hashmap. Corrected style nits raised 
in
the thread[7]. Thanks to Junio and Matthieu for their 
suggestions.

[PATCH v4]: Introduced `config_set` construct which points to a ordered set of
config-files cached as hashmaps. Added relevant API functions. For more
details see the documentation. Rewrote the git_config_get* family to use
`config_set` internally. Added tests for both config_set API and 
git_config_get
family. Added type specific API functions which parses the found value 
and
converts it into a specific type.
Most of the changes implemented are the result of discussion in [6].
Thanks to Eric, Ramsay, Junio, Matthieu  Karsten for their suggestions
and review.

[PATCH v3]: Added flag for NULL values that were causing segfaults in some 
cases.
Added test-config for usage examples.
Minor changes and corrections. Refer to discussion thread[5] for more 
details.
Thanks to Matthieu, Jeff and Junio for their valuable suggestions.

[PATCH v2]:Changed the string_list to a struct instead of pointer to a struct.
Added string-list initilization functions.
Minor mistakes corrected acoording to review comments[4]. Thanks to
Eric and Matthieu for their review.

[PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten 
Bogershausen and
Jeff King has been implemented[1]. Complete rewrite of config_cache*() 
family
using git_config() as hook as suggested by Jeff. Thanks for the review.

[RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten 
Bogershausen.
Added cache invalidation when config file is changed.[2]
I am using git_config_set_multivar_in_file() as an update hook.

This is patch is for this year's GSoC. My project is
Git Config API improvements. The link of my proposal is appended below [3].

The aim of this patch series is to generate a cache for querying values from
the config files in a non-callback manner as the current method reads and
parses the config files every time a value is queried for.

The cache is generated from hooking the update_cache function to the current
parsing and callback mechanism in config.c. It is implemented as an hashmap
using the hashmap-api with variables and its corresponding values list as
its members. The values in the list are sorted in order of increasing priority.
The cache is initialised the first time when any of the new query functions is
called. It is invalidated by using git_config_set_multivar_in_file() as an
update hook.

We add two new functions to the config-api, git_config_get_string() and
git_config_get_string_multi() for querying in a non callback manner from
the cache.

[1] http://marc.info/?t=14017206626r=1w=2
[2] 
http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html
[3] 
https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing
[4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369
[5] http://thread.gmane.org/gmane.comp.version-control.git/251704/
[6] http://thread.gmane.org/gmane.comp.version-control.git/252329/
[7] http://marc.info/?t=14042811521r=1w=2
[8] http://article.gmane.org/gmane.comp.version-control.git/252942/
[9] http://thread.gmane.org/gmane.comp.version-control.git/252959/

Tanay Abhra (2):
  config-hash.c
  test-config

 .gitignore |   1 +
 Documentation/technical/api-config.txt | 134 +++
 Makefile   |   2 +
 cache.h|  34 
 config-hash.c  | 295 +
 config.c   |   3 +
 t/t1308-config-set.sh  | 168 +++
 test-config.c  | 125 ++
 8 files changed, 762 insertions(+)
 create mode 100644 config-hash.c
 create mode 100755 t/t1308-config-set.sh
 create mode 100644 test-config.c

-- 
1.9.0.GIT

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 2/2] test-config: Add tests for the config_set API

2014-07-09 Thread Tanay Abhra
Expose the `config_set` C API as a set of simple commands in order to
facilitate testing. Add tests for the `config_set` API as well as for
`git_config_get_*()` family for the usual config files.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 .gitignore|   1 +
 Makefile  |   1 +
 t/t1308-config-set.sh | 168 ++
 test-config.c | 125 +
 4 files changed, 295 insertions(+)
 create mode 100755 t/t1308-config-set.sh
 create mode 100644 test-config.c

diff --git a/.gitignore b/.gitignore
index 42294e5..eeb66e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -176,6 +176,7 @@
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
 /test-chmtime
+/test-config
 /test-ctype
 /test-date
 /test-delta
diff --git a/Makefile b/Makefile
index d503f78..e875070 100644
--- a/Makefile
+++ b/Makefile
@@ -548,6 +548,7 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-config
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
new file mode 100755
index 000..27bce1e
--- /dev/null
+++ b/t/t1308-config-set.sh
@@ -0,0 +1,168 @@
+#!/bin/sh
+
+test_description='Test git config-set API in different settings'
+
+. ./test-lib.sh
+
+# 'check section.key value' verifies that the entry for section.key is
+# 'value'
+check() {
+   echo $2 expected 
+   test-config get_value $1 actual 21 
+   test_cmp expected actual
+}
+
+test_expect_success 'setup default config' '
+   cat .git/config  EOF
+   [core]
+   penguin = very blue
+   Movie = BadPhysics
+   UPPERCASE = true
+   MixedCase = true
+   my =
+   foo
+   baz = sam
+   [Cores]
+   WhatEver = Second
+   baz = bar
+   [cores]
+   baz = bat
+   [CORES]
+   baz = ball
+   [my Foo bAr]
+   hi = mixed-case
+   [my FOO BAR]
+   hi = upper-case
+   [my foo bar]
+   hi = lower-case
+   [core]
+   baz = bat
+   baz = hask
+   [lamb]
+   chop = 65
+   head = none
+   [goat]
+   legs = 4
+   head = true
+   skin = false
+   nose = 1
+   horns
+   EOF
+'
+
+test_expect_success 'get value for a simple key' '
+   check core.penguin very blue
+'
+
+test_expect_success 'get value for a key with value as an empty string' '
+   check core.my 
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+   check core.foo (NULL)
+'
+
+test_expect_success 'upper case key' '
+   check core.UPPERCASE true
+'
+
+test_expect_success 'mixed case key' '
+   check core.MixedCase true
+'
+
+test_expect_success 'key and value with mixed case' '
+   check core.Movie BadPhysics
+'
+
+test_expect_success 'key with case sensitive subsection' '
+   check my.Foo bAr.hi mixed-case 
+   check my.FOO BAR.hi upper-case 
+   check my.foo bar.hi lower-case
+'
+
+test_expect_success 'key with case insensitive section header' '
+   check cores.baz ball 
+   check Cores.baz ball 
+   check CORES.baz ball 
+   check coreS.baz ball
+'
+
+test_expect_success 'find value with misspelled key' '
+   test_must_fail check my.fOo Bar.hi Value not found for \my.fOo 
Bar.hi\
+'
+
+test_expect_success 'find value with the highest priority' '
+   check core.baz hask
+'
+
+test_expect_success 'find integer value for a key' '
+   echo 65 expect 
+   test-config get_int lamb.chop actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'find integer if value is non parse-able' '
+   echo 65 expect 
+   test_must_fail test-config get_int lamb.head actual 
+   test_must_fail test_cmp expect actual
+'
+
+test_expect_success 'find bool value for the entered key' '
+   cat expect -\EOF 
+   1
+   0
+   1
+   1
+   1
+   EOF
+   test-config get_bool goat.head actual 
+   test-config get_bool goat.skin actual 
+   test-config get_bool goat.nose actual 
+   test-config get_bool goat.horns actual 
+   test-config get_bool goat.legs actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'find multiple values' '
+   cat expect -\EOF 
+   sam
+   bat
+   hask
+   EOF
+   test-config get_value_multi core.bazactual 
+   test_cmp expect actual
+'
+
+test_expect_success 'find value from a configset' '
+   cat config2 -\EOF 
+   [core]
+   baz = lama
+   [my]
+   new = silk
+   [core]
+   baz = ball
+   EOF
+   echo silk expect 
+   test-config configset_get_value my.new config2 

[PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Tanay Abhra
Currently `git_config()` uses a callback mechanism and file rereads for
config values. Due to this approach, it is not uncommon for the config
files to be parsed several times during the run of a git program, with
different callbacks picking out different variables useful to themselves.

Add a `config_set`, that can be used to construct an in-memory cache for
config-like files that the caller specifies (i.e., files like `.gitmodules`,
`~/.gitconfig` etc.). Add two external functions `git_configset_get_value`
and `git_configset_get_value_multi` for querying from the config sets.
`git_configset_get_value` follows `last one wins` semantic (i.e. if there
are multiple matches for the queried key in the files of the configset the
value returned will be the last entry in `value_list`).
`git_configset_get_value_multi` returns a list of values sorted in order of
increasing priority (i.e. last match will be at the end of the list). Add
type specific query functions like `git_configset_get_bool` and similar.

Add a default `config_set`, `the_config_set` to cache all key-value pairs
read from usual config files (repo specific .git/config, user wide
~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set`
is populated using `git_config()`.

Add two external functions `git_config_get_value` and 
`git_config_get_value_multi` for querying in a non-callback manner from
`the_config_set`. Also, add type specific query functions that are
implemented as a thin wrapper around the `config_set` API.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 Documentation/technical/api-config.txt | 134 +++
 Makefile   |   1 +
 cache.h|  34 
 config-hash.c  | 295 +
 config.c   |   3 +
 5 files changed, 467 insertions(+)
 create mode 100644 config-hash.c

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 230b3a0..65a6717 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -77,6 +77,81 @@ To read a specific file in git-config format, use
 `git_config_from_file`. This takes the same callback and data parameters
 as `git_config`.
 
+Querying For Specific Variables
+---
+
+For programs wanting to query for specific variables in a non-callback
+manner, the config API provides two functions `git_config_get_value`
+and `git_config_get_value_multi`. They both read values from an internal
+cache generated previously from reading the config files.
+
+`int git_config_get_value(const char *key, const char **value)`::
+
+   Finds the highest-priority value for the configuration variable `key`,
+   stores the pointer to it in `value` and returns 0. When the
+   configuration variable `key` is not found, returns 1 without touching
+   `value`. The caller should not free or modify `value`, as it is owned
+   by the cache.
+
+`const struct string_list *git_config_get_value_multi(const char *key)`::
+
+   Finds and returns the value list, sorted in order of increasing priority
+   for the configuration variable `key`. When the configuration variable
+   `key` is not found, returns NULL. The caller should not free or modify
+   the returned pointer, as it is owned by the cache.
+
+`void git_config_clear(void)`::
+
+   Resets and invalidates the config cache.
+
+The config API also provides type specific API functions which do conversion
+as well as retrieval for the queried variable, including:
+
+`int git_config_get_int(const char *key, int *dest)`::
+
+   Finds and parses the value to an integer for the configuration variable
+   `key`. Dies on error; otherwise, stores the value of the parsed integer 
in
+   `dest` and returns 0. When the configuration variable `key` is not 
found,
+   returns 1 without touching `dest`.
+
+`int git_config_get_ulong(const char *key, unsigned long *dest)`::
+
+   Similar to `git_config_get_int` but for unsigned longs.
+
+`int git_config_get_bool(const char *key, int *dest)`::
+
+   Finds and parses the value into a boolean value, for the configuration
+   variable `key`respecting keywords like true and false. Integer
+   values are converted into true/false values (when they are non-zero or
+   zero, respectively). Other values cause a die(). If parsing is 
successful,
+   stores the value of the parsed result in `dest` and returns 0. When the
+   configuration variable `key` is not found, returns 1 without touching
+   `dest`.
+
+`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`::
+
+   Similar to `git_config_get_bool`, except that integers are copied as-is,
+   and `is_bool` flag is unset.
+
+`int git_config_get_maybe_bool(const char *key, int *dest)`::
+
+   Similar to `git_config_get_bool`, except 

Re: [PATCH v6 2/2] test-config: Add tests for the config_set API

2014-07-09 Thread Tanay Abhra


On 7/7/2014 10:34 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 diff --git a/t/t1308-config-hash.sh b/t/t1308-config-hash.sh
 new file mode 100755
 index 000..ad99f8b
 --- /dev/null
 +++ b/t/t1308-config-hash.sh
 +test_expect_success 'setup default config' '
 +cat .git/config  EOF
 
 Missing  here (sorry, I should have noticed the first time).
 

Does a single cat command warrant a ``? It errors out when I try
to add it there.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 27/32] prune: strategies for linked checkouts

2014-07-09 Thread Eric Sunshine
On Wed, Jul 9, 2014 at 3:33 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 (alias R=$GIT_COMMON_DIR/repos/id)

  - linked checkouts are supposed to keep its location in $R/gitdir up
to date. The use case is auto fixup after a manual checkout move.

  - linked checkouts are supposed to update mtime of $R/gitdir. If
$R/gitdir's mtime is older than a limit, and it points to nowhere,
repos/id is to be pruned.

  - If $R/locked exists, repos/id is not supposed to be pruned. If
$R/locked exists and $R/gitdir's mtime is older than a really long
limit, warn about old unused repo.

  - git checkout --to is supposed to make a hard link named $R/link
pointing to the .git file on supported file systems to help detect
the user manually deleting the checkout. If $R/link exists and its
link count is greated than 1, the repo is kept.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
 diff --git a/builtin/prune.c b/builtin/prune.c
 index 144a3bd..6db6bcc 100644
 --- a/builtin/prune.c
 +++ b/builtin/prune.c
 @@ -112,6 +112,70 @@ static void prune_object_dir(const char *path)
 }
  }

 +static const char *prune_repo_dir(const char *id, struct stat *st)
 +{
 +   char *path;
 +   int fd, len;
 +   if (file_exists(git_path(repos/%s/locked, id)))
 +   return NULL;
 +   if (stat(git_path(repos/%s/gitdir, id), st)) {
 +   st-st_mtime = expire;
 +   return _(gitdir does not exist);

If a plain file exists in 'repos' for some reason, it will be caught
by this case. Would it make sense, however, to handle that specially
and report a more accurate message, such as not a repo or some such?

 +   }
 +   fd = open(git_path(repos/%s/gitdir, id), O_RDONLY);

If 'gitdir' fails to open for some reason (lack of permissions, it's a
directory, etc.), the subsequent read_in_full() will crash.

 +   len = st-st_size;
 +   path = xmalloc(len + 1);
 +   read_in_full(fd, path, len);
 +   close(fd);

strbuf_readfile() might make this a bit cleaner (though has higher overhead).

 +   while (path[len - 1] == '\n' || path[len - 1] == '\r')
 +   len--;

If, for some reason, 'gitdir' content is empty or consists only of CR
and LF, this will access memory outside of the allocated region.
Probably want:

while (len  0  (... || ...))

 +   path[len] = '\0';
 +   if (!file_exists(path)) {

What happens if 'path' ends up empty (due to hand-editing of 'gitdir'
by the user, for instance)? Does this case deserve an appropriate
diagnostic (corrupt 'gitdir' file or something)?

 +   struct stat st_link;
 +   free(path);
 +   /*
 +* the repo is moved manually and has not been
 +* accessed since?
 +*/
 +   if (!stat(git_path(repos/%s/link, id), st_link) 
 +   st_link.st_nlink  1)
 +   return NULL;
 +   return _(gitdir points to non-existing file);

s/existing/existent/
s/file/location/

 +   }
 +   free(path);
 +   return NULL;
 +}
 +
 +static void prune_repos_dir(void)
 +{
 +   const char *reason;
 +   DIR *dir = opendir(git_path(repos));
 +   struct dirent *d;
 +   int removed = 0;
 +   struct stat st;
 +   if (!dir)
 +   return;
 +   while ((d = readdir(dir)) != NULL) {
 +   if (!strcmp(d-d_name, .) || !strcmp(d-d_name, ..))
 +   continue;
 +   if ((reason = prune_repo_dir(d-d_name, st)) != NULL 
 +   st.st_mtime = expire) {
 +   struct strbuf sb = STRBUF_INIT;
 +   if (show_only || verbose)
 +   printf(_(Removing repos/%s: %s\n), 
 d-d_name, reason);
 +   if (show_only)
 +   continue;
 +   strbuf_addstr(sb, git_path(repos/%s, d-d_name));
 +   remove_dir_recursively(sb, 0);

What happens if this entry in 'repos' is a plain file (or other
non-directory)? Based upon my reading of remove_dir_recursively(), it
won't be deleted, yet the logic of prune_repo_dir() implies that such
an entry should be pruned. Perhaps handle this case specially with
unlink()?

 +   strbuf_release(sb);
 +   removed = 1;
 +   }
 +   }
 +   closedir(dir);
 +   if (removed)
 +   rmdir(git_path(repos));

This works, but at first glance it seems strange not to be checking
'show_only' before calling destructive rmdir().

However, stepping back, it's not quite clear what the intent is.
Ignoring the return value of rmdir() implies that you trust it to Do
The Right Thing: succeed when 'repos' is empty and fail when not. This
assumption of behavior applies regardless of whether or not any
content of 'repos' was removed, so the 'removed' flag does not seem

Re: t5150-request-pull.sh fails on newest master in Debian

2014-07-09 Thread Øyvind A . Holm
On 9 July 2014 03:18, David Turner dtur...@twopensource.com wrote:
 On Wed, 2014-07-09 at 02:52 +0200, Øyvind A. Holm wrote:
  On 3 July 2014 23:55, Øyvind A. Holm su...@sunbase.org wrote:
   When compiling newest master (v2.0.1-472-g6f92e5f) on Debian 7.5
   (64-bit), t5150-request-pull.sh fails when compiling with
   [snip]
 
  FYI, t5150-request-pull.sh passes all tests now on newest master
  (v2.0.1-474-g72c7794) in Debian. There are two new commits on master
  since I wrote this, and the commit that makes things work again is
  4602f1a (diff-tree: call free_commit_list() instead of duplicating
  its code). Reverting this commit brings the failure back.
 
  The whole thing is still a mystery to me, though. I can't see why
  this should have anything to do with the use of ./configure
  --prefix.

 The problem only happens when a ref with an allowed wildcard winds up
 on a page boundary (with the wildcard before the page boundary).  This
 depends intricately on the details of memory allocation, so pretty
 much anything could make it come and go.

Aha, that makes sense. Sheer luck that the results were that consistent
during testing, then.

 Does the fix I posted work for you?  If not, let me know and I'll look
 into it more.

Sorry, didn't notice you posted that to the list. Today I learned that
Gmail doesn't put mails adressed to me and the list in the inbox. :(

The commit fixed it, yes. Thanks for the patch. It now works on both
Debian servers. Have run all tests on one of the servers, and will
repeat on other machines, too.

Thanks,
Øyvind
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] refs.c: handle REFNAME_REFSPEC_PATTERN at end of page

2014-07-09 Thread Øyvind A . Holm
On 7 July 2014 20:05, Junio C Hamano gits...@pobox.com wrote:
 David Turner dtur...@twopensource.com writes:
  When a ref crosses a memory page boundary, we restart the parsing at
  the beginning with the bytewise code.  Pass the original flags to
  that code, rather than the current flags.

 Good.

I've run the whole test suite with this patch applied, and it fixes the
problem on 64-bit Debian 7.5.

 I probably should add:
 Reported-by: Øyvind A. Holm su...@sunbase.org

 before your sign-off.

Thanks. :)

Cheers,
Øyvind
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 2/2] test-config: Add tests for the config_set API

2014-07-09 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 On 7/7/2014 10:34 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 diff --git a/t/t1308-config-hash.sh b/t/t1308-config-hash.sh
 new file mode 100755
 index 000..ad99f8b
 --- /dev/null
 +++ b/t/t1308-config-hash.sh
 +test_expect_success 'setup default config' '
 +   cat .git/config  EOF
 
 Missing  here (sorry, I should have noticed the first time).
 

 Does a single cat command warrant a ``? It errors out when I try
 to add it there.

Ahh, my bad, I didn't notice that cat was the only command in the test.

You're right, no  needed or possible here.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 2/2] test-config: Add tests for the config_set API

2014-07-09 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 +test_expect_success 'find value with misspelled key' '
 + test_must_fail check my.fOo Bar.hi Value not found for \my.fOo 
 Bar.hi\
 +'

Sorry, this is still not right. You're checking that either test-config
OR test_cmp fails. You want to check both.

Basically, you can't use the check helper here. Your v5 was right for
this test:

test_expect_success 'find value with misspelled key' '
echo Value not found for \my.fOo Bar.hi\ expect 
test_must_fail test-config get_value my.fOo Bar.hi actual 
test_cmp expect actual
'


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 diff --git a/Documentation/technical/api-config.txt 
 b/Documentation/technical/api-config.txt
 index 230b3a0..65a6717 100644
 --- a/Documentation/technical/api-config.txt
 +++ b/Documentation/technical/api-config.txt

 +`int git_config_get_bool(const char *key, int *dest)`::
 +
 + Finds and parses the value into a boolean value, for the configuration
 + variable `key`respecting keywords like true and false. Integer

Missing space after ` = badly formatted in HTML.

I didn't find any other formatting error, but you may double-check with

cd Documentation/  make technical/api-config.html  firefox 
technical/api-config.html

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Tanay Abhra


On 7/9/2014 5:42 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 diff --git a/Documentation/technical/api-config.txt 
 b/Documentation/technical/api-config.txt
 index 230b3a0..65a6717 100644
 --- a/Documentation/technical/api-config.txt
 +++ b/Documentation/technical/api-config.txt
 
 +`int git_config_get_bool(const char *key, int *dest)`::
 +
 +Finds and parses the value into a boolean value, for the configuration
 +variable `key`respecting keywords like true and false. Integer
 
 Missing space after ` = badly formatted in HTML.
 
 I didn't find any other formatting error, but you may double-check with
 
 cd Documentation/  make technical/api-config.html  firefox 
 technical/api-config.html
 

Sorry for the wrong version number on the cover letter.
Thanks, for the review, I will check it ASAP.

Also, I tried implementing Junio's request about saving the line number and the 
file name for each
key-value pair. I had some success, but with some changes,

1. config-hash.c had to be shifted to config.c entirely.
2. string_list util pointer had to be used for each value.

I am appending the diff below, the only changes from version 7 is that, a new 
struct 'key_source'
has been added and `config_hash_add_value` has been altered a little.

For debugging `git_configset_get_value` prints the line number and file name 
for each value.
What are your views about it, too late for inclusion? too hackey or contrived?
I has started on a fresh page for it, but I soon saw that I was reimplementing 
what was already
available in git_config(), so I took this path.

One more thing, Karsten's string-intern API can be used for saving file names 
as they are repeated a
lot.

-- 8 --
diff --git a/config.c b/config.c
index a1aef1c..697ec1c 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,8 @@
 #include exec_cmd.h
 #include strbuf.h
 #include quote.h
+#include string-list.h
+#include hashmap.h

 struct config_source {
struct config_source *prev;
@@ -33,10 +35,314 @@ struct config_source {
long (*do_ftell)(struct config_source *c);
 };

+struct config_hash_entry {
+   struct hashmap_entry ent;
+   char *key;
+   struct string_list value_list;
+};
+
+struct key_source {
+   const char *filename;
+   int linenr;
+};
+
 static struct config_source *cf;

 static int zlib_compression_seen;

+/*
+ * Default config_set that contains key-value pairs from the usual set of 
config
+ * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG
+ * config file and the global /etc/gitconfig)
+ */
+static struct config_set the_config_set;
+
+static int config_hash_add_value(const char *, const char *, struct hashmap *);
+
+static int config_hash_entry_cmp(const struct config_hash_entry *e1,
+const struct config_hash_entry *e2, const void 
*unused)
+{
+   return strcmp(e1-key, e2-key);
+}
+
+static void configset_init(struct config_set *cs)
+{
+   if (!cs-hash_initialized) {
+   hashmap_init(cs-config_hash, 
(hashmap_cmp_fn)config_hash_entry_cmp, 0);
+   cs-hash_initialized = 1;
+   }
+}
+
+static int config_hash_callback(const char *key, const char *value, void *cb)
+{
+   struct config_set *cs = cb;
+   config_hash_add_value(key, value, cs-config_hash);
+   return 0;
+}
+
+int git_configset_add_file(struct config_set *cs, const char *filename)
+{
+   int ret = 0;
+   configset_init(cs);
+   ret = git_config_from_file(config_hash_callback, filename, cs);
+   if (!ret)
+   return 0;
+   else {
+   hashmap_free(cs-config_hash, 1);
+   cs-hash_initialized = 0;
+   return -1;
+   }
+}
+
+static struct config_hash_entry *config_hash_find_entry(const char *key,
+   struct hashmap 
*config_hash)
+{
+   struct config_hash_entry k;
+   struct config_hash_entry *found_entry;
+   char *normalized_key;
+   int ret;
+   /*
+* `key` may come from the user, so normalize it before using it
+* for querying entries from the hashmap.
+*/
+   ret = git_config_parse_key(key, normalized_key, NULL);
+
+   if (ret)
+   return NULL;
+
+   hashmap_entry_init(k, strhash(normalized_key));
+   k.key = normalized_key;
+   found_entry = hashmap_get(config_hash, k, NULL);
+   free(normalized_key);
+   return found_entry;
+}
+
+static struct string_list *configset_get_list(const char *key, struct 
config_set *cs)
+{
+   struct config_hash_entry *e = config_hash_find_entry(key, 
cs-config_hash);
+   return e ? e-value_list : NULL;
+}
+
+static int config_hash_add_value(const char *key, const char *value, struct 
hashmap *config_hash)
+{
+   struct config_hash_entry *e;
+   struct string_list_item *si;
+   struct key_source *ks = xmalloc(sizeof(*e));
+   e = config_hash_find_entry(key, 

Re: [PATCH v7 2/2] test-config: Add tests for the config_set API

2014-07-09 Thread Tanay Abhra


On 7/9/2014 5:43 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 +test_expect_success 'find value with misspelled key' '
 +test_must_fail check my.fOo Bar.hi Value not found for \my.fOo 
 Bar.hi\
 +'
 
 Sorry, this is still not right. You're checking that either test-config
 OR test_cmp fails. You want to check both.
 
 Basically, you can't use the check helper here. Your v5 was right for
 this test:
 
 test_expect_success 'find value with misspelled key' '
   echo Value not found for \my.fOo Bar.hi\ expect 
   test_must_fail test-config get_value my.fOo Bar.hi actual 
   test_cmp expect actual
 '
 
 

Yup, I had thought so too, that was the reason I left the  chain in check()
in version 6. I will revert back to v5 for it.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 Also, I tried implementing Junio's request about saving the line
 number and the file name for each
 key-value pair. I had some success, but with some changes,

My opinion on this:

* It's low priority. I think the order of priority should be (high to
  low)

  1) Finish and get the current series into pu (we're almost there, it's
 not time to back off and restart something new).

  2) Work on the other series that uses the new API. We don't need to
 change _all_ the uses, but we do need a few tens of them to
 validate the fact that the new API is nice and convenient to use.

  3) Get new actual features for the user (tidy up config files, give
 error messages based on numbers).

  You probably won't finish everything, so just think: if the GSoC ends
  between 1) and 2), how serious is it? if it ends between 2) and 3),
  how serious? If reverse the order, then the risk of having nothing
  finished and mergeable at the end is high. If it happens, the
  community may or may not take over and finish it ...

* Still, making sure that the (file, line) is doable later without too
  much change is good. So, if indeed, moving all code to another file is
  required, then it may make sense to do it now to avoid code move
  later.

 1. config-hash.c had to be shifted to config.c entirely.

Why? I guess one reason is the use of struct cf (are there others?), but
moving just

config_hash_callback
config_hash_add_value
git_configset_add_file

to config.c should do it. Then, config.c could use config-hash.c.

But a cleaner way would be to allow the callback to receive the
config_file struct without accessing it as a global variable (currently,
we have no way to parse two config files in parallel for example).

In practice, it should be possible to pass a 4th pointer argument to the
callback, and keep compatibility with 3 arguments callback (having too
many arguments in not a problem will all ABI I know), but I'm don't
think this is allowed in theory.

On overall, I'm not convinced we should move the code. When the argument
I need to merge these two things otherwise it doesn't compile comes in
a discussion, it usually means there's an architecture issue
somewhere ;-).

 One more thing, Karsten's string-intern API can be used for saving
 file names as they are repeated a
 lot.

This, or storing the list of filenames in struct config_set (more or
less as you did in previous patch), and store pointers to the same
string in each config_hash_entry.

But strdup-ing all filenames seems a bit heavy to me. Even though we're
not talking about performance-critical part of Git, I don't like the
idea of wasting too much ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t5150-request-pull.sh fails on newest master in Debian

2014-07-09 Thread René Scharfe

Am 09.07.2014 03:18, schrieb David Turner:

On Wed, 2014-07-09 at 02:52 +0200, Øyvind A. Holm wrote:

On 3 July 2014 23:55, Øyvind A. Holm su...@sunbase.org wrote:

When compiling newest master (v2.0.1-472-g6f92e5f) on Debian 7.5
(64-bit), t5150-request-pull.sh fails when compiling with

$ make configure
$ ./configure --prefix=/usr/local/varprg/git.master.v2.0.1-472-g6f92e5f
$ make prefix=/usr/local/varprg/git.master.v2.0.1-472-g6f92e5f
$ cd t
$ ./t5150-request-pull.sh


FYI, t5150-request-pull.sh passes all tests now on newest master
(v2.0.1-474-g72c7794) in Debian. There are two new commits on master
since I wrote this, and the commit that makes things work again is
4602f1a (diff-tree: call free_commit_list() instead of duplicating
its code). Reverting this commit brings the failure back.

The whole thing is still a mystery to me, though. I can't see why this
should have anything to do with the use of ./configure --prefix.


The problem only happens when a ref with an allowed wildcard winds up on
a page boundary (with the wildcard before the page boundary).  This
depends intricately on the details of memory allocation, so pretty much
anything could make it come and go.

Does the fix I posted work for you?  If not, let me know and I'll look
into it more.


Sounds fragile overall.  How could a test program look like?  All I can 
think of is a brute force check of all combinations of three characters 
(is that enough?), PAGE_SIZE offsets, three flags, with and without 
.lock appended (and embedded?) against the old implementation, which 
must be quite expensive.


Some callers of check_refname_format() know the length of the string or 
can determine it cheaply because they copy the whole string anyway. 
Would it make sense to do away with the page boundary magic and require 
the callers of the fast version to pass that length?  The tailing bytes 
(up to 15) would have to be loaded carefully, though.  Not sure.


René
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 02/10] replace: add --graft option

2014-07-09 Thread Junio C Hamano
Christian Couder chrisc...@tuxfamily.org writes:

 The usage string for this option is:

 git replace [-f] --graft commit [parent...]

 First we create a new commit that is the same as commit
 except that its parents are [parents...]

 Then we create a replace ref that replace commit with
 the commit we just created.

 With this new option, it should be straightforward to
 convert grafts to replace refs.

Sensible.


 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
  builtin/replace.c | 74 
 ++-
  1 file changed, 73 insertions(+), 1 deletion(-)

 diff --git a/builtin/replace.c b/builtin/replace.c
 index 1bb491d..ad47237 100644
 --- a/builtin/replace.c
 +++ b/builtin/replace.c
 @@ -17,6 +17,7 @@
  static const char * const git_replace_usage[] = {
   N_(git replace [-f] object replacement),
   N_(git replace [-f] --edit object),
 + N_(git replace [-f] --graft commit [parent...]),
   N_(git replace -d object...),
   N_(git replace [--format=format] [-l [pattern]]),
   NULL
 @@ -294,6 +295,66 @@ static int edit_and_replace(const char *object_ref, int 
 force)
   return replace_object_sha1(object_ref, old, replacement, new, force);
  }
  
 +static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 +{
 + struct strbuf new_parents = STRBUF_INIT;
 + const char *parent_start, *parent_end;
 + int i;
 +
 + /* find existing parents */
 + parent_start = buf-buf;
 + parent_start += 46; /* tree  + hex sha1 + \n */
 + parent_end = parent_start;
 +
 + while (starts_with(parent_end, parent ))
 + parent_end += 48; /* parent  + hex sha1 + \n */
 +
 + /* prepare new parents */
 + for (i = 1; i  argc; i++) {

It looks somewhat strange that both replace_parents() and
create_graft() take familiar-looking argc, argv pair, but one
ignores argv[0] and uses the remainder and the other uses argv[0].
Shouldn't this function consume argv[] starting from [0] for
consistency?  You'd obviously need to update the caller to adjust
the arguments it gives to this function.

 +static int create_graft(int argc, const char **argv, int force)
 +{
 + unsigned char old[20], new[20];
 + const char *old_ref = argv[0];
 +...
 +
 + replace_parents(buf, argc, argv);
 +
 + if (write_sha1_file(buf.buf, buf.len, commit_type, new))
 + die(_(could not write replacement commit for: '%s'), old_ref);
 +
 + strbuf_release(buf);
 +
 + if (!hashcmp(old, new))
 + return error(new commit is the same as the old one: '%s', 
 sha1_to_hex(old));

Is this really an error?  It may be a warning-worthy situation for a
user or a script to end up doing a no-op graft, e.g.

git replace --graft HEAD HEAD^

but I wonder if it is more convenient to signal an error (like this
patch does) or just ignore the request and return without adding the
replace ref.

Other than these two points, looks good to me.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] enums: remove trailing ',' after last item in enum

2014-07-09 Thread Junio C Hamano
Ronnie Sahlberg sahlb...@google.com writes:

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
 ---

Looks good; thanks.

  builtin/clean.c | 2 +-
  builtin/tag.c   | 2 +-
  pretty.c| 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/builtin/clean.c b/builtin/clean.c
 index 9a91515..27701d2 100644
 --- a/builtin/clean.c
 +++ b/builtin/clean.c
 @@ -48,7 +48,7 @@ enum color_clean {
   CLEAN_COLOR_PROMPT = 2,
   CLEAN_COLOR_HEADER = 3,
   CLEAN_COLOR_HELP = 4,
 - CLEAN_COLOR_ERROR = 5,
 + CLEAN_COLOR_ERROR = 5
  };
  
  #define MENU_OPTS_SINGLETON  01
 diff --git a/builtin/tag.c b/builtin/tag.c
 index c6e8a71..ef76556 100644
 --- a/builtin/tag.c
 +++ b/builtin/tag.c
 @@ -83,7 +83,7 @@ static int in_commit_list(const struct commit_list *want, 
 struct commit *c)
  enum contains_result {
   CONTAINS_UNKNOWN = -1,
   CONTAINS_NO = 0,
 - CONTAINS_YES = 1,
 + CONTAINS_YES = 1
  };
  
  /*
 diff --git a/pretty.c b/pretty.c
 index 4f51287..924bc61 100644
 --- a/pretty.c
 +++ b/pretty.c
 @@ -274,7 +274,7 @@ static void add_rfc822_quoted(struct strbuf *out, const 
 char *s, int len)
  
  enum rfc2047_type {
   RFC2047_SUBJECT,
 - RFC2047_ADDRESS,
 + RFC2047_ADDRESS
  };
  
  static int is_rfc2047_special(char ch, enum rfc2047_type type)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 My opinion on this:

 * It's low priority. I think the order of priority should be (high to
   low)

   1) Finish and get the current series into pu (we're almost there, it's
  not time to back off and restart something new).

   2) Work on the other series that uses the new API. We don't need to
  change _all_ the uses, but we do need a few tens of them to
  validate the fact that the new API is nice and convenient to use.

   3) Get new actual features for the user (tidy up config files, give
  error messages based on numbers).

   You probably won't finish everything, so just think: if the GSoC ends
   between 1) and 2), how serious is it? if it ends between 2) and 3),
   how serious? If reverse the order, then the risk of having nothing
   finished and mergeable at the end is high. If it happens, the
   community may or may not take over and finish it ...

 * Still, making sure that the (file, line) is doable later without too
   much change is good. So, if indeed, moving all code to another file is
   required, then it may make sense to do it now to avoid code move
   later.

Good thinking.  As long as the code is prepared, it is a good idea
to start small and bite off only we can chew at once, do things
incrementally.

 1. config-hash.c had to be shifted to config.c entirely.

 Why? I guess one reason is the use of struct cf (are there others?), but
 moving just

 config_hash_callback
 config_hash_add_value
 git_configset_add_file

 to config.c should do it. Then, config.c could use config-hash.c.

I am not sure why you guys needed a new file config-hash.c to begin
with, actually.  Besides, hash in its name is an implementation
detail (what it gives us is a way to grab values for configuration
variables from a config set) which we would rather not want to see.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 A brute force approach could be to simply run the config file(s)
 through sort and compare them:

 sort .git/modules/sub1/config expect 
 sort sub1/.git/config actual 
 test_cmp expect actual 

Or git config --list from these files, perhaps?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: move detection doesnt take filename into account

2014-07-09 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Jul 01, 2014 at 10:08:15AM -0700, Junio C Hamano wrote:

 I didn't think it through but my gut feeling is that we could change
 the name similarity score to be the length of the tail part that
 matches (e.g. 1.a to a/2.a that has the same two bytes at the tail
 is a better match than to a/2.b that does not share any tail, and to
 a/1.a that shares the three bytes at the tail is an even better
 match).

 The delta heuristics in pack-objects use pack_name_hash, which claims:

 /*
  * This effectively just creates a sortable number from the
  * last sixteen non-whitespace characters. Last characters
  * count most, so things that end in .c sort together.
  */

 which might be another option (and seems like a superset of the basename
 check, short of basenames that are longer than 16 characters).

Perhaps.

I am however not sure if the code to compute similarity score is as
OK with false positives, i.e. dissimilar names that happen to hash
together getting clumped in a same bin or in close bins, as the
existing callers of pack_name_hash().



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Tanay Abhra
On 7/9/2014 7:49 PM, Matthieu Moy wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 
 My opinion on this:
 
 * It's low priority. I think the order of priority should be (high to
   low)
 
   1) Finish and get the current series into pu (we're almost there, it's
  not time to back off and restart something new).

Noted. I will send the revised series tomorrow ASAP.

 
   2) Work on the other series that uses the new API. We don't need to
  change _all_ the uses, but we do need a few tens of them to
  validate the fact that the new API is nice and convenient to use.
 

Okay, I have updated the series and will send the new one by friday.
Still, I am aiming to rewrite at least half of total calls next weeks
when (1) is in pu.

   3) Get new actual features for the user (tidy up config files, give
  error messages based on numbers).
 
   You probably won't finish everything, so just think: if the GSoC ends
   between 1) and 2), how serious is it? if it ends between 2) and 3),
   how serious? If reverse the order, then the risk of having nothing
   finished and mergeable at the end is high. If it happens, the
   community may or may not take over and finish it ...


Noted, still I want to add that even when GSoC finishes, I won't leave the
work unfinished. I had said that I wanted to continue being part of the Git
community and I mean that. :)

 * Still, making sure that the (file, line) is doable later without too
   much change is good. So, if indeed, moving all code to another file is
   required, then it may make sense to do it now to avoid code move
   later.


Yes, the only problem I see is that the future readers of config.c might
read two versions of the git_config_type helpers and may get confused,
as they have similar names as git_config_*()  git_config_get_*().
That was the reason in the first place that I moved the code into a new file.

 1. config-hash.c had to be shifted to config.c entirely.
 
 Why? I guess one reason is the use of struct cf (are there others?), but

Nope, just for using struct cf. All old API functions raise error by
accessing cf globally for the file name and line number.

 moving just
 
 config_hash_callback
 config_hash_add_value
 git_configset_add_file
 
 to config.c should do it. Then, config.c could use config-hash.c.
 
 But a cleaner way would be to allow the callback to receive the
 config_file struct without accessing it as a global variable (currently,
 we have no way to parse two config files in parallel for example).
 
 In practice, it should be possible to pass a 4th pointer argument to the
 callback, and keep compatibility with 3 arguments callback (having too
 many arguments in not a problem will all ABI I know), but I'm don't
 think this is allowed in theory.
 
 On overall, I'm not convinced we should move the code. When the argument
 I need to merge these two things otherwise it doesn't compile comes in
 a discussion, it usually means there's an architecture issue
 somewhere ;-).


I have to decide on what to do next on moving the contents to config.c or not.
Seeing Junio's comments on the topic seems that he wasn't convinced in the
first place that we needed a new file. What should we do, as I am sending the
revised patch tomorrow? The move will be trivial, just cutting and pasting the
contents. Other approaches you mentioned are also doable but, after a certain
amount of retooling. I am open to any method you think would be best.

Cheers,
Tanay Abhra.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword

2014-07-09 Thread Fabian Ruch
On 07/08/2014 10:45 PM, Junio C Hamano wrote:
 Fabian Ruch baf...@gmail.com writes:
 Fabian Ruch (19):
   rebase -i: Failed reword prints redundant error message
   rebase -i: reword complains about empty commit despite --keep-empty
   rebase -i: reword executes pre-commit hook on interim commit
   rebase -i: Teach do_pick the option --edit
   rebase -i: Implement reword in terms of do_pick
   rebase -i: Stop on root commits with empty log messages
   rebase -i: The replay of root commits is not shown with --verbose
   rebase -i: Root commits are replayed with an unnecessary option
   rebase -i: Commit only once when rewriting picks
   rebase -i: Do not die in do_pick
   rebase -i: Teach do_pick the option --amend
   rebase -i: Teach do_pick the option --file
   rebase -i: Prepare for squash in terms of do_pick --amend
   rebase -i: Implement squash in terms of do_pick
   rebase -i: Explicitly distinguish replay commands and exec tasks
   rebase -i: Parse to-do list command line options
   rebase -i: Teach do_pick the option --reset-author
   rebase -i: Teach do_pick the option --signoff
   rebase -i: Enable options --signoff, --reset-author for pick, reword
 
 After rebase -i:, some begin with lowercase and many begin with
 capital, which makes the short-log output look distracting.

The ones that begin with lower-case letters are the ones that begin with
the command name reword. All first lines are typed in lower case now.

Sorry for the noise.

   Fabian
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] dir: remove PATH_MAX limitation

2014-07-09 Thread Junio C Hamano
Karsten Blees karsten.bl...@gmail.com writes:

 'git status' segfaults if a directory is longer than PATH_MAX, because
 processing .gitignore files in prep_exclude() writes past the end of a
 PATH_MAX-bounded buffer.

 Remove the limitation by using strbuf instead.

 Note: this fix just 'abuses' strbuf as string allocator, len is always 0.
 prep_exclude() can probably be simplified using more strbuf APIs.

In addition to what Jeff already said, I am afraid that this may
make things a lot worse for normal case.  By sizing the strbuf to
absolute minimum as you dig deeper at each level, wouldn't you copy
and reallocate the buffer a lot more, compared to the case where
everything fits in the original buffer?


 Signed-off-by: Karsten Blees bl...@dcon.de
 ---
  dir.c | 35 +++
  dir.h |  4 ++--
  2 files changed, 21 insertions(+), 18 deletions(-)

 diff --git a/dir.c b/dir.c
 index e65888d..8d4d83c 100644
 --- a/dir.c
 +++ b/dir.c
 @@ -798,7 +798,7 @@ static void prep_exclude(struct dir_struct *dir, const 
 char *base, int baselen)
* path being checked. */
   while ((stk = dir-exclude_stack) != NULL) {
   if (stk-baselen = baselen 
 - !strncmp(dir-basebuf, base, stk-baselen))
 + !strncmp(dir-base.buf, base, stk-baselen))
   break;
   el = group-el[dir-exclude_stack-exclude_ix];
   dir-exclude_stack = stk-prev;
 @@ -833,48 +833,50 @@ static void prep_exclude(struct dir_struct *dir, const 
 char *base, int baselen)
   stk-baselen = cp - base;
   stk-exclude_ix = group-nr;
   el = add_exclude_list(dir, EXC_DIRS, NULL);
 - memcpy(dir-basebuf + current, base + current,
 + strbuf_grow(dir-base, stk-baselen);
 + memcpy(dir-base.buf + current, base + current,
  stk-baselen - current);
  
   /* Abort if the directory is excluded */
   if (stk-baselen) {
   int dt = DT_DIR;
 - dir-basebuf[stk-baselen - 1] = 0;
 + dir-base.buf[stk-baselen - 1] = 0;
   dir-exclude = last_exclude_matching_from_lists(dir,
 - dir-basebuf, stk-baselen - 1,
 - dir-basebuf + current, dt);
 - dir-basebuf[stk-baselen - 1] = '/';
 + dir-base.buf, stk-baselen - 1,
 + dir-base.buf + current, dt);
 + dir-base.buf[stk-baselen - 1] = '/';
   if (dir-exclude 
   dir-exclude-flags  EXC_FLAG_NEGATIVE)
   dir-exclude = NULL;
   if (dir-exclude) {
 - dir-basebuf[stk-baselen] = 0;
 + dir-base.buf[stk-baselen] = 0;
   dir-exclude_stack = stk;
   return;
   }
   }
  
 - /* Try to read per-directory file unless path is too long */
 - if (dir-exclude_per_dir 
 - stk-baselen + strlen(dir-exclude_per_dir)  PATH_MAX) {
 - strcpy(dir-basebuf + stk-baselen,
 + /* Try to read per-directory file */
 + if (dir-exclude_per_dir) {
 + strbuf_grow(dir-base, stk-baselen +
 + strlen(dir-exclude_per_dir));
 + strcpy(dir-base.buf + stk-baselen,
   dir-exclude_per_dir);
   /*
 -  * dir-basebuf gets reused by the traversal, but we
 +  * dir-base gets reused by the traversal, but we
* need fname to remain unchanged to ensure the src
* member of each struct exclude correctly
* back-references its source file.  Other invocations
* of add_exclude_list provide stable strings, so we
* strdup() and free() here in the caller.
*/
 - el-src = strdup(dir-basebuf);
 - add_excludes_from_file_to_list(dir-basebuf,
 - dir-basebuf, stk-baselen, el, 1);
 + el-src = strdup(dir-base.buf);
 + add_excludes_from_file_to_list(dir-base.buf,
 + dir-base.buf, stk-baselen, el, 1);
   }
   dir-exclude_stack = stk;
   current = stk-baselen;
   }
 - dir-basebuf[baselen] = '\0';
 + dir-base.buf[baselen] = '\0';
  }
  
  /*
 @@ -1671,4 +1673,5 @@ void clear_directory(struct dir_struct *dir)
   free(stk);
   stk = prev;
   }
 + strbuf_release(dir-base);
  }
 diff --git a/dir.h b/dir.h
 index 

Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Johannes Sixt
Am 08.07.2014 21:34, schrieb Jens Lehmann:
 And Msysgit complains 
 error: fchmod on c:/xxxt/trash 
 directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
  failed: Function not implemented
 
 I'm not sure what this is about, seems to happen during the cp -R of
 the repo under .git/modules into the submodule.

No. It happens because fchmod() is not implemented in our Windows port.

Please see my band-aid patch at
http://thread.gmane.org/gmane.comp.version-control.git/248154/focus=20266
The sub-thread ended inconclusive.

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

2014-07-09 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 * Still, making sure that the (file, line) is doable later without too
   much change is good. So, if indeed, moving all code to another file is
   required, then it may make sense to do it now to avoid code move
   later.

 Good thinking.  As long as the code is prepared, it is a good idea
 to start small and bite off only we can chew at once, do things
 incrementally.

After thinking about it a bit more, I think file, line support
needs to be done not as a mere client of the generic, uncached
git_config() API that we have had for forever, like the current
iteration, but needs to know a bit more about the state the caller
of the callback (namely, git_parse_source()), and we obviously do
not want to expose that machinery to anybody other than the
implementation of the config subsystem (to which the new facility
this GSoC project adds belongs to), so in that sense you have to
have your code in the same config.c file anyway.

It is somewhat dissapointing that this initial implementation was
done as a client of the traditional git_config(), by the way.  I
would have expected that it would be the other way around, in that
the traditional callers of git_config() would behefit automatically
by having the cache layer below it.

But we have to start from somewhere.  Perhaps the round after this
one can rename the exisiting implementation of git_config() to
something else internal to the caching layer and give the existing
callers a compatible interface that is backed by this new caching
layer in a transparent way.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC v2 02/19] rebase -i: reword complains about empty commit despite --keep-empty

2014-07-09 Thread Fabian Ruch
Junio C Hamano writes:
 Fabian Ruch baf...@gmail.com writes:
 Subject: rebase -i: reword complains about empty commit despite --keep-empty
 
 I had to read the title and then the log twice before understanding
 that the above is not change the complaint message (i.e. reword
 complaints spelled incorrectly) but is a description of the current
 behaviour (i.e. the command complains when 'reword' is used on an
 empty commit) that is not accompanied by an evaluation (ok, it
 complains; are you saying it is a good thing or a bad thing?) or an
 action (if it is a bad thing, what are you doing about it?).
 
 Perhaps
 
   rebase -i: allow rewording an empty commit
 
 or something?

I thought ...despite --keep-empty would already imply that reword
complains about empty commit is not supposed to happen, the action
would have been obvious. However, I understand that --keep-empty is
first of all concerned with which commits of $upstream...$orig_head end
up on the initial to-do list and the git-rebase manual page doesn't
mention that it picks commits using the --allow-empty option. It is
simply a necessity of a script not to complain about something it put on
the to-do list itself.

The subject reads now

rebase -i: allow rewording an empty commit without complaints

trying to convey that this is not a new feature but a bug fix.

   Fabian
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Jens Lehmann
Am 09.07.2014 08:14, schrieb Torsten Bögershausen:
 
 There seems to be some other trouble under Mac OS, not yet fully tracked 
 down,
 (may be related to the diff -r)
 Torsten sees failures of this kind under Mac OS:

 diff -r .git/modules/sub1/config sub1/.git/config
 6d5
  worktree = ../../../sub1
 8a8
  worktree = ../../../sub1
 So the config contains the same content, but the worktree setting moved
 to a different line. This seems to be the result of setting core.worktree
 in the test_git_directory_is_unchanged function just before the diff -r,
 but only under Mac OS.

 So I was suspecting diff -r beinng non-portable, but that doesn't seem to be 
 the problem here.
 (But I wouldn't be surprised if there where problems with diff -r on some 
 Unix systems)
 Anyway, checking all the files in the working tree seems to be a good thing 
 to do,
 but that does not necessarily work for .git/config.

I agree, but this case is special. The test asserts that nobody
added, modified or removed *anything* inside the .git directory.
The reason for problem we are seeing here is that I have to
remove the core.worktree setting when moving the git directory
from .git/modules into the submodule work tree. So the test adds
it again to be able to diff it, and this happens in a different
line only on Mac OS as comparing the two core sections shows:

 -
 [core]
 repositoryformatversion = 0
 filemode = true
 bare = false
 logallrefupdates = true
 worktree = ../../../sub1
 ignorecase = true
 precomposeunicode = true
 [remote origin]

vs.

 
 [core]
 repositoryformatversion = 0
 filemode = true
 bare = false
 logallrefupdates = true
 ignorecase = true
 precomposeunicode = true
 worktree = ../../../sub1
 [remote origin]

And now it's clear what happens here: On Mac OS the ignorecase
and precomposeunicode settings are added behind the worktree
line, then re-adding worktree later for the comparison adds it
after these two.

Could you please test the following? It should avoid this kind
of problem by removing the core.worktree setting temporarily
from the original config in .git/modules instead of adding it
temporarily to .git/config:
---8---
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 3584755..98c86e3 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -143,18 +143,18 @@ replace_gitfile_with_git_dir () {
 }

 # Test that the .git directory in the submodule is unchanged (except for the
-# core.worktree setting, which we temporarily restore). Call this function
+# core.worktree setting, which we temporarily remove). Call this function
 # before test_submodule_content as the latter might write the index file
 # leading to false positive index differences.
 test_git_directory_is_unchanged () {
(
-   cd $1 
-   git config core.worktree ../../../$1
+   cd .git/modules/$1 
+   git config --unset core.worktree
) 
diff -r .git/modules/$1 $1/.git 
(
-   cd $1 
-   GIT_WORK_TREE=. git config --unset core.worktree
+   cd .git/modules/$1 
+   git config core.worktree ../../../$1
)
 }


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Junio C Hamano
Johannes Sixt j...@kdbg.org writes:

 Am 08.07.2014 21:34, schrieb Jens Lehmann:
 And Msysgit complains 
 error: fchmod on c:/xxxt/trash 
 directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
  failed: Function not implemented
 
 I'm not sure what this is about, seems to happen during the cp -R of
 the repo under .git/modules into the submodule.

 No. It happens because fchmod() is not implemented in our Windows port.

 Please see my band-aid patch at
 http://thread.gmane.org/gmane.comp.version-control.git/248154/focus=20266
 The sub-thread ended inconclusive.

We need to start somewhere, and a no-op fchmod() in your patch may
be as a good place to start as anything.  At least we would then
keep the old behaviour without introducing any new failure.

An alternative might be to use chmod() after we are done writing to
the config.lock in order to avoid the use of fchmod() altogether,
which I think can replace the existing two callsites of fchmod().
That approach might be a more expedient, but may turn out to be
undesirable in the longer term.

I also wonder if this carry forward the original permission bits
when updating an existing file by first writing the updated contents
into a lockfile and then renaming it after we are done pattern
ought to be done in the lockfile API at commit_lock_file() time (Duy
and Michael Cc'ed for their input, as they have recently touched
lockfile API implementation in their series somewhat), not at the
level of the user of the lockfile API like Eric's patch daa22c6f
(config: preserve config file permissions on edits, 2014-05-06) did
only for the config file.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 I agree, but this case is special. The test asserts that nobody
 added, modified or removed *anything* inside the .git directory.
 The reason for problem we are seeing here is that I have to
 remove the core.worktree setting when moving the git directory
 from .git/modules into the submodule work tree.

Hmph.  Comparing the files with core.worktree removed sounds like a
workaround that knows too much about the implementation detail of
what is being tested.  I am just wondering if core.worktree will
stay forever be the only thing that is special, or there may come
other things (perhaps as a fallout of integrating things like Duy's
multiple-worktree stuff).

But perhaps we cannot do better than this.

 Could you please test the following? It should avoid this kind
 of problem by removing the core.worktree setting temporarily
 from the original config in .git/modules instead of adding it
 temporarily to .git/config:
 ---8---
 diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
 index 3584755..98c86e3 100755
 --- a/t/lib-submodule-update.sh
 +++ b/t/lib-submodule-update.sh
 @@ -143,18 +143,18 @@ replace_gitfile_with_git_dir () {
  }

  # Test that the .git directory in the submodule is unchanged (except for the
 -# core.worktree setting, which we temporarily restore). Call this function
 +# core.worktree setting, which we temporarily remove). Call this function
  # before test_submodule_content as the latter might write the index file
  # leading to false positive index differences.
  test_git_directory_is_unchanged () {
   (
 - cd $1 
 - git config core.worktree ../../../$1
 + cd .git/modules/$1 
 + git config --unset core.worktree
   ) 
   diff -r .git/modules/$1 $1/.git 
   (
 - cd $1 
 - GIT_WORK_TREE=. git config --unset core.worktree
 + cd .git/modules/$1 
 + git config core.worktree ../../../$1
   )
  }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/2] t/Makefile: check helper scripts for non-portable shell commands too

2014-07-09 Thread Jens Lehmann
Currently only the t[0-9][0-9][0-9][0-9]-*.sh scripts are tested for
shell incompatibilities using the check-non-portable-shell.pl script. This
makes it easy to miss non-POSIX constructs added to one of the t/*lib*.sh
helper scripts, as they aren't automatically detected.

Fix that by adding a THELPERS variable containing all shell scripts that
aren't tests and add these to the test-lint-shell-syntax target too.

Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
 t/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/Makefile b/t/Makefile
index 8fd1a72..7fa6692 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -29,6 +29,7 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst 
','\'',$(TEST_RESULTS_DIRECTORY))
 T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
 TSVN = $(sort $(wildcard t91[0-9][0-9]-*.sh))
 TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
+THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))

 all: $(DEFAULT_TEST_TARGET)

@@ -65,7 +66,7 @@ test-lint-executable:
echo 2 non-executable tests: $$bad; exit 1; }

 test-lint-shell-syntax:
-   @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T)
+   @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS)

 aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
-- 
2.0.1.476.gf051ede


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/2] t/Makefile: always test all lint targets when running tests

2014-07-09 Thread Jens Lehmann
Only the two targets test-lint-duplicates and test-lint-executable are
currently executed when running the test target. This was done on purpose
when the TEST_LINT variable was added in 81127d74 to avoid twisted shell
scripting by developers only to avoid false positives that might result
from the rather simple minded tests, e.g. test-lint-shell-syntax. But it
looks like it might be better to include all lint tests to help developers
to detect non portable shell constructs before the patch is sent to the
list and reviewed there.

Change the TEST_LINT variable to run all lint test unless the TEST_LINT
variable is overridden. If we hit false positives more often than helping
developers to avoid non-portable code (or add less accurate or slow tests
later) we could still fall back to exclude them like 81127d74 proposed.

Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
 t/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/Makefile b/t/Makefile
index 7fa6692..43b15e3 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -13,7 +13,7 @@ TAR ?= $(TAR)
 RM ?= rm -f
 PROVE ?= prove
 DEFAULT_TEST_TARGET ?= test
-TEST_LINT ?= test-lint-duplicates test-lint-executable
+TEST_LINT ?= test-lint

 ifdef TEST_OUTPUT_DIRECTORY
 TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
-- 
2.0.1.476.gf051ede


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 0/2] always run all lint targets when running the test suite

2014-07-09 Thread Jens Lehmann
Am 09.07.2014 07:42, schrieb Junio C Hamano:
 On Tue, Jul 8, 2014 at 12:24 PM, Jens Lehmann jens.lehm...@web.de wrote:

 Am 07.07.2014 20:13, schrieb Junio C Hamano:

 So I am not very enthusiastic to see this change myself.

 Ok, I understand we do not want to lightly risk false positives. I
 just noticed that I accidentally forgot to sign off this series, so
 I'd resend just the first patch with a proper SOB, ok?
 
 
 Nah, let's do both and how it plays out. My not being very enthusiastic
 myself does not necessarily mean that it is bad for the project. Maybe
 most people like it and if I cannot bear with it I can always turn it off
 myself for my environment.
 
 I just have a strange feeling that we may be seeing some twisted shell
 script updates and when the author gets asked why it was written in
 such a strange way, the answer might turn out to be just to work around
 the false positive from the test-lint, which I would not want to see.

Me neither. But until then it might well be that the benefit of having
this test on by default outweighs this potential problem. It would have
surely detected my fingers typing echo -n without my brain being alert
enough to catch this portability issue ;-)

This is the updated version with proper SOBs and an updated commit message
for 2/2 which is trying to sum up the considerations raised in this thread.


Jens Lehmann (2):
  t/Makefile: check helper scripts for non-portable shell commands too
  t/Makefile: always test all lint targets when running tests

 t/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.0.1.476.gf051ede


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re: [PATCH v2 3/4] use new config API for worktree configurations of submodules

2014-07-09 Thread Heiko Voigt
On Tue, Jul 08, 2014 at 01:14:20PM -0700, Junio C Hamano wrote:
 Heiko Voigt hvo...@hvoigt.net writes:
 
  diff --git a/builtin/checkout.c b/builtin/checkout.c
  index 07cf555..03ea20d 100644
  --- a/builtin/checkout.c
  +++ b/builtin/checkout.c
  @@ -18,6 +18,7 @@
   #include xdiff-interface.h
   #include ll-merge.h
   #include resolve-undo.h
  +#include submodule-config.h
   #include submodule.h
   #include argv-array.h
   
 
 Hmph.  What is this change about?  
 
 Nobody in checkout.c needs anything new, yet we add a new include?

This is because I moved the parse_submodule_config_option() function
into the submodule-config.c module. This was necessary so all parsed
submodule values are stored in the cache with the null_sha1. We use
static functions from this module to do this and thats thats the reason
for the move. 

  diff --git a/diff.c b/diff.c
  index f72769a..f692a3c 100644
  --- a/diff.c
  +++ b/diff.c
  @@ -13,6 +13,7 @@
   #include utf8.h
   #include userdiff.h
   #include sigchain.h
  +#include submodule-config.h
   #include submodule.h
   #include ll-merge.h
   #include string-list.h
 
 Likewise.

Same as above.

  It is somewhat unclear to me what real change that improves the life
 of end-users this series brings to us.   The test-submodule-config
 test program obviously is new but that does not really count until
 we see real uses.

Do you mean the API improvements? I split this series off from my
recursive fetch series since this infrastructure is also needed by Jens
recursive checkout series.

I am currently working on finishing the recursive fetch series here[1].
So until now we do not have any improvements for the end-user but only
in the API for the developer. With my series it is possible to easily
lookup what configuration for which submodule is in which revision. That
makes is possible to also implement the recursive fetch logic for
renamed submodules[2]. We are also able to decide whether (and from
where) a new submodules repository can possibly be cloned during
recursive fetch.

A clone on recursive fetch for new submodule makes sure we have
everything available, so a recursive checkout later can work without the
need for an extra fetch.

Does that make the improvements in my series clear for you? I would wait
until my recursive fetch series is ready so we have real uses. Since
there are others (namely Jens or submodule support for 'git archive')
that need it I think it makes sense to review and merge this separately
into master so they have a stable API to code against.

Cheers Heiko

[1] https://github.com/hvoigt/git/commits/hv/fetch-submodules-recursive
[2] 
https://github.com/hvoigt/git/commit/975c370856c3b8f96ab0c5a3ed754e3839f4de45
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Eric Wong
Junio C Hamano gits...@pobox.com wrote:
 Johannes Sixt j...@kdbg.org writes:
  Am 08.07.2014 21:34, schrieb Jens Lehmann:
  And Msysgit complains 
  error: fchmod on c:/xxxt/trash 
  directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
   failed: Function not implemented
  
  I'm not sure what this is about, seems to happen during the cp -R of
  the repo under .git/modules into the submodule.
 
  No. It happens because fchmod() is not implemented in our Windows port.
 
  Please see my band-aid patch at
  http://thread.gmane.org/gmane.comp.version-control.git/248154/focus=20266
  The sub-thread ended inconclusive.
 
 We need to start somewhere, and a no-op fchmod() in your patch may
 be as a good place to start as anything.  At least we would then
 keep the old behaviour without introducing any new failure.

Right, this likely makes the most sense for single-user systems or
systesm without a *nix-like permission system.

 An alternative might be to use chmod() after we are done writing to
 the config.lock in order to avoid the use of fchmod() altogether,
 which I think can replace the existing two callsites of fchmod().
 That approach might be a more expedient, but may turn out to be
 undesirable in the longer term.

In that case, we would need to open with mode=0600 to avoid a window
where the file may be world-readable with any data in it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: No fchmod() under msygit - Was: Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Eric Wong
Torsten Bögershausen tbo...@web.de wrote:
 (And why is it   0 and not   0777)

This is to preserve the uncommon sticky/sgid/suid bits.  Probably not
needed, but better to keep as much intact as possible.

 Can we avoid the fchmod()  all together ?

For single-user systems, sure.

For multi-user systems with git-imap-send users and passwords in
$GIT_CONFIG, I suggest not.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


t3200-branch.sh number 102 fails when run under make test

2014-07-09 Thread Keller, Jacob E
Hello,

I recently cloned the master branch of the git repo, and when I ran make
test, it fails on test 102 of the t3200-branch.sh test cases.

not ok 102 - tracking with unexpected .fetch refspec
#
#   rm -rf a b c d 
#   git init a 
#   (
#   cd a 
#   test_commit a
#   ) 
#   git init b 
#   (
#   cd b 
#   test_commit b
#   ) 
#   git init c 
#   (
#   cd c 
#   test_commit c 
#   git remote add a ../a 
#   git remote add b ../b 
#   git fetch --all
#   ) 
#   git init d 
#   (
#   cd d 
#   git remote add c ../c 
#   git config remote.c.fetch 
+refs/remotes/*:refs/remotes/* 
#   git fetch c 
#   git branch --track local/a/master remotes/a/master 
#   test $(git config branch.local/a/master.remote) = c 

#   test $(git config branch.local/a/master.merge) = 
refs/remotes/a/master 
#   git rev-parse --verify a expect 
#   git rev-parse --verify local/a/master actual 
#   test_cmp expect actual
#   )
#
# failed 1 among 102 test(s)
1..102

However, when I run the test file manually it passes. I am currently
running through a verbose output test run to see if I can find more
useful output..

For reference, the commit I am testing against is:

72c779457cd7 (line-log: use commit_list_append() instead of duplicating its 
code)

Thanks,
Jake
N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf

Re: t3200-branch.sh number 102 fails when run under make test

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 08:37:51PM +, Keller, Jacob E wrote:

 I recently cloned the master branch of the git repo, and when I ran make
 test, it fails on test 102 of the t3200-branch.sh test cases.

Just a guess, but try reverting 745224e (refs.c: SSE2 optimizations for
check_refname_component, 2014-06-18).

That commit causes some weird memory accesses that only show up under
certain conditions[1]. There's a possible fix that is not yet applied,
but reverting should be an easy way to test.

-Peff

[1] http://thread.gmane.org/gmane.comp.version-control.git/252881
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t3200-branch.sh number 102 fails when run under make test

2014-07-09 Thread Keller, Jacob E
On Wed, 2014-07-09 at 13:37 -0700, Jacob E Keller wrote:
 Hello,
 
 I recently cloned the master branch of the git repo, and when I ran make
 test, it fails on test 102 of the t3200-branch.sh test cases.
 
 not ok 102 - tracking with unexpected .fetch refspec
 #
 #   rm -rf a b c d 
 #   git init a 
 #   (
 #   cd a 
 #   test_commit a
 #   ) 
 #   git init b 
 #   (
 #   cd b 
 #   test_commit b
 #   ) 
 #   git init c 
 #   (
 #   cd c 
 #   test_commit c 
 #   git remote add a ../a 
 #   git remote add b ../b 
 #   git fetch --all
 #   ) 
 #   git init d 
 #   (
 #   cd d 
 #   git remote add c ../c 
 #   git config remote.c.fetch 
 +refs/remotes/*:refs/remotes/* 
 #   git fetch c 
 #   git branch --track local/a/master remotes/a/master 
 #   test $(git config branch.local/a/master.remote) = 
 c 
 #   test $(git config branch.local/a/master.merge) = 
 refs/remotes/a/master 
 #   git rev-parse --verify a expect 
 #   git rev-parse --verify local/a/master actual 
 #   test_cmp expect actual
 #   )
 #
 # failed 1 among 102 test(s)
 1..102
 
 However, when I run the test file manually it passes. I am currently
 running through a verbose output test run to see if I can find more
 useful output..
 
 For reference, the commit I am testing against is:
 
 72c779457cd7 (line-log: use commit_list_append() instead of duplicating its 
 code)
 
 Thanks,
 Jake

I ran the test wit the GIT_TEST_OPS set to --verbose, and the output I got is:
expecting success: 
rm -rf a b c d 
git init a 
(
cd a 
test_commit a
) 
git init b 
(
cd b 
test_commit b
) 
git init c 
(
cd c 
test_commit c 
git remote add a ../a 
git remote add b ../b 
git fetch --all
) 
git init d 
(
cd d 
git remote add c ../c 
git config remote.c.fetch +refs/remotes/*:refs/remotes/* 
git fetch c 
git branch --track local/a/master remotes/a/master 
test $(git config branch.local/a/master.remote) = c 
test $(git config branch.local/a/master.merge) = 
refs/remotes/a/master 
git rev-parse --verify a expect 
git rev-parse --verify local/a/master actual 
test_cmp expect actual
)

Initialized empty Git repository in /home/jekeller/git/git/t/trash 
directory.t3200-branch/a/.git/
[master (root-commit) ce450c7] a
 Author: A U Thor aut...@example.com
 1 file changed, 1 insertion(+)
 create mode 100644 a.t
Initialized empty Git repository in /home/jekeller/git/git/t/trash 
directory.t3200-branch/b/.git/
[master (root-commit) 19acec0] b
 Author: A U Thor aut...@example.com
 1 file changed, 1 insertion(+)
 create mode 100644 b.t
Initialized empty Git repository in /home/jekeller/git/git/t/trash 
directory.t3200-branch/c/.git/
[master (root-commit) ea1ac38] c
 Author: A U Thor aut...@example.com
 1 file changed, 1 insertion(+)
 create mode 100644 c.t
fatal: Invalid refspec '+refs/heads/*:refs/remotes/b/*'
not ok 102 - tracking with unexpected .fetch refspec
#   
#   rm -rf a b c d 
#   git init a 
#   (
#   cd a 
#   test_commit a
#   ) 
#   git init b 
#   (
#   cd b 
#   test_commit b
#   ) 
#   git init c 
#   (
#   cd c 
#   test_commit c 
#   git remote add a ../a 
#   git remote add b ../b 
#   git fetch --all
#   ) 
#   git init d 
#   (
#   cd d 
#   git remote add c ../c 
#   git config remote.c.fetch 
+refs/remotes/*:refs/remotes/* 
#   git fetch c 
#   git branch --track local/a/master remotes/a/master 
#   test $(git config branch.local/a/master.remote) = c 

#   test $(git config branch.local/a/master.merge) = 
refs/remotes/a/master 
#   git rev-parse --verify a expect 
#   git rev-parse --verify local/a/master actual 
#

Re: Unexpected end of command stream message looks irrelevant when I try to pull a non-existing branch

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 11:37:51AM +0400, Dmitry wrote:

 I'm using Git 1.8.1 and when I run the following command:
 
 git pull origin matser
 
 I get the following output:
 
 fatal: couldn't find remote ref matser
 Unexpected end of command stream
 
 The first line in the output is right on the money but the second one
 looks completely irrelevant - the command is well formed except I
 perhaps mistyped the branch name. It looks like there's some bug that
 prevents the program from just exiting after printing the first line
 and so the second line is being output.

I imagine your origin remote is over http. For some protocols, git
delegates the hard work to a helper program and communicates over a
pipe. In this case, the parent git process detects a problem and dies.
The second message comes from the helper, who is surprised that the
parent has gone away.

Probably the right solution is teaching the parent to properly hang up
the connection with the helper before exiting (alternatively, we could
just silence the helper; that means we would get less output when the
parent really does unexpectedly go away, but that isn't supposed to ever
happen).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: t3200-branch.sh number 102 fails when run under make test

2014-07-09 Thread Keller, Jacob E
On Wed, 2014-07-09 at 16:54 -0400, Jeff King wrote:
 On Wed, Jul 09, 2014 at 08:37:51PM +, Keller, Jacob E wrote:
 
  I recently cloned the master branch of the git repo, and when I ran make
  test, it fails on test 102 of the t3200-branch.sh test cases.
 
 Just a guess, but try reverting 745224e (refs.c: SSE2 optimizations for
 check_refname_component, 2014-06-18).
 
 That commit causes some weird memory accesses that only show up under
 certain conditions[1]. There's a possible fix that is not yet applied,
 but reverting should be an easy way to test.
 
 -Peff
 
 [1] http://thread.gmane.org/gmane.comp.version-control.git/252881
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yes, performing the revert appears to fix the issue. Hopefully the
proposed fix also works.

Thanks,
Jake
N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf

[PATCH] remote-curl: do not complain on EOF from parent git

2014-07-09 Thread Jeff King
The parent git process is supposed to send us an empty line
to indicate that the conversation is over. However, the
parent process may die() if there is a problem with the
operaiton (e.g., we try to fetch a ref that does not exist).
In this case, it produces a useful message, but then
remote-curl _also_ produces an unhelpful message:

  $ git pull origin matser
  fatal: couldn't find remote ref matser
  Unexpected end of command stream

The right way to fix this is to teach the parent git to
always cleanly close the connection to the helper, letting
it know that we are done. Implementing that is rather
clunky, though, as it would involve either replacing die()
operations with returning errors up the stack (until we
disconnect the transport), or adding an atexit handler to
clean up any transport helpers left open.

It's much simpler to just suppress the EOF message in
remote-curl. It was not added to address any real-world
situation in the first place, but rather a we should
probably report unexpected things suggestion[1].

It is the parent git which drives the operation, and whose
exit value actually matters. If the parent dies, then the
helper has no need to complain (except as a debugging aid).
In the off chance that the pipe is closed without the parent
dying, the parent can still notice the non-zero exit code.

[1] http://article.gmane.org/gmane.comp.version-control.git/176036

Reported-by: Dmitry wiped...@yandex.ru
Signed-off-by: Jeff King p...@peff.net
---
The original discussion that led to this code being implemented was due
to us checking the helper's exit code in the first place. However, we
seem to be inconsistent about doing so. I'm not inclined to pursue it
further, though, as these subtle details of the transport helper code
usually turn into a can of worms, and more importantly, I don't think it
hurts anything in the real world. Either the parent git gets the
expected protocol output from the helper or it doesn't, and we report
errors on that. An error from a helper after the operation completes is
not really important to the parent git either way.

 remote-curl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 4493b38..0454ffc 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -971,8 +971,6 @@ int main(int argc, const char **argv)
if (strbuf_getline(buf, stdin, '\n') == EOF) {
if (ferror(stdin))
fprintf(stderr, Error reading command 
stream\n);
-   else
-   fprintf(stderr, Unexpected end of command 
stream\n);
return 1;
}
if (buf.len == 0)
-- 
2.0.0.566.gfe3e6b2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jacob Keller
Add support for configuring default sort ordering for git tags. Command
line option will override this configured value, using the exact same
syntax.

Cc: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Cc: Junio C Hamano gits...@pobox.com
Signed-off-by: Jacob Keller jacob.e.kel...@intel.com
---
 Documentation/config.txt  |  7 +++
 Documentation/git-tag.txt |  3 ++-
 builtin/tag.c | 47 ---
 t/t7004-tag.sh| 21 +
 4 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bdb9662..0152582445b0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2354,6 +2354,13 @@ submodule.name.ignore::
--ignore-submodules option. The 'git submodule' commands are not
affected by this setting.
 
+tag.sort::
+   This variable is used to control the sort ordering of tags when
+   displayed via linkgit:git-tag[5]. The current supported types are
+   refname for lexicographic order (default) or version:refname or
+   v:refname for tag names treated as versions. You may prepend a - to
+   reverse the sort ordering.
+
 tar.umask::
This variable can be used to restrict the permission bits of
tar archive entries.  The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index b424a1bc48bb..b338260f9f63 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -99,7 +99,7 @@ OPTIONS
Sort in a specific order. Supported type is refname
(lexicographic order), version:refname or v:refname (tag
names are treated as versions). Prepend - to reverse sort
-   order.
+   order. See configuration variable tag.sort for more details.
 
 --column[=options]::
 --no-column::
@@ -317,6 +317,7 @@ include::date-formats.txt[]
 SEE ALSO
 
 linkgit:git-check-ref-format[1].
+linkgit:git-config[1].
 
 GIT
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index ef765563388c..e8ada8b716ee 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -18,6 +18,8 @@
 #include sha1-array.h
 #include column.h
 
+static char *configured_tag_sort;
+
 static const char * const git_tag_usage[] = {
N_(git tag [-a|-s|-u key-id] [-f] [-m msg|-F file] tagname 
[head]),
N_(git tag -d tagname...),
@@ -346,9 +348,28 @@ static const char tag_template_nocleanup[] =
Lines starting with '%c' will be kept; you may remove them
 yourself if you want to.\n);
 
+static void set_configured_tag_sort(const char *key)
+{
+   free(configured_tag_sort);
+   configured_tag_sort = xstrdup(key);
+}
+
+static const char *get_configured_tag_sort(void)
+{
+   if (configured_tag_sort)
+   return configured_tag_sort;
+   return refname;
+}
+
 static int git_tag_config(const char *var, const char *value, void *cb)
 {
-   int status = git_gpg_config(var, value, cb);
+   int status;
+
+   if (!strcmp(var, tag.sort)) {
+   set_configured_tag_sort(value);
+   }
+
+   status = git_gpg_config(var, value, cb);
if (status)
return status;
if (starts_with(var, column.))
@@ -519,9 +540,9 @@ static int parse_opt_points_at(const struct option *opt 
__attribute__((unused)),
return 0;
 }
 
-static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
+static int parse_sort_string(const char *arg)
 {
-   int *sort = opt-value;
+   int sort = 0;
int flags = 0;
 
if (*arg == '-') {
@@ -529,16 +550,26 @@ static int parse_opt_sort(const struct option *opt, const 
char *arg, int unset)
arg++;
}
if (starts_with(arg, version:)) {
-   *sort = VERCMP_SORT;
+   sort = VERCMP_SORT;
arg += 8;
} else if (starts_with(arg, v:)) {
-   *sort = VERCMP_SORT;
+   sort = VERCMP_SORT;
arg += 2;
} else
-   *sort = STRCMP_SORT;
+   sort = STRCMP_SORT;
if (strcmp(arg, refname))
die(_(unsupported sort specification %s), arg);
-   *sort |= flags;
+   sort |= flags;
+
+   return sort;
+}
+
+static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
+{
+   int *sort = opt-value;
+
+   *sort = parse_sort_string(arg);
+
return 0;
 }
 
@@ -606,6 +637,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
memset(opt, 0, sizeof(opt));
 
+   sort = parse_sort_string(get_configured_tag_sort());
+
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
if (keyid) {
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e4ab0f5b6419..1e8300f6ed7c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1423,6 +1423,27 @@ EOF
test_cmp expect actual
 '
 
+test_expect_success 'configured lexical 

Re: [PATCH] remote-curl: do not complain on EOF from parent git

2014-07-09 Thread Keller, Jacob E
On Wed, 2014-07-09 at 17:20 -0400, Jeff King wrote:
 The parent git process is supposed to send us an empty line
 to indicate that the conversation is over. However, the
 parent process may die() if there is a problem with the
 operaiton (e.g., we try to fetch a ref that does not exist). 

Nitpick, but you probably meant operation here.

Thanks,
Jake

 In this case, it produces a useful message, but then
 remote-curl _also_ produces an unhelpful message:
 
   $ git pull origin matser
   fatal: couldn't find remote ref matser
   Unexpected end of command stream
 
 The right way to fix this is to teach the parent git to
 always cleanly close the connection to the helper, letting
 it know that we are done. Implementing that is rather
 clunky, though, as it would involve either replacing die()
 operations with returning errors up the stack (until we
 disconnect the transport), or adding an atexit handler to
 clean up any transport helpers left open.
 
 It's much simpler to just suppress the EOF message in
 remote-curl. It was not added to address any real-world
 situation in the first place, but rather a we should
 probably report unexpected things suggestion[1].
 
 It is the parent git which drives the operation, and whose
 exit value actually matters. If the parent dies, then the
 helper has no need to complain (except as a debugging aid).
 In the off chance that the pipe is closed without the parent
 dying, the parent can still notice the non-zero exit code.
 
 [1] http://article.gmane.org/gmane.comp.version-control.git/176036
 
 Reported-by: Dmitry wiped...@yandex.ru
 Signed-off-by: Jeff King p...@peff.net
 ---
 The original discussion that led to this code being implemented was due
 to us checking the helper's exit code in the first place. However, we
 seem to be inconsistent about doing so. I'm not inclined to pursue it
 further, though, as these subtle details of the transport helper code
 usually turn into a can of worms, and more importantly, I don't think it
 hurts anything in the real world. Either the parent git gets the
 expected protocol output from the helper or it doesn't, and we report
 errors on that. An error from a helper after the operation completes is
 not really important to the parent git either way.
 
  remote-curl.c | 2 --
  1 file changed, 2 deletions(-)
 
 diff --git a/remote-curl.c b/remote-curl.c
 index 4493b38..0454ffc 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -971,8 +971,6 @@ int main(int argc, const char **argv)
   if (strbuf_getline(buf, stdin, '\n') == EOF) {
   if (ferror(stdin))
   fprintf(stderr, Error reading command 
 stream\n);
 - else
 - fprintf(stderr, Unexpected end of command 
 stream\n);
   return 1;
   }
   if (buf.len == 0)




Re: [PATCH] remote-curl: do not complain on EOF from parent git

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 09:25:18PM +, Keller, Jacob E wrote:

 On Wed, 2014-07-09 at 17:20 -0400, Jeff King wrote:
  The parent git process is supposed to send us an empty line
  to indicate that the conversation is over. However, the
  parent process may die() if there is a problem with the
  operaiton (e.g., we try to fetch a ref that does not exist). 
 
 Nitpick, but you probably meant operation here.

I did (and I probably meant to turn on spellcheck, too...).

I'll repost in a minute, as I have some follow-on patches. Thanks.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] remote-curl: do not complain on EOF from parent git

2014-07-09 Thread Jeff King
The parent git process is supposed to send us an empty line
to indicate that the conversation is over. However, the
parent process may die() if there is a problem with the
operation (e.g., we try to fetch a ref that does not exist).
In this case, it produces a useful message, but then
remote-curl _also_ produces an unhelpful message:

  $ git pull origin matser
  fatal: couldn't find remote ref matser
  Unexpected end of command stream

The right way to fix this is to teach the parent git to
always cleanly close the connection to the helper, letting
it know that we are done. Implementing that is rather
clunky, though, as it would involve either replacing die()
operations with returning errors up the stack (until we
disconnect the transport), or adding an atexit handler to
clean up any transport helpers left open.

It's much simpler to just suppress the EOF message in
remote-curl. It was not added to address any real-world
situation in the first place, but rather a we should
probably report unexpected things suggestion[1].

It is the parent git which drives the operation, and whose
exit value actually matters. If the parent dies, then the
helper has no need to complain (except as a debugging aid).
In the off chance that the pipe is closed without the parent
dying, it can still notice the non-zero exit code.

[1] http://article.gmane.org/gmane.comp.version-control.git/176036

Signed-off-by: Jeff King p...@peff.net
---
Repost fixing commit message typo (and indicating it as the first of 3
patches).

 remote-curl.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 4493b38..0454ffc 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -971,8 +971,6 @@ int main(int argc, const char **argv)
if (strbuf_getline(buf, stdin, '\n') == EOF) {
if (ferror(stdin))
fprintf(stderr, Error reading command 
stream\n);
-   else
-   fprintf(stderr, Unexpected end of command 
stream\n);
return 1;
}
if (buf.len == 0)
-- 
2.0.0.566.gfe3e6b2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] remote-curl: mark helper-protocol errors more clearly

2014-07-09 Thread Jeff King
When we encounter an error in remote-curl, we generally just
report it to stderr. There is no need for the user to care
that the could not connect to server error was generated
by git-remote-https rather than a function in the parent
git-fetch process.

However, when the error is in the protocol between git and
the helper, it makes sense to clearly identify which side is
complaining. These cases shouldn't ever happen, but when
they do, we can make them less confusing by being more
verbose.

Signed-off-by: Jeff King p...@peff.net
---
 remote-curl.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index a619b64..1f6905d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -949,7 +949,7 @@ int main(int argc, const char **argv)
git_extract_argv0_path(argv[0]);
setup_git_directory_gently(nongit);
if (argc  2) {
-   error(remote needed);
+   error(remote-curl: usage: git remote-curl remote [url]);
return 1;
}
 
@@ -970,14 +970,14 @@ int main(int argc, const char **argv)
do {
if (strbuf_getline(buf, stdin, '\n') == EOF) {
if (ferror(stdin))
-   error(error reading command stream);
+   error(remote-curl: error reading command 
stream from git);
return 1;
}
if (buf.len == 0)
break;
if (starts_with(buf.buf, fetch )) {
if (nongit)
-   die(Fetch attempted without a local repo);
+   die(remote-curl: fetch attempted without a 
local repo);
parse_fetch(buf);
 
} else if (!strcmp(buf.buf, list) || starts_with(buf.buf, 
list )) {
@@ -1014,7 +1014,7 @@ int main(int argc, const char **argv)
printf(\n);
fflush(stdout);
} else {
-   error(unknown command '%s', buf.buf);
+   error(remote-curl: unknown command '%s' from git, 
buf.buf);
return 1;
}
strbuf_reset(buf);
-- 
2.0.0.566.gfe3e6b2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] remote-curl: use error instead of fprintf(stderr)

2014-07-09 Thread Jeff King
We usually prefix our error messages with error: , but
many error messages from remote-curl are simply printed with
fprintf. This can make the output a little harder to read
(especially because such message may be intermingled with
errors from the parent git process).

There is no reason to avoid error(), as we are already
calling it many places (in addition to libgit.a functions
which use it).

While we're adjusting the messages, we can also drop the
capitalization which makes them unlike other git error
messages.

Signed-off-by: Jeff King p...@peff.net
---
I suspect there may be some more improvements we can make (e.g., the
fetch failed below seems like it might be redundant with what git
fetch will print). But hunting them is a lot of work for little gain;
I'd rather wait to see them in practice and fix them on a case by case
basis.

 remote-curl.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 0454ffc..a619b64 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -399,7 +399,7 @@ static curlioerr rpc_ioctl(CURL *handle, int cmd, void 
*clientp)
rpc-pos = 0;
return CURLIOE_OK;
}
-   fprintf(stderr, Unable to rewind rpc post data - try 
increasing http.postBuffer\n);
+   error(unable to rewind rpc post data - try increasing 
http.postBuffer);
return CURLIOE_FAILRESTART;
 
default:
@@ -709,7 +709,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)
free(targets[i]);
free(targets);
 
-   return ret ? error(Fetch failed.) : 0;
+   return ret ? error(fetch failed.) : 0;
 }
 
 static int fetch_git(struct discovery *heads,
@@ -949,7 +949,7 @@ int main(int argc, const char **argv)
git_extract_argv0_path(argv[0]);
setup_git_directory_gently(nongit);
if (argc  2) {
-   fprintf(stderr, Remote needed\n);
+   error(remote needed);
return 1;
}
 
@@ -970,7 +970,7 @@ int main(int argc, const char **argv)
do {
if (strbuf_getline(buf, stdin, '\n') == EOF) {
if (ferror(stdin))
-   fprintf(stderr, Error reading command 
stream\n);
+   error(error reading command stream);
return 1;
}
if (buf.len == 0)
@@ -1014,7 +1014,7 @@ int main(int argc, const char **argv)
printf(\n);
fflush(stdout);
} else {
-   fprintf(stderr, Unknown command '%s'\n, buf.buf);
+   error(unknown command '%s', buf.buf);
return 1;
}
strbuf_reset(buf);
-- 
2.0.0.566.gfe3e6b2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/14] Add submodule test harness

2014-07-09 Thread Junio C Hamano
Eric Wong normalper...@yhbt.net writes:

 Junio C Hamano gits...@pobox.com wrote:
 Johannes Sixt j...@kdbg.org writes:
  Am 08.07.2014 21:34, schrieb Jens Lehmann:
  And Msysgit complains 
  error: fchmod on c:/xxxt/trash 
  directory.t7613-merge-submodule/submodule_update_repo/.git/modules/sub1/config.lock
   failed: Function not implemented
  
  I'm not sure what this is about, seems to happen during the cp -R of
  the repo under .git/modules into the submodule.
 
  No. It happens because fchmod() is not implemented in our Windows port.
 
  Please see my band-aid patch at
  http://thread.gmane.org/gmane.comp.version-control.git/248154/focus=20266
  The sub-thread ended inconclusive.
 
 We need to start somewhere, and a no-op fchmod() in your patch may
 be as a good place to start as anything.  At least we would then
 keep the old behaviour without introducing any new failure.

 Right, this likely makes the most sense for single-user systems or
 systesm without a *nix-like permission system.

 An alternative might be to use chmod() after we are done writing to
 the config.lock in order to avoid the use of fchmod() altogether,
 which I think can replace the existing two callsites of fchmod().
 That approach might be a more expedient, but may turn out to be
 undesirable in the longer term.

 In that case, we would need to open with mode=0600 to avoid a window
 where the file may be world-readable with any data in it.

Yes, of course.

To elaborate what I was alluding to at the end of the message you
are responding to a bit more, if we were to move this grab perms
from existing file (if there is any) and propagate to the new one
into the lockfile API, 

 - in hold_lock_file_for_update(), we would record the permission of
   the original file, if any, to a new field in struct lock_file;
 - open with 0600 or tighter in lock_file(), and

 - either before closing the file use fchmod() or after closing and
   moving the file use chmod() to propagate the permission.

If the original did not exist, we would pass 0666 to open as before
in lock_file() and do not bother chmod/fchmod at the end.

Or something like that, perhaps.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:

 Add support for configuring default sort ordering for git tags. Command
 line option will override this configured value, using the exact same
 syntax.

This makes sense, and was something I was expecting to add once tag and
branch both learned for-each-ref's sorting code. I don't think there
will be any compatibility problems in adding it now, though; the
ultimate interface will be the same (it will just know about more types
of sorting).

 +tag.sort::
 + This variable is used to control the sort ordering of tags when
 + displayed via linkgit:git-tag[5]. The current supported types are
 + refname for lexicographic order (default) or version:refname or
 + v:refname for tag names treated as versions. You may prepend a - to
 + reverse the sort ordering.

Your link to git-tag[5] should be to git-tag[1], I think. The final
number is the manpage section.

I was going to complain that this is duplicating the sort documentation
from git-tag, but I see you actually moved it from the --sort option to
here, and refer back from there to here.

I think I'd prefer it the other way around (and explain this as behave
as if --sort=value had been given). As the sort options grow, it
seems likely that we will grow a new section in the git-tag manpage (and
eventually probably feed that content from an include that works for all
of for-each-ref, branch, and tag).

 +static char *configured_tag_sort;

When dealing with a config option that also has a command-line version,
we usually forgo this separate storage for the configured value.
Instead, we just set sort directly from the config callback (which may
require making it a global so the callback can access it), and then let
the command-line parser overwrite it.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: move detection doesnt take filename into account

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 08:51:07AM -0700, Junio C Hamano wrote:

  The delta heuristics in pack-objects use pack_name_hash, which claims:
 
  /*
   * This effectively just creates a sortable number from the
   * last sixteen non-whitespace characters. Last characters
   * count most, so things that end in .c sort together.
   */
 
  which might be another option (and seems like a superset of the basename
  check, short of basenames that are longer than 16 characters).
 
 Perhaps.
 
 I am however not sure if the code to compute similarity score is as
 OK with false positives, i.e. dissimilar names that happen to hash
 together getting clumped in a same bin or in close bins, as the
 existing callers of pack_name_hash().

I think the hash here does not collide in that way. It really is just
the last sixteen characters shoved into a uint32_t.

But thinking on it more, that is useful to the delta code because it
wants to create a sorted list of items. In the rename code we are doing
pairwise comparisons, so we are more flexible. We can compare whole
basenames, or whole suffixes (so a/foo/bar.c is closer to
b/foo/bar.c than to c/other/bar.c). Or just use a general-purpose
edit-distance function.

The tricky part is that the rename detection seems to take the score as
a binary 0/1 is it the same, but we would want to express more nuance
(i.e., the best match among those that have similar content scores).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/4] use new config API for worktree configurations of submodules

2014-07-09 Thread Junio C Hamano
Heiko Voigt hvo...@hvoigt.net writes:

 On Tue, Jul 08, 2014 at 01:14:20PM -0700, Junio C Hamano wrote:
 Heiko Voigt hvo...@hvoigt.net writes:
 
  diff --git a/builtin/checkout.c b/builtin/checkout.c
  index 07cf555..03ea20d 100644
  --- a/builtin/checkout.c
  +++ b/builtin/checkout.c
  @@ -18,6 +18,7 @@
   #include xdiff-interface.h
   #include ll-merge.h
   #include resolve-undo.h
  +#include submodule-config.h
   #include submodule.h
   #include argv-array.h
   
 
 Hmph.  What is this change about?  
 
 Nobody in checkout.c needs anything new, yet we add a new include?

 This is because I moved the parse_submodule_config_option() function
 into the submodule-config.c module. This was necessary so all parsed
 submodule values are stored in the cache with the null_sha1. We use
 static functions from this module to do this and thats thats the reason
 for the move. 

  diff --git a/diff.c b/diff.c
  index f72769a..f692a3c 100644
  --- a/diff.c
  ...
 Likewise.

 Same as above.

Can there be any caller that include and use submodule-config.h that
does not need anythjing from submodule.h?  Or vice versa?

It just did not look like these two headers describe independent
subsystems but they almost always have to go hand-in-hand.  And if
that is the case, perhaps it is not such a good idea to add it as a
new header.  That was where my question came from.

 Does that make the improvements in my series clear for you? I would wait
 until my recursive fetch series is ready so we have real uses. Since
 there are others (namely Jens or submodule support for 'git archive')
 that need it I think it makes sense to review and merge this separately
 into master so they have a stable API to code against.

Sure.  If we have sufficient amount of client code to judge the
goodness of the API design against, there is no need to wait until
all possible client code becomes ready.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Keller, Jacob E
On Wed, 2014-07-09 at 17:58 -0400, Jeff King wrote:
 On Wed, Jul 09, 2014 at 02:21:00PM -0700, Jacob Keller wrote:
 
  Add support for configuring default sort ordering for git tags. Command
  line option will override this configured value, using the exact same
  syntax.
 
 This makes sense, and was something I was expecting to add once tag and
 branch both learned for-each-ref's sorting code. I don't think there
 will be any compatibility problems in adding it now, though; the
 ultimate interface will be the same (it will just know about more types
 of sorting).
 

I only noticed the sort recently, and I first tried to add an alias like
tag = tag --sort=v:refname but git didn't pick up the alias of the
name already. I use a lot of version-style tags so I wanted this to be
default. I did notice that the format of the sort parameter was actually
pretty complex, so it seemed that more was intended to be added to it.

The only main issue would be the location of the sort-configure code,
but that is actually easy to move so I don't think it's a big deal. The
configuration interface should remain similar.

  +tag.sort::
  +   This variable is used to control the sort ordering of tags when
  +   displayed via linkgit:git-tag[5]. The current supported types are
  +   refname for lexicographic order (default) or version:refname or
  +   v:refname for tag names treated as versions. You may prepend a - to
  +   reverse the sort ordering.
 
 Your link to git-tag[5] should be to git-tag[1], I think. The final
 number is the manpage section.
 

Which I thought was 5 for the commands? Or is it always [1]?

 I was going to complain that this is duplicating the sort documentation
 from git-tag, but I see you actually moved it from the --sort option to
 here, and refer back from there to here.
 

I actually didn't remove anything from git-tag, I only added the
reference to git-config. But I can keep from duplicating the
documentation in there. I like the idea of using an included file
though.

 I think I'd prefer it the other way around (and explain this as behave
 as if --sort=value had been given). As the sort options grow, it
 seems likely that we will grow a new section in the git-tag manpage (and
 eventually probably feed that content from an include that works for all
 of for-each-ref, branch, and tag).

yes, I agree this makes more sense.

 
  +static char *configured_tag_sort;
 
 When dealing with a config option that also has a command-line version,
 we usually forgo this separate storage for the configured value.
 Instead, we just set sort directly from the config callback (which may
 require making it a global so the callback can access it), and then let
 the command-line parser overwrite it.
 

Alright that makes sense. I will send a v2 soon. Thanks for the
comments.

 -Peff

Regards,
Jake


Re: [PATCH] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jeff King
On Wed, Jul 09, 2014 at 10:07:20PM +, Keller, Jacob E wrote:

 I only noticed the sort recently, and I first tried to add an alias like
 tag = tag --sort=v:refname but git didn't pick up the alias of the
 name already.

It was only added recently (v2.0.0). :)

As you noticed, we do not allow aliases of actual git commands. I think
it would be nice to notice and complain rather than silently ignoring
them, though.

 I use a lot of version-style tags so I wanted this to be default. I
 did notice that the format of the sort parameter was actually pretty
 complex, so it seemed that more was intended to be added to it.

Yeah, the complexity is in preparation for becoming more flexible. I
think we'd consider short options for commonly-used sorts, but were
waiting for them to prove their value in the field.

 The only main issue would be the location of the sort-configure code,
 but that is actually easy to move so I don't think it's a big deal. The
 configuration interface should remain similar.

Agreed. It's not a problem moving code around. But once a user-facing
feature is released, we try to keep compatibility with it. So as long as
the config option's format stays the same (or strictly increases in
features), that is fine.

  Your link to git-tag[5] should be to git-tag[1], I think. The final
  number is the manpage section.
 
 Which I thought was 5 for the commands? Or is it always [1]?

Commands are 1. File formats are 5 (so there are some things in section
5 in git). man man has more.

  I was going to complain that this is duplicating the sort documentation
  from git-tag, but I see you actually moved it from the --sort option to
  here, and refer back from there to here.
 
 I actually didn't remove anything from git-tag, I only added the
 reference to git-config. But I can keep from duplicating the
 documentation in there. I like the idea of using an included file
 though.

Oh, sorry, I just misread the patch. Then I'll fall back to my original
complaint. :) We should not duplicate, but just refer back to
git-tag(1).

 Alright that makes sense. I will send a v2 soon. Thanks for the
 comments.

You're welcome. Thanks for working on git (and welcome to the list, I
think).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: move detection doesnt take filename into account

2014-07-09 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I think the hash here does not collide in that way. It really is just
 the last sixteen characters shoved into a uint32_t.

All bytes overlap with their adjacent byte because they are shifted
by only 2 bits, not 8 bits, when a new byte is brought in.  We can
say that the topmost two bits of the result must have come from the
last character, but other than these, there are more than one input
byte for each bit position to be set/unset by, so two names that human
would not consider similar would be given the same hash, no?

That is useful for delta code because the code only needs that
similar things are grouped together, it does not mind things that
are not similar is also mixed to a group, as the end result is
primarily determined by similarity of the actual contents, not
pathnames.

What is under topic in this discussion is the other way around; we
know two paths have contents of the same similarity to the third one
and want to tie-break these two using how similar their pathnames
are to the third one.  
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] makefile: add ability to run specific test files

2014-07-09 Thread Jacob Keller
Running a specific test file manually does not obtain the exact
environment setup by the Makefile. Add ability to run any of the tests
in the t/ directory so that a user can more quickly debug a failing
test. Otherwise, the entire test suite needs to be run, which can take a
vary long time.

Signed-off-by: Jacob Keller jacob.e.kel...@intel.com
---
 Makefile | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 07ea1058379a..86bdc4ed1ee9 100644
--- a/Makefile
+++ b/Makefile
@@ -2262,13 +2262,18 @@ export TEST_NO_MALLOC_CHECK
 
 ### Testing rules
 
+T = $(sort $(wildcard t/t[0-9][0-9][0-9][0-9]-*.sh))
+
+$(T):
+   $(MAKE) -C t $(notdir $@)
+
 test: all
$(MAKE) -C t/ all
 
 perf: all
$(MAKE) -C t/perf/ all
 
-.PHONY: test perf
+.PHONY: test perf $(T)
 
 test-ctype$X: ctype.o
 
-- 
2.0.1.475.g9b8d714

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] tag: support configuring --sort via .gitconfig

2014-07-09 Thread Jacob Keller
Add support for configuring default sort ordering for git tags. Command
line option will override this configured value, using the exact same
syntax.

Cc: Jeff King p...@peff.net
Signed-off-by: Jacob Keller jacob.e.kel...@intel.com
---
Repost with changes suggested by Peff. These include fixing documentation to
remove duplicatation, and having git-config reference git-tag. Directly assign
a tag_sort global that we use for the config and option variable. 

 Documentation/config.txt  |  6 +
 Documentation/git-tag.txt |  1 +
 builtin/tag.c | 60 ++-
 t/t7004-tag.sh| 21 +
 4 files changed, 67 insertions(+), 21 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bdb9662..ad8e75fed988 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2354,6 +2354,12 @@ submodule.name.ignore::
--ignore-submodules option. The 'git submodule' commands are not
affected by this setting.
 
+tag.sort::
+   This variable is used to control the sort ordering of tags. It is
+   interpreted precisely the same way as --sort=value. If --sort is
+   given on the command line it will override the selection chosen in the
+   configuration. See linkgit:git-tag[1] for more details.
+
 tar.umask::
This variable can be used to restrict the permission bits of
tar archive entries.  The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index b424a1bc48bb..2d246725aeb5 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -317,6 +317,7 @@ include::date-formats.txt[]
 SEE ALSO
 
 linkgit:git-check-ref-format[1].
+linkgit:git-config[1].
 
 GIT
 ---
diff --git a/builtin/tag.c b/builtin/tag.c
index ef765563388c..a679e32db866 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -18,6 +18,8 @@
 #include sha1-array.h
 #include column.h
 
+static int tag_sort = 0;
+
 static const char * const git_tag_usage[] = {
N_(git tag [-a|-s|-u key-id] [-f] [-m msg|-F file] tagname 
[head]),
N_(git tag -d tagname...),
@@ -346,9 +348,39 @@ static const char tag_template_nocleanup[] =
Lines starting with '%c' will be kept; you may remove them
 yourself if you want to.\n);
 
+static int parse_sort_string(const char *arg)
+{
+   int sort = 0;
+   int flags = 0;
+
+   if (*arg == '-') {
+   flags |= REVERSE_SORT;
+   arg++;
+   }
+   if (starts_with(arg, version:)) {
+   sort = VERCMP_SORT;
+   arg += 8;
+   } else if (starts_with(arg, v:)) {
+   sort = VERCMP_SORT;
+   arg += 2;
+   } else
+   sort = STRCMP_SORT;
+   if (strcmp(arg, refname))
+   die(_(unsupported sort specification %s), arg);
+   sort |= flags;
+
+   return sort;
+}
+
 static int git_tag_config(const char *var, const char *value, void *cb)
 {
-   int status = git_gpg_config(var, value, cb);
+   int status;
+
+   if (!strcmp(var, tag.sort)) {
+   tag_sort = parse_sort_string(value);
+   }
+
+   status = git_gpg_config(var, value, cb);
if (status)
return status;
if (starts_with(var, column.))
@@ -522,23 +554,9 @@ static int parse_opt_points_at(const struct option *opt 
__attribute__((unused)),
 static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
 {
int *sort = opt-value;
-   int flags = 0;
 
-   if (*arg == '-') {
-   flags |= REVERSE_SORT;
-   arg++;
-   }
-   if (starts_with(arg, version:)) {
-   *sort = VERCMP_SORT;
-   arg += 8;
-   } else if (starts_with(arg, v:)) {
-   *sort = VERCMP_SORT;
-   arg += 2;
-   } else
-   *sort = STRCMP_SORT;
-   if (strcmp(arg, refname))
-   die(_(unsupported sort specification %s), arg);
-   *sort |= flags;
+   *sort = parse_sort_string(arg);
+
return 0;
 }
 
@@ -552,7 +570,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
-   int cmdmode = 0, sort = 0;
+   int cmdmode = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
@@ -578,7 +596,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT__FORCE(force, N_(replace the tag if exists)),
OPT_COLUMN(0, column, colopts, N_(show tag list in 
columns)),
{
-   OPTION_CALLBACK, 0, sort, sort, N_(type), N_(sort 
tags),
+   OPTION_CALLBACK, 0, sort, tag_sort, N_(type), 
N_(sort tags),

Re: [PATCH] makefile: add ability to run specific test files

2014-07-09 Thread Junio C Hamano
Jacob Keller jacob.e.kel...@intel.com writes:

 Running a specific test file manually does not obtain the exact
 environment setup by the Makefile.

What kind of things are missing, exactly?  Perhaps that is something
you need to fix, instead of mucking with the top-level Makefile.

I recall last time when I did a patch like this I was told to look
into make -C t ;-)  What is different this round?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >