Re: [PATCH] imap-send: clarify CRAM-MD5 vs LOGIN documentation
Junio C Hamano gits...@pobox.com wrote: Both patches make sense to me, but can you please sign-off your patches? Oops, sorry about that. Re-roll on its way... Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Thames, Dover: Southwest 4 or 5, increasing 6 at times. Slight or moderate. Fair. Good. -- 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/2] imap-send: create target mailbox if it is missing
Some MUAs delete their drafts folder when it is empty, so
git imap-send should be able to create it if necessary.
This change checks that the folder exists immediately after
login and tries to create it if it is missing.
There was some vestigial code to handle a [TRYCREATE] response
from the server when an APPEND target is missing. However this
code never ran (the create and trycreate flags were never set)
and when I tried to make it run I found that the code had already
thrown away the contents of the message it was trying to append.
Signed-off-by: Tony Finch d...@dotat.at
---
imap-send.c | 56 +---
1 file changed, 25 insertions(+), 31 deletions(-)
diff --git a/imap-send.c b/imap-send.c
index 524fbab..5e4a24e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -128,7 +128,6 @@ struct imap_cmd_cb {
char *data;
int dlen;
int uid;
- unsigned create:1, trycreate:1;
};
struct imap_cmd {
@@ -714,8 +713,8 @@ static int parse_response_code(struct imap_store *ctx,
struct imap_cmd_cb *cb,
static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
{
struct imap *imap = ctx-imap;
- struct imap_cmd *cmdp, **pcmdp, *ncmdp;
- char *cmd, *arg, *arg1, *p;
+ struct imap_cmd *cmdp, **pcmdp;
+ char *cmd, *arg, *arg1;
int n, resp, resp2, tag;
for (;;) {
@@ -801,30 +800,9 @@ static int get_cmd_result(struct imap_store *ctx, struct
imap_cmd *tcmd)
if (!strcmp(OK, arg))
resp = DRV_OK;
else {
- if (!strcmp(NO, arg)) {
- if (cmdp-cb.create cmd
(cmdp-cb.trycreate || !memcmp(cmd, [TRYCREATE], 11))) { /* SELECT, APPEND or
UID COPY */
- p = strchr(cmdp-cmd, '');
- if (!issue_imap_cmd(ctx, NULL,
CREATE \%.*s\, (int)(strchr(p + 1, '') - p + 1), p)) {
- resp = RESP_BAD;
- goto normal;
- }
- /* not waiting here violates
the spec, but a server that does not
- grok this nonetheless
violates it too. */
- cmdp-cb.create = 0;
- if (!(ncmdp =
issue_imap_cmd(ctx, cmdp-cb, %s, cmdp-cmd))) {
- resp = RESP_BAD;
- goto normal;
- }
- free(cmdp-cmd);
- free(cmdp);
- if (!tcmd)
- return 0; /*
ignored */
- if (cmdp == tcmd)
- tcmd = ncmdp;
- continue;
- }
+ if (!strcmp(NO, arg))
resp = RESP_NO;
- } else /*if (!strcmp(BAD, arg))*/
+ else /*if (!strcmp(BAD, arg))*/
resp = RESP_BAD;
fprintf(stderr, IMAP command '%s' returned
response (%s) - %s\n,
memcmp(cmdp-cmd, LOGIN, 5) ?
@@ -833,7 +811,6 @@ static int get_cmd_result(struct imap_store *ctx, struct
imap_cmd *tcmd)
}
if ((resp2 = parse_response_code(ctx, cmdp-cb, cmd))
resp)
resp = resp2;
- normal:
if (cmdp-cb.done)
cmdp-cb.done(ctx, cmdp, resp);
free(cmdp-cb.data);
@@ -944,7 +921,7 @@ static int auth_cram_md5(struct imap_store *ctx, struct
imap_cmd *cmd, const cha
return 0;
}
-static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
+static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char
*folder)
{
struct credential cred = CREDENTIAL_INIT;
struct imap_store *ctx;
@@ -1156,6 +1133,25 @@ static struct imap_store *imap_open_store(struct
imap_server_conf *srvc)
credential_approve(cred);
credential_clear(cred);
+ /* check the target mailbox exists */
+ ctx-name = folder;
+ switch (imap_exec(ctx, NULL, EXAMINE \%s\, ctx-name)) {
+ case RESP_OK:
+ /* ok */
+ break;
+ case RESP_BAD:
+
[PATCH 1/2] imap-send: clarify CRAM-MD5 vs LOGIN documentation
Explicitly mention that leaving imap.authMethod unset makes git imap-send use the basic IMAP plaintext LOGIN command. Signed-off-by: Tony Finch d...@dotat.at --- Documentation/git-imap-send.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt index 875d283..770cbe8 100644 --- a/Documentation/git-imap-send.txt +++ b/Documentation/git-imap-send.txt @@ -76,7 +76,8 @@ imap.preformattedHTML:: imap.authMethod:: Specify authenticate method for authentication with IMAP server. - Current supported method is 'CRAM-MD5' only. + Current supported method is 'CRAM-MD5' only. If this is not set + then 'git imap-send' uses the basic IMAP plaintext LOGIN command. Examples -- 2.1.0.rc0.229.gaee38de -- 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 v5 3/7] change `git_config()` return value to void
Currently `git_config()` returns an integer signifying an error code.
During a previous rewrite of the function most of the code was shifted
to `git_config_with_options()`. `git_config_with_options()` normally
returns positive values if its `config_source` parameter is set as NULL,
as most errors are fatal, and non-fatal potential errors are guarded
by if statements that are entered only when no error is possible.
Still a negative value can be returned in case of race condition between
`access_or_die()` `git_config_from_file()`. Also, all callers of
`git_config()` ignore the return value except for one case in branch.c.
Change `git_config()` return value to void and make it die if it receives
a negative return value from `git_config_with_options()`.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
branch.c | 5 +
cache.h | 2 +-
config.c | 16 ++--
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/branch.c b/branch.c
index 46e8aa8..735767d 100644
--- a/branch.c
+++ b/branch.c
@@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char
*branch_name)
strbuf_addf(name, branch.%s.description, branch_name);
cb.config_name = name.buf;
cb.value = NULL;
- if (git_config(read_branch_desc_cb, cb) 0) {
- strbuf_release(name);
- return -1;
- }
+ git_config(read_branch_desc_cb, cb);
if (cb.value)
strbuf_addstr(buf, cb.value);
strbuf_release(name);
diff --git a/cache.h b/cache.h
index 7292aef..c61919d 100644
--- a/cache.h
+++ b/cache.h
@@ -1294,7 +1294,7 @@ extern int git_config_from_buf(config_fn_t fn, const char
*name,
const char *buf, size_t len, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern int git_config(config_fn_t fn, void *);
+extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
int respect_includes);
diff --git a/config.c b/config.c
index 4a15383..c0faaca 100644
--- a/config.c
+++ b/config.c
@@ -1235,9 +1235,21 @@ struct key_value_info {
int linenr;
};
-int git_config(config_fn_t fn, void *data)
+void git_config(config_fn_t fn, void *data)
{
- return git_config_with_options(fn, data, NULL, 1);
+ if (git_config_with_options(fn, data, NULL, 1) 0)
+ /*
+* git_config_with_options() normally returns only
+* positive values, as most errors are fatal, and
+* non-fatal potential errors are guarded by if
+* statements that are entered only when no error is
+* possible.
+*
+* If we ever encounter a non-fatal error, it means
+* something went really wrong and we should stop
+* immediately.
+*/
+ die(Unknown error occured while reading the configuration
files);
}
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
--
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 v5 1/7] config.c: fix accuracy of line number in errors
From: Matthieu Moy matthieu@grenoble-inp.fr
If a callback returns a negative value to `git_config*()` family,
they call `die()` while printing the line number and the file name.
Currently the printed line number is off by one, thus printing the
wrong line number.
Make `linenr` point to the line we just parsed during the call
to callback to get accurate line number in error messages.
Commit-message-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index a191328..ed5fc8e 100644
--- a/config.c
+++ b/config.c
@@ -244,6 +244,7 @@ static int get_next_char(void)
cf-linenr++;
if (c == EOF) {
cf-eof = 1;
+ cf-linenr++;
c = '\n';
}
return c;
@@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
{
int c;
char *value;
+ int ret;
/* Get the full name */
for (;;) {
@@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
if (!value)
return -1;
}
- return fn(name-buf, value, data);
+ /*
+* We already consumed the \n, but we need linenr to point to
+* the line we just parsed during the call to fn to get
+* accurate line number in error messages.
+*/
+ cf-linenr--;
+ ret = fn(name-buf, value, data);
+ cf-linenr++;
+ return ret;
}
static int get_extended_base_var(struct strbuf *name, int c)
--
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 7/7] add tests for `git_config_get_string_const()`
Add tests for `git_config_get_string_const()`, check whether it
dies printing the line number and the file name if a NULL
value is retrieved for the given key.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
t/t1308-config-set.sh | 10 ++
test-config.c | 10 ++
2 files changed, 20 insertions(+)
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index e2f9d0b..f012dd6 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -119,6 +119,16 @@ test_expect_success 'find integer value for a key' '
check_config get_int lamb.chop 65
'
+test_expect_success 'find string value for a key' '
+ check_config get_string case.baz hask
+ check_config expect_code 1 get_string case.ba Value not found for
\case.ba\
+'
+
+test_expect_success 'check line error when NULL string is queried' '
+ test_expect_code 128 test-config get_string case.foo 2result
+ grep fatal: bad config variable '\''case.foo'\'' at file line 7 in
.git/config result
+'
+
test_expect_success 'find integer if value is non parse-able' '
check_config expect_code 128 get_int lamb.head
'
diff --git a/test-config.c b/test-config.c
index 9dd1b22..6a77552 100644
--- a/test-config.c
+++ b/test-config.c
@@ -16,6 +16,8 @@
*
* get_bool - print bool value for the entered key or die
*
+ * get_string - print string value for the entered key or die
+ *
* configset_get_value - returns value with the highest priority for the
entered key
* from a config_set constructed from files entered as
arguments.
*
@@ -84,6 +86,14 @@ int main(int argc, char **argv)
printf(Value not found for \%s\\n, argv[2]);
goto exit1;
}
+ } else if (argc == 3 !strcmp(argv[1], get_string)) {
+ if (!git_config_get_string_const(argv[2], v)) {
+ printf(%s\n, v);
+ goto exit0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ goto exit1;
+ }
} else if (!strcmp(argv[1], configset_get_value)) {
for (i = 3; i argc; i++) {
int err;
--
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 v5 5/7] add a test for semantic errors in config files
Semantic errors (for example, for alias.* variables NULL values are not allowed) in configuration files cause a die printing the line number and file name of the offending value. Add a test documenting that such errors cause a die printing the accurate line number and file name. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..e2f9d0b 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -197,4 +197,15 @@ test_expect_success 'proper error on error in custom config files' ' test_cmp expect actual ' +test_expect_success 'check line errors for malformed values' ' + mv .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + cat .git/config -\EOF + [alias] + br + EOF + test_expect_code 128 git br 2result + grep fatal: bad config variable '\''alias.br'\'' at file line 2 in .git/config result +' + test_done -- 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 v5 2/7] add line number and file name info to `config_set`
Store file name and line number for each key-value pair in the cache
during parsing of the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index ed5fc8e..4a15383 100644
--- a/config.c
+++ b/config.c
@@ -1230,6 +1230,11 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
+
int git_config(config_fn_t fn, void *data)
{
return git_config_with_options(fn, data, NULL, 1);
@@ -1260,6 +1265,9 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
static int configset_add_value(struct config_set *cs, const char *key, const
char *value)
{
struct config_set_element *e;
+ struct string_list_item *si;
+ struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
+
e = configset_find_element(cs, key);
/*
* Since the keys are being fed by git_config*() callback mechanism,
they
@@ -1272,7 +1280,16 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
string_list_init(e-value_list, 1);
hashmap_add(cs-config_hash, e);
}
- string_list_append_nodup(e-value_list, value ? xstrdup(value) : NULL);
+ si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+ if (cf) {
+ kv_info-filename = strintern(cf-name);
+ kv_info-linenr = cf-linenr;
+ } else {
+ /* for values read from `git_config_from_parameters()` */
+ kv_info-filename = strintern(parameter);
+ kv_info-linenr = 0;
+ }
+ si-util = kv_info;
return 0;
}
@@ -1299,7 +1316,7 @@ void git_configset_clear(struct config_set *cs)
hashmap_iter_init(cs-config_hash, iter);
while ((entry = hashmap_iter_next(iter))) {
free(entry-key);
- string_list_clear(entry-value_list, 0);
+ string_list_clear(entry-value_list, 1);
}
hashmap_free(cs-config_hash, 1);
cs-hash_initialized = 0;
--
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 6/7] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name
of the highest priority value for the configuration variable `key`.
It has usage in non-callback based config value retrieval where we can
raise an error and die if there is a semantic error.
For example,
if (!git_config_get_value(key, value)) {
/* NULL values not allowed */
if (!value)
git_config_die(key);
else
/* do work */
}
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-config.txt | 5 +
cache.h| 1 +
config.c | 27 +--
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index 815c1ee..e7ec7cc 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,6 +155,11 @@ as well as retrieval for the queried variable, including:
Similar to `git_config_get_string`, but expands `~` or `~user` into
the user's home directory when found at the beginning of the path.
+`void git_die_config(const char *key)`::
+
+ Dies printing the line number and the file name of the highest
+ priority value for the configuration variable `key`.
+
See test-config.c for usage examples.
Value Parsing Helpers
diff --git a/cache.h b/cache.h
index 4d3c6bd..243f618 100644
--- a/cache.h
+++ b/cache.h
@@ -1406,6 +1406,7 @@ extern int git_config_get_bool(const char *key, int
*dest);
extern int git_config_get_bool_or_int(const char *key, int *is_bool, int
*dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
+extern void git_die_config(const char *key);
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index 4937515..03369b8 100644
--- a/config.c
+++ b/config.c
@@ -1515,8 +1515,12 @@ const struct string_list
*git_config_get_value_multi(const char *key)
int git_config_get_string_const(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_string_const(the_config_set, key, dest);
+ ret = git_configset_get_string_const(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
}
int git_config_get_string(const char *key, char **dest)
@@ -1557,10 +1561,29 @@ int git_config_get_maybe_bool(const char *key, int
*dest)
int git_config_get_pathname(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_pathname(the_config_set, key, dest);
+ ret = git_configset_get_pathname(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
}
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(unable to parse '%s' from command-line config, key);
+ else
+ die(bad config variable '%s' at file line %d in %s,
+ key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
+
/*
* Find all the stuff for git_config_set() below.
*/
--
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 v5 4/7] rewrite git_config() to use the config-set API
Of all the functions in `git_config*()` family, `git_config()` has the
most invocations in the whole code base. Each `git_config()` invocation
causes config file rereads which can be avoided using the config-set API.
Use the config-set API to rewrite `git_config()` to use the config caching
layer to avoid config file rereads on each invocation during a git process
lifetime. First invocation constructs the cache, and after that for each
successive invocation, `git_config()` feeds values from the config cache
instead of rereading the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 24 +
config.c| 57 ++---
t/t4055-diff-context.sh | 2 +-
3 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index c61919d..4d3c6bd 100644
--- a/cache.h
+++ b/cache.h
@@ -8,6 +8,7 @@
#include gettext.h
#include convert.h
#include trace.h
+#include string-list.h
#include SHA1_HEADER
#ifndef git_SHA_CTX
@@ -1351,9 +1352,32 @@ extern int parse_config_key(const char *var,
const char **subsection, int *subsection_len,
const char **key);
+struct config_set_element {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list value_list;
+};
+
+struct configset_list_item {
+ struct config_set_element *e;
+ int value_index;
+};
+
+/*
+ * the contents of the list are ordered according to their
+ * position in the config files and order of parsing the files.
+ * (i.e. key-value pair at the last position of .git/config will
+ * be at the last item of the list)
+ */
+struct configset_list {
+ struct configset_list_item *items;
+ unsigned int nr, alloc;
+};
+
struct config_set {
struct hashmap config_hash;
int hash_initialized;
+ struct configset_list list;
};
extern void git_configset_init(struct config_set *cs);
diff --git a/config.c b/config.c
index c0faaca..4937515 100644
--- a/config.c
+++ b/config.c
@@ -35,12 +35,6 @@ struct config_source {
long (*do_ftell)(struct config_source *c);
};
-struct config_set_element {
- struct hashmap_entry ent;
- char *key;
- struct string_list value_list;
-};
-
static struct config_source *cf;
static int zlib_compression_seen;
@@ -1235,7 +1229,7 @@ struct key_value_info {
int linenr;
};
-void git_config(config_fn_t fn, void *data)
+static void git_config_raw(config_fn_t fn, void *data)
{
if (git_config_with_options(fn, data, NULL, 1) 0)
/*
@@ -1252,6 +1246,39 @@ void git_config(config_fn_t fn, void *data)
die(Unknown error occured while reading the configuration
files);
}
+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+ int i, value_index;
+ struct string_list *values;
+ struct config_set_element *entry;
+ struct configset_list *list = cs-list;
+ struct key_value_info *kv_info;
+
+ for (i = 0; i list-nr; i++) {
+ entry = list-items[i].e;
+ value_index = list-items[i].value_index;
+ values = entry-value_list;
+ if (fn(entry-key, values-items[value_index].string, data)
0) {
+ kv_info = values-items[value_index].util;
+ if (!kv_info-linenr)
+ die(unable to parse '%s' from command-line
config, entry-key);
+ else
+ die(bad config variable '%s' at file line %d
in %s,
+ entry-key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
+ }
+}
+
+static void git_config_check_init(void);
+
+void git_config(config_fn_t fn, void *data)
+{
+ git_config_check_init();
+ configset_iter(the_config_set, fn, data);
+}
+
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
{
struct config_set_element k;
@@ -1278,6 +1305,7 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
{
struct config_set_element *e;
struct string_list_item *si;
+ struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
e = configset_find_element(cs, key);
@@ -1293,6 +1321,12 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+
+ ALLOC_GROW(cs-list.items, cs-list.nr + 1, cs-list.alloc);
+ l_item = cs-list.items[cs-list.nr++];
+ l_item-e = e;
+ l_item-value_index = e-value_list.nr - 1;
+
if (cf) {
[PATCH v5 0/7] Rewrite `git_config()` using config-set API
[PATCH v5]: New patch added (3/7). git_config() now returns void. [PATCH v4]: One style nit corrected, also added key to error messages. Diff with v3 at the bottom for easy review. [PATCH V3]:All the suggestions in [3] applied. Built on top of [1]. [PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows correct parsing order. Reordered the patches. Removed xfuncname patch as it was unnecssary. This series builds on the top of topic[1] in the mailing list with name git config cache special querying API utilizing the cache. This series aims to do these three things, * Use the config-set API to rewrite git_config(). * Solve any legacy bugs in the previous system while at it. * To be feature complete compared to the previous git_config() implementation, which I think it is now. (added the line number and file name info just for completeness) Also, I haven't yet checked the exact improvements but still as a teaser, git status now only rereads the configuration files twice instead of four times. [1]: http://thread.gmane.org/gmane.comp.version-control.git/254286 [2]: http://thread.gmane.org/gmane.comp.version-control.git/254101 [3]: http://thread.gmane.org/gmane.comp.version-control.git/254211 Tanay Abhra (7): config.c: fix accuracy of line number in errors add line number and file name info to `config_set` change `git_config()` return value to void rewrite git_config() to use the config-set API add a test for semantic errors in config files config: add `git_die_config()` to the config-set API add tests for `git_config_get_string_const()` Documentation/technical/api-config.txt | 5 ++ branch.c | 5 +- cache.h| 27 ++- config.c | 131 + t/t1308-config-set.sh | 21 ++ t/t4055-diff-context.sh| 2 +- test-config.c | 10 +++ 7 files changed, 181 insertions(+), 20 deletions(-) -- 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
Re: [PATCH v2 2/2] Make locked paths absolute when current directory is changed
On Thu, Jul 31, 2014 at 10:01 AM, Yue Lin Ho yuelinho...@gmail.com wrote: Hi: How do I trace these patches applied? They are not applied yet. I'll needto redo them on top of rs/strbuf-getcwd. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] nd/multiple-work-trees follow-ups
On Thu, Jul 31, 2014 at 12:51 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: The series has entered 'next' so I can't replace patches any more. Besides the brown paper bag fixes, checkout now rejects if a branch is already checked out elsewhere. I do not think we would want to rush the entire series to 'master' before the upcoming release, so we could squash these fixes into the original when we rewind 'next' post release, if you wanted to. Great. Please remove it from next at the next rewind. The fix-ups are easier to review than wholesale replacements; keep them coming as needed. Will do. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/7] change `git_config()` return value to void
Tanay Abhra tanay...@gmail.com writes:
Signed-off-by: Tanay Abhra tanay...@gmail.com
I think I deserve a bit of credit here ;-).
{
- return git_config_with_options(fn, data, NULL, 1);
+ if (git_config_with_options(fn, data, NULL, 1) 0)
+ /*
+ * git_config_with_options() normally returns only
+ * positive values, as most errors are fatal, and
+ * non-fatal potential errors are guarded by if
+ * statements that are entered only when no error is
+ * possible.
+ *
+ * If we ever encounter a non-fatal error, it means
+ * something went really wrong and we should stop
+ * immediately.
+ */
+ die(Unknown error occured while reading the configuration
files);
}
My bad, but this should be die(_(...));, so that the message can be
translated. Not really serious since it's not really meant to be seen by
the user, though.
--
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: [PATCH v5 4/7] rewrite git_config() to use the config-set API
Tanay Abhra tanay...@gmail.com writes: + if (!kv_info-linenr) + die(unable to parse '%s' from command-line config, entry-key); + else + die(bad config variable '%s' at file line %d in %s, Also two missing _(...) here. Note that there are missing _(...) in config.c before your patch too. I'll send a patch to fix this regardless of your series. -- 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: [PATCH 6/7] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes: + if (!kv_info-linenr) + die(unable to parse '%s' from command-line config, key); + else + die(bad config variable '%s' at file line %d in %s, And two more missing _(...). -- 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
[PATCH] config.c: mark error and warnings strings for translation
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Noticed while reviewing Tanay's patches, but this one is independant
from his series, both syntactically and semantically.
config.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/config.c b/config.c
index a191328..76eeb63 100644
--- a/config.c
+++ b/config.c
@@ -457,9 +457,9 @@ static int git_parse_source(config_fn_t fn, void *data)
break;
}
if (cf-die_on_error)
- die(bad config file line %d in %s, cf-linenr, cf-name);
+ die(_(bad config file line %d in %s), cf-linenr, cf-name);
else
- return error(bad config file line %d in %s, cf-linenr,
cf-name);
+ return error(_(bad config file line %d in %s), cf-linenr,
cf-name);
}
static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -575,9 +575,9 @@ static void die_bad_number(const char *name, const char
*value)
value = ;
if (cf cf-name)
- die(bad numeric config value '%s' for '%s' in %s: %s,
+ die(_(bad numeric config value '%s' for '%s' in %s: %s),
value, name, cf-name, reason);
- die(bad numeric config value '%s' for '%s': %s, value, name, reason);
+ die(_(bad numeric config value '%s' for '%s': %s), value, name,
reason);
}
int git_config_int(const char *name, const char *value)
@@ -662,7 +662,7 @@ int git_config_pathname(const char **dest, const char *var,
const char *value)
return config_error_nonbool(var);
*dest = expand_user_path(value);
if (!*dest)
- die(Failed to expand user dir in: '%s', value);
+ die(_(Failed to expand user dir in: '%s'), value);
return 0;
}
@@ -740,7 +740,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
zlib_compression_level = level;
zlib_compression_seen = 1;
return 0;
@@ -751,7 +751,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
core_compression_level = level;
core_compression_seen = 1;
if (!zlib_compression_seen)
@@ -873,7 +873,7 @@ static int git_default_core_config(const char *var, const
char *value)
else if (!strcmp(value, link))
object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
else
- die(Invalid mode for object creation: %s, value);
+ die(_(Invalid mode for object creation: %s), value);
return 0;
}
@@ -1173,7 +1173,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 */
- die(unable to parse command-line config);
+ die(_(unable to parse command-line config));
break;
case 0: /* found nothing */
break;
@@ -1514,7 +1514,7 @@ static int store_aux(const char *key, const char *value,
void *cb)
case KEY_SEEN:
if (matches(key, value)) {
if (store.seen == 1 store.multi_replace == 0) {
- warning(%s has multiple values, key);
+ warning(_(%s has multiple values), key);
}
ALLOC_GROW(store.offset, store.seen + 1,
--
2.0.2.737.gfb43bde
--
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 3/7] change `git_config()` return value to void
Matthieu Moy matthieu@grenoble-inp.fr writes:
Tanay Abhra tanay...@gmail.com writes:
Signed-off-by: Tanay Abhra tanay...@gmail.com
I think I deserve a bit of credit here ;-).
{
-return git_config_with_options(fn, data, NULL, 1);
+if (git_config_with_options(fn, data, NULL, 1) 0)
+/*
+ * git_config_with_options() normally returns only
+ * positive values, as most errors are fatal, and
+ * non-fatal potential errors are guarded by if
+ * statements that are entered only when no error is
+ * possible.
+ *
+ * If we ever encounter a non-fatal error, it means
+ * something went really wrong and we should stop
+ * immediately.
+ */
+die(Unknown error occured while reading the configuration
files);
}
My bad, but this should be die(_(...));, so that the message can be
translated. Not really serious since it's not really meant to be seen by
the user, though.
Also, other error messages do not start with a capital, hence it should
be unknown..., not Unknown. My bad again.
--
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: [PATCH v4 0/5] git_config callers rewritten with the new config cache API
On 30/07/14 17:45, Matthieu Moy wrote: Tanay Abhra tanay...@gmail.com writes: Also, any thoughts on what to do with git_default_config()? We can, 1 make a new function git_load_default_config(), use it for the rewrites. That seems the most sensible option. It could be called it git.c before the command-dependant part, so that any call to git loads this. I didn't check if it was correct (e.g. do some command rely on the fact that the default config is not loaded?) Hmm, here be dragons ... :-P I don't know that there has actually been any kind of policy regarding the reading of config files/variables in git (or if there is a different policy for plumbing vs porcelain), but it has always seemed to be somewhat ad-hoc; each command decides for itself what it wants to read. However, with increased use of common code which _uses_ certain config variables for correct operation, the 'choice' is much harder to make (and may change after the fact!). For example, about a year ago I submitted a couple of patches which added a call to git_config(git_default_config, NULL) to both 'git pack-refs' and 'git show-refs'. This was as a result of the 'mh/ref-races' branch which introduced a 'stat_validity' api for checking if the packed-refs file had changed on the filesystem since last you looked. This re-used some of the same code used to handle index updates that used config variables like core.checkstat and core.trustctime. These config variables can affect the correctness and/or the efficiency of the code on some platforms (e.g. cygwin, mingw). [Note: 'stat_validity' has since been re-used again (why not?) in some shallow clone code, so similar comments may apply ... I haven't looked.] However, those patches were dropped, because it resulted in an (unwanted) change in behaviour. In particular, 'git show-refs' changed behaviour because it now 'listened' to core.abbrev! I started to look at splitting the 'core config variables' into two groups; essential variables that _all_ git commands should read for correct/efficient/consistent behaviour and everything else (mainly UI related variables). However, something else came up ... Just an FYI. ATB, Ramsay Jones -- 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 v4 2/5] notes.c: replace `git_config()` with `git_config_get_value_multi()`
Matthieu Moy matthieu@grenoble-inp.fr writes:
Tanay Abhra tanay...@gmail.com writes:
On 7/30/2014 7:43 PM, Matthieu Moy wrote:
* if (!values-items[i].string)
config_error_nonbool(
= This check could be done once and for all in a function, say
git_config_get_value_multi_nonbool, a trivial wrapper around
git_config_get_value_multi like
const struct string_list *git_configset_get_value_multi_nonbool(struct
config_set *cs, const char *key)
{
struct string_list l = git_configset_get_value_multi(cs, key);
// possibly if(l) depending on the point above.
for (i = 0; i values-nr; i++) {
if (!values-items[i].string)
git_config_die(key);
}
return l;
}
Not worth it, most the multi value calls do not die on a nonbool.
Can you cite some multi-value variables that can be nonbool? I can't
find many multi-valued variables, and I can't find any which would allow
bool and nonbool.
Thinking a bit more about it: we actually need more than your patch and
my code example above to give accurate error messages. Your code gives
no error message, and mine uses git_config_die(key); which gives the
line of the _last_ entry, but not necessarily the right line.
The right line number should be extracted from the info field of the
string_list. It's not completely trivial, hence I'd rather have a helper
doing it well in config.c than letting callers re-do the check and
possibly give wrong line in their error message as I did in my first
attempt.
I think you can introduce a helper git_config_die_linenr(key, linenr)
that displays the right error message. Then git_config_die becomes a
wrapper around it that does the lookup to find linenr from the hash.
You already have a duplicate piece of code in your other series:
if (!kv_info-linenr)
die(unable to parse '%s' from command-line
config, entry-key);
else
die(bad config variable '%s' at file line %d
in %s,
entry-key,
kv_info-linenr,
kv_info-filename);
That would be the content of git_config_die_linenr().
--
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: [PATCH v5 3/7] change `git_config()` return value to void
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 4:52 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
Signed-off-by: Tanay Abhra tanay...@gmail.com
I think I deserve a bit of credit here ;-).
Yes, but to show credit would I have to write from you or
signed-off-by? :)
Original-patch-by: would be fine.
My bad, but this should be die(_(...));, so that the message can be
translated. Not really serious since it's not really meant to be seen by
the user, though.
Noted. Though there are not many cases till I have read where error messages
are marked
for translation.
Yes, see my other message.
Overall, there are still many untranslated messages in Git's codebase:
$ git grep 'die(_(' | wc -l
517
$ git grep 'die(' | wc -l
1247
Part of the reason is that die() was written and used far before the
translation effort has started.
--
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: [PATCH v5 3/7] change `git_config()` return value to void
On 7/31/2014 4:52 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
Signed-off-by: Tanay Abhra tanay...@gmail.com
I think I deserve a bit of credit here ;-).
Yes, but to show credit would I have to write from you or
signed-off-by? :)
{
-return git_config_with_options(fn, data, NULL, 1);
+if (git_config_with_options(fn, data, NULL, 1) 0)
+/*
+ * git_config_with_options() normally returns only
+ * positive values, as most errors are fatal, and
+ * non-fatal potential errors are guarded by if
+ * statements that are entered only when no error is
+ * possible.
+ *
+ * If we ever encounter a non-fatal error, it means
+ * something went really wrong and we should stop
+ * immediately.
+ */
+die(Unknown error occured while reading the configuration
files);
}
My bad, but this should be die(_(...));, so that the message can be
translated. Not really serious since it's not really meant to be seen by
the user, though.
Noted. Though there are not many cases till I have read where error messages
are marked
for translation.
--
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] config.c: mark error and warnings strings for translation
On 7/31/2014 5:01 PM, Matthieu Moy wrote:
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Noticed while reviewing Tanay's patches, but this one is independant
from his series, both syntactically and semantically.
config.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/config.c b/config.c
index a191328..76eeb63 100644
--- a/config.c
+++ b/config.c
@@ -457,9 +457,9 @@ static int git_parse_source(config_fn_t fn, void *data)
break;
}
if (cf-die_on_error)
- die(bad config file line %d in %s, cf-linenr, cf-name);
+ die(_(bad config file line %d in %s), cf-linenr, cf-name);
else
- return error(bad config file line %d in %s, cf-linenr,
cf-name);
+ return error(_(bad config file line %d in %s), cf-linenr,
cf-name);
Can I package your patch with mine in which I am going to change the
error message to print the variable name also?
}
static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -575,9 +575,9 @@ static void die_bad_number(const char *name, const char
*value)
value = ;
if (cf cf-name)
- die(bad numeric config value '%s' for '%s' in %s: %s,
+ die(_(bad numeric config value '%s' for '%s' in %s: %s),
value, name, cf-name, reason);
- die(bad numeric config value '%s' for '%s': %s, value, name, reason);
+ die(_(bad numeric config value '%s' for '%s': %s), value, name,
reason);
}
int git_config_int(const char *name, const char *value)
@@ -662,7 +662,7 @@ int git_config_pathname(const char **dest, const char
*var, const char *value)
return config_error_nonbool(var);
*dest = expand_user_path(value);
if (!*dest)
- die(Failed to expand user dir in: '%s', value);
+ die(_(Failed to expand user dir in: '%s'), value);
nit : error messages start with a small letter. same case below for Invalid.
;)
return 0;
}
@@ -740,7 +740,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
zlib_compression_level = level;
zlib_compression_seen = 1;
return 0;
@@ -751,7 +751,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
core_compression_level = level;
core_compression_seen = 1;
if (!zlib_compression_seen)
@@ -873,7 +873,7 @@ static int git_default_core_config(const char *var, const
char *value)
else if (!strcmp(value, link))
object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
else
- die(Invalid mode for object creation: %s, value);
+ die(_(Invalid mode for object creation: %s), value);
return 0;
}
@@ -1173,7 +1173,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 */
- die(unable to parse command-line config);
+ die(_(unable to parse command-line config));
break;
case 0: /* found nothing */
break;
@@ -1514,7 +1514,7 @@ static int store_aux(const char *key, const char
*value, void *cb)
case KEY_SEEN:
if (matches(key, value)) {
if (store.seen == 1 store.multi_replace == 0) {
- warning(%s has multiple values, key);
+ warning(_(%s has multiple values), key);
}
ALLOC_GROW(store.offset, store.seen + 1,
--
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 v4 2/5] notes.c: replace `git_config()` with `git_config_get_value_multi()`
On 7/31/2014 5:08 PM, Matthieu Moy wrote:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Tanay Abhra tanay...@gmail.com writes:
On 7/30/2014 7:43 PM, Matthieu Moy wrote:
* if (!values-items[i].string)
config_error_nonbool(
= This check could be done once and for all in a function, say
git_config_get_value_multi_nonbool, a trivial wrapper around
git_config_get_value_multi like
const struct string_list *git_configset_get_value_multi_nonbool(struct
config_set *cs, const char *key)
{
struct string_list l = git_configset_get_value_multi(cs, key);
// possibly if(l) depending on the point above.
for (i = 0; i values-nr; i++) {
if (!values-items[i].string)
git_config_die(key);
}
return l;
}
Not worth it, most the multi value calls do not die on a nonbool.
Can you cite some multi-value variables that can be nonbool? I can't
find many multi-valued variables, and I can't find any which would allow
bool and nonbool.
Thinking a bit more about it: we actually need more than your patch and
my code example above to give accurate error messages. Your code gives
no error message, and mine uses git_config_die(key); which gives the
line of the _last_ entry, but not necessarily the right line.
The right line number should be extracted from the info field of the
string_list. It's not completely trivial, hence I'd rather have a helper
doing it well in config.c than letting callers re-do the check and
possibly give wrong line in their error message as I did in my first
attempt.
I think you can introduce a helper git_config_die_linenr(key, linenr)
that displays the right error message. Then git_config_die becomes a
wrapper around it that does the lookup to find linenr from the hash.
You already have a duplicate piece of code in your other series:
if (!kv_info-linenr)
die(unable to parse '%s' from command-line
config, entry-key);
else
die(bad config variable '%s' at file line %d
in %s,
entry-key,
kv_info-linenr,
kv_info-filename);
That would be the content of git_config_die_linenr().
Thanks. I will add it in the next reroll.
--
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] config.c: mark error and warnings strings for translation
Tanay Abhra tanay...@gmail.com writes: On 7/31/2014 5:01 PM, Matthieu Moy wrote: Signed-off-by: Matthieu Moy matthieu@imag.fr --- Noticed while reviewing Tanay's patches, but this one is independant from his series, both syntactically and semantically. config.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/config.c b/config.c index a191328..76eeb63 100644 --- a/config.c +++ b/config.c @@ -457,9 +457,9 @@ static int git_parse_source(config_fn_t fn, void *data) break; } if (cf-die_on_error) -die(bad config file line %d in %s, cf-linenr, cf-name); +die(_(bad config file line %d in %s), cf-linenr, cf-name); else -return error(bad config file line %d in %s, cf-linenr, cf-name); +return error(_(bad config file line %d in %s), cf-linenr, cf-name); Can I package your patch with mine in which I am going to change the error message to print the variable name also? Yes, you can include it as PATCH 1 for example, and then build on top. @@ -662,7 +662,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value) return config_error_nonbool(var); *dest = expand_user_path(value); if (!*dest) -die(Failed to expand user dir in: '%s', value); +die(_(Failed to expand user dir in: '%s'), value); nit : error messages start with a small letter. same case below for Invalid. ;) Indeed. Then I'm letting you continue on the patch. -- 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
[BUG?] log -g does not respect pathspec
git log -g and git log -g -- something show the same thing. From the implementation point of view, I guess that's understandable because reflog takes a different codepath. But I think the user does not expect that.. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] gitweb: fix typo in man page
Signed-off-by: Tony Finch d...@dotat.at --- Documentation/gitweb.conf.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt index ebe7a6c..29f1e06 100644 --- a/Documentation/gitweb.conf.txt +++ b/Documentation/gitweb.conf.txt @@ -487,7 +487,7 @@ By default it is set to (), i.e. an empty list. This means that gitweb would not try to create project URL (to fetch) from project name. $projects_list_group_categories:: - Whether to enables the grouping of projects by category on the project + Whether to enable the grouping of projects by category on the project list page. The category of a project is determined by the `$GIT_DIR/category` file or the `gitweb.category` variable in each repository's configuration. Disabled by default (set to 0). -- 2.1.0.rc0.229.gaee38de -- 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/5] gitweb: improve directory hierarchy handling
There are two main things in this little seris: The second and third patches improve gitweb's project filter feature, which is for listing just the projects in a subdirectory. The fourth and fifth allow the admin to use a directory hierarchy to automatically categorize projects in gitweb. Tony Finch (5): gitweb: fix typo in man page gitweb: if the PATH_INFO is incomplete, use it as a project_filter gitweb: add a link under the search box to clear a project filter gitweb: optionally set project category from its pathname gitweb: make category headings into links when they are directories Documentation/gitweb.conf.txt | 8 +- gitweb/gitweb.perl| 62 --- 2 files changed, 59 insertions(+), 11 deletions(-) -- 2.1.0.rc0.229.gaee38de -- 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/5] gitweb: add a link under the search box to clear a project filter
Previously when a project filter was active, the only simple way
to clear it was by clicking the home link in the breadcrumbs, which
is not very obvious.
This change adds another home link under the search box which clears
both project filter and search, next to the existing link that
clears the search and keeps the project filter.
Signed-off-by: Tony Finch d...@dotat.at
---
gitweb/gitweb.perl | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 12aba8f..d1e6b79 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5545,10 +5545,13 @@ sub git_project_search_form {
/span\n .
$cgi-submit(-name = 'btnS', -value = 'Search') .
$cgi-end_form() . \n .
- $cgi-a({-href = href(project = undef, searchtext = undef,
-project_filter = $project_filter)},
- esc_html(List all projects$limit)) . br /\n;
- print /div\n;
+ $cgi-a({-href = $my_uri}, esc_html(List all projects));
+ print / .
+ $cgi-a({-href = href(project = undef, action = project_list,
+project_filter = $project_filter)},
+ esc_html(List projects$limit))
+ if $project_filter;
+ print br /\n/div\n;
}
# entry for given @keys needs filling if at least one of keys in list
--
2.1.0.rc0.229.gaee38de
--
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/5] gitweb: optionally set project category from its pathname
When repositories are organized in a hierarchial directory tree
it is convenient if gitweb project categories can be set
automatically based on their parent directory, so that users
do not have to set the same information twice.
Signed-off-by: Tony Finch d...@dotat.at
---
Documentation/gitweb.conf.txt | 6 ++
gitweb/gitweb.perl| 13 ++---
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index 29f1e06..7c0de18 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -492,6 +492,12 @@ $projects_list_group_categories::
`$GIT_DIR/category` file or the `gitweb.category` variable in each
repository's configuration. Disabled by default (set to 0).
+$projects_list_directory_is_category::
+ Whether to set a project's category to its parent directory, i.e. its
+ pathname excluding the `/repo.git` leaf name. This is only used if
+ the repo has no explicit setting, and if the pathname has more than
+ one component. Disabled by default (set to 0).
+
$project_list_default_category::
Default category for projects for which none is specified. If this is
set to the empty string, such projects will remain uncategorized and
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d1e6b79..edbc058 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -129,6 +129,10 @@ our $projects_list_description_width = 25;
# (enabled if this variable evaluates to true)
our $projects_list_group_categories = 0;
+# project's category defaults to its parent directory
+# (enabled if this variable evaluates to true)
+our $projects_list_directory_is_category = 0;
+
# default category if none specified
# (leave the empty string for no category)
our $project_list_default_category = ;
@@ -2904,7 +2908,11 @@ sub git_get_project_description {
sub git_get_project_category {
my $path = shift;
- return git_get_file_or_project_config($path, 'category');
+ my $cat = git_get_file_or_project_config($path, 'category');
+ return $cat if $cat;
+ return $1 if $projects_list_directory_is_category
+ $path =~ m,^(.*)/[^/]*$,;
+ return $project_list_default_category;
}
@@ -5618,8 +5626,7 @@ sub fill_project_list_info {
}
if ($projects_list_group_categories
project_info_needs_filling($pr, $filter_set-('category')))
{
- my $cat = git_get_project_category($pr-{'path'}) ||
-
$project_list_default_category;
+ my $cat = git_get_project_category($pr-{'path'});
$pr-{'category'} = to_utf8($cat);
}
--
2.1.0.rc0.229.gaee38de
--
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/5] gitweb: make category headings into links when they are directories
When $projects_list_category_is_directory is turned on, project
categories can be useful as project filters, so with that setting
gitweb now makes the category headings into project_filter links
(like the breadcrumbs).
Signed-off-by: Tony Finch d...@dotat.at
---
gitweb/gitweb.perl | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index edbc058..32e65ae 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5834,8 +5834,18 @@ sub git_project_list_body {
if ($check_forks) {
print td/td\n;
}
- print td class=\category\
colspan=\5\.esc_html($cat)./td\n;
- print /tr\n;
+ print td class=\category\ colspan=\5\;
+ if ($projects_list_directory_is_category) {
+ print $cgi-a({-href =
+ href(project = undef,
+ project_filter = $cat,
+ action = project_list),
+ -class = list},
+ esc_html($cat));
+ } else {
+ print esc_html($cat);
+ }
+ print /td\n/tr\n;
}
git_project_list_rows($categories{$cat}, undef, undef,
$check_forks);
--
2.1.0.rc0.229.gaee38de
--
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/5] gitweb: if the PATH_INFO is incomplete, use it as a project_filter
Previously gitweb would ignore partial PATH_INFO. For example,
it would produce a project list for the top URL
https://www.example.org/projects/
and a project summary for
https://www.example.org/projects/git/git.git
but if you tried to list just the git-related projects with
https://www.example.org/projects/git/
you would get a list of all projects, same as the top URL.
As well as fixing that omission, this change also makes gitweb
generate PATH_INFO-style URLs for project filter links, such
as in the breadcrumbs.
Signed-off-by: Tony Finch d...@dotat.at
---
gitweb/gitweb.perl | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..12aba8f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -891,7 +891,17 @@ sub evaluate_path_info {
while ($project !check_head_link($projectroot/$project)) {
$project =~ s,/*[^/]*$,,;
}
- return unless $project;
+ # If there is no project, use the PATH_INFO as a project filter if it
+ # is a directory in the projectroot. (It can't be a subdirectory of a
+ # repo because we just verified that isn't the case.)
+ unless ($project) {
+ if (-d $projectroot/$path_info) {
+ $path_info =~ s,/+$,,;
+ $input_params{'project_filter'} = $path_info;
+ $path_info = ;
+ }
+ return;
+ }
$input_params{'project'} = $project;
# do not change any parameters if an action is given using the query
string
@@ -1356,6 +1366,18 @@ sub href {
}
my $use_pathinfo = gitweb_check_feature('pathinfo');
+
+ # we have to check for a project_filter first because handling the full
+ # project-plus-parameters deletes some of the paramaters we check here
+ if (!defined $params{'project'} $params{'project_filter'}
+ $params{'action'} eq project_list
+ (exists $params{-path_info} ? $params{-path_info} : $use_pathinfo))
{
+ $href =~ s,/$,,;
+ $href .= /.esc_path_info($params{'project_filter'})./;
+ delete $params{'project_filter'};
+ delete $params{'action'};
+ }
+
if (defined $params{'project'}
(exists $params{-path_info} ? $params{-path_info} : $use_pathinfo))
{
# try to put as many parameters as possible in PATH_INFO:
--
2.1.0.rc0.229.gaee38de
--
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 0/3] Keep .lock file paths absolute
v3 requires rs/strbuf-getcwd, turns paths to absolute from the beginning, and kills the last use of PATH_MAX in lockfile.c thanks to strbuf_readlink(). Nguyễn Thái Ngọc Duy (3): lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) lockfile.c: remove PATH_MAX limit in resolve_symlink() lockfile.c: store absolute path cache.h | 2 +- lockfile.c| 95 +++ t/t2107-update-index-basic.sh | 15 +++ 3 files changed, 58 insertions(+), 54 deletions(-) -- 2.1.0.rc0.78.gc0d8480 -- 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 2/3] lockfile.c: remove PATH_MAX limit in resolve_symlink()
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
lockfile.c | 47 ++-
1 file changed, 14 insertions(+), 33 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 968b28f..154915f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -85,52 +85,33 @@ static char *last_path_elm(char *p)
static char *resolve_symlink(const char *in)
{
- static char p[PATH_MAX];
- size_t s = sizeof(p);
+ static struct strbuf p = STRBUF_INIT;
+ struct strbuf link = STRBUF_INIT;
int depth = MAXDEPTH;
- if (strlen(in) = sizeof(p))
- return NULL;
- strcpy(p, in);
+ strbuf_reset(p);
+ strbuf_addstr(p, in);
while (depth--) {
- char link[PATH_MAX];
- int link_len = readlink(p, link, sizeof(link));
- if (link_len 0) {
- /* not a symlink anymore */
- return p;
- }
- else if (link_len sizeof(link))
- /* readlink() never null-terminates */
- link[link_len] = '\0';
- else {
- warning(%s: symlink too long, p);
- return p;
- }
+ if (strbuf_readlink(link, p.buf, 0) 0)
+ break; /* not a symlink anymore */
- if (is_absolute_path(link)) {
+ if (is_absolute_path(link.buf)) {
/* absolute path simply replaces p */
- if (link_len s)
- strcpy(p, link);
- else {
- warning(%s: symlink too long, p);
- return p;
- }
+ strbuf_reset(p);
+ strbuf_addbuf(p, link);
} else {
/*
* link is a relative path, so I must replace the
* last element of p with it.
*/
- char *r = (char *)last_path_elm(p);
- if (r - p + link_len s)
- strcpy(r, link);
- else {
- warning(%s: symlink too long, p);
- return p;
- }
+ char *r = (char *)last_path_elm(p.buf);
+ strbuf_setlen(p, r - p.buf);
+ strbuf_addbuf(p, link);
}
}
- return p;
+ strbuf_release(link);
+ return p.buf;
}
--
2.1.0.rc0.78.gc0d8480
--
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 1/3] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h| 2 +-
lockfile.c | 56
2 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/cache.h b/cache.h
index cc46be4..0d8dce7 100644
--- a/cache.h
+++ b/cache.h
@@ -539,7 +539,7 @@ struct lock_file {
int fd;
pid_t owner;
char on_list;
- char filename[PATH_MAX];
+ char *filename;
};
#define LOCK_DIE_ON_ERROR 1
#define LOCK_NODEREF 2
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..968b28f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -7,13 +7,19 @@
static struct lock_file *lock_file_list;
static const char *alternate_index_output;
+static void clear_filename(struct lock_file *lk)
+{
+ free(lk-filename);
+ lk-filename = NULL;
+}
+
static void remove_lock_file(void)
{
pid_t me = getpid();
while (lock_file_list) {
if (lock_file_list-owner == me
- lock_file_list-filename[0]) {
+ lock_file_list-filename) {
if (lock_file_list-fd = 0)
close(lock_file_list-fd);
unlink_or_warn(lock_file_list-filename);
@@ -77,10 +83,16 @@ static char *last_path_elm(char *p)
* Always returns p.
*/
-static char *resolve_symlink(char *p, size_t s)
+static char *resolve_symlink(const char *in)
{
+ static char p[PATH_MAX];
+ size_t s = sizeof(p);
int depth = MAXDEPTH;
+ if (strlen(in) = sizeof(p))
+ return NULL;
+ strcpy(p, in);
+
while (depth--) {
char link[PATH_MAX];
int link_len = readlink(p, link, sizeof(link));
@@ -124,17 +136,12 @@ static char *resolve_symlink(char *p, size_t s)
static int lock_file(struct lock_file *lk, const char *path, int flags)
{
- /*
-* subtract 5 from size to make sure there's room for adding
-* .lock for the lock file name
-*/
- static const size_t max_path_len = sizeof(lk-filename) - 5;
-
- if (strlen(path) = max_path_len)
+ int len;
+ if (!(flags LOCK_NODEREF) !(path = resolve_symlink(path)))
return -1;
+ len = strlen(path) + 5; /* .lock */
+ lk-filename = xmallocz(len);
strcpy(lk-filename, path);
- if (!(flags LOCK_NODEREF))
- resolve_symlink(lk-filename, max_path_len);
strcat(lk-filename, .lock);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
@@ -153,7 +160,7 @@ static int lock_file(struct lock_file *lk, const char
*path, int flags)
lk-filename);
}
else
- lk-filename[0] = 0;
+ clear_filename(lk);
return lk-fd;
}
@@ -231,16 +238,17 @@ int close_lock_file(struct lock_file *lk)
int commit_lock_file(struct lock_file *lk)
{
- char result_file[PATH_MAX];
- size_t i;
- if (lk-fd = 0 close_lock_file(lk))
+ char *result_file;
+ if ((lk-fd = 0 close_lock_file(lk)) || !lk-filename)
return -1;
- strcpy(result_file, lk-filename);
- i = strlen(result_file) - 5; /* .lock */
- result_file[i] = 0;
- if (rename(lk-filename, result_file))
+ result_file = xmemdupz(lk-filename,
+ strlen(lk-filename) - 5 /* .lock */);
+ if (rename(lk-filename, result_file)) {
+ free(result_file);
return -1;
- lk-filename[0] = 0;
+ }
+ free(result_file);
+ clear_filename(lk);
return 0;
}
@@ -260,11 +268,11 @@ void set_alternate_index_output(const char *name)
int commit_locked_index(struct lock_file *lk)
{
if (alternate_index_output) {
- if (lk-fd = 0 close_lock_file(lk))
+ if ((lk-fd = 0 close_lock_file(lk)) || !lk-filename)
return -1;
if (rename(lk-filename, alternate_index_output))
return -1;
- lk-filename[0] = 0;
+ clear_filename(lk);
return 0;
}
else
@@ -273,10 +281,10 @@ int commit_locked_index(struct lock_file *lk)
void rollback_lock_file(struct lock_file *lk)
{
- if (lk-filename[0]) {
+ if (lk-filename) {
if (lk-fd = 0)
close(lk-fd);
unlink_or_warn(lk-filename);
}
- lk-filename[0] = 0;
+ clear_filename(lk);
}
--
2.1.0.rc0.78.gc0d8480
--
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 3/3] lockfile.c: store absolute path
Locked paths can be saved in a linked list so that if something wrong
happens, *.lock are removed. For relative paths, this works fine if we
keep cwd the same, which is true 99% of time except:
- update-index and read-tree hold the lock on $GIT_DIR/index really
early, then later on may call setup_work_tree() to move cwd.
- Suppose a lock is being held (e.g. by git add) then somewhere
down the line, somebody calls real_path (e.g. link_alt_odb_entry),
which temporarily moves cwd away and back.
During that time when cwd is moved (either permanently or temporarily)
and we decide to die(), attempts to remove relative *.lock will fail,
and the next operation will complain that some files are still locked.
Avoid this case by turning relative paths to absolute before storing
the path in filename field.
Reported-by: Yue Lin Ho yuelinho...@gmail.com
Helped-by: Ramsay Jones ram...@ramsay1.demon.co.uk
Helped-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
lockfile.c| 10 +-
t/t2107-update-index-basic.sh | 15 +++
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index 154915f..0aa70a5 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -117,13 +117,13 @@ static char *resolve_symlink(const char *in)
static int lock_file(struct lock_file *lk, const char *path, int flags)
{
- int len;
+ struct strbuf sb = STRBUF_INIT;
+
if (!(flags LOCK_NODEREF) !(path = resolve_symlink(path)))
return -1;
- len = strlen(path) + 5; /* .lock */
- lk-filename = xmallocz(len);
- strcpy(lk-filename, path);
- strcat(lk-filename, .lock);
+ strbuf_add_absolute_path(sb, path);
+ strbuf_addstr(sb, .lock);
+ lk-filename = strbuf_detach(sb, NULL);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
if (!lock_file_list) {
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 1bafb90..dfe02f4 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -65,4 +65,19 @@ test_expect_success '--cacheinfo mode,sha1,path (new
syntax)' '
test_cmp expect actual
'
+test_expect_success '.lock files cleaned up' '
+ mkdir cleanup
+ (
+ cd cleanup
+ mkdir worktree
+ git init repo
+ cd repo
+ git config core.worktree ../../worktree
+ # --refresh triggers late setup_work_tree,
+ # active_cache_changed is zero, rollback_lock_file fails
+ git update-index --refresh
+ ! test -f .git/index.lock
+ )
+'
+
test_done
--
2.1.0.rc0.78.gc0d8480
--
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 1/2] config.c: mark error and warnings strings for translation
From: Matthieu Moy matthieu@imag.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
v2: error messages now start with a small letter.
config.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/config.c b/config.c
index 058505c..7330789 100644
--- a/config.c
+++ b/config.c
@@ -442,9 +442,9 @@ static int git_parse_source(config_fn_t fn, void *data)
break;
}
if (cf-die_on_error)
- die(bad config file line %d in %s, cf-linenr, cf-name);
+ die(_(bad config file line %d in %s), cf-linenr, cf-name);
else
- return error(bad config file line %d in %s, cf-linenr,
cf-name);
+ return error(_(bad config file line %d in %s), cf-linenr,
cf-name);
}
static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -560,9 +560,9 @@ static void die_bad_number(const char *name, const char
*value)
value = ;
if (cf cf-name)
- die(bad numeric config value '%s' for '%s' in %s: %s,
+ die(_(bad numeric config value '%s' for '%s' in %s: %s),
value, name, cf-name, reason);
- die(bad numeric config value '%s' for '%s': %s, value, name, reason);
+ die(_(bad numeric config value '%s' for '%s': %s), value, name,
reason);
}
int git_config_int(const char *name, const char *value)
@@ -647,7 +647,7 @@ int git_config_pathname(const char **dest, const char *var,
const char *value)
return config_error_nonbool(var);
*dest = expand_user_path(value);
if (!*dest)
- die(Failed to expand user dir in: '%s', value);
+ die(_(failed to expand user dir in: '%s'), value);
return 0;
}
@@ -725,7 +725,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
zlib_compression_level = level;
zlib_compression_seen = 1;
return 0;
@@ -736,7 +736,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
core_compression_level = level;
core_compression_seen = 1;
if (!zlib_compression_seen)
@@ -858,7 +858,7 @@ static int git_default_core_config(const char *var, const
char *value)
else if (!strcmp(value, link))
object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
else
- die(Invalid mode for object creation: %s, value);
+ die(_(invalid mode for object creation: %s), value);
return 0;
}
@@ -1158,7 +1158,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 */
- die(unable to parse command-line config);
+ die(_(unable to parse command-line config));
break;
case 0: /* found nothing */
break;
@@ -1243,7 +1243,7 @@ static int store_aux(const char *key, const char *value,
void *cb)
case KEY_SEEN:
if (matches(key, value)) {
if (store.seen == 1 store.multi_replace == 0) {
- warning(%s has multiple values, key);
+ warning(_(%s has multiple values), key);
}
ALLOC_GROW(store.offset, store.seen + 1,
--
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 2/2] add variable name to `git_config_*()` error message
Whenever a callback returns a negative value, the functions of `git_config_*()` family die printing the line number and file name. In addition to them, add the variable name to the error message. Signed-off-by: Tanay Abhra tanay...@gmail.com --- config.c| 4 ++-- t/t4055-diff-context.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index 7330789..77407da 100644 --- a/config.c +++ b/config.c @@ -442,9 +442,9 @@ static int git_parse_source(config_fn_t fn, void *data) break; } if (cf-die_on_error) - die(_(bad config file line %d in %s), cf-linenr, cf-name); + die(_(bad config variable '%s' at file line %d in %s), var-buf, cf-linenr, cf-name); else - return error(_(bad config file line %d in %s), cf-linenr, cf-name); + return error(_(bad config variable '%s' at file line %d in %s), var-buf, cf-linenr, cf-name); } static int parse_unit_factor(const char *end, uintmax_t *val) diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh index cd04543..741e080 100755 --- a/t/t4055-diff-context.sh +++ b/t/t4055-diff-context.sh @@ -79,7 +79,7 @@ test_expect_success 'non-integer config parsing' ' test_expect_success 'negative integer config parsing' ' git config diff.context -1 test_must_fail git diff 2output - test_i18ngrep bad config file output + test_i18ngrep bad config variable output ' test_expect_success '-U0 is valid, so is diff.context=0' ' -- 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
Re: [PATCH v2 1/2] config.c: mark error and warnings strings for translation
Tanay Abhra tanay...@gmail.com writes: From: Matthieu Moy matthieu@imag.fr Signed-off-by: Matthieu Moy matthieu@imag.fr --- v2: error messages now start with a small letter. Thanks. Ack on both patches. Is there any reason not to include these two patches in the larger series? (There's a semantic dependency on the changed error message and test) -- 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
[PATCH] git-push: fix link in man page
Signed-off-by: Tony Finch d...@dotat.at --- Documentation/git-push.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 21cd455..c0d7403 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -33,7 +33,7 @@ When the command line does not specify what to push with `refspec...` arguments or `--all`, `--mirror`, `--tags` options, the command finds the default `refspec` by consulting `remote.*.push` configuration, and if it is not found, honors `push.default` configuration to decide -what to push (See gitlink:git-config[1] for the meaning of `push.default`). +what to push (See linkgit:git-config[1] for the meaning of `push.default`). OPTIONS[[OPTIONS]] -- 2.1.0.rc0.229.gaee38de -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add variable name to `git_config_*()` error message
Junio, drop (2/2) of this series, it has conflicts with ta/config-set in pu. This patch can easily come later. Sorry for the inconvenience. Patch 1/2 is OK. Thanks. -- 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 v6 1/7] config.c: fix accuracy of line number in errors
From: Matthieu Moy matthieu@grenoble-inp.fr
If a callback returns a negative value to `git_config*()` family,
they call `die()` while printing the line number and the file name.
Currently the printed line number is off by one, thus printing the
wrong line number.
Make `linenr` point to the line we just parsed during the call
to callback to get accurate line number in error messages.
Commit-message-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index a191328..ed5fc8e 100644
--- a/config.c
+++ b/config.c
@@ -244,6 +244,7 @@ static int get_next_char(void)
cf-linenr++;
if (c == EOF) {
cf-eof = 1;
+ cf-linenr++;
c = '\n';
}
return c;
@@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
{
int c;
char *value;
+ int ret;
/* Get the full name */
for (;;) {
@@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
if (!value)
return -1;
}
- return fn(name-buf, value, data);
+ /*
+* We already consumed the \n, but we need linenr to point to
+* the line we just parsed during the call to fn to get
+* accurate line number in error messages.
+*/
+ cf-linenr--;
+ ret = fn(name-buf, value, data);
+ cf-linenr++;
+ return ret;
}
static int get_extended_base_var(struct strbuf *name, int c)
--
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 v6 0/7] Rewrite `git_config()` using config-set API
[Patch v6]: Added _() to error messages.
Diff between v6 and v4 at the bottom.
[PATCH v5]: New patch added (3/7). git_config() now returns void.
[PATCH v4]: One style nit corrected, also added key to error messages.
[PATCH V3]:All the suggestions in [3] applied. Built on top of [1].
[PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows
correct parsing order. Reordered the patches. Removed xfuncname patch
as it was unnecssary.
This series builds on the top of topic[1] in the mailing list with name
git config cache special querying API utilizing the cache.
This series aims to do these three things,
* Use the config-set API to rewrite git_config().
* Solve any legacy bugs in the previous system while at it.
* To be feature complete compared to the previous git_config() implementation,
which I think it is now. (added the line number and file name info just for
completeness)
Also, I haven't yet checked the exact improvements but still as a teaser,
git status now only rereads the configuration files twice instead of four
times.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/254286
[2]: http://thread.gmane.org/gmane.comp.version-control.git/254101
[3]: http://thread.gmane.org/gmane.comp.version-control.git/254211
Tanay Abhra (7):
config.c: fix accuracy of line number in errors
add line number and file name info to `config_set`
change `git_config()` return value to void
rewrite git_config() to use the config-set API
add a test for semantic errors in config files
config: add `git_die_config()` to the config-set API
add tests for `git_config_get_string_const()`
Documentation/technical/api-config.txt | 5 ++
branch.c | 5 +-
cache.h| 27 ++-
config.c | 131 +
t/t1308-config-set.sh | 21 ++
t/t4055-diff-context.sh| 2 +-
test-config.c | 10 +++
7 files changed, 181 insertions(+), 20 deletions(-)
--
1.9.0.GIT
-- 8 --
diff --git a/branch.c b/branch.c
index 46e8aa8..735767d 100644
--- a/branch.c
+++ b/branch.c
@@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char
*branch_name)
strbuf_addf(name, branch.%s.description, branch_name);
cb.config_name = name.buf;
cb.value = NULL;
- if (git_config(read_branch_desc_cb, cb) 0) {
- strbuf_release(name);
- return -1;
- }
+ git_config(read_branch_desc_cb, cb);
if (cb.value)
strbuf_addstr(buf, cb.value);
strbuf_release(name);
diff --git a/cache.h b/cache.h
index 8512225..243f618 100644
--- a/cache.h
+++ b/cache.h
@@ -1295,7 +1295,7 @@ extern int git_config_from_buf(config_fn_t fn, const char
*name,
const char *buf, size_t len, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern int git_config(config_fn_t fn, void *);
+extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
int respect_includes);
diff --git a/config.c b/config.c
index f9bf60a..15fcd91 100644
--- a/config.c
+++ b/config.c
@@ -1229,12 +1229,24 @@ struct key_value_info {
int linenr;
};
-static int git_config_raw(config_fn_t fn, void *data)
+static void git_config_raw(config_fn_t fn, void *data)
{
- return git_config_with_options(fn, data, NULL, 1);
+ if (git_config_with_options(fn, data, NULL, 1) 0)
+ /*
+* git_config_with_options() normally returns only
+* positive values, as most errors are fatal, and
+* non-fatal potential errors are guarded by if
+* statements that are entered only when no error is
+* possible.
+*
+* If we ever encounter a non-fatal error, it means
+* something went really wrong and we should stop
+* immediately.
+*/
+ die(_(unknown error occured while reading the configuration
files));
}
-static int configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
{
int i, value_index;
struct string_list *values;
@@ -1249,23 +1261,22 @@ static int configset_iter(struct config_set *cs,
config_fn_t fn, void *data)
if (fn(entry-key, values-items[value_index].string, data)
0) {
kv_info = values-items[value_index].util;
if (!kv_info-linenr)
- die(unable to parse '%s' from command-line
config,
[PATCH v6 5/7] add a test for semantic errors in config files
Semantic errors (for example, for alias.* variables NULL values are not allowed) in configuration files cause a die printing the line number and file name of the offending value. Add a test documenting that such errors cause a die printing the accurate line number and file name. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..e2f9d0b 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -197,4 +197,15 @@ test_expect_success 'proper error on error in custom config files' ' test_cmp expect actual ' +test_expect_success 'check line errors for malformed values' ' + mv .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + cat .git/config -\EOF + [alias] + br + EOF + test_expect_code 128 git br 2result + grep fatal: bad config variable '\''alias.br'\'' at file line 2 in .git/config result +' + test_done -- 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 v6 2/7] add line number and file name info to `config_set`
Store file name and line number for each key-value pair in the cache
during parsing of the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index ed5fc8e..4a15383 100644
--- a/config.c
+++ b/config.c
@@ -1230,6 +1230,11 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
+
int git_config(config_fn_t fn, void *data)
{
return git_config_with_options(fn, data, NULL, 1);
@@ -1260,6 +1265,9 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
static int configset_add_value(struct config_set *cs, const char *key, const
char *value)
{
struct config_set_element *e;
+ struct string_list_item *si;
+ struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
+
e = configset_find_element(cs, key);
/*
* Since the keys are being fed by git_config*() callback mechanism,
they
@@ -1272,7 +1280,16 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
string_list_init(e-value_list, 1);
hashmap_add(cs-config_hash, e);
}
- string_list_append_nodup(e-value_list, value ? xstrdup(value) : NULL);
+ si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+ if (cf) {
+ kv_info-filename = strintern(cf-name);
+ kv_info-linenr = cf-linenr;
+ } else {
+ /* for values read from `git_config_from_parameters()` */
+ kv_info-filename = strintern(parameter);
+ kv_info-linenr = 0;
+ }
+ si-util = kv_info;
return 0;
}
@@ -1299,7 +1316,7 @@ void git_configset_clear(struct config_set *cs)
hashmap_iter_init(cs-config_hash, iter);
while ((entry = hashmap_iter_next(iter))) {
free(entry-key);
- string_list_clear(entry-value_list, 0);
+ string_list_clear(entry-value_list, 1);
}
hashmap_free(cs-config_hash, 1);
cs-hash_initialized = 0;
--
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 v6 7/7] add tests for `git_config_get_string_const()`
Add tests for `git_config_get_string_const()`, check whether it
dies printing the line number and the file name if a NULL
value is retrieved for the given key.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
t/t1308-config-set.sh | 10 ++
test-config.c | 10 ++
2 files changed, 20 insertions(+)
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index e2f9d0b..f012dd6 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -119,6 +119,16 @@ test_expect_success 'find integer value for a key' '
check_config get_int lamb.chop 65
'
+test_expect_success 'find string value for a key' '
+ check_config get_string case.baz hask
+ check_config expect_code 1 get_string case.ba Value not found for
\case.ba\
+'
+
+test_expect_success 'check line error when NULL string is queried' '
+ test_expect_code 128 test-config get_string case.foo 2result
+ grep fatal: bad config variable '\''case.foo'\'' at file line 7 in
.git/config result
+'
+
test_expect_success 'find integer if value is non parse-able' '
check_config expect_code 128 get_int lamb.head
'
diff --git a/test-config.c b/test-config.c
index 9dd1b22..6a77552 100644
--- a/test-config.c
+++ b/test-config.c
@@ -16,6 +16,8 @@
*
* get_bool - print bool value for the entered key or die
*
+ * get_string - print string value for the entered key or die
+ *
* configset_get_value - returns value with the highest priority for the
entered key
* from a config_set constructed from files entered as
arguments.
*
@@ -84,6 +86,14 @@ int main(int argc, char **argv)
printf(Value not found for \%s\\n, argv[2]);
goto exit1;
}
+ } else if (argc == 3 !strcmp(argv[1], get_string)) {
+ if (!git_config_get_string_const(argv[2], v)) {
+ printf(%s\n, v);
+ goto exit0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ goto exit1;
+ }
} else if (!strcmp(argv[1], configset_get_value)) {
for (i = 3; i argc; i++) {
int err;
--
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 v6 6/7] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name
of the highest priority value for the configuration variable `key`.
It has usage in non-callback based config value retrieval where we can
raise an error and die if there is a semantic error.
For example,
if (!git_config_get_value(key, value)) {
/* NULL values not allowed */
if (!value)
git_config_die(key);
else
/* do work */
}
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-config.txt | 5 +
cache.h| 1 +
config.c | 27 +--
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index 815c1ee..e7ec7cc 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,6 +155,11 @@ as well as retrieval for the queried variable, including:
Similar to `git_config_get_string`, but expands `~` or `~user` into
the user's home directory when found at the beginning of the path.
+`void git_die_config(const char *key)`::
+
+ Dies printing the line number and the file name of the highest
+ priority value for the configuration variable `key`.
+
See test-config.c for usage examples.
Value Parsing Helpers
diff --git a/cache.h b/cache.h
index 4d3c6bd..243f618 100644
--- a/cache.h
+++ b/cache.h
@@ -1406,6 +1406,7 @@ extern int git_config_get_bool(const char *key, int
*dest);
extern int git_config_get_bool_or_int(const char *key, int *is_bool, int
*dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
+extern void git_die_config(const char *key);
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index 6b4dd65..15fcd91 100644
--- a/config.c
+++ b/config.c
@@ -1515,8 +1515,12 @@ const struct string_list
*git_config_get_value_multi(const char *key)
int git_config_get_string_const(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_string_const(the_config_set, key, dest);
+ ret = git_configset_get_string_const(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
}
int git_config_get_string(const char *key, char **dest)
@@ -1557,10 +1561,29 @@ int git_config_get_maybe_bool(const char *key, int
*dest)
int git_config_get_pathname(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_pathname(the_config_set, key, dest);
+ ret = git_configset_get_pathname(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key);
+ return ret;
}
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(_(unable to parse '%s' from command-line config), key);
+ else
+ die(_(bad config variable '%s' at file line %d in %s),
+ key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
+
/*
* Find all the stuff for git_config_set() below.
*/
--
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 v6 4/7] rewrite git_config() to use the config-set API
Of all the functions in `git_config*()` family, `git_config()` has the
most invocations in the whole code base. Each `git_config()` invocation
causes config file rereads which can be avoided using the config-set API.
Use the config-set API to rewrite `git_config()` to use the config caching
layer to avoid config file rereads on each invocation during a git process
lifetime. First invocation constructs the cache, and after that for each
successive invocation, `git_config()` feeds values from the config cache
instead of rereading the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 24 +
config.c| 57 ++---
t/t4055-diff-context.sh | 2 +-
3 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index c61919d..4d3c6bd 100644
--- a/cache.h
+++ b/cache.h
@@ -8,6 +8,7 @@
#include gettext.h
#include convert.h
#include trace.h
+#include string-list.h
#include SHA1_HEADER
#ifndef git_SHA_CTX
@@ -1351,9 +1352,32 @@ extern int parse_config_key(const char *var,
const char **subsection, int *subsection_len,
const char **key);
+struct config_set_element {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list value_list;
+};
+
+struct configset_list_item {
+ struct config_set_element *e;
+ int value_index;
+};
+
+/*
+ * the contents of the list are ordered according to their
+ * position in the config files and order of parsing the files.
+ * (i.e. key-value pair at the last position of .git/config will
+ * be at the last item of the list)
+ */
+struct configset_list {
+ struct configset_list_item *items;
+ unsigned int nr, alloc;
+};
+
struct config_set {
struct hashmap config_hash;
int hash_initialized;
+ struct configset_list list;
};
extern void git_configset_init(struct config_set *cs);
diff --git a/config.c b/config.c
index ecbe14f..6b4dd65 100644
--- a/config.c
+++ b/config.c
@@ -35,12 +35,6 @@ struct config_source {
long (*do_ftell)(struct config_source *c);
};
-struct config_set_element {
- struct hashmap_entry ent;
- char *key;
- struct string_list value_list;
-};
-
static struct config_source *cf;
static int zlib_compression_seen;
@@ -1235,7 +1229,7 @@ struct key_value_info {
int linenr;
};
-void git_config(config_fn_t fn, void *data)
+static void git_config_raw(config_fn_t fn, void *data)
{
if (git_config_with_options(fn, data, NULL, 1) 0)
/*
@@ -1252,6 +1246,39 @@ void git_config(config_fn_t fn, void *data)
die(_(unknown error occured while reading the configuration
files));
}
+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+ int i, value_index;
+ struct string_list *values;
+ struct config_set_element *entry;
+ struct configset_list *list = cs-list;
+ struct key_value_info *kv_info;
+
+ for (i = 0; i list-nr; i++) {
+ entry = list-items[i].e;
+ value_index = list-items[i].value_index;
+ values = entry-value_list;
+ if (fn(entry-key, values-items[value_index].string, data)
0) {
+ kv_info = values-items[value_index].util;
+ if (!kv_info-linenr)
+ die(_(unable to parse '%s' from command-line
config), entry-key);
+ else
+ die(_(bad config variable '%s' at file line %d
in %s),
+ entry-key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
+ }
+}
+
+static void git_config_check_init(void);
+
+void git_config(config_fn_t fn, void *data)
+{
+ git_config_check_init();
+ configset_iter(the_config_set, fn, data);
+}
+
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
{
struct config_set_element k;
@@ -1278,6 +1305,7 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
{
struct config_set_element *e;
struct string_list_item *si;
+ struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
e = configset_find_element(cs, key);
@@ -1293,6 +1321,12 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+
+ ALLOC_GROW(cs-list.items, cs-list.nr + 1, cs-list.alloc);
+ l_item = cs-list.items[cs-list.nr++];
+ l_item-e = e;
+ l_item-value_index = e-value_list.nr - 1;
+
if (cf) {
[PATCH v6 3/7] change `git_config()` return value to void
Currently `git_config()` returns an integer signifying an error code.
During rewrites of the function most of the code was shifted to
`git_config_with_options()`. `git_config_with_options()` normally
returns positive values if its `config_source` parameter is set as NULL,
as most errors are fatal, and non-fatal potential errors are guarded
by if statements that are entered only when no error is possible.
Still a negative value can be returned in case of race condition between
`access_or_die()` `git_config_from_file()`. Also, all callers of
`git_config()` ignore the return value except for one case in branch.c.
Change `git_config()` return value to void and make it die if it receives
a negative value from `git_config_with_options()`.
Original-patch-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
branch.c | 5 +
cache.h | 2 +-
config.c | 16 ++--
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/branch.c b/branch.c
index 46e8aa8..735767d 100644
--- a/branch.c
+++ b/branch.c
@@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char
*branch_name)
strbuf_addf(name, branch.%s.description, branch_name);
cb.config_name = name.buf;
cb.value = NULL;
- if (git_config(read_branch_desc_cb, cb) 0) {
- strbuf_release(name);
- return -1;
- }
+ git_config(read_branch_desc_cb, cb);
if (cb.value)
strbuf_addstr(buf, cb.value);
strbuf_release(name);
diff --git a/cache.h b/cache.h
index 7292aef..c61919d 100644
--- a/cache.h
+++ b/cache.h
@@ -1294,7 +1294,7 @@ extern int git_config_from_buf(config_fn_t fn, const char
*name,
const char *buf, size_t len, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern int git_config(config_fn_t fn, void *);
+extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
int respect_includes);
diff --git a/config.c b/config.c
index 4a15383..ecbe14f 100644
--- a/config.c
+++ b/config.c
@@ -1235,9 +1235,21 @@ struct key_value_info {
int linenr;
};
-int git_config(config_fn_t fn, void *data)
+void git_config(config_fn_t fn, void *data)
{
- return git_config_with_options(fn, data, NULL, 1);
+ if (git_config_with_options(fn, data, NULL, 1) 0)
+ /*
+* git_config_with_options() normally returns only
+* positive values, as most errors are fatal, and
+* non-fatal potential errors are guarded by if
+* statements that are entered only when no error is
+* possible.
+*
+* If we ever encounter a non-fatal error, it means
+* something went really wrong and we should stop
+* immediately.
+*/
+ die(_(unknown error occured while reading the configuration
files));
}
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
--
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
Re: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes:
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(_(unable to parse '%s' from command-line config), key);
+ else
+ die(_(bad config variable '%s' at file line %d in %s),
+ key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
Extra whitespace before }.
Also, didn't we agree that it was a good thing to factor this
if/then/else into a helper function?
--
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: [PATCH v6 0/7] Rewrite `git_config()` using config-set API
Tanay Abhra tanay...@gmail.com writes: [Patch v6]: Added _() to error messages. Diff between v6 and v4 at the bottom. Except for the tiny style issue in PATCH 6, the series is now 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: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
On 7/31/2014 9:25 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+void git_die_config(const char *key)
+{
+const struct string_list *values;
+struct key_value_info *kv_info;
+values = git_config_get_value_multi(key);
+kv_info = values-items[values-nr - 1].util;
+if (!kv_info-linenr)
+die(_(unable to parse '%s' from command-line config), key);
+else
+die(_(bad config variable '%s' at file line %d in %s),
+key,
+kv_info-linenr,
+kv_info-filename);
+ }
Extra whitespace before }.
Also, didn't we agree that it was a good thing to factor this
if/then/else into a helper function?
I have been thinking about it, wouldn't it be better to give users
a function like,
git_config_die_exact(key, value);
where user supplies key value both and it would print the correct message
based
on that. git_die_config() will be a wrapper over it, supplying the last value.
It will also replace the redundant code in git_config_raw() and help in the
multi value case. If the idea sounds okay, I will send a reroll.
--
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 v6 6/7] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 9:25 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(_(unable to parse '%s' from command-line config), key);
+ else
+ die(_(bad config variable '%s' at file line %d in %s),
+ key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
Extra whitespace before }.
Also, didn't we agree that it was a good thing to factor this
if/then/else into a helper function?
I have been thinking about it, wouldn't it be better to give users
a function like,
git_config_die_exact(key, value);
where user supplies key value both and it would print the correct message
based
on that.
I suggested git_config_die_linenr(key, linenr), and I now realize it
should take the value too.
You're suggesting git_config_die_exact(key, value). Is it a typo that
you forgot the line number, or is it intentional? If intentional, I
don't think it solves your issue:
[section]
key
key
There are two errors in this file, and you need to provide a line
number. key and value alone do not allow you to know which line the
error is. You can use a convention like complain on the first value
equal to the argument, but I'm not sure that would always work. And
that seems backward to me to reconstruct the line number since the
function can be called from places where the line number is already
known (while iterating over the string_list for example).
--
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: stash-p broken?
Jeff King p...@peff.net writes: Hmph. To be honest, I am starting to wonder if implying --first-parent is a more sensible option for stash list. It matches stash show, at least, and it is not unreasonable to simply present the changes in the working tree by default, and ignore the index. People who are more clueful can pick apart the commits using git log themselves. Yup, I tend to agree. After thinking about this a lot, I really wish that we did not have stash apply and stash pop, but just stash save and a single stash restore that either checks out the original branch (if its head have not moved) or detaches the HEAD at the original commit (otherwise), and then restores the state of the index and the working tree (with or without untracked) and nothing else, but the details are too long to fit in this topic ;-) -- 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 0/5] nd/multiple-work-trees follow-ups
Duy Nguyen pclo...@gmail.com writes: On Thu, Jul 31, 2014 at 12:51 AM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: The series has entered 'next' so I can't replace patches any more. Besides the brown paper bag fixes, checkout now rejects if a branch is already checked out elsewhere. I do not think we would want to rush the entire series to 'master' before the upcoming release, so we could squash these fixes into the original when we rewind 'next' post release, if you wanted to. Great. Please remove it from next at the next rewind. I'd likely forget so please remind me when the time comes ;-) In the meantime, I made a note in the What's cooking report to hold the topic in 'next' and not advance it to 'master'. -- 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/6] alias.c: replace `git_config()` with `git_config_get_string()`
Ramsay Jones ram...@ramsay1.demon.co.uk writes: On 21/07/14 12:12, Tanay Abhra wrote: -char *alias_lookup(const char *alias) +char *alias_lookup(const char* alias) No, this is not C++. :-D Why would C++ make a difference? Shouldn't you *never* do that? -- Hi! I'm a .signature virus! Copy me into your ~/.signature to help me spread! -- 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 v6 6/7] config: add `git_die_config()` to the config-set API
On 7/31/2014 10:22 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 9:25 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(_(unable to parse '%s' from command-line config), key);
+ else
+ die(_(bad config variable '%s' at file line %d in %s),
+ key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
Extra whitespace before }.
Also, didn't we agree that it was a good thing to factor this
if/then/else into a helper function?
I have been thinking about it, wouldn't it be better to give users
a function like,
git_config_die_exact(key, value);
where user supplies key value both and it would print the correct message
based
on that.
I suggested git_config_die_linenr(key, linenr), and I now realize it
should take the value too.
You're suggesting git_config_die_exact(key, value). Is it a typo that
you forgot the line number, or is it intentional? If intentional, I
don't think it solves your issue:
[section]
key
key
There are two errors in this file, and you need to provide a line
number. key and value alone do not allow you to know which line the
error is. You can use a convention like complain on the first value
equal to the argument, but I'm not sure that would always work. And
that seems backward to me to reconstruct the line number since the
function can be called from places where the line number is already
known (while iterating over the string_list for example).
Still the user would have to know that the linenr info is there.
First hear my argument, then we can go either way.
Let's first see the previous code behavior, git_config() would die on the
first corrupt value, we wouldn't live to see the future value.
for example,
[section]
key // error(old git_config() would die here)
key = good_value
[section]
key //again error
Now for the new behavior,
single valued callers use git_config_get_value() which will directly
supply the last value, so we don't see the first error value.
For such cases, git_die_config(key) is enough.
The new git_config() works exactly as the old code, it would die
on the first case of erroneous value. Here, git_die_config_exact(key, value)
would be enough.
The last case is git_config_get_value_multi(), here we iterate over the keys,
and then call git_die_config_exact(key, value) for the erroneous value.
(pros and cons: abstracts the error message implementation from the user
but there is an extra call to git_config_get_value_multi(), but its cheap and
we are dying anyway)
+ if (load_config_refs) {
+ values = git_config_get_value_multi(notes.displayref);
+ if (values) {
+ for (i = 0; i values-nr; i++) {
+ if (!values-items[i].string) {
+
config_error_nonbool(notes.displayref);
+
git_die_config_exact(notes.displayref, values-items[i].string);
+ }
+ else
+
string_list_add_refs_by_glob(display_notes_refs,
+
values-items[i].string);
+ }
+ }
+ }
What do you think?
--
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 v6 6/7] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 10:22 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 9:25 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+void git_die_config(const char *key)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ if (!kv_info-linenr)
+ die(_(unable to parse '%s' from command-line config), key);
+ else
+ die(_(bad config variable '%s' at file line %d in %s),
+ key,
+ kv_info-linenr,
+ kv_info-filename);
+ }
Extra whitespace before }.
Also, didn't we agree that it was a good thing to factor this
if/then/else into a helper function?
I have been thinking about it, wouldn't it be better to give users
a function like,
git_config_die_exact(key, value);
where user supplies key value both and it would print the correct message
based
on that.
I suggested git_config_die_linenr(key, linenr), and I now realize it
should take the value too.
You're suggesting git_config_die_exact(key, value). Is it a typo that
you forgot the line number, or is it intentional? If intentional, I
don't think it solves your issue:
[section]
key
key
There are two errors in this file, and you need to provide a line
number. key and value alone do not allow you to know which line the
error is. You can use a convention like complain on the first value
equal to the argument, but I'm not sure that would always work. And
that seems backward to me to reconstruct the line number since the
function can be called from places where the line number is already
known (while iterating over the string_list for example).
Still the user would have to know that the linenr info is there.
First hear my argument, then we can go either way.
Let's first see the previous code behavior, git_config() would die on the
first corrupt value, we wouldn't live to see the future value.
for example,
[section]
key // error(old git_config() would die here)
key = good_value
[section]
key //again error
Now for the new behavior,
single valued callers use git_config_get_value() which will directly
supply the last value, so we don't see the first error value.
For such cases, git_die_config(key) is enough.
Yes. And it is better than the old behavior which was dying on the
erroneous value without giving a chance to the user to override the
boggus value in a more specific config file (e.g. if your sysadmin
messed-up /etc/gitconfig).
But since git_die_config(key) is simpler to use for the caller, it's
independant from git_die_config_exact()'s parameters.
The new git_config() works exactly as the old code, it would die
on the first case of erroneous value. Here, git_die_config_exact(key, value)
would be enough.
Yes. But this callsite has the line number information, so it could use
it.
The last case is git_config_get_value_multi(), here we iterate over the keys,
and then call git_die_config_exact(key, value) for the erroneous value.
(pros and cons: abstracts the error message implementation from the user
but there is an extra call to git_config_get_value_multi(), but its cheap and
we are dying anyway)
This is the part I find weird. You're calling git_die_config_exact() on
the first boggus value, and git_die_config_exact() will notify an error
at the line of the last boggus value.
I agree it works (if we give only one error message, it can be the first
or the last, the user doesn't really care), but I find the
implementation backwards. You have the line number already, as you are
iterating over the string_list which contain it. So why forget the line
number, and recompute one, possibly different, right after?
So, I see only cases where you already have the line number, hence no
reason to recompute it.
--
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: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
On 7/31/2014 11:39 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 10:22 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 9:25 PM, Matthieu Moy wrote:
Tanay Abhra tanay...@gmail.com writes:
+void git_die_config(const char *key)
+{
+const struct string_list *values;
+struct key_value_info *kv_info;
+values = git_config_get_value_multi(key);
+kv_info = values-items[values-nr - 1].util;
+if (!kv_info-linenr)
+die(_(unable to parse '%s' from command-line config),
key);
+else
+die(_(bad config variable '%s' at file line %d in %s),
+key,
+kv_info-linenr,
+kv_info-filename);
+ }
Extra whitespace before }.
Also, didn't we agree that it was a good thing to factor this
if/then/else into a helper function?
I have been thinking about it, wouldn't it be better to give users
a function like,
git_config_die_exact(key, value);
where user supplies key value both and it would print the correct
message based
on that.
I suggested git_config_die_linenr(key, linenr), and I now realize it
should take the value too.
You're suggesting git_config_die_exact(key, value). Is it a typo that
you forgot the line number, or is it intentional? If intentional, I
don't think it solves your issue:
[section]
key
key
There are two errors in this file, and you need to provide a line
number. key and value alone do not allow you to know which line the
error is. You can use a convention like complain on the first value
equal to the argument, but I'm not sure that would always work. And
that seems backward to me to reconstruct the line number since the
function can be called from places where the line number is already
known (while iterating over the string_list for example).
Still the user would have to know that the linenr info is there.
First hear my argument, then we can go either way.
Let's first see the previous code behavior, git_config() would die on the
first corrupt value, we wouldn't live to see the future value.
for example,
[section]
key // error(old git_config() would die here)
key = good_value
[section]
key //again error
Now for the new behavior,
single valued callers use git_config_get_value() which will directly
supply the last value, so we don't see the first error value.
For such cases, git_die_config(key) is enough.
Yes. And it is better than the old behavior which was dying on the
erroneous value without giving a chance to the user to override the
boggus value in a more specific config file (e.g. if your sysadmin
messed-up /etc/gitconfig).
But since git_die_config(key) is simpler to use for the caller, it's
independant from git_die_config_exact()'s parameters.
The new git_config() works exactly as the old code, it would die
on the first case of erroneous value. Here, git_die_config_exact(key, value)
would be enough.
Yes. But this callsite has the line number information, so it could use
it.
The last case is git_config_get_value_multi(), here we iterate over the keys,
and then call git_die_config_exact(key, value) for the erroneous value.
(pros and cons: abstracts the error message implementation from the user
but there is an extra call to git_config_get_value_multi(), but its cheap and
we are dying anyway)
This is the part I find weird. You're calling git_die_config_exact() on
the first boggus value, and git_die_config_exact() will notify an error
at the line of the last boggus value.
Hmn, we may have some confusion here. I meant the implementation of
git_config_exact() to look like this,
void git_die_config_exact(const char *key, const char *value)
{
int i;
const struct string_list *values;
struct key_value_info *kv_info;
values = git_config_get_value_multi(key);
for (i = 0; i values.nr; i++) {
kv_info = values-items[i].util;
/* A null check will be here also */
if (!strcmp(value, values-items[i].string)) {
if (!kv_info-linenr)
die(_(unable to parse '%s' from command-line
config), key);
else
die(_(bad config variable '%s' at file line %d
in %s),
key,
kv_info-linenr,
kv_info-filename);
}
}
}
The above code would print the info on first bogus value.
I am only rooting for it because the caller has to think very little to use it.
It's your call, I am open to both ideas.
I agree it works (if we give only one error message, it can be the first
or the last, the user doesn't really care), but I find the
implementation backwards. You have the
Re: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 11:39 PM, Matthieu Moy wrote:
This is the part I find weird. You're calling git_die_config_exact() on
the first boggus value, and git_die_config_exact() will notify an error
at the line of the last boggus value.
Hmn, we may have some confusion here. I meant the implementation of
git_config_exact() to look like this,
void git_die_config_exact(const char *key, const char *value)
{
int i;
const struct string_list *values;
struct key_value_info *kv_info;
values = git_config_get_value_multi(key);
for (i = 0; i values.nr; i++) {
kv_info = values-items[i].util;
/* A null check will be here also */
if (!strcmp(value, values-items[i].string)) {
if (!kv_info-linenr)
die(_(unable to parse '%s' from command-line
config), key);
else
die(_(bad config variable '%s' at file line %d
in %s),
key,
kv_info-linenr,
kv_info-filename);
}
}
}
The above code would print the info on first bogus value.
OK, I got confused because git_die_config() errors out at the first
error. So, your version works, but I do not see any added value in this
extra complexity.
To be cleary, my version would be
NORETURN static /* not sure about the static */
void git_die_config_linenr(const char *key,
const char *filename, int linenr)
{
if (!linenr)
die(_(unable to parse '%s' from command-line config), key);
else
die(_(bad config variable '%s' at file line %d in %s),
key,
linenr,
filename);
}
(hmm, I actually do not need the value, it's not printed)
NORETURN
void git_die_config(const char *key)
{
const struct string_list *values;
struct key_value_info *kv_info;
values = git_config_get_value_multi(key);
kv_info = values-items[values-nr - 1].util;
git_die_config_linenr(key, kv_info-linenr, kv_info-filename);
}
(we forgot the NORETURN in previous version BTW. It should be there in
both functions here and in the .h file)
In my version, git_die_config uses git_die_config_linenr, and there's no
issue with first/last boggus value. Callers of git_die_config_linenr
have all the information to call it directly.
There are 3 code path that leads to the final die() calls, and having
this little helper allows making sure the messages are the same for
every callers, by construction (as opposed to cut-and-pasting).
Really, I don't see any reason to do anything more complex.
--
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: [PATCH 2/2] imap-send: create target mailbox if it is missing
Tony Finch d...@dotat.at writes:
Some MUAs delete their drafts folder when it is empty, so
git imap-send should be able to create it if necessary.
This change checks that the folder exists immediately after
login and tries to create it if it is missing.
There was some vestigial code to handle a [TRYCREATE] response
from the server when an APPEND target is missing. However this
code never ran (the create and trycreate flags were never set)
and when I tried to make it run I found that the code had already
thrown away the contents of the message it was trying to append.
Signed-off-by: Tony Finch d...@dotat.at
---
The basic idea looks good, but I have doubts on one point.
diff --git a/imap-send.c b/imap-send.c
index 524fbab..5e4a24e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1156,6 +1133,25 @@ static struct imap_store *imap_open_store(struct
imap_server_conf *srvc)
credential_approve(cred);
credential_clear(cred);
+ /* check the target mailbox exists */
+ ctx-name = folder;
+ switch (imap_exec(ctx, NULL, EXAMINE \%s\, ctx-name)) {
+ case RESP_OK:
+ /* ok */
+ break;
+ case RESP_BAD:
+ fprintf(stderr, IMAP error: could not check mailbox\n);
+ goto bail;
+ case RESP_NO:
+ if (imap_exec(ctx, NULL, CREATE \%s\, ctx-name) ==
RESP_OK) {
+ imap_info(Created missing mailbox\n);
+ } else {
+ fprintf(stderr, IMAP error: could not create missing
mailbox\n);
+ goto bail;
+ }
+ break;
+ }
At any and all the existing places that goto bail in the function,
we know we failed to authenticate. I think they are all sensible
places to call credential_reject().
On the other hand, at this point before you try to check the target
mailbox exists, we have authenticated sucessfully, we know the
credential used was good, and called credential_approve() to mark it
as such. I do agree that you would want to signal an error to the
caller upon these two failures, but I do not think you want to goto
bail and reject the credential. The error you observed in the new
codepath is caused by something else, not authentication failure,
and in such a case you do not want to cause the credential helper to
evict the user/pass pair from the keyring, no?
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] imap-send: create target mailbox if it is missing
Junio C Hamano gits...@pobox.com wrote: The basic idea looks good, but I have doubts on one point. Thanks for spotting the mistake in the error handling. I'll send an update with a fix. Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ South Utsire: Southwesterly 4 or 5, occasionally 6 at first in south, backing southeasterly later. Moderate. Thundery showers. Good. -- 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
Bug report about symlinks
Greetings from Russia, comrads! I've noticed something strange with git status when replacing a folder with symlink to another folder. There is a git repo with script with demo in the attachment. Yours sincerely, NickKolok aka Nikolay Avdeev. Доброго времени суток, товарищи! При замене папки на симлинк на другую папку с похожими, но не одинаковыми файлами git status ведёт себя странно. Прилагаю архив с репозиторием. В скрипте - пример и пояснения. С уважением, Николай Авдеев aka NickKolok. git-bug-demo.tar.bz2 Description: application/bzip
Re: [PATCH v6 6/7] config: add `git_die_config()` to the config-set API
Matthieu Moy matthieu@grenoble-inp.fr writes:
Tanay Abhra tanay...@gmail.com writes:
On 7/31/2014 11:39 PM, Matthieu Moy wrote:
This is the part I find weird. You're calling git_die_config_exact() on
the first boggus value, and git_die_config_exact() will notify an error
at the line of the last boggus value.
Hmn, we may have some confusion here. I meant the implementation of
git_config_exact() to look like this,
void git_die_config_exact(const char *key, const char *value)
{
int i;
const struct string_list *values;
struct key_value_info *kv_info;
values = git_config_get_value_multi(key);
for (i = 0; i values.nr; i++) {
kv_info = values-items[i].util;
/* A null check will be here also */
if (!strcmp(value, values-items[i].string)) {
if (!kv_info-linenr)
die(_(unable to parse '%s' from command-line
config), key);
else
die(_(bad config variable '%s' at file line %d
in %s),
key,
kv_info-linenr,
kv_info-filename);
}
}
}
The above code would print the info on first bogus value.
OK, I got confused because git_die_config() errors out at the first
error. So, your version works, but I do not see any added value in this
extra complexity.
Hmm, I am still confused ;-)
Can there be more than one 'i' whose value-items[i].string is the
same as the given value? IOW, if you have [user] nameEOL in
both .git/config and ~/.gitconfig, don't we want to make sure that
we complain on the one in .git/config, which would have taken
precedence if it were spelled correctly?
Your version below makes it very clear that you only complain on the
last one, no matter how many identical copies of the value for the
given key are defined incorrectly the same way, which is easier to
understand what is going on.
To be cleary, my version would be
NORETURN static /* not sure about the static */
void git_die_config_linenr(const char *key,
const char *filename, int linenr)
{
if (!linenr)
die(_(unable to parse '%s' from command-line config), key);
else
die(_(bad config variable '%s' at file line %d in %s),
key,
linenr,
filename);
}
(hmm, I actually do not need the value, it's not printed)
NORETURN
void git_die_config(const char *key)
{
const struct string_list *values;
struct key_value_info *kv_info;
values = git_config_get_value_multi(key);
kv_info = values-items[values-nr - 1].util;
git_die_config_linenr(key, kv_info-linenr, kv_info-filename);
}
(we forgot the NORETURN in previous version BTW. It should be there in
both functions here and in the .h file)
In my version, git_die_config uses git_die_config_linenr, and there's no
issue with first/last boggus value. Callers of git_die_config_linenr
have all the information to call it directly.
There are 3 code path that leads to the final die() calls, and having
this little helper allows making sure the messages are the same for
every callers, by construction (as opposed to cut-and-pasting).
Really, I don't see any reason to do anything more complex.
--
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/5] refs.c: replace the onerr argument in update_ref with a strbuf err
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/checkout.c | 7 +--
builtin/clone.c | 23 +++
builtin/merge.c | 20 +---
builtin/notes.c | 24 ++--
builtin/reset.c | 12
builtin/update-ref.c | 7 +--
notes-cache.c| 2 +-
notes-utils.c| 5 +++--
refs.c | 14 +++---
refs.h | 10 ++
transport-helper.c | 7 ++-
transport.c | 9 ++---
12 files changed, 81 insertions(+), 59 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 808c58f..a9ec5be 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -580,6 +580,8 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
{
struct strbuf msg = STRBUF_INIT;
const char *old_desc, *reflog_msg;
+ struct strbuf err = STRBUF_INIT;
+
if (opts-new_branch) {
if (opts-new_orphan_branch) {
if (opts-new_branch_log !log_all_ref_updates) {
@@ -617,8 +619,9 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
if (!strcmp(new-name, HEAD) !new-path !opts-force_detach) {
/* Nothing to do. */
} else if (opts-force_detach || !new-path) { /* No longer on any
branch. */
- update_ref(msg.buf, HEAD, new-commit-object.sha1, NULL,
- REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+ if (update_ref(msg.buf, HEAD, new-commit-object.sha1, NULL,
+ REF_NODEREF, err))
+ die(%s, err.buf);
if (!opts-quiet) {
if (old-path advice_detached_head)
detach_advice(new-name);
diff --git a/builtin/clone.c b/builtin/clone.c
index 7737e12..99e23cf 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -521,6 +521,7 @@ static void write_remote_refs(const struct ref *local_refs)
static void write_followtags(const struct ref *refs, const char *msg)
{
const struct ref *ref;
+ struct strbuf err = STRBUF_INIT;
for (ref = refs; ref; ref = ref-next) {
if (!starts_with(ref-name, refs/tags/))
continue;
@@ -528,8 +529,9 @@ static void write_followtags(const struct ref *refs, const
char *msg)
continue;
if (!has_sha1_file(ref-old_sha1))
continue;
- update_ref(msg, ref-name, ref-old_sha1,
- NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+ if (update_ref(msg, ref-name, ref-old_sha1,
+ NULL, 0, err))
+ die(%s, err.buf);
}
}
@@ -592,28 +594,33 @@ static void update_remote_refs(const struct ref *refs,
static void update_head(const struct ref *our, const struct ref *remote,
const char *msg)
{
+ struct strbuf err = STRBUF_INIT;
+
if (our starts_with(our-name, refs/heads/)) {
/* Local default branch link */
create_symref(HEAD, our-name, NULL);
if (!option_bare) {
const char *head = skip_prefix(our-name,
refs/heads/);
- update_ref(msg, HEAD, our-old_sha1, NULL, 0,
- UPDATE_REFS_DIE_ON_ERR);
+ if (update_ref(msg, HEAD, our-old_sha1, NULL, 0,
+ err))
+ die(%s, err.buf);
install_branch_config(0, head, option_origin,
our-name);
}
} else if (our) {
struct commit *c = lookup_commit_reference(our-old_sha1);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
- update_ref(msg, HEAD, c-object.sha1,
- NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+ if (update_ref(msg, HEAD, c-object.sha1,
+ NULL, REF_NODEREF, err))
+ die(%s, err.buf);
} else if (remote) {
/*
* We know remote HEAD points to a non-branch, or
* HEAD points to a branch but we don't know which one.
* Detach HEAD in all these cases.
*/
- update_ref(msg, HEAD, remote-old_sha1,
- NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+ if (update_ref(msg, HEAD, remote-old_sha1,
+NULL, REF_NODEREF, err))
+ die(%s, err.buf);
}
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 428ca24..17dda64 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -396,9 +396,11 @@ static void finish(struct commit *head_commit,
printf(_(No merge message -- not updating HEAD\n));
else {
[PATCH 0/5] ref-transactions-req-strbuf-err
List,
This is the next patch series in the ref transaction work.
This patch series is called ref-transactions-req-strbuf-err and builds ontop
of the series called ref-transactions-req-packed-refs which is origin/pu
This patch series mainly adds some nice strbuf arguments to some functions to
pass errors back to callers.
The only thing noteworthy is that we finally get to remove
-enum action_on_err {
- UPDATE_REFS_MSG_ON_ERR,
- UPDATE_REFS_DIE_ON_ERR,
- UPDATE_REFS_QUIET_ON_ERR
-};
aside from that there is little/nothing much interesting in there.
Ronnie Sahlberg (5):
refs.c: replace the onerr argument in update_ref with a strbuf err
refs.c: make add_packed_ref return an error instead of calling die
refs.c: make lock_packed_refs take an err argument
refs.c: add an err argument to commit_packed_refs
refs.c: add an err argument to pack_refs
builtin/checkout.c | 7 ++-
builtin/clone.c | 23 +---
builtin/merge.c | 20 ---
builtin/notes.c | 24 +
builtin/pack-refs.c | 8 ++-
builtin/reset.c | 12 +++--
builtin/update-ref.c | 7 ++-
notes-cache.c| 2 +-
notes-utils.c| 5 +-
refs.c | 148 +--
refs.h | 13 ++---
transport-helper.c | 7 ++-
transport.c | 9 ++--
13 files changed, 170 insertions(+), 115 deletions(-)
--
2.0.1.523.g70700c9
--
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/5] refs.c: add an err argument to pack_refs
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/pack-refs.c | 8 +++-
refs.c | 13 ++---
refs.h | 3 ++-
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b20b1ec..da5d46a 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -10,6 +10,7 @@ static char const * const pack_refs_usage[] = {
int cmd_pack_refs(int argc, const char **argv, const char *prefix)
{
unsigned int flags = PACK_REFS_PRUNE;
+ struct strbuf err = STRBUF_INIT;
struct option opts[] = {
OPT_BIT(0, all, flags, N_(pack everything),
PACK_REFS_ALL),
OPT_BIT(0, prune, flags, N_(prune loose refs (default)),
PACK_REFS_PRUNE),
@@ -17,5 +18,10 @@ int cmd_pack_refs(int argc, const char **argv, const char
*prefix)
};
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
- return pack_refs(flags);
+ if (pack_refs(flags, err)) {
+ error(%s, err.buf);
+ strbuf_release(err);
+ return 1;
+ }
+ return 0;
}
diff --git a/refs.c b/refs.c
index 19e73f3..5875c29 100644
--- a/refs.c
+++ b/refs.c
@@ -2328,7 +2328,7 @@ static int commit_packed_refs(struct strbuf *err)
strbuf_addf(err, error writing packed-refs. %s,
strerror(errno));
return -1;
- }
+ }
data.fd = packed_ref_cache-lock-fd;
data.err = err;
@@ -2482,24 +2482,23 @@ static void prune_refs(struct ref_to_prune *r)
}
}
-int pack_refs(unsigned int flags)
+int pack_refs(unsigned int flags, struct strbuf *err)
{
struct pack_refs_cb_data cbdata;
- struct strbuf err = STRBUF_INIT;
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- if (lock_packed_refs(err))
- die(%s, err.buf);
+ if (lock_packed_refs(err))
+ return -1;
cbdata.packed_refs = get_packed_refs(ref_cache);
do_for_each_entry_in_dir(get_loose_refs(ref_cache), 0,
pack_if_possible_fn, cbdata);
- if (commit_packed_refs(err))
- die(%s, err.buf);
+ if (commit_packed_refs(err))
+ return -1;
prune_refs(cbdata.ref_to_prune);
return 0;
diff --git a/refs.h b/refs.h
index dee9a98..1a98e27 100644
--- a/refs.h
+++ b/refs.h
@@ -122,8 +122,9 @@ extern void warn_dangling_symrefs(FILE *fp, const char
*msg_fmt, const struct st
/*
* Write a packed-refs file for the current repository.
* flags: Combination of the above PACK_REFS_* flags.
+ * Returns 0 on success and fills in err on failure.
*/
-int pack_refs(unsigned int flags);
+int pack_refs(unsigned int flags, struct strbuf *err);
extern int ref_exists(const char *);
--
2.0.1.523.g70700c9
--
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/5] refs.c: make add_packed_ref return an error instead of calling die
Change add_packed_ref to return an error instead of calling die().
Update all callers to check the return value of add_packed_ref.
We can also skip checking the refname format since this function is now
static and only called from the transaction code.
If we are updating a ref and the refname is bad then we fail the transaction
already in transaction_update_sha1().
For the ref deletion case the only caveat is that we would not want
to move the badly named ref into the packed refs file during transaction
commit. This again is not a problem since if the name is bad, then
resolve_ref_unsafe() will fail which protects us from calling add_packed_ref()
with the bad name.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 65eee72..0aad8c8 100644
--- a/refs.c
+++ b/refs.c
@@ -1135,17 +1135,16 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
return get_packed_ref_dir(get_packed_ref_cache(refs));
}
-static void add_packed_ref(const char *refname, const unsigned char *sha1)
+static int add_packed_ref(const char *refname, const unsigned char *sha1)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
if (!packed_ref_cache-lock)
- die(internal error: packed refs not locked);
- if (check_refname_format(refname,
REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
- die(Reference has invalid format: '%s', refname);
+ return -1;
add_ref(get_packed_ref_dir(packed_ref_cache),
create_ref_entry(refname, sha1, REF_ISPACKED));
+ return 0;
}
/*
@@ -3666,7 +3665,13 @@ int transaction_commit(struct ref_transaction
*transaction,
RESOLVE_REF_READING, NULL))
continue;
- add_packed_ref(update-refname, sha1);
+ if (add_packed_ref(update-refname, sha1)) {
+ if (err)
+ strbuf_addf(err, Failed to add %s to packed
+ refs, update-refname);
+ ret = -1;
+ goto cleanup;
+ }
need_repack = 1;
}
if (need_repack) {
@@ -3778,7 +3783,13 @@ int transaction_commit(struct ref_transaction
*transaction,
packed = get_packed_refs(ref_cache);
remove_entry(packed, update-refname);
- add_packed_ref(update-refname, update-new_sha1);
+ if (add_packed_ref(update-refname, update-new_sha1)) {
+ if (err)
+ strbuf_addf(err, Failed to add %s to packed
+ refs, update-refname);
+ ret = -1;
+ goto cleanup;
+ }
need_repack = 1;
try_remove_empty_parents((char *)update-refname);
--
2.0.1.523.g70700c9
--
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/5] refs.c: make lock_packed_refs take an err argument
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 0aad8c8..cfe1292 100644
--- a/refs.c
+++ b/refs.c
@@ -2270,13 +2270,17 @@ static int write_packed_entry_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
-/* This should return a meaningful errno on failure */
-static int lock_packed_refs(int flags)
+static int lock_packed_refs(struct strbuf *err)
{
struct packed_ref_cache *packed_ref_cache;
- if (hold_lock_file_for_update(packlock, git_path(packed-refs),
flags) 0)
+ if (hold_lock_file_for_update(packlock, git_path(packed-refs),
+ 0) 0) {
+ if (err)
+ unable_to_lock_message(git_path(packed-refs),
+ errno, err);
return -1;
+ }
/*
* Get the current packed-refs while holding the lock. If the
* packed-refs file has been modified since we last read it,
@@ -2460,11 +2464,14 @@ static void prune_refs(struct ref_to_prune *r)
int pack_refs(unsigned int flags)
{
struct pack_refs_cb_data cbdata;
+ struct strbuf err = STRBUF_INIT;
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- lock_packed_refs(LOCK_DIE_ON_ERROR);
+ if (lock_packed_refs(err))
+ die(%s, err.buf);
+
cbdata.packed_refs = get_packed_refs(ref_cache);
do_for_each_entry_in_dir(get_loose_refs(ref_cache), 0,
@@ -3626,10 +3633,7 @@ int transaction_commit(struct ref_transaction
*transaction,
}
/* Lock packed refs during commit */
- if (lock_packed_refs(0)) {
- if (err)
- unable_to_lock_message(git_path(packed-refs),
- errno, err);
+ if (lock_packed_refs(err)) {
ret = -1;
goto cleanup;
}
@@ -3684,10 +3688,7 @@ int transaction_commit(struct ref_transaction
*transaction,
goto cleanup;
}
/* lock the packed refs again so no one can change it */
- if (lock_packed_refs(0)) {
- if (err)
- unable_to_lock_message(git_path(packed-refs),
- errno, err);
+ if (lock_packed_refs(err)) {
ret = -1;
goto cleanup;
}
--
2.0.1.523.g70700c9
--
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/5] refs.c: add an err argument to commit_packed_refs
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 85 +++---
1 file changed, 50 insertions(+), 35 deletions(-)
diff --git a/refs.c b/refs.c
index cfe1292..19e73f3 100644
--- a/refs.c
+++ b/refs.c
@@ -2232,8 +2232,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* Write an entry to the packed-refs file for the specified refname.
* If peeled is non-NULL, write it as the entry's peeled value.
*/
-static void write_packed_entry(int fd, char *refname, unsigned char *sha1,
- unsigned char *peeled)
+static int write_packed_entry(int fd, char *refname, unsigned char *sha1,
+ unsigned char *peeled, struct strbuf *err)
{
char line[PATH_MAX + 100];
int len;
@@ -2241,33 +2241,50 @@ static void write_packed_entry(int fd, char *refname,
unsigned char *sha1,
len = snprintf(line, sizeof(line), %s %s\n,
sha1_to_hex(sha1), refname);
/* this should not happen but just being defensive */
- if (len sizeof(line))
- die(too long a refname '%s', refname);
- write_or_die(fd, line, len);
-
+ if (len sizeof(line)) {
+ strbuf_addf(err, too long a refname '%s', refname);
+ return -1;
+ }
+ if (write_in_full(fd, line, len) != len) {
+ strbuf_addf(err, error writing to packed-refs. %s,
+ strerror(errno));
+ return -1;
+ }
if (peeled) {
if (snprintf(line, sizeof(line), ^%s\n,
sha1_to_hex(peeled)) != PEELED_LINE_LENGTH)
die(internal error);
- write_or_die(fd, line, PEELED_LINE_LENGTH);
+ len = PEELED_LINE_LENGTH;
+ if (write_in_full(fd, line, len) != len) {
+ strbuf_addf(err, error writing to packed-refs. %s,
+ strerror(errno));
+ return -1;
+ }
}
+ return 0;
}
+struct write_packed_data {
+ int fd;
+ struct strbuf *err;
+};
+
/*
* An each_ref_entry_fn that writes the entry to a packed-refs file.
*/
static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
{
- int *fd = cb_data;
+ struct write_packed_data *data = cb_data;
enum peel_status peel_status = peel_entry(entry, 0);
- if (peel_status != PEEL_PEELED peel_status != PEEL_NON_TAG)
- error(internal error: %s is not a valid packed reference!,
- entry-name);
- write_packed_entry(*fd, entry-name, entry-u.value.sha1,
- peel_status == PEEL_PEELED ?
- entry-u.value.peeled : NULL);
- return 0;
+ if (peel_status != PEEL_PEELED peel_status != PEEL_NON_TAG) {
+ strbuf_addf(data-err, internal error: %s is not a
+ valid packed reference!, entry-name);
+ return -1;
+ }
+ return write_packed_entry(data-fd, entry-name, entry-u.value.sha1,
+ peel_status == PEEL_PEELED ?
+ entry-u.value.peeled : NULL, data-err);
}
static int lock_packed_refs(struct strbuf *err)
@@ -2296,30 +2313,34 @@ static int lock_packed_refs(struct strbuf *err)
/*
* Commit the packed refs changes.
- * On error we must make sure that errno contains a meaningful value.
*/
-static int commit_packed_refs(void)
+static int commit_packed_refs(struct strbuf *err)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
int error = 0;
- int save_errno = 0;
+ struct write_packed_data data;
if (!packed_ref_cache-lock)
die(internal error: packed-refs not locked);
- write_or_die(packed_ref_cache-lock-fd,
-PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+ if (write_in_full(packed_ref_cache-lock-fd,
+ PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER)) 0) {
+ strbuf_addf(err, error writing packed-refs. %s,
+ strerror(errno));
+ return -1;
+ }
+ data.fd = packed_ref_cache-lock-fd;
+ data.err = err;
do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
-0, write_packed_entry_fn,
-packed_ref_cache-lock-fd);
+0, write_packed_entry_fn, data);
if (commit_lock_file(packed_ref_cache-lock)) {
- save_errno = errno;
+ strbuf_addf(err, unable to overwrite old ref-pack
+ file. %s, strerror(errno));
error = -1;
}
packed_ref_cache-lock = NULL;
[PATCH 1/5] receive-pack.c: add protocol support to negotiate atomic-push
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/receive-pack.c | 6 +-
send-pack.c| 12 +---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0565b94..f6b20cb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -36,6 +36,7 @@ static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
static int use_sideband;
+static int use_atomic_push;
static int quiet;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
@@ -142,7 +143,8 @@ static void show_ref(const char *path, const unsigned char
*sha1)
else
packet_write(1, %s %s%c%s%s agent=%s\n,
sha1_to_hex(sha1), path, 0,
- report-status delete-refs side-band-64k quiet,
+ report-status delete-refs side-band-64k quiet
+ atomic-push,
prefer_ofs_delta ? ofs-delta : ,
git_user_agent_sanitized());
sent_capabilities = 1;
@@ -892,6 +894,8 @@ static struct command *read_head_info(struct sha1_array
*shallow)
use_sideband = LARGE_PACKET_MAX;
if (parse_feature_request(feature_list, quiet))
quiet = 1;
+ if (parse_feature_request(feature_list, atomic-push))
+ use_atomic_push = 1;
}
cmd = xcalloc(1, sizeof(struct command) + len - 80);
hashcpy(cmd-old_sha1, old_sha1);
diff --git a/send-pack.c b/send-pack.c
index 6129b0f..f91b8d9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -205,6 +205,7 @@ int send_pack(struct send_pack_args *args,
int use_sideband = 0;
int quiet_supported = 0;
int agent_supported = 0;
+ int atomic_push_supported = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
@@ -224,6 +225,8 @@ int send_pack(struct send_pack_args *args,
agent_supported = 1;
if (server_supports(no-thin))
args-use_thin_pack = 0;
+ if (server_supports(atomic-push))
+ atomic_push_supported = 1;
if (!remote_refs) {
fprintf(stderr, No refs in common and none specified; doing
nothing.\n
@@ -269,17 +272,20 @@ int send_pack(struct send_pack_args *args,
char *old_hex = sha1_to_hex(ref-old_sha1);
char *new_hex = sha1_to_hex(ref-new_sha1);
int quiet = quiet_supported (args-quiet ||
!args-progress);
+ int atomic_push = atomic_push_supported;
if (!cmds_sent (status_report || use_sideband ||
- quiet || agent_supported)) {
+ quiet || agent_supported ||
+ atomic_push)) {
packet_buf_write(req_buf,
-%s %s %s%c%s%s%s%s%s,
+%s %s %s%c%s%s%s%s%s%s,
old_hex, new_hex, ref-name, 0,
status_report ?
report-status : ,
use_sideband ?
side-band-64k : ,
quiet ? quiet : ,
agent_supported ? agent= :
,
-agent_supported ?
git_user_agent_sanitized() :
+agent_supported ?
git_user_agent_sanitized() : ,
+atomic_push ? atomic-push :
);
}
else
--
2.0.1.528.gd0e7a84
--
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/5] ref-transactions-send-pack
List, This small patch series adds atomic-push support to for pushes. By default git will use the old style non-atomic updates for pushes, as not to cause disruption in client scripts that may depend on that behaviour. Command line arguments are introduced to allow the client side to request/ negotiate atomic pushes if the remote repo supports it. There is also a new configuration variable where a repo can set that it wants all pushes to become atomic whether the client requests it or not. This patch series is called ref-transactions-send-pack and depends on/is built ontop of the series called ref-transactions-req-strbuf-err Ronnie Sahlberg (5): receive-pack.c: add protocol support to negotiate atomic-push send-pack.c: add an --atomic-push command line argument receive-pack.c: use a single transaction when atomic-push is negotiated receive-pack.c: add receive.atomicpush configuration option push.c: add an --atomic-push argument Documentation/config.txt| 5 Documentation/git-push.txt | 7 - Documentation/git-send-pack.txt | 7 - builtin/push.c | 2 ++ builtin/receive-pack.c | 66 + builtin/send-pack.c | 6 +++- send-pack.c | 18 +-- send-pack.h | 1 + transport.c | 1 + transport.h | 1 + 10 files changed, 96 insertions(+), 18 deletions(-) -- 2.0.1.528.gd0e7a84 -- 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/5] receive-pack.c: use a single transaction when atomic-push is negotiated
Update receive-pack to use an atomic transaction IFF the client negotiated
that it wanted atomic-push.
This leaves the default behaviour to be the old non-atomic one ref at a
time update. This is to cause as little disruption as possible to existing
clients. It is unknown if there are client scripts that depend on the old
non-atomic behaviour so we make it opt-in for now.
Later patch in this series also adds a configuration variable where you can
override the atomic push behaviour on the receiving repo and force it
to use atomic updates always.
If it turns out over time that there are no client scripts that depend on the
old behaviour we can change git to default to use atomic pushes by default
and instead offer an opt-out argument for people that do not want atomic
updates at all.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/receive-pack.c | 55 --
1 file changed, 44 insertions(+), 11 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f6b20cb..47f778d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -47,6 +47,8 @@ static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
static const char *alt_shallow_file;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -577,26 +579,38 @@ static char *update(struct command *cmd, struct
shallow_info *si)
return NULL; /* good */
}
else {
- struct strbuf err = STRBUF_INIT;
- struct ref_transaction *transaction;
-
if (shallow_update si-shallow_ref[cmd-index]
update_shallow_ref(cmd, si))
return xstrdup(shallow error);
-
- transaction = transaction_begin(err);
- if (!transaction ||
- transaction_update_sha1(transaction, namespaced_name,
+ if (!use_atomic_push) {
+ transaction = transaction_begin(err);
+ if (!transaction) {
+ char *str = xstrdup(err.buf);
+
+ strbuf_release(err);
+ transaction_free(transaction);
+ rp_error(%s, str);
+ return str;
+ }
+ }
+ if (transaction_update_sha1(transaction, namespaced_name,
new_sha1, old_sha1, 0, 1, push,
- err) ||
- transaction_commit(transaction, err)) {
- char *str = strbuf_detach(err, NULL);
- transaction_free(transaction);
+ err)) {
+ char *str = xstrdup(err.buf);
+ strbuf_release(err);
+ transaction_free(transaction);
rp_error(%s, str);
return str;
}
+ if (!use_atomic_push transaction_commit(transaction, err)) {
+ char *str = xstrdup(err.buf);
+ strbuf_release(err);
+ transaction_free(transaction);
+ rp_error(%s, str);
+ return str;
+ }
transaction_free(transaction);
strbuf_release(err);
return NULL; /* good */
@@ -810,6 +824,16 @@ static void execute_commands(struct command *commands,
return;
}
+ if (use_atomic_push) {
+ transaction = transaction_begin(err);
+ if (!transaction) {
+ error(%s, err.buf);
+ strbuf_release(err);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = transaction error;
+ return;
+ }
+ }
data.cmds = commands;
data.si = si;
if (check_everything_connected(iterate_receive_command_list, 0, data))
@@ -848,6 +872,14 @@ static void execute_commands(struct command *commands,
}
}
+ if (use_atomic_push) {
+ if (transaction_commit(transaction, err)) {
+ rp_error(%s, err.buf);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = err.buf;
+ }
+ transaction_free(transaction);
+ }
if (shallow_update !checked_connectivity)
error(BUG: run 'git fsck' for safety.\n
If there are errors, try to remove
@@ -1250,5 +1282,6 @@ int cmd_receive_pack(int argc, const char **argv, const
char *prefix)
[PATCH 5/5] push.c: add an --atomic-push argument
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
Documentation/git-push.txt | 7 ++-
builtin/push.c | 2 ++
transport.c| 1 +
transport.h| 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21cd455..b80b0ac 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
SYNOPSIS
[verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run]
[--receive-pack=git-receive-pack]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n |
--dry-run] [--receive-pack=git-receive-pack]
[--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u
| --set-upstream]
[--force-with-lease[=refname[:expect]]]
[--no-verify] [repository [refspec...]]
@@ -129,6 +129,11 @@ already exists on the remote side.
from the remote but are pointing at commit-ish that are
reachable from the refs being pushed.
+--atomic-push::
+ Try using atomic push. If atomic push is negotiated with the server
+ then any push covering multiple refs will be atomic. Either all
+ refs are updated, or on error, no refs are updated.
+
--receive-pack=git-receive-pack::
--exec=git-receive-pack::
Path to the 'git-receive-pack' program on the remote
diff --git a/builtin/push.c b/builtin/push.c
index f8dfea4..f37390c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -507,6 +507,8 @@ int cmd_push(int argc, const char **argv, const char
*prefix)
OPT_BIT(0, no-verify, flags, N_(bypass pre-push hook),
TRANSPORT_PUSH_NO_HOOK),
OPT_BIT(0, follow-tags, flags, N_(push missing but relevant
tags),
TRANSPORT_PUSH_FOLLOW_TAGS),
+ OPT_BIT(0, atomic-push, flags, N_(use atomic push, if
available),
+ TRANSPORT_ATOMIC_PUSH),
OPT_END()
};
diff --git a/transport.c b/transport.c
index a3b7f48..ab5f553 100644
--- a/transport.c
+++ b/transport.c
@@ -837,6 +837,7 @@ static int git_transport_push(struct transport *transport,
struct ref *remote_re
args.progress = transport-progress;
args.dry_run = !!(flags TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags TRANSPORT_PUSH_PORCELAIN);
+ args.use_atomic_push = !!(flags TRANSPORT_ATOMIC_PUSH);
ret = send_pack(args, data-fd, data-conn, remote_refs,
data-extra_have);
diff --git a/transport.h b/transport.h
index 02ea248..407d641 100644
--- a/transport.h
+++ b/transport.h
@@ -123,6 +123,7 @@ struct transport {
#define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256
#define TRANSPORT_PUSH_NO_HOOK 512
#define TRANSPORT_PUSH_FOLLOW_TAGS 1024
+#define TRANSPORT_ATOMIC_PUSH 2048
#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
#define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) -
gettext_width(x)), (x)
--
2.0.1.528.gd0e7a84
--
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/5] send-pack.c: add an --atomic-push command line argument
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
Documentation/git-send-pack.txt | 7 ++-
builtin/send-pack.c | 6 +-
send-pack.c | 8 +++-
send-pack.h | 1 +
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index dc3a568..4ee2ca1 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another
repository
SYNOPSIS
[verse]
-'git send-pack' [--all] [--dry-run] [--force]
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory
[ref...]
+'git send-pack' [--all] [--dry-run] [--force]
[--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push]
[host:]directory [ref...]
DESCRIPTION
---
@@ -52,6 +52,11 @@ OPTIONS
Send a thin pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
+--atomic-push::
+ With atomic-push all refs are updated in one single atomic transaction.
+ This means that if any of the refs fails then the entire push will
+ fail without changing any refs.
+
host::
A remote host to house the repository. When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index f420b74..78e7d8f 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -13,7 +13,7 @@
#include sha1-array.h
static const char send_pack_usage[] =
-git send-pack [--all | --mirror] [--dry-run] [--force]
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory
[ref...]\n
+git send-pack [--all | --mirror] [--dry-run] [--force]
[--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push]
[host:]directory [ref...]\n
--all and explicit ref specification are mutually exclusive.;
static struct send_pack_args args;
@@ -165,6 +165,10 @@ int cmd_send_pack(int argc, const char **argv, const char
*prefix)
args.use_thin_pack = 1;
continue;
}
+ if (!strcmp(arg, --atomic-push)) {
+ args.use_atomic_push = 1;
+ continue;
+ }
if (!strcmp(arg, --stateless-rpc)) {
args.stateless_rpc = 1;
continue;
diff --git a/send-pack.c b/send-pack.c
index f91b8d9..66f3724 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -228,6 +228,11 @@ int send_pack(struct send_pack_args *args,
if (server_supports(atomic-push))
atomic_push_supported = 1;
+ if (args-use_atomic_push !atomic_push_supported) {
+ fprintf(stderr, Server does not support atomic-push.);
+ return -1;
+ }
+
if (!remote_refs) {
fprintf(stderr, No refs in common and none specified; doing
nothing.\n
Perhaps you should specify a branch such as
'master'.\n);
@@ -272,7 +277,8 @@ int send_pack(struct send_pack_args *args,
char *old_hex = sha1_to_hex(ref-old_sha1);
char *new_hex = sha1_to_hex(ref-new_sha1);
int quiet = quiet_supported (args-quiet ||
!args-progress);
- int atomic_push = atomic_push_supported;
+ int atomic_push = atomic_push_supported
+ args-use_atomic_push;
if (!cmds_sent (status_report || use_sideband ||
quiet || agent_supported ||
diff --git a/send-pack.h b/send-pack.h
index 8e84392..0374ed8 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -10,6 +10,7 @@ struct send_pack_args {
force_update:1,
use_thin_pack:1,
use_ofs_delta:1,
+ use_atomic_push:1,
dry_run:1,
stateless_rpc:1;
};
--
2.0.1.528.gd0e7a84
--
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/5] receive-pack.c: add receive.atomicpush configuration option
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
Documentation/config.txt | 5 +
builtin/receive-pack.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bd..75ce157 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2038,6 +2038,11 @@ rebase.autostash::
successful rebase might result in non-trivial conflicts.
Defaults to false.
+receive.atomicpush::
+ By default, git-receive-pack will only use atomic ref transactions
+ if the client negotiates it. When set to true, git-receive-pack
+ will force atomic ref updates for all client pushes.
+
receive.autogc::
By default, git-receive-pack will run git-gc --auto after
receiving data from git-push and updating refs. You can stop
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 47f778d..9fa637a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -132,6 +132,11 @@ static int receive_pack_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (strcmp(var, receive.atomicpush) == 0) {
+ use_atomic_push = git_config_bool(var, value);
+ return 0;
+ }
+
return git_default_config(var, value, cb);
}
--
2.0.1.528.gd0e7a84
--
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: Transaction patch series overview
List, please see here an overview and ordering of the ref transaction patch series. These series build on each other and needs to be applied in the order listed below. This is an update. rs/ref-transaction-0 --- Early part of the ref transaction topic. * rs/ref-transaction-0: refs.c: change ref_transaction_update() to do error checking and return status refs.c: remove the onerr argument to ref_transaction_commit update-ref: use err argument to get error from ref_transaction_commit refs.c: make update_ref_write update a strbuf on failure refs.c: make ref_update_reject_duplicates take a strbuf argument for errors refs.c: log_ref_write should try to return meaningful errno refs.c: make resolve_ref_unsafe set errno to something meaningful on error refs.c: commit_packed_refs to return a meaningful errno on failure refs.c: make remove_empty_directories always set errno to something sane refs.c: verify_lock should set errno to something meaningful refs.c: make sure log_ref_setup returns a meaningful errno refs.c: add an err argument to repack_without_refs lockfile.c: make lock_file return a meaningful errno on failurei lockfile.c: add a new public function unable_to_lock_message refs.c: add a strbuf argument to ref_transaction_commit for error logging refs.c: allow passing NULL to ref_transaction_free refs.c: constify the sha arguments for ref_transaction_create|delete|update refs.c: ref_transaction_commit should not free the transaction refs.c: remove ref_transaction_rollback Has been merged into next. ref-transaction-1 (2014-07-16) 20 commits - Second batch of ref transactions - refs.c: make delete_ref use a transaction - refs.c: make prune_ref use a transaction to delete the ref - refs.c: remove lock_ref_sha1 - refs.c: remove the update_ref_write function - refs.c: remove the update_ref_lock function - refs.c: make lock_ref_sha1 static - walker.c: use ref transaction for ref updates - fast-import.c: use a ref transaction when dumping tags - receive-pack.c: use a reference transaction for updating the refs - refs.c: change update_ref to use a transaction - branch.c: use ref transaction for all ref updates - fast-import.c: change update_branch to use ref transactions - sequencer.c: use ref transactions for all ref updates - commit.c: use ref transactions for updates - replace.c: use the ref transaction functions for updates - tag.c: use ref transactions when doing updates - refs.c: add transaction.status and track OPEN/CLOSED/ERROR - refs.c: make ref_transaction_begin take an err argument - refs.c: update ref_transaction_delete to check for error and return status - refs.c: change ref_transaction_create to do error checking and return status (this branch is used by rs/ref-transaction, rs/ref-transaction-multi, rs/ref-transaction-reflog and rs/ref-transaction-rename.) The second batch of the transactional ref update series. Has been merged into pu rs/ref-transaction (2014-07-17) 12 commits - - refs.c: fix handling of badly named refs - refs.c: make write_ref_sha1 static - fetch.c: change s_update_ref to use a ref transaction - refs.c: propagate any errno==ENOTDIR from _commit back to the callers - refs.c: pass a skip list to name_conflict_fn - refs.c: call lock_ref_sha1_basic directly from commit - refs.c: move the check for valid refname to lock_ref_sha1_basic - refs.c: pass NULL as *flags to read_ref_full - refs.c: pass the ref log message to _create/delete/update instead of _commit - refs.c: add an err argument to delete_ref_loose - wrapper.c: add a new function unlink_or_msg - wrapper.c: simplify warn_if_unremovable (this branch is used by rs/ref-transaction-multi, rs/ref-transaction-reflog and rs/ref-transaction-rename; uses rs/ref-transaction-1.) The third and final part of the basic ref-transaction work. Has been merged into pu. rs/ref-transaction-reflog (2014-07-23) 15 commits --- - refs.c: allow deleting refs with a broken sha1 - refs.c: remove lock_any_ref_for_update - refs.c: make unlock_ref/close_ref/commit_ref static - refs.c: rename log_ref_setup to create_reflog - reflog.c: use a reflog transaction when writing during expire - refs.c: allow multiple reflog updates during a single transaction - refs.c: only write reflog update if msg is non-NULL - refs.c: add a flag to allow reflog updates to truncate the log - refs.c: add a transaction function to append a reflog entry - lockfile.c: make hold_lock_file_for_append preserve meaningful errno - refs.c: add a function to append a reflog entry to a fd - refs.c: add a new update_type field to ref_update - refs.c: rename the transaction functions -
Re: Bug report about symlinks
Am 31.07.2014 um 21:50 schrieb Nikolay Avdeev: I've noticed something strange with git status when replacing a folder with symlink to another folder. There is a git repo with script with demo in the attachment. Let's try and make this a bit easier for folks to follow along. # Create test repo with two directories with two files each. $ git init Initialized empty Git repository in /tmp/.git/ $ mkdir a b $ echo x a/equal $ echo x b/equal $ echo y a/different $ echo z b/different $ git add a b $ git commit -minitial [master (root-commit) 6d36895] initial 4 files changed, 4 insertions(+) create mode 100644 a/different create mode 100644 a/equal create mode 100644 b/different create mode 100644 b/equal # Replace one directory with a symlink to the other. $ rm -rf b $ ln -s a b # First git status call. $ git status On branch master Changes not staged for commit: (use git add/rm file... to update what will be committed) (use git checkout -- file... to discard changes in working directory) deleted:b/different Untracked files: (use git add file... to include in what will be committed) b no changes added to commit (use git add and/or git commit -a) # Stage the changes. $ git add b # Second git status call. $ git status On branch master Changes to be committed: (use git reset HEAD file... to unstage) new file: b deleted:b/different deleted:b/equal # Commit the staged changes. $ git commit -msymlinked [master 4498f28] symlinked 3 files changed, 1 insertion(+), 2 deletions(-) create mode 12 b delete mode 100644 b/different delete mode 100644 b/equal That the first and second status call report different results is a feature; staging changes using git add alters the status. A commit after the first status call would be empty. It could be debated if the first git status call should also report b/equal as deleted. René -- 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
article: Using a rolling hash to break up binary files
I thought it worth bring to the list's attention a recent article on CodeProject that may be of interest to those looking at splitting binary files into deterministic hunks. http://www.codeproject.com/Articles/801608/Using-a-rolling-hash-to-break-up-binary-files It's based on Rabin and Karp's algorithm http://en.wikipedia.org/wiki/Rabin%E2%80%93Karp_algorithm. -- Philip -- 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
cherry picking and merge
Cherry picking doesn’t work as well as it should. I was testing on git version 1.7.9.5. Put in a line in a file, call it: first version then cherry pick this into your branch. Then update on master and transform that into: second version then, merge that branch back to master. Death in the form of conflicts. In gcc land, I do this sort of thing all the time, and I need a merging subsystem to actually keep track of things. I can manage this will diff and patch and it works flawlessly. The point of using something better than diff and patch is for it to be better than diff and patch. I’d like for merge to merge in the work that has yet to be merged. Not that, plus blindly try and apply or reapply cherry picked items.-- 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: cherry picking and merge
On Thu, Jul 31, 2014 at 05:58:17PM -0700, Mike Stump wrote: Cherry picking doesn’t work as well as it should. I was testing on git version 1.7.9.5. Put in a line in a file, call it: first version then cherry pick this into your branch. Then update on master and transform that into: second version then, merge that branch back to master. Death in the form of conflicts. In gcc land, I do this sort of thing all the time, and I need a merging subsystem to actually keep track of things. I can manage this will diff and patch and it works flawlessly. The point of using something better than diff and patch is for it to be better than diff and patch. I’d like for merge to merge in the work that has yet to be merged. Not that, plus blindly try and apply or reapply cherry picked items. You're not the first person to be surprised by the way merge works. From the git-merge manpage: [This behavior] occurs because only the heads and the merge base are considered when performing a merge, not the individual commits. (That was added after 1.7.9.5.) If you want the behavior of applying multiple patches in a row, you want to use git rebase, not git merge. Since rebase re-applies the patches of each of your commits on top of another branch, the identical change won't cause conflicts. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: article: Using a rolling hash to break up binary files
On Thu, Jul 31, 2014 at 3:31 PM, Philip Oakley philipoak...@iee.org wrote: I thought it worth bring to the list's attention a recent article on CodeProject that may be of interest to those looking at splitting binary files into deterministic hunks. http://www.codeproject.com/Articles/801608/Using-a-rolling-hash-to-break-up-binary-files It's based on Rabin and Karp's algorithm http://en.wikipedia.org/wiki/Rabin%E2%80%93Karp_algorithm. If I remember right, this is how bup[1] works. Its certainly what we do for delta compressing files. [1] https://github.com/bup/bup -- 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
