Re: [PATCH v3 1/4] replace: add --graft option

2014-06-08 Thread Christian Couder
On Fri, Jun 6, 2014 at 5:44 PM, Christian Couder
christian.cou...@gmail.com wrote:

 /* find existing parents */
 strbuf_addstr(buf, commit-buffer);

Unfortunately, it looks like the above will not work if the commit-buffer
contains an embedded NUL. I wonder if it is a real problem or 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


Re: [PATCH] completion: Handle '!f() { ... }; f' aliases

2014-06-08 Thread Eric Sunshine
On Sat, Jun 7, 2014 at 10:10 AM, Steffen Prohaska proha...@zib.de wrote:
 '!f() { ... }; f' is a recommended pattern to declare more complex
 aliases (see git wiki [1]).  This commit teaches the completion to
 handle them.

 When determining which completion to use for an alias, the opening brace
 is now ignored in order to continue the search for a git command inside
 the function body.  For example, the alias '!f() { git commit ... }' now
 triggers commit completion.  Previously, the search stopped on '{', and
 the completion tried it to determine how to complete, which obviously
 was useless.

 Furthermore, the null command ':' is now skipped, so that it can be used
 as a workaround to declare the desired completion style.  For example,
 the alias '!f() { : git commit ; if ...  ' now triggers commit
 completion.

 [1] https://git.wiki.kernel.org/index.php/Aliases

 Signed-off-by: Steffen Prohaska proha...@zib.de
 ---
 diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
 index 2d4beb5..ea48681 100755
 --- a/t/t9902-completion.sh
 +++ b/t/t9902-completion.sh
 @@ -550,6 +550,26 @@ test_expect_success 'complete files' '
 test_completion git add mom momified
  '

 +test_expect_success 'completion uses cmd completion for alias !f() { 
 VAR=val git cmd ... }' '
 +   git config alias.co !f() { VAR=val git checkout ... ; } f 

test_config would be an appropriate replacement for git config +
git config --unset.

 +   test_completion git co m -\EOF 
 +   master Z
 +   mybranch Z
 +   mytag Z
 +   EOF
 +   git config --unset alias.co
 +'
 +
 +test_expect_success 'completion used cmd completion for alias !f() { : git 
 cmd ; ... }' '
 +   git config alias.co !f() { : git checkout ; if ... } f 

Ditto.

 +   test_completion git co m -\EOF 
 +   master Z
 +   mybranch Z
 +   mytag Z
 +   EOF
 +   git config --unset alias.co
 +'
 +
  test_expect_failure 'complete with tilde expansion' '
 git init tmp  cd tmp 
 test_when_finished cd ..  rm -rf tmp 
 --
 2.0.0.244.g4e8e734
--
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 1/2] refs.c: write updates to packed refs when a transaction has more than one ref

2014-06-08 Thread Eric Sunshine
On Thu, Jun 5, 2014 at 7:26 PM, Ronnie Sahlberg sahlb...@google.com wrote:
 When we are updating more than one single ref, i.e. not a commit, then
 write the updated refs directly to the packed refs file instead of writing
 them as loose refs.

 Change clone to use a transaction instead of using the pacekd refs api.

s/pacekd/packed/

 Signed-off-by: Ronnie Sahlberg sahlb...@google.com
--
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/5] implement submodule config cache for lookup of submodule names

2014-06-08 Thread Eric Sunshine
On Thu, Jun 5, 2014 at 2:07 AM, Heiko Voigt hvo...@hvoigt.net wrote:
 This submodule configuration cache allows us to lazily read .gitmodules
 configurations by commit into a runtime cache which can then be used to
 easily lookup values from it. Currently only the values for path or name
 are stored but it can be extended for any value needed.

 [...]

 Signed-off-by: Heiko Voigt hvo...@hvoigt.net
 ---
 diff --git a/t/t7410-submodule-config.sh b/t/t7410-submodule-config.sh
 new file mode 100755
 index 000..ea453c5
 --- /dev/null
 +++ b/t/t7410-submodule-config.sh
 @@ -0,0 +1,73 @@
 +#!/bin/sh
 +#
 +# Copyright (c) 2014 Heiko Voigt
 +#
 +
 +test_description='Test submodules config cache infrastructure
 +
 +This test verifies that parsing .gitmodules configuration directly
 +from the database works.
 +'
 +
 +TEST_NO_CREATE_REPO=1
 +. ./test-lib.sh
 +
 +test_expect_success 'submodule config cache setup' '
 +   mkdir submodule 
 +   (cd submodule 
 +   git init

Broken -chain.

 +   echo a a 
 +   git add . 
 +   git commit -ma
 +   ) 
 +   mkdir super 
 +   (cd super 
 +   git init 
 +   git submodule add ../submodule 
 +   git submodule add ../submodule a 
 +   git commit -m add as submodule and as a 
 +   git mv a b 
 +   git commit -m move a to b
 +   )
 +'
 +
 +cat super/expect EOF
 +Submodule name: 'a' for path 'a'
 +Submodule name: 'a' for path 'b'
 +Submodule name: 'submodule' for path 'submodule'
 +Submodule name: 'submodule' for path 'submodule'
 +EOF
 +
 +test_expect_success 'test parsing of submodule config' '
 +   (cd super 
 +   test-submodule-config \
 +   HEAD^ a \
 +   HEAD b \
 +   HEAD^ submodule \
 +   HEAD submodule \
 +   actual 
 +   test_cmp expect actual
 +   )
 +'
 +
 +cat super/expect_error EOF
 +Submodule name: 'a' for path 'b'
 +Submodule name: 'submodule' for path 'submodule'
 +EOF
 +
 +test_expect_success 'error in one submodule config lets continue' '
 +   (cd super 
 +   cp .gitmodules .gitmodules.bak 
 +   echo   value = \ .gitmodules 
 +   git add .gitmodules 
 +   mv .gitmodules.bak .gitmodules 
 +   git commit -m add error 
 +   test-submodule-config \
 +   HEAD b \
 +   HEAD submodule \
 +   actual 
 +   test_cmp expect_error actual
 +   )
 +'
 +
 +test_done
--
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] Fix t0001: test git init when run via an alias

2014-06-08 Thread Nguyễn Thái Ngọc Duy
Commit 4ad8332 (t0001: test git init when run via an alias -
2010-11-26) noted breakages when running init via alias. The problem
is for alias to be used, $GIT_DIR must be searched, but 'init' and
'clone' are not happy with that. So we start a new process like an
external command, with clean environment in this case. Env variables
that are set by command line (e.g. git --git-dir=.. ) are kept.

This should also fix autocorrecting a command typo to init because
it's the same problem: aliases are read, then init is unhappy with
$GIT_DIR already set up because of that.

Reminded-by: David Turner dtur...@twopensource.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git.c   | 52 
 t/t0001-init.sh |  4 ++--
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/git.c b/git.c
index 7780572..d1e49da 100644
--- a/git.c
+++ b/git.c
@@ -20,6 +20,42 @@ const char git_more_info_string[] =
 
 static struct startup_info git_startup_info;
 static int use_pager = -1;
+static char orig_cwd[PATH_MAX];
+static const char *env_names[] = {
+   GIT_DIR_ENVIRONMENT,
+   GIT_WORK_TREE_ENVIRONMENT,
+   GIT_IMPLICIT_WORK_TREE_ENVIRONMENT,
+   GIT_PREFIX_ENVIRONMENT
+};
+static char *orig_env[4];
+static int saved_environment;
+
+static void save_env(void)
+{
+   int i;
+   if (saved_environment)
+   return;
+   saved_environment = 1;
+   getcwd(orig_cwd, sizeof(orig_cwd));
+   for (i = 0; i  ARRAY_SIZE(env_names); i++) {
+   orig_env[i] = getenv(env_names[i]);
+   if (orig_env[i])
+   orig_env[i] = xstrdup(orig_env[i]);
+   }
+}
+
+static void restore_env(void)
+{
+   int i;
+   if (*orig_cwd  chdir(orig_cwd))
+   die_errno(could not move to %s, orig_cwd);
+   for (i = 0; i  ARRAY_SIZE(env_names); i++) {
+   if (orig_env[i])
+   setenv(env_names[i], orig_env[i], 1);
+   else
+   unsetenv(env_names[i]);
+   }
+}
 
 static void commit_pager_choice(void) {
switch (use_pager) {
@@ -272,6 +308,7 @@ static int handle_alias(int *argcp, const char ***argv)
  * RUN_SETUP for reading from the configuration file.
  */
 #define NEED_WORK_TREE (13)
+#define NO_SETUP   (14)
 
 struct cmd_struct {
const char *cmd;
@@ -352,7 +389,7 @@ static struct cmd_struct commands[] = {
{ cherry, cmd_cherry, RUN_SETUP },
{ cherry-pick, cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
{ clean, cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-   { clone, cmd_clone },
+   { clone, cmd_clone, NO_SETUP },
{ column, cmd_column, RUN_SETUP_GENTLY },
{ commit, cmd_commit, RUN_SETUP | NEED_WORK_TREE },
{ commit-tree, cmd_commit_tree, RUN_SETUP },
@@ -378,8 +415,8 @@ static struct cmd_struct commands[] = {
{ hash-object, cmd_hash_object },
{ help, cmd_help },
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
-   { init, cmd_init_db },
-   { init-db, cmd_init_db },
+   { init, cmd_init_db, NO_SETUP },
+   { init-db, cmd_init_db, NO_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
@@ -484,6 +521,10 @@ static void handle_builtin(int argc, const char **argv)
struct cmd_struct *p = commands+i;
if (strcmp(p-cmd, cmd))
continue;
+   if (saved_environment  (p-option  NO_SETUP)) {
+   restore_env();
+   break;
+   }
exit(run_builtin(p, argc, argv));
}
 }
@@ -539,7 +580,10 @@ static int run_argv(int *argcp, const char ***argv)
 * of overriding git log with git show by having
 * alias.log = show
 */
-   if (done_alias || !handle_alias(argcp, argv))
+   if (done_alias)
+   break;
+   save_env();
+   if (!handle_alias(argcp, argv))
break;
done_alias = 1;
}
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 2f30203..e62c0ff 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -56,7 +56,7 @@ test_expect_success 'plain through aliased command, outside 
any git repo' '
check_config plain-aliased/.git false unset
 '
 
-test_expect_failure 'plain nested through aliased command' '
+test_expect_success 'plain nested through aliased command' '
(
git init plain-ancestor-aliased 
cd plain-ancestor-aliased 
@@ -68,7 +68,7 @@ test_expect_failure 'plain nested through aliased command' '
check_config plain-ancestor-aliased/plain-nested/.git false unset
 '
 
-test_expect_failure 'plain nested in bare through aliased command' '
+test_expect_success 'plain 

Re: [PATCH v5] Add an explicit GIT_DIR to the list of excludes

2014-06-08 Thread Duy Nguyen
On Thu, Jun 5, 2014 at 3:15 AM, Pasha Bolokhov pasha.bolok...@gmail.com wrote:
 +   /* only add it if GIT_DIR does not end with '.git' or '/.git' */
 +   if (len  4 || strcmp(n_git + len - 4, .git) ||
 +   (len  4  n_git[len - 5] != '/')) {

Hmm.. should we exclude foobar.git as well?
-- 
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


Re: [PATCH v3 1/4] replace: add --graft option

2014-06-08 Thread Jeff King
On Sun, Jun 08, 2014 at 08:49:45AM +0200, Christian Couder wrote:

 On Fri, Jun 6, 2014 at 5:44 PM, Christian Couder
 christian.cou...@gmail.com wrote:
 
  /* find existing parents */
  strbuf_addstr(buf, commit-buffer);
 
 Unfortunately, it looks like the above will not work if the commit-buffer
 contains an embedded NUL. I wonder if it is a real problem or not.

I ran into a similar problem recently[1] and have been pondering
solutions to know the size of commit-buffer. What I've been come up
with is:

  1. Look up the object size via sha1_object_info. Besides being
 inefficient (which probably does not matter for you here, but might
 for using commit-buffer in a traversal), it strikes me as
 inelegant; is it possible for commit-buffer to ever disagree in
 size with the results of sha1_object_info, and if so, what happens?

  2. Add an extra member len to struct commit. This is simple, but
 bloats struct commit, which may have a performance impact for
 things like rev-list, where the field will be unused.

  3. Store the length of objects as a size_t, exactly sizeof(size_t)
 bytes before the object buffer. Provide a macro:

   #define OBJECT_SIZE(buf) (((size_t *)(buf))[-1])

 to access it. Most callers can just use the buffer as-is, but
 anybody who calls free() would need to be adjusted to use a special
 object_free.

  4. Keep a static commit_slab that points to the length for each parsed
 commit. We pay the same memory cost as (2), but as it's not part of
 the struct, the cache effects are minimized.

-Peff

[1] http://article.gmane.org/gmane.comp.version-control.git/250480
--
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 v3 1/4] replace: add --graft option

2014-06-08 Thread Jeff King
On Sun, Jun 08, 2014 at 07:23:33AM -0400, Jeff King wrote:

   4. Keep a static commit_slab that points to the length for each parsed
  commit. We pay the same memory cost as (2), but as it's not part of
  the struct, the cache effects are minimized.

I think I favor this solution, which would look something like this:

-- 8 --
Subject: [PATCH] commit: add slab for commit buffer size

We store the commit object buffer for later reuse as
commit-buffer. However, since we store only a pointer, we
must treat the result as a NUL-terminated string. This is
generally OK for pretty-printing, but could be a problem for
other uses.

Adding a len member to struct commit would solve this,
but at the cost of bloating the struct even for callers who
do not care about the size or buffer (e.g., traversals like
rev-list or merge-base). Instead, let's use a commit_slab so
that the memory is used only when save_commit_buffer is in
effect (and even then, it should have less cache impact on
most uses of struct commit).

Signed-off-by: Jeff King p...@peff.net
---
I think it would make sense to actually take this one step further,
though, and move commit-buffer into the slab, as well. That has two
advantages:

  1. It further decreases the size of struct commit for callers who do
 not use save_commit_buffer.

  2. It ensures that no new callers crop up who set commit-buffer but
 to not save the size in the slab (you can see in the patch below
 that I had to modify builtin/blame.c, which (ab)uses
 commit-buffer).

It would be more disruptive to existing callers, but I think the end
result would be pretty clean. The API would be something like:

  /* attach buffer to commit */
  set_commit_buffer(struct commit *, void *buf, unsigned long size);

  /* get buffer, either from slab cache or by calling read_sha1_file */
  void *get_commit_buffer(struct commit *, unsigned long *size);

  /* free() an allocated buffer from above, noop for cached buffer */
  void unused_commit_buffer(struct commit *, void *buf);

  /* drop saved commit buffer to free memory */
  void free_commit_buffer(struct commit *);

The get function would serve the existing callers in pretty.c, as well
as the one I'm adding elsewhere in show_signature. And it should work as
a drop-in read_sha1_file/free replacement for you here.

 builtin/blame.c |  2 +-
 commit.c| 13 -
 commit.h|  1 +
 object.c|  2 +-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index a52a279..1945ea4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
ident, ident, path,
(!contents_from ? path :
 (!strcmp(contents_from, -) ? standard input : 
contents_from)));
-   commit-buffer = strbuf_detach(msg, NULL);
+   set_commit_buffer(commit, strbuf_detach(msg, NULL), msg.len);
 
if (!contents_from || strcmp(-, contents_from)) {
struct stat st;
diff --git a/commit.c b/commit.c
index f479331..71143ed 100644
--- a/commit.c
+++ b/commit.c
@@ -302,6 +302,17 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
return 0;
 }
 
+define_commit_slab(commit_size_slab, unsigned long);
+static struct commit_size_slab commit_size;
+
+void set_commit_buffer(struct commit *item, void *buffer, unsigned long size)
+{
+   if (!commit_size.stride)
+   init_commit_size_slab(commit_size);
+   *commit_size_slab_at(commit_size, item) = size;
+   item-buffer = buffer;
+}
+
 int parse_commit(struct commit *item)
 {
enum object_type type;
@@ -324,7 +335,7 @@ int parse_commit(struct commit *item)
}
ret = parse_commit_buffer(item, buffer, size);
if (save_commit_buffer  !ret) {
-   item-buffer = buffer;
+   set_commit_buffer(item, buffer, size);
return 0;
}
free(buffer);
diff --git a/commit.h b/commit.h
index a9f177b..7704ab2 100644
--- a/commit.h
+++ b/commit.h
@@ -48,6 +48,7 @@ struct commit *lookup_commit_reference_by_name(const char 
*name);
 struct commit *lookup_commit_or_die(const unsigned char *sha1, const char 
*ref_name);
 
 int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long 
size);
+void set_commit_buffer(struct commit *item, void *buffer, unsigned long size);
 int parse_commit(struct commit *item);
 void parse_commit_or_die(struct commit *item);
 
diff --git a/object.c b/object.c
index 57a0890..c1c6a24 100644
--- a/object.c
+++ b/object.c
@@ -198,7 +198,7 @@ struct object *parse_object_buffer(const unsigned char 
*sha1, enum object_type t
if (parse_commit_buffer(commit, buffer, size))
return NULL;
if (!commit-buffer) {
-   commit-buffer = 

Re: [PATCH v3 1/4] replace: add --graft option

2014-06-08 Thread Jeff King
On Sun, Jun 08, 2014 at 08:04:39AM -0400, Jeff King wrote:

 diff --git a/builtin/blame.c b/builtin/blame.c
 index a52a279..1945ea4 100644
 --- a/builtin/blame.c
 +++ b/builtin/blame.c
 @@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct 
 diff_options *opt,
   ident, ident, path,
   (!contents_from ? path :
(!strcmp(contents_from, -) ? standard input : 
 contents_from)));
 - commit-buffer = strbuf_detach(msg, NULL);
 + set_commit_buffer(commit, strbuf_detach(msg, NULL), msg.len);

Side note: this is wrong, as the fake commit created by blame here does
not have its index field set. This is a bug waiting to happen the
first time somebody uses a slab in builtin/blame.c. It looks like
merge-recursive does the same thing in make_virtual_commit.

We probably want to provide a function to allocate a commit, including
the index, and use it consistently.

I'll try to work up a series doing that, and the fuller slab API I
mentioned in the previous message, but probably not until tomorrow.

-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] api-strbuf.txt minor typos

2014-06-08 Thread Jeremiah Mahler
Fixed some minor typos in api-strbuf.txt: 'A' instead of 'An', 'have'
instead of 'has', a overlong line, and 'another' instead of 'an other'.

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 Documentation/technical/api-strbuf.txt | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/technical/api-strbuf.txt 
b/Documentation/technical/api-strbuf.txt
index 077a709..f9c06a7 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -7,10 +7,10 @@ use the mem* functions than a str* one (memchr vs. strchr 
e.g.).
 Though, one has to be careful about the fact that str* functions often
 stop on NULs and that strbufs may have embedded NULs.
 
-An strbuf is NUL terminated for convenience, but no function in the
+A strbuf is NUL terminated for convenience, but no function in the
 strbuf API actually relies on the string being free of NULs.
 
-strbufs has some invariants that are very important to keep in mind:
+strbufs have some invariants that are very important to keep in mind:
 
 . The `buf` member is never NULL, so it can be used in any usual C
 string operations safely. strbuf's _have_ to be initialized either by
@@ -56,8 +56,8 @@ Data structures
 * `struct strbuf`
 
 This is the string buffer structure. The `len` member can be used to
-determine the current length of the string, and `buf` member provides access to
-the string itself.
+determine the current length of the string, and `buf` member provides
+access to the string itself.
 
 Functions
 -
@@ -202,7 +202,7 @@ strbuf_addstr(sb, immediate string);
 
 `strbuf_addbuf`::
 
-   Copy the contents of an other buffer at the end of the current one.
+   Copy the contents of another buffer at the end of the current one.
 
 `strbuf_adddup`::
 
-- 
2.0.0.574.gc6f278f

--
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