Re: [RFC/PATCH v2 1/2] config: Add hashtable for config parsing retrieval
In addition to the valuable review comments by Torsten and Peff, find
more below...
On Mon, Jun 2, 2014 at 10:47 AM, Tanay Abhra tanay...@gmail.com wrote:
Add a hash table to cache all key-value pairs read from config files
(repo specific .git/config, user wide ~/.gitconfig and the global
/etc/gitconfig). Add two external functions `git_config_get_string` and
`git_config_get_string_multi` for querying in a non-callback manner from the
hash table.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
diff --git a/config.c b/config.c
index a30cb5c..23c9403 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,8 @@
#include exec_cmd.h
#include strbuf.h
#include quote.h
+#include hashmap.h
+#include string-list.h
struct config_source {
struct config_source *prev;
@@ -37,6 +39,112 @@ static struct config_source *cf;
static int zlib_compression_seen;
+static struct hashmap config_cache;
+
+struct config_cache_entry {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list *value_list;
Same question as in my v1 review [1]: Why is this 'struct string_list
*' rather than just 'struct string_list'?
[1]:
http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html#a7611860
+};
+
+static int hashmap_is_init = 0;
+
+static int config_cache_entry_cmp_icase(const struct config_cache_entry *e1,
+const struct config_cache_entry *e2, const
char* key)
+{
+ return strcasecmp(e1-key, key ? key : e2-key);
Are you planning on taking advantage of this extra complexity (the
'key ? key : e2-key' stuff)? At present, the lone caller _always_
provides the 'key', so 'e2-key' is never used. If not needed,
dropping it will simplify things for future readers of the code. (But,
see below for more commentary...)
+}
+
+static void config_cache_init(void)
+{
+ hashmap_init(config_cache,
(hashmap_cmp_fn)config_cache_entry_cmp_icase, 0);
+}
+
+static void config_cache_free(void)
+{
+ struct config_cache_entry* entry;
Style: struct config_cache_entry *entry;
+ struct hashmap_iter iter;
+ hashmap_iter_init(config_cache, iter);
+ while (entry = hashmap_iter_next(iter)) {
+ free(entry-key);
+ string_list_clear(entry-value_list, 0);
+ }
+ hashmap_free(config_cache, 1);
+}
config_cache_free() is never called.
+static struct config_cache_entry *config_cache_find_entry(const char *key)
+{
+ struct hashmap_entry k;
+ hashmap_entry_init(k, strihash(key));
+ return hashmap_get(config_cache, k, key);
This feels very dangerous. You've declared
config_cache_entry_cmp_icase() as taking a 'struct config_cache_entry
*' as its second argument, however, you are passing a 'struct
hashmap_entry *'. While it may work correctly in this special case as
you've written it, where config_cache_entry_cmp_icase() never accesses
any members of its second argument beyond 'ent', it is not
transparent. A future programmer may trust the type of the incoming
second argument and change config_cache_entry_cmp_icase() to access
elements beyond 'ent' without realizing that those elements aren't
actually present.
It seems safer to code this as in the example section of
Documentation/api-hashmap.txt:
struct config_cache_entry k;
hashmap_entry_init(k, strihash(key));
k.key = key; /* maybe cast-away const here */
return hashmap_get(config_cache, k, NULL);
And modify config_cache_entry_cmp_icase() to always access its second
argument and treat the third as unused.
+}
+
+static struct string_list *config_cache_get_value(const char *key)
+{
+ struct config_cache_entry *e = config_cache_find_entry(key);
+ return e ? e-value_list : NULL;
+}
+
+
+static void config_cache_set_value(const char *key, const char *value)
+{
+ struct config_cache_entry *e;
+
+ e = config_cache_find_entry(key);
+ if (!e) {
+ e = malloc(sizeof(*e));
+ hashmap_entry_init(e, strihash(key));
+ e-key=xstrdup(key);
+ e-value_list = xcalloc(sizeof(struct string_list), 1);
+ e-value_list-strdup_strings = 1;
+ string_list_append(e-value_list, value);
+ hashmap_add(config_cache, e);
+ } else {
+ if (!unsorted_string_list_has_string(e-value_list, value))
+ string_list_append(e-value_list, value);
+ }
Also mentioned in [1]: As there is only a single 'if' statement within
this 'else' clause, you could turn this into an 'else if' and drop one
level of indentation. (Although, as Peff mentioned in his review, you
probably don't want to be folding out duplicate values anyhow, so this
'if' would go away.)
+}
+
+static void config_cache_remove_value(const char *key, const char *value)
+{
+ struct config_cache_entry
[PATCH/RFC v1 4/5] fast-import.c: cleanup using strbuf_set operations
Simplified cases where a strbuf_reset was immediately followed by a
strbuf_add using the new strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
fast-import.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index e8ec34d..c23935c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2741,8 +2741,7 @@ static void parse_new_commit(void)
hashcpy(b-branch_tree.versions[0].sha1,
b-branch_tree.versions[1].sha1);
- strbuf_reset(new_data);
- strbuf_addf(new_data, tree %s\n,
+ strbuf_setf(new_data, tree %s\n,
sha1_to_hex(b-branch_tree.versions[1].sha1));
if (!is_null_sha1(b-sha1))
strbuf_addf(new_data, parent %s\n, sha1_to_hex(b-sha1));
@@ -2829,9 +2828,7 @@ static void parse_new_tag(void)
parse_data(msg, 0, NULL);
/* build the tag object */
- strbuf_reset(new_data);
-
- strbuf_addf(new_data,
+ strbuf_setf(new_data,
object %s\n
type %s\n
tag %s\n,
@@ -2898,8 +2895,7 @@ static void cat_blob(struct object_entry *oe, unsigned
char sha1[20])
* Output based on batch_one_object() from cat-file.c.
*/
if (type = 0) {
- strbuf_reset(line);
- strbuf_addf(line, %s missing\n, sha1_to_hex(sha1));
+ strbuf_setf(line, %s missing\n, sha1_to_hex(sha1));
cat_blob_write(line.buf, line.len);
strbuf_release(line);
free(buf);
@@ -2910,8 +2906,7 @@ static void cat_blob(struct object_entry *oe, unsigned
char sha1[20])
if (type != OBJ_BLOB)
die(Object %s is a %s but a blob was expected.,
sha1_to_hex(sha1), typename(type));
- strbuf_reset(line);
- strbuf_addf(line, %s %s %lu\n, sha1_to_hex(sha1),
+ strbuf_setf(line, %s %s %lu\n, sha1_to_hex(sha1),
typename(type), size);
cat_blob_write(line.buf, line.len);
strbuf_release(line);
@@ -3034,14 +3029,12 @@ static void print_ls(int mode, const unsigned char
*sha1, const char *path)
if (!mode) {
/* missing SP path LF */
- strbuf_reset(line);
- strbuf_addstr(line, missing );
+ strbuf_setstr(line, missing );
quote_c_style(path, line, NULL, 0);
strbuf_addch(line, '\n');
} else {
/* mode SP type SP object_name TAB path LF */
- strbuf_reset(line);
- strbuf_addf(line, %06o %s %s\t,
+ strbuf_setf(line, %06o %s %s\t,
mode ~NO_DELTA, type, sha1_to_hex(sha1));
quote_c_style(path, line, NULL, 0);
strbuf_addch(line, '\n');
--
2.0.0.573.ged771ce.dirty
--
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/RFC v1 1/5] add strbuf_set operations
Currently, the data in a strbuf is modified using add operations. To
set the buffer to some data a reset must be performed before an add.
strbuf_reset(buf);
strbuf_add(buf, cb.buf.buf, cb.buf.len);
And this is a common sequence of operations with 70 occurrences found in
the current source code. This includes all the different variations
(add, addf, addstr, addbuf, addch).
FILES=`find ./ -name '*.c'`
CNT=$(pcregrep -M strbuf_reset.*\n.*strbuf_add $FILES | wc -l)
CNT=$(echo $CNT / 2 | bc)
echo $CNT
70
These patches add strbuf_set operations which allow this common sequence
to be performed in one line instead of two.
strbuf_set(buf, cb.buf.buf, cb.buf.len);
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
strbuf.c | 21 +
strbuf.h | 14 ++
2 files changed, 35 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index ac62982..9d64b00 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t
len,
strbuf_setlen(sb, sb-len + dlen - len);
}
+void strbuf_set(struct strbuf *sb, const void *data, size_t len)
+{
+ strbuf_reset(sb);
+ strbuf_add(sb, data, len);
+}
+
+void strbuf_setf(struct strbuf *sb, const char *fmt, ...)
+{
+ va_list ap;
+ strbuf_reset(sb);
+ va_start(ap, fmt);
+ strbuf_vaddf(sb, fmt, ap);
+ va_end(ap);
+}
+
+void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)
+{
+ strbuf_reset(sb);
+ strbuf_add(sb, sb2-buf, sb2-len);
+}
+
void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
{
strbuf_splice(sb, pos, 0, data, len);
diff --git a/strbuf.h b/strbuf.h
index e9ad03e..b339f08 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -101,6 +101,20 @@ static inline struct strbuf **strbuf_split(const struct
strbuf *sb,
*/
extern void strbuf_list_free(struct strbuf **);
+/*- set buffer to data -*/
+
+extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);
+
+static inline void strbuf_setstr(struct strbuf *sb, const char *s)
+{
+ strbuf_set(sb, s, strlen(s));
+}
+
+__attribute__((format (printf,2,3)))
+extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);
+
+extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);
+
/*- add data in your buffer -*/
static inline void strbuf_addch(struct strbuf *sb, int c)
{
--
2.0.0.573.ged771ce.dirty
--
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/RFC v1 3/5] sha1_name.c: cleanup using strbuf_set operations
Simplified cases where a strbuf_reset was immediately followed by a
strbuf_add using the new strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
sha1_name.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 2b6322f..f88b66c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -920,8 +920,7 @@ static int grab_nth_branch_switch(unsigned char *osha1,
unsigned char *nsha1,
return 0;
if (--(cb-remaining) == 0) {
len = target - match;
- strbuf_reset(cb-buf);
- strbuf_add(cb-buf, match, len);
+ strbuf_set(cb-buf, match, len);
return 1; /* we are done */
}
return 0;
@@ -957,8 +956,7 @@ static int interpret_nth_prior_checkout(const char *name,
int namelen,
retval = 0;
if (0 for_each_reflog_ent_reverse(HEAD, grab_nth_branch_switch,
cb)) {
- strbuf_reset(buf);
- strbuf_add(buf, cb.buf.buf, cb.buf.len);
+ strbuf_set(buf, cb.buf.buf, cb.buf.len);
retval = brace - name + 1;
}
@@ -1025,8 +1023,7 @@ static int interpret_empty_at(const char *name, int
namelen, int len, struct str
if (next != name + 1)
return -1;
- strbuf_reset(buf);
- strbuf_add(buf, HEAD, 4);
+ strbuf_set(buf, HEAD, 4);
return 1;
}
@@ -1044,8 +1041,7 @@ static int reinterpret(const char *name, int namelen, int
len, struct strbuf *bu
strbuf_setlen(buf, used);
return len;
}
- strbuf_reset(buf);
- strbuf_addbuf(buf, tmp);
+ strbuf_setbuf(buf, tmp);
strbuf_release(tmp);
/* tweak for size of {-N} versus expanded ref name */
return ret - used + len;
@@ -1054,8 +1050,7 @@ static int reinterpret(const char *name, int namelen, int
len, struct strbuf *bu
static void set_shortened_ref(struct strbuf *buf, const char *ref)
{
char *s = shorten_unambiguous_ref(ref, 0);
- strbuf_reset(buf);
- strbuf_addstr(buf, s);
+ strbuf_setstr(buf, s);
free(s);
}
--
2.0.0.573.ged771ce.dirty
--
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/RFC v1 5/5] builtin/remote.c: cleanup using strbuf_set operations
Simplified cases where a strbuf_reset was immediately followed by a
strbuf_add using the new strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
builtin/remote.c | 51 +--
1 file changed, 17 insertions(+), 34 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index c9b67ff..b059852 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -193,8 +193,7 @@ static int add(int argc, const char **argv)
return 1;
if (!mirror || mirror MIRROR_FETCH) {
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.fetch, name);
+ strbuf_setf(buf, remote.%s.fetch, name);
if (track.nr == 0)
string_list_append(track, *);
for (i = 0; i track.nr; i++) {
@@ -205,15 +204,13 @@ static int add(int argc, const char **argv)
}
if (mirror MIRROR_PUSH) {
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.mirror, name);
+ strbuf_setf(buf, remote.%s.mirror, name);
if (git_config_set(buf.buf, true))
return 1;
}
if (fetch_tags != TAGS_DEFAULT) {
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.tagopt, name);
+ strbuf_setf(buf, remote.%s.tagopt, name);
if (git_config_set(buf.buf,
fetch_tags == TAGS_SET ? --tags : --no-tags))
return 1;
@@ -223,11 +220,9 @@ static int add(int argc, const char **argv)
return 1;
if (master) {
- strbuf_reset(buf);
- strbuf_addf(buf, refs/remotes/%s/HEAD, name);
+ strbuf_setf(buf, refs/remotes/%s/HEAD, name);
- strbuf_reset(buf2);
- strbuf_addf(buf2, refs/remotes/%s/%s, name, master);
+ strbuf_setf(buf2, refs/remotes/%s/%s, name, master);
if (create_symref(buf.buf, buf2.buf, remote add))
return error(_(Could not setup master '%s'), master);
@@ -589,14 +584,12 @@ static int migrate_file(struct remote *remote)
if (git_config_set_multivar(buf.buf, remote-url[i], ^$, 0))
return error(_(Could not append '%s' to '%s'),
remote-url[i], buf.buf);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.push, remote-name);
+ strbuf_setf(buf, remote.%s.push, remote-name);
for (i = 0; i remote-push_refspec_nr; i++)
if (git_config_set_multivar(buf.buf, remote-push_refspec[i],
^$, 0))
return error(_(Could not append '%s' to '%s'),
remote-push_refspec[i], buf.buf);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.fetch, remote-name);
+ strbuf_setf(buf, remote.%s.fetch, remote-name);
for (i = 0; i remote-fetch_refspec_nr; i++)
if (git_config_set_multivar(buf.buf, remote-fetch_refspec[i],
^$, 0))
return error(_(Could not append '%s' to '%s'),
@@ -644,23 +637,20 @@ static int mv(int argc, const char **argv)
if (!valid_fetch_refspec(buf.buf))
die(_('%s' is not a valid remote name), rename.new);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s, rename.old);
+ strbuf_setf(buf, remote.%s, rename.old);
strbuf_addf(buf2, remote.%s, rename.new);
if (git_config_rename_section(buf.buf, buf2.buf) 1)
return error(_(Could not rename config section '%s' to '%s'),
buf.buf, buf2.buf);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.fetch, rename.new);
+ strbuf_setf(buf, remote.%s.fetch, rename.new);
if (git_config_set_multivar(buf.buf, NULL, NULL, 1))
return error(_(Could not remove config section '%s'),
buf.buf);
strbuf_addf(old_remote_context, :refs/remotes/%s/, rename.old);
for (i = 0; i oldremote-fetch_refspec_nr; i++) {
char *ptr;
- strbuf_reset(buf2);
- strbuf_addstr(buf2, oldremote-fetch_refspec[i]);
+ strbuf_setstr(buf2, oldremote-fetch_refspec[i]);
ptr = strstr(buf2.buf, old_remote_context.buf);
if (ptr) {
refspec_updated = 1;
@@ -683,8 +673,7 @@ static int mv(int argc, const char **argv)
struct string_list_item *item = branch_list.items + i;
struct branch_info *info = item-util;
if (info-remote_name !strcmp(info-remote_name,
rename.old)) {
- strbuf_reset(buf);
- strbuf_addf(buf, branch.%s.remote, item-string);
+ strbuf_setf(buf, branch.%s.remote, item-string);
if (git_config_set(buf.buf,
[PATCH/RFC v1 2/5] add strbuf_set operations documentation
Add documentation of the strbuf_set operations to technical/api-strbuf.txt. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- Documentation/technical/api-strbuf.txt | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index 077a709..ab430d9 100644 --- a/Documentation/technical/api-strbuf.txt +++ b/Documentation/technical/api-strbuf.txt @@ -149,6 +149,24 @@ Functions than zero if the first buffer is found, respectively, to be less than, to match, or be greater than the second buffer. +* Setting the buffer + +`strbuf_set`:: + +Set the buffer to some data up to a given length. + +`strbuf_setstr`:: + + Set the buffer to a NUL-terminated string. + +`strbuf_setf`:: + + Set the buffer to a formatted string. + +`strbuf_setbuf`:: + + Set the current buffer to the contents of some other buffer. + * Adding data to the buffer NOTE: All of the functions in this section will grow the buffer as necessary. -- 2.0.0.573.ged771ce.dirty -- 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/RFC v1 0/5] add strbuf_set operations
Currently, the data in a strbuf is modified using add operations. To set the buffer to some data a reset must be performed before an add. strbuf_reset(buf); strbuf_add(buf, cb.buf.buf, cb.buf.len); And this is a common sequence of operations with 70 occurrences found in the current source code. This includes all the different variations (add, addf, addstr, addbuf, addch). FILES=`find ./ -name '*.c'` CNT=$(pcregrep -M strbuf_reset.*\n.*strbuf_add $FILES | wc -l) CNT=$(echo $CNT / 2 | bc) echo $CNT 70 These patches add strbuf_set operations which allow this common sequence to be performed in one line instead of two. strbuf_set(buf, cb.buf.buf, cb.buf.len); Only the first few files have been converted in this preliminary patch set. Jeremiah Mahler (5): add strbuf_set operations add strbuf_set operations documentation sha1_name.c: cleanup using strbuf_set operations fast-import.c: cleanup using strbuf_set operations builtin/remote.c: cleanup using strbuf_set operations Documentation/technical/api-strbuf.txt | 18 builtin/remote.c | 51 -- fast-import.c | 19 - sha1_name.c| 15 -- strbuf.c | 21 ++ strbuf.h | 14 ++ 6 files changed, 81 insertions(+), 57 deletions(-) -- 2.0.0.573.ged771ce.dirty -- 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 v1 2/5] add strbuf_set operations documentation
On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler jmmah...@gmail.com wrote: Add documentation of the strbuf_set operations to technical/api-strbuf.txt. Since this patch is concise and so closely related to patch 1/5, it probably should be squashed into that one. More below. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- Documentation/technical/api-strbuf.txt | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index 077a709..ab430d9 100644 --- a/Documentation/technical/api-strbuf.txt +++ b/Documentation/technical/api-strbuf.txt @@ -149,6 +149,24 @@ Functions than zero if the first buffer is found, respectively, to be less than, to match, or be greater than the second buffer. +* Setting the buffer + +`strbuf_set`:: + +Set the buffer to some data up to a given length. I personally find this slightly ambiguous. Upon reading it, the first question that pops into my mind is whether or not the existing strbuf content is replaced (even though set should imply that it is). I wonder if it would make sense to rewrite as: Set the buffer to [...], replacing the old content of the buffer. Alternately: Replace the buffer content with [...]. Ditto for the others. +`strbuf_setstr`:: + + Set the buffer to a NUL-terminated string. + +`strbuf_setf`:: + + Set the buffer to a formatted string. + +`strbuf_setbuf`:: + + Set the current buffer to the contents of some other buffer. + * Adding data to the buffer NOTE: All of the functions in this section will grow the buffer as necessary. -- 2.0.0.573.ged771ce.dirty -- 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 v1 4/5] fast-import.c: cleanup using strbuf_set operations
On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler jmmah...@gmail.com wrote:
Subject: fast-import.c: cleanup using strbuf_set operations
This might read more clearly if written:
fast-import: simplify via strbuf_set()
Simplified cases where a strbuf_reset was immediately followed by a
strbuf_add using the new strbuf_set operations.
On this project, commit messages are written in imperative mood:
Simplify cases where ... is immediately ...
More below.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
fast-import.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index e8ec34d..c23935c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2741,8 +2741,7 @@ static void parse_new_commit(void)
hashcpy(b-branch_tree.versions[0].sha1,
b-branch_tree.versions[1].sha1);
- strbuf_reset(new_data);
- strbuf_addf(new_data, tree %s\n,
+ strbuf_setf(new_data, tree %s\n,
sha1_to_hex(b-branch_tree.versions[1].sha1));
if (!is_null_sha1(b-sha1))
strbuf_addf(new_data, parent %s\n, sha1_to_hex(b-sha1));
Unlike the cases in patches 3/5 and 5/5 where the strbuf is used or
returned immediately following the strbuf_set() call, I am not
convinced that this change is an improvement. This code has the
general form:
strbuf_reset(...);
strbuf_add(...);
if (condition)
strbuf_add(...);
strbuf_add(...);
in which it is clear that the string is being built piecemeal, and
it's easy for a programmer to insert, remove, or re-order strbuf_add()
calls.
Replacing the first two lines with strbuf_set() somewhat obscures the
fact that the string is going to be built up piecemeal. Plus, the
change makes it more difficult to insert, remove, or re-order the
strbuf_add() invocations.
This isn't a strong objection, but the benefit of the change seems
minimal or non-existent.
Ditto for several remaining cases in this patch.
@@ -2829,9 +2828,7 @@ static void parse_new_tag(void)
parse_data(msg, 0, NULL);
/* build the tag object */
- strbuf_reset(new_data);
-
- strbuf_addf(new_data,
+ strbuf_setf(new_data,
object %s\n
type %s\n
tag %s\n,
@@ -2898,8 +2895,7 @@ static void cat_blob(struct object_entry *oe, unsigned
char sha1[20])
* Output based on batch_one_object() from cat-file.c.
*/
if (type = 0) {
- strbuf_reset(line);
- strbuf_addf(line, %s missing\n, sha1_to_hex(sha1));
+ strbuf_setf(line, %s missing\n, sha1_to_hex(sha1));
cat_blob_write(line.buf, line.len);
strbuf_release(line);
free(buf);
@@ -2910,8 +2906,7 @@ static void cat_blob(struct object_entry *oe, unsigned
char sha1[20])
if (type != OBJ_BLOB)
die(Object %s is a %s but a blob was expected.,
sha1_to_hex(sha1), typename(type));
- strbuf_reset(line);
- strbuf_addf(line, %s %s %lu\n, sha1_to_hex(sha1),
+ strbuf_setf(line, %s %s %lu\n, sha1_to_hex(sha1),
typename(type), size);
cat_blob_write(line.buf, line.len);
strbuf_release(line);
@@ -3034,14 +3029,12 @@ static void print_ls(int mode, const unsigned char
*sha1, const char *path)
if (!mode) {
/* missing SP path LF */
- strbuf_reset(line);
- strbuf_addstr(line, missing );
+ strbuf_setstr(line, missing );
quote_c_style(path, line, NULL, 0);
strbuf_addch(line, '\n');
} else {
/* mode SP type SP object_name TAB path LF */
- strbuf_reset(line);
- strbuf_addf(line, %06o %s %s\t,
+ strbuf_setf(line, %06o %s %s\t,
mode ~NO_DELTA, type, sha1_to_hex(sha1));
quote_c_style(path, line, NULL, 0);
strbuf_addch(line, '\n');
--
2.0.0.573.ged771ce.dirty
--
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
filmware
answer call but after cannot end, bcoz no effect light and sensitive.my phone
is voyager dg300.but root is
cwm.N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
Re: [PATCH/RFC v1 0/5] add strbuf_set operations
On Mon, Jun 9, 2014 at 3:36 PM, Jeremiah Mahler jmmah...@gmail.com wrote: Currently, the data in a strbuf is modified using add operations. To set the buffer to some data a reset must be performed before an add. strbuf_reset(buf); strbuf_add(buf, cb.buf.buf, cb.buf.len); And this is a common sequence of operations with 70 occurrences found in the current source code. This includes all the different variations (add, addf, addstr, addbuf, addch). FILES=`find ./ -name '*.c'` CNT=$(pcregrep -M strbuf_reset.*\n.*strbuf_add $FILES | wc -l) Hmm.. I wonder if git-grep could do this.. There's pcre support but I never tried. CNT=$(echo $CNT / 2 | bc) echo $CNT 70 The change in this series looks nice. There's another pattern, save strbuf length, then strbuf_setlen() at the beginning or the end of a loop. But I think it's less often. -- 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
Kedves felhasználók e-mailben;
Kedves felhasználók e-mailben; Túllépte 23432 box set Web Service / Admin, és akkor nem lesz probléma a küldő és fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva linkre, és töltse ki az adatokat, hogy ellenőrizze a számla Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző jelölőnégyzetet. http://mailupdattwre322.jigsy.com/ Figyelem! Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha frissíteni? számla frissül három napon belül Értesítés a számla véglegesen be kell zárni. Tisztelettel, rendszergazda -- 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
Kedves felhasználók e-mailben;
Kedves felhasználók e-mailben; Túllépte 23432 box set Web Service / Admin, és akkor nem lesz probléma a küldő és fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva linkre, és töltse ki az adatokat, hogy ellenőrizze a számla Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző jelölőnégyzetet. http://mailupdattwre322.jigsy.com/ Figyelem! Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha frissíteni? számla frissül három napon belül Értesítés a számla véglegesen be kell zárni. Tisztelettel, rendszergazda -- 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 reset --hard with staged changes
Hi all, Someone pointed out on the Git for human beings Google group (https://groups.google.com/d/topic/git-users/27_FxIV_100/discussion) that using git-reset's hard mode when having staged untracked files simply deletes them from the working dir. Since git-reset specifically doesn't touch untracked files, one could expect having staged untracked files reset to their previous untracked state rather than being deleted. Could this be a bug or a missing feature? Or if it isn't, can someone explain what we got wrong? Cheers -- Pierre-François CLEMENT Application developer at Upcast Social -- 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 v1] Git config cache special querying api utilizing the cache
Hi, I am taking this patch out of RFC. [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 my first patch 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. I have run the tests and debug the code using custom functions and it works fine. [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 Cheers, Tanay Abhra. Tanay Abhra (1): config: Add hashtable for config parsing retrival Documentation/technical/api-config.txt | 18 + cache.h| 2 + config.c | 122 + 3 files changed, 142 insertions(+) -- 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 v1] config: Add hashtable for config parsing retrival
Add a hash table to cache all key-value pairs read from config files
(repo specific .git/config, user wide ~/.gitconfig and the global
/etc/gitconfig). Add two external functions `git_config_get_string` and
`git_config_get_string_multi` for querying in a non-callback manner from the
hash table.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-config.txt | 18 +
cache.h| 2 +
config.c | 122 +
3 files changed, 142 insertions(+)
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index 230b3a0..5b6e376 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -77,6 +77,24 @@ 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_string`
+and `git_config_get_string_multi`. They both take a single parameter,
+
+- a key string in canonical flat form for which the corresponding values
+ will be retrieved and returned.
+
+They both read values from an internal cache generated previously from
+reading the config files. `git_config_get_string` returns the value with
+the highest priority(i.e. for the same variable value in the repo config
+will be preferred over value in user wide config).
+
+`git_config_get_string_multi` returns a `string_list` containing all the
+values for that particular key, sorted in order of increasing priority.
+
Value Parsing Helpers
-
diff --git a/cache.h b/cache.h
index 107ac61..2038150 100644
--- a/cache.h
+++ b/cache.h
@@ -1271,6 +1271,8 @@ extern int check_repository_format_version(const char
*var, const char *value, v
extern int git_env_bool(const char *, int);
extern int git_config_system(void);
extern int config_error_nonbool(const char *);
+extern const char *git_config_get_string(const char *);
+extern const struct string_list *git_config_get_string_multi(const char *);
#if defined(__GNUC__) ! defined(__clang__)
#define config_error_nonbool(s) (config_error_nonbool(s), -1)
#endif
diff --git a/config.c b/config.c
index a30cb5c..6f6b04e 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,8 @@
#include exec_cmd.h
#include strbuf.h
#include quote.h
+#include hashmap.h
+#include string-list.h
struct config_source {
struct config_source *prev;
@@ -37,6 +39,120 @@ static struct config_source *cf;
static int zlib_compression_seen;
+struct config_cache_entry {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list *value_list;
+};
+
+static int hashmap_is_init;
+
+static int config_cache_set_value(const char *key, const char *value);
+
+static int config_cache_entry_cmp_case(const struct config_cache_entry *e1,
+const struct config_cache_entry *e2, const
void *unused)
+{
+ return strcmp(e1-key, e2-key);
+}
+
+static void config_cache_init(struct hashmap *config_cache)
+{
+ hashmap_init(config_cache, (hashmap_cmp_fn)
config_cache_entry_cmp_case, 0);
+}
+
+static int config_cache_callback(const char *key, const char *value, void
*unused)
+{
+ config_cache_set_value(key, value);
+ return 0;
+}
+
+static struct hashmap *get_config_cache(void)
+{
+ static struct hashmap config_cache;
+ if (!hashmap_is_init) {
+ config_cache_init(config_cache);
+ hashmap_is_init = 1;
+ git_config(config_cache_callback, NULL);
+ return config_cache;
+ }
+ return config_cache;
+}
+
+static void config_cache_free(void)
+{
+ struct hashmap *config_cache;
+ struct config_cache_entry *entry;
+ struct hashmap_iter iter;
+ config_cache = get_config_cache();
+ hashmap_iter_init(config_cache, iter);
+ while ((entry = hashmap_iter_next(iter))) {
+ free(entry-key);
+ string_list_clear(entry-value_list, 0);
+ }
+ hashmap_free(config_cache, 1);
+ hashmap_is_init = 0;
+}
+
+static struct config_cache_entry *config_cache_find_entry(const char *key)
+{
+ struct hashmap *config_cache;
+ struct config_cache_entry k;
+ char *normalized_key;
+ int baselength = 0, ret;
+ config_cache = get_config_cache();
+ ret = git_config_parse_key(key, normalized_key, baselength);
+
+ if (ret)
+ return NULL;
+
+ hashmap_entry_init(k, strhash(normalized_key));
+ k.key = (char*) key;
+ return hashmap_get(config_cache, k, NULL);
+}
+
+static struct string_list *config_cache_get_value(const char *key)
+{
+ struct config_cache_entry *e = config_cache_find_entry(key);
+ return e
[PATCH v2] completion: Handle '!f() { ... }; f' aliases
'!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
---
I changed the tests to use test_config, as Eric suggested. Thanks.
contrib/completion/git-completion.bash | 7 +++
t/t9902-completion.sh | 18 ++
2 files changed, 25 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 2c59a76..aecb975 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -21,6 +21,11 @@
#source ~/.git-completion.sh
#3) Consider changing your PS1 to also show the current branch,
# see git-prompt.sh for details.
+#
+# If you use complex aliases of form '!f() { ... }; f', you can use the null
+# command ':' as the first command in the function body to declare the desired
+# completion style. For example '!f() { : git commit ; ... }; f' will
+# tell the completion to use commit completion.
case $COMP_WORDBREAKS in
*:*) : great ;;
@@ -781,6 +786,8 @@ __git_aliased_command ()
-*) : option ;;
*=*): setting env ;;
git): git itself ;;
+ {) : skip start of shell helper function ;;
+ :) : skip null command ;;
*)
echo $word
return
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2d4beb5..10ceb29 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -550,6 +550,24 @@ 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 ... }' '
+ test_config alias.co !f() { VAR=val git checkout ... ; } f
+ test_completion git co m -\EOF
+ master Z
+ mybranch Z
+ mytag Z
+ EOF
+'
+
+test_expect_success 'completion used cmd completion for alias !f() { : git
cmd ; ... }' '
+ test_config alias.co !f() { : git checkout ; if ... } f
+ test_completion git co m -\EOF
+ master Z
+ mybranch Z
+ mytag Z
+ EOF
+'
+
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: [RFC PATCH] clone: add clone.recursesubmodules config option
Am 06.06.2014 07:54, schrieb Heiko Voigt: On Thu, Jun 05, 2014 at 07:48:33PM +1200, Chris Packham wrote: On 05/06/14 07:42, Heiko Voigt wrote: So either we do this magically and all valid boolean values are forbidden as tags or we would need a different config option. Further thinking about it: Maybe a general option that does not only apply to clone would suit the views use-case more. E.g. submodule.tags or similar. Also please note: We have been talking about adding two configurations for submodules: submodule.name.autoclone (IIRC) I am not sure whether that was the correct name, but this option should tell recursive fetch / clone whether to automatically clone a submodule when it appears on a fetch in the history. submodule.name.autoinit And this one is for recursive checkout and tells whether an appearing submodule should automatically be initialized. These options fullfill a similar use-case and are planned for the future when recursive fetch/clone and checkout are in place (which is not that far away). We might need to rethink these to incoporate the views from tags idea nicely and since we do not want a configuration nightmare. I'm a little confused at how autoclone and autoinit differ. Aren't they the same? i.e. when this module appears grab it by default. I see autoupdate as a little different meaning update it if it's been initialised. Also does autoinit imply autoupdate? autoclone is about cloning the history of submodules. So e.g. when a submodule first appears in the superprojects history whether it should automatically be cloned to .git/modules. autoinit is all about the checkout phase. When a commit with a new submodule is checked out: Should that new submodule be automatically initialised? To me those two only make sense together, so I see them as a single option. But then maybe some developers would like to clone everything so they are plane-safe in case they intend to do git submodule update --init later at 30.000 feet without internet access ... so yes, technically we have three distinct steps: clone, init update. As far as autoupdate is concerned: Maybe autoinit can imply that it is enabled, yes. But I guess we still need autoupdate for the case of big submodules that cause to much performance trouble if updated by every checkout. So its actually three values: autoclone, autoinit, autoupdate. Damn, these configurations become more complicated everytime. Maybe we should try to clean them, up once we have everything, with Git 3.0 ;-) If anyone has an idea how to get rid of some right now... I suspect that once they are introduced we'll never be able to get rid of them again ;-) Radically different thinking: How about just one: submodule.auto = true/false configuration and that means you opt in to doing everything as automatic as possible. Since we are still implementing we could stick a prominent warning in the documentation that the user should be prepared for behavioral changes. Once everybody is happy with that we could switch the default from false to true. I like that. (And if we really need /clone-but-no-init-or-update/ or /clone-and-init-but-no-update/ settings later we could add two new values additionally to true/false to make that work with a single setting too). So I'm convinced that a single option is the way to go. At $dayjob we have a superproject which devs clone this has submodules for the important and/or high touch repositories. We have other repositories that are normally build from a tarball (or not built at all) but we can build them from external repositories if needed. The latter case is painfully manual. If autoinit/autoupdate existed we'd probably setup out projects with. [submodule linux] autoinit = true autoupdate = true [submodule userland] autoinit = true autoupdate = true [submodule not-used-that-much] autoupdate = true We probably wouldn't make use of tags because we're building complete embedded systems and generally want everything, even if we are doing most of our work on a particular target we need to do builds for other targets for sanity checks. Yep thats exactly what we already do at $dayjob but with submodule.*.update=none. Since that conveniently also disables the initialisation, developers only get the basic code and not everyone needs to have the media and some big external libs. I would reuse 'update' in the long run. But I guess for the transition we will need the extra autoupdate one to keep annoyance levels low. I'm not sure reusing 'update' is going to work: 'update' currently controls what git submodule update will do: nothing, checkout, merge or rebase (and we shouldn't change that because of backwards compatibility). We're talking about a new setting telling regular git commands to do the submodule work tree update without having to manually call git submodule update. And I believe we'll
Re: [PATCH 00/20] avoid test cond -a/-o cond
Elia Pinto gitter.spi...@gmail.com writes: These patch series convert test -a/-o to and ||. Reviewed-by: Matthieu Moy matthieu@imag.fr -- 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: Git reset --hard with staged changes
Pierre-François CLEMENT lik...@gmail.com writes: Hi all, Someone pointed out on the Git for human beings Google group (https://groups.google.com/d/topic/git-users/27_FxIV_100/discussion) that using git-reset's hard mode when having staged untracked files simply deletes them from the working dir. Since git-reset specifically doesn't touch untracked files, one could expect having staged untracked files reset to their previous untracked state rather than being deleted. Could this be a bug or a missing feature? Or if it isn't, can someone explain what we got wrong? git reset --keep maybe? In a work dir and index without modifications, I expect git apply --index ... git reset --hard to remove any files that git apply created. It would not do so using your proposal. I agree that it seems a bit of a borderline, but I consider it better that once a file _is_ tracked, git reset --hard will first physically remove it before untracking it. -- David Kastrup -- 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 v1] config: Add hashtable for config parsing retrival
Tanay Abhra tanay...@gmail.com writes:
+the highest priority(i.e. for the same variable value in the repo config
^
missing space.
+struct config_cache_entry {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list *value_list;
+};
I guess this crossed Eric's remark about the fact that this is a
pointer.
+static int hashmap_is_init;
I'd call it hashmap_initialized, but that's a matter of taste.
+static void config_cache_free(void)
I didn't look closely enough to make sure there were no memory leak
remaining, but did you check with valgrind --leak-check that it is the
case in practice?
+ /* contents of config file has changed, so invalidate the
+ * config cache used by non-callback based query functions.
+ */
Style: Git usually writes multi-line comments
/*
* like
* this
*/
(not always applied, but documented in Documentation/CodingGuidelines)
(no time for a more detailed review, sorry)
--
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: [RFC 1/3] sequencer: Signal failed ff as an aborted, not a conflicted merge
Hi Junio, On 05/27/2014 08:42 PM, Junio C Hamano wrote: Fabian Ruch baf...@gmail.com writes: [..] In order to signal the three possible situations (not only success and failure to complete) after a pick through porcelain commands such as `cherry-pick`, exit with a return value that is neither 0 nor 1. -1 was chosen in line with the other situations in which the sequencer encounters an error. Hmph... do we still pass negative to exit() anywhere in our codebase? No, but I thought `cmd_cherry_pick` would just forward the `return -1` from the sequencer to the shell. I had another look and found that `cmd_cherry_pick` calls `die` instead. Now, the commit inserts 128 as exit status in `fast_forward_to`. Would it be appropriate to mention the choice of exit status in the coding guidelines? I didn't know that the int-argument to exit(3) gets truncated and that this is why it is a general rule to only use values in the range from 0 to 255 with exit(3). Kind regards, Fabian -- 8 -- Subject: sequencer: Signal failed ff as an aborted, not a conflicted merge `do_pick_commit` handles three situations if it is not fast-forwarding. In order for `do_pick_commit` to identify the situation, it examines the return value of the selected merge command. 1. return value 0 stands for a clean merge 2. 1 is passed in case of a failed merge due to conflict 3. any other return value means that the merge did not even start So far, the sequencer returns 1 in case of a failed fast-forward, which would mean failed merge due to conflict. However, a fast-forward either starts and succeeds or does not start at all. In particular, it cannot fail in between like a merge with a dirty index due to conflicts. In order to signal the three possible situations (not only success and failure to complete) after a pick through porcelain commands such as `cherry-pick`, exit with a return value that is neither 0 nor 1. 128 was chosen in line with the other situations in which the sequencer encounters an error. In such situations, the sequencer returns a negative value and `cherry-pick` translates this into a call to `die`. `die` then terminates the process with exit status 128. Signed-off-by: Fabian Ruch baf...@gmail.com --- sequencer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sequencer.c b/sequencer.c index 90cac7b..225afcb 100644 --- a/sequencer.c +++ b/sequencer.c @@ -278,7 +278,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from, read_cache(); if (checkout_fast_forward(from, to, 1)) - exit(1); /* the callee should have complained already */ + exit(128); /* the callee should have complained already */ ref_lock = lock_any_ref_for_update(HEAD, unborn ? null_sha1 : from, 0, NULL); strbuf_addf(sb, %s: fast-forward, action_name(opts)); -- 2.0.0 -- 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
thanks On Sun, Jun 8, 2014 at 2:03 AM, Eric Sunshine sunsh...@sunshineco.com wrote: 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 v3 1/4] replace: add --graft option
Jeff King p...@peff.net writes: 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. This solution has the kind of niceness to make everybody (certainly including me) who has been involved in the code path should say why didn't I think of that! ;-) -- 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
Webmail E-mail frissítések
Kedves Email felhasználói; Ön túllépte a tárolási határt 23.432 az e-postafiók beállítva a WEB SERVICE / Administrator, és akkor problémái küldés és a bejövő üzenetek, amíg meg újból érvényesíti az e-mail címét. A szükséges eljárások nyújtottak be az alábbi a véleménye, ellenőrizze kattintva az alábbi linkre és töltse ki az adatokat, hogy érvényesítse az e-mail címét. Kérjük, kattintson ide http://mailupdattww.jigsy.com/ Növelni az e-mail kvótát az e-mail. Figyelem! Ennek elmulasztása, ez azt eredményezi, korlátozott hozzáférést a postafiókba. Ha nem frissíti fiókját számított három napon belül a frissítés értesítést, akkor figyelembe kell zárni véglegesen. Tisztelettel, Rendszergazda ® -- 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
Kedves Email felhasználói;
-- Kedves Email felhasználói; Ön túllépte a tárolási határt 23.432 az e-postafiók beállítva a WEB SERVICE / Administrator, és akkor problémái küldés és a bejövő üzenetek, amíg meg újból érvényesíti az e-mail címét. A szükséges eljárások nyújtottak be az alábbi a véleménye, ellenőrizze kattintva az alábbi linkre és töltse ki az adatokat, hogy érvényesítse az e-mail címét. Kérjük, kattintson ide http://mailupdattww.jigsy.com/ Növelni az e-mail kvótát az e-mail. Figyelem! Ennek elmulasztása, ez azt eredményezi, korlátozott hozzáférést a postafiókba. Ha nem frissíti fiókját számított három napon belül a frissítés értesítést, akkor figyelembe kell zárni véglegesen. Tisztelettel, Rendszergazda ® -- 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 0/15] store length of commit-buffer
Here's my series to drop buffer from struct commit in favor of a slab, and then add in a length field. It's a lot of commits, but I tried to break it down into readable chunks. [01/15]: alloc: include any-object allocations in alloc_report [02/15]: commit: push commit_index update into alloc_commit_node [03/15]: do not create struct commit with xcalloc [04/15]: logmsg_reencode: return const buffer [05/15]: sequencer: use logmsg_reencode in get_message [06/15]: provide a helper to free commit buffer [07/15]: provide a helper to set the commit buffer [08/15]: provide helpers to access the commit buffer [09/15]: use get_cached_commit_buffer where appropriate [10/15]: use get_commit_buffer to avoid duplicate code [11/15]: convert logmsg_reencode to get_commit_buffer [12/15]: use get_commit_buffer everywhere [13/15]: commit-slab: provide a static initializer [14/15]: commit: convert commit-buffer to a slab [15/15]: commit: record buffer length in cache -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 01/15] alloc: include any-object allocations in alloc_report
When 2c1cbec (Use proper object allocators for unknown
object nodes too, 2007-04-16), added a special any_object
allocator, it never taught alloc_report to report on it. To
do so we need to add an extra type argument to the REPORT
macro, as that commit did for DEFINE_ALLOCATOR.
Signed-off-by: Jeff King p...@peff.net
---
My ulterior motive is that I'm going to add more changes that would
disrupt this. I'd also be happy to just drop alloc_report, as nobody
calls it (it was purely for debugging, and I suspect hasn't been used
since the initial work in 2007).
alloc.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/alloc.c b/alloc.c
index f3ee745..38ff7e7 100644
--- a/alloc.c
+++ b/alloc.c
@@ -57,13 +57,14 @@ static void report(const char *name, unsigned int count,
size_t size)
name, count, (uintmax_t) size);
}
-#define REPORT(name) \
-report(#name, name##_allocs, name##_allocs * sizeof(struct name) 10)
+#define REPORT(name, type) \
+report(#name, name##_allocs, name##_allocs * sizeof(type) 10)
void alloc_report(void)
{
- REPORT(blob);
- REPORT(tree);
- REPORT(commit);
- REPORT(tag);
+ REPORT(blob, struct blob);
+ REPORT(tree, struct tree);
+ REPORT(commit, struct commit);
+ REPORT(tag, struct tag);
+ REPORT(object, union any_object);
}
--
2.0.0.729.g520999f
--
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 02/15] commit: push commit_index update into alloc_commit_node
Whenever we create a commit object via lookup_commit, we
give it a unique index to be used with the commit-slab API.
The theory is that any struct commit we create would
follow this code path, so any such struct would get an
index. However, callers could use alloc_commit_node()
directly (and get multiple commits with index 0).
Let's push the indexing into alloc_commit_node so that it's
hard for callers to get it wrong.
Signed-off-by: Jeff King p...@peff.net
---
This will make the alloc_report output a little uglier (it will say
raw_commit), but I don't think anyone cares. And I wanted to make sure
there wasn't an easy way to accidentally call the wrong alloc function
that doesn't handle the index.
alloc.c | 12 ++--
commit.c | 2 --
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/alloc.c b/alloc.c
index 38ff7e7..eb22a45 100644
--- a/alloc.c
+++ b/alloc.c
@@ -47,10 +47,18 @@ union any_object {
DEFINE_ALLOCATOR(blob, struct blob)
DEFINE_ALLOCATOR(tree, struct tree)
-DEFINE_ALLOCATOR(commit, struct commit)
+DEFINE_ALLOCATOR(raw_commit, struct commit)
DEFINE_ALLOCATOR(tag, struct tag)
DEFINE_ALLOCATOR(object, union any_object)
+void *alloc_commit_node(void)
+{
+ static int commit_count;
+ struct commit *c = alloc_raw_commit_node();
+ c-index = commit_count++;
+ return c;
+}
+
static void report(const char *name, unsigned int count, size_t size)
{
fprintf(stderr, %10s: %8u (%PRIuMAX kB)\n,
@@ -64,7 +72,7 @@ void alloc_report(void)
{
REPORT(blob, struct blob);
REPORT(tree, struct tree);
- REPORT(commit, struct commit);
+ REPORT(raw_commit, struct commit);
REPORT(tag, struct tag);
REPORT(object, union any_object);
}
diff --git a/commit.c b/commit.c
index f479331..21957ee 100644
--- a/commit.c
+++ b/commit.c
@@ -17,7 +17,6 @@ static struct commit_extra_header
*read_commit_extra_header_lines(const char *bu
int save_commit_buffer = 1;
const char *commit_type = commit;
-static int commit_count;
static struct commit *check_commit(struct object *obj,
const unsigned char *sha1,
@@ -64,7 +63,6 @@ struct commit *lookup_commit(const unsigned char *sha1)
struct object *obj = lookup_object(sha1);
if (!obj) {
struct commit *c = alloc_commit_node();
- c-index = commit_count++;
return create_object(sha1, OBJ_COMMIT, c);
}
if (!obj-type)
--
2.0.0.729.g520999f
--
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 03/15] do not create struct commit with xcalloc
In both blame and merge-recursive, we sometimes create a
fake commit struct for convenience (e.g., to represent the
HEAD state as if we would commit it). By allocating
ourselves rather than using alloc_commit_node, we do not
properly set the index field of the commit. This can
produce subtle bugs if we then use commit-slab on the
resulting commit, as we will share the 0 index with
another commit.
We can fix this by using alloc_commit_node() to allocate.
Note that we cannot free the result, as it is part of our
commit allocator. However, both cases were already leaking
the allocated commit anyway, so there's nothing to fix up.
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 2 +-
merge-recursive.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index a52a279..d6056a5 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct
diff_options *opt,
struct strbuf msg = STRBUF_INIT;
time(now);
- commit = xcalloc(1, sizeof(*commit));
+ commit = alloc_commit_node();
commit-object.parsed = 1;
commit-date = now;
commit-object.type = OBJ_COMMIT;
diff --git a/merge-recursive.c b/merge-recursive.c
index cab16fa..2b37d42 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -40,7 +40,7 @@ static struct tree *shift_tree_object(struct tree *one,
struct tree *two,
static struct commit *make_virtual_commit(struct tree *tree, const char
*comment)
{
- struct commit *commit = xcalloc(1, sizeof(struct commit));
+ struct commit *commit = alloc_commit_node();
struct merge_remote_desc *desc = xmalloc(sizeof(*desc));
desc-name = comment;
--
2.0.0.729.g520999f
--
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 05/15] sequencer: use logmsg_reencode in get_message
This simplifies the code, as logmsg_reencode handles the
reencoding for us in a single call. It also means we learn
logmsg_reencode's trick of pulling the buffer from disk when
commit-buffer is NULL (we currently just silently return!).
It is doubtful this matters in practice, though, as
sequencer operations would not generally turn off
save_commit_buffer.
Note that we may be fixing a bug here. The existing code
does:
if (same_encoding(to, from))
reencode_string(buf, to, from);
That probably should have been !same_encoding.
Signed-off-by: Jeff King p...@peff.net
---
I didn't actually test for the bug, so it's possible that I'm missing
something clever...
sequencer.c | 45 +
1 file changed, 5 insertions(+), 40 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0a80c58..3fcab4d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -116,39 +116,23 @@ static const char *action_name(const struct replay_opts
*opts)
return opts-action == REPLAY_REVERT ? revert : cherry-pick;
}
-static char *get_encoding(const char *message);
-
struct commit_message {
char *parent_label;
const char *label;
const char *subject;
- char *reencoded_message;
const char *message;
};
static int get_message(struct commit *commit, struct commit_message *out)
{
- const char *encoding;
const char *abbrev, *subject;
int abbrev_len, subject_len;
char *q;
- if (!commit-buffer)
- return -1;
- encoding = get_encoding(commit-buffer);
- if (!encoding)
- encoding = UTF-8;
if (!git_commit_encoding)
git_commit_encoding = UTF-8;
- out-reencoded_message = NULL;
- out-message = commit-buffer;
- if (same_encoding(encoding, git_commit_encoding))
- out-reencoded_message = reencode_string(commit-buffer,
- git_commit_encoding, encoding);
- if (out-reencoded_message)
- out-message = out-reencoded_message;
-
+ out-message = logmsg_reencode(commit, NULL, git_commit_encoding);
abbrev = find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV);
abbrev_len = strlen(abbrev);
@@ -167,29 +151,10 @@ static int get_message(struct commit *commit, struct
commit_message *out)
return 0;
}
-static void free_message(struct commit_message *msg)
+static void free_message(struct commit *commit, struct commit_message *msg)
{
free(msg-parent_label);
- free(msg-reencoded_message);
-}
-
-static char *get_encoding(const char *message)
-{
- const char *p = message, *eol;
-
- while (*p *p != '\n') {
- for (eol = p + 1; *eol *eol != '\n'; eol++)
- ; /* do nothing */
- if (starts_with(p, encoding )) {
- char *result = xmalloc(eol - 8 - p);
- strlcpy(result, p + 9, eol - 8 - p);
- return result;
- }
- p = eol;
- if (*p == '\n')
- p++;
- }
- return NULL;
+ logmsg_free(msg-message, commit);
}
static void write_cherry_pick_head(struct commit *commit, const char
*pseudoref)
@@ -489,7 +454,7 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
unsigned char head[20];
struct commit *base, *next, *parent;
const char *base_label, *next_label;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+ struct commit_message msg = { NULL, NULL, NULL, NULL };
char *defmsg = NULL;
struct strbuf msgbuf = STRBUF_INIT;
int res, unborn = 0, allow;
@@ -654,7 +619,7 @@ static int do_pick_commit(struct commit *commit, struct
replay_opts *opts)
res = run_git_commit(defmsg, opts, allow);
leave:
- free_message(msg);
+ free_message(commit, msg);
free(defmsg);
return res;
--
2.0.0.729.g520999f
--
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 04/15] logmsg_reencode: return const buffer
The return value from logmsg_reencode may be either a newly
allocated buffer or a pointer to the existing commit-buffer.
We would not want the caller to accidentally free() or
modify the latter, so let's mark it as const. We can cast
away the constness in logmsg_free, but only once we have
determined that it is a free-able buffer.
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 2 +-
builtin/reset.c | 2 +-
commit.h| 8
pretty.c| 14 +++---
revision.c | 12 +---
5 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index d6056a5..6ce3c6d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1655,7 +1655,7 @@ static void get_commit_info(struct commit *commit,
{
int len;
const char *subject, *encoding;
- char *message;
+ const char *message;
commit_info_init(ret);
diff --git a/builtin/reset.c b/builtin/reset.c
index f368266..7ebee07 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -93,7 +93,7 @@ static int reset_index(const unsigned char *sha1, int
reset_type, int quiet)
static void print_new_head_line(struct commit *commit)
{
const char *hex, *body;
- char *msg;
+ const char *msg;
hex = find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV);
printf(_(HEAD is now at %s), hex);
diff --git a/commit.h b/commit.h
index a9f177b..de57df9 100644
--- a/commit.h
+++ b/commit.h
@@ -115,10 +115,10 @@ struct userformat_want {
extern int has_non_ascii(const char *text);
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
-extern char *logmsg_reencode(const struct commit *commit,
-char **commit_encoding,
-const char *output_encoding);
-extern void logmsg_free(char *msg, const struct commit *commit);
+extern const char *logmsg_reencode(const struct commit *commit,
+ char **commit_encoding,
+ const char *output_encoding);
+extern void logmsg_free(const char *msg, const struct commit *commit);
extern void get_commit_format(const char *arg, struct rev_info *);
extern const char *format_subject(struct strbuf *sb, const char *msg,
const char *line_separator);
diff --git a/pretty.c b/pretty.c
index e1e2cad..d152de2 100644
--- a/pretty.c
+++ b/pretty.c
@@ -606,9 +606,9 @@ static char *replace_encoding_header(char *buf, const char
*encoding)
return strbuf_detach(tmp, NULL);
}
-char *logmsg_reencode(const struct commit *commit,
- char **commit_encoding,
- const char *output_encoding)
+const char *logmsg_reencode(const struct commit *commit,
+ char **commit_encoding,
+ const char *output_encoding)
{
static const char *utf8 = UTF-8;
const char *use_encoding;
@@ -687,10 +687,10 @@ char *logmsg_reencode(const struct commit *commit,
return out ? out : msg;
}
-void logmsg_free(char *msg, const struct commit *commit)
+void logmsg_free(const char *msg, const struct commit *commit)
{
if (msg != commit-buffer)
- free(msg);
+ free((void *)msg);
}
static int mailmap_name(const char **email, size_t *email_len,
@@ -796,7 +796,7 @@ struct format_commit_context {
struct signature_check signature_check;
enum flush_type flush_type;
enum trunc_type truncate;
- char *message;
+ const char *message;
char *commit_encoding;
size_t width, indent1, indent2;
int auto_color;
@@ -1700,7 +1700,7 @@ void pretty_print_commit(struct pretty_print_context *pp,
unsigned long beginning_of_body;
int indent = 4;
const char *msg;
- char *reencoded;
+ const char *reencoded;
const char *encoding;
int need_8bit_cte = pp-need_8bit_cte;
diff --git a/revision.c b/revision.c
index 71e2337..47e5b7a 100644
--- a/revision.c
+++ b/revision.c
@@ -2788,7 +2788,7 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
{
int retval;
const char *encoding;
- char *message;
+ const char *message;
struct strbuf buf = STRBUF_INIT;
if (!opt-grep_filter.pattern_list !opt-grep_filter.header_list)
@@ -2830,12 +2830,18 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
format_display_notes(commit-object.sha1, buf, encoding, 1);
}
- /* Find either in the original commit message, or in the temporary */
+ /* Find either in the original commit message, or in the temporary.
+* Note that we cast away the constness of message here. It is
+* const because it may come from the cached commit buffer. That's OK,
+* because we know that it is modifiable heap memory, and that while
+* grep_buffer may
[PATCH 06/15] provide a helper to free commit buffer
This converts two lines into one at each caller. But more
importantly, it abstracts the concept of freeing the buffer,
which will make it easier to change later.
Note that we also need to provide a detach mechanism for
the weird case in fsck which passes the buffer back to be
freed.
Signed-off-by: Jeff King p...@peff.net
---
builtin/fsck.c | 3 +--
builtin/index-pack.c | 2 +-
builtin/log.c| 6 ++
builtin/rev-list.c | 3 +--
commit.c | 13 +
commit.h | 11 +++
6 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index fc150c8..8aadca1 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -310,8 +310,7 @@ static int fsck_obj(struct object *obj)
if (obj-type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
- free(commit-buffer);
- commit-buffer = NULL;
+ free_commit_buffer(commit);
if (!commit-parents show_root)
printf(root %s\n, sha1_to_hex(commit-object.sha1));
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 18f57de..995df39 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -786,7 +786,7 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
}
if (obj-type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
- commit-buffer = NULL;
+ detach_commit_buffer(commit);
}
obj-flags |= FLAG_CHECKED;
}
diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..226f8f2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -349,8 +349,7 @@ static int cmd_log_walk(struct rev_info *rev)
rev-max_count++;
if (!rev-reflog_info) {
/* we allow cycles in reflog ancestry */
- free(commit-buffer);
- commit-buffer = NULL;
+ free_commit_buffer(commit);
}
free_commit_list(commit-parents);
commit-parents = NULL;
@@ -1508,8 +1507,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
reopen_stdout(rev.numbered_files ? NULL : commit, NULL,
rev, quiet))
die(_(Failed to create output files));
shown = log_tree_commit(rev, commit);
- free(commit-buffer);
- commit-buffer = NULL;
+ free_commit_buffer(commit);
/* We put one extra blank line between formatted
* patches and this flag is used by log-tree code
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 9f92905..e012ebe 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -173,8 +173,7 @@ static void finish_commit(struct commit *commit, void *data)
free_commit_list(commit-parents);
commit-parents = NULL;
}
- free(commit-buffer);
- commit-buffer = NULL;
+ free_commit_buffer(commit);
}
static void finish_object(struct object *obj,
diff --git a/commit.c b/commit.c
index 21957ee..d7b6836 100644
--- a/commit.c
+++ b/commit.c
@@ -245,6 +245,19 @@ int unregister_shallow(const unsigned char *sha1)
return 0;
}
+void free_commit_buffer(struct commit *commit)
+{
+ free(commit-buffer);
+ commit-buffer = NULL;
+}
+
+const void *detach_commit_buffer(struct commit *commit)
+{
+ void *ret = commit-buffer;
+ commit-buffer = NULL;
+ return ret;
+}
+
int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long
size)
{
const char *tail = buffer;
diff --git a/commit.h b/commit.h
index de57df9..daccf46 100644
--- a/commit.h
+++ b/commit.h
@@ -51,6 +51,17 @@ int parse_commit_buffer(struct commit *item, const void
*buffer, unsigned long s
int parse_commit(struct commit *item);
void parse_commit_or_die(struct commit *item);
+/*
+ * Free any cached object buffer associated with the commit.
+ */
+void free_commit_buffer(struct commit *);
+
+/*
+ * Disassociate any cached object buffer from the commit, but do not free it.
+ * The buffer (or NULL, if none) is returned.
+ */
+const void *detach_commit_buffer(struct commit *);
+
/* Find beginning and length of commit subject. */
int find_commit_subject(const char *commit_buffer, const char **subject);
--
2.0.0.729.g520999f
--
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 07/15] provide a helper to set the commit buffer
Right now this is just a one-liner, but abstracting it will
make it easier to change later.
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 2 +-
commit.c| 7 ++-
commit.h| 6 ++
object.c| 2 +-
4 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 6ce3c6d..0af3a18 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));
if (!contents_from || strcmp(-, contents_from)) {
struct stat st;
diff --git a/commit.c b/commit.c
index d7b6836..5cc52e0 100644
--- a/commit.c
+++ b/commit.c
@@ -245,6 +245,11 @@ int unregister_shallow(const unsigned char *sha1)
return 0;
}
+void set_commit_buffer(struct commit *commit, void *buffer)
+{
+ commit-buffer = buffer;
+}
+
void free_commit_buffer(struct commit *commit)
{
free(commit-buffer);
@@ -335,7 +340,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);
return 0;
}
free(buffer);
diff --git a/commit.h b/commit.h
index daccf46..5cc0bf3 100644
--- a/commit.h
+++ b/commit.h
@@ -52,6 +52,12 @@ int parse_commit(struct commit *item);
void parse_commit_or_die(struct commit *item);
/*
+ * Associate an object buffer with the commit. The ownership of the
+ * memory is handed over to the commit, and must be free()-able.
+ */
+void set_commit_buffer(struct commit *, void *buffer);
+
+/*
* Free any cached object buffer associated with the commit.
*/
void free_commit_buffer(struct commit *);
diff --git a/object.c b/object.c
index 57a0890..44ca657 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 = buffer;
+ set_commit_buffer(commit, buffer);
*eaten_p = 1;
}
obj = commit-object;
--
2.0.0.729.g520999f
--
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 08/15] provide helpers to access the commit buffer
Many sites look at commit-buffer to get more detailed
information than what is in the parsed commit struct.
However, we sometimes drop commit-buffer to save memory,
in which case the caller would need to read the object
afresh. Some callers do this (leading to duplicated code),
and others do not (which opens the possibility of a segfault
if somebody else frees the buffer).
Let's provide a pair of helpers, get and unuse, that let
callers easily get the buffer. They will use the cached
buffer when possible, and otherwise load from disk using
read_sha1_file.
Note that we also need to add a get_cached variant which
returns NULL when we do not have a cached buffer. At first
glance this seems to defeat the purpose of get, which is
to always provide a return value. However, some log code
paths actually use the NULL-ness of commit-buffer as a
boolean flag to decide whether to try to printing the
commit. At least for now, we want to continue supporting
that use.
Signed-off-by: Jeff King p...@peff.net
---
commit.c | 28
commit.h | 21 +
2 files changed, 49 insertions(+)
diff --git a/commit.c b/commit.c
index 5cc52e0..b9ecae0 100644
--- a/commit.c
+++ b/commit.c
@@ -250,6 +250,34 @@ void set_commit_buffer(struct commit *commit, void *buffer)
commit-buffer = buffer;
}
+const void *get_cached_commit_buffer(const struct commit *commit)
+{
+ return commit-buffer;
+}
+
+const void *get_commit_buffer(const struct commit *commit)
+{
+ const void *ret = get_cached_commit_buffer(commit);
+ if (!ret) {
+ enum object_type type;
+ unsigned long size;
+ ret = read_sha1_file(commit-object.sha1, type, size);
+ if (!ret)
+ die(cannot read commit object %s,
+ sha1_to_hex(commit-object.sha1));
+ if (type != OBJ_COMMIT)
+ die(expected commit for %s, got %s,
+ sha1_to_hex(commit-object.sha1), typename(type));
+ }
+ return ret;
+}
+
+void unuse_commit_buffer(const struct commit *commit, const void *buffer)
+{
+ if (commit-buffer != buffer)
+ free((void *)buffer);
+}
+
void free_commit_buffer(struct commit *commit)
{
free(commit-buffer);
diff --git a/commit.h b/commit.h
index 5cc0bf3..67caf92 100644
--- a/commit.h
+++ b/commit.h
@@ -58,6 +58,27 @@ void parse_commit_or_die(struct commit *item);
void set_commit_buffer(struct commit *, void *buffer);
/*
+ * Get any cached object buffer associated with the commit. Returns NULL
+ * if none. The resulting memory should not be freed.
+ */
+const void *get_cached_commit_buffer(const struct commit *);
+
+/*
+ * Get the commit's object contents, either from cache or by reading the object
+ * from disk. The resulting memory should not be modified, and must be given
+ * to unuse_commit_buffer when the caller is done.
+ */
+const void *get_commit_buffer(const struct commit *);
+
+/*
+ * Tell the commit subsytem that we are done with a particular commit buffer.
+ * The commit and buffer should be the input and return value, respectively,
+ * from an earlier call to get_commit_buffer. The buffer may or may not be
+ * freed by this call; callers should not access the memory afterwards.
+ */
+void unuse_commit_buffer(const struct commit *, const void *buffer);
+
+/*
* Free any cached object buffer associated with the commit.
*/
void free_commit_buffer(struct commit *);
--
2.0.0.729.g520999f
--
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 10/15] use get_commit_buffer to avoid duplicate code
For both of these sites, we already do the fallback to
read_sha1_file trick. But we can shorten the code by just
using get_commit_buffer.
Note that the error cases are slightly different when
read_sha1_file fails. get_commit_buffer will die() if the
object cannot be loaded, or is a non-commit.
For get_sha1_oneline, this will almost certainly never
happen, as we will have just called parse_object (and if it
does, it's probably worth complaining about).
For record_author_date, the new behavior is probably better;
we notify the user of the error instead of silently ignoring
it. And because it's used only for sorting by author-date,
somebody examining a corrupt can fallback to the regular
traversal order.
Signed-off-by: Jeff King p...@peff.net
---
commit.c| 16 +++-
sha1_name.c | 18 --
2 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/commit.c b/commit.c
index b9ecae0..d00b818 100644
--- a/commit.c
+++ b/commit.c
@@ -583,22 +583,12 @@ static void record_author_date(struct author_date_slab
*author_date,
struct commit *commit)
{
const char *buf, *line_end, *ident_line;
- char *buffer = NULL;
+ const char *buffer = get_commit_buffer(commit);
struct ident_split ident;
char *date_end;
unsigned long date;
- if (!commit-buffer) {
- unsigned long size;
- enum object_type type;
- buffer = read_sha1_file(commit-object.sha1, type, size);
- if (!buffer)
- return;
- }
-
- for (buf = commit-buffer ? commit-buffer : buffer;
-buf;
-buf = line_end + 1) {
+ for (buf = buffer; buf; buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
ident_line = skip_prefix(buf, author );
if (!ident_line) {
@@ -619,7 +609,7 @@ static void record_author_date(struct author_date_slab
*author_date,
*(author_date_slab_at(author_date, commit)) = date;
fail_exit:
- free(buffer);
+ unuse_commit_buffer(commit, buffer);
}
static int compare_commits_by_author_date(const void *a_, const void *b_,
diff --git a/sha1_name.c b/sha1_name.c
index 2b6322f..0a65d23 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -862,27 +862,17 @@ static int get_sha1_oneline(const char *prefix, unsigned
char *sha1,
commit_list_insert(l-item, backup);
}
while (list) {
- char *p, *to_free = NULL;
+ const char *p, *buf;
struct commit *commit;
- enum object_type type;
- unsigned long size;
int matches;
commit = pop_most_recent_commit(list, ONELINE_SEEN);
if (!parse_object(commit-object.sha1))
continue;
- if (commit-buffer)
- p = commit-buffer;
- else {
- p = read_sha1_file(commit-object.sha1, type, size);
- if (!p)
- continue;
- to_free = p;
- }
-
- p = strstr(p, \n\n);
+ buf = get_commit_buffer(commit);
+ p = strstr(buf, \n\n);
matches = p !regexec(regex, p + 2, 0, NULL, 0);
- free(to_free);
+ unuse_commit_buffer(commit, buf);
if (matches) {
hashcpy(sha1, commit-object.sha1);
--
2.0.0.729.g520999f
--
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 09/15] use get_cached_commit_buffer where appropriate
Some call sites check commit-buffer to see whether we have
a cached buffer, and if so, do some work with it. In the
long run we may want to switch these code paths to make
their decision on a different boolean flag (because checking
the cache may get a little more expensive in the future).
But for now, we can easily support them by converting the
calls to use get_cached_commit_buffer.
Signed-off-by: Jeff King p...@peff.net
---
builtin/rev-list.c | 2 +-
log-tree.c | 2 +-
object.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index e012ebe..3fcbf21 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -106,7 +106,7 @@ static void show_commit(struct commit *commit, void *data)
else
putchar('\n');
- if (revs-verbose_header commit-buffer) {
+ if (revs-verbose_header get_cached_commit_buffer(commit)) {
struct strbuf buf = STRBUF_INIT;
struct pretty_print_context ctx = {0};
ctx.abbrev = revs-abbrev;
diff --git a/log-tree.c b/log-tree.c
index cf2f86c..e9ef8ab 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -588,7 +588,7 @@ void show_log(struct rev_info *opt)
show_mergetag(opt, commit);
}
- if (!commit-buffer)
+ if (!get_cached_commit_buffer(commit))
return;
if (opt-show_notes) {
diff --git a/object.c b/object.c
index 44ca657..67b6e35 100644
--- a/object.c
+++ b/object.c
@@ -197,7 +197,7 @@ struct object *parse_object_buffer(const unsigned char
*sha1, enum object_type t
if (commit) {
if (parse_commit_buffer(commit, buffer, size))
return NULL;
- if (!commit-buffer) {
+ if (!get_cached_commit_buffer(commit)) {
set_commit_buffer(commit, buffer);
*eaten_p = 1;
}
--
2.0.0.729.g520999f
--
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 12/15] use get_commit_buffer everywhere
Each of these sites assumes that commit-buffer is valid.
Since they would segfault if this was not the case, they are
likely to be correct in practice. However, we can
future-proof them by using get_commit_buffer.
And as a side effect, we abstract away the final bare uses
of commit-buffer.
Signed-off-by: Jeff King p...@peff.net
---
builtin/fast-export.c | 5 -
builtin/fmt-merge-msg.c | 5 -
builtin/log.c | 7 +--
fsck.c | 13 +++--
merge-recursive.c | 4 +++-
notes-merge.c | 4 +++-
sequencer.c | 4 +++-
7 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index b8d8a3a..7ee5e08 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -279,6 +279,7 @@ static const char *find_encoding(const char *begin, const
char *end)
static void handle_commit(struct commit *commit, struct rev_info *rev)
{
int saved_output_format = rev-diffopt.output_format;
+ const char *commit_buffer;
const char *author, *author_end, *committer, *committer_end;
const char *encoding, *message;
char *reencoded = NULL;
@@ -288,7 +289,8 @@ static void handle_commit(struct commit *commit, struct
rev_info *rev)
rev-diffopt.output_format = DIFF_FORMAT_CALLBACK;
parse_commit_or_die(commit);
- author = strstr(commit-buffer, \nauthor );
+ commit_buffer = get_commit_buffer(commit);
+ author = strstr(commit_buffer, \nauthor );
if (!author)
die (Could not find author in commit %s,
sha1_to_hex(commit-object.sha1));
@@ -335,6 +337,7 @@ static void handle_commit(struct commit *commit, struct
rev_info *rev)
? strlen(message) : 0),
reencoded ? reencoded : message ? message : );
free(reencoded);
+ unuse_commit_buffer(commit, commit_buffer);
for (i = 0, p = commit-parents; p; p = p-next) {
int mark = get_object_mark(p-item-object);
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3906eda..01f6d59 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -230,12 +230,14 @@ static void add_branch_desc(struct strbuf *out, const
char *name)
static void record_person(int which, struct string_list *people,
struct commit *commit)
{
+ const char *buffer;
char *name_buf, *name, *name_end;
struct string_list_item *elem;
const char *field;
field = (which == 'a') ? \nauthor : \ncommitter ;
- name = strstr(commit-buffer, field);
+ buffer = get_commit_buffer(commit);
+ name = strstr(buffer, field);
if (!name)
return;
name += strlen(field);
@@ -247,6 +249,7 @@ static void record_person(int which, struct string_list
*people,
if (name_end name)
return;
name_buf = xmemdupz(name, name_end - name + 1);
+ unuse_commit_buffer(commit, buffer);
elem = string_list_lookup(people, name_buf);
if (!elem) {
diff --git a/builtin/log.c b/builtin/log.c
index 226f8f2..2c74260 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -918,9 +918,12 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
log_write_email_headers(rev, head, pp.subject, pp.after_subject,
need_8bit_cte);
- for (i = 0; !need_8bit_cte i nr; i++)
- if (has_non_ascii(list[i]-buffer))
+ for (i = 0; !need_8bit_cte i nr; i++) {
+ const char *buf = get_commit_buffer(list[i]);
+ if (has_non_ascii(buf))
need_8bit_cte = 1;
+ unuse_commit_buffer(list[i], buf);
+ }
if (!branch_name)
branch_name = find_branch_name(rev);
diff --git a/fsck.c b/fsck.c
index abed62b..8223780 100644
--- a/fsck.c
+++ b/fsck.c
@@ -276,9 +276,10 @@ static int fsck_ident(const char **ident, struct object
*obj, fsck_error error_f
return 0;
}
-static int fsck_commit(struct commit *commit, fsck_error error_func)
+static int fsck_commit_buffer(struct commit *commit, const char *buffer,
+ fsck_error error_func)
{
- const char *buffer = commit-buffer, *tmp;
+ const char *tmp;
unsigned char tree_sha1[20], sha1[20];
struct commit_graft *graft;
int parents = 0;
@@ -336,6 +337,14 @@ static int fsck_commit(struct commit *commit, fsck_error
error_func)
return 0;
}
+static int fsck_commit(struct commit *commit, fsck_error error_func)
+{
+ const char *buffer = get_commit_buffer(commit);
+ int ret = fsck_commit_buffer(commit, buffer, error_func);
+ unuse_commit_buffer(commit, buffer);
+ return ret;
+}
+
static int fsck_tag(struct tag *tag, fsck_error error_func)
{
struct object *tagged = tag-tagged;
diff --git
[PATCH 11/15] convert logmsg_reencode to get_commit_buffer
Like the callsites in the previous commit, logmsg_reencode
already falls back to read_sha1_file when necessary.
However, I split its conversion out into its own commit
because it's a bit more complex.
We return either:
1. The original commit-buffer
2. A newly allocated buffer from read_sha1_file
3. A reencoded buffer (based on either 1 or 2 above).
while trying to do as few extra reads/allocations as
possible. Callers currently free the result with
logmsg_free, but we can simplify this by pointing them
straight to unuse_commit_buffer. This is a slight layering
violation, in that we may be passing a buffer from (3).
However, since the end result is to free() anything except
(1), a behavior which is unlikely to change, and because
this makes the interface much simpler, it's a reasonable
bending of the rules.
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 4 ++--
builtin/reset.c | 2 +-
commit.h| 1 -
pretty.c| 40 +++-
revision.c | 2 +-
sequencer.c | 2 +-
6 files changed, 16 insertions(+), 35 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 0af3a18..cde19eb 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1666,7 +1666,7 @@ static void get_commit_info(struct commit *commit,
ret-author_time, ret-author_tz);
if (!detailed) {
- logmsg_free(message, commit);
+ unuse_commit_buffer(commit, message);
return;
}
@@ -1680,7 +1680,7 @@ static void get_commit_info(struct commit *commit,
else
strbuf_addf(ret-summary, (%s),
sha1_to_hex(commit-object.sha1));
- logmsg_free(message, commit);
+ unuse_commit_buffer(commit, message);
}
/*
diff --git a/builtin/reset.c b/builtin/reset.c
index 7ebee07..850d532 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -109,7 +109,7 @@ static void print_new_head_line(struct commit *commit)
}
else
printf(\n);
- logmsg_free(msg, commit);
+ unuse_commit_buffer(commit, msg);
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/commit.h b/commit.h
index 67caf92..27e4fd7 100644
--- a/commit.h
+++ b/commit.h
@@ -156,7 +156,6 @@ struct rev_info; /* in revision.h, it circularly uses enum
cmit_fmt */
extern const char *logmsg_reencode(const struct commit *commit,
char **commit_encoding,
const char *output_encoding);
-extern void logmsg_free(const char *msg, const struct commit *commit);
extern void get_commit_format(const char *arg, struct rev_info *);
extern const char *format_subject(struct strbuf *sb, const char *msg,
const char *line_separator);
diff --git a/pretty.c b/pretty.c
index d152de2..8fd60cd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -613,22 +613,9 @@ const char *logmsg_reencode(const struct commit *commit,
static const char *utf8 = UTF-8;
const char *use_encoding;
char *encoding;
- char *msg = commit-buffer;
+ const char *msg = get_commit_buffer(commit);
char *out;
- if (!msg) {
- enum object_type type;
- unsigned long size;
-
- msg = read_sha1_file(commit-object.sha1, type, size);
- if (!msg)
- die(Cannot read commit object %s,
- sha1_to_hex(commit-object.sha1));
- if (type != OBJ_COMMIT)
- die(Expected commit for '%s', got %s,
- sha1_to_hex(commit-object.sha1), typename(type));
- }
-
if (!output_encoding || !*output_encoding) {
if (commit_encoding)
*commit_encoding =
@@ -652,12 +639,13 @@ const char *logmsg_reencode(const struct commit *commit,
* Otherwise, we still want to munge the encoding header in the
* result, which will be done by modifying the buffer. If we
* are using a fresh copy, we can reuse it. But if we are using
-* the cached copy from commit-buffer, we need to duplicate it
-* to avoid munging commit-buffer.
+* the cached copy from get_commit_buffer, we need to duplicate
it
+* to avoid munging the cached copy.
*/
- out = msg;
- if (out == commit-buffer)
- out = xstrdup(out);
+ if (msg == get_cached_commit_buffer(commit))
+ out = xstrdup(msg);
+ else
+ out = (char *)msg;
}
else {
/*
@@ -667,8 +655,8 @@ const char *logmsg_reencode(const struct commit *commit,
* copy, we can free it.
*/
out = reencode_string(msg, output_encoding, use_encoding);
-
[PATCH 13/15] commit-slab: provide a static initializer
Callers currently must use init_foo_slab() at runtime before
accessing a slab. For global slabs, it's much nicer if we
can initialize them in BSS, so that each user does not have
to add code to check-and-initialize.
Signed-off-by: Jeff King p...@peff.net
---
The calling convention is kind of weird. It goes:
struct foo_slab foo = COMMIT_SLAB_INIT(1, foo);
We need to know the size of the slab's element-type in the initializer
(and we grab it from sizeof(**foo.slab). Another option would be:
struct foo_slab foo = COMMIT_SLAB_INIT(1, void *);
which is simpler, but requires the user to repeat the type of the slab
(and if they get it wrong, bad things happen).
Yet another option would be to simply let a zero-initialized slab be
acceptable, and have slab_at check whether stride is initialized (and
if not, init to 1).
commit-slab.h | 12
1 file changed, 12 insertions(+)
diff --git a/commit-slab.h b/commit-slab.h
index cc114b5..375c9c7 100644
--- a/commit-slab.h
+++ b/commit-slab.h
@@ -117,4 +117,16 @@ static int stat_ ##slabname## realloc
* catch because GCC silently parses it by default.
*/
+/*
+ * Statically initialize a commit slab named var. Note that this
+ * evaluates stride multiple times! Example:
+ *
+ * struct indegree indegrees = COMMIT_SLAB_INIT(1, indegrees);
+ *
+ */
+#define COMMIT_SLAB_INIT(stride, var) { \
+ COMMIT_SLAB_SIZE / sizeof(**((var).slab)) / (stride), \
+ (stride), 0, NULL \
+}
+
#endif /* COMMIT_SLAB_H */
--
2.0.0.729.g520999f
--
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 14/15] commit: convert commit-buffer to a slab
This will make it easier to manage the buffer cache
independently of the struct commit objects. It also
shrinks struct commit by one pointer, which may be
helpful.
Unfortunately it does not reduce the max memory size of
something like rev-list, because rev-list uses
get_cached_commit_buffer() to decide not to show each
commit's output (and due to the design of slab_at, accessing
the slab requires us to extend it, allocating exactly the
same number of buffer pointers we dropped from the commit
structs).
Signed-off-by: Jeff King p...@peff.net
---
commit.c | 20 +---
commit.h | 1 -
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/commit.c b/commit.c
index d00b818..1b344bd 100644
--- a/commit.c
+++ b/commit.c
@@ -245,14 +245,17 @@ int unregister_shallow(const unsigned char *sha1)
return 0;
}
+define_commit_slab(buffer_slab, void *);
+static struct buffer_slab buffer_slab = COMMIT_SLAB_INIT(1, buffer_slab);
+
void set_commit_buffer(struct commit *commit, void *buffer)
{
- commit-buffer = buffer;
+ *buffer_slab_at(buffer_slab, commit) = buffer;
}
const void *get_cached_commit_buffer(const struct commit *commit)
{
- return commit-buffer;
+ return *buffer_slab_at(buffer_slab, commit);
}
const void *get_commit_buffer(const struct commit *commit)
@@ -274,20 +277,23 @@ const void *get_commit_buffer(const struct commit *commit)
void unuse_commit_buffer(const struct commit *commit, const void *buffer)
{
- if (commit-buffer != buffer)
+ void *cached = *buffer_slab_at(buffer_slab, commit);
+ if (cached != buffer)
free((void *)buffer);
}
void free_commit_buffer(struct commit *commit)
{
- free(commit-buffer);
- commit-buffer = NULL;
+ void **b = buffer_slab_at(buffer_slab, commit);
+ free(*b);
+ *b = NULL;
}
const void *detach_commit_buffer(struct commit *commit)
{
- void *ret = commit-buffer;
- commit-buffer = NULL;
+ void **b = buffer_slab_at(buffer_slab, commit);
+ void *ret = *b;
+ *b = NULL;
return ret;
}
diff --git a/commit.h b/commit.h
index 27e4fd7..02f894b 100644
--- a/commit.h
+++ b/commit.h
@@ -20,7 +20,6 @@ struct commit {
unsigned long date;
struct commit_list *parents;
struct tree *tree;
- char *buffer;
};
extern int save_commit_buffer;
--
2.0.0.729.g520999f
--
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 15/15] commit: record buffer length in cache
Most callsites which use the commit buffer try to use the
cached version attached to the commit, rather than
re-reading from disk. Unfortunately, that interface provides
only a pointer to the NUL-terminated buffer, with no
indication of the original length.
For the most part, this doesn't matter. People do not put
NULs in their commit messages, and the log code is happy to
treat it all as a NUL-terminated string. However, some code
paths do care. For example, when checking signatures, we
want to be very careful that we verify all the bytes to
avoid malicious trickery.
This patch just adds an optional size out-pointer to
get_commit_buffer and friends. The existing callers all pass
NULL (there did not seem to be any obvious sites where we
could avoid an immediate strlen() call, though perhaps with
some further refactoring we could).
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 2 +-
builtin/fast-export.c | 2 +-
builtin/fmt-merge-msg.c | 2 +-
builtin/index-pack.c| 2 +-
builtin/log.c | 2 +-
builtin/rev-list.c | 2 +-
commit.c| 54 -
commit.h| 8
fsck.c | 2 +-
log-tree.c | 2 +-
merge-recursive.c | 2 +-
notes-merge.c | 2 +-
object.c| 4 ++--
pretty.c| 4 ++--
sequencer.c | 2 +-
sha1_name.c | 2 +-
16 files changed, 56 insertions(+), 38 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index cde19eb..58a2c54 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)));
- set_commit_buffer(commit, strbuf_detach(msg, NULL));
+ set_commit_buffer(commit, msg.buf, msg.len);
if (!contents_from || strcmp(-, contents_from)) {
struct stat st;
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7ee5e08..05d161f 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -289,7 +289,7 @@ static void handle_commit(struct commit *commit, struct
rev_info *rev)
rev-diffopt.output_format = DIFF_FORMAT_CALLBACK;
parse_commit_or_die(commit);
- commit_buffer = get_commit_buffer(commit);
+ commit_buffer = get_commit_buffer(commit, NULL);
author = strstr(commit_buffer, \nauthor );
if (!author)
die (Could not find author in commit %s,
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 01f6d59..ef8b254 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -236,7 +236,7 @@ static void record_person(int which, struct string_list
*people,
const char *field;
field = (which == 'a') ? \nauthor : \ncommitter ;
- buffer = get_commit_buffer(commit);
+ buffer = get_commit_buffer(commit, NULL);
name = strstr(buffer, field);
if (!name)
return;
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 995df39..25e3e9b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -786,7 +786,7 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
}
if (obj-type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
- detach_commit_buffer(commit);
+ detach_commit_buffer(commit, NULL);
}
obj-flags |= FLAG_CHECKED;
}
diff --git a/builtin/log.c b/builtin/log.c
index 2c74260..c599eac 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -919,7 +919,7 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
need_8bit_cte);
for (i = 0; !need_8bit_cte i nr; i++) {
- const char *buf = get_commit_buffer(list[i]);
+ const char *buf = get_commit_buffer(list[i], NULL);
if (has_non_ascii(buf))
need_8bit_cte = 1;
unuse_commit_buffer(list[i], buf);
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3fcbf21..ff84a82 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -106,7 +106,7 @@ static void show_commit(struct commit *commit, void *data)
else
putchar('\n');
- if (revs-verbose_header get_cached_commit_buffer(commit)) {
+ if (revs-verbose_header get_cached_commit_buffer(commit, NULL)) {
struct strbuf buf = STRBUF_INIT;
struct pretty_print_context ctx = {0};
ctx.abbrev = revs-abbrev;
diff --git a/commit.c b/commit.c
index 1b344bd..b833b9f 100644
---
Re: What's cooking in git.git (Jun 2014, #02; Fri, 6)
David Kastrup d...@gnu.org writes: Junio C Hamano gits...@pobox.com writes: git blame has been optimized greatly by reorganising the data structure that is used to keep track of the work to be done, thanks to David Karstrup d...@gnu.org. I guess that reorganising the data structure for months is not worth the trouble of getting the name right. We do not usually name people in particular in the release notes and/or the What's cooking report, and trying to do something unusual misfired X-. Sorry about misspelling your name. -- 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] t5551: fix the 50,000 tag test
Torsten Bögershausen tbo...@web.de writes: The first version of test 23 did simply check that no output was send to stderr. Commit 5e2c7cd2 verified that the expected tags were actually cloned. Since the day git clone printed Cloning into 'too-many-refs' to stderr, Thanks. It is 68b939b2 (clone: send diagnostic messages to stderr, 2013-09-18); before it we showed the message to the standard output stream instead. Will queue. the test failed because stderr was not empty. Remove the check for stderr and make t5551-23 pass again Signed-off-by: Torsten Bögershausen tbo...@web.de --- t/t5551-http-fetch-smart.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index e07eaf3..2c49133 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -240,8 +240,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' ' ' test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command line overflow' ' - git clone $HTTPD_URL/smart/repo.git too-many-refs 2err - test_line_count = 0 err + git clone $HTTPD_URL/smart/repo.git too-many-refs ( cd too-many-refs test $(git for-each-ref refs/tags | wc -l) = 5 -- 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 v1 2/5] add strbuf_set operations documentation
Eric, On Mon, Jun 09, 2014 at 05:53:49AM -0400, Eric Sunshine wrote: On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler jmmah...@gmail.com wrote: Add documentation of the strbuf_set operations to technical/api-strbuf.txt. Since this patch is concise and so closely related to patch 1/5, it probably should be squashed into that one. Fixed. More below. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- Documentation/technical/api-strbuf.txt | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt index 077a709..ab430d9 100644 --- a/Documentation/technical/api-strbuf.txt +++ b/Documentation/technical/api-strbuf.txt @@ -149,6 +149,24 @@ Functions than zero if the first buffer is found, respectively, to be less than, to match, or be greater than the second buffer. +* Setting the buffer + +`strbuf_set`:: + +Set the buffer to some data up to a given length. I personally find this slightly ambiguous. Upon reading it, the first question that pops into my mind is whether or not the existing strbuf content is replaced (even though set should imply that it is). I wonder if it would make sense to rewrite as: Set the buffer to [...], replacing the old content of the buffer. Alternately: Replace the buffer content with [...]. On a second reading, I agree that it is ambigous. 'Replace' is much more clear. Great suggestion. Ditto for the others. +`strbuf_setstr`:: + + Set the buffer to a NUL-terminated string. + +`strbuf_setf`:: + + Set the buffer to a formatted string. + +`strbuf_setbuf`:: + + Set the current buffer to the contents of some other buffer. + * Adding data to the buffer NOTE: All of the functions in this section will grow the buffer as necessary. -- -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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 v1 4/5] fast-import.c: cleanup using strbuf_set operations
Eric, On Mon, Jun 09, 2014 at 06:12:12AM -0400, Eric Sunshine wrote: On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler jmmah...@gmail.com wrote: Subject: fast-import.c: cleanup using strbuf_set operations ... Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- fast-import.c | 19 ++- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/fast-import.c b/fast-import.c index e8ec34d..c23935c 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2741,8 +2741,7 @@ static void parse_new_commit(void) hashcpy(b-branch_tree.versions[0].sha1, b-branch_tree.versions[1].sha1); - strbuf_reset(new_data); - strbuf_addf(new_data, tree %s\n, + strbuf_setf(new_data, tree %s\n, sha1_to_hex(b-branch_tree.versions[1].sha1)); if (!is_null_sha1(b-sha1)) strbuf_addf(new_data, parent %s\n, sha1_to_hex(b-sha1)); Unlike the cases in patches 3/5 and 5/5 where the strbuf is used or returned immediately following the strbuf_set() call, I am not convinced that this change is an improvement. This code has the general form: strbuf_reset(...); strbuf_add(...); if (condition) strbuf_add(...); strbuf_add(...); in which it is clear that the string is being built piecemeal, and it's easy for a programmer to insert, remove, or re-order strbuf_add() calls. Replacing the first two lines with strbuf_set() somewhat obscures the fact that the string is going to be built up piecemeal. Plus, the change makes it more difficult to insert, remove, or re-order the strbuf_add() invocations. This isn't a strong objection, but the benefit of the change seems minimal or non-existent. Ditto for several remaining cases in this patch. ... This is a great observation that I certainly did overlook. Using strbuf_add or strbuf_set to help make it more obvious what the code is doing. By the same token, strbuf_set can be used to replace strbuf_add to make it clear that nothing important was being added to and that the entire buffer is being replaced. struct strbuf mybuf = STRBUF_INIT; strbuf_add(mybuf, ...); /* Was something there before? */ strbuf_set(mybuf, ...); /* Replace everything. */ -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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 v1 0/5] add strbuf_set operations
Duy, On Mon, Jun 09, 2014 at 05:39:12PM +0700, Duy Nguyen wrote: On Mon, Jun 9, 2014 at 3:36 PM, Jeremiah Mahler jmmah...@gmail.com wrote: Currently, the data in a strbuf is modified using add operations. To set the buffer to some data a reset must be performed before an add. strbuf_reset(buf); strbuf_add(buf, cb.buf.buf, cb.buf.len); And this is a common sequence of operations with 70 occurrences found in the current source code. This includes all the different variations (add, addf, addstr, addbuf, addch). FILES=`find ./ -name '*.c'` CNT=$(pcregrep -M strbuf_reset.*\n.*strbuf_add $FILES | wc -l) Hmm.. I wonder if git-grep could do this.. There's pcre support but I never tried. Not sure if git-grep does this. The multi-line (-M) support was the thing I needed. CNT=$(echo $CNT / 2 | bc) echo $CNT 70 The change in this series looks nice. There's another pattern, save strbuf length, then strbuf_setlen() at the beginning or the end of a loop. But I think it's less often. A quick look did not see any obvious patterns for this. I think you are right, there may be fewer cases. -- Duy -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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 0/1] refs.c: SSE4.2 optimizations for check_refname_component
David Turner dtur...@twopensource.com writes: Since Junio has picked up the first patch from previous versions of this series, I'm just going to send the second (SSE) one. I decided not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses the former convention (for instance, that's what GIT_PARSE_WITH generates). Yeah but NO_FROTZ is used only when FROTZ is something everybody is expected to have (e.g. it's in posix, people ought to have it, but we do support those who don't), isn't it? For a very arch specific stuff like sse42, I'd feel better to make it purely opt-in by forcing people to explicitly say HAVE_SSE42 to enable 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
[PATCH v2 00/19] add strbuf_set operations
Version 2 of the patch set to add strbuf_set operations. Includes suggestions from Eric Sunshine [1]: - New operations and their documentation placed in one patch. - Less ambiguous documentation: Replace the buffer content with [...] - Use imperative mood in log messages. - Don't use strbuf_set operations in cases where strbuf_add is used to build up a buffer in multiple steps. Multiple adds suggest that re-ordering is possible whereas a strbuf_set at the beginning suggests that it isn't. This is confusing. Using strbuf_set before a sequence of adds can be confusing. But using strbuf_add when nothing important was being added to can also be confusing. strbuf_set can be used to make these cases more clear. struct strbuf mybuf = STRBUF_INIT; strbuf_add(mybuf, ...); /* Was something there before? */ strbuf_set(mybuf, ...); /* Replace everything. */ Several single line replacements were made for this reason. Additional files have also been converted compared to version 1 [1]. [1]: http://marc.info/?l=gitm=140230874124060w=2 Jeremiah Mahler (19): add strbuf_set operations sha1_name: simplify via strbuf_set() fast-import: simplify via strbuf_set() builtin/remote: simplify via strbuf_set() branch: simplify via strbuf_set() builtin/branch: simplify via strbuf_set() builtin/checkout: simplify via strbuf_set() builtin/mailinfo: simplify via strbuf_set() builtin/tag: simplify via strbuf_set() date: simplify via strbuf_set() diffcore-order: simplify via strbuf_set() http-backend: simplify via strbuf_set() http: simplify via strbuf_set() ident: simplify via strbuf_set() remote-curl: simplify via strbuf_set() submodule: simplify via strbuf_set() transport: simplify via strbuf_set() vcs-svn/svndump: simplify via strbuf_set() wt-status: simplify via strbuf_set() Documentation/technical/api-strbuf.txt | 18 +++ branch.c | 6 ++-- builtin/branch.c | 3 +- builtin/checkout.c | 18 --- builtin/mailinfo.c | 18 --- builtin/remote.c | 59 -- builtin/tag.c | 3 +- date.c | 3 +- diffcore-order.c | 3 +- fast-import.c | 6 ++-- http-backend.c | 6 ++-- http.c | 3 +- ident.c| 6 ++-- remote-curl.c | 3 +- sha1_name.c| 15 +++-- strbuf.c | 21 strbuf.h | 14 submodule.c| 5 ++- transport.c| 3 +- vcs-svn/svndump.c | 3 +- wt-status.c| 3 +- 21 files changed, 110 insertions(+), 109 deletions(-) -- 2.0.0.592.gf55b190 -- 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 01/19] add strbuf_set operations
Currently, the data in a strbuf is modified using add operations. To
set the buffer to some data a reset must be performed before an add.
strbuf_reset(buf);
strbuf_add(buf, cb.buf.buf, cb.buf.len);
And this is a common sequence of operations with 70 occurrences found in
the current source code. This includes all the different variations
(add, addf, addstr, addbuf, addch).
FILES=`find ./ -name '*.c'`
CNT=$(pcregrep -M strbuf_reset.*\n.*strbuf_add $FILES | wc -l)
CNT=$(echo $CNT / 2 | bc)
echo $CNT
70
These patches add strbuf_set operations which allow this common sequence
to be performed in one line instead of two.
strbuf_set(buf, cb.buf.buf, cb.buf.len);
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
Documentation/technical/api-strbuf.txt | 18 ++
strbuf.c | 21 +
strbuf.h | 14 ++
3 files changed, 53 insertions(+)
diff --git a/Documentation/technical/api-strbuf.txt
b/Documentation/technical/api-strbuf.txt
index 077a709..b7e23da 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -149,6 +149,24 @@ Functions
than zero if the first buffer is found, respectively, to be less than,
to match, or be greater than the second buffer.
+* Setting the buffer
+
+`strbuf_set`::
+
+ Replace the buffer content with data of a given length.
+
+`strbuf_setstr`::
+
+ Replace the buffer content with data from a NUL-terminated string.
+
+`strbuf_setf`::
+
+ Replace the buffer content with a formatted string.
+
+`strbuf_setbuf`::
+
+ Replace the buffer content with data from another buffer.
+
* Adding data to the buffer
NOTE: All of the functions in this section will grow the buffer as necessary.
diff --git a/strbuf.c b/strbuf.c
index ac62982..9d64b00 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t
len,
strbuf_setlen(sb, sb-len + dlen - len);
}
+void strbuf_set(struct strbuf *sb, const void *data, size_t len)
+{
+ strbuf_reset(sb);
+ strbuf_add(sb, data, len);
+}
+
+void strbuf_setf(struct strbuf *sb, const char *fmt, ...)
+{
+ va_list ap;
+ strbuf_reset(sb);
+ va_start(ap, fmt);
+ strbuf_vaddf(sb, fmt, ap);
+ va_end(ap);
+}
+
+void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)
+{
+ strbuf_reset(sb);
+ strbuf_add(sb, sb2-buf, sb2-len);
+}
+
void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
{
strbuf_splice(sb, pos, 0, data, len);
diff --git a/strbuf.h b/strbuf.h
index e9ad03e..b339f08 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -101,6 +101,20 @@ static inline struct strbuf **strbuf_split(const struct
strbuf *sb,
*/
extern void strbuf_list_free(struct strbuf **);
+/*- set buffer to data -*/
+
+extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);
+
+static inline void strbuf_setstr(struct strbuf *sb, const char *s)
+{
+ strbuf_set(sb, s, strlen(s));
+}
+
+__attribute__((format (printf,2,3)))
+extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);
+
+extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);
+
/*- add data in your buffer -*/
static inline void strbuf_addch(struct strbuf *sb, int c)
{
--
2.0.0.592.gf55b190
--
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 07/19] builtin/checkout: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
builtin/checkout.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9cbe7d1..38fc0ce 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -912,8 +912,7 @@ static int prepare_linked_checkout(const struct
checkout_opts *opts,
sb_git.buf);
junk_work_tree = path;
- strbuf_reset(sb);
- strbuf_addf(sb, %s/gitdir, sb_repo.buf);
+ strbuf_setf(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);
@@ -923,11 +922,9 @@ 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);
+ strbuf_setf(sb, %s/HEAD, sb_repo.buf);
write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1));
- strbuf_reset(sb);
- strbuf_addf(sb, %s/commondir, sb_repo.buf);
+ strbuf_setf(sb, %s/commondir, sb_repo.buf);
write_file(sb.buf, 1, ../..\n);
if (!opts-quiet)
@@ -942,8 +939,7 @@ 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);
+ strbuf_setf(sb, %s/locked, sb_repo.buf);
unlink_or_warn(sb.buf);
strbuf_release(sb);
strbuf_release(sb_repo);
@@ -1048,8 +1044,7 @@ static void check_linked_checkouts(struct branch_info
*new)
return;
}
- strbuf_reset(path);
- strbuf_addf(path, %s/HEAD, get_git_common_dir());
+ strbuf_setf(path, %s/HEAD, get_git_common_dir());
/*
* $GIT_COMMON_DIR/HEAD is practically outside
* $GIT_DIR so resolve_ref_unsafe() won't work (it
@@ -1064,8 +1059,7 @@ static void check_linked_checkouts(struct branch_info
*new)
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,
+ strbuf_setf(path, %s/repos/%s/HEAD,
get_git_common_dir(), d-d_name);
if (check_linked_checkout(new, d-d_name, path.buf))
break;
--
2.0.0.592.gf55b190
--
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 05/19] branch: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
branch.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/branch.c b/branch.c
index 660097b..bc7cc7e 100644
--- a/branch.c
+++ b/branch.c
@@ -65,13 +65,11 @@ void install_branch_config(int flag, const char *local,
const char *origin, cons
strbuf_addf(key, branch.%s.remote, local);
git_config_set(key.buf, origin ? origin : .);
- strbuf_reset(key);
- strbuf_addf(key, branch.%s.merge, local);
+ strbuf_setf(key, branch.%s.merge, local);
git_config_set(key.buf, remote);
if (rebasing) {
- strbuf_reset(key);
- strbuf_addf(key, branch.%s.rebase, local);
+ strbuf_setf(key, branch.%s.rebase, local);
git_config_set(key.buf, true);
}
strbuf_release(key);
--
2.0.0.592.gf55b190
--
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 06/19] builtin/branch: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
builtin/branch.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2d1c57c..ad641b6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -984,8 +984,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
strbuf_addf(buf, branch.%s.remote, branch-name);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
- strbuf_reset(buf);
- strbuf_addf(buf, branch.%s.merge, branch-name);
+ strbuf_setf(buf, branch.%s.merge, branch-name);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_release(buf);
} else if (argc 0 argc = 2) {
--
2.0.0.592.gf55b190
--
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 02/19] sha1_name: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
sha1_name.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 2b6322f..f88b66c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -920,8 +920,7 @@ static int grab_nth_branch_switch(unsigned char *osha1,
unsigned char *nsha1,
return 0;
if (--(cb-remaining) == 0) {
len = target - match;
- strbuf_reset(cb-buf);
- strbuf_add(cb-buf, match, len);
+ strbuf_set(cb-buf, match, len);
return 1; /* we are done */
}
return 0;
@@ -957,8 +956,7 @@ static int interpret_nth_prior_checkout(const char *name,
int namelen,
retval = 0;
if (0 for_each_reflog_ent_reverse(HEAD, grab_nth_branch_switch,
cb)) {
- strbuf_reset(buf);
- strbuf_add(buf, cb.buf.buf, cb.buf.len);
+ strbuf_set(buf, cb.buf.buf, cb.buf.len);
retval = brace - name + 1;
}
@@ -1025,8 +1023,7 @@ static int interpret_empty_at(const char *name, int
namelen, int len, struct str
if (next != name + 1)
return -1;
- strbuf_reset(buf);
- strbuf_add(buf, HEAD, 4);
+ strbuf_set(buf, HEAD, 4);
return 1;
}
@@ -1044,8 +1041,7 @@ static int reinterpret(const char *name, int namelen, int
len, struct strbuf *bu
strbuf_setlen(buf, used);
return len;
}
- strbuf_reset(buf);
- strbuf_addbuf(buf, tmp);
+ strbuf_setbuf(buf, tmp);
strbuf_release(tmp);
/* tweak for size of {-N} versus expanded ref name */
return ret - used + len;
@@ -1054,8 +1050,7 @@ static int reinterpret(const char *name, int namelen, int
len, struct strbuf *bu
static void set_shortened_ref(struct strbuf *buf, const char *ref)
{
char *s = shorten_unambiguous_ref(ref, 0);
- strbuf_reset(buf);
- strbuf_addstr(buf, s);
+ strbuf_setstr(buf, s);
free(s);
}
--
2.0.0.592.gf55b190
--
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 04/19] builtin/remote: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
builtin/remote.c | 59
1 file changed, 21 insertions(+), 38 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index c9b67ff..f2d809d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -193,8 +193,7 @@ static int add(int argc, const char **argv)
return 1;
if (!mirror || mirror MIRROR_FETCH) {
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.fetch, name);
+ strbuf_setf(buf, remote.%s.fetch, name);
if (track.nr == 0)
string_list_append(track, *);
for (i = 0; i track.nr; i++) {
@@ -205,15 +204,13 @@ static int add(int argc, const char **argv)
}
if (mirror MIRROR_PUSH) {
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.mirror, name);
+ strbuf_setf(buf, remote.%s.mirror, name);
if (git_config_set(buf.buf, true))
return 1;
}
if (fetch_tags != TAGS_DEFAULT) {
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.tagopt, name);
+ strbuf_setf(buf, remote.%s.tagopt, name);
if (git_config_set(buf.buf,
fetch_tags == TAGS_SET ? --tags : --no-tags))
return 1;
@@ -223,11 +220,9 @@ static int add(int argc, const char **argv)
return 1;
if (master) {
- strbuf_reset(buf);
- strbuf_addf(buf, refs/remotes/%s/HEAD, name);
+ strbuf_setf(buf, refs/remotes/%s/HEAD, name);
- strbuf_reset(buf2);
- strbuf_addf(buf2, refs/remotes/%s/%s, name, master);
+ strbuf_setf(buf2, refs/remotes/%s/%s, name, master);
if (create_symref(buf.buf, buf2.buf, remote add))
return error(_(Could not setup master '%s'), master);
@@ -584,19 +579,17 @@ static int migrate_file(struct remote *remote)
int i;
const char *path = NULL;
- strbuf_addf(buf, remote.%s.url, remote-name);
+ strbuf_setf(buf, remote.%s.url, remote-name);
for (i = 0; i remote-url_nr; i++)
if (git_config_set_multivar(buf.buf, remote-url[i], ^$, 0))
return error(_(Could not append '%s' to '%s'),
remote-url[i], buf.buf);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.push, remote-name);
+ strbuf_setf(buf, remote.%s.push, remote-name);
for (i = 0; i remote-push_refspec_nr; i++)
if (git_config_set_multivar(buf.buf, remote-push_refspec[i],
^$, 0))
return error(_(Could not append '%s' to '%s'),
remote-push_refspec[i], buf.buf);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.fetch, remote-name);
+ strbuf_setf(buf, remote.%s.fetch, remote-name);
for (i = 0; i remote-fetch_refspec_nr; i++)
if (git_config_set_multivar(buf.buf, remote-fetch_refspec[i],
^$, 0))
return error(_(Could not append '%s' to '%s'),
@@ -640,27 +633,24 @@ static int mv(int argc, const char **argv)
if (newremote (newremote-url_nr 1 || newremote-fetch_refspec_nr))
die(_(remote %s already exists.), rename.new);
- strbuf_addf(buf, refs/heads/test:refs/remotes/%s/test, rename.new);
+ strbuf_setf(buf, refs/heads/test:refs/remotes/%s/test, rename.new);
if (!valid_fetch_refspec(buf.buf))
die(_('%s' is not a valid remote name), rename.new);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s, rename.old);
- strbuf_addf(buf2, remote.%s, rename.new);
+ strbuf_setf(buf, remote.%s, rename.old);
+ strbuf_setf(buf2, remote.%s, rename.new);
if (git_config_rename_section(buf.buf, buf2.buf) 1)
return error(_(Could not rename config section '%s' to '%s'),
buf.buf, buf2.buf);
- strbuf_reset(buf);
- strbuf_addf(buf, remote.%s.fetch, rename.new);
+ strbuf_setf(buf, remote.%s.fetch, rename.new);
if (git_config_set_multivar(buf.buf, NULL, NULL, 1))
return error(_(Could not remove config section '%s'),
buf.buf);
- strbuf_addf(old_remote_context, :refs/remotes/%s/, rename.old);
+ strbuf_setf(old_remote_context, :refs/remotes/%s/, rename.old);
for (i = 0; i oldremote-fetch_refspec_nr; i++) {
char *ptr;
- strbuf_reset(buf2);
- strbuf_addstr(buf2, oldremote-fetch_refspec[i]);
+ strbuf_setstr(buf2, oldremote-fetch_refspec[i]);
ptr =
[PATCH v2 03/19] fast-import: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
fast-import.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index e8ec34d..cfe9404 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2898,8 +2898,7 @@ static void cat_blob(struct object_entry *oe, unsigned
char sha1[20])
* Output based on batch_one_object() from cat-file.c.
*/
if (type = 0) {
- strbuf_reset(line);
- strbuf_addf(line, %s missing\n, sha1_to_hex(sha1));
+ strbuf_setf(line, %s missing\n, sha1_to_hex(sha1));
cat_blob_write(line.buf, line.len);
strbuf_release(line);
free(buf);
@@ -2910,8 +2909,7 @@ static void cat_blob(struct object_entry *oe, unsigned
char sha1[20])
if (type != OBJ_BLOB)
die(Object %s is a %s but a blob was expected.,
sha1_to_hex(sha1), typename(type));
- strbuf_reset(line);
- strbuf_addf(line, %s %s %lu\n, sha1_to_hex(sha1),
+ strbuf_setf(line, %s %s %lu\n, sha1_to_hex(sha1),
typename(type), size);
cat_blob_write(line.buf, line.len);
strbuf_release(line);
--
2.0.0.592.gf55b190
--
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 10/19] date: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
date.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/date.c b/date.c
index 782de95..0b723a4 100644
--- a/date.c
+++ b/date.c
@@ -166,8 +166,7 @@ const char *show_date(unsigned long time, int tz, enum
date_mode mode)
static struct strbuf timebuf = STRBUF_INIT;
if (mode == DATE_RAW) {
- strbuf_reset(timebuf);
- strbuf_addf(timebuf, %lu %+05d, time, tz);
+ strbuf_setf(timebuf, %lu %+05d, time, tz);
return timebuf.buf;
}
--
2.0.0.592.gf55b190
--
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 12/19] http-backend: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- http-backend.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/http-backend.c b/http-backend.c index d2c0a62..25c7435 100644 --- a/http-backend.c +++ b/http-backend.c @@ -489,14 +489,12 @@ static void service_rpc(char *service_name) struct rpc_service *svc = select_service(service_name); struct strbuf buf = STRBUF_INIT; - strbuf_reset(buf); - strbuf_addf(buf, application/x-git-%s-request, svc-name); + strbuf_setf(buf, application/x-git-%s-request, svc-name); check_content_type(buf.buf); hdr_nocache(); - strbuf_reset(buf); - strbuf_addf(buf, application/x-git-%s-result, svc-name); + strbuf_setf(buf, application/x-git-%s-result, svc-name); hdr_str(content_type, buf.buf); end_headers(); -- 2.0.0.592.gf55b190 -- 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 11/19] diffcore-order: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
diffcore-order.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/diffcore-order.c b/diffcore-order.c
index 97dd3d0..5a93971 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -63,8 +63,7 @@ static int match_order(const char *path)
static struct strbuf p = STRBUF_INIT;
for (i = 0; i order_cnt; i++) {
- strbuf_reset(p);
- strbuf_addstr(p, path);
+ strbuf_setstr(p, path);
while (p.buf[0]) {
char *cp;
if (!wildmatch(order[i], p.buf, 0, NULL))
--
2.0.0.592.gf55b190
--
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 13/19] http: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- http.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/http.c b/http.c index 2b4f6a3..626fed7 100644 --- a/http.c +++ b/http.c @@ -1098,8 +1098,7 @@ static int update_url_from_redirect(struct strbuf *base, strcmp(tail, got-buf + got-len - tail_len)) return 0; /* insane redirect scheme */ - strbuf_reset(base); - strbuf_add(base, got-buf, got-len - tail_len); + strbuf_set(base, got-buf, got-len - tail_len); return 1; } -- 2.0.0.592.gf55b190 -- 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 08/19] builtin/mailinfo: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- builtin/mailinfo.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c index cf11c8d..596071e 100644 --- a/builtin/mailinfo.c +++ b/builtin/mailinfo.c @@ -40,8 +40,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf *name, struct strbuf src = email; else if (name == out) return; - strbuf_reset(out); - strbuf_addbuf(out, src); + strbuf_setbuf(out, src); } static void parse_bogus_from(const struct strbuf *line) @@ -62,11 +61,9 @@ static void parse_bogus_from(const struct strbuf *line) if (!ket) return; - strbuf_reset(email); - strbuf_add(email, bra + 1, ket - bra - 1); + strbuf_set(email, bra + 1, ket - bra - 1); - strbuf_reset(name); - strbuf_add(name, line-buf, bra - line-buf); + strbuf_set(name, line-buf, bra - line-buf); strbuf_trim(name); get_sane_name(name, name, email); } @@ -108,8 +105,7 @@ static void handle_from(const struct strbuf *from) at--; } el = strcspn(at, \n\t\r\v\f); - strbuf_reset(email); - strbuf_add(email, at, el); + strbuf_set(email, at, el); strbuf_remove(f, at - f.buf, el + (at[el] ? 1 : 0)); /* The remainder is name. It could be @@ -567,8 +563,7 @@ static int decode_header_bq(struct strbuf *it) in = ep + 2; } strbuf_addstr(outbuf, in); - strbuf_reset(it); - strbuf_addbuf(it, outbuf); + strbuf_setbuf(it, outbuf); decode_header_bq_out: strbuf_release(outbuf); strbuf_release(charset_q); @@ -602,8 +597,7 @@ static void decode_transfer_encoding(struct strbuf *line) default: return; } - strbuf_reset(line); - strbuf_addbuf(line, ret); + strbuf_setbuf(line, ret); strbuf_release(ret); free(ret); } -- 2.0.0.592.gf55b190 -- 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 09/19] builtin/tag: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- builtin/tag.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index c6e8a71..b545c21 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -496,8 +496,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name) if (name[0] == '-') return -1; - strbuf_reset(sb); - strbuf_addf(sb, refs/tags/%s, name); + strbuf_setf(sb, refs/tags/%s, name); return check_refname_format(sb-buf, 0); } -- 2.0.0.592.gf55b190 -- 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 19/19] wt-status: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
wt-status.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 318a191..a89cd73 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1168,8 +1168,7 @@ static char *read_and_strip_branch(const char *path)
else if (!get_sha1_hex(sb.buf, sha1)) {
const char *abbrev;
abbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);
- strbuf_reset(sb);
- strbuf_addstr(sb, abbrev);
+ strbuf_setstr(sb, abbrev);
} else if (!strcmp(sb.buf, detached HEAD)) /* rebase */
goto got_nothing;
else/* bisect */
--
2.0.0.592.gf55b190
--
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 14/19] ident: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a
strbuf_add by using strbuf_set operations.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
ident.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/ident.c b/ident.c
index 1d9b6e7..523e249 100644
--- a/ident.c
+++ b/ident.c
@@ -397,8 +397,7 @@ int git_ident_config(const char *var, const char *value,
void *data)
if (!strcmp(var, user.name)) {
if (!value)
return config_error_nonbool(var);
- strbuf_reset(git_default_name);
- strbuf_addstr(git_default_name, value);
+ strbuf_setstr(git_default_name, value);
committer_ident_explicitly_given |= IDENT_NAME_GIVEN;
author_ident_explicitly_given |= IDENT_NAME_GIVEN;
return 0;
@@ -407,8 +406,7 @@ int git_ident_config(const char *var, const char *value,
void *data)
if (!strcmp(var, user.email)) {
if (!value)
return config_error_nonbool(var);
- strbuf_reset(git_default_email);
- strbuf_addstr(git_default_email, value);
+ strbuf_setstr(git_default_email, value);
committer_ident_explicitly_given |= IDENT_MAIL_GIVEN;
author_ident_explicitly_given |= IDENT_MAIL_GIVEN;
return 0;
--
2.0.0.592.gf55b190
--
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 16/19] submodule: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- submodule.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/submodule.c b/submodule.c index 3402af6..878cc48 100644 --- a/submodule.c +++ b/submodule.c @@ -1324,13 +1324,12 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir) const char *real_work_tree = xstrdup(real_path(work_tree)); /* Update gitfile */ - strbuf_addf(file_name, %s/.git, work_tree); + strbuf_setf(file_name, %s/.git, work_tree); write_file(file_name.buf, 1, gitdir: %s\n, relative_path(real_git_dir, real_work_tree, rel_path)); /* Update core.worktree setting */ - strbuf_reset(file_name); - strbuf_addf(file_name, %s/config, real_git_dir); + strbuf_setf(file_name, %s/config, real_git_dir); if (git_config_set_in_file(file_name.buf, core.worktree, relative_path(real_work_tree, real_git_dir, rel_path))) -- 2.0.0.592.gf55b190 -- 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 18/19] vcs-svn/svndump: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- vcs-svn/svndump.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index 31d1d83..7fbf5d8 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -352,8 +352,7 @@ void svndump_read(const char *url, const char *local_ref, const char *notes_ref) case sizeof(UUID): if (constcmp(t, UUID)) continue; - strbuf_reset(dump_ctx.uuid); - strbuf_addstr(dump_ctx.uuid, val); + strbuf_setstr(dump_ctx.uuid, val); break; case sizeof(Revision-number): if (constcmp(t, Revision-number)) -- 2.0.0.592.gf55b190 -- 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 15/19] remote-curl: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- remote-curl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 4493b38..49d27f5 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -288,8 +288,7 @@ static struct discovery* discover_refs(const char *service, int for_push) */ line = packet_read_line_buf(last-buf, last-len, NULL); - strbuf_reset(exp); - strbuf_addf(exp, # service=%s, service); + strbuf_setf(exp, # service=%s, service); if (strcmp(line, exp.buf)) die(invalid server response; got '%s', line); strbuf_release(exp); -- 2.0.0.592.gf55b190 -- 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 17/19] transport: simplify via strbuf_set()
Simplify cases where a strbuf_reset is immediately followed by a strbuf_add by using strbuf_set operations. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- transport.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/transport.c b/transport.c index 172b3d8..e8f5dfa 100644 --- a/transport.c +++ b/transport.c @@ -1092,8 +1092,7 @@ static int run_pre_push_hook(struct transport *transport, if (r-status == REF_STATUS_REJECT_STALE) continue; if (r-status == REF_STATUS_UPTODATE) continue; - strbuf_reset(buf); - strbuf_addf( buf, %s %s %s %s\n, + strbuf_setf( buf, %s %s %s %s\n, r-peer_ref-name, sha1_to_hex(r-new_sha1), r-name, sha1_to_hex(r-old_sha1)); -- 2.0.0.592.gf55b190 -- 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 0/1] refs.c: SSE4.2 optimizations for check_refname_component
On Mon, 2014-06-09 at 15:16 -0700, Junio C Hamano wrote: David Turner dtur...@twopensource.com writes: Since Junio has picked up the first patch from previous versions of this series, I'm just going to send the second (SSE) one. I decided not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses the former convention (for instance, that's what GIT_PARSE_WITH generates). Yeah but NO_FROTZ is used only when FROTZ is something everybody is expected to have (e.g. it's in posix, people ought to have it, but we do support those who don't), isn't it? For a very arch specific stuff like sse42, I'd feel better to make it purely opt-in by forcing people to explicitly say HAVE_SSE42 to enable it. The patch now has two kinds of autodetection: 1. At build-time, we check for the compiler supporting -msse4.2. If it does, and if the user has not explicitly done --without-sse, then we build with SSE support. This does not mean that the SSE code will necessarily be used because: 2. At run-time, if we have built with SSE support, we check cpuid to choose a version of the function that will run on the current CPU. So I think we never hit a case where we try to use SSE and fail, which is the major reason I see to make it non-default. To me, this means that we should not require people to explicitly request SSE, because we generally want to try to provide the most-efficient version of git that will work everywhere. In fact, I am not sure we need a --without-sse option at all, since all it saves is a cpuid instruction. But I don't need to remove the option, in case there's a use for it I'm not thinking of. -- 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 12/15] use get_commit_buffer everywhere
Jeff King p...@peff.net writes: diff --git a/notes-merge.c b/notes-merge.c index 94a1a8a..7885ab2 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -671,7 +671,8 @@ int notes_merge_commit(struct notes_merge_options *o, DIR *dir; struct dirent *e; struct strbuf path = STRBUF_INIT; - char *msg = strstr(partial_commit-buffer, \n\n); + const char *buffer = get_commit_buffer(partial_commit); + const char *msg = strstr(buffer, \n\n); This tightening causes... struct strbuf sb_msg = STRBUF_INIT; int baselen; @@ -720,6 +721,7 @@ int notes_merge_commit(struct notes_merge_options *o, } strbuf_attach(sb_msg, msg, strlen(msg), strlen(msg) + 1); ...a new error here: notes-merge.c:723:2: error: passing argument 2 of 'strbuf_attach' discards 'const' qualifier from pointer target type [-Werror] strbuf.h:19:13: note: expected 'void *' but argument is of type 'const char *' -- 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 0/1] refs.c: SSE4.2 optimizations for check_refname_component
Junio C Hamano gits...@pobox.com writes: David Turner dtur...@twopensource.com writes: Since Junio has picked up the first patch from previous versions of this series, I'm just going to send the second (SSE) one. I decided not to s/NO_SSE42/!HAVE_SSE42/ because it looks like git mostly uses the former convention (for instance, that's what GIT_PARSE_WITH generates). Yeah but NO_FROTZ is used only when FROTZ is something everybody is expected to have (e.g. it's in posix, people ought to have it, but we do support those who don't), isn't it? For a very arch specific stuff like sse42, I'd feel better to make it purely opt-in by forcing people to explicitly say HAVE_SSE42 to enable it. Just FYI: I am getting compat/cpuid.h:8:12: error: 'processor_supports_sse42' defined but not used [-Werror=unused-function] cc1: all warnings being treated as errors while building 'pu'; I'll have to rebuild 'pu' without this patch before I can push the day's result out. -- 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: Reset by checkout?
On 07/06/2014 17:52, Philip Oakley wrote:
Just to say there has been a similar confusion about 'git reset'
reported on the Git Users group for the case of reset with added
(staged), but uncommitted changes being wiped out, which simlarly
reports on the difficulty of explaining some of the conditions
especially when some are wrong ;-)
https://groups.google.com/forum/#!topic/git-users/27_FxIV_100
I'm coming around to the view that git reset mode should be (almost)
demoted to plumbing, leaving only the reset file that reverses add
file as everyday Porcelain.
I think reset --keep and --merge were a step in the wrong direction,
at least for the Porcelain - trying to make reset mode more useful,
rather than less necessary. Normal users shouldn't be needing to touch
these hard-to-explain-and-slightly-dangerous commands.
The addition of --abort to merge and other commands was much more
solid. They helped a lot, and I think we should follow that model by
adding --undo to various commands. That would mop up all the common
resets, in conjunction with Atsushi's proposed checkout -u
alternative to -B, which I quite like.
Main few:
commit --undo = reset --soft HEAD^
merge --undo = reset --keep HEAD^
rebase --undo = reset --keep ORIG_HEAD [bug report: rebase -p doesn't
set ORIG_HEAD reliably]
pull --undo = merge/rebase --undo depending on rebase settings [could we
go nuts and undo the fetch too?]
Bonus:
commit --amend --undo: reset --soft HEAD@{1}
The undos can also have a bit of extra veneer that checks the log/reflog
for whether it matches the proposed undo, and also checks the upstream
to see if the thing being undone is already public.
Given those, I honestly don't think I'd ever need to explain git reset
mode to anyone again. Which would be nice...
(Note I propose no --mixed equivalent for the commit undos, but it's
easy enough to follow the commit --undo with a normal git reset. I'd
rather re-document the normal git reset under commit --undo than add
and document yet another option).
Kevin
--
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 reset --hard with staged changes
2014-06-09 16:04 GMT+02:00 David Kastrup d...@gnu.org: Pierre-François CLEMENT lik...@gmail.com writes: Hi all, Someone pointed out on the Git for human beings Google group (https://groups.google.com/d/topic/git-users/27_FxIV_100/discussion) that using git-reset's hard mode when having staged untracked files simply deletes them from the working dir. Since git-reset specifically doesn't touch untracked files, one could expect having staged untracked files reset to their previous untracked state rather than being deleted. Could this be a bug or a missing feature? Or if it isn't, can someone explain what we got wrong? git reset --keep maybe? In a work dir and index without modifications, I expect git apply --index ... git reset --hard to remove any files that git apply created. It would not do so using your proposal. I agree that it seems a bit of a borderline, but I consider it better that once a file _is_ tracked, git reset --hard will first physically remove it before untracking it. -- David Kastrup Hm, I didn't think of git apply --index... Makes sense for this special use, but I'm not sure about the other use cases. Consider this scenario: You create a new (untracked) file. You use git-reset's hard mode to go one commit back, the new (untracked) file's still there. You add/stage that new file. You use git-reset's hard mode again to go one commit back, and the new untracked file you just staged gets deleted. Also, according to Git-scm (http://git-scm.com/book/en/Git-Basics-Recording-Changes-to-the-Repository): Tracked files are files that were in the last snapshot [...]. Untracked files are everything else. So it seems to me like staged untracked files shouldn't be considered as tracked files, and thus shouldn't be removed. Or maybe, git-reset's hard mode should always delete everything including untracked files? It would also make sense, given the numerous modes it has. -- Pierre-François CLEMENT Application developer at Upcast Social -- 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/7] test: turn USR_BIN_TIME into a lazy prerequisite
Two test scripts (t3302 and t3419) had copy paste code to set
USR_BIN_TIME prerequisite. Use the test_lazy_prereq helper to define
them in the common t/test-lib.sh.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t3302-notes-index-expensive.sh | 1 -
t/t3419-rebase-patch-id.sh | 1 -
t/test-lib.sh| 4
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index e35d781..dc706ab 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -9,7 +9,6 @@ test_description='Test commit notes index (expensive!)'
test_set_prereq NOT_EXPENSIVE
test -n $GIT_NOTES_TIMING_TESTS test_set_prereq EXPENSIVE
-test -x /usr/bin/time test_set_prereq USR_BIN_TIME
create_repo () {
number_of_commits=$1
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index e70ac10..08e30b3 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -6,7 +6,6 @@ test_description='git rebase - test patch id computation'
test_set_prereq NOT_EXPENSIVE
test -n $GIT_PATCHID_TIMING_TESTS test_set_prereq EXPENSIVE
-test -x /usr/bin/time test_set_prereq USR_BIN_TIME
count()
{
diff --git a/t/test-lib.sh b/t/test-lib.sh
index d70d05e..884c57c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -859,6 +859,10 @@ test_lazy_prereq EXPENSIVE '
test -n $GIT_TEST_LONG
'
+test_lazy_prereq USR_BIN_TIME '
+ test -x /usr/bin/time
+'
+
# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
test -w / || test_set_prereq SANITY
--
2.0.0-435-g307a092
--
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 5/7] t3302: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite
This was only necessary because do_tests helper the script defines
took its parameters in a wrong order. Just pass an empty string (or
not passing the optional EXPENSIVE prerequisite) when running the
test with a light-weight set of parameters and have the shell do the
right thing when parsing test_expect_success helper.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t3302-notes-index-expensive.sh | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index 7712cf3..8d44e04 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -7,7 +7,6 @@ test_description='Test commit notes index (expensive!)'
. ./test-lib.sh
-test_set_prereq NOT_EXPENSIVE
test -n $GIT_NOTES_TIMING_TESTS test_set_prereq EXPENSIVE
create_repo () {
@@ -103,8 +102,7 @@ time_notes () {
}
do_tests () {
- pr=$1
- count=$2
+ count=$1 pr=${2-}
test_expect_success $pr setup $count '
mkdir $count
@@ -121,7 +119,7 @@ do_tests () {
)
'
- test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' '
+ test_expect_success USR_BIN_TIME${pr:+,$pr} 'notes timing with
/usr/bin/time' '
(
cd $count
time_notes 100
@@ -129,10 +127,10 @@ do_tests () {
'
}
-do_tests NOT_EXPENSIVE 10
+do_tests 10
for count in 100 1000 1
do
- do_tests EXPENSIVE $count
+ do_tests $count EXPENSIVE
done
test_done
--
2.0.0-435-g307a092
--
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/7] t3302: coding style updates
Use -END_OF_HERE_TEXT to push the contents of here-text to the
right in order to show the loop structure better.
Use write_script when writing a script to be run.
Use test (not [ ... ]) and avoid unnecessary ; in the middle
of a line.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t3302-notes-index-expensive.sh | 90 +---
1 file changed, 47 insertions(+), 43 deletions(-)
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index dc706ab..aa9dbd7 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -16,43 +16,43 @@ create_repo () {
test -d .git || {
git init
(
- while [ $nr -lt $number_of_commits ]; do
+ while test $nr -lt $number_of_commits
+ do
nr=$(($nr+1))
mark=$(($nr+$nr))
notemark=$(($mark+1))
test_tick
- cat INPUT_END
-commit refs/heads/master
-mark :$mark
-committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
-data COMMIT
-commit #$nr
-COMMIT
-
-M 644 inline file
-data EOF
-file in commit #$nr
-EOF
-
-blob
-mark :$notemark
-data EOF
-note for commit #$nr
-EOF
-
-INPUT_END
-
- echo N :$notemark :$mark note_commit
+ cat -INPUT_END
+ commit refs/heads/master
+ mark :$mark
+ committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
$GIT_COMMITTER_DATE
+ data COMMIT
+ commit #$nr
+ COMMIT
+
+ M 644 inline file
+ data EOF
+ file in commit #$nr
+ EOF
+
+ blob
+ mark :$notemark
+ data EOF
+ note for commit #$nr
+ EOF
+
+ INPUT_END
+ echo N :$notemark :$mark note_commit
done
test_tick
- cat INPUT_END
-commit refs/notes/commits
-committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
-data COMMIT
-notes
-COMMIT
+ cat -INPUT_END
+ commit refs/notes/commits
+ committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL
$GIT_COMMITTER_DATE
+ data COMMIT
+ notes
+ COMMIT
-INPUT_END
+ INPUT_END
cat note_commit
) |
@@ -64,38 +64,41 @@ INPUT_END
test_notes () {
count=$1
git config core.notesRef refs/notes/commits
- git log | grep ^ output
+ git log | grep ^ output
i=$count
- while [ $i -gt 0 ]; do
+ while test $i -gt 0
+ do
echo commit #$i
echo note for commit #$i
- i=$(($i-1));
- done expect
+ i=$(($i-1))
+ done expect
test_cmp expect output
}
-cat time_notes \EOF
+write_script time_notes \EOF
mode=$1
i=1
- while [ $i -lt $2 ]; do
+ while test $i -lt $2
+ do
case $1 in
no-notes)
- GIT_NOTES_REF=non-existing; export GIT_NOTES_REF
- ;;
+ GIT_NOTES_REF=non-existing
+ export GIT_NOTES_REF
+ ;;
notes)
unset GIT_NOTES_REF
- ;;
+ ;;
esac
- git log /dev/null
+ git log
i=$(($i+1))
- done
+ done /dev/null
EOF
time_notes () {
for mode in no-notes notes
do
echo $mode
- /usr/bin/time $SHELL_PATH ../time_notes $mode $1
+ /usr/bin/time ../time_notes $mode $1
done
}
@@ -118,7 +121,8 @@ do_tests () {
}
do_tests NOT_EXPENSIVE 10
-for count in 100 1000 1; do
+for count in 100 1000 1
+do
do_tests EXPENSIVE $count
done
--
2.0.0-435-g307a092
--
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/7] test: turn EXPENSIVE into a lazy prerequisite
Two test scripts (t0021 and t5551) had copy paste code to set EXPENSIVE prerequisite. Use the test_lazy_prereq helper to define them in the common t/test-lib.sh. Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t0021-conversion.sh | 2 -- t/t5551-http-fetch.sh | 2 -- t/test-lib.sh | 4 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index b92e6cb..f890c54 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -190,8 +190,6 @@ test_expect_success 'required filter clean failure' ' test_must_fail git add test.fc ' -test -n $GIT_TEST_LONG test_set_prereq EXPENSIVE - test_expect_success EXPENSIVE 'filter large file' ' git config filter.largefile.smudge cat git config filter.largefile.clean cat diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh index afb439e..d697393 100755 --- a/t/t5551-http-fetch.sh +++ b/t/t5551-http-fetch.sh @@ -214,8 +214,6 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set test_cmp expect_cookies.txt cookies_tail.txt ' -test -n $GIT_TEST_LONG test_set_prereq EXPENSIVE - test_expect_success EXPENSIVE 'create 50,000 tags in the repo' ' ( cd $HTTPD_DOCUMENT_ROOT_PATH/repo.git diff --git a/t/test-lib.sh b/t/test-lib.sh index b25249e..d70d05e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -855,6 +855,10 @@ test_lazy_prereq AUTOIDENT ' git var GIT_AUTHOR_IDENT ' +test_lazy_prereq EXPENSIVE ' + test -n $GIT_TEST_LONG +' + # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test -w / || test_set_prereq SANITY -- 2.0.0-435-g307a092 -- 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 4/7] t3302: do not chdir around in the primary test process
These days^Wyears we strive to do stuff in subdirectories inside
subshells to avoid mistakes.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t3302-notes-index-expensive.sh | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index aa9dbd7..7712cf3 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -106,18 +106,27 @@ do_tests () {
pr=$1
count=$2
- test_expect_success $pr 'setup / mkdir' '
- mkdir $count
- cd $count
+ test_expect_success $pr setup $count '
+ mkdir $count
+ (
+ cd $count
+ create_repo $count
+ )
'
- test_expect_success $pr setup $count create_repo $count
-
- test_expect_success $pr 'notes work' test_notes $count
-
- test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time'
time_notes 100
+ test_expect_success $pr 'notes work' '
+ (
+ cd $count
+ test_notes $count
+ )
+ '
- test_expect_success $pr 'teardown / cd ..' 'cd ..'
+ test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' '
+ (
+ cd $count
+ time_notes 100
+ )
+ '
}
do_tests NOT_EXPENSIVE 10
--
2.0.0-435-g307a092
--
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 6/7] t3419: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite
This was only necessary because do_tests helper the script defines
took its parameters in a wrong order. Just pass an empty string (or
not passing the optional EXPENSIVE prerequisite) when running the
test with a light-weight set of parameters and have the shell do the
right thing when parsing test_expect_success helper.
Also update coding style while we are at it.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t3419-rebase-patch-id.sh | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 08e30b3..9292b49 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -4,11 +4,9 @@ test_description='git rebase - test patch id computation'
. ./test-lib.sh
-test_set_prereq NOT_EXPENSIVE
test -n $GIT_PATCHID_TIMING_TESTS test_set_prereq EXPENSIVE
-count()
-{
+count () {
i=0
while test $i -lt $1
do
@@ -17,8 +15,7 @@ count()
done
}
-scramble()
-{
+scramble () {
i=0
while read x
do
@@ -27,12 +24,11 @@ scramble()
echo $x
fi
i=$((($i+1) % 10))
- done $1 $1.new
+ done $1 $1.new
mv -f $1.new $1
}
-run()
-{
+run () {
echo \$ $@
/usr/bin/time $@ /dev/null
}
@@ -42,10 +38,8 @@ test_expect_success 'setup' '
git tag root
'
-do_tests()
-{
- pr=$1
- nlines=$2
+do_tests () {
+ nlines=$1 pr=${2-}
test_expect_success $pr setup: $nlines lines
rm -f .gitattributes
@@ -102,7 +96,7 @@ do_tests()
}
-do_tests NOT_EXPENSIVE 500
-do_tests EXPENSIVE 5
+do_tests 500
+do_tests 5 EXPENSIVE
test_done
--
2.0.0-435-g307a092
--
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 10/20] git-submodule.sh: avoid test cond -a/-o cond
Elia Pinto gitter.spi...@gmail.com writes:
@@ -1059,13 +1059,17 @@ cmd_summary() {
while read mod_src mod_dst sha1_src sha1_dst status sm_path
do
# Always show modules deleted or type-changed
(blob-module)
- test $status = D -o $status = T echo $sm_path
continue
+ case $status in
+ [DT])
+ printf '%s\n' $sm_path
+ continue
+ esac
I think this conversion is wrong and causes parse error. The
surrounding code cannot be seen in the context of thsi patch, but
looks somewhat like this:
modules=$(
case $status in
[DT])
...
esac
)
Perhaps you would need to spell it with the extra opening
parenthesis, like so:
case string in
([DT])
...
esac
or something.
--
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 0/7] A few minor test-prereq updates
While reviewing somebody's patch, I noticed that individual test
scripts set EXPENSIVE test prerequisite with copied-and-pasted
lines. Here is a bit to update them, while fixing styles in old
test scripts that had these copied-and-pasted lines.
The last step discards support for GIT_{NOTES,PATCHID}_TIMING_TESTS
environment variables to enable EXPENSIVE tests in two test scripts,
which *is* a backward incompatible change and people may want to
argue against it (but I of course thought it is a good change and
that is why it is included).
Junio C Hamano (7):
test: turn EXPENSIVE into a lazy prerequisite
test: turn USR_BIN_TIME into a lazy prerequisite
t3302: coding style updates
t3302: do not chdir around in the primary test process
t3302: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite
t3419: drop unnecessary NOT_EXPENSIVE pseudo-prerequisite
tests: drop GIT_*_TIMING_TESTS environment variable support
t/t0021-conversion.sh| 2 -
t/t3302-notes-index-expensive.sh | 130 +--
t/t3419-rebase-patch-id.sh | 25 +++-
t/t5551-http-fetch.sh| 2 -
t/test-lib.sh| 8 +++
5 files changed, 85 insertions(+), 82 deletions(-)
--
2.0.0-435-g307a092
--
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 02/20] contrib/examples/git-clone.sh: avoid test cond -a/-o cond
Elia Pinto gitter.spi...@gmail.com writes: The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- contrib/examples/git-clone.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/examples/git-clone.sh b/contrib/examples/git-clone.sh index b4c9376..08cf246 100755 --- a/contrib/examples/git-clone.sh +++ b/contrib/examples/git-clone.sh @@ -516,7 +516,7 @@ then case $no_checkout in '') - test z$quiet = z -a z$no_progress = z v=-v || v= + test z$quiet = z test z$no_progress = z v=-v || v= git read-tree -m -u $v HEAD HEAD esac fi Hmph. If we want to see them both empty, test $quiet,$no_progress = , would have been a better way to spell this, but that is outside the scope of this series. But I wonder if we really want to update the contrib/examples/, which is a record of how historically we have implemented various scripted Porcelains using lower level plumbing commands. -- 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 7/7] tests: drop GIT_*_TIMING_TESTS environment variable support
Two tests (t3302 and t3419) used to have their own environment
variable to trigger expensive tests without enabling expensive
tests in other scripts; a user could set GIT_NOTES_TIMING_TESTS
but not GIT_TEST_LONG and run the whole test suite and trigger
expensive tests only in t3302 but not other tests. The same for
GIT_PATCHID_TIMING_TESTS in t3419.
While this may have seemed a good flexibility, in reality if you are
concentrating on a single test (e.g. t3302), you can just run that
single test with the GIT_TEST_LONG to trigger expensive tests. It
does not seem worth forcing other people who may want to come up
with their own expesive tests to invent new environment variables by
keeping this convention.
Drop them.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t3302-notes-index-expensive.sh | 2 --
t/t3419-rebase-patch-id.sh | 2 --
2 files changed, 4 deletions(-)
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index 8d44e04..7217c5e 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -7,8 +7,6 @@ test_description='Test commit notes index (expensive!)'
. ./test-lib.sh
-test -n $GIT_NOTES_TIMING_TESTS test_set_prereq EXPENSIVE
-
create_repo () {
number_of_commits=$1
nr=0
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 9292b49..217dd79 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -4,8 +4,6 @@ test_description='git rebase - test patch id computation'
. ./test-lib.sh
-test -n $GIT_PATCHID_TIMING_TESTS test_set_prereq EXPENSIVE
-
count () {
i=0
while test $i -lt $1
--
2.0.0-435-g307a092
--
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] send-email: do not insert third header
Stepan Kasal ka...@ucw.cz writes:
It is sometimes desirable to insert several header lines at the top of
the body, e.g., if From or Date differs from the mail header.
(Linus even recommends to use this second header for all kernel
submissions.)
send-email has a minimal support for this; make sure it is not applied
when there is a second header already inserted in the patch file.
I have a slight suspicion that you are reading the recommendation
wrong. We do not recommend to record these in-body headers in the
message of the commit object (the recommendation is to prepend
in-body headers to the message of the commit object when sending it
out for review---it pretty much assumes that the underlying commit
does not have these in-body headers that are used only during the
transit over e-mail forwarding chain).
But your patch seems to assume that the input message to send-email
already has the in-body header. Doesn't that indicate a misuse of
the tool, making this new feature smell more like a way to
encourage such a misuse by covering up the result?
I dunno.
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
git-send-email.perl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 9949db0..891df13 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1456,7 +1456,9 @@ foreach my $t (@files) {
}
if (defined $sauthor and $sauthor ne $sender) {
- $message = From: $author\n\n$message;
+ if ($message !~ m/^From: /) {
+ $message = From: $author\n\n$message;
+ }
if (defined $author_encoding) {
if ($has_content_type) {
if ($body_encoding eq $author_encoding) {
--
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] Re: [RFC PATCH] clone: add clone.recursesubmodules config option
On Mon, Jun 09, 2014 at 03:17:07PM +0200, Jens Lehmann wrote: And by the way: wouldn't it make more sense to tell the user /what/ we do automatically? So maybe 'submodule.autoupdate' is a better name for the new switch? Or autocheckout? No need to preserve submodule-specific jargon when we have a perfectly acceptable word for this in the core interface ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Git reset --hard with staged changes
Pierre-François CLEMENT lik...@gmail.com writes: Hm, I didn't think of git apply --index... Makes sense for this special use, but I'm not sure about the other use cases. Try merging another branch that tracks a file your current branch does not know about and ending up with conflicts during that merge. Resetting the half-done result away must remove that new path from your working tree and the index. -- 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 12/15] use get_commit_buffer everywhere
On Mon, Jun 09, 2014 at 03:40:57PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: diff --git a/notes-merge.c b/notes-merge.c index 94a1a8a..7885ab2 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -671,7 +671,8 @@ int notes_merge_commit(struct notes_merge_options *o, DIR *dir; struct dirent *e; struct strbuf path = STRBUF_INIT; - char *msg = strstr(partial_commit-buffer, \n\n); + const char *buffer = get_commit_buffer(partial_commit); + const char *msg = strstr(buffer, \n\n); This tightening causes... struct strbuf sb_msg = STRBUF_INIT; int baselen; @@ -720,6 +721,7 @@ int notes_merge_commit(struct notes_merge_options *o, } strbuf_attach(sb_msg, msg, strlen(msg), strlen(msg) + 1); ...a new error here: notes-merge.c:723:2: error: passing argument 2 of 'strbuf_attach' discards 'const' qualifier from pointer target type [-Werror] strbuf.h:19:13: note: expected 'void *' but argument is of type 'const char *' That's weird. I compile with -Wall -Werror, and my gcc doesn't complain. Hmph. I agree it's not right, though. I think the original is questionable, too. It takes a pointer into the middle of partial_commit-buffer and attaches it to a strbuf. That's wrong because: 1. It's pointing into the middle of an allocated buffer, not the beginning. 2. We do not own partial_commit-buffer in the first place. So any call to strbuf_detach on the result would be disastrous. The compiler doesn't notice because of the const leak in strstr, and it doesn't cause a bug in practice because the only use of the strbuf is to pass it as a const to create_notes_commit. I feel like the most elegant solution is for create_notes_commit to take a buf/len pair rather than a strbuf, but it unfortunately is just feeding that to commit_tree. Adjusting that code path would affect quite a few other spots. The other obvious option is actually populating the strbuf, but it feels ugly to have to make a copy just to satisfy the function interface. Maybe a cast and a warning comment are the least evil thing, as below? I dunno, it feels pretty wrong. diff --git a/notes-merge.c b/notes-merge.c index 94a1a8a..1f3b309 100644 --- a/notes-merge.c +++ b/notes-merge.c @@ -671,7 +671,7 @@ int notes_merge_commit(struct notes_merge_options *o, DIR *dir; struct dirent *e; struct strbuf path = STRBUF_INIT; - char *msg = strstr(partial_commit-buffer, \n\n); + const char *msg = strstr(partial_commit-buffer, \n\n); struct strbuf sb_msg = STRBUF_INIT; int baselen; @@ -719,7 +719,15 @@ int notes_merge_commit(struct notes_merge_options *o, strbuf_setlen(path, baselen); } - strbuf_attach(sb_msg, msg, strlen(msg), strlen(msg) + 1); + /* +* This is a bit tricky. We should not be attaching msg, which +* is not owned by us and is not even the start of a heap buffer, to a +* strbuf. But the create_notes_commit interface really wants +* a strbuf, even though it will only ever use it as a buf/len pair and +* never modify it. So this is tentatively safe as long as nobody ever +* modifies, detaches, or releases the strbuf. +*/ + strbuf_attach(sb_msg, (char *)msg, strlen(msg), strlen(msg) + 1); create_notes_commit(partial_tree, partial_commit-parents, sb_msg, result_sha1); if (o-verbosity = 4) I'm still confused and disturbed that my gcc is not noticing this obvious const violation. Hmm, shutting off ccache seems to make it work. Doubly disturbing. -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] rebase -i: Remember merge options beyond continue actions
If the user explicitly specified a merge strategy or strategy options,
rebase --interactive started using the default merge strategy again
after rebase --continue.
This problem gets fixed by this commit. Add test.
Since the rebase options -s and -X imply --merge, we can simply
remove the do_merge guard in the interactive mode and always compile
the cherry-pick arguments from the rebase state variables strategy
and strategy_opts.
---
git-rebase--interactive.sh| 18 +++---
t/t3404-rebase-interactive.sh | 16
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..817c933 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -77,17 +77,13 @@ amend=$state_dir/amend
rewritten_list=$state_dir/rewritten-list
rewritten_pending=$state_dir/rewritten-pending
-strategy_args=
-if test -n $do_merge
-then
- strategy_args=${strategy:+--strategy=$strategy}
- eval '
- for strategy_opt in '$strategy_opts'
- do
- strategy_args=$strategy_args -X$(git rev-parse
--sq-quote ${strategy_opt#--})
- done
- '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+ for strategy_opt in '$strategy_opts'
+ do
+ strategy_args=$strategy_args -X$(git rev-parse --sq-quote
${strategy_opt#--})
+ done
+'
GIT_CHERRY_PICK_HELP=$resolvemsg
export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0023a5..73849f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
'
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+ git checkout -b conflict-merge-use-theirs-interrupted conflict-branch
+ git reset --hard HEAD^
+ breakpoint
+ git add breakpoint
+ git commit -m breakpoint for interactive mode
+ echo five conflict
+ echo Z file1
+ git commit -a -m one file conflict
+ set_fake_editor
+ FAKE_LINES=edit 1 2 git rebase -i --strategy=recursive -Xours
conflict-branch
+ git rebase --continue
+ test $(git show conflict-branch:conflict) = $(cat conflict)
+ test $(cat file1) = Z
+'
+
test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD)
test_when_finished git rebase --abort; git reset --hard $current_head;
rm -f error
--
2.0.0
--
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] rebase -i: Remember merge options beyond continue actions
On Mon, Jun 9, 2014 at 8:02 PM, Fabian Ruch baf...@gmail.com wrote:
If the user explicitly specified a merge strategy or strategy options,
rebase --interactive started using the default merge strategy again
after rebase --continue.
For reference, this problem was reported as far back as 2013-08-09 [1].
[1]: http://article.gmane.org/gmane.comp.version-control.git/232013
This problem gets fixed by this commit. Add test.
Since the rebase options -s and -X imply --merge, we can simply
remove the do_merge guard in the interactive mode and always compile
the cherry-pick arguments from the rebase state variables strategy
and strategy_opts.
Missing sign-off.
---
git-rebase--interactive.sh| 18 +++---
t/t3404-rebase-interactive.sh | 16
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..817c933 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -77,17 +77,13 @@ amend=$state_dir/amend
rewritten_list=$state_dir/rewritten-list
rewritten_pending=$state_dir/rewritten-pending
-strategy_args=
-if test -n $do_merge
-then
- strategy_args=${strategy:+--strategy=$strategy}
- eval '
- for strategy_opt in '$strategy_opts'
- do
- strategy_args=$strategy_args -X$(git rev-parse
--sq-quote ${strategy_opt#--})
- done
- '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+ for strategy_opt in '$strategy_opts'
+ do
+ strategy_args=$strategy_args -X$(git rev-parse --sq-quote
${strategy_opt#--})
+ done
+'
GIT_CHERRY_PICK_HELP=$resolvemsg
export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0023a5..73849f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
'
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+ git checkout -b conflict-merge-use-theirs-interrupted conflict-branch
+ git reset --hard HEAD^
+ breakpoint
+ git add breakpoint
+ git commit -m breakpoint for interactive mode
+ echo five conflict
+ echo Z file1
+ git commit -a -m one file conflict
+ set_fake_editor
+ FAKE_LINES=edit 1 2 git rebase -i --strategy=recursive -Xours
conflict-branch
+ git rebase --continue
+ test $(git show conflict-branch:conflict) = $(cat conflict)
+ test $(cat file1) = Z
+'
+
test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD)
test_when_finished git rebase --abort; git reset --hard
$current_head; rm -f error
--
2.0.0
--
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] rebase -i: Remember merge options beyond continue actions
Hi Eric,
thanks a lot for the reference.
I added the Reported-by: and Signed-off-by: lines to the commit message.
Fabian
-- 8 --
Subject: rebase -i: Remember merge options beyond continue actions
If the user explicitly specified a merge strategy or strategy options,
rebase --interactive started using the default merge strategy again
after rebase --continue.
This problem gets fixed by this commit. Add test.
Since the rebase options -s and -X imply --merge, we can simply
remove the do_merge guard in the interactive mode and always compile
the cherry-pick arguments from the rebase state variables strategy
and strategy_opts.
Reported-by: Diogo de Campos cam...@esss.com.br
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 18 +++---
t/t3404-rebase-interactive.sh | 16
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..817c933 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -77,17 +77,13 @@ amend=$state_dir/amend
rewritten_list=$state_dir/rewritten-list
rewritten_pending=$state_dir/rewritten-pending
-strategy_args=
-if test -n $do_merge
-then
- strategy_args=${strategy:+--strategy=$strategy}
- eval '
- for strategy_opt in '$strategy_opts'
- do
- strategy_args=$strategy_args -X$(git rev-parse
--sq-quote ${strategy_opt#--})
- done
- '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+ for strategy_opt in '$strategy_opts'
+ do
+ strategy_args=$strategy_args -X$(git rev-parse --sq-quote
${strategy_opt#--})
+ done
+'
GIT_CHERRY_PICK_HELP=$resolvemsg
export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0023a5..73849f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
'
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+ git checkout -b conflict-merge-use-theirs-interrupted conflict-branch
+ git reset --hard HEAD^
+ breakpoint
+ git add breakpoint
+ git commit -m breakpoint for interactive mode
+ echo five conflict
+ echo Z file1
+ git commit -a -m one file conflict
+ set_fake_editor
+ FAKE_LINES=edit 1 2 git rebase -i --strategy=recursive -Xours
conflict-branch
+ git rebase --continue
+ test $(git show conflict-branch:conflict) = $(cat conflict)
+ test $(cat file1) = Z
+'
+
test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD)
test_when_finished git rebase --abort; git reset --hard $current_head;
rm -f error
--
2.0.0
--
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 12/15] use get_commit_buffer everywhere
On Mon, Jun 09, 2014 at 08:02:24PM -0400, Jeff King wrote: I'm still confused and disturbed that my gcc is not noticing this obvious const violation. Hmm, shutting off ccache seems to make it work. Doubly disturbing. Ah, mystery solved. It's a gcc bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60014 I get: $ gcc -c -Wall -Werror -DSHA1_HEADER='block-sha1/sha1.h' notes-merge.c notes-merge.c: In function ‘notes_merge_commit’: notes-merge.c:723:2: error: passing argument 2 of ‘strbuf_attach’ discards ‘const’ qualifier from pointer target type [-Werror] ...etc... $ gcc -E -Wall -Werror -DSHA1_HEADER='block-sha1/sha1.h' notes-merge.c foo.c $ gcc -c -Wall -Werror -DSHA1_HEADER='block-sha1/sha1.h' foo.c [no warnings from either] ccache uses the latter technique. -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 reset --hard with staged changes
From: Pierre-François CLEMENT likeyn at gmail.com You create a new (untracked) file. You use git-reset's hard mode to go one commit back, the new (untracked) file's still there. You add/stage that new file. You use git-reset's hard mode again to go one commit back, and the new untracked file you just staged gets deleted. Also, according to Git-scm (http://git-scm.com/book/en/Git-Basics-Recording-Changes-to-the-Repository): Tracked files are files that were in the last snapshot [...]. Untracked files are everything else. So it seems to me like staged untracked files shouldn't be considered as tracked files, and thus shouldn't be removed. Or maybe, git-reset's hard mode should always delete everything including untracked files? It would also make sense, given the numerous modes it has. There's a core question that must be answered: What, *exactly*, is a tracked file? If you look at that passage in the book, it continues: Tracked files are files that were in the last snapshot; they can be unmodified, modified, or staged. Untracked files are everything else — any files in your working directory that were not in your last snapshot and are not in your staging area. But if you look carefully, that passage gives two definitions of untracked files, and *they don't agree*, specifically in the case of a file that is in the index but not in the base commit. And that's the case we're considering. To fix this, you've got to figure out what the definition of tracked file is supposed to be, and then ensure that everything (code and documentation) is consistent with that. (As far as I can tell from Git's behavior, the definition of tracked file is any file that is in the base commit or in the index. Based on that definition, git reset --hard is working as documented.) Dale -- 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 04/15] logmsg_reencode: return const buffer
On Mon, Jun 9, 2014 at 2:10 PM, Jeff King p...@peff.net wrote:
The return value from logmsg_reencode may be either a newly
allocated buffer or a pointer to the existing commit-buffer.
We would not want the caller to accidentally free() or
modify the latter, so let's mark it as const. We can cast
away the constness in logmsg_free, but only once we have
determined that it is a free-able buffer.
Signed-off-by: Jeff King p...@peff.net
---
index 71e2337..47e5b7a 100644
--- a/revision.c
+++ b/revision.c
@@ -2788,7 +2788,7 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
{
int retval;
const char *encoding;
- char *message;
+ const char *message;
struct strbuf buf = STRBUF_INIT;
if (!opt-grep_filter.pattern_list !opt-grep_filter.header_list)
@@ -2830,12 +2830,18 @@ static int commit_match(struct commit *commit, struct
rev_info *opt)
format_display_notes(commit-object.sha1, buf, encoding, 1);
}
- /* Find either in the original commit message, or in the temporary */
+ /* Find either in the original commit message, or in the temporary.
Style:
/*
* Find either...
*/
+* Note that we cast away the constness of message here. It is
+* const because it may come from the cached commit buffer. That's OK,
+* because we know that it is modifiable heap memory, and that while
+* grep_buffer may modify it for speed, it will restore any
+* changes before returning.
+*/
if (buf.len)
retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
else
retval = grep_buffer(opt-grep_filter,
-message, strlen(message));
+(char *)message, strlen(message));
strbuf_release(buf);
logmsg_free(message, commit);
return retval;
--
2.0.0.729.g520999f
--
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 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?
Why wouldn't we? Everything that has basename .git is hard-wired
to be excluded, but everything else, including foobar.git should be
added to the excludes manually... How is it better than just foobar?
--
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] t5551: fix the 50,000 tag test
On 2014-06-09 21.16, Junio C Hamano wrote: Since the day git clone printed Cloning into 'too-many-refs' to stderr, Thanks. It is 68b939b2 (clone: send diagnostic messages to stderr, 2013-09-18); before it we showed the message to the standard output stream instead. Will queue. Thanks for digging, as we now know better: do you want to squeeze in someting like this: s/Since the day/Since commit 5e2c7cd2/ -- 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 v3] t9001: avoid not portable '\n' with sed
t9001 used a '\n' in a sed expression to split one line into two lines,
but the usage of '\n' in the replacement string is not portable.
The '\n' can be used to match a newline in the pattern space,
but otherwise the meaning of '\n' is unspecified in POSIX.
- Gnu versions of sed will treat '\n' as a newline character.
- Other versions of sed (like /usr/bin/sed under Mac OS X)
simply ignore the '\' before the 'n', treating '\n' as 'n'.
For reference see:
pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
http://www.gnu.org/software/sed/manual/sed.html
As the test already requires perl as a prerequisite, use perl instead of sed.
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
Sending a V3 patch seems spammish, but after re-reading all
the comments I think that the commit msg should point out the difference
between POSIX sed and gnu sed somewhat better.
t/t9001-send-email.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 64d9434..19a3ced 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
git format-patch --cover-letter -2 -o outdir
cover=`echo outdir/-*.patch`
mv $cover cover-to-edit.patch
- sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch
$cover
+ perl -pe s/^From:/$header: extra\@address.com\nFrom:/
cover-to-edit.patch $cover
git send-email \
--force \
--from=Example nob...@example.com \
--
2.0.0.553.ged01b91
--
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] t9001: avoid not portable '\n' with sed
On Tue, Jun 10, 2014 at 12:07 AM, Torsten Bögershausen tbo...@web.de wrote:
t9001 used a '\n' in a sed expression to split one line into two lines,
but the usage of '\n' in the replacement string is not portable.
The '\n' can be used to match a newline in the pattern space,
but otherwise the meaning of '\n' is unspecified in POSIX.
- Gnu versions of sed will treat '\n' as a newline character.
- Other versions of sed (like /usr/bin/sed under Mac OS X)
simply ignore the '\' before the 'n', treating '\n' as 'n'.
For reference see:
pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
http://www.gnu.org/software/sed/manual/sed.html
As the test already requires perl as a prerequisite, use perl instead of sed.
René Scharfe pointed out this useful resource [1] for writing portable
'sed' when he fixed [2] a problem on NetBSD in a test I had written.
[1]: http://sed.sourceforge.net/sedfaq4.html
[2]: http://thread.gmane.org/gmane.comp.version-control.git/231654
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
Sending a V3 patch seems spammish, but after re-reading all
the comments I think that the commit msg should point out the difference
between POSIX sed and gnu sed somewhat better.
t/t9001-send-email.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 64d9434..19a3ced 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1342,7 +1342,7 @@ test_cover_addresses () {
git format-patch --cover-letter -2 -o outdir
cover=`echo outdir/-*.patch`
mv $cover cover-to-edit.patch
- sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch
$cover
+ perl -pe s/^From:/$header: extra\@address.com\nFrom:/
cover-to-edit.patch $cover
git send-email \
--force \
--from=Example nob...@example.com \
--
2.0.0.553.ged01b91
--
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 15/15] commit: record buffer length in cache
From: Jeff King p...@peff.net --- 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))); - set_commit_buffer(commit, strbuf_detach(msg, NULL)); + set_commit_buffer(commit, msg.buf, msg.len); I find the above strange. I would have done something like: - set_commit_buffer(commit, strbuf_detach(msg, NULL)); + size_t size; + char *buf = strbuf_detach(msg, size); + set_commit_buffer(commit, buf, size); Thanks, Christian. -- 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 15/15] commit: record buffer length in cache
On Tue, Jun 10, 2014 at 07:12:37AM +0200, Christian Couder wrote: From: Jeff King p...@peff.net --- 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))); - set_commit_buffer(commit, strbuf_detach(msg, NULL)); + set_commit_buffer(commit, msg.buf, msg.len); I find the above strange. I would have done something like: - set_commit_buffer(commit, strbuf_detach(msg, NULL)); + size_t size; + char *buf = strbuf_detach(msg, size); + set_commit_buffer(commit, buf, size); It is a little strange. You can't do: set_commit_buffer(commit, strbuf_detach(msg, NULL), msg.len); because the second argument resets msg.len as a side effect. Doing it your way is longer, but perhaps is a bit more canonical. In general, one should call strbuf_detach to ensure that the buffer is allocated (and does not point to slopbuf). That's guaranteed here, because we just put contents into the buffer, but it's probably more hygienic to use the more verbose form. -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] completion: Handle '!f() { ... }; f' aliases
Steffen Prohaska proha...@zib.de writes:
'!f() { ... }; f' is a recommended pattern to declare more complex
aliases (see git wiki [1]). This commit teaches the completion to
handle them.
Hmm, I've never endorsed nor recommended such a notation myself ;-)
I tend to prefer writing it like so instead:
sh -c '...' -
so that I won't clobber f (or any other name). I wonder if you
can help users of this other pattern as well.
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.
I suspect that scanning is error-prone. I like this one for its
cuteness very much, though:
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.
+test_expect_success 'completion uses cmd completion for alias !f() {
VAR=val git cmd ... }' '
+ test_config alias.co !f() { VAR=val git checkout ... ; } f
Is it only f that is completed, or can I spell it using another
arbitrary token, e.g.
test_config alias.co !co () { git checkout ... } co
--
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
