* [PATCH 01/10] strbuf_split: add a max parameter
2011-06-09 15:50 ` Jeff King
@ 2011-06-09 15:51 ` Jeff King
2011-06-13 17:30 ` Junio C Hamano
2011-06-09 15:51 ` [PATCH 02/10] fix "git -c" parsing of values with equals signs Jeff King
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:51 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
Sometimes when splitting, you only want a limited number of
fields, and for the final field to contain "everything
else", even if it includes the delimiter.
This patch introduces strbuf_split_max, which provides a
"max number of fields" parameter; it behaves similarly to
perl's "split" with a 3rd field.
The existing 2-argument form of strbuf_split is retained for
compatibility and ease-of-use.
Signed-off-by: Jeff King <peff@peff.net>
---
I am tempted to just call this new one strbuf_split and update all
callers. There aren't that many.
strbuf.c | 7 +++++--
strbuf.h | 7 ++++++-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 09c43ae..64f6c1e 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -103,7 +103,7 @@ void strbuf_ltrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
-struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
+struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max)
{
int alloc = 2, pos = 0;
char *n, *p;
@@ -114,7 +114,10 @@ struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
p = n = sb->buf;
while (n < sb->buf + sb->len) {
int len;
- n = memchr(n, delim, sb->len - (n - sb->buf));
+ if (max <= 0 || pos + 1 < max)
+ n = memchr(n, delim, sb->len - (n - sb->buf));
+ else
+ n = NULL;
if (pos + 1 >= alloc) {
alloc = alloc * 2;
ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
diff --git a/strbuf.h b/strbuf.h
index 9e6d9fa..4cf1dcd 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -44,7 +44,12 @@ extern void strbuf_rtrim(struct strbuf *);
extern void strbuf_ltrim(struct strbuf *);
extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
-extern struct strbuf **strbuf_split(const struct strbuf *, int delim);
+extern struct strbuf **strbuf_split_max(const struct strbuf *,
+ int delim, int max);
+static inline struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
+{
+ return strbuf_split_max(sb, delim, 0);
+}
extern void strbuf_list_free(struct strbuf **);
/*----- add data in your buffer -----*/
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 01/10] strbuf_split: add a max parameter
2011-06-09 15:51 ` [PATCH 01/10] strbuf_split: add a max parameter Jeff King
@ 2011-06-13 17:30 ` Junio C Hamano
2011-06-13 19:20 ` Jeff King
0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2011-06-13 17:30 UTC (permalink / raw)
To: Jeff King; +Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy
Jeff King <peff@peff.net> writes:
> I am tempted to just call this new one strbuf_split and update all
> callers. There aren't that many.
Yes, that is indeed tempting, and because we have a new parameter the
compiler will catch any new callers that pop up in a mismerge so that
would be perfectly safe.
> -struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
> +struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max)
> {
> int alloc = 2, pos = 0;
> char *n, *p;
> @@ -114,7 +114,10 @@ struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
> p = n = sb->buf;
> while (n < sb->buf + sb->len) {
> int len;
> - n = memchr(n, delim, sb->len - (n - sb->buf));
> + if (max <= 0 || pos + 1 < max)
> + n = memchr(n, delim, sb->len - (n - sb->buf));
> + else
> + n = NULL;
> if (pos + 1 >= alloc) {
> alloc = alloc * 2;
> ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
Hmm, even when we know the value of max, we go exponential, and even do so
by hand without using ALLOC_GROW(). Somewhat sad.
Also do we currently rely on the bug that strbuf_split() returns (NULL,)
instead of ("", NULL) when given an empty string? If not, perhaps...
strbuf.c | 50 +++++++++++++++++++++++++++++++-------------------
1 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 09c43ae..7a8ee3a 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -103,31 +103,43 @@ void strbuf_ltrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
-struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
+struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max)
{
- int alloc = 2, pos = 0;
char *n, *p;
- struct strbuf **ret;
+ struct strbuf **ret = NULL;
struct strbuf *t;
-
- ret = xcalloc(alloc, sizeof(struct strbuf *));
- p = n = sb->buf;
- while (n < sb->buf + sb->len) {
- int len;
- n = memchr(n, delim, sb->len - (n - sb->buf));
- if (pos + 1 >= alloc) {
- alloc = alloc * 2;
- ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
+ int pass, count = 0;
+
+ for (pass = 0; pass < 2; pass++) {
+ /* First pass counts, second pass allocates and fills */
+ if (pass)
+ ret = xcalloc(count + 1 + !count, sizeof(struct strbuf *));
+ count = 0;
+ n = sb->buf;
+ while (n < sb->buf + sb->len) {
+ p = n;
+ if (max <= 0 || count + 1 < max)
+ n = memchr(n, delim, sb->len - (n - sb->buf));
+ else
+ n = NULL;
+ if (!n)
+ n = sb->buf + sb->len - 1;
+
+ if (pass) {
+ int len = n - p + 1;
+ t = xmalloc(sizeof(struct strbuf));
+ strbuf_init(t, len);
+ strbuf_add(t, p, len);
+ ret[count] = t;
+ }
+ count++;
+ n++;
}
- if (!n)
- n = sb->buf + sb->len - 1;
- len = n - p + 1;
+ }
+ if (!count) {
t = xmalloc(sizeof(struct strbuf));
- strbuf_init(t, len);
- strbuf_add(t, p, len);
- ret[pos] = t;
- ret[++pos] = NULL;
- p = ++n;
+ strbuf_init(t, 0);
+ ret[0] = t;
}
return ret;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 01/10] strbuf_split: add a max parameter
2011-06-13 17:30 ` Junio C Hamano
@ 2011-06-13 19:20 ` Jeff King
0 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-13 19:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy
On Mon, Jun 13, 2011 at 10:30:07AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I am tempted to just call this new one strbuf_split and update all
> > callers. There aren't that many.
>
> Yes, that is indeed tempting, and because we have a new parameter the
> compiler will catch any new callers that pop up in a mismerge so that
> would be perfectly safe.
Should we also change the naming later in the series to remain
consistent with strbuf_add. IOW, to end up at:
struct strbuf **strbuf_split(const char *buf, int len, int delim, int max);
struct strbuf **strbuf_split_str(const char *s, int delim, int max);
struct strbuf **strbuf_split_buf(const struct strbuf *, int delim, int max);
(though I think consistency would also dictate "splitstr" and "splitbuf"
without the extra underscore. Personally I find it a bit unreadable).
> > -struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
> > +struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max)
> > {
> > int alloc = 2, pos = 0;
> > char *n, *p;
> > @@ -114,7 +114,10 @@ struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
> > p = n = sb->buf;
> > while (n < sb->buf + sb->len) {
> > int len;
> > - n = memchr(n, delim, sb->len - (n - sb->buf));
> > + if (max <= 0 || pos + 1 < max)
> > + n = memchr(n, delim, sb->len - (n - sb->buf));
> > + else
> > + n = NULL;
> > if (pos + 1 >= alloc) {
> > alloc = alloc * 2;
> > ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
>
> Hmm, even when we know the value of max, we go exponential, and even do so
> by hand without using ALLOC_GROW(). Somewhat sad.
Thanks for reminding me. I noticed it wasn't using ALLOC_GROW, but
decided not to change it because I wanted to introduce an optimization
later on not to grow beyond max. But then I forgot. :)
The optimization I was going to do was to simply allocate "max" slots at
the beginning (if it's defined). You know you can't grow beyond that,
and in most splits with a max, the caller is expecting all of them to be
filled.
But your two-pass patch below is also reasonable.
> Also do we currently rely on the bug that strbuf_split() returns (NULL,)
> instead of ("", NULL) when given an empty string? If not, perhaps...
I assumed that behavior was not a bug (and even had to avoid a segfault
with it in a later series, as you saw). But thinking on it more, it
really is one; splitting even a single character without delimiter ends
up with a non-NULL portion, and I think the empty string should do the
same.
> strbuf.c | 50 +++++++++++++++++++++++++++++++-------------------
> 1 files changed, 31 insertions(+), 19 deletions(-)
I think your patch looks reasonable. In theory doing two passes over a
very large buffer (e.g., splitting lines from a large commit message)
might be slightly less efficient, but I imagine it is drowned out in the
noise of malloc'ing strbufs.
> + for (pass = 0; pass < 2; pass++) {
> + /* First pass counts, second pass allocates and fills */
Maybe it is just me, but I tend not to like writing multi-pass stuff
like this as a for-loop, but instead to factor it into a function with
an "actually allocate" parameter. I find it makes the code much more
obvious.
> + if (!count) {
> t = xmalloc(sizeof(struct strbuf));
> - strbuf_init(t, len);
> - strbuf_add(t, p, len);
> - ret[pos] = t;
> - ret[++pos] = NULL;
> - p = ++n;
> + strbuf_init(t, 0);
> + ret[0] = t;
> }
I think my test in 4/10 (which avoids the segfault by checking
explicitly for NULL in the caller) should go with this part, and then
4/10 can go away.
-Peff
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 02/10] fix "git -c" parsing of values with equals signs
2011-06-09 15:50 ` Jeff King
2011-06-09 15:51 ` [PATCH 01/10] strbuf_split: add a max parameter Jeff King
@ 2011-06-09 15:51 ` Jeff King
2011-06-09 15:52 ` [PATCH 03/10] config: die on error in command-line config Jeff King
` (7 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:51 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
If you do something like:
git -c core.foo="value with = in it" ...
we would split your option on "=" into three fields and
throw away the third one. With this patch we correctly take
everything after the first "=" as the value (keys cannot
have an equals sign in them, so the parsing is unambiguous).
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 2 +-
t/t1300-repo-config.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/config.c b/config.c
index e0b3b80..aa5eb78 100644
--- a/config.c
+++ b/config.c
@@ -45,7 +45,7 @@ static int git_config_parse_parameter(const char *text,
struct strbuf tmp = STRBUF_INIT;
struct strbuf **pair;
strbuf_addstr(&tmp, text);
- pair = strbuf_split(&tmp, '=');
+ pair = strbuf_split_max(&tmp, '=', 2);
if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=')
strbuf_setlen(pair[0], pair[0]->len - 1);
strbuf_trim(pair[0]);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3db5626..ca5058e 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -904,4 +904,10 @@ test_expect_success 'git -c works with aliases of builtins' '
test_cmp expect actual
'
+test_expect_success 'git -c does not split values on equals' '
+ echo "value with = in it" >expect &&
+ git -c core.foo="value with = in it" config core.foo >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 03/10] config: die on error in command-line config
2011-06-09 15:50 ` Jeff King
2011-06-09 15:51 ` [PATCH 01/10] strbuf_split: add a max parameter Jeff King
2011-06-09 15:51 ` [PATCH 02/10] fix "git -c" parsing of values with equals signs Jeff King
@ 2011-06-09 15:52 ` Jeff King
2011-06-09 15:52 ` [PATCH 04/10] config: avoid segfault when parsing " Jeff King
` (6 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:52 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
The error handling for git_config is somewhat confusing. We
collect errors from running git_config_from_file on the
various config files and carefully pass them back up. But
the two odd things are:
1. We actually die on most errors in git_config_from_file.
In fact, the only error we actually pass back up is if
fopen() fails on the file.
2. Most callers of git_config do not check the error
return at all, but will continue if git_config reports
an error.
When the code for "git -c core.foo=bar" was added, it
dutifully passed errors up the call stack, only for them to
be eventually ignored. This makes it inconsistent with the
file-parsing code, which will die when it sees malformed
config. And it's somewhat unsafe, because it means an error
in parsing a typo like:
git -c clean.requireforce=ture clean
will continue the command, ignoring the config the user
tried to give.
Signed-off-by: Jeff King <peff@peff.net>
---
Another option would be to just make git_config call die() on error
instead of returning the error code that is ignored everywhere. But I
wasn't sure if anybody was relying on the "fopen failure is silently
ignored" behavior.
config.c | 2 +-
t/t1300-repo-config.sh | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/config.c b/config.c
index aa5eb78..ebd404a 100644
--- a/config.c
+++ b/config.c
@@ -856,7 +856,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
switch (git_config_from_parameters(fn, data)) {
case -1: /* error */
- ret--;
+ die("unable to parse command-line config");
break;
case 0: /* found nothing */
break;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index ca5058e..584e956 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -910,4 +910,12 @@ test_expect_success 'git -c does not split values on equals' '
test_cmp expect actual
'
+test_expect_success 'git -c dies on bogus config' '
+ test_must_fail git -c core.bare=foo rev-parse
+'
+
+test_expect_success 'git -c complains about empty key' '
+ test_must_fail git -c "=foo" rev-parse
+'
+
test_done
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 04/10] config: avoid segfault when parsing command-line config
2011-06-09 15:50 ` Jeff King
` (2 preceding siblings ...)
2011-06-09 15:52 ` [PATCH 03/10] config: die on error in command-line config Jeff King
@ 2011-06-09 15:52 ` Jeff King
2011-06-13 17:30 ` Junio C Hamano
2011-06-09 15:54 ` [PATCH 05/10] strbuf: allow strbuf_split to work on non-strbufs Jeff King
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:52 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
We already check for an empty key on the left side of an
equals, but we would segfault if there was no content at
all.
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 2 ++
t/t1300-repo-config.sh | 4 ++++
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/config.c b/config.c
index ebd404a..2517b62 100644
--- a/config.c
+++ b/config.c
@@ -46,6 +46,8 @@ static int git_config_parse_parameter(const char *text,
struct strbuf **pair;
strbuf_addstr(&tmp, text);
pair = strbuf_split_max(&tmp, '=', 2);
+ if (!pair[0])
+ return error("bogus config parameter: %s", text);
if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=')
strbuf_setlen(pair[0], pair[0]->len - 1);
strbuf_trim(pair[0]);
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 584e956..3e140c1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -918,4 +918,8 @@ test_expect_success 'git -c complains about empty key' '
test_must_fail git -c "=foo" rev-parse
'
+test_expect_success 'git -c complains about empty key and value' '
+ test_must_fail git -c "" rev-parse
+'
+
test_done
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 04/10] config: avoid segfault when parsing command-line config
2011-06-09 15:52 ` [PATCH 04/10] config: avoid segfault when parsing " Jeff King
@ 2011-06-13 17:30 ` Junio C Hamano
2011-06-13 19:22 ` Jeff King
0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2011-06-13 17:30 UTC (permalink / raw)
To: Jeff King
Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
Jeff King <peff@peff.net> writes:
> We already check for an empty key on the left side of an
> equals, but we would segfault if there was no content at
> all.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> config.c | 2 ++
> t/t1300-repo-config.sh | 4 ++++
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/config.c b/config.c
> index ebd404a..2517b62 100644
> --- a/config.c
> +++ b/config.c
> @@ -46,6 +46,8 @@ static int git_config_parse_parameter(const char *text,
> struct strbuf **pair;
> strbuf_addstr(&tmp, text);
> pair = strbuf_split_max(&tmp, '=', 2);
> + if (!pair[0])
> + return error("bogus config parameter: %s", text);
This feels wrong.
Asking strbuf_split() to split a string "foo" with "=" delimiter would
give you one element array ("foo", NULL), a string "fo" would give you
("fo", NULL), and a string "f" would give you ("f", NULL). Shouldn't we
get ("", NULL) if we ask it to split ""?
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 04/10] config: avoid segfault when parsing command-line config
2011-06-13 17:30 ` Junio C Hamano
@ 2011-06-13 19:22 ` Jeff King
0 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-13 19:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy
On Mon, Jun 13, 2011 at 10:30:22AM -0700, Junio C Hamano wrote:
> > + if (!pair[0])
> > + return error("bogus config parameter: %s", text);
>
> This feels wrong.
>
> Asking strbuf_split() to split a string "foo" with "=" delimiter would
> give you one element array ("foo", NULL), a string "fo" would give you
> ("fo", NULL), and a string "f" would give you ("f", NULL). Shouldn't we
> get ("", NULL) if we ask it to split ""?
Yeah, I was making the assumption that strbuf_split was not bugging, and
coding to its output. But I think you are right, that it is simply
returning bogus output.
-Peff
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 05/10] strbuf: allow strbuf_split to work on non-strbufs
2011-06-09 15:50 ` Jeff King
` (3 preceding siblings ...)
2011-06-09 15:52 ` [PATCH 04/10] config: avoid segfault when parsing " Jeff King
@ 2011-06-09 15:54 ` Jeff King
2011-06-09 15:55 ` [PATCH 06/10] config: use strbuf_split_str instead of a temporary strbuf Jeff King
` (4 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:54 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
The strbuf_split function takes a strbuf as input, and
outputs a list of strbufs. However, there is no reason that
the input has to be a strbuf, and not an arbitrary buffer.
This patch adds strbuf_split_buf for a length-delimited
buffer, and strbuf_split_str for NUL-terminated strings.
Signed-off-by: Jeff King <peff@peff.net>
---
This makes the naming convention slightly different than "add", which
uses:
add - add data by pointer and length
addstr - add data by pointer to NUL-terminated string
addbuf - add strbuf
but here we have:
strbuf_split - split a strbuf
strbuf_split_str - split a NUL-terminated string
strbuf_split_buf - split pointer and length
It's that way because strbuf_split was introduced first with the current
semantics. I don't know if it is worth fixing the inconsistency.
strbuf.c | 12 ++++++------
strbuf.h | 12 +++++++++++-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 64f6c1e..1a7df12 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -103,19 +103,19 @@ void strbuf_ltrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
-struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max)
+struct strbuf **strbuf_split_buf(const char *str, size_t slen, int delim, int max)
{
int alloc = 2, pos = 0;
- char *n, *p;
+ const char *n, *p;
struct strbuf **ret;
struct strbuf *t;
ret = xcalloc(alloc, sizeof(struct strbuf *));
- p = n = sb->buf;
- while (n < sb->buf + sb->len) {
+ p = n = str;
+ while (n < str + slen) {
int len;
if (max <= 0 || pos + 1 < max)
- n = memchr(n, delim, sb->len - (n - sb->buf));
+ n = memchr(n, delim, slen - (n - str));
else
n = NULL;
if (pos + 1 >= alloc) {
@@ -123,7 +123,7 @@ struct strbuf **strbuf_split_max(const struct strbuf *sb, int delim, int max)
ret = xrealloc(ret, sizeof(struct strbuf *) * alloc);
}
if (!n)
- n = sb->buf + sb->len - 1;
+ n = str + slen - 1;
len = n - p + 1;
t = xmalloc(sizeof(struct strbuf));
strbuf_init(t, len);
diff --git a/strbuf.h b/strbuf.h
index 4cf1dcd..46a33f8 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -44,8 +44,18 @@ extern void strbuf_rtrim(struct strbuf *);
extern void strbuf_ltrim(struct strbuf *);
extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
-extern struct strbuf **strbuf_split_max(const struct strbuf *,
+extern struct strbuf **strbuf_split_buf(const char *, size_t,
int delim, int max);
+static inline struct strbuf **strbuf_split_str(const char *str,
+ int delim, int max)
+{
+ return strbuf_split_buf(str, strlen(str), delim, max);
+}
+static inline struct strbuf **strbuf_split_max(const struct strbuf *sb,
+ int delim, int max)
+{
+ return strbuf_split_buf(sb->buf, sb->len, delim, max);
+}
static inline struct strbuf **strbuf_split(const struct strbuf *sb, int delim)
{
return strbuf_split_max(sb, delim, 0);
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 06/10] config: use strbuf_split_str instead of a temporary strbuf
2011-06-09 15:50 ` Jeff King
` (4 preceding siblings ...)
2011-06-09 15:54 ` [PATCH 05/10] strbuf: allow strbuf_split to work on non-strbufs Jeff King
@ 2011-06-09 15:55 ` Jeff King
2011-06-09 15:55 ` [PATCH 07/10] parse-options: add OPT_STRING_LIST helper Jeff King
` (3 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:55 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
This saves an allocation and copy, and also fixes a minor
memory leak.
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 2517b62..189b766 100644
--- a/config.c
+++ b/config.c
@@ -42,10 +42,8 @@ void git_config_push_parameter(const char *text)
static int git_config_parse_parameter(const char *text,
config_fn_t fn, void *data)
{
- struct strbuf tmp = STRBUF_INIT;
struct strbuf **pair;
- strbuf_addstr(&tmp, text);
- pair = strbuf_split_max(&tmp, '=', 2);
+ pair = strbuf_split_str(text, '=', 2);
if (!pair[0])
return error("bogus config parameter: %s", text);
if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=')
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 07/10] parse-options: add OPT_STRING_LIST helper
2011-06-09 15:50 ` Jeff King
` (5 preceding siblings ...)
2011-06-09 15:55 ` [PATCH 06/10] config: use strbuf_split_str instead of a temporary strbuf Jeff King
@ 2011-06-09 15:55 ` Jeff King
2011-06-09 15:55 ` [PATCH 08/10] remote: use new OPT_STRING_LIST Jeff King
` (2 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:55 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
This just adds repeated invocations of an option to a list
of strings. Using the "--no-<var>" form will reset the list
to empty.
Signed-off-by: Jeff King <peff@peff.net>
---
parse-options.c | 17 +++++++++++++++++
parse-options.h | 4 ++++
t/t0040-parse-options.sh | 17 +++++++++++++++++
test-parse-options.c | 6 ++++++
4 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 73bd28a..879ea82 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
#include "cache.h"
#include "commit.h"
#include "color.h"
+#include "string-list.h"
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -687,3 +688,19 @@ int parse_options_concat(struct option *dst, size_t dst_size, struct option *src
}
return -1;
}
+
+int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
+{
+ struct string_list *v = opt->value;
+
+ if (unset) {
+ string_list_clear(v, 0);
+ return 0;
+ }
+
+ if (!arg)
+ return -1;
+
+ string_list_append(v, xstrdup(arg));
+ return 0;
+}
diff --git a/parse-options.h b/parse-options.h
index d1b12fe..05eb09b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -130,6 +130,9 @@ struct option {
(h), PARSE_OPT_NOARG, NULL, (p) }
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), "n", (h) }
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
+#define OPT_STRING_LIST(s, l, v, a, h) \
+ { OPTION_CALLBACK, (s), (l), (v), (a), \
+ (h), 0, &parse_opt_string_list }
#define OPT_UYN(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG, &parse_opt_tertiary }
#define OPT_DATE(s, l, v, h) \
@@ -204,6 +207,7 @@ extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
extern int parse_opt_with_commit(const struct option *, const char *, int);
extern int parse_opt_tertiary(const struct option *, const char *, int);
+extern int parse_opt_string_list(const struct option *, const char *, int);
#define OPT__VERBOSE(var, h) OPT_BOOLEAN('v', "verbose", (var), (h))
#define OPT__QUIET(var, h) OPT_BOOLEAN('q', "quiet", (var), (h))
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ae26614..007f39d 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -28,6 +28,7 @@ String options
--st <st> get another string (pervert ordering)
-o <str> get another string
--default-string set string to default
+ --list <str> add str to list
Magic arguments
--quux means --quux
@@ -337,4 +338,20 @@ test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '
test_cmp expect output
'
+cat >>expect <<'EOF'
+list: foo
+list: bar
+list: baz
+EOF
+test_expect_success '--list keeps list of strings' '
+ test-parse-options --list foo --list=bar --list=baz >output &&
+ test_cmp expect output
+'
+
+test_expect_success '--no-list resets list' '
+ test-parse-options --list=other --list=irrelevant --list=options \
+ --no-list --list=foo --list=bar --list=baz >output &&
+ test_cmp expect output
+'
+
test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 4e3710b..91a5701 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "parse-options.h"
+#include "string-list.h"
static int boolean = 0;
static int integer = 0;
@@ -9,6 +10,7 @@ static int verbose = 0, dry_run = 0, quiet = 0;
static char *string = NULL;
static char *file = NULL;
static int ambiguous;
+static struct string_list list;
static int length_callback(const struct option *opt, const char *arg, int unset)
{
@@ -54,6 +56,7 @@ int main(int argc, const char **argv)
OPT_STRING('o', NULL, &string, "str", "get another string"),
OPT_SET_PTR(0, "default-string", &string,
"set string to default", (unsigned long)"default"),
+ OPT_STRING_LIST(0, "list", &list, "str", "add str to list"),
OPT_GROUP("Magic arguments"),
OPT_ARGUMENT("quux", "means --quux"),
OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
@@ -85,6 +88,9 @@ int main(int argc, const char **argv)
printf("dry run: %s\n", dry_run ? "yes" : "no");
printf("file: %s\n", file ? file : "(not set)");
+ for (i = 0; i < list.nr; i++)
+ printf("list: %s\n", list.items[i].string);
+
for (i = 0; i < argc; i++)
printf("arg %02d: %s\n", i, argv[i]);
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 08/10] remote: use new OPT_STRING_LIST
2011-06-09 15:50 ` Jeff King
` (6 preceding siblings ...)
2011-06-09 15:55 ` [PATCH 07/10] parse-options: add OPT_STRING_LIST helper Jeff King
@ 2011-06-09 15:55 ` Jeff King
2011-06-09 15:56 ` [PATCH 09/10] config: make git_config_parse_parameter a public function Jeff King
2011-06-09 15:57 ` [PATCH 10/10] clone: accept config options on the command line Jeff King
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:55 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
This saves us having our own callback function.
Signed-off-by: Jeff King <peff@peff.net>
---
This isn't technically related to the rest of the series, but is a
cleanup we can do because of the previous commit.
builtin/remote.c | 14 ++------------
1 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 9ff1cac..05b1f5b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -88,16 +88,6 @@ static inline int postfixcmp(const char *string, const char *postfix)
return strcmp(string + len1 - len2, postfix);
}
-static int opt_parse_track(const struct option *opt, const char *arg, int not)
-{
- struct string_list *list = opt->value;
- if (not)
- string_list_clear(list, 0);
- else
- string_list_append(list, arg);
- return 0;
-}
-
static int fetch_remote(const char *name)
{
const char *argv[] = { "fetch", name, NULL, NULL };
@@ -176,8 +166,8 @@ static int add(int argc, const char **argv)
TAGS_SET),
OPT_SET_INT(0, NULL, &fetch_tags,
"or do not fetch any tag at all (--no-tags)", TAGS_UNSET),
- OPT_CALLBACK('t', "track", &track, "branch",
- "branch(es) to track", opt_parse_track),
+ OPT_STRING_LIST('t', "track", &track, "branch",
+ "branch(es) to track"),
OPT_STRING('m', "master", &master, "branch", "master branch"),
{ OPTION_CALLBACK, 0, "mirror", &mirror, "push|fetch",
"set up remote as a mirror to push to or fetch from",
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/10] config: make git_config_parse_parameter a public function
2011-06-09 15:50 ` Jeff King
` (7 preceding siblings ...)
2011-06-09 15:55 ` [PATCH 08/10] remote: use new OPT_STRING_LIST Jeff King
@ 2011-06-09 15:56 ` Jeff King
2011-06-09 15:57 ` [PATCH 10/10] clone: accept config options on the command line Jeff King
9 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:56 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
We use this internally to parse "git -c core.foo=bar", but
the general format of "key=value" is useful for other
places.
Signed-off-by: Jeff King <peff@peff.net>
---
This could perhaps have a better name than "parameter". I dunno.
cache.h | 2 ++
config.c | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index e11cf6a..933be72 100644
--- a/cache.h
+++ b/cache.h
@@ -1063,6 +1063,8 @@ extern int config_error_nonbool(const char *);
extern const char *get_log_output_encoding(void);
extern const char *get_commit_output_encoding(void);
+extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
+
extern const char *config_exclusive_filename;
#define MAX_GITNAME (1000)
diff --git a/config.c b/config.c
index 189b766..25d998c 100644
--- a/config.c
+++ b/config.c
@@ -39,8 +39,8 @@ void git_config_push_parameter(const char *text)
strbuf_release(&env);
}
-static int git_config_parse_parameter(const char *text,
- config_fn_t fn, void *data)
+int git_config_parse_parameter(const char *text,
+ config_fn_t fn, void *data)
{
struct strbuf **pair;
pair = strbuf_split_str(text, '=', 2);
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/10] clone: accept config options on the command line
2011-06-09 15:50 ` Jeff King
` (8 preceding siblings ...)
2011-06-09 15:56 ` [PATCH 09/10] config: make git_config_parse_parameter a public function Jeff King
@ 2011-06-09 15:57 ` Jeff King
2011-06-09 17:10 ` Bert Wesarg
2011-06-09 22:34 ` Matthieu Moy
9 siblings, 2 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 15:57 UTC (permalink / raw)
To: Claire Fousse; +Cc: git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
Clone does all of init, "remote add", fetch, and checkout
without giving the user a chance to intervene and set any
configuration. This patch allows you to set config options
in the newly created repository after the clone, but before
we do any other operations.
In many cases, this is a minor convenience over something
like:
git clone git://...
git config core.whatever true
But in some cases, it can bring extra efficiency by changing
how the fetch or checkout work. For example, setting
line-ending config before the checkout avoids having to
re-checkout all of the contents with the correct line
endings.
It also provides a mechanism for passing information to remote
helpers during a clone; the helpers may read the git config
to influence how they operate.
Signed-off-by: Jeff King <peff@peff.net>
---
Yay, the actual patch. Even if we don't end up using this for transport
helpers, I think it's a sane thing to allow (e.g., see the final test
below).
Documentation/git-clone.txt | 9 +++++++++
builtin/clone.c | 21 ++++++++++++++++++++-
t/t5707-clone-config.sh | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+), 1 deletions(-)
create mode 100755 t/t5707-clone-config.sh
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index b093e45..3e24064 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -159,6 +159,15 @@ objects from the source repository into a pack in the cloned repository.
Specify the directory from which templates will be used;
(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
+--config <key>=<value>::
+-c <key>=<value>::
+ Set a configuration variable in the newly-cloned repository. The
+ key is in the same as expected by linkgit:git-config[1] (e.g.,
+ `core.eol=true`). If multiple values are given for the same key,
+ each value will be written to the config file. This makes it
+ safe, for example, to add additional fetch refspecs to the
+ origin remote.
+
--depth <depth>::
Create a 'shallow' clone with a history truncated to the
specified number of revisions. A shallow repository has a
diff --git a/builtin/clone.c b/builtin/clone.c
index f579794..a15784a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -46,6 +46,7 @@ static const char *real_git_dir;
static char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
static int option_progress;
+static struct string_list option_config;
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -83,7 +84,8 @@ static struct option builtin_clone_options[] = {
"create a shallow clone of that depth"),
OPT_STRING(0, "separate-git-dir", &real_git_dir, "gitdir",
"separate git dir from working tree"),
-
+ OPT_STRING_LIST('c', "config", &option_config, "key=value",
+ "set config inside the new repository"),
OPT_END()
};
@@ -364,6 +366,22 @@ static void write_remote_refs(const struct ref *local_refs)
clear_extra_refs();
}
+static int write_one_config(const char *key, const char *value, void *data)
+{
+ return git_config_set_multivar(key, value ? value : "true", "^$", 0);
+}
+
+static void write_config(struct string_list *config)
+{
+ int i;
+
+ for (i = 0; i < config->nr; i++) {
+ if (git_config_parse_parameter(config->items[i].string,
+ write_one_config, NULL) < 0)
+ die("unable to write parameters to config file");
+ }
+}
+
int cmd_clone(int argc, const char **argv, const char *prefix)
{
int is_bundle = 0, is_local;
@@ -482,6 +500,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
printf(_("Cloning into %s...\n"), dir);
}
init_db(option_template, INIT_DB_QUIET);
+ write_config(&option_config);
/*
* At this point, the config exists, so we do not need the
diff --git a/t/t5707-clone-config.sh b/t/t5707-clone-config.sh
new file mode 100755
index 0000000..27d730c
--- /dev/null
+++ b/t/t5707-clone-config.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='tests for git clone -c key=value'
+. ./test-lib.sh
+
+test_expect_success 'clone -c sets config in cloned repo' '
+ rm -rf child &&
+ git clone -c core.foo=bar . child &&
+ echo bar >expect &&
+ git --git-dir=child/.git config core.foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone -c can set multi-keys' '
+ rm -rf child &&
+ git clone -c core.foo=bar -c core.foo=baz . child &&
+ { echo bar; echo baz; } >expect &&
+ git --git-dir=child/.git config --get-all core.foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone -c without a value is boolean true' '
+ rm -rf child &&
+ git clone -c core.foo . child &&
+ echo true >expect &&
+ git --git-dir=child/.git config --bool core.foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone -c config is available during clone' '
+ echo content >file &&
+ git add file &&
+ git commit -m one &&
+ rm -rf child &&
+ git clone -c core.autocrlf . child &&
+ printf "content\\r\\n" >expect &&
+ test_cmp expect child/file
+'
+
+test_done
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 10/10] clone: accept config options on the command line
2011-06-09 15:57 ` [PATCH 10/10] clone: accept config options on the command line Jeff King
@ 2011-06-09 17:10 ` Bert Wesarg
2011-06-09 17:12 ` Jeff King
2011-06-09 22:34 ` Matthieu Moy
1 sibling, 1 reply; 25+ messages in thread
From: Bert Wesarg @ 2011-06-09 17:10 UTC (permalink / raw)
To: Jeff King
Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
On Thu, Jun 9, 2011 at 17:57, Jeff King <peff@peff.net> wrote:
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index b093e45..3e24064 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -159,6 +159,15 @@ objects from the source repository into a pack in the cloned repository.
> Specify the directory from which templates will be used;
> (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
>
> +--config <key>=<value>::
> +-c <key>=<value>::
> + Set a configuration variable in the newly-cloned repository. The
Shouldn't this be 'newly-created repository', because the
configuration variables are set before the actual cloning, but after
initializing the new repository.
Bert
> + key is in the same as expected by linkgit:git-config[1] (e.g.,
> + `core.eol=true`). If multiple values are given for the same key,
> + each value will be written to the config file. This makes it
> + safe, for example, to add additional fetch refspecs to the
> + origin remote.
> +
> --depth <depth>::
> Create a 'shallow' clone with a history truncated to the
> specified number of revisions. A shallow repository has a
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/10] clone: accept config options on the command line
2011-06-09 17:10 ` Bert Wesarg
@ 2011-06-09 17:12 ` Jeff King
2011-06-09 20:56 ` Jeff King
0 siblings, 1 reply; 25+ messages in thread
From: Jeff King @ 2011-06-09 17:12 UTC (permalink / raw)
To: Bert Wesarg
Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
On Thu, Jun 09, 2011 at 07:10:33PM +0200, Bert Wesarg wrote:
> On Thu, Jun 9, 2011 at 17:57, Jeff King <peff@peff.net> wrote:
> > diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> > index b093e45..3e24064 100644
> > --- a/Documentation/git-clone.txt
> > +++ b/Documentation/git-clone.txt
> > @@ -159,6 +159,15 @@ objects from the source repository into a pack in the cloned repository.
> > Specify the directory from which templates will be used;
> > (See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
> >
> > +--config <key>=<value>::
> > +-c <key>=<value>::
> > + Set a configuration variable in the newly-cloned repository. The
>
> Shouldn't this be 'newly-created repository', because the
> configuration variables are set before the actual cloning, but after
> initializing the new repository.
Yeah, I think your wording is better. It might also make sense to
explicitly say that the config takes effect during the fetch and
checkout phases of the clone (there's otherwise no point, of course, but
it never hurts to make the documentation clear).
-Peff
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 10/10] clone: accept config options on the command line
2011-06-09 17:12 ` Jeff King
@ 2011-06-09 20:56 ` Jeff King
0 siblings, 0 replies; 25+ messages in thread
From: Jeff King @ 2011-06-09 20:56 UTC (permalink / raw)
To: Bert Wesarg
Cc: Claire Fousse, git, Sylvain Boulme, Matthieu Moy, Junio C Hamano
On Thu, Jun 09, 2011 at 01:12:14PM -0400, Jeff King wrote:
> > > +-c <key>=<value>::
> > > + Set a configuration variable in the newly-cloned repository. The
> >
> > Shouldn't this be 'newly-created repository', because the
> > configuration variables are set before the actual cloning, but after
> > initializing the new repository.
>
> Yeah, I think your wording is better. It might also make sense to
> explicitly say that the config takes effect during the fetch and
> checkout phases of the clone (there's otherwise no point, of course, but
> it never hurts to make the documentation clear).
There was also an unrelated typo in the newly-added documentation. So
here's a replacement 10/10 with the fixed documentation.
-- >8 --
Subject: [PATCH] clone: accept config options on the command line
Clone does all of init, "remote add", fetch, and checkout
without giving the user a chance to intervene and set any
configuration. This patch allows you to set config options
in the newly created repository after the clone, but before
we do any other operations.
In many cases, this is a minor convenience over something
like:
git clone git://...
git config core.whatever true
But in some cases, it can bring extra efficiency by changing
how the fetch or checkout work. For example, setting
line-ending config before the checkout avoids having to
re-checkout all of the contents with the correct line
endings.
It also provides a mechanism for passing information to remote
helpers during a clone; the helpers may read the git config
to influence how they operate.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-clone.txt | 11 +++++++++++
builtin/clone.c | 21 ++++++++++++++++++++-
t/t5707-clone-config.sh | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 71 insertions(+), 1 deletions(-)
create mode 100755 t/t5707-clone-config.sh
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index b093e45..4b8b26b 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -159,6 +159,17 @@ objects from the source repository into a pack in the cloned repository.
Specify the directory from which templates will be used;
(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
+--config <key>=<value>::
+-c <key>=<value>::
+ Set a configuration variable in the newly-created repository;
+ this takes effect immediately after the repository is
+ initialized, but before the remote history is fetched or any
+ files checked out. The key is in the same format as expected by
+ linkgit:git-config[1] (e.g., `core.eol=true`). If multiple
+ values are given for the same key, each value will be written to
+ the config file. This makes it safe, for example, to add
+ additional fetch refspecs to the origin remote.
+
--depth <depth>::
Create a 'shallow' clone with a history truncated to the
specified number of revisions. A shallow repository has a
diff --git a/builtin/clone.c b/builtin/clone.c
index f579794..a15784a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -46,6 +46,7 @@ static const char *real_git_dir;
static char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
static int option_progress;
+static struct string_list option_config;
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -83,7 +84,8 @@ static struct option builtin_clone_options[] = {
"create a shallow clone of that depth"),
OPT_STRING(0, "separate-git-dir", &real_git_dir, "gitdir",
"separate git dir from working tree"),
-
+ OPT_STRING_LIST('c', "config", &option_config, "key=value",
+ "set config inside the new repository"),
OPT_END()
};
@@ -364,6 +366,22 @@ static void write_remote_refs(const struct ref *local_refs)
clear_extra_refs();
}
+static int write_one_config(const char *key, const char *value, void *data)
+{
+ return git_config_set_multivar(key, value ? value : "true", "^$", 0);
+}
+
+static void write_config(struct string_list *config)
+{
+ int i;
+
+ for (i = 0; i < config->nr; i++) {
+ if (git_config_parse_parameter(config->items[i].string,
+ write_one_config, NULL) < 0)
+ die("unable to write parameters to config file");
+ }
+}
+
int cmd_clone(int argc, const char **argv, const char *prefix)
{
int is_bundle = 0, is_local;
@@ -482,6 +500,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
printf(_("Cloning into %s...\n"), dir);
}
init_db(option_template, INIT_DB_QUIET);
+ write_config(&option_config);
/*
* At this point, the config exists, so we do not need the
diff --git a/t/t5707-clone-config.sh b/t/t5707-clone-config.sh
new file mode 100755
index 0000000..27d730c
--- /dev/null
+++ b/t/t5707-clone-config.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='tests for git clone -c key=value'
+. ./test-lib.sh
+
+test_expect_success 'clone -c sets config in cloned repo' '
+ rm -rf child &&
+ git clone -c core.foo=bar . child &&
+ echo bar >expect &&
+ git --git-dir=child/.git config core.foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone -c can set multi-keys' '
+ rm -rf child &&
+ git clone -c core.foo=bar -c core.foo=baz . child &&
+ { echo bar; echo baz; } >expect &&
+ git --git-dir=child/.git config --get-all core.foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone -c without a value is boolean true' '
+ rm -rf child &&
+ git clone -c core.foo . child &&
+ echo true >expect &&
+ git --git-dir=child/.git config --bool core.foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'clone -c config is available during clone' '
+ echo content >file &&
+ git add file &&
+ git commit -m one &&
+ rm -rf child &&
+ git clone -c core.autocrlf . child &&
+ printf "content\\r\\n" >expect &&
+ test_cmp expect child/file
+'
+
+test_done
--
1.7.6.rc1.36.g91167
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 10/10] clone: accept config options on the command line
2011-06-09 15:57 ` [PATCH 10/10] clone: accept config options on the command line Jeff King
2011-06-09 17:10 ` Bert Wesarg
@ 2011-06-09 22:34 ` Matthieu Moy
1 sibling, 0 replies; 25+ messages in thread
From: Matthieu Moy @ 2011-06-09 22:34 UTC (permalink / raw)
To: Jeff King; +Cc: Claire Fousse, git, Sylvain Boulme, Junio C Hamano
Jeff King <peff@peff.net> writes:
> Yay, the actual patch. Even if we don't end up using this for transport
> helpers, I think it's a sane thing to allow (e.g., see the final test
> below).
Ack.
I reviewed quickly the serie, which seems OK. Nice work, amazing serie
of patches to end up with this trivial one ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 25+ messages in thread