Re: [PATCH v3 1/4] replace: add --graft option
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
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
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
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
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
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
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
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
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
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