Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints

2014-08-07 Thread Peter Krefting

Fabian Ruch:


  2. Notice ourselves that the end-result of the whole squash is an
 empty commit, and stop to let the user deal with it.


This patch chooses the second alternative. Either way seems OK. The 
crucial consensus of the discussion was to silently throw away empty 
interim commits.


Yes, the important part is that giving good advice is better than 
giving bad advice. Thank you for taking your time to fix this.


I haven't reviewed the changes themselves, but I am happy with the 
underlying idea.


--
\\// Peter - http://www.softwolves.pp.se/
--
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 v8 8/8] add tests for `git_config_get_string_const()`

2014-08-07 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Tanay Abhra tanay...@gmail.com writes:

 ...
 +   grep line 7.*.git/config\|.git/config.*line 7 result
 +'

 This is still dependant on the locale (line is translated). You need
 to use test_i18ngrep instead of grep here (see its definition and
 comment in t/test-lib.sh).

 I don't think you need these two alternatives OTOH.

 BTW, Junio, I don't understand your remark This test is too tight (the
 full string) in the previous iteration. Can you elaborate?

 The original test was trying to match the string fully, i.e.

 +grep fatal: bad config variable '\''alias.br'\'' at file line 2 in 
 .git/config result

 As I already was feeling funny about seeing the phrase file line
 in the message and expecting it to be corrected, I thought I should
 encourage a check that does not depend on minor phrasing changes, if
 it can be done without bending backwards.

OK.

I personally prefer tight tests in this case, as the patch doing the
rephrase sees what changed by running the tests, and documents the
visible change by changing the expected in the tests scripts. But no
strong opinion here, I'd be fine with e.g.

test_i18ngrep fatal: .* alias.br.*line 2 in .git/config result

-- 
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: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Tony Finch
Nico Williams n...@cryptonector.com wrote:
 On Wed, Aug 06, 2014 at 08:31:18PM +0200, Jakub Narębski wrote:
  On Wed, Aug 6, 2014 at 6:26 PM, Nico Williams n...@cryptonector.com wrote:
  
   My proposal was to put this sort of ancillary history info in a
   branch object (and that branches should be objects).
 
  Is it something like object-ified reflog, similar to how replacement
  objects (git-replace) can be thought to be object-ified grafts (I know
  they are more)? Do I understand it correctly?

 Yes, per-branch.  At push time a commit would be pushed to the upstream
 branch listing the commits pushed now (and who by).  Locally every
 rebase/cherry-pick/merge/commit onto the branch would appear in the
 branch object's history, kinda just like the reflog.  The main
 difference is that the upstream branch's history could be viewed.

  With rebases the problem is that it would be nice to have (at least
  for a short time) the history of series of patches (the metahistory,
  or history of a branch), but usually one doesn't need old pre-rebase
  version after cleaning up the history for publishing.

 Right.

I have been fiddling around in this area.

What I want to be able to do is develop fixes for open source code that I
run, and get those fixes upstream. This means I need a rebasing workflow,
to keep the patches up-to-date and to deal with code review feedback.

But this is inconvenient for deploying the patched version to production
(which is the point of developing the fixes) - I want a fast-forwarding
branch for that. And it would be nice to be able to share the history of
the patch series, so others can see what changed between revisions more
easily.

So I have a small tool which maintains a publication branch which tracks
the head of a rebasing branch. It's reasonably satisfactory so far...

https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git

... though the structure of the publication branch is weird and not very
easy to navigate. You can see it in action in my git.git repo:

https://git.csx.cam.ac.uk/x/ucs/git/git.git/shortlog/refs/heads/ucam/fanf2/patch

Tony.
-- 
f.anthony.n.finch  d...@dotat.at  http://dotat.at/
Irish Sea: Variable 4. Slight. Showers. Good.

[PATCH v9 0/8] Rewrite `git_config()` using config-set API

2014-08-07 Thread Tanay Abhra
[Patch v9]: Changed the grep statements in patch 7/8 and 8/8.

[Patch v8]: git_die_config now allows custom error messages.
new tests are now not too reliant on specific strings.

[Patch v7]: style nit corrected. (1/8) is Matthieu's translation patch.
git_die_config_linenr() helper function added. Diff between v6
and v7 appended for review.

[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 or (ta/config-set 
in pu).

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


Matthieu Moy (1):
  config.c: mark error and warnings strings for translation

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
  config: add `git_die_config()` to the config-set API
  rewrite git_config() to use the config-set API
  add a test for semantic errors in config files
  add tests for `git_config_get_string_const()`

 Documentation/technical/api-config.txt |  13 +++
 branch.c   |   5 +-
 cache.h|  34 +++-
 config.c   | 152 +++--
 t/t1308-config-set.sh  |  21 +
 t/t4055-diff-context.sh|   2 +-
 test-config.c  |  10 +++
 7 files changed, 207 insertions(+), 30 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


[PATCH v9 1/8] config.c: mark error and warnings strings for translation

2014-08-07 Thread Tanay Abhra
From: Matthieu Moy matthieu@imag.fr

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 config.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/config.c b/config.c
index a191328..34940fd 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,
-- 
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 v9 5/8] config: add `git_die_config()` to the config-set API

2014-08-07 Thread Tanay Abhra
Add `git_die_config` that dies printing the line number and the file name
of the highest priority value for the configuration variable `key`. A custom
error message is also printed before dying, specified by the caller, which can
be skipped if `err` argument is set to NULL.

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)){
if (!strcmp(value, foo))
git_config_die(key, value: `%s` is illegal, value);
else
/* do work */
}

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 Documentation/technical/api-config.txt | 13 
 cache.h|  3 +++
 config.c   | 39 --
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 21f280c..0d8b99b 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,6 +155,19 @@ 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.
 
+`git_die_config(const char *key, const char *err, ...)`::
+
+   First prints the error message specified by the caller in `err` and then
+   dies printing the line number and the file name of the highest priority
+   value for the configuration variable `key`.
+
+`void git_die_config_linenr(const char *key, const char *filename, int 
linenr)`::
+
+   Helper function which formats the die error message according to the
+   parameters entered. Used by `git_die_config()`. It can be used by 
callers
+   handling `git_config_get_value_multi()` to print the correct error 
message
+   for the desired value.
+
 See test-config.c for usage examples.
 
 Value Parsing Helpers
diff --git a/cache.h b/cache.h
index f11ce41..89a0d51 100644
--- a/cache.h
+++ b/cache.h
@@ -1388,6 +1388,9 @@ struct key_value_info {
int linenr;
 };
 
+extern NORETURN void git_die_config(const char *key, const char *err, ...) 
__attribute__((format(printf, 2, 3)));
+extern NORETURN void git_die_config_linenr(const char *key, const char 
*filename, int linenr);
+
 extern int committer_ident_sufficiently_given(void);
 extern int author_ident_sufficiently_given(void);
 
diff --git a/config.c b/config.c
index 4cefa25..5ae9ab0 100644
--- a/config.c
+++ b/config.c
@@ -1469,8 +1469,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, NULL);
+   return ret;
 }
 
 int git_config_get_string(const char *key, char **dest)
@@ -1511,8 +1515,39 @@ 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, NULL);
+   return ret;
+}
+
+NORETURN
+void git_die_config_linenr(const char *key, const char *filename, int linenr)
+{
+   if (!filename)
+   die(_(unable to parse '%s' from command-line config), key);
+   else
+   die(_(bad config variable '%s' in file '%s' at line %d),
+   key, filename, linenr);
+}
+
+NORETURN __attribute__((format(printf, 2, 3)))
+void git_die_config(const char *key, const char *err, ...)
+{
+   const struct string_list *values;
+   struct key_value_info *kv_info;
+
+   if (err) {
+   va_list params;
+   va_start(params, err);
+   vreportf(error: , err, params);
+   va_end(params);
+   }
+   values = git_config_get_value_multi(key);
+   kv_info = values-items[values-nr - 1].util;
+   git_die_config_linenr(key, kv_info-filename, kv_info-linenr);
 }
 
 /*
-- 
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 v9 2/8] config.c: fix accuracy of line number in errors

2014-08-07 Thread Tanay Abhra
From: Matthieu Moy matthieu@imag.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
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 config.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index 34940fd..bb4629e 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 v9 4/8] change `git_config()` return value to void

2014-08-07 Thread Tanay Abhra
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 0b1bdfd..f11ce41 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 e4d745e..4cefa25 100644
--- a/config.c
+++ b/config.c
@@ -1230,9 +1230,21 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
 }
 
-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 v9 3/8] add line number and file name info to `config_set`

2014-08-07 Thread Tanay Abhra
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
---
 cache.h  |  5 +
 config.c | 16 ++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index 7292aef..0b1bdfd 100644
--- a/cache.h
+++ b/cache.h
@@ -1383,6 +1383,11 @@ 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);
 
+struct key_value_info {
+   const char *filename;
+   int linenr;
+};
+
 extern int committer_ident_sufficiently_given(void);
 extern int author_ident_sufficiently_given(void);
 
diff --git a/config.c b/config.c
index bb4629e..e4d745e 100644
--- a/config.c
+++ b/config.c
@@ -1260,6 +1260,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 +1275,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 = NULL;
+   kv_info-linenr = -1;
+   }
+   si-util = kv_info;
 
return 0;
 }
@@ -1299,7 +1311,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 v9 8/8] add tests for `git_config_get_string_const()`

2014-08-07 Thread Tanay Abhra
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 9cc678d..ea0bce2 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 
+   test_i18ngrep fatal: .*case\.foo.*\.git/config.*line 7 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 v9 6/8] rewrite git_config() to use the config-set API

2014-08-07 Thread Tanay Abhra
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| 51 +
 t/t4055-diff-context.sh |  2 +-
 3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 89a0d51..2693a37 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 5ae9ab0..427850a 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;
@@ -1230,7 +1224,7 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
 }
 
-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)
/*
@@ -1247,6 +1241,33 @@ 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;
+   git_die_config_linenr(entry-key, kv_info-filename, 
kv_info-linenr);
+   }
+   }
+}
+
+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;
@@ -1273,6 +1294,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);
@@ -1288,6 +1310,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) {
kv_info-filename = strintern(cf-name);
kv_info-linenr = cf-linenr;
@@ -1311,6 +1339,9 @@ void git_configset_init(struct config_set *cs)
 {
hashmap_init(cs-config_hash, (hashmap_cmp_fn)config_set_element_cmp, 
0);
cs-hash_initialized = 1;
+   cs-list.nr = 0;
+

[PATCH v9 7/8] add a test for semantic errors in config files

2014-08-07 Thread Tanay Abhra
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..9cc678d 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 
+   test_i18ngrep fatal: .*alias\.br.*\.git/config.*line 2 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


Re: [PATCH v9 0/8] Rewrite `git_config()` using config-set API

2014-08-07 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 [Patch v9]: Changed the grep statements in patch 7/8 and 8/8.

Good. I think it adresses all previous comments.

-- 
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: Pluggable backends for refs,wip

2014-08-07 Thread Michael Haggerty
On 08/05/2014 02:40 PM, Ronnie Sahlberg wrote:
 Please see
 https://github.com/rsahlberg/git/tree/backend-struct-db-2
 for an example of a pluggable backend for refs storage.
 
 This series contain changes to make it possible to add new backends
 for handling/storage of refs and implements one new backend :
 refs-be-be.c .
 
 This new backend offloads the actual refs handling to a small database
 daemon with which ita talks via a very simple rpc protocol. That
 daemon in turn then connects to the datastore and read/writes the
 values to it.
 [...]

Ronnie,

This is awesome!  Congratulations on your progress.

I'm still on vacation and haven't yet looked at the code.  I will be
back next week and hope to find time to check it out, and also to do
some more review of the code that you have already submitted to git core.


Have you thought about how to test alternate reference backends?  This
will be very important to getting one or more of them accepted into git
core (not to mention giving people confidence to actually *use* them!)

It seems to me that a few steps are needed:

* Each backend would need a suite of backend-aware tests that verify
proper operation *within* the backend.  These tests would mostly use
low-level plumbing commands like update-refs to create/modify/delete
references, and would be allowed to grub around in the filesystem, talk
directly with the database, etc. to make sure that the commands have the
correct effects.  For example, for the traditional filesystem backend,
these tests would be the ones to check that creating a reference causes
a file to spring into existence under $GIT_DIR/refs.

The tests for pack-refs, and all tests that care about the distinction
between packed and loose refs, would become part of the backend-aware
tests for the filesystem backend.

All of the backend-aware tests should be run every time the test suite
is run (provided, of course, that the correct prerequisites are
available, and subject to being turned off manually).

* The rest of the test suite has to be made backend-agnostic.  For
example, such tests should *not* be allowed to look under $GIT_DIR for
the existence/absence of loose reference files [1] but would rather have
to inquire about references via git commands.

* It should be possible for the developer to choose easily which
reference backend to use when running the agnostic part of the test
suite.  The chosen backend should be used to run *all* backend-agnostic
tests.

A database-backed backend might even want to be testable in two modes:
one with the DB daemon running constantly, and one where the daemon is
stopped and started between each pair of Git commands.

So after the changes, a single run of the test suite should run the
backend-aware tests for *all* known backends followed by the
backend-agnostic tests for a single selected backend.

Michael

[1] When I was working on my quagga-reference spike [2] I found that a
lot of the test suite uses knowledge about how references and reflogs
are stored by the filesystem backend and just grabs at the files rather
than accessing the references using git commands.  It will take some
work to clean this up.

[2] http://thread.gmane.org/gmane.comp.version-control.git/243726

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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] various contrib: Fix links in man pages

2014-08-07 Thread Stefan Beller
Inspired by 2147fa7e (2014-07-31 git-push: fix link in man page),
I grepped through the whole tree searching for 'gitlink:' occurrences.

Signed-off-by: Stefan Beller stefanbel...@gmail.com
---
 contrib/convert-objects/git-convert-objects.txt | 2 +-
 contrib/examples/git-svnimport.txt  | 2 +-
 contrib/gitview/gitview.txt | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/convert-objects/git-convert-objects.txt 
b/contrib/convert-objects/git-convert-objects.txt
index 0565d83..f871880 100644
--- a/contrib/convert-objects/git-convert-objects.txt
+++ b/contrib/convert-objects/git-convert-objects.txt
@@ -26,4 +26,4 @@ Documentation by David Greaves, Junio C Hamano and the 
git-list git@vger.kernel
 
 GIT
 ---
-Part of the gitlink:git[7] suite
+Part of the linkgit:git[7] suite
diff --git a/contrib/examples/git-svnimport.txt 
b/contrib/examples/git-svnimport.txt
index 3bb871e..3f0a9c3 100644
--- a/contrib/examples/git-svnimport.txt
+++ b/contrib/examples/git-svnimport.txt
@@ -176,4 +176,4 @@ Documentation by Matthias Urlichs sm...@smurf.noris.de.
 
 GIT
 ---
-Part of the gitlink:git[7] suite
+Part of the linkgit:git[7] suite
diff --git a/contrib/gitview/gitview.txt b/contrib/gitview/gitview.txt
index 9e12f97..7b5f900 100644
--- a/contrib/gitview/gitview.txt
+++ b/contrib/gitview/gitview.txt
@@ -28,7 +28,7 @@ OPTIONS
 
 args::
 
-   All the valid option for gitlink:git-rev-list[1].
+   All the valid option for linkgit:git-rev-list[1].
 
 Key Bindings
 
-- 
2.1.0.rc0.52.gaa544bf

--
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: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Nico Williams
On Thu, Aug 07, 2014 at 12:38:48PM +0100, Tony Finch wrote:
 I have been fiddling around in this area.
 
 What I want to be able to do is develop fixes for open source code
 that I run, and get those fixes upstream. This means I need a rebasing
 workflow, to keep the patches up-to-date and to deal with code review
 feedback.

Right.

 But this is inconvenient for deploying the patched version to
 production (which is the point of developing the fixes) - I want a

I'm not sure I follow this.  You deploy what you build, and you build
the HEAD of the production branch, whatever that is.  If it gets
rebased, so it it does.

 fast-forwarding branch for that. And it would be nice to be able to
 share the history of the patch series, so others can see what changed
 between revisions more easily.

But yes, it's nice to have a history of all the rebases.  For example:
so you can show the work you've done (rebasing to please an upstream is
work).

The reflog does this, of course, but you can't push it.  Of course, my
conception of branch object wouldn't push rebase history to an upstream
that doesn't want it, but you could push it to repos that do.

 So I have a small tool which maintains a publication branch which
 tracks the head of a rebasing branch. It's reasonably satisfactory so
 far...
 
 https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git

Yeah, that's useful.

Nico
-- 
--
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: Subtree with submodule inside?

2014-08-07 Thread Heiko Voigt
On Wed, Aug 06, 2014 at 04:51:52PM -0700, Jonathan Nieder wrote:
 Junio C Hamano wrote:
  Jonathan Nieder jrnie...@gmail.com writes:
 
   2. Submodules aware of their superproject and of the parent's branches.
  In other words, submodules would act as though under refs/ they
  had a symlink
 
 parent - ../../../refs
 
  So you could do
 
 git checkout --recurse-submodules master
 
 cd path/to/submodule
 git checkout parent/heads/next
 
  This would avoid danger from git gc in submodules and would
  get rid of most of the motivation for named branches in the
  submodule, I'd think.
 
  Are you assuming that they share their object stores?
 
 No.  The 'symlink' thing is a think-o.  (When trying to explain the
 idea I ended up oversimplifying and speaking nonsense.)
 
 What I wanted to say is that parent/heads/next would be a way to
 refer from the submodule to the same commit as
 
   refs/heads/next:path/to/submodule
 
 refers to in the parent.

I like this idea. It could solve many issues and help in many cases I think.
Since we are currently quite busy with other things I took the liberty of
adding an ideas section in Jens submodule wiki[1]. This way we do not forget
about it and/or can refer others to it more easily.

I would appreciate if someone could have a look whether I described the idea
clearly enough.

Cheers Heiko

[1] 
https://github.com/jlehmann/git-submod-enhancements/wiki#dynamic-superproject-refs-in-submodules
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 05/11] fetchpack.c: replace `git_config()` with `git_config_get_*()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_*()` family instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 fetch-pack.c | 35 ---
 1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index b8a58fa..a13e9db 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -869,34 +869,15 @@ static struct ref *do_fetch_pack(struct fetch_pack_args 
*args,
return ref;
 }
 
-static int fetch_pack_config(const char *var, const char *value, void *cb)
+static void fetch_pack_config(void)
 {
-   if (strcmp(var, fetch.unpacklimit) == 0) {
-   fetch_unpack_limit = git_config_int(var, value);
-   return 0;
-   }
-
-   if (strcmp(var, transfer.unpacklimit) == 0) {
-   transfer_unpack_limit = git_config_int(var, value);
-   return 0;
-   }
-
-   if (strcmp(var, repack.usedeltabaseoffset) == 0) {
-   prefer_ofs_delta = git_config_bool(var, value);
-   return 0;
-   }
-
-   if (!strcmp(var, fetch.fsckobjects)) {
-   fetch_fsck_objects = git_config_bool(var, value);
-   return 0;
-   }
-
-   if (!strcmp(var, transfer.fsckobjects)) {
-   transfer_fsck_objects = git_config_bool(var, value);
-   return 0;
-   }
+   git_config_get_int(fetch.unpacklimit, fetch_unpack_limit);
+   git_config_get_int(transfer.unpacklimit, transfer_unpack_limit);
+   git_config_get_bool(repack.usedeltabaseoffset, prefer_ofs_delta);
+   git_config_get_bool(fetch.fsckobjects, fetch_fsck_objects);
+   git_config_get_bool(transfer.fsckobjects, transfer_fsck_objects);
 
-   return git_default_config(var, value, cb);
+   git_config(git_default_config, NULL);
 }
 
 static void fetch_pack_setup(void)
@@ -904,7 +885,7 @@ static void fetch_pack_setup(void)
static int did_setup;
if (did_setup)
return;
-   git_config(fetch_pack_config, NULL);
+   fetch_pack_config();
if (0 = transfer_unpack_limit)
unpack_limit = transfer_unpack_limit;
else if (0 = fetch_unpack_limit)
-- 
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 v2 06/11] rerere.c: replace `git_config()` with `git_config_get_*()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_*()` family instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 rerere.c | 43 ---
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/rerere.c b/rerere.c
index d84b495..20b18ad 100644
--- a/rerere.c
+++ b/rerere.c
@@ -573,15 +573,11 @@ static int do_plain_rerere(struct string_list *rr, int fd)
return write_rr(rr, fd);
 }
 
-static int git_rerere_config(const char *var, const char *value, void *cb)
+static void git_rerere_config(void)
 {
-   if (!strcmp(var, rerere.enabled))
-   rerere_enabled = git_config_bool(var, value);
-   else if (!strcmp(var, rerere.autoupdate))
-   rerere_autoupdate = git_config_bool(var, value);
-   else
-   return git_default_config(var, value, cb);
-   return 0;
+   git_config_get_bool(rerere.enabled, rerere_enabled);
+   git_config_get_bool(rerere.autoupdate, rerere_autoupdate);
+   git_config(git_default_config, NULL);
 }
 
 static int is_rerere_enabled(void)
@@ -606,7 +602,7 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 {
int fd;
 
-   git_config(git_rerere_config, NULL);
+   git_rerere_config();
if (!is_rerere_enabled())
return -1;
 
@@ -699,24 +695,6 @@ static void unlink_rr_item(const char *name)
rmdir(git_path(rr-cache/%s, name));
 }
 
-struct rerere_gc_config_cb {
-   int cutoff_noresolve;
-   int cutoff_resolve;
-};
-
-static int git_rerere_gc_config(const char *var, const char *value, void *cb)
-{
-   struct rerere_gc_config_cb *cf = cb;
-
-   if (!strcmp(var, gc.rerereresolved))
-   cf-cutoff_resolve = git_config_int(var, value);
-   else if (!strcmp(var, gc.rerereunresolved))
-   cf-cutoff_noresolve = git_config_int(var, value);
-   else
-   return git_default_config(var, value, cb);
-   return 0;
-}
-
 void rerere_gc(struct string_list *rr)
 {
struct string_list to_remove = STRING_LIST_INIT_DUP;
@@ -724,9 +702,12 @@ void rerere_gc(struct string_list *rr)
struct dirent *e;
int i, cutoff;
time_t now = time(NULL), then;
-   struct rerere_gc_config_cb cf = { 15, 60 };
+   int cutoff_noresolve = 15;
+   int cutoff_resolve = 60;
 
-   git_config(git_rerere_gc_config, cf);
+   git_config_get_int(gc.rerereresolved, cutoff_resolve);
+   git_config_get_int(gc.rerereunresolved, cutoff_noresolve);
+   git_config(git_default_config, NULL);
dir = opendir(git_path(rr-cache));
if (!dir)
die_errno(unable to open rr-cache directory);
@@ -736,12 +717,12 @@ void rerere_gc(struct string_list *rr)
 
then = rerere_last_used_at(e-d_name);
if (then) {
-   cutoff = cf.cutoff_resolve;
+   cutoff = cutoff_resolve;
} else {
then = rerere_created_at(e-d_name);
if (!then)
continue;
-   cutoff = cf.cutoff_noresolve;
+   cutoff = cutoff_noresolve;
}
if (then  now - cutoff * 86400)
string_list_append(to_remove, e-d_name);
-- 
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 v2 08/11] pager.c: replace `git_config()` with `git_config_get_value()`

2014-08-07 Thread Tanay Abhra
Use `git_config_get_value()` instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 pager.c | 40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/pager.c b/pager.c
index 8b5cbc5..b7eb7e7 100644
--- a/pager.c
+++ b/pager.c
@@ -6,12 +6,6 @@
 #define DEFAULT_PAGER less
 #endif
 
-struct pager_config {
-   const char *cmd;
-   int want;
-   char *value;
-};
-
 /*
  * This is split up from the rest of git so that we can do
  * something different on Windows.
@@ -155,30 +149,22 @@ int decimal_width(int number)
return width;
 }
 
-static int pager_command_config(const char *var, const char *value, void *data)
+/* returns 0 for no pager, 1 for use pager, and -1 for not specified */
+int check_pager_config(const char *cmd)
 {
-   struct pager_config *c = data;
-   if (starts_with(var, pager.)  !strcmp(var + 6, c-cmd)) {
-   int b = git_config_maybe_bool(var, value);
+   int want = -1;
+   struct strbuf key = STRBUF_INIT;
+   const char *value = NULL;
+   strbuf_addf(key, pager.%s, cmd);
+   if (!git_config_get_value(key.buf, value)) {
+   int b = git_config_maybe_bool(key.buf, value);
if (b = 0)
-   c-want = b;
+   want = b;
else {
-   c-want = 1;
-   c-value = xstrdup(value);
+   want = 1;
+   pager_program = xstrdup(value);
}
}
-   return 0;
-}
-
-/* returns 0 for no pager, 1 for use pager, and -1 for not specified */
-int check_pager_config(const char *cmd)
-{
-   struct pager_config c;
-   c.cmd = cmd;
-   c.want = -1;
-   c.value = NULL;
-   git_config(pager_command_config, c);
-   if (c.value)
-   pager_program = c.value;
-   return c.want;
+   strbuf_release(key);
+   return want;
 }
-- 
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 v2 04/11] archive.c: replace `git_config()` with `git_config_get_bool()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_bool()` family instead of `git_config()` to take advantage 
of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 archive.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/archive.c b/archive.c
index 3fc0fb2..952a659 100644
--- a/archive.c
+++ b/archive.c
@@ -402,14 +402,6 @@ static int parse_archive_args(int argc, const char **argv,
return argc;
 }
 
-static int git_default_archive_config(const char *var, const char *value,
- void *cb)
-{
-   if (!strcmp(var, uploadarchive.allowunreachable))
-   remote_allow_unreachable = git_config_bool(var, value);
-   return git_default_config(var, value, cb);
-}
-
 int write_archive(int argc, const char **argv, const char *prefix,
  int setup_prefix, const char *name_hint, int remote)
 {
@@ -420,7 +412,9 @@ int write_archive(int argc, const char **argv, const char 
*prefix,
if (setup_prefix  prefix == NULL)
prefix = setup_git_directory_gently(nongit);
 
-   git_config(git_default_archive_config, NULL);
+   git_config_get_bool(uploadarchive.allowunreachable, 
remote_allow_unreachable);
+   git_config(git_default_config, NULL);
+
init_tar_archiver();
init_zip_archiver();
 
-- 
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 v2 10/11] alias.c: replace `git_config()` with `git_config_get_string()`

2014-08-07 Thread Tanay Abhra
Use `git_config_get_string()` instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 alias.c | 25 ++---
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/alias.c b/alias.c
index 758c867..6aa164a 100644
--- a/alias.c
+++ b/alias.c
@@ -1,26 +1,13 @@
 #include cache.h
 
-static const char *alias_key;
-static char *alias_val;
-
-static int alias_lookup_cb(const char *k, const char *v, void *cb)
-{
-   const char *name;
-   if (skip_prefix(k, alias., name)  !strcmp(name, alias_key)) {
-   if (!v)
-   return config_error_nonbool(k);
-   alias_val = xstrdup(v);
-   return 0;
-   }
-   return 0;
-}
-
 char *alias_lookup(const char *alias)
 {
-   alias_key = alias;
-   alias_val = NULL;
-   git_config(alias_lookup_cb, NULL);
-   return alias_val;
+   char *v = NULL;
+   struct strbuf key = STRBUF_INIT;
+   strbuf_addf(key, alias.%s, alias);
+   git_config_get_string(key.buf, v);
+   strbuf_release(key);
+   return v;
 }
 
 #define SPLIT_CMDLINE_BAD_ENDING 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 v2 01/11] daemon.c: replace `git_config()` with `git_config_get_bool()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_bool()` family instead of `git_config()` to take advantage 
of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 daemon.c | 26 --
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/daemon.c b/daemon.c
index e6b51ed..6f78b61 100644
--- a/daemon.c
+++ b/daemon.c
@@ -230,23 +230,6 @@ struct daemon_service {
int overridable;
 };
 
-static struct daemon_service *service_looking_at;
-static int service_enabled;
-
-static int git_daemon_config(const char *var, const char *value, void *cb)
-{
-   const char *service;
-
-   if (skip_prefix(var, daemon., service) 
-   !strcmp(service, service_looking_at-config_name)) {
-   service_enabled = git_config_bool(var, value);
-   return 0;
-   }
-
-   /* we are not interested in parsing any other configuration here */
-   return 0;
-}
-
 static int daemon_error(const char *dir, const char *msg)
 {
if (!informative_errors)
@@ -324,6 +307,7 @@ static int run_service(const char *dir, struct 
daemon_service *service)
 {
const char *path;
int enabled = service-enabled;
+   struct strbuf var = STRBUF_INIT;
 
loginfo(Request %s for '%s', service-name, dir);
 
@@ -354,11 +338,9 @@ static int run_service(const char *dir, struct 
daemon_service *service)
}
 
if (service-overridable) {
-   service_looking_at = service;
-   service_enabled = -1;
-   git_config(git_daemon_config, NULL);
-   if (0 = service_enabled)
-   enabled = service_enabled;
+   strbuf_addf(var, daemon.%s, service-config_name);
+   git_config_get_bool(var.buf, enabled);
+   strbuf_release(var);
}
if (!enabled) {
logerror('%s': service not enabled for '%s',
-- 
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 v2 00/11] git_config callers rewritten with the new config-set API

2014-08-07 Thread Tanay Abhra
[v2]: git_die_config() messages changed. Diff between v1 and v2 is at the 
bottom.

The ta/config-set API is more or less solidified.

This series builds on the top of 4c715ebb in pu (ta/config-set). On top of it,
it also requires series [1] (Rewrite `git_config()` using config-set API) for
proper error checking.

This series is the first batch of patches which rewrites the existing callers
using a non-callback approach.
This series aims to,

* rewrite the existing callers, as you can see from the diff stat the bew API
  provides a much concise and clear control flow.

* stress test the new API, see if any corner cases or deficiencies arise or not.

The series passes all the tests, only thing to watch is that the config 
variables
that have been rewritten are single valued only. Though I have tried my best to
ascertain it, still mistakes may arise.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/254633/

Tanay Abhra (11):
  daemon.c: replace `git_config()` with `git_config_get_bool()` family
  http-backend.c: replace `git_config()` with `git_config_get_bool()`
family
  read-cache.c: replace `git_config()` with `git_config_get_*()` family
  archive.c: replace `git_config()` with `git_config_get_bool()` family
  fetchpack.c: replace `git_config()` with `git_config_get_*()` family
  rerere.c: replace `git_config()` with `git_config_get_*()` family
  builtin/gc.c: replace `git_config()` with `git_config_get_*()` family
  pager.c: replace `git_config()` with `git_config_get_value()`
  imap-send.c: replace `git_config()` with `git_config_get_*()` family
  alias.c: replace `git_config()` with `git_config_get_string()`
  branch.c: replace `git_config()` with `git_config_get_string()

 alias.c| 25 ++--
 archive.c  | 12 +++-
 branch.c   | 27 +++---
 builtin/gc.c   | 51 -
 daemon.c   | 26 -
 fetch-pack.c   | 35 --
 http-backend.c | 31 --
 imap-send.c| 60 +-
 pager.c| 40 +--
 read-cache.c   | 14 +++---
 rerere.c   | 43 -
 11 files changed, 114 insertions(+), 250 deletions(-)

-- 
1.9.0.GIT


-- 8 --
diff --git a/builtin/gc.c b/builtin/gc.c
index 4612ef5..5173657 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -76,8 +76,8 @@ static void gc_config(void)
if (strcmp(prune_expire, now)) {
  unsigned long now = approxidate(now);
  if (approxidate(prune_expire) = now) {
-   error(_(Invalid %s: '%s'), gc.pruneexpire, prune_expire);
-   git_die_config(gc.pruneexpire);
+   git_die_config(gc.pruneexpire, _(Invalid gc.pruneexpire: '%s'),
+   prune_expire);
  }
}
  }


diff --git a/daemon.c b/daemon.c
index fb16664..6f78b61 100644
--- a/daemon.c
+++ b/daemon.c
@@ -342,7 +342,6 @@ static int run_service(const char *dir, struct 
daemon_service *service)
git_config_get_bool(var.buf, enabled);
strbuf_release(var);
  }
-
  if (!enabled) {
logerror('%s': service not enabled for '%s',
   service-name, path);
diff --git a/imap-send.c b/imap-send.c
index 586bdd8..618d75b 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1336,8 +1336,7 @@ static void git_imap_config(void)
 
  if (!git_config_get_value(imap.host, val)) {
if (!val) {
- config_error_nonbool(imap.host);
- git_die_config(imap.host);
+ git_die_config(imap.host, Missing value for 'imap.host');
} else {
  if (starts_with(val, imap:))
  val += 5;
-- 8 --
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 02/11] http-backend.c: replace `git_config()` with `git_config_get_bool()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_bool()` family instead of `git_config()` to take advantage 
of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 http-backend.c | 31 ---
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index 80790bb..106ca6b 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -219,29 +219,22 @@ static void get_idx_file(char *name)
send_local_file(application/x-git-packed-objects-toc, name);
 }
 
-static int http_config(const char *var, const char *value, void *cb)
+static void http_config(void)
 {
-   const char *p;
+   int i, value = 0;
+   struct strbuf var = STRBUF_INIT;
 
-   if (!strcmp(var, http.getanyfile)) {
-   getanyfile = git_config_bool(var, value);
-   return 0;
-   }
+   git_config_get_bool(http.getanyfile, getanyfile);
 
-   if (skip_prefix(var, http., p)) {
-   int i;
-
-   for (i = 0; i  ARRAY_SIZE(rpc_service); i++) {
-   struct rpc_service *svc = rpc_service[i];
-   if (!strcmp(p, svc-config_name)) {
-   svc-enabled = git_config_bool(var, value);
-   return 0;
-   }
-   }
+   for (i = 0; i  ARRAY_SIZE(rpc_service); i++) {
+   struct rpc_service *svc = rpc_service[i];
+   strbuf_addf(var, http.%s, svc-config_name);
+   if (!git_config_get_bool(var.buf, value))
+   svc-enabled = value;
+   strbuf_reset(var);
}
 
-   /* we are not interested in parsing any other configuration here */
-   return 0;
+   strbuf_release(var);
 }
 
 static struct rpc_service *select_service(const char *name)
@@ -627,7 +620,7 @@ int main(int argc, char **argv)
access(git-daemon-export-ok, F_OK) )
not_found(Repository not exported: '%s', dir);
 
-   git_config(http_config, NULL);
+   http_config();
cmd-imp(cmd_arg);
return 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 v2 03/11] read-cache.c: replace `git_config()` with `git_config_get_*()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_*()` family instead of `git_config()` to take
advantage of the config-set API which provides a cleaner control flow.

Use an intermediate value, as `version` can not be used directly in
git_config_get_int() due to incompatible type.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 read-cache.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 5d3c8bd..acb132d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1238,24 +1238,16 @@ static struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static int index_format_config(const char *var, const char *value, void *cb)
-{
-   unsigned int *version = cb;
-   if (!strcmp(var, index.version)) {
-   *version = git_config_int(var, value);
-   return 0;
-   }
-   return 1;
-}
-
 static unsigned int get_index_format_default(void)
 {
char *envversion = getenv(GIT_INDEX_VERSION);
char *endp;
+   int value;
unsigned int version = INDEX_FORMAT_DEFAULT;
 
if (!envversion) {
-   git_config(index_format_config, version);
+   if (!git_config_get_int(index.version, value))
+   version = value;
if (version  INDEX_FORMAT_LB || INDEX_FORMAT_UB  version) {
warning(_(index.version set, but the value is 
invalid.\n
  Using version %i), INDEX_FORMAT_DEFAULT);
-- 
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 v2 09/11] imap-send.c: replace `git_config()` with `git_config_get_*()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_*()` family instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 imap-send.c | 60 ++--
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 524fbab..618d75b 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1326,43 +1326,35 @@ static int split_msg(struct strbuf *all_msgs, struct 
strbuf *msg, int *ofs)
 
 static char *imap_folder;
 
-static int git_imap_config(const char *key, const char *val, void *cb)
+static void git_imap_config(void)
 {
-   if (!skip_prefix(key, imap., key))
-   return 0;
+   const char *val = NULL;
+
+   git_config_get_bool(imap.sslverify, server.ssl_verify);
+   git_config_get_bool(imap.preformattedhtml, server.use_html);
+   git_config_get_string(imap.folder, imap_folder);
 
-   /* check booleans first, and barf on others */
-   if (!strcmp(sslverify, key))
-   server.ssl_verify = git_config_bool(key, val);
-   else if (!strcmp(preformattedhtml, key))
-   server.use_html = git_config_bool(key, val);
-   else if (!val)
-   return config_error_nonbool(key);
-
-   if (!strcmp(folder, key)) {
-   imap_folder = xstrdup(val);
-   } else if (!strcmp(host, key)) {
-   if (starts_with(val, imap:))
-   val += 5;
-   else if (starts_with(val, imaps:)) {
-   val += 6;
-   server.use_ssl = 1;
+   if (!git_config_get_value(imap.host, val)) {
+   if (!val) {
+   git_die_config(imap.host, Missing value for 
'imap.host');
+   } else {
+   if (starts_with(val, imap:))
+   val += 5;
+   else if (starts_with(val, imaps:)) {
+   val += 6;
+   server.use_ssl = 1;
+   }
+   if (starts_with(val, //))
+   val += 2;
+   server.host = xstrdup(val);
}
-   if (starts_with(val, //))
-   val += 2;
-   server.host = xstrdup(val);
-   } else if (!strcmp(user, key))
-   server.user = xstrdup(val);
-   else if (!strcmp(pass, key))
-   server.pass = xstrdup(val);
-   else if (!strcmp(port, key))
-   server.port = git_config_int(key, val);
-   else if (!strcmp(tunnel, key))
-   server.tunnel = xstrdup(val);
-   else if (!strcmp(authmethod, key))
-   server.auth_method = xstrdup(val);
+   }
 
-   return 0;
+   git_config_get_string(imap.user, server.user);
+   git_config_get_string(imap.pass, server.pass);
+   git_config_get_int(imap.port, server.port);
+   git_config_get_string(imap.tunnel, server.tunnel);
+   git_config_get_string(imap.authmethod, server.auth_method);
 }
 
 int main(int argc, char **argv)
@@ -1383,7 +1375,7 @@ int main(int argc, char **argv)
usage(imap_send_usage);
 
setup_git_directory_gently(nongit_ok);
-   git_config(git_imap_config, NULL);
+   git_imap_config();
 
if (!server.port)
server.port = server.use_ssl ? 993 : 143;
-- 
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 v2 07/11] builtin/gc.c: replace `git_config()` with `git_config_get_*()` family

2014-08-07 Thread Tanay Abhra
Use `git_config_get_*()` family instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 builtin/gc.c | 51 ---
 1 file changed, 20 insertions(+), 31 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8d219d8..ced1456 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -55,44 +55,33 @@ static void remove_pidfile_on_signal(int signo)
raise(signo);
 }
 
-static int gc_config(const char *var, const char *value, void *cb)
+static void gc_config(void)
 {
-   if (!strcmp(var, gc.packrefs)) {
+   const char *value;
+
+   if (!git_config_get_value(gc.packrefs, value)) {
if (value  !strcmp(value, notbare))
pack_refs = -1;
else
-   pack_refs = git_config_bool(var, value);
-   return 0;
-   }
-   if (!strcmp(var, gc.aggressivewindow)) {
-   aggressive_window = git_config_int(var, value);
-   return 0;
-   }
-   if (!strcmp(var, gc.aggressivedepth)) {
-   aggressive_depth = git_config_int(var, value);
-   return 0;
-   }
-   if (!strcmp(var, gc.auto)) {
-   gc_auto_threshold = git_config_int(var, value);
-   return 0;
-   }
-   if (!strcmp(var, gc.autopacklimit)) {
-   gc_auto_pack_limit = git_config_int(var, value);
-   return 0;
+   pack_refs = git_config_bool(gc.packrefs, value);
}
-   if (!strcmp(var, gc.autodetach)) {
-   detach_auto = git_config_bool(var, value);
-   return 0;
-   }
-   if (!strcmp(var, gc.pruneexpire)) {
-   if (value  strcmp(value, now)) {
+
+   git_config_get_int(gc.aggressivewindow, aggressive_window);
+   git_config_get_int(gc.aggressivedepth, aggressive_depth);
+   git_config_get_int(gc.auto, gc_auto_threshold);
+   git_config_get_int(gc.autopacklimit, gc_auto_pack_limit);
+   git_config_get_bool(gc.autodetach, detach_auto);
+
+   if (!git_config_get_string_const(gc.pruneexpire, prune_expire)) {
+   if (strcmp(prune_expire, now)) {
unsigned long now = approxidate(now);
-   if (approxidate(value) = now)
-   return error(_(Invalid %s: '%s'), var, value);
+   if (approxidate(prune_expire) = now) {
+   git_die_config(gc.pruneexpire, _(Invalid 
gc.pruneexpire: '%s'),
+   prune_expire);
+   }
}
-   return git_config_string(prune_expire, var, value);
}
-   return git_default_config(var, value, cb);
+   git_config(git_default_config, NULL);
 }
 
 static int too_many_loose_objects(void)
@@ -301,7 +290,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
argv_array_pushl(prune, prune, --expire, NULL );
argv_array_pushl(rerere, rerere, gc, NULL);
 
-   git_config(gc_config, NULL);
+   gc_config();
 
if (pack_refs  0)
pack_refs = !is_bare_repository();
-- 
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 v2 11/11] branch.c: replace `git_config()` with `git_config_get_string()

2014-08-07 Thread Tanay Abhra
Use `git_config_get_string()` instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 branch.c | 27 +++
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/branch.c b/branch.c
index 735767d..df6b120 100644
--- a/branch.c
+++ b/branch.c
@@ -140,30 +140,17 @@ static int setup_tracking(const char *new_ref, const char 
*orig_ref,
return 0;
 }
 
-struct branch_desc_cb {
-   const char *config_name;
-   const char *value;
-};
-
-static int read_branch_desc_cb(const char *var, const char *value, void *cb)
-{
-   struct branch_desc_cb *desc = cb;
-   if (strcmp(desc-config_name, var))
-   return 0;
-   free((char *)desc-value);
-   return git_config_string(desc-value, var, value);
-}
-
 int read_branch_desc(struct strbuf *buf, const char *branch_name)
 {
-   struct branch_desc_cb cb;
+   char *v = NULL;
struct strbuf name = STRBUF_INIT;
strbuf_addf(name, branch.%s.description, branch_name);
-   cb.config_name = name.buf;
-   cb.value = NULL;
-   git_config(read_branch_desc_cb, cb);
-   if (cb.value)
-   strbuf_addstr(buf, cb.value);
+   if (git_config_get_string(name.buf, v)) {
+   strbuf_release(name);
+   return -1;
+   }
+   strbuf_addstr(buf, v);
+   free(v);
strbuf_release(name);
return 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


possibly a spurious conflict in a three way merge

2014-08-07 Thread meanlogin

git 2.0.4 on ubuntu 14.04 64

1. new repo
2. commit test.txt to master:
line1
line1

3. branch and checkout branch1
4. make and commit the following change to branch1:
#line1
#line2

5. checkout master
6. make and commit the following change to master:
line1
#line2

7. merge branch1, git sees a conflict:
 HEAD
line1
===
#line1
 branch1
#line2

Why?  The first line changed in branch1 but not in master so a 3 way 
merge should take branch1 changes.  Beyond Compare ( used as a 
mergetool) does not flag any conflicts.

--
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: Bug#757297: 'git status' output is confusing after 'git add -N'

2014-08-07 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Thu, Aug 7, 2014 at 7:34 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Package: git
 Version: 1:2.0.0-1
 Tags: upstream

   $ git init foo
   Initialized empty Git repository in /tmp/t/foo/.git/
   $ cd foo
   $ echo hi README
   $ git add -N README
   $ git status
   On branch master

   Initial commit

   Changes to be committed:
 (use git rm --cached file... to unstage)

   new file:   README

   Changes not staged for commit:
 (use git add file... to update what will be committed)
 (use git checkout -- file... to discard changes in working directory)

   modified:   README

 If I then run git commit, it does not actually commit the addition
 of the README file.

 We used to reject such a commit operation before 3f6d56d (commit:
 ignore intent-to-add entries instead of refusing - 2012-02-07) so it
 was harder to misunderstand this case.

 It would be clearer to have a separate section,like so:

   Tracked files not to be committed:
 (use git rm --cached file... to stop tracking)

new file:   README


 Or make the Changes not staged for commit part say new file:
 README (modified is implied)

Yeah, after reading the justification in the quoted commit, I agree
that it is status that is at fault in the above; new file: README
is part of Changes not staged for commit in this case (it is told
to the index, but the user never said it is for commit yet, which
is the whole point of -N), so instead of adding a new section, I
agree that it should be classified as new file not modified
there.



--
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] various contrib: Fix links in man pages

2014-08-07 Thread Junio C Hamano
Stefan Beller stefanbel...@gmail.com writes:

 Inspired by 2147fa7e (2014-07-31 git-push: fix link in man page),
 I grepped through the whole tree searching for 'gitlink:' occurrences.

 Signed-off-by: Stefan Beller stefanbel...@gmail.com
 ---

Thanks; I did the same earlier but only for Documentation/ which was
not very useful X-.



  contrib/convert-objects/git-convert-objects.txt | 2 +-
  contrib/examples/git-svnimport.txt  | 2 +-
  contrib/gitview/gitview.txt | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/contrib/convert-objects/git-convert-objects.txt 
 b/contrib/convert-objects/git-convert-objects.txt
 index 0565d83..f871880 100644
 --- a/contrib/convert-objects/git-convert-objects.txt
 +++ b/contrib/convert-objects/git-convert-objects.txt
 @@ -26,4 +26,4 @@ Documentation by David Greaves, Junio C Hamano and the 
 git-list git@vger.kernel
  
  GIT
  ---
 -Part of the gitlink:git[7] suite
 +Part of the linkgit:git[7] suite
 diff --git a/contrib/examples/git-svnimport.txt 
 b/contrib/examples/git-svnimport.txt
 index 3bb871e..3f0a9c3 100644
 --- a/contrib/examples/git-svnimport.txt
 +++ b/contrib/examples/git-svnimport.txt
 @@ -176,4 +176,4 @@ Documentation by Matthias Urlichs sm...@smurf.noris.de.
  
  GIT
  ---
 -Part of the gitlink:git[7] suite
 +Part of the linkgit:git[7] suite
 diff --git a/contrib/gitview/gitview.txt b/contrib/gitview/gitview.txt
 index 9e12f97..7b5f900 100644
 --- a/contrib/gitview/gitview.txt
 +++ b/contrib/gitview/gitview.txt
 @@ -28,7 +28,7 @@ OPTIONS
  
  args::
  
 - All the valid option for gitlink:git-rev-list[1].
 + All the valid option for linkgit:git-rev-list[1].
  
  Key Bindings
  
--
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: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Tony Finch
Nico Williams n...@cryptonector.com wrote:
 On Thu, Aug 07, 2014 at 12:38:48PM +0100, Tony Finch wrote:

  But [a rebasing workflow] is inconvenient for deploying the patched
  version to production (which is the point of developing the fixes) - I
  want a fast-forwarding branch for that.

 I'm not sure I follow this.  You deploy what you build, and you build
 the HEAD of the production branch, whatever that is.  If it gets
 rebased, so it it does.

The problem is that the production branch gets copied around: pushed to
the repo server, pulled by other team members, etc. Forced pushes
are accident-prone, as is resetting a rebased branch after a pull.

  So I have a small tool which maintains a publication branch which
  tracks the head of a rebasing branch. It's reasonably satisfactory so
  far...
 
  https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git

 Yeah, that's useful.

Glad you think so :-)

Tony.
-- 
f.anthony.n.finch  d...@dotat.at  http://dotat.at/
Thames: Northeast veering southeast 4 or 5, occasionally 6. Slight, becoming
slight or moderate. Fair then rain or thundery showers. Good, becoming
moderate or poor for a time.
--
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 11/11] branch.c: replace `git_config()` with `git_config_get_string()

2014-08-07 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 Use `git_config_get_string()` instead of `git_config()` to take advantage of
 the config-set API which provides a cleaner control flow.

 Signed-off-by: Tanay Abhra tanay...@gmail.com
 ---
  branch.c | 27 +++
  1 file changed, 7 insertions(+), 20 deletions(-)

 diff --git a/branch.c b/branch.c
 index 735767d..df6b120 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -140,30 +140,17 @@ static int setup_tracking(const char *new_ref, const 
 char *orig_ref,
   return 0;
  }
  
 -struct branch_desc_cb {
 - const char *config_name;
 - const char *value;
 -};
 -
 -static int read_branch_desc_cb(const char *var, const char *value, void *cb)
 -{
 - struct branch_desc_cb *desc = cb;
 - if (strcmp(desc-config_name, var))
 - return 0;
 - free((char *)desc-value);
 - return git_config_string(desc-value, var, value);
 -}
 -
  int read_branch_desc(struct strbuf *buf, const char *branch_name)
  {
 - struct branch_desc_cb cb;
 + char *v = NULL;
   struct strbuf name = STRBUF_INIT;
   strbuf_addf(name, branch.%s.description, branch_name);
 - cb.config_name = name.buf;
 - cb.value = NULL;
 - git_config(read_branch_desc_cb, cb);
 - if (cb.value)
 - strbuf_addstr(buf, cb.value);
 + if (git_config_get_string(name.buf, v)) {
 + strbuf_release(name);
 + return -1;
 + }
 + strbuf_addstr(buf, v);
 + free(v);

There's a behavior change here, but I think it is the right thing to do.

It lacks a proper commit message though:

As a reminder, your patch change `git_config()` return value to void
in the other series did:

--- 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);

So, before it, read_branch_desc() was returning -1 iff git_config()
failed, which essentially never happened.

Now, you're retoring a similar if, but you strbuf_release and return
-1 if no value is found for the variable.

There are 3 callers of read_branch_desc:

builtin/branch.c:   read_branch_desc(buf, branch_name);
builtin/fmt-merge-msg.c:if (!read_branch_desc(desc, name)) {
builtin/log.c:  read_branch_desc(desc, branch_name);

Only the one in fmt-merge-msg.c uses the return value:

static void add_branch_desc(struct strbuf *out, const char *name)
{
struct strbuf desc = STRBUF_INIT;

if (!read_branch_desc(desc, name)) {
const char *bp = desc.buf;
while (*bp) { /* (1) */
const char *ep = strchrnul(bp, '\n');
if (*ep)
ep++;
strbuf_addf(out,   : %.*s, (int)(ep - bp), bp);
bp = ep;
}
if (out-buf[out-len - 1] != '\n') /* (2) */
strbuf_addch(out, '\n');
}
strbuf_release(desc);
}

the (1) part is a no-op if no value is found, but the old code was still
adding a \n in the (2) part, even when no value was found.

So, the new code is better than the old one, but your patch does a bit
more than the commit message claims.

-- 
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] builtin/log.c: fix minor memory leak

2014-08-07 Thread Matthieu Moy
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
Valgrind confirms, one less unreachable block ;-).

 builtin/log.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/log.c b/builtin/log.c
index 4389722..e4d8122 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -857,20 +857,21 @@ static void add_branch_description(struct strbuf *buf, 
const char *branch_name)
 {
struct strbuf desc = STRBUF_INIT;
if (!branch_name || !*branch_name)
return;
read_branch_desc(desc, branch_name);
if (desc.len) {
strbuf_addch(buf, '\n');
strbuf_addbuf(buf, desc);
strbuf_addch(buf, '\n');
}
+   strbuf_release(desc);
 }
 
 static char *find_branch_name(struct rev_info *rev)
 {
int i, positive = -1;
unsigned char branch_sha1[20];
const unsigned char *tip_sha1;
const char *ref, *v;
char *full_ref, *branch = NULL;
 
-- 
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: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Nico Williams
On Thu, Aug 07, 2014 at 05:42:34PM +0100, Tony Finch wrote:
 Nico Williams n...@cryptonector.com wrote:
  On Thu, Aug 07, 2014 at 12:38:48PM +0100, Tony Finch wrote:
   But [a rebasing workflow] is inconvenient for deploying the patched
   version to production (which is the point of developing the fixes) - I
   want a fast-forwarding branch for that.
 
  I'm not sure I follow this.  You deploy what you build, and you build
  the HEAD of the production branch, whatever that is.  If it gets
  rebased, so it it does.
 
 The problem is that the production branch gets copied around: pushed to
 the repo server, pulled by other team members, etc. Forced pushes
 are accident-prone, as is resetting a rebased branch after a pull.

When I rebase and I need the old HEAD around I do something like this:

$ git checkout $branch_to_rebase
$ ver=${branch_to_rebase##*-}
$ git checkout -b ${branch_to_rebase%-${ver}}-$((ver+1))
$ git rebase ...

or like this:

$ git checkout $branch_to_rebase
$ git branch ${branch_to_rebase}-$(date +%Y-%m-%d)
$ git rebase ...

Either way I retain the old HEAD with some name.  This requires
discipline, so scripting it is useful.  But if you want discipline then
you want git to know that for this branch, don't prune/gc old HEADs
orphaned after rebases and push the rebase history for this branch.

   https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git
 
  Yeah, that's useful.
 
 Glad you think so :-)

Thank you.

Nico
-- 
--
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: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Tony Finch
Nico Williams n...@cryptonector.com wrote:
 On Thu, Aug 07, 2014 at 05:42:34PM +0100, Tony Finch wrote:
 
  The problem is that the production branch gets copied around: pushed to
  the repo server, pulled by other team members, etc. Forced pushes
  are accident-prone, as is resetting a rebased branch after a pull.

 When I rebase and I need the old HEAD around I do something like this:
 [...]
 Either way I retain the old HEAD with some name.

Hmm, yes, I can see that would work. However my previous workflow was
rather branch-heavy and I found the accumulation of names annoying. I have
not yet had enough usage out of git-repub to see if it goes too far in the
direction of lack-of-names. A big omission is no opportunity to edit its
commit messages.

Tony.
-- 
f.anthony.n.finch  d...@dotat.at  http://dotat.at/
Rockall: Southwesterly becoming cyclonic in north, 5 to 7. Moderate or rough.
Rain or showers. Moderate or 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 v3 11/11] branch.c: replace `git_config()` with `git_config_get_string()

2014-08-07 Thread Tanay Abhra
Use `git_config_get_string()` instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow. While we are at
it, return -1 if we find no value for the queried variable. Original code
returned 0 for all cases, which was checked by `add_branch_desc()` in
fmt-merge-msg.c resulting in addition of a spurious newline to the `out`
strbuf. Now, the newline addition is skipped as -1 is returned to the caller
if no value is found.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
v3: Changed the commit message to a more appropriate one.

 branch.c | 27 +++
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/branch.c b/branch.c
index 735767d..df6b120 100644
--- a/branch.c
+++ b/branch.c
@@ -140,30 +140,17 @@ static int setup_tracking(const char *new_ref, const char 
*orig_ref,
return 0;
 }

-struct branch_desc_cb {
-   const char *config_name;
-   const char *value;
-};
-
-static int read_branch_desc_cb(const char *var, const char *value, void *cb)
-{
-   struct branch_desc_cb *desc = cb;
-   if (strcmp(desc-config_name, var))
-   return 0;
-   free((char *)desc-value);
-   return git_config_string(desc-value, var, value);
-}
-
 int read_branch_desc(struct strbuf *buf, const char *branch_name)
 {
-   struct branch_desc_cb cb;
+   char *v = NULL;
struct strbuf name = STRBUF_INIT;
strbuf_addf(name, branch.%s.description, branch_name);
-   cb.config_name = name.buf;
-   cb.value = NULL;
-   git_config(read_branch_desc_cb, cb);
-   if (cb.value)
-   strbuf_addstr(buf, cb.value);
+   if (git_config_get_string(name.buf, v)) {
+   strbuf_release(name);
+   return -1;
+   }
+   strbuf_addstr(buf, v);
+   free(v);
strbuf_release(name);
return 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] builtin/log.c: fix minor memory leak

2014-08-07 Thread Jonathan Nieder
Matthieu Moy wrote:

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 Valgrind confirms, one less unreachable block ;-).

This belongs in the commit message.

[...]
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -857,20 +857,21 @@ static void add_branch_description(struct strbuf *buf, 
 const char *branch_name)
  {
   struct strbuf desc = STRBUF_INIT;
   if (!branch_name || !*branch_name)
   return;
   read_branch_desc(desc, branch_name);
   if (desc.len) {
   strbuf_addch(buf, '\n');
   strbuf_addbuf(buf, desc);
   strbuf_addch(buf, '\n');
   }
 + strbuf_release(desc);

This is an old one.  The leak was introduced by v1.7.9-rc1~1^2~12
(format-patch: use branch description in cover letter, 2011-09-21).

I was a little scared to see a leak in 'git log' code, since most of
what log does involves looping over many commits.  Luckily this one is
only used in make_cover_letter to create a cover letter describing the
single branch on the command line, making it is a small, one-time
leak.

Less noise from static and dynamic analysis tools is still worthwhile,
so for what it's worth,

Reviewed-by: Jonathan Nieder jrnie...@gmail.com

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 v2 00/11] git_config callers rewritten with the new config-set API

2014-08-07 Thread Matthieu Moy
Tanay Abhra tanay...@gmail.com writes:

 [v2]: git_die_config() messages changed. Diff between v1 and v2 is at the 
 bottom.

I went through the series once more, and all the changes look good.

v3 for PATCH 11/11 addresses my comment about the commit message in v2.

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: possibly a spurious conflict in a three way merge

2014-08-07 Thread Jonathan Nieder
Hi,

meanlo...@gmail.com wrote:

 2. commit test.txt to master:
 line1
 line1

 3. branch and checkout branch1
 4. make and commit the following change to branch1:
 #line1
 #line2

 5. checkout master
 6. make and commit the following change to master:
 line1
 #line2

 7. merge branch1, git sees a conflict:
  HEAD
 line1
 ===
 #line1
  branch1
 #line2

 Why?

Thanks for a clear example.  On branch1 you made the following change:

 (a) modify lines 1 and 2

On master, you made a different change:

 (b) just modify line 2

The changes touch the same chunk of lines and don't produce the same
result there.  Git is being conservative and letting you know that the
two branches didn't agree about what line 1 should say.

On the other hand, if you had a larger files and on branch1 made the
following change:

 (a) modify lines 1 and 101

and on master, you made a different change:

 (b) just modify line 101

then the changes are touching different parts of the code and are
merged independently.  The result would be a clean merge where lines 1
and 101 are both modified.

Git's conservatism can be helpful when working with code, where
adjacent lines are likely to be affecting a single behavior and the
conflict can help the operator to know to be extra careful.  It makes
less sense for line-oriented input where every line is independent.

If you are often making changes to a line-oriented file, it may make
sense to set up a custom merge driver to override git's merge behavior
for that file.  See 'Defining a custom merge driver' in
gitattributes(5) for details about how that works.

Hope that helps,
Jonathan
--
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 v9 5/8] config: add `git_die_config()` to the config-set API

2014-08-07 Thread Junio C Hamano
Tanay Abhra tanay...@gmail.com writes:

 diff --git a/Documentation/technical/api-config.txt 
 b/Documentation/technical/api-config.txt
 index 21f280c..0d8b99b 100644
 --- a/Documentation/technical/api-config.txt
 +++ b/Documentation/technical/api-config.txt
 @@ -155,6 +155,19 @@ 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.
  
 +`git_die_config(const char *key, const char *err, ...)`::
 +
 + First prints the error message specified by the caller in `err` and then
 + dies printing the line number and the file name of the highest priority
 + value for the configuration variable `key`.

Reviewed with a wider context, I notice that this entry alone lacks
the return type.  I am assuming that this is just an oversight, and
adding 'void ' in front of the filename to match the next entry is
simple enough.

 +`void git_die_config_linenr(const char *key, const char *filename, int 
 linenr)`::
 + ...
 +extern NORETURN void git_die_config(const char *key, const char *err, ...) 
 __attribute__((format(printf, 2, 3)));
 ...
 +NORETURN __attribute__((format(printf, 2, 3)))
 +void git_die_config(const char *key, const char *err, ...)
 +{

My first reaction was that it might make the compiler unhappy to
declare that the err is a printf-like format string and then to
allow some callers to pass NULL to the function.  My build however
does not seem to complain, so perhaps this is OK.

 + const struct string_list *values;
 + struct key_value_info *kv_info;
 +
 + if (err) {
 + va_list params;
 + va_start(params, err);
 + vreportf(error: , err, params);
 + va_end(params);
 + }
 + values = git_config_get_value_multi(key);
 + kv_info = values-items[values-nr - 1].util;
 + git_die_config_linenr(key, kv_info-filename, kv_info-linenr);
  }
  
  /*
--
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 v8 0/8] Rewrite `git_config()` using config-set API

2014-08-07 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Tanay Abhra tanay...@gmail.com writes:

 --- a/cache.h
 +++ b/cache.h
 @@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int 
 *dest);
 [...]
 +struct key_value_info {
 +const char *filename;
 +int linenr;
 +};
 [...]
 diff --git a/config.c b/config.c
 index cf9124f..427850a 100644
 --- a/config.c
 +++ b/config.c
 @@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void 
 *data,
  return ret;
  }
  
 -struct key_value_info {
 -const char *filename;
 -int linenr;
 -};
 -

 Why is this needed? Are you now using key_value_info outside config.c?
 Or is it a leftover from a previous experiment?

Has this been resolved in the new round?
--
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] builtin/log.c: fix minor memory leak

2014-08-07 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Matthieu Moy wrote:

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 Valgrind confirms, one less unreachable block ;-).

 This belongs in the commit message.

 [...]
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -857,20 +857,21 @@ static void add_branch_description(struct strbuf *buf, 
 const char *branch_name)
  {
  struct strbuf desc = STRBUF_INIT;
  if (!branch_name || !*branch_name)
  return;
  read_branch_desc(desc, branch_name);
  if (desc.len) {
  strbuf_addch(buf, '\n');
  strbuf_addbuf(buf, desc);
  strbuf_addch(buf, '\n');
  }
 +strbuf_release(desc);

 This is an old one.  The leak was introduced by v1.7.9-rc1~1^2~12
 (format-patch: use branch description in cover letter, 2011-09-21).

 I was a little scared to see a leak in 'git log' code, since most of
 what log does involves looping over many commits.  Luckily this one is
 only used in make_cover_letter to create a cover letter describing the
 single branch on the command line, making it is a small, one-time
 leak.

Exactly ;-).


 Less noise from static and dynamic analysis tools is still worthwhile,
 so for what it's worth,

 Reviewed-by: Jonathan Nieder jrnie...@gmail.com

 Thanks.

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 v9 3/8] add line number and file name info to `config_set`

2014-08-07 Thread Ramsay Jones
On 07/08/14 12:59, Tanay Abhra wrote:
 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
 ---
  cache.h  |  5 +
  config.c | 16 ++--
  2 files changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/cache.h b/cache.h
 index 7292aef..0b1bdfd 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -1383,6 +1383,11 @@ 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);
  
 +struct key_value_info {
 + const char *filename;
 + int linenr;
 +};
 +

I haven't checked, but does this series now include a user for
this struct outside of config.c? If not, then I think it would
be better to leave the declaration in config.c until it is needed.
(To make it easier to see if it is necessary in the context of the
patch which will make use of it).

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 v8 0/8] Rewrite `git_config()` using config-set API

2014-08-07 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Tanay Abhra tanay...@gmail.com writes:

 --- a/cache.h
 +++ b/cache.h
 @@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int 
 *dest);
 [...]
 +struct key_value_info {
 +   const char *filename;
 +   int linenr;
 +};
 [...]
 diff --git a/config.c b/config.c
 index cf9124f..427850a 100644
 --- a/config.c
 +++ b/config.c
 @@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void 
 *data,
 return ret;
  }
  
 -struct key_value_info {
 -   const char *filename;
 -   int linenr;
 -};
 -

 Why is this needed? Are you now using key_value_info outside config.c?
 Or is it a leftover from a previous experiment?

 Has this been resolved in the new round?

Tanay explained in another subthread why this was needed. For callers
iterating over the string_list who want to get the file/line info, they
need to be able to cast the void * pointer to struct key_value_info *.

-- 
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 v9 3/8] add line number and file name info to `config_set`

2014-08-07 Thread Matthieu Moy
Ramsay Jones ram...@ramsay1.demon.co.uk writes:

 On 07/08/14 12:59, Tanay Abhra wrote:
 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
 ---
  cache.h  |  5 +
  config.c | 16 ++--
  2 files changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/cache.h b/cache.h
 index 7292aef..0b1bdfd 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -1383,6 +1383,11 @@ 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);
  
 +struct key_value_info {
 +const char *filename;
 +int linenr;
 +};
 +

 I haven't checked, but does this series now include a user for
 this struct outside of config.c? If not, then I think it would
 be better to leave the declaration in config.c until it is needed.
 (To make it easier to see if it is necessary in the context of the
 patch which will make use of it).

I disagree: this patch series is essentially about introducing a new
API, and this struct declaration is part of the API.

It seemed strange to me to see the code movement in the patch from two
versions of the series, but the patch itself does not move the code, it
just adds new code directly where it belongs.

-- 
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 v9 3/8] add line number and file name info to `config_set`

2014-08-07 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramsay Jones ram...@ramsay1.demon.co.uk writes:
 ...
 diff --git a/cache.h b/cache.h
 ...
 +struct key_value_info {
 +   const char *filename;
 +   int linenr;
 +};
 +

 I haven't checked, but does this series now include a user for
 this struct outside of config.c? If not, then I think it would
 be better to leave the declaration in config.c until it is needed.
 (To make it easier to see if it is necessary in the context of the
 patch which will make use of it).

 I disagree: this patch series is essentially about introducing a new
 API, and this struct declaration is part of the API.

Hmm, is it?  How would the customer of the API use it?  die_config
and friends may internally use the information recorded using the
structure, but I had an impression that it is an implementation
detail that does not need to be exposed to the customers of the API.
Am I mistaken?

--
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 00/11] git_config callers rewritten with the new config-set API

2014-08-07 Thread Junio C Hamano
Tanay Abhra tanay...@gmail.com writes:

  11 files changed, 114 insertions(+), 250 deletions(-)

Nice reduction.
--
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 v9 3/8] add line number and file name info to `config_set`

2014-08-07 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramsay Jones ram...@ramsay1.demon.co.uk writes:
 ...
 diff --git a/cache.h b/cache.h
 ...
 +struct key_value_info {
 +  const char *filename;
 +  int linenr;
 +};
 +

 I haven't checked, but does this series now include a user for
 this struct outside of config.c? If not, then I think it would
 be better to leave the declaration in config.c until it is needed.
 (To make it easier to see if it is necessary in the context of the
 patch which will make use of it).

 I disagree: this patch series is essentially about introducing a new
 API, and this struct declaration is part of the API.

 Hmm, is it?  How would the customer of the API use it?  die_config
 and friends may internally use the information recorded using the
 structure, but I had an impression that it is an implementation
 detail that does not need to be exposed to the customers of the API.
 Am I mistaken?

It does if you want to provide error message while iterating over the
string_list. Not the common case, but shouldn't be forbidden either.

-- 
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 v8 0/8] Rewrite `git_config()` using config-set API

2014-08-07 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Why is this needed? Are you now using key_value_info outside config.c?
 Or is it a leftover from a previous experiment?

 Has this been resolved in the new round?

 Tanay explained in another subthread why this was needed. For callers
 iterating over the string_list who want to get the file/line info, they
 need to be able to cast the void * pointer to struct key_value_info *.

For callers that want to see all the multi-values, it would be
preferrable for the iterator to pass the filename and the linenumber
to the callback function, instead of exposing its implementation
detail as a single string list and telling them to pick it apart,
no?

Not a very convincing argument, but OK for now in the sense that we
can fix it later if we wanted to before it gets too late.
--
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] stash: default listing to working-tree diff

2014-08-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 See the patch below, which I think could replace the top three from
 jk/stash-list-p (or really, could replace the whole series, and the
 bottom three could go into their own topic).

Sounds sensible.  Let's split those three changes into a separate
topic (jk/pretty-empty-format) and queue this independently.

Thanks.

 -- 8 --
 Subject: stash: default listing to working-tree diff

 When you list stashes, you can provide arbitrary git-log
 options to change the display. However, adding just -p
 does nothing, because each stash is actually a merge commit.

 This implementation detail is easy to forget, leading to
 confused users who think -p is not working. We can make
 this easier by defaulting to --first-parent -m, which will
 show the diff against the working tree. This omits the
 index portion of the stash entirely, but it's simple and it
 matches what git stash show provides.

 People who are more clueful about stash's true form can use
 --cc to override the -m, and the --first-parent will
 then do nothing. For diffs, it only affects non-combined
 diffs, so --cc overrides it. And for the traversal, we are
 walking the linear reflog anyway, so we do not even care
 about the parents.

 Signed-off-by: Jeff King p...@peff.net
 ---
  git-stash.sh |  2 +-
  t/t3903-stash.sh | 42 ++
  2 files changed, 43 insertions(+), 1 deletion(-)

 diff --git a/git-stash.sh b/git-stash.sh
 index bcc757b..9c1ba8e 100755
 --- a/git-stash.sh
 +++ b/git-stash.sh
 @@ -297,7 +297,7 @@ have_stash () {
  
  list_stash () {
   have_stash || return 0
 - git log --format=%gd: %gs -g $@ $ref_stash --
 + git log --format=%gd: %gs -g --first-parent -m $@ $ref_stash --
  }
  
  show_stash () {
 diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
 index 5b79b21..1e29962 100755
 --- a/t/t3903-stash.sh
 +++ b/t/t3903-stash.sh
 @@ -685,4 +685,46 @@ test_expect_success 'handle stash specification with 
 spaces' '
   grep pig file
  '
  
 +test_expect_success 'setup stash with index and worktree changes' '
 + git stash clear 
 + git reset --hard 
 + echo index file 
 + git add file 
 + echo working file 
 + git stash
 +'
 +
 +test_expect_success 'stash list implies --first-parent -m' '
 + cat expect -\EOF 
 + stash@{0}: WIP on master: b27a2bc subdir
 +
 + diff --git a/file b/file
 + index 257cc56..d26b33d 100644
 + --- a/file
 + +++ b/file
 + @@ -1 +1 @@
 + -foo
 + +working
 + EOF
 + git stash list -p actual 
 + test_cmp expect actual
 +'
 +
 +test_expect_success 'stash list --cc shows combined diff' '
 + cat expect -\EOF 
 + stash@{0}: WIP on master: b27a2bc subdir
 +
 + diff --cc file
 + index 257cc56,9015a7a..d26b33d
 + --- a/file
 + +++ b/file
 + @@@ -1,1 -1,1 +1,1 @@@
 + - foo
 +  -index
 + ++working
 + EOF
 + git stash list -p --cc actual 
 + test_cmp expect actual
 +'
 +
  test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints

2014-08-07 Thread Eric Sunshine
On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch baf...@gmail.com wrote:
 The to-do list commands `squash` and `fixup` apply the changes
 introduced by the named commit to the tree but instead of creating
 a new commit on top of the current head it replaces the previous
 commit with a new commit that records the updated tree. If the
 result is an empty commit git-rebase stops with the error message

You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.

 This message is not very helpful because neither does git-rebase
 support an option `--allow-empty` nor does the messages say how to
 resume the rebase. Firstly, change the error message to

The squash result is empty and --keep-empty was not specified.

You can remove the squash commit now with

  git reset HEAD^

Once you are down, run

I guess you meant: s/down/done

Same issue with the actually message in the code (below).

  git rebase --continue

 If the user wishes to squash a sequence of commits into one
 commit, f. i.

pick A
squash Revert A
squash A'

 , it does not matter for the end result that the first squash
 result, or any sub-sequence in general, is going to be empty. The
 squash message is not affected at all by which commits are created
 and only the commit created by the last line in the sequence will
 end up in the final history. Secondly, print the error message
 only if the whole squash sequence produced an empty commit.

 Lastly, since an empty squash commit is not a failure to rewrite
 the history as planned, issue the message above as a mere warning
 and interrupt the rebase with the return value zero. The
 interruption should be considered as a notification with the
 chance to undo it on the spot. Specifying the `--keep-empty`
 option tells git-rebase to keep empty squash commits in the
 rebased history without notification.

 Add tests.

 Reported-by: Peter Krefting pe...@softwolves.pp.se
 Signed-off-by: Fabian Ruch baf...@gmail.com
 ---
 Hi,

 Peter Krefting is cc'd as the author of the bug report Confusing
 error message in rebase when commit becomes empty discussed on the
 mailing list in June. Phil Hord and Jeff King both participated in
 the problem discussion which ended with two proposals by Jeff.

 Jeff King writes:
   1. Always keep such empty commits. A user who is surprised by them
  being empty can then revisit them. Or drop them by doing another
  rebase without --keep-empty.

   2. Notice ourselves that the end-result of the whole squash is an
  empty commit, and stop to let the user deal with it.

 This patch chooses the second alternative. Either way seems OK. The
 crucial consensus of the discussion was to silently throw away empty
 interim commits.

Fabian

  git-rebase--interactive.sh| 20 +++---
  t/t3404-rebase-interactive.sh | 62 
 +++
  2 files changed, 79 insertions(+), 3 deletions(-)

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index 3222bf6..8820eac 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -549,7 +549,7 @@ do_next () {
 squash|s|fixup|f)
 # This is an intermediate commit; its message will 
 only be
 # used in case of trouble.  So use the long version:
 -   do_with_author output git commit 
 --allow-empty-message \
 +   do_with_author output git commit 
 --allow-empty-message --allow-empty \
 --amend --no-verify -F $squash_msg \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1 $rest
 @@ -558,18 +558,32 @@ do_next () {
 # This is the final command of this squash/fixup group
 if test -f $fixup_msg
 then
 -   do_with_author git commit 
 --allow-empty-message \
 +   do_with_author git commit 
 --allow-empty-message --allow-empty \
 --amend --no-verify -F $fixup_msg \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1 $rest
 else
 cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
 rm -f $GIT_DIR/MERGE_MSG
 -   do_with_author git commit --amend --no-verify 
 -F $GIT_DIR/SQUASH_MSG -e \
 +   do_with_author git commit --allow-empty 
 --amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1 

Re: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Nico Williams
On Thu, Aug 7, 2014 at 12:47 PM, Tony Finch d...@dotat.at wrote:
 Nico Williams n...@cryptonector.com wrote:
 Either way I retain the old HEAD with some name.

 Hmm, yes, I can see that would work. However my previous workflow was
 rather branch-heavy and I found the accumulation of names annoying. I have
 not yet had enough usage out of git-repub to see if it goes too far in the
 direction of lack-of-names. A big omission is no opportunity to edit its
 commit messages.

Oh, I just read your script more carefully and looked at your example
history again.  You're using parent metadata in the commits to keep
the history alive without the extra names, correct?  *That* is
_clever_.  Hats off.  I may have to steal this script :)

Nico
--
--
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: Branch objects (was: Re: cherry picking and merge)

2014-08-07 Thread Nico Williams
On Thu, Aug 7, 2014 at 6:38 AM, Tony Finch d...@dotat.at wrote:
 So I have a small tool which maintains a publication branch which tracks
 the head of a rebasing branch. It's reasonably satisfactory so far...

 https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git

 ... though the structure of the publication branch is weird and not very
 easy to navigate. You can see it in action in my git.git repo:

You know, maybe you could even use this to automatically figure out
the merge base for downstreams that follow your rebased branch:
auto-generate the git rebase --onto head head as it was prior to
all the upstream rebases.

That would be awesome, particularly if integrated into git.  It would
then be fine to rebase published branches in most cases, for example.

Nico
--
--
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


Merging semi-parallel universes into one coherent repository

2014-08-07 Thread Ryan Gaffney
Hello

I created a post on SO about this; pasting it here for convenience:

= = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =
http://stackoverflow.com/questions/25193518/fixing-branches-disconnected-from-master

After completing a repository migration, we noticed that all of our
branches are all behind and ahead of master by hundreds to thousands
of commits.

Some context -- originally I used git-svn to migrate the repositories
from SVN to git. Later down the line I used this solution to merge
multiple (7) repositories into one.

However, I also improvised on that solution for the repositories'
branches. The result was disconnected branches -- branches that have
all of the commits and proper change history, but are not actually
associated with master in any way since the commits in the branches
have different SHA's from those in the new, shared master.

Additionally, the original branching in the separate repositories
happened at different revisions, for example project X might have
branched for release 1.0 at revision 30 whereas project Y might have
branched fro release 1.0 at revision 42... and all of them have
different times (and perhaps multiple times) where they merged back to
master.

Is there any clean way to fix this link to make it look like all of
these repositories branched together? And to fix the commits so they
actually properly map to commits in master?
= = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = =

I am sure there is a better way to word the above... but basically I
have seven used-to-be-separate repositories with the same branches
(say r1.0 as above).  After some magic with filter-branch, they are
all one repository -- all history in-tact.  Additionally, same-named
branches had their histories mashed together.  That's all peachy.

What's *not* peachy is that each of these branches shares commits with
master and doesn't know it.  Thus each branch looks disconnected
from master, e.g. there is no pretty branch graph when you look
through the history with git log --pretty=format:%h %s --graph.

Does anybody know of a way to rectify this issue?

Let me know if any clarifications are needed.  Help is much appreciated...

-- Ryan
--
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] Resending - Updated Bulgarian translation of git-gui

2014-08-07 Thread Alexander Shopov
This is the updated Bulgarian translation og git-gui. It has been
synced with git  gitk.

I have just checked the archives of the mailing list - the patch was
never sent, just the previous version of the cover letter. Resending
so that it can be merged for 2.1.

Kind regards:
al_shopov

Alexander Shopov (1):
  git-gui i18n: Updated Bulgarian translation (520t,0f,0u)

 po/bg.po | 3572 +++---
 1 file changed, 1781 insertions(+), 1791 deletions(-)

-- 
1.9.3

--
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