[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 --- 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
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
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 --- 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) { -
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 > 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 v5] Add an explicit GIT_DIR to the list of excludes
On Thu, Jun 5, 2014 at 3:15 AM, Pasha Bolokhov 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
[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 Signed-off-by: Nguyễn Thái Ngọc Duy --- 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 (1<<3) +#define NO_SETUP (1<<4) 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' ' +tes
Re: [PATCH 2/5] implement submodule config cache for lookup of submodule names
On Thu, Jun 5, 2014 at 2:07 AM, Heiko Voigt 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 > --- > 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 < +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 < +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
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 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 -- 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 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 > --- > 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 completion for alias !f() { > VAR=val git ... }' ' > + 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 completion for alias !f() { : git > ; ... }' ' > + 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