Re: [PATCH 0/3] Re: [PATCH] unpack-tree.c: remove dead code

2014-08-13 Thread Stefan Beller
On 13.08.2014 01:57, Jonathan Nieder wrote:
 Stefan Beller wrote:
 
 In line 1763 of unpack-tree.c we have a condition on the current tree
 [...]
 
 The description is describing why the patch is *correct* (i.e., not
 going to introduce a bug), while what the reader wants to know is why
 the change is *desirable*.

Indeed. Thanks for the reminder!

 
 Is this about making the code more readable, or robust, or suppressing
 a static analysis error, or something else?  What did the user or
 reader want to do that they couldn't do before and now can after this
 patch?

In my opinion it's making the code easier to read as there are less
lines of code with less conditionals.
The supression of a static code analysis warning is rather a desired
side effect, but not the main reason for the patch.


 
 [...]
 --- a/unpack-trees.c
 +++ b/unpack-trees.c
 @@ -1789,15 +1789,11 @@ int twoway_merge(const struct cache_entry * const 
 *src,
  /* 20 or 21 */
  return merged_entry(newtree, current, o);
  }
 +else if (o-gently) {
 +return  -1 ;
 +}
 
 (not about this patch) Elsewhere git uses the 'cuddled else':

Yes, I intentionally used this style, as the surrounding code was
using this style. You already added the reformatting follow up patch,
thanks!

 
   if (foo) {
   ...
   } else if (bar) {
   ...
   } else {
   ...
   }
 
 That stylefix would be a topic for a different patch, though.
 
  else {
 -/* all other failures */
 -if (oldtree)
 -return o-gently ? -1 : reject_merge(oldtree, 
 o);
 -if (current)
 -return o-gently ? -1 : reject_merge(current, 
 o);
 -if (newtree)
 -return o-gently ? -1 : reject_merge(newtree, 
 o);
 -return -1;
 
 Does the static analysis tool support comments like
 
   if (oldtree)
   ...
   if (current)
   ...
   ...
 
   /* not reached */
   return -1;
 
 ?  That might be the simplest minimally invasive fix for what coverity
 pointed out.

I was looking for things like that, but either the
extensive documentation is well hidden or there is only short
tutorial-like documentation, which doesn't cover this case.


 
 Now that we're looking there, though, it's worth understanding why we
 do the 'if oldtree exists, use it, else fall back to, etc' thing.  Was
 this meant as futureproofing in case commands like 'git checkout' want
 to do rename detection some day?
 
 Everywhere else in the file that reject_merge is used, it is as
 
   return o-gently ? -1 : reject_merge(..., o);
 
 The one exception is
 
   !current 
   oldtree 
   newtree 
   oldtree != newtree 
   !initial_checkout
 
 (#17), which seems like a bug (it should have the same check).  Would
 it make sense to inline the o-gently check into reject_merge so callers
 don't have to care?
 
 In that spirit, I suspect the simplest fix would be
 
   else
   return o-gently ? -1 : reject_merge(current, o);
 
 and then all calls could be replaced in a followup patch.
 
 Sensible?

I need to read more code to follow.

Thanks for picking up my inital patch and improving. :)
Stefan

 
 Thanks,
 
 Jonathan Nieder (2):
   unpack-trees: use 'cuddled' style for if-else cascade
   checkout -m: attempt merge when deletion of path was staged
 
 Stefan Beller (1):
   unpack-trees: simplify 'all other failures' case
 
  unpack-trees.c | 31 ++-
  1 file changed, 10 insertions(+), 21 deletions(-)
 

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] merge-recursive.c: replace `git_config()` with `git_config_get_int()`

2014-08-13 Thread Tanay Abhra
Use `git_config_get_int()` 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
---
 merge-recursive.c | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 1d332b8..8ab944c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2026,22 +2026,12 @@ int merge_recursive_generic(struct merge_options *o,
return clean ? 0 : 1;
 }
 
-static int merge_recursive_config(const char *var, const char *value, void *cb)
+static void merge_recursive_config(struct merge_options *o)
 {
-   struct merge_options *o = cb;
-   if (!strcmp(var, merge.verbosity)) {
-   o-verbosity = git_config_int(var, value);
-   return 0;
-   }
-   if (!strcmp(var, diff.renamelimit)) {
-   o-diff_rename_limit = git_config_int(var, value);
-   return 0;
-   }
-   if (!strcmp(var, merge.renamelimit)) {
-   o-merge_rename_limit = git_config_int(var, value);
-   return 0;
-   }
-   return git_xmerge_config(var, value, cb);
+   git_config_get_int(merge.verbosity, o-verbosity);
+   git_config_get_int(diff.renamelimit, o-diff_rename_limit);
+   git_config_get_int(merge.renamelimit, o-merge_rename_limit);
+   git_config(git_xmerge_config, NULL);
 }
 
 void init_merge_options(struct merge_options *o)
@@ -2052,7 +2042,7 @@ void init_merge_options(struct merge_options *o)
o-diff_rename_limit = -1;
o-merge_rename_limit = -1;
o-renormalize = 0;
-   git_config(merge_recursive_config, o);
+   merge_recursive_config(o);
if (getenv(GIT_MERGE_VERBOSITY))
o-verbosity =
strtol(getenv(GIT_MERGE_VERBOSITY), NULL, 10);
-- 
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 1/4] fast-import.c: replace `git_config()` with `git_config_get_*()` family

2014-08-13 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
---
 fast-import.c | 42 +++---
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index d73f58c..eca5ed4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3274,36 +3274,32 @@ static void parse_option(const char *option)
die(This version of fast-import does not support option: %s, option);
 }
 
-static int git_pack_config(const char *k, const char *v, void *cb)
+static void git_pack_config(void)
 {
-   if (!strcmp(k, pack.depth)) {
-   max_depth = git_config_int(k, v);
+   int indexversion_value;
+   unsigned long packsizelimit_value;
+
+   if (!git_config_get_ulong(pack.depth, max_depth)) {
if (max_depth  MAX_DEPTH)
max_depth = MAX_DEPTH;
-   return 0;
}
-   if (!strcmp(k, pack.compression)) {
-   int level = git_config_int(k, v);
-   if (level == -1)
-   level = Z_DEFAULT_COMPRESSION;
-   else if (level  0 || level  Z_BEST_COMPRESSION)
-   die(bad pack compression level %d, level);
-   pack_compression_level = level;
+   if (!git_config_get_int(pack.compression, pack_compression_level)) {
+   if (pack_compression_level == -1)
+   pack_compression_level = Z_DEFAULT_COMPRESSION;
+   else if (pack_compression_level  0 ||
+pack_compression_level  Z_BEST_COMPRESSION)
+   die(bad pack compression level %d, 
pack_compression_level);
pack_compression_seen = 1;
-   return 0;
}
-   if (!strcmp(k, pack.indexversion)) {
-   pack_idx_opts.version = git_config_int(k, v);
+   if (!git_config_get_int(pack.indexversion, indexversion_value)) {
+   pack_idx_opts.version = indexversion_value;
if (pack_idx_opts.version  2)
-   die(bad pack.indexversion=%PRIu32,
-   pack_idx_opts.version);
-   return 0;
+   die(bad pack.indexversion=%PRIu32, 
pack_idx_opts.version);
}
-   if (!strcmp(k, pack.packsizelimit)) {
-   max_packsize = git_config_ulong(k, v);
-   return 0;
-   }
-   return git_default_config(k, v, cb);
+   if (!git_config_get_ulong(pack.packsizelimit, packsizelimit_value))
+   max_packsize = packsizelimit_value;
+
+   git_config(git_default_config, NULL);
 }
 
 static const char fast_import_usage[] =
@@ -3356,7 +3352,7 @@ int main(int argc, char **argv)
 
setup_git_directory();
reset_pack_idx_option(pack_idx_opts);
-   git_config(git_pack_config, NULL);
+   git_pack_config();
if (!pack_compression_seen  core_compression_seen)
pack_compression_level = core_compression_level;
 
-- 
1.9.0.GIT

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] ll-merge.c: refactor `read_merge_config()` to use `git_config_string()`

2014-08-13 Thread Tanay Abhra
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 ll-merge.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/ll-merge.c b/ll-merge.c
index fb61ea6..8ea03e5 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -225,11 +225,8 @@ static int read_merge_config(const char *var, const char 
*value, void *cb)
const char *key, *name;
int namelen;
 
-   if (!strcmp(var, merge.default)) {
-   if (value)
-   default_ll_merge = xstrdup(value);
-   return 0;
-   }
+   if (!strcmp(var, merge.default))
+   return git_config_string(default_ll_merge, var, value);
 
/*
 * We are not interested in anything but merge.name.variable;
@@ -254,12 +251,8 @@ static int read_merge_config(const char *var, const char 
*value, void *cb)
ll_user_merge_tail = (fn-next);
}
 
-   if (!strcmp(name, key)) {
-   if (!value)
-   return error(%s: lacks value, var);
-   fn-description = xstrdup(value);
-   return 0;
-   }
+   if (!strcmp(name, key))
+   return git_config_string(fn-description, var, value);
 
if (!strcmp(driver, key)) {
if (!value)
@@ -285,12 +278,8 @@ static int read_merge_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
-   if (!strcmp(recursive, key)) {
-   if (!value)
-   return error(%s: lacks value, var);
-   fn-recursive = xstrdup(value);
-   return 0;
-   }
+   if (!strcmp(recursive, key))
+   return git_config_string(fn-recursive, var, value);
 
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 4/4] builtin/apply.c: replace `git_config()` with `git_config_get_string_const()`

2014-08-13 Thread Tanay Abhra
Use `git_config_get_string_const()` 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/apply.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index be2b4ce..66acf32 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4269,13 +4269,11 @@ static int apply_patch(int fd, const char *filename, 
int options)
return 0;
 }
 
-static int git_apply_config(const char *var, const char *value, void *cb)
+static void git_apply_config(void)
 {
-   if (!strcmp(var, apply.whitespace))
-   return git_config_string(apply_default_whitespace, var, value);
-   else if (!strcmp(var, apply.ignorewhitespace))
-   return git_config_string(apply_default_ignorewhitespace, var, 
value);
-   return git_default_config(var, value, cb);
+   git_config_get_string_const(apply.whitespace, 
apply_default_whitespace);
+   git_config_get_string_const(apply.ignorewhitespace, 
apply_default_ignorewhitespace);
+   git_config(git_default_config, NULL);
 }
 
 static int option_parse_exclude(const struct option *opt,
@@ -4423,7 +4421,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
 
prefix = prefix_;
prefix_length = prefix ? strlen(prefix) : 0;
-   git_config(git_apply_config, NULL);
+   git_apply_config();
if (apply_default_whitespace)
parse_whitespace_option(apply_default_whitespace);
if (apply_default_ignorewhitespace)
-- 
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


Undefined reference to __builtin_ctzll

2014-08-13 Thread Радослав Йовчев
Dear GIT community,


I found myself in situation where I had to install GIT on Debian 3.1
sarge.  It comes with GCC 3.3.5. I tried to built from source but the
libgcc was not providing the ctzll function, thus I decided to put an
implementation.


I do not know how to post and do a nice patch (and whether somebody
will care), but I guess, for reference I can post my solution. Just
appended in compat/strlcpy.c the following:


int __builtin_ctzll (long long x)
{
int i;
for (i = 0; i  8 * sizeof (long long); ++i)
if (x  ((long long) 1   i))
break;
return i;
}


I guess that some ifdef macro can be used to detect compiler version
or missing __builtin_ctzll.


With best regards and hopes that this can help somebody in similar situation,
Radoslav.
--
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] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

2014-08-13 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Junio C Hamano gits...@pobox.com writes:

 So I think the reasoning (i.e. is a descendant is not quite right)
 is correct, but the updated text is not quite right.  Changing it
 further to only the committer timestamps and identities would
 change is probably not an improvement, either.  Force the rebase
 that would otherwise be a no-op may be a better phrasing that does
 not risk going stale even if we update what are preserved and what
 are modified in the future.

 Also I notice the sentence Normally non-interactive...in such a
 situation is not helping the reader in this description very much.
 I wonder if we should keep it if we are rewriting this paragraph.

 How about doing it this way, perhaps?

[...]

  -f::
  --force-rebase::
 - Force the rebase even if the current branch is a descendant
 - of the commit you are rebasing onto.  Normally non-interactive rebase 
 will
 - exit with the message Current branch is up to date in such a
 - situation.
 - Incompatible with the --interactive option.
 + Force a rebase even if the current branch is up-to-date and
 + the command without `--force` would return without doing anything.
  +
  You may find this (or --no-ff with an interactive rebase) helpful after
  reverting a topic branch merge, as this option recreates the topic branch 
 with

It's OK with me, as in fact I think that there is no good explanation
for current git behavior, and thus it's git behavior that should have
been changed, not the documentation. I.e., git must not rebase anything
when Current branch is a descendant of the commit you are rebasing
onto, unless -f is given. Simple, reasonable, straightforward.

The version you propose at least does not lie, so it's definiteley an
improvement. However,

Force the rebase when the command exits with Current branch is up to
date message.

reads even more simple and clear for me.

-- 
Sergey.
--
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: Undefined reference to __builtin_ctzll

2014-08-13 Thread Erik Faye-Lund
On Wed, Aug 13, 2014 at 10:36 AM, Радослав Йовчев radoslav...@gmail.com wrote:
 Dear GIT community,


 I found myself in situation where I had to install GIT on Debian 3.1
 sarge.  It comes with GCC 3.3.5. I tried to built from source but the
 libgcc was not providing the ctzll function, thus I decided to put an
 implementation.


 I do not know how to post and do a nice patch (and whether somebody
 will care), but I guess, for reference I can post my solution. Just
 appended in compat/strlcpy.c the following:


 int __builtin_ctzll (long long x)
 {
 int i;
 for (i = 0; i  8 * sizeof (long long); ++i)
 if (x  ((long long) 1   i))
 break;
 return i;
 }


 I guess that some ifdef macro can be used to detect compiler version
 or missing __builtin_ctzll.

It seems __builtin_ctzll is only available in GCC 3.4.0 and beyond, so
I think a better fix is something like this:

diff --git a/ewah/ewok.h b/ewah/ewok.h
index 43adeb5..2700fa3 100644
--- a/ewah/ewok.h
+++ b/ewah/ewok.h
@@ -47,7 +47,7 @@ static inline uint32_t ewah_bit_popcount64(uint64_t x)
  return (x * 0x0101010101010101ULL)  56;
 }

-#ifdef __GNUC__
+#if (__GNUC__ == 3  __GNUC_MINOR__ = 4) || __GNUC__  3
 #define ewah_bit_ctz64(x) __builtin_ctzll(x)
 #else
 static inline int ewah_bit_ctz64(uint64_t x)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/6] sha1_file.c: do not die failing to malloc in unpack_compressed_entry

2014-08-13 Thread Nguyễn Thái Ngọc Duy
Fewer die() gives better control to the caller, provided that the
caller _can_ handle it. And in unpack_compressed_entry() case, it can,
because unpack_compressed_entry() already returns NULL if it fails to
inflate data.

A side effect from this is fsck continues to run when very large blobs
are present (and do not fit in memory).

Noticed-by: Dale R. Worley wor...@alum.mit.edu
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 sha1_file.c  | 4 +++-
 t/t1050-large.sh | 6 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index 3f70b1d..330862b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1923,7 +1923,9 @@ static void *unpack_compressed_entry(struct packed_git *p,
git_zstream stream;
unsigned char *buffer, *in;
 
-   buffer = xmallocz(size);
+   buffer = xmallocz_gentle(size);
+   if (!buffer)
+   return NULL;
memset(stream, 0, sizeof(stream));
stream.next_out = buffer;
stream.avail_out = size + 1;
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index aea4936..5642f84 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '
git archive --format=zip HEAD /dev/null
 '
 
+test_expect_success 'fsck' '
+   test_must_fail git fsck 2err 
+   n=$(grep error: attempting to allocate .* over limit err | wc -l) 
+   test $n -gt 1
+'
+
 test_done
-- 
2.1.0.rc0.78.gc0d8480

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 3/6] unpack-objects: continue when fail to malloc due to large objects

2014-08-13 Thread Nguyễn Thái Ngọc Duy
As a recovery tool, unpack-objects should go on unpacking as many
objects as it can.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/unpack-objects.c | 42 +-
 t/t1050-large.sh |  7 +++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 99cde45..8b5c67e 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -88,10 +88,50 @@ static void use(int bytes)
consumed_bytes += bytes;
 }
 
+static void inflate_and_throw_away(unsigned long size)
+{
+   git_zstream stream;
+   char buf[8192];
+
+   memset(stream, 0, sizeof(stream));
+   stream.next_out = (unsigned char *)buf;
+   stream.avail_out = sizeof(buf);
+   stream.next_in = fill(1);
+   stream.avail_in = len;
+   git_inflate_init(stream);
+
+   for (;;) {
+   int ret = git_inflate(stream, 0);
+   use(len - stream.avail_in);
+   if (stream.total_out == size  ret == Z_STREAM_END)
+   break;
+   if (ret != Z_OK) {
+   error(inflate returned %d, ret);
+   if (!recover)
+   exit(1);
+   has_errors = 1;
+   break;
+   }
+   stream.next_out = (unsigned char *)buf;
+   stream.avail_out = sizeof(buf);
+   stream.next_in = fill(1);
+   stream.avail_in = len;
+   }
+   git_inflate_end(stream);
+}
+
 static void *get_data(unsigned long size)
 {
git_zstream stream;
-   void *buf = xmalloc(size);
+   void *buf = xmalloc_gentle(size);
+
+   if (!buf) {
+   if (!recover)
+   exit(1);
+   has_errors = 1;
+   inflate_and_throw_away(size);
+   return NULL;
+   }
 
memset(stream, 0, sizeof(stream));
 
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index 5642f84..eec2cca 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -169,4 +169,11 @@ test_expect_success 'fsck' '
test $n -gt 1
 '
 
+test_expect_success 'unpack-objects' '
+   P=`ls .git/objects/pack/*.pack` 
+   git unpack-objects -n -r $P 2err
+   test $? = 1 
+   grep error: attempting to allocate .* over limit err
+'
+
 test_done
-- 
2.1.0.rc0.78.gc0d8480

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 6/6] diff: shortcut for diff'ing two binary SHA-1 objects

2014-08-13 Thread Nguyễn Thái Ngọc Duy
If we are given two SHA-1 and asked to determine if they are different
(but not _what_ differences), we know right away by comparing SHA-1.

A side effect of this patch is, because large files are marked binary,
diff-tree will not need to unpack them. 'diff-index --cached' will not
either. But 'diff-files' still does.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 diff.c   | 13 +
 t/t1050-large.sh |  8 
 2 files changed, 21 insertions(+)

diff --git a/diff.c b/diff.c
index d381a6f..b85bcfb 100644
--- a/diff.c
+++ b/diff.c
@@ -2324,6 +2324,19 @@ static void builtin_diff(const char *name_a,
} else if (!DIFF_OPT_TST(o, TEXT) 
( (!textconv_one  diff_filespec_is_binary(one)) ||
  (!textconv_two  diff_filespec_is_binary(two)) )) {
+   if (!one-data  !two-data 
+   S_ISREG(one-mode)  S_ISREG(two-mode) 
+   !DIFF_OPT_TST(o, BINARY)) {
+   if (!hashcmp(one-sha1, two-sha1)) {
+   if (must_show_header)
+   fprintf(o-file, %s, header.buf);
+   goto free_ab_and_return;
+   }
+   fprintf(o-file, %s, header.buf);
+   fprintf(o-file, %sBinary files %s and %s differ\n,
+   line_prefix, lbl[0], lbl[1]);
+   goto free_ab_and_return;
+   }
if (fill_mmfile(mf1, one)  0 || fill_mmfile(mf2, two)  0)
die(unable to read files to diff);
/* Quite common confusing case */
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index 711f22c..b294963 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -116,6 +116,14 @@ test_expect_success 'diff --stat' '
git diff --stat HEAD^ HEAD
 '
 
+test_expect_success 'diff' '
+   git diff HEAD^ HEAD
+'
+
+test_expect_success 'diff --cached' '
+   git diff --cached HEAD^
+'
+
 test_expect_success 'hash-object' '
git hash-object large1
 '
-- 
2.1.0.rc0.78.gc0d8480

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/6] wrapper.c: introduce gentle xmalloc(z) that does not die()

2014-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-compat-util.h |  2 ++
 wrapper.c | 73 +++
 2 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..0e541e7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -592,7 +592,9 @@ extern try_to_free_t set_try_to_free_routine(try_to_free_t);
 #endif
 extern char *xstrdup(const char *str);
 extern void *xmalloc(size_t size);
+extern void *xmalloc_gentle(size_t size);
 extern void *xmallocz(size_t size);
+extern void *xmallocz_gentle(size_t size);
 extern void *xmemdupz(const void *data, size_t len);
 extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..ad0992a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -9,16 +9,23 @@ static void do_nothing(size_t size)
 
 static void (*try_to_free_routine)(size_t size) = do_nothing;
 
-static void memory_limit_check(size_t size)
+static int memory_limit_check(size_t size, int gentle)
 {
static int limit = -1;
if (limit == -1) {
const char *env = getenv(GIT_ALLOC_LIMIT);
limit = env ? atoi(env) * 1024 : 0;
}
-   if (limit  size  limit)
-   die(attempting to allocate %PRIuMAX over limit %d,
-   (intmax_t)size, limit);
+   if (limit  size  limit) {
+   if (gentle) {
+   error(attempting to allocate %PRIuMAX over limit %d,
+ (intmax_t)size, limit);
+   return -1;
+   } else
+   die(attempting to allocate %PRIuMAX over limit %d,
+   (intmax_t)size, limit);
+   }
+   return 0;
 }
 
 try_to_free_t set_try_to_free_routine(try_to_free_t routine)
@@ -42,11 +49,12 @@ char *xstrdup(const char *str)
return ret;
 }
 
-void *xmalloc(size_t size)
+static void *do_xmalloc(size_t size, int gentle)
 {
void *ret;
 
-   memory_limit_check(size);
+   if (memory_limit_check(size, gentle))
+   return NULL;
ret = malloc(size);
if (!ret  !size)
ret = malloc(1);
@@ -55,9 +63,16 @@ void *xmalloc(size_t size)
ret = malloc(size);
if (!ret  !size)
ret = malloc(1);
-   if (!ret)
-   die(Out of memory, malloc failed (tried to allocate 
%lu bytes),
-   (unsigned long)size);
+   if (!ret) {
+   if (!gentle)
+   die(Out of memory, malloc failed (tried to 
allocate %lu bytes),
+   (unsigned long)size);
+   else {
+   error(Out of memory, malloc failed (tried to 
allocate %lu bytes),
+ (unsigned long)size);
+   return NULL;
+   }
+   }
}
 #ifdef XMALLOC_POISON
memset(ret, 0xA5, size);
@@ -65,16 +80,42 @@ void *xmalloc(size_t size)
return ret;
 }
 
-void *xmallocz(size_t size)
+void *xmalloc(size_t size)
+{
+   return do_xmalloc(size, 0);
+}
+
+void *xmalloc_gentle(size_t size)
+{
+   return do_xmalloc(size, 1);
+}
+
+static void *do_xmallocz(size_t size, int gentle)
 {
void *ret;
-   if (unsigned_add_overflows(size, 1))
-   die(Data too large to fit into virtual memory space.);
-   ret = xmalloc(size + 1);
-   ((char*)ret)[size] = 0;
+   if (unsigned_add_overflows(size, 1)) {
+   if (gentle) {
+   error(Data too large to fit into virtual memory 
space.);
+   return NULL;
+   } else
+   die(Data too large to fit into virtual memory space.);
+   }
+   ret = do_xmalloc(size + 1, gentle);
+   if (ret)
+   ((char*)ret)[size] = 0;
return ret;
 }
 
+void *xmallocz(size_t size)
+{
+   return do_xmallocz(size, 0);
+}
+
+void *xmallocz_gentle(size_t size)
+{
+   return do_xmallocz(size, 1);
+}
+
 /*
  * xmemdupz() allocates (len + 1) bytes of memory, duplicates len bytes of
  * data to the allocated memory, zero terminates the allocated memory,
@@ -96,7 +137,7 @@ void *xrealloc(void *ptr, size_t size)
 {
void *ret;
 
-   memory_limit_check(size);
+   memory_limit_check(size, 0);
ret = realloc(ptr, size);
if (!ret  !size)
ret = realloc(ptr, 1);
@@ -115,7 +156,7 @@ void *xcalloc(size_t nmemb, size_t size)
 {
void *ret;
 
-   memory_limit_check(size * nmemb);
+   memory_limit_check(size * nmemb, 0);
ret = calloc(nmemb, size);
if (!ret  (!nmemb || !size))
ret = calloc(1, 1);
-- 
2.1.0.rc0.78.gc0d8480

--
To unsubscribe from 

[PATCH v3 4/6] diff.c: allow to pass more flags to diff_populate_filespec

2014-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 diff.c| 13 +++--
 diffcore-rename.c |  6 --
 diffcore.h|  3 ++-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/diff.c b/diff.c
index 867f034..f4b7421 100644
--- a/diff.c
+++ b/diff.c
@@ -376,7 +376,7 @@ static unsigned long diff_filespec_size(struct 
diff_filespec *one)
 {
if (!DIFF_FILE_VALID(one))
return 0;
-   diff_populate_filespec(one, 1);
+   diff_populate_filespec(one, CHECK_SIZE_ONLY);
return one-size;
 }
 
@@ -1910,11 +1910,11 @@ static void show_dirstat(struct diff_options *options)
diff_free_filespec_data(p-one);
diff_free_filespec_data(p-two);
} else if (DIFF_FILE_VALID(p-one)) {
-   diff_populate_filespec(p-one, 1);
+   diff_populate_filespec(p-one, CHECK_SIZE_ONLY);
copied = added = 0;
diff_free_filespec_data(p-one);
} else if (DIFF_FILE_VALID(p-two)) {
-   diff_populate_filespec(p-two, 1);
+   diff_populate_filespec(p-two, CHECK_SIZE_ONLY);
copied = 0;
added = p-two-size;
diff_free_filespec_data(p-two);
@@ -2668,8 +2668,9 @@ static int diff_populate_gitlink(struct diff_filespec *s, 
int size_only)
  * grab the data for the blob (or file) for our own in-core comparison.
  * diff_filespec has data and size fields for this purpose.
  */
-int diff_populate_filespec(struct diff_filespec *s, int size_only)
+int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 {
+   int size_only = flags  CHECK_SIZE_ONLY;
int err = 0;
/*
 * demote FAIL to WARN to allow inspecting the situation
@@ -4688,8 +4689,8 @@ static int diff_filespec_check_stat_unmatch(struct 
diff_filepair *p)
!DIFF_FILE_VALID(p-two) ||
(p-one-sha1_valid  p-two-sha1_valid) ||
(p-one-mode != p-two-mode) ||
-   diff_populate_filespec(p-one, 1) ||
-   diff_populate_filespec(p-two, 1) ||
+   diff_populate_filespec(p-one, CHECK_SIZE_ONLY) ||
+   diff_populate_filespec(p-two, CHECK_SIZE_ONLY) ||
(p-one-size != p-two-size) ||
!diff_filespec_is_identical(p-one, p-two)) /* (2) */
p-skip_stat_unmatch_result = 1;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 2e44a37..4e132f1 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -147,9 +147,11 @@ static int estimate_similarity(struct diff_filespec *src,
 * is a possible size - we really should have a flag to
 * say whether the size is valid or not!)
 */
-   if (!src-cnt_data  diff_populate_filespec(src, 1))
+   if (!src-cnt_data 
+   diff_populate_filespec(src, CHECK_SIZE_ONLY))
return 0;
-   if (!dst-cnt_data  diff_populate_filespec(dst, 1))
+   if (!dst-cnt_data 
+   diff_populate_filespec(dst, CHECK_SIZE_ONLY))
return 0;
 
max_size = ((src-size  dst-size) ? src-size : dst-size);
diff --git a/diffcore.h b/diffcore.h
index c876dac..c80df18 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -55,7 +55,8 @@ extern void free_filespec(struct diff_filespec *);
 extern void fill_filespec(struct diff_filespec *, const unsigned char *,
  int, unsigned short);
 
-extern int diff_populate_filespec(struct diff_filespec *, int);
+#define CHECK_SIZE_ONLY 1
+extern int diff_populate_filespec(struct diff_filespec *, unsigned int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 extern void diff_free_filespec_blob(struct diff_filespec *);
 extern int diff_filespec_is_binary(struct diff_filespec *);
-- 
2.1.0.rc0.78.gc0d8480

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/6] Large file improvements

2014-08-13 Thread Nguyễn Thái Ngọc Duy
Since v2:

 - reword the fsck patch to center around unpack_compressed_entry instead
 - make sure unpack-objects survive out of memory error because of large files
 - rename diff_filespec_population flags
 - make git diff tree tree work on large files

Nguyễn Thái Ngọc Duy (6):
  wrapper.c: introduce gentle xmalloc(z) that does not die()
  sha1_file.c: do not die failing to malloc in unpack_compressed_entry
  unpack-objects: continue when fail to malloc due to large objects
  diff.c: allow to pass more flags to diff_populate_filespec
  diff --stat: mark any file larger than core.bigfilethreshold binary
  diff: shortcut for diff'ing two binary SHA-1 objects

 Documentation/config.txt|  3 +-
 Documentation/gitattributes.txt |  4 +--
 builtin/unpack-objects.c| 42 +++-
 diff.c  | 52 +
 diffcore-rename.c   |  6 ++--
 diffcore.h  |  4 ++-
 git-compat-util.h   |  2 ++
 sha1_file.c |  4 ++-
 t/t1050-large.sh| 25 ++
 wrapper.c   | 73 -
 10 files changed, 177 insertions(+), 38 deletions(-)

-- 
2.1.0.rc0.78.gc0d8480

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 5/6] diff --stat: mark any file larger than core.bigfilethreshold binary

2014-08-13 Thread Nguyễn Thái Ngọc Duy
Too large files may lead to failure to allocate memory. If it happens
here, it could impact quite a few commands that involve
diff. Moreover, too large files are inefficient to compare anyway (and
most likely non-text), so mark them binary and skip looking at their
content.

Noticed-by: Dale R. Worley wor...@alum.mit.edu
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt|  3 ++-
 Documentation/gitattributes.txt |  4 ++--
 diff.c  | 26 ++
 diffcore.h  |  1 +
 t/t1050-large.sh|  4 
 5 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c55c22a..53df40e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -499,7 +499,8 @@ core.bigFileThreshold::
Files larger than this size are stored deflated, without
attempting delta compression.  Storing large files without
delta compression avoids excessive memory usage, at the
-   slight expense of increased disk usage.
+   slight expense of increased disk usage. Additionally files
+   larger than this size are allways treated as binary.
 +
 Default is 512 MiB on all platforms.  This should be reasonable
 for most projects as source code and other text files can still
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 643c1ba..9b45bda 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -440,8 +440,8 @@ Unspecified::
 
A path to which the `diff` attribute is unspecified
first gets its contents inspected, and if it looks like
-   text, it is treated as text.  Otherwise it would
-   generate `Binary files differ`.
+   text and is smaller than core.bigFileThreshold, it is treated
+   as text. Otherwise it would generate `Binary files differ`.
 
 String::
 
diff --git a/diff.c b/diff.c
index f4b7421..d381a6f 100644
--- a/diff.c
+++ b/diff.c
@@ -2188,8 +2188,8 @@ int diff_filespec_is_binary(struct diff_filespec *one)
one-is_binary = one-driver-binary;
else {
if (!one-data  DIFF_FILE_VALID(one))
-   diff_populate_filespec(one, 0);
-   if (one-data)
+   diff_populate_filespec(one, CHECK_BINARY);
+   if (one-is_binary == -1  one-data)
one-is_binary = buffer_is_binary(one-data,
one-size);
if (one-is_binary == -1)
@@ -2725,6 +2725,11 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
}
if (size_only)
return 0;
+   if ((flags  CHECK_BINARY) 
+   s-size  big_file_threshold  s-is_binary == -1) {
+   s-is_binary = 1;
+   return 0;
+   }
fd = open(s-path, O_RDONLY);
if (fd  0)
goto err_empty;
@@ -2746,16 +2751,21 @@ int diff_populate_filespec(struct diff_filespec *s, 
unsigned int flags)
}
else {
enum object_type type;
-   if (size_only) {
+   if (size_only || (flags  CHECK_BINARY)) {
type = sha1_object_info(s-sha1, s-size);
if (type  0)
die(unable to read %s, sha1_to_hex(s-sha1));
-   } else {
-   s-data = read_sha1_file(s-sha1, type, s-size);
-   if (!s-data)
-   die(unable to read %s, sha1_to_hex(s-sha1));
-   s-should_free = 1;
+   if (size_only)
+   return 0;
+   if (s-size  big_file_threshold  s-is_binary == -1) 
{
+   s-is_binary = 1;
+   return 0;
+   }
}
+   s-data = read_sha1_file(s-sha1, type, s-size);
+   if (!s-data)
+   die(unable to read %s, sha1_to_hex(s-sha1));
+   s-should_free = 1;
}
return 0;
 }
diff --git a/diffcore.h b/diffcore.h
index c80df18..33ea2de 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -56,6 +56,7 @@ extern void fill_filespec(struct diff_filespec *, const 
unsigned char *,
  int, unsigned short);
 
 #define CHECK_SIZE_ONLY 1
+#define CHECK_BINARY2
 extern int diff_populate_filespec(struct diff_filespec *, unsigned int);
 extern void diff_free_filespec_data(struct diff_filespec *);
 extern void diff_free_filespec_blob(struct diff_filespec *);
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index eec2cca..711f22c 100755
--- a/t/t1050-large.sh
+++ 

Re: [PATCH 1/4] fast-import.c: replace `git_config()` with `git_config_get_*()` family

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

  fast-import.c | 42 +++---
  1 file changed, 19 insertions(+), 23 deletions(-)

Only 4 lines less, how disappointing ;-).

More seriously, the old code was essentially dealing with special
values, which your new code needs to do too, so you can hardly do any
less.

 + if (!git_config_get_int(pack.compression, pack_compression_level)) {
 + if (pack_compression_level == -1)
 + pack_compression_level = Z_DEFAULT_COMPRESSION;
 + else if (pack_compression_level  0 ||
 +  pack_compression_level  Z_BEST_COMPRESSION)
 + die(bad pack compression level %d, 
 pack_compression_level);

That would be a good use for git_die_config(), to give a better error
message, right?

 - if (!strcmp(k, pack.indexversion)) {
 - pack_idx_opts.version = git_config_int(k, v);
 + if (!git_config_get_int(pack.indexversion, indexversion_value)) {
 + pack_idx_opts.version = indexversion_value;

I wondered why you were not assigning to pack_idx_opts.version directly,
but pack_idx_opts.version is uint32 and you don't have
config_get_uint32, so it's OK.

   if (pack_idx_opts.version  2)
 - die(bad pack.indexversion=%PRIu32,
 - pack_idx_opts.version);
 - return 0;
 + die(bad pack.indexversion=%PRIu32, 
 pack_idx_opts.version);

One more opportunity for git_die_config()?

Not that it's terribly important, but I think it's good that your
refactoring also brings a few end-users benefits. It will help you show
off when you tell your friends what you did this summer (not I did
useless code churn ;-) ), and helps everybody see the benefits of your
GSoC ;-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] ll-merge.c: refactor `read_merge_config()` to use `git_config_string()`

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

 Signed-off-by: Tanay Abhra tanay...@gmail.com
 ---
  ll-merge.c | 23 ++-
  1 file changed, 6 insertions(+), 17 deletions(-)

 diff --git a/ll-merge.c b/ll-merge.c
 index fb61ea6..8ea03e5 100644
 --- a/ll-merge.c
 +++ b/ll-merge.c
 @@ -225,11 +225,8 @@ static int read_merge_config(const char *var, const char 
 *value, void *cb)
   const char *key, *name;
   int namelen;
  
 - if (!strcmp(var, merge.default)) {
 - if (value)
 - default_ll_merge = xstrdup(value);
 - return 0;
 - }
 + if (!strcmp(var, merge.default))
 + return git_config_string(default_ll_merge, var, value);

Previously, merge.default without value was a no-op. It's now an error.

I think it makes perfect sense, but should be documented in the log
message.

Also, I think you should explain briefly the reason for not using your
non-callback API here.

-- 
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 3/4] merge-recursive.c: replace `git_config()` with `git_config_get_int()`

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

  merge-recursive.c | 22 ++
  1 file changed, 6 insertions(+), 16 deletions(-)

  builtin/apply.c | 12 +---
  1 file changed, 5 insertions(+), 7 deletions(-)

These two look straightforward and good.

-- 
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 1/4] fast-import.c: replace `git_config()` with `git_config_get_*()` family

2014-08-13 Thread Tanay Abhra
  if (pack_idx_opts.version  2)
 -die(bad pack.indexversion=%PRIu32,
 -pack_idx_opts.version);
 -return 0;
 +die(bad pack.indexversion=%PRIu32, 
 pack_idx_opts.version);
 
 One more opportunity for git_die_config()?


Yup, I had thought of changing that and above to git_die_config(), but didn't do
it, will send a revised patch.


 Not that it's terribly important, but I think it's good that your
 refactoring also brings a few end-users benefits. It will help you show

I have been rewriting callers and the law of diminishing returns kicked in. I 
had
rewritten some other call sites but I didn't see them bringing any benefits 
(cleaner
control flow, less lines, bugs eliminated), so I left them out.

 off when you tell your friends what you did this summer (not I did
 useless code churn ;-) ), and helps everybody see the benefits of your
 GSoC ;-).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/5] fast-import.c: replace `git_config()` with `git_config_get_*()` family

2014-08-13 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
---
 fast-import.c | 44 +---
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index d73f58c..34e780d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3274,36 +3274,34 @@ static void parse_option(const char *option)
die(This version of fast-import does not support option: %s, option);
 }

-static int git_pack_config(const char *k, const char *v, void *cb)
+static void git_pack_config(void)
 {
-   if (!strcmp(k, pack.depth)) {
-   max_depth = git_config_int(k, v);
+   int indexversion_value;
+   unsigned long packsizelimit_value;
+
+   if (!git_config_get_ulong(pack.depth, max_depth)) {
if (max_depth  MAX_DEPTH)
max_depth = MAX_DEPTH;
-   return 0;
}
-   if (!strcmp(k, pack.compression)) {
-   int level = git_config_int(k, v);
-   if (level == -1)
-   level = Z_DEFAULT_COMPRESSION;
-   else if (level  0 || level  Z_BEST_COMPRESSION)
-   die(bad pack compression level %d, level);
-   pack_compression_level = level;
+   if (!git_config_get_int(pack.compression, pack_compression_level)) {
+   if (pack_compression_level == -1)
+   pack_compression_level = Z_DEFAULT_COMPRESSION;
+   else if (pack_compression_level  0 ||
+pack_compression_level  Z_BEST_COMPRESSION)
+   git_die_config(pack.compression,
+   bad pack compression level %d, 
pack_compression_level);
pack_compression_seen = 1;
-   return 0;
}
-   if (!strcmp(k, pack.indexversion)) {
-   pack_idx_opts.version = git_config_int(k, v);
+   if (!git_config_get_int(pack.indexversion, indexversion_value)) {
+   pack_idx_opts.version = indexversion_value;
if (pack_idx_opts.version  2)
-   die(bad pack.indexversion=%PRIu32,
-   pack_idx_opts.version);
-   return 0;
+   git_die_config(pack.indexversion,
+   bad pack.indexversion=%PRIu32, 
pack_idx_opts.version);
}
-   if (!strcmp(k, pack.packsizelimit)) {
-   max_packsize = git_config_ulong(k, v);
-   return 0;
-   }
-   return git_default_config(k, v, cb);
+   if (!git_config_get_ulong(pack.packsizelimit, packsizelimit_value))
+   max_packsize = packsizelimit_value;
+
+   git_config(git_default_config, NULL);
 }

 static const char fast_import_usage[] =
@@ -3356,7 +3354,7 @@ int main(int argc, char **argv)

setup_git_directory();
reset_pack_idx_option(pack_idx_opts);
-   git_config(git_pack_config, NULL);
+   git_pack_config();
if (!pack_compression_seen  core_compression_seen)
pack_compression_level = core_compression_level;

-- 
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 2/4] ll-merge.c: refactor `read_merge_config()` to use `git_config_string()`

2014-08-13 Thread Tanay Abhra
There is one slight behavior change, previously merge.default
silently ignored a NULL value and didn't raise any error. But,
in the same function, all other values raise an error on a NULL
value. So to conform with other call sites in Git, a NULL value
for merge.default raises an error.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
We cannot easily use the new config-set API here, because
much of the function is dedicated to processing
merge.name.variable which does not easily translate to
the new API. If it were for variables like,
merge.summary, merge.tool, and merge.verbosity, we
could use the new API.

 ll-merge.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/ll-merge.c b/ll-merge.c
index fb61ea6..8ea03e5 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -225,11 +225,8 @@ static int read_merge_config(const char *var, const char 
*value, void *cb)
const char *key, *name;
int namelen;

-   if (!strcmp(var, merge.default)) {
-   if (value)
-   default_ll_merge = xstrdup(value);
-   return 0;
-   }
+   if (!strcmp(var, merge.default))
+   return git_config_string(default_ll_merge, var, value);

/*
 * We are not interested in anything but merge.name.variable;
@@ -254,12 +251,8 @@ static int read_merge_config(const char *var, const char 
*value, void *cb)
ll_user_merge_tail = (fn-next);
}

-   if (!strcmp(name, key)) {
-   if (!value)
-   return error(%s: lacks value, var);
-   fn-description = xstrdup(value);
-   return 0;
-   }
+   if (!strcmp(name, key))
+   return git_config_string(fn-description, var, value);

if (!strcmp(driver, key)) {
if (!value)
@@ -285,12 +278,8 @@ static int read_merge_config(const char *var, const char 
*value, void *cb)
return 0;
}

-   if (!strcmp(recursive, key)) {
-   if (!value)
-   return error(%s: lacks value, var);
-   fn-recursive = xstrdup(value);
-   return 0;
-   }
+   if (!strcmp(recursive, key))
+   return git_config_string(fn-recursive, var, value);

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 v2 23/23] rebase -i: enable options --signoff, --reset-author for pick, reword

2014-08-13 Thread Michael Haggerty
On 08/07/2014 01:59 AM, Fabian Ruch wrote:
 pick and reword are atomic to-do list commands in the sense that they
 open a new task which is closed after the respective command is
 completed. squash and fixup are not atomic. They create a new task
 which is not completed until the last squash or fixup is processed.

I don't understand the distinction that you are attempting to draw
between atomic and non-atomic commands.  For example, in the
following command list:

pick 111
squash 222
fixup 333

the pick command doesn't seem very atomic, because the *end* result of
the three commands is a single commit that is affected by all three
commands.  Furthermore, if we change the example to

pick 111
squash --reset-author 222
fixup --signoff 333

then isn't it clear that the user's intention was to apply both options,
--reset-author and --signoff, to the resulting commit?  In other
words, it seems to me that any options on such a chain of lines should
be collected and applied to the final commit as a whole.

 Lift the general unknown option blockade for the pick and reword
 commands. If `do_cmd` comes across one of the options `--signoff` and
 `--reset-author` while parsing a to-do entry and the scheduled
 command is either `pick` or `reword`, relay the option to `do_pick`.

The new user-exposed options should be documented in the git-rebase(1)
manpage and probably also in the help text that is appended to every
rebase -i todo list.

 The `do_pick` options `--gpg-sign` and `--file` are not yet supported
 because `do_cmd` cannot handle option arguments and options with
 spaces at the moment. It is true that edit is one of the atomic
 commands but it displays hash information when the rebase is stopped
 and some options rewrite the picked commit which alters that
 information. squash and fixup still do not accept user options as the
 interplay of `--reset-author` and the author script are yet to be
 determined.
 [...]

Michael

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


Re: [PATCH v2 2/4] ll-merge.c: refactor `read_merge_config()` to use `git_config_string()`

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

 There is one slight behavior change, previously merge.default
 silently ignored a NULL value and didn't raise any error. But,
 in the same function, all other values raise an error on a NULL
 value. So to conform with other call sites in Git, a NULL value
 for merge.default raises an error.

Good, thanks.

 We cannot easily use the new config-set API here, because
 much of the function is dedicated to processing
 merge.name.variable which does not easily translate to
 the new API. If it were for variables like,
 merge.summary, merge.tool, and merge.verbosity, we
 could use the new API.

I think this would deserve to be in the commit message, but I'm fine
with keeping it here too.

-- 
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 v2 1/5] fast-import.c: replace `git_config()` with `git_config_get_*()` family

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

 + if (!git_config_get_int(pack.compression, pack_compression_level)) {
 + if (pack_compression_level == -1)
 + pack_compression_level = Z_DEFAULT_COMPRESSION;
 + else if (pack_compression_level  0 ||
 +  pack_compression_level  Z_BEST_COMPRESSION)
 + git_die_config(pack.compression,
 + bad pack compression level %d, 
 pack_compression_level);

Perfect. With v2 for PATCH 2 and PATCH 5, the series looks good to me.

-- 
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/RFC v2 1/2] git_default_config() rewritten using the config-set API

2014-08-13 Thread Tanay Abhra
git_default_config() now uses config-set API functions to query for
values.

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Sorry, for the short log message, I will explain why.
The git_default_config() rewrite is 100% complete, the only
problem remains is the call sites; there are too many of them.
Some are called from callback functions which pass the remaining
variables to git_default_config() which they do not have any use for.
Those call sites can remain as they are, because git_default_config()
is a single call function now, and is guarded by a sentinel value.
So after one call, it would just return immediately instead of going on
checking.

For callers like git_config(git_default_config, NULL) (there are 38 of them),
we can leave them as they are or rewrite them as I have illustrated in the
next attached patch.

I will take this series out of RFC as soon as we have decided what to do with
the call sites.

Cheers,
Tanay Abhra.

 advice.c |  17 ++--
 advice.h |   2 +-
 cache.h  |   2 +-
 config.c | 346 ++-
 ident.c  |  17 ++--
 5 files changed, 136 insertions(+), 248 deletions(-)

diff --git a/advice.c b/advice.c
index 9b42033..34e1ccc 100644
--- a/advice.c
+++ b/advice.c
@@ -59,22 +59,17 @@ void advise(const char *advice, ...)
strbuf_release(buf);
 }

-int git_default_advice_config(const char *var, const char *value)
+void git_default_advice_config(void)
 {
-   const char *k;
+   struct strbuf var = STRBUF_INIT;
int i;

-   if (!skip_prefix(var, advice., k))
-   return 0;
-
for (i = 0; i  ARRAY_SIZE(advice_config); i++) {
-   if (strcmp(k, advice_config[i].name))
-   continue;
-   *advice_config[i].preference = git_config_bool(var, value);
-   return 0;
+   strbuf_addf(var, advice.%s, advice_config[i].name);
+   git_config_get_bool(var.buf, advice_config[i].preference);
+   strbuf_reset(var);
}
-
-   return 0;
+   strbuf_release(var);
 }

 int error_resolve_conflict(const char *me)
diff --git a/advice.h b/advice.h
index 5ecc6c1..5bfe46c 100644
--- a/advice.h
+++ b/advice.h
@@ -19,7 +19,7 @@ extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
 extern int advice_rm_hints;

-int git_default_advice_config(const char *var, const char *value);
+void git_default_advice_config(void);
 __attribute__((format (printf, 1, 2)))
 void advise(const char *advice, ...);
 int error_resolve_conflict(const char *me);
diff --git a/cache.h b/cache.h
index 2693a37..fa28b40 100644
--- a/cache.h
+++ b/cache.h
@@ -1065,7 +1065,7 @@ extern const char *fmt_name(const char *name, const char 
*email);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
 extern const char *git_pager(int stdout_is_tty);
-extern int git_ident_config(const char *, const char *, void *);
+extern void git_ident_config(void);

 struct ident_split {
const char *name_begin;
diff --git a/config.c b/config.c
index 427850a..36b9124 100644
--- a/config.c
+++ b/config.c
@@ -46,6 +46,8 @@ static int zlib_compression_seen;
  */
 static struct config_set the_config_set;

+static int default_config_loaded;
+
 static int config_file_fgetc(struct config_source *conf)
 {
return fgetc(conf-u.file);
@@ -670,147 +672,91 @@ int git_config_pathname(const char **dest, const char 
*var, const char *value)
return 0;
 }

-static int git_default_core_config(const char *var, const char *value)
+static void git_default_core_config(void)
 {
+   const char *value = NULL;
+   unsigned long v_l = 0;
+   int abbrev;
+   const char *comment;
+
/* This needs a better name */
-   if (!strcmp(var, core.filemode)) {
-   trust_executable_bit = git_config_bool(var, value);
-   return 0;
-   }
-   if (!strcmp(var, core.trustctime)) {
-   trust_ctime = git_config_bool(var, value);
-   return 0;
-   }
-   if (!strcmp(var, core.checkstat)) {
+   git_config_get_bool(core.filemode, trust_executable_bit);
+   git_config_get_bool(core.trustctime, trust_ctime);
+
+   if (!git_config_get_value(core.checkstat, value)) {
if (!strcasecmp(value, default))
check_stat = 1;
else if (!strcasecmp(value, minimal))
check_stat = 0;
}

-   if (!strcmp(var, core.quotepath)) {
-   quote_path_fully = git_config_bool(var, value);
-   return 0;
-   }
+   git_config_get_bool(core.quotepath, quote_path_fully);
+   git_config_get_bool(core.symlinks, has_symlinks);
+   git_config_get_bool(core.ignorecase, ignore_case);
+   git_config_get_pathname(core.attributesfile, git_attributes_file);
+   git_config_get_bool(core.bare, is_bare_repository_cfg);
+   

[PATCH/RFC v2 2/2] use the new git_default_config()

2014-08-13 Thread Tanay Abhra
If we change the signature to void git_default_config(void),
we would have to use a patch like this to change the call sites
of the function. This patch is just for illustrative purpose,
I couldn't finalize if this was unnecessary code cruft or
a valid approach.

---
 builtin/check-attr.c | 2 +-
 builtin/check-ignore.c   | 2 +-
 builtin/check-mailmap.c  | 2 +-
 builtin/checkout-index.c | 2 +-
 builtin/clone.c  | 2 +-
 builtin/config.c | 2 +-
 builtin/describe.c   | 2 +-
 builtin/fast-export.c| 2 +-
 builtin/for-each-ref.c   | 2 +-
 builtin/hash-object.c| 2 +-
 builtin/init-db.c| 2 +-
 builtin/ls-files.c   | 2 +-
 builtin/ls-tree.c| 2 +-
 builtin/merge-base.c | 2 +-
 builtin/mv.c | 2 +-
 builtin/name-rev.c   | 2 +-
 builtin/notes.c  | 2 +-
 builtin/push.c   | 2 +-
 builtin/read-tree.c  | 2 +-
 builtin/reset.c  | 2 +-
 builtin/rev-list.c   | 2 +-
 builtin/rev-parse.c  | 2 +-
 builtin/revert.c | 4 ++--
 builtin/rm.c | 2 +-
 builtin/shortlog.c   | 2 +-
 builtin/stripspace.c | 2 +-
 builtin/symbolic-ref.c   | 2 +-
 builtin/unpack-file.c| 2 +-
 builtin/unpack-objects.c | 2 +-
 builtin/update-index.c   | 2 +-
 builtin/update-ref.c | 2 +-
 builtin/update-server-info.c | 2 +-
 builtin/var.c| 2 +-
 builtin/verify-pack.c| 2 +-
 builtin/write-tree.c | 2 +-
 http-fetch.c | 2 +-
 pager.c  | 2 +-
 37 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/builtin/check-attr.c b/builtin/check-attr.c
index 5600ec3..e2d7826 100644
--- a/builtin/check-attr.c
+++ b/builtin/check-attr.c
@@ -105,7 +105,7 @@ int cmd_check_attr(int argc, const char **argv, const char 
*prefix)
if (!is_bare_repository())
setup_work_tree();

-   git_config(git_default_config, NULL);
+   git_default_config();

argc = parse_options(argc, argv, prefix, check_attr_options,
 check_attr_usage, PARSE_OPT_KEEP_DASHDASH);
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 594463a..c14c977 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -144,7 +144,7 @@ int cmd_check_ignore(int argc, const char **argv, const 
char *prefix)
int num_ignored;
struct dir_struct dir;

-   git_config(git_default_config, NULL);
+   git_default_config();

argc = parse_options(argc, argv, prefix, check_ignore_options,
 check_ignore_usage, 0);
diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c
index 8f4d809..f9d0de6 100644
--- a/builtin/check-mailmap.c
+++ b/builtin/check-mailmap.c
@@ -40,7 +40,7 @@ int cmd_check_mailmap(int argc, const char **argv, const char 
*prefix)
int i;
struct string_list mailmap = STRING_LIST_INIT_NODUP;

-   git_config(git_default_config, NULL);
+   git_default_config();
argc = parse_options(argc, argv, prefix, check_mailmap_options,
 check_mailmap_usage, 0);
if (argc == 0  !use_stdin)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 05edd9e..197a987 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -213,7 +213,7 @@ int cmd_checkout_index(int argc, const char **argv, const 
char *prefix)
if (argc == 2  !strcmp(argv[1], -h))
usage_with_options(builtin_checkout_index_usage,
   builtin_checkout_index_options);
-   git_config(git_default_config, NULL);
+   git_default_config();
state.base_dir = ;
prefix_length = prefix ? strlen(prefix) : 0;

diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..bcfd322 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -864,7 +864,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
init_db(option_template, INIT_DB_QUIET);
write_config(option_config);

-   git_config(git_default_config, NULL);
+   git_default_config();

if (option_bare) {
if (option_mirror)
diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..eed430d 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -558,7 +558,7 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
die(editing stdin is not supported);
if (given_config_source.blob)
die(editing blobs is not supported);
-   git_config(git_default_config, NULL);
+   git_default_config();
launch_editor(given_config_source.file ?
  given_config_source.file : git_path(config),
  NULL, NULL);
diff --git a/builtin/describe.c b/builtin/describe.c
index 

Re: [PATCH 2/3] unpack-trees: use 'cuddled' style for if-else cascade

2014-08-13 Thread Ronnie Sahlberg
On Tue, Aug 12, 2014 at 5:00 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Match the predominant style in git by following KR style for if/else
 cascades.  Documentation/CodingStyle from linux.git explains:

   Note that the closing brace is empty on a line of its own, _except_ in
   the cases where it is followed by a continuation of the same statement,
   ie a while in a do-statement or an else in an if-statement, like
   this:

 if (x == y) {
 ..
 } else if (x  y) {
 ...
 } else {
 
 }

   Rationale: KR.

   Also, note that this brace-placement also minimizes the number of empty
   (or almost empty) lines, without any loss of readability.

 Signed-off-by: Jonathan Nieder jrnie...@gmail.com

Reviewed-by: Ronnie Sahlberg sahlb...@google.com

 ---
  unpack-trees.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)

 diff --git a/unpack-trees.c b/unpack-trees.c
 index f4a9aa9..187b15b 100644
 --- a/unpack-trees.c
 +++ b/unpack-trees.c
 @@ -1771,8 +1771,7 @@ int twoway_merge(const struct cache_entry * const *src,
 return merged_entry(newtree, current, 
 o);
 }
 return o-gently ? -1 : reject_merge(current, o);
 -   }
 -   else if ((!oldtree  !newtree) || /* 4 and 5 */
 +   } else if ((!oldtree  !newtree) || /* 4 and 5 */
  (!oldtree  newtree 
   same(current, newtree)) || /* 6 and 7 */
  (oldtree  newtree 
 @@ -1781,17 +1780,14 @@ int twoway_merge(const struct cache_entry * const 
 *src,
   !same(oldtree, newtree)  /* 18 and 19 */
   same(current, newtree))) {
 return keep_entry(current, o);
 -   }
 -   else if (oldtree  !newtree  same(current, oldtree)) {
 +   } else if (oldtree  !newtree  same(current, oldtree)) {
 /* 10 or 11 */
 return deleted_entry(oldtree, current, o);
 -   }
 -   else if (oldtree  newtree 
 +   } else if (oldtree  newtree 
  same(current, oldtree)  !same(current, newtree)) {
 /* 20 or 21 */
 return merged_entry(newtree, current, o);
 -   }
 -   else
 +   } else
 return o-gently ? -1 : reject_merge(current, o);
 }
 else if (newtree) {
 --
 --
 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v2 1/2] git_default_config() rewritten using the config-set API

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

 git_default_config() now uses config-set API functions to query for
 values.

I believe you missed a few spots:

$ git grep -n 'git_default_config[^(]'
Documentation/user-manual.txt:4287:git_config(git_default_config);
archive.c:416:  git_config(git_default_config, NULL);
builtin/config.c:577:   git_config(git_default_config, NULL);
color.h:73: * if you are just going to change to git_default_config, too.
fetch-pack.c:880:   git_config(git_default_config, NULL);
http.c:393: config.cascade_fn = git_default_config;
rerere.c:580:   git_config(git_default_config, NULL);
rerere.c:710:   git_config(git_default_config, NULL);

The following ones should probably be rewritten too:

archive.c:416:  git_config(git_default_config, NULL);
builtin/config.c:577:   git_config(git_default_config, NULL);
fetch-pack.c:880:   git_config(git_default_config, NULL);
rerere.c:580:   git_config(git_default_config, NULL);
rerere.c:710:   git_config(git_default_config, NULL);

-- 
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/RFC v2 1/2] git_default_config() rewritten using the config-set API

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

 git_default_config() now uses config-set API functions to query for
 values.

 Signed-off-by: Tanay Abhra tanay...@gmail.com
 ---
 Sorry, for the short log message, I will explain why.
 The git_default_config() rewrite is 100% complete, the only
 problem remains is the call sites; there are too many of them.
 Some are called from callback functions which pass the remaining
 variables to git_default_config() which they do not have any use for.
 Those call sites can remain as they are, because git_default_config()
 is a single call function now, and is guarded by a sentinel value.

They can remain as they are, but it would also be relatively easy to
turn them into non-callback style by doing something like this on each
call:

--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -37,7 +37,7 @@ static int commit_tree_config(const char *var, const char 
*value, void *cb)
sign_commit = git_config_bool(var, value) ?  : NULL;
return 0;
}
-   return git_default_config(var, value, cb);
+   return 0
 }
 
 int cmd_commit_tree(int argc, const char **argv, const char *prefix)
@@ -49,6 +49,7 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
struct strbuf buffer = STRBUF_INIT;
 
git_config(commit_tree_config, NULL);
+   git_default_config();
 
if (argc  2 || !strcmp(argv[1], -h))
usage(commit_tree_usage);

 -int git_default_config(const char *var, const char *value, void *dummy)
 +int git_default_config(const char *unused, const char *unused2, void *dummy)

By having these dummy arguments, you force callers to pass dummy actual
parameters.

Actually, you don't pass anything in PATCH 2, hence the result is not
compilable:

http-fetch.c:70:2: error: too few arguments to function ‘git_default_config’
  git_default_config();
  ^
In file included from http-fetch.c:1:0:
cache.h:1299:12: note: declared here
 extern int git_default_config(const char *, const char *, void *);

After your patch, there are two things git_default_config do:

1) normal callers want to call git_default_config();

2) callback-style callers want to write
   return git_default_config(var, value, cb);

I think this deserves two functions, calling each others:

/* For 1) */
void git_load_default_config(void)
{
do the actual stuff
}

/* For 2) */
int git_default_config(const char *unused, const char *unused2, void *dummy)
{
if (default_config_loaded)
return;
git_load_default_config(NULL, NULL, NULL);
default_config_loaded = 1;
return 0;
}

In an ideal world, git_default_config would disappear after the rewrite
is completed. In practice, it may stay if needed, it doesn't harm
anyone.

PATCH 2 would turn git_config(git_default_config, NULL); into
git_load_default_config().

 @@ -2082,6 +1977,7 @@ int git_config_set_multivar_in_file(const char 
 *config_filename,

   /* Invalidate the config cache */
   git_config_clear();
 + default_config_loaded = 0;

What about the other callsite in setup.c? We may have left the
configuration half-loaded, and if anyone calls git_load_default_config()
again after that, we do want to reload it, don't we?

Which leads to another question: why not put this default_config_loaded
= 0; inside git_config_clear(), to avoid forgetting?

 index 1d9b6e7..0db595f 100644
 --- a/ident.c
 +++ b/ident.c
 @@ -392,29 +392,26 @@ int author_ident_sufficiently_given(void)
   return ident_is_sufficient(author_ident_explicitly_given);
  }

 -int git_ident_config(const char *var, const char *value, void *data)
 +void git_ident_config(void)
  {
 - if (!strcmp(var, user.name)) {
 + const char *value = NULL;
 +
 + if (!git_config_get_value(user.name, value)) {
   if (!value)
 - return config_error_nonbool(var);
 + git_die_config(user.name, Missing value for 
 'user.name');

I'd rather have git_config_get_string() and a free() afterwards to avoid
duplicating this Missing value for 'user.name' (which should be _()-ed
if it stays).

 -
 - if (!strcmp(var, user.email)) {
 + if (!git_config_get_value(user.email, value)) {
   if (!value)
 - return config_error_nonbool(var);
 + git_die_config(user.email, Missing value for 
 'user.email');

Likewise.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs

2014-08-13 Thread Ronnie Sahlberg
David,

One possible solution can be to use the external database daemon I am
working of for ref transactions.
Since this makes all refs be stored in a dedicated database instead of
the filesystem you no longer are dependent on file system semantics.

While not in the official git trees yet I would appreciate any testing
and comments you may have it you want to test it.
https://github.com/rsahlberg/git/tree/backend-struct-db-2

Not for production use but seeing if it does build and works on your
platforms would be great.


refsd-tdb: example refs database daemon

refsd-tdb.c is a simple reference implementation of a refs daemon.
This will eventually be hosted in a separate project but can live in this
branch for now.

Compile with :
gcc refsd-tdb.c -o refsd-tdb -l tdb

Run with:
./refsd-tdb /tmp/refsd.socket /tmp /tmp/refsd.log

Once the refs daemon is running you can start using it with newly
created git repositories by specifying the --db-socket and --db-repo-name
arguments to git clone/init-db :

git clone --db-repo-name=ROCKy --db-socket=/tmp/refsd.socket some repo .

This refs daemon is an example. It should be relatively straightforward
to modify it to attach to any other kind of data store.

regards
ronnie sahlberg


On Wed, Jun 11, 2014 at 3:30 PM, David Turner dtur...@twopensource.com wrote:
 This issue bit us again recently.

 In talking with some colleagues, I realized that the previous version
 of this patch, in addition to being potentially slow, was incomplete.
 Specifically, it didn't handle the case of refs/heads/case/one vs
 refs/heads/CASE/two; these are case clones even though they strcasecmp
 different.

 This new version contains code to prevent this, as well as tests for
 this case.

 Also it uses a hashmap to make lookups constant-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
--
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] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

2014-08-13 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 ... I.e., git must not rebase anything
 when Current branch is a descendant of the commit you are rebasing
 onto, unless -f is given. Simple, reasonable, straightforward.

It may be simple and straightforward, but breaks the use case the
plain vanilla rebase is used for, doesn't it?  You seem to ignore,
or perhaps you don't realize the existence of, the need of those who
want to flatten the history on top of the tip from the other side;
your statements in the pull --rebase[=preserve] thread may be
coming from the same place, I suspect.

For the flatten the history on top of that commit use case, two
conditions must be satisfied before the command can say the history
is already in the desired shape and nothing needs to be done to
allow it to make a short-cut.  It is not sufficient for the current
tip to be merely descendant of the tip from the other side (which is
the documentation bug we are fixing here).  The history between the
two commits need also to be linear, or it would not be flattening.

Punting when when only one of the two conditions is met and
requiring --force to perform what the user wanted to ask the
command to do does not fall into the reasonable category.

If your workflow does not involve the flattening plain vanilla
rebase, not using it is perfectly fine.  But please do not break
it for other people only because you do not use it yourself.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/4] ll-merge.c: refactor `read_merge_config()` to use `git_config_string()`

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

 There is one slight behavior change, previously merge.default
 silently ignored a NULL value and didn't raise any error. But,
 in the same function, all other values raise an error on a NULL
 value. So to conform with other call sites in Git, a NULL value
 for merge.default raises an error.

Better explained than v1 ;-)

 Signed-off-by: Tanay Abhra tanay...@gmail.com
 ---
 We cannot easily use the new config-set API here, because
 much of the function is dedicated to processing
 merge.name.variable which does not easily translate to
 the new API. If it were for variables like,
 merge.summary, merge.tool, and merge.verbosity, we
 could use the new API.

I think this comment belongs to the log message, if only to serve as
a reminder for us that the API needs to be made more usable when the
caller wants to use these three-level names, which are quite common.
This code path knows the name of a low-level merge driver and wants
to learn everything about that driver.  Another code path may know
the name of the branch and may want to scan branch.name.*.

  ll-merge.c | 23 ++-
  1 file changed, 6 insertions(+), 17 deletions(-)

 diff --git a/ll-merge.c b/ll-merge.c
 index fb61ea6..8ea03e5 100644
 --- a/ll-merge.c
 +++ b/ll-merge.c
 @@ -225,11 +225,8 @@ static int read_merge_config(const char *var, const char 
 *value, void *cb)
   const char *key, *name;
   int namelen;

 - if (!strcmp(var, merge.default)) {
 - if (value)
 - default_ll_merge = xstrdup(value);
 - return 0;
 - }
 + if (!strcmp(var, merge.default))
 + return git_config_string(default_ll_merge, var, value);

   /*
* We are not interested in anything but merge.name.variable;
 @@ -254,12 +251,8 @@ static int read_merge_config(const char *var, const char 
 *value, void *cb)
   ll_user_merge_tail = (fn-next);
   }

 - if (!strcmp(name, key)) {
 - if (!value)
 - return error(%s: lacks value, var);
 - fn-description = xstrdup(value);
 - return 0;
 - }
 + if (!strcmp(name, key))
 + return git_config_string(fn-description, var, value);

   if (!strcmp(driver, key)) {
   if (!value)
 @@ -285,12 +278,8 @@ static int read_merge_config(const char *var, const char 
 *value, void *cb)
   return 0;
   }

 - if (!strcmp(recursive, key)) {
 - if (!value)
 - return error(%s: lacks value, var);
 - fn-recursive = xstrdup(value);
 - return 0;
 - }
 + if (!strcmp(recursive, key))
 + return git_config_string(fn-recursive, var, value);

   return 0;
  }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] fast-import.c: replace `git_config()` with `git_config_get_*()` family

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

 Not that it's terribly important, but I think it's good that your
 refactoring also brings a few end-users benefits. It will help you show
 off when you tell your friends what you did this summer (not I did
 useless code churn ;-) ), and helps everybody see the benefits of your
 GSoC ;-).

;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC v2 1/2] git_default_config() rewritten using the config-set API

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

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

 git_default_config() now uses config-set API functions to query for
 values.

 I believe you missed a few spots:

 $ git grep -n 'git_default_config[^(]'
 Documentation/user-manual.txt:4287:git_config(git_default_config);
 archive.c:416:  git_config(git_default_config, NULL);
 builtin/config.c:577:   git_config(git_default_config, NULL);
 color.h:73: * if you are just going to change to git_default_config, too.
 fetch-pack.c:880:   git_config(git_default_config, NULL);
 http.c:393: config.cascade_fn = git_default_config;
 rerere.c:580:   git_config(git_default_config, NULL);
 rerere.c:710:   git_config(git_default_config, NULL);

 The following ones should probably be rewritten too:

 archive.c:416:  git_config(git_default_config, NULL);
 builtin/config.c:577:   git_config(git_default_config, NULL);
 fetch-pack.c:880:   git_config(git_default_config, NULL);
 rerere.c:580:   git_config(git_default_config, NULL);
 rerere.c:710:   git_config(git_default_config, NULL);

For a one-person toy project it is OK to repurpose the existing
git_default_config() to do completely different thing and make it a
flag day to switch the entire codebase, but in a collaborative
environment where there may be multiple topics in flight, some of
which may be happening where you are not even aware of, it is better
to remove the existing git_default_config() and use a different name
for the different function you are introducing, to force new places
that expect the old git_default_config() to work as before to be
noticed with a linkage error.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-imap-send: simplify tunnel construction

2014-08-13 Thread Bernhard Reiter

Signed-off-by: Bernhard Reiter ock...@raz.or.at
---
 imap-send.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


diff --git a/imap-send.c b/imap-send.c
index 524fbab..fb01a9c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -961,17 +961,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
 	/* open connection to IMAP server */
 
 	if (srvc-tunnel) {
-		const char *argv[] = { srvc-tunnel, NULL };
 		struct child_process tunnel = {NULL};
 
 		imap_info(Starting tunnel '%s'... , srvc-tunnel);
 
-		tunnel.argv = argv;
+		argv_array_push(tunnel.args, srvc-tunnel);
 		tunnel.use_shell = 1;
 		tunnel.in = -1;
 		tunnel.out = -1;
 		if (start_command(tunnel))
-			die(cannot start proxy %s, argv[0]);
+			die(cannot start proxy %s, srvc-tunnel);
 
 		imap-buf.sock.fd[0] = tunnel.out;
 		imap-buf.sock.fd[1] = tunnel.in;



[PATCH] http.c: die if curl_*_init fails

2014-08-13 Thread Bernhard Reiter

Signed-off-by: Bernhard Reiter ock...@raz.or.at
---
 http.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)


diff --git a/http.c b/http.c
index c8cd50d..afe4fc5 100644
--- a/http.c
+++ b/http.c
@@ -300,6 +300,9 @@ static CURL *get_curl_handle(void)
 {
 	CURL *result = curl_easy_init();
 
+	if (!result)
+		die(curl_easy_init failed);
+
 	if (!curl_ssl_verify) {
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0);
 		curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0);
@@ -399,7 +402,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 	git_config(urlmatch_config_entry, config);
 	free(normalized_url);
 
-	curl_global_init(CURL_GLOBAL_ALL);
+	if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+		die(curl_global_init failed);
 
 	http_proactive_auth = proactive_auth;
 



Re: [PATCH 3/3] checkout -m: attempt merge when deletion of path was staged

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

 twoway_merge() is missing an o-gently check in the case where a file
 that needs to be modified is missing from the index but present in the
 old and new trees.  As a result, in this case 'git checkout -m' errors
 out instead of trying to perform a merge.

I see two hunks in threeway_merge(), so two existing callers there
will not change their behaviour.  Two hunks in twoway_merge() means
that among three existing callers in that function, this one at the
end (not shown in your patch) changes its behaviour:

else if (newtree) {
if (oldtree  !o-initial_checkout) {
/*
 * deletion of the path was staged;
 */
if (same(oldtree, newtree))
return 1;
return reject_merge(oldtree, o);
}
return merged_entry(newtree, current, o);
}
return deleted_entry(oldtree, current, o);

 This is the most iffy of the three patches, mostly because I was too
 lazy to write a test.

You would trigger this codepath by jumping from an old revision to a
new revision after git rm $path any path that has been modified
between the two.  The only behaviour difference is that it will stop
issuing an error message---the checkout -m will successfully switch
between the revs and leave the index in a we modified, they removed
conflicting state with or without your patch.
--
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 3/3] checkout -m: attempt merge when deletion of path was staged

2014-08-13 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Jonathan Nieder jrnie...@gmail.com writes:

 twoway_merge() is missing an o-gently check in the case where a file
 that needs to be modified is missing from the index but present in the
 old and new trees.  As a result, in this case 'git checkout -m' errors
 out instead of trying to perform a merge.

 I see two hunks in threeway_merge(), so two existing callers there
 will not change their behaviour.  Two hunks in twoway_merge() means
 that among three existing callers in that function, this one at the
 end (not shown in your patch) changes its behaviour:

   else if (newtree) {
   if (oldtree  !o-initial_checkout) {
   /*
* deletion of the path was staged;
*/
   if (same(oldtree, newtree))
   return 1;
   return reject_merge(oldtree, o);
   }
   return merged_entry(newtree, current, o);
   }
   return deleted_entry(oldtree, current, o);

 This is the most iffy of the three patches, mostly because I was too
 lazy to write a test.

 You would trigger this codepath by jumping from an old revision to a
 new revision after git rm $path any path that has been modified
 between the two.  The only behaviour difference is that it will stop
 issuing an error message---the checkout -m will successfully switch
 between the revs and leave the index in a we modified, they removed
 conflicting state with or without your patch.

IOW, something like this perhaps?

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0c9ec0a..cedbb6a 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -223,6 +223,23 @@ test_expect_success 'checkout --merge --conflict=diff3 
branch' '
test_cmp two expect
 '
 
+test_expect_success 'switch to another branch while carrying a deletion' '
+
+   git checkout -f master  git reset --hard  git clean -f 
+   git rm two 
+
+   test_must_fail git checkout simple 2errs 
+   test_i18ngrep overwritten errs 
+
+   git checkout --merge simple 2errs 
+   ! test_i18ngrep overwritten errs 
+   git ls-files -u 
+   test_must_fail git cat-file -t :0:two 
+   test $(git cat-file -t :1:two) = blob 
+   test $(git cat-file -t :2:two) = blob 
+   test_must_fail git cat-file -t :3:two
+'
+
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 
git config advice.detachedHead false 
--
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-13 Thread Phil Hord
Thanks for working on this.

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

s/down/done


  git rebase --continue


I like the direction of this rewording, but it seems to hide the
instructions for the user who wants to keep the empty commit.

I'm not sure how to improve it.  Maybe this:

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

You may remove the squash commit now with

  git reset HEAD^

or keep it by doing nothing extra.

Continue the rebase with

  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.

This patch does _not_ silently throw away empty commits. I wonder if
such behavior could be triggered with something like --no-keep-empty.
Maybe that belongs in another patch.


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 

Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs

2014-08-13 Thread David Turner
On Wed, 2014-08-13 at 09:20 -0700, Ronnie Sahlberg wrote:
 David,
 
 One possible solution can be to use the external database daemon I am
 working of for ref transactions.
 Since this makes all refs be stored in a dedicated database instead of
 the filesystem you no longer are dependent on file system semantics.
 
 While not in the official git trees yet I would appreciate any testing
 and comments you may have it you want to test it.
 https://github.com/rsahlberg/git/tree/backend-struct-db-2
 
 Not for production use but seeing if it does build and works on your
 platforms would be great.

Thanks. We ultimately went with a custom hook (which also enforces other
business rules), but I'm very excited about the new backend and hope it
will improve performance generally.

One thing you could do to increase adoption would be to make a Homebrew
formula for tdb. Homebrew is the defacto standard Mac packaging system.
Sadly, a large number of engineers use Macs, so this is a large barrier
-- especially since the tdb build is somewhat baroque.

It would also be great if it didn't require a fresh clone; for a large
organization with large repos, requiring every engineer to re-clone is
somewhat onerous.  I assume it's straightforward to do a conversion from
an existing set of refs but that it just hasn't been a priority for you
yet.

Thanks so much for working on this!

--
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 3/3] checkout -m: attempt merge when deletion of path was staged

2014-08-13 Thread Johannes Sixt
Am 13.08.2014 20:59, schrieb Junio C Hamano:
 diff --git a/t/t7201-co.sh b/t/t7201-co.sh
 index 0c9ec0a..cedbb6a 100755
 --- a/t/t7201-co.sh
 +++ b/t/t7201-co.sh
 @@ -223,6 +223,23 @@ test_expect_success 'checkout --merge --conflict=diff3 
 branch' '
   test_cmp two expect
  '
  
 +test_expect_success 'switch to another branch while carrying a deletion' '
 +
 + git checkout -f master  git reset --hard  git clean -f 
 + git rm two 
 +
 + test_must_fail git checkout simple 2errs 
 + test_i18ngrep overwritten errs 
 +
 + git checkout --merge simple 2errs 
 + ! test_i18ngrep overwritten errs 

This must be written as

test_i18ngrep ! overwritten errs 

 + git ls-files -u 
 + test_must_fail git cat-file -t :0:two 
 + test $(git cat-file -t :1:two) = blob 
 + test $(git cat-file -t :2:two) = blob 
 + test_must_fail git cat-file -t :3:two
 +'
 +
  test_expect_success 'checkout to detach HEAD (with advice declined)' '
  
   git config advice.detachedHead false 
 

I see a few wrong usages in the current code base. Here's a fix.

--- 8 ---
Subject: [PATCH] tests: fix negated test_i18ngrep calls

The helper function test_i18ngrep pretends that it found the expected
results when it is running under GETTEXT_POISON. For this reason, it must
not be used negated like so

   ! test_i18ngrep foo bar

because the test case would fail under GETTEXT_POISON. The function offers
a special syntax to test that a pattern is *not* found:

   test_i18ngrep ! foo bar

Convert incorrect uses to this syntax.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 t/t4018-diff-funcname.sh | 8 
 t/t9800-git-p4-basic.sh  | 2 +-
 t/t9807-git-p4-submit.sh | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 34591c2..1dbaa38 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -52,15 +52,15 @@ do
echo *.java diff=$p .gitattributes 
test_expect_code 1 git diff --no-index \
A.java B.java 2msg 
-   ! test_i18ngrep fatal msg 
-   ! test_i18ngrep error msg
+   test_i18ngrep ! fatal msg 
+   test_i18ngrep ! error msg
'
test_expect_success builtin $p wordRegex pattern compiles '
echo *.java diff=$p .gitattributes 
test_expect_code 1 git diff --no-index --word-diff \
A.java B.java 2msg 
-   ! test_i18ngrep fatal msg 
-   ! test_i18ngrep error msg
+   test_i18ngrep ! fatal msg 
+   test_i18ngrep ! error msg
'
 done
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 665607c..5b56212 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -145,7 +145,7 @@ test_expect_success 'exit when p4 fails to produce 
marshaled output' '
test_expect_code 1 git p4 clone --dest=$git //depot errs 21
) 
cat errs 
-   ! test_i18ngrep Traceback errs
+   test_i18ngrep ! Traceback errs
 '
 
 # Hide a file from p4d, make sure we catch its complaint.  This won't fail in
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 7fab2ed..1f74a88 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -404,7 +404,7 @@ test_expect_success 'submit --prepare-p4-only' '
git p4 submit --prepare-p4-only out 
test_i18ngrep prepared for submission out 
test_i18ngrep must be deleted out 
-   ! test_i18ngrep everything below this line is just the diff 
out
+   test_i18ngrep ! everything below this line is just the diff 
out
) 
(
cd $cli 
-- 
2.0.0.12.gbcf935e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/6] diff --stat: mark any file larger than core.bigfilethreshold binary

2014-08-13 Thread Eric Sunshine
On Wed, Aug 13, 2014 at 6:57 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 Too large files may lead to failure to allocate memory. If it happens
 here, it could impact quite a few commands that involve
 diff. Moreover, too large files are inefficient to compare anyway (and
 most likely non-text), so mark them binary and skip looking at their
 content.

 Noticed-by: Dale R. Worley wor...@alum.mit.edu
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Documentation/config.txt|  3 ++-
  Documentation/gitattributes.txt |  4 ++--
  diff.c  | 26 ++
  diffcore.h  |  1 +
  t/t1050-large.sh|  4 
  5 files changed, 27 insertions(+), 11 deletions(-)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index c55c22a..53df40e 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -499,7 +499,8 @@ core.bigFileThreshold::
 Files larger than this size are stored deflated, without
 attempting delta compression.  Storing large files without
 delta compression avoids excessive memory usage, at the
 -   slight expense of increased disk usage.
 +   slight expense of increased disk usage. Additionally files
 +   larger than this size are allways treated as binary.

s/allways/always/

  Default is 512 MiB on all platforms.  This should be reasonable
  for most projects as source code and other text files can still
--
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 3/3] checkout -m: attempt merge when deletion of path was staged

2014-08-13 Thread Junio C Hamano
Johannes Sixt j...@kdbg.org writes:

 Am 13.08.2014 20:59, schrieb Junio C Hamano:
 diff --git a/t/t7201-co.sh b/t/t7201-co.sh
 index 0c9ec0a..cedbb6a 100755
 --- a/t/t7201-co.sh
 +++ b/t/t7201-co.sh
 @@ -223,6 +223,23 @@ test_expect_success 'checkout --merge --conflict=diff3 
 branch' '
  test_cmp two expect
  '
  
 +test_expect_success 'switch to another branch while carrying a deletion' '
 +
 +git checkout -f master  git reset --hard  git clean -f 
 +git rm two 
 +
 +test_must_fail git checkout simple 2errs 
 +test_i18ngrep overwritten errs 
 +
 +git checkout --merge simple 2errs 
 +! test_i18ngrep overwritten errs 

 This must be written as

   test_i18ngrep ! overwritten errs 

Oops. Thanks for spotting.

 I see a few wrong usages in the current code base. Here's a fix.

Will apply; thanks.

 --- 8 ---
 Subject: [PATCH] tests: fix negated test_i18ngrep calls

 The helper function test_i18ngrep pretends that it found the expected
 results when it is running under GETTEXT_POISON. For this reason, it must
 not be used negated like so

! test_i18ngrep foo bar

 because the test case would fail under GETTEXT_POISON. The function offers
 a special syntax to test that a pattern is *not* found:

test_i18ngrep ! foo bar

 Convert incorrect uses to this syntax.

 Signed-off-by: Johannes Sixt j...@kdbg.org
 ---
  t/t4018-diff-funcname.sh | 8 
  t/t9800-git-p4-basic.sh  | 2 +-
  t/t9807-git-p4-submit.sh | 2 +-
  3 files changed, 6 insertions(+), 6 deletions(-)

 diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
 index 34591c2..1dbaa38 100755
 --- a/t/t4018-diff-funcname.sh
 +++ b/t/t4018-diff-funcname.sh
 @@ -52,15 +52,15 @@ do
   echo *.java diff=$p .gitattributes 
   test_expect_code 1 git diff --no-index \
   A.java B.java 2msg 
 - ! test_i18ngrep fatal msg 
 - ! test_i18ngrep error msg
 + test_i18ngrep ! fatal msg 
 + test_i18ngrep ! error msg
   '
   test_expect_success builtin $p wordRegex pattern compiles '
   echo *.java diff=$p .gitattributes 
   test_expect_code 1 git diff --no-index --word-diff \
   A.java B.java 2msg 
 - ! test_i18ngrep fatal msg 
 - ! test_i18ngrep error msg
 + test_i18ngrep ! fatal msg 
 + test_i18ngrep ! error msg
   '
  done
  
 diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
 index 665607c..5b56212 100755
 --- a/t/t9800-git-p4-basic.sh
 +++ b/t/t9800-git-p4-basic.sh
 @@ -145,7 +145,7 @@ test_expect_success 'exit when p4 fails to produce 
 marshaled output' '
   test_expect_code 1 git p4 clone --dest=$git //depot errs 21
   ) 
   cat errs 
 - ! test_i18ngrep Traceback errs
 + test_i18ngrep ! Traceback errs
  '
  
  # Hide a file from p4d, make sure we catch its complaint.  This won't fail in
 diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
 index 7fab2ed..1f74a88 100755
 --- a/t/t9807-git-p4-submit.sh
 +++ b/t/t9807-git-p4-submit.sh
 @@ -404,7 +404,7 @@ test_expect_success 'submit --prepare-p4-only' '
   git p4 submit --prepare-p4-only out 
   test_i18ngrep prepared for submission out 
   test_i18ngrep must be deleted out 
 - ! test_i18ngrep everything below this line is just the diff 
 out
 + test_i18ngrep ! everything below this line is just the diff 
 out
   ) 
   (
   cd $cli 
--
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/23] refs-common.c: move delete_ref to the common code

2014-08-13 Thread Ronnie Sahlberg
This change moves delete_ref() to the refs-common.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 18 ++
 refs.c| 19 ---
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index cb884b2..71ad358 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -25,3 +25,21 @@ int update_ref(const char *action, const char *refname,
return 0;
 }
 
+int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
+{
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
+
+   transaction = transaction_begin(err);
+   if (!transaction ||
+   transaction_delete_sha1(transaction, refname, sha1, delopt,
+   sha1  !is_null_sha1(sha1), NULL, err) ||
+   transaction_commit(transaction, err)) {
+   error(%s, err.buf);
+   transaction_free(transaction);
+   strbuf_release(err);
+   return 1;
+   }
+   transaction_free(transaction);
+   return 0;
+}
diff --git a/refs.c b/refs.c
index eb66cf7..faf794c 100644
--- a/refs.c
+++ b/refs.c
@@ -2622,25 +2622,6 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
-{
-   struct ref_transaction *transaction;
-   struct strbuf err = STRBUF_INIT;
-
-   transaction = transaction_begin(err);
-   if (!transaction ||
-   transaction_delete_sha1(transaction, refname, sha1, delopt,
-   sha1  !is_null_sha1(sha1), NULL, err) ||
-   transaction_commit(transaction, err)) {
-   error(%s, err.buf);
-   transaction_free(transaction);
-   strbuf_release(err);
-   return 1;
-   }
-   transaction_free(transaction);
-   return 0;
-}
-
 struct rename_reflog_cb {
struct ref_transaction *transaction;
const char *refname;
-- 
2.0.1.556.g3edca4c

--
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/23] refs-common.c: create a file to host all common refs code

2014-08-13 Thread Ronnie Sahlberg
Create refs-common.c which will hold all backend agnostic refs code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 Makefile  | 1 +
 refs-common.c | 2 ++
 2 files changed, 3 insertions(+)
 create mode 100644 refs-common.c

diff --git a/Makefile b/Makefile
index 07ea105..7705136 100644
--- a/Makefile
+++ b/Makefile
@@ -858,6 +858,7 @@ LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
+LIB_OBJS += refs-common.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
 LIB_OBJS += rerere.o
diff --git a/refs-common.c b/refs-common.c
new file mode 100644
index 000..44d96d2
--- /dev/null
+++ b/refs-common.c
@@ -0,0 +1,2 @@
+/* common code for all ref backends */
+
-- 
2.0.1.556.g3edca4c

--
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/23] backend-struct-db

2014-08-13 Thread Ronnie Sahlberg
List, please review

This series is called backend-struct-db and is also available at
https://github.com/rsahlberg/git/tree/backend-struct-db

This series is built on and follows after the series
ref-transactions-send-pack


This series does not change any logic or behaviour but mainly just shuffles
code around and adds method pointers for the backend functions.

The first patch adds a new public function for checking if a refname is
available or not. This function is needed not because we want to have
different is_refname_available semantics for different backends, we don't,
but because its implementation is quite dependant on the backend type.

15 of the patches, the refs-common.c patches, focuses on moving all backend
agnostic refs functions to a common file. This file will contain all
backend agnostic refs functions.

The last 6 patches adds a backend structure with the methods we need to
describe a pluggable backend. Currently we only have one built in backend,
the current files based backend. These patches do not change any of the
behavior other than that we now call the methods through backend specific
wrapper functions rather than calling them directly.


At this stage we now have a defined set of methods needed for a refs
backend and we can start building and adding new types of ref backends
to git.


Version 2:
- Do not use C99 style initializers as suggested by David Turner.
- Make head_ref_namespaced a common function instead of a backend function


Ronnie Sahlberg (23):
  refs.c: create a public function for is_refname_available
  refs-common.c: create a file to host all common refs code
  refs-common.c: move update_ref to refs-common.c
  refs-common.c: move delete_ref to the common code
  refs-common.c: move rename_ref to the common code
  refs-common.c: move read_ref_at to the refs common file
  refs-common.c: move the hidden refs functions to the common code
  refs-common.c: move dwim and friend functions to refs common
  refs-common.c: move warn_if_dangling_symref* to refs-common
  refs-common.c: move read_ref, read_ref_full and ref_exists to common
  refs-common.c: move resolve_refdup to common
  refs-common.c: move check_refname_component to the common code
  refs-common.c: move is_branch to the common code
  refs-common.c: move names_conflict to the common code
  refs-common.c: move prettify_refname to the common code
  refs-common.c: move ref iterators to the common code
  refs-common.c: move head_ref_namespaced to the common file
  refs.c: add a backend method structure with transaction functions
  refs.c: add reflog backend methods
  refs.c: add methods for misc ref operations
  refs.c: add methods for head_ref*
  refs.c: add methods for the ref iterators
  refs-be-files.c: rename refs.c to refs-be-files.c

 Makefile|3 +-
 refs-be-files.c | 3323 
 refs-common.c   |  966 +
 refs.c  | 4082 ---
 refs.h  |  111 ++
 5 files changed, 4402 insertions(+), 4083 deletions(-)
 create mode 100644 refs-be-files.c
 create mode 100644 refs-common.c
 delete mode 100644 refs.c

-- 
2.0.1.556.g3edca4c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 16/23] refs-common.c: move ref iterators to the common code

2014-08-13 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 81 +++
 refs.c| 81 ---
 2 files changed, 81 insertions(+), 81 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index d8a295c..3b20db3 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -668,6 +668,87 @@ out:
return cp - refname;
 }
 
+/* The argument to filter_refs */
+struct ref_filter {
+   const char *pattern;
+   each_ref_fn *fn;
+   void *cb_data;
+};
+
+static int filter_refs(const char *refname, const unsigned char *sha1, int 
flags,
+  void *data)
+{
+   struct ref_filter *filter = (struct ref_filter *)data;
+   if (wildmatch(filter-pattern, refname, 0, NULL))
+   return 0;
+   return filter-fn(refname, sha1, flags, filter-cb_data);
+}
+
+int for_each_tag_ref(each_ref_fn fn, void *cb_data)
+{
+   return for_each_ref_in(refs/tags/, fn, cb_data);
+}
+
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+{
+   return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
+}
+
+int for_each_branch_ref(each_ref_fn fn, void *cb_data)
+{
+   return for_each_ref_in(refs/heads/, fn, cb_data);
+}
+
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+{
+   return for_each_ref_in_submodule(submodule, refs/heads/, fn, cb_data);
+}
+
+int for_each_remote_ref(each_ref_fn fn, void *cb_data)
+{
+   return for_each_ref_in(refs/remotes/, fn, cb_data);
+}
+
+int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+{
+   return for_each_ref_in_submodule(submodule, refs/remotes/, fn, 
cb_data);
+}
+
+int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
+   const char *prefix, void *cb_data)
+{
+   struct strbuf real_pattern = STRBUF_INIT;
+   struct ref_filter filter;
+   int ret;
+
+   if (!prefix  !starts_with(pattern, refs/))
+   strbuf_addstr(real_pattern, refs/);
+   else if (prefix)
+   strbuf_addstr(real_pattern, prefix);
+   strbuf_addstr(real_pattern, pattern);
+
+   if (!has_glob_specials(pattern)) {
+   /* Append implied '/' '*' if not present. */
+   if (real_pattern.buf[real_pattern.len - 1] != '/')
+   strbuf_addch(real_pattern, '/');
+   /* No need to check for '*', there is none. */
+   strbuf_addch(real_pattern, '*');
+   }
+
+   filter.pattern = real_pattern.buf;
+   filter.fn = fn;
+   filter.cb_data = cb_data;
+   ret = for_each_ref(filter_refs, filter);
+
+   strbuf_release(real_pattern);
+   return ret;
+}
+
+int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
+{
+   return for_each_glob_ref_in(fn, pattern, NULL, cb_data);
+}
+
 int check_refname_format(const char *refname, int flags)
 {
int component_len, component_count = 0;
diff --git a/refs.c b/refs.c
index fb9c614..9aa88ef 100644
--- a/refs.c
+++ b/refs.c
@@ -1377,22 +1377,6 @@ const char *resolve_ref_unsafe(const char *refname, 
unsigned char *sha1, int fla
}
 }
 
-/* The argument to filter_refs */
-struct ref_filter {
-   const char *pattern;
-   each_ref_fn *fn;
-   void *cb_data;
-};
-
-static int filter_refs(const char *refname, const unsigned char *sha1, int 
flags,
-  void *data)
-{
-   struct ref_filter *filter = (struct ref_filter *)data;
-   if (wildmatch(filter-pattern, refname, 0, NULL))
-   return 0;
-   return filter-fn(refname, sha1, flags, filter-cb_data);
-}
-
 enum peel_status {
/* object was peeled successfully: */
PEEL_PEELED = 0,
@@ -1646,36 +1630,6 @@ int for_each_ref_in_submodule(const char *submodule, 
const char *prefix,
return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
 
-int for_each_tag_ref(each_ref_fn fn, void *cb_data)
-{
-   return for_each_ref_in(refs/tags/, fn, cb_data);
-}
-
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
-{
-   return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
-}
-
-int for_each_branch_ref(each_ref_fn fn, void *cb_data)
-{
-   return for_each_ref_in(refs/heads/, fn, cb_data);
-}
-
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
-{
-   return for_each_ref_in_submodule(submodule, refs/heads/, fn, cb_data);
-}
-
-int for_each_remote_ref(each_ref_fn fn, void *cb_data)
-{
-   return for_each_ref_in(refs/remotes/, fn, cb_data);
-}
-
-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
-{
-   return for_each_ref_in_submodule(submodule, refs/remotes/, fn, 
cb_data);
-}
-
 int for_each_replace_ref(each_ref_fn fn, void 

[PATCH v2 12/23] refs-common.c: move check_refname_component to the common code

2014-08-13 Thread Ronnie Sahlberg
This function does not contain any backend specific code so we
can move it to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 110 ++
 refs.c| 110 --
 2 files changed, 110 insertions(+), 110 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 655a1a0..f8b79e0 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -571,3 +571,113 @@ char *resolve_refdup(const char *ref, unsigned char 
*sha1, int flags, int *ref_f
const char *ret = resolve_ref_unsafe(ref, sha1, flags, ref_flag);
return ret ? xstrdup(ret) : NULL;
 }
+
+/*
+ * How to handle various characters in refnames:
+ * 0: An acceptable character for refs
+ * 1: End-of-component
+ * 2: ., look for a preceding . to reject .. in refs
+ * 3: {, look for a preceding @ to reject @{ in refs
+ * 4: A bad character: ASCII control characters, ~, ^, : or SP
+ */
+static unsigned char refname_disposition[256] = {
+   1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+   4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+   4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,
+   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
+   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
+   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
+};
+
+/*
+ * Try to read one refname component from the front of refname.
+ * Return the length of the component found, or -1 if the component is
+ * not legal.  It is legal if it is something reasonable to have under
+ * .git/refs/; We do not like it if:
+ *
+ * - any path component of it begins with ., or
+ * - it has double dots .., or
+ * - it has ASCII control character, ~, ^, : or SP, anywhere, or
+ * - it ends with a /.
+ * - it ends with .lock
+ * - it contains a \ (backslash)
+ */
+static int check_refname_component(const char *refname, int flags)
+{
+   const char *cp;
+   char last = '\0';
+
+   for (cp = refname; ; cp++) {
+   int ch = *cp  255;
+   unsigned char disp = refname_disposition[ch];
+   switch (disp) {
+   case 1:
+   goto out;
+   case 2:
+   if (last == '.')
+   return -1; /* Refname contains ... */
+   break;
+   case 3:
+   if (last == '@')
+   return -1; /* Refname contains @{. */
+   break;
+   case 4:
+   return -1;
+   }
+   last = ch;
+   }
+out:
+   if (cp == refname)
+   return 0; /* Component has zero length. */
+   if (refname[0] == '.') {
+   if (!(flags  REFNAME_DOT_COMPONENT))
+   return -1; /* Component starts with '.'. */
+   /*
+* Even if leading dots are allowed, don't allow .
+* as a component (.. is prevented by a rule above).
+*/
+   if (refname[1] == '\0')
+   return -1; /* Component equals .. */
+   }
+   if (cp - refname = 5  !memcmp(cp - 5, .lock, 5))
+   return -1; /* Refname ends with .lock. */
+   return cp - refname;
+}
+
+int check_refname_format(const char *refname, int flags)
+{
+   int component_len, component_count = 0;
+
+   if (!strcmp(refname, @))
+   /* Refname is a single character '@'. */
+   return -1;
+
+   while (1) {
+   /* We are at the start of a path component. */
+   component_len = check_refname_component(refname, flags);
+   if (component_len = 0) {
+   if ((flags  REFNAME_REFSPEC_PATTERN) 
+   refname[0] == '*' 
+   (refname[1] == '\0' || refname[1] == 
'/')) {
+   /* Accept one wildcard as a full refname 
component. */
+   flags = ~REFNAME_REFSPEC_PATTERN;
+   component_len = 1;
+   } else {
+   return -1;
+   }
+   }
+   component_count++;
+   if (refname[component_len] == '\0')
+   break;
+   /* Skip to next component. */
+   refname += component_len + 1;
+   }
+
+   if (refname[component_len - 1] == '.')
+   return -1; /* Refname ends with '.'. */
+   if (!(flags  REFNAME_ALLOW_ONELEVEL)  component_count  2)
+   return -1; /* Refname has only one component. */
+   return 0;
+}
diff --git a/refs.c b/refs.c
index ed7bc61..55bced9 100644
--- a/refs.c
+++ b/refs.c
@@ -6,25 

[PATCH v2 18/23] refs.c: add a backend method structure with transaction functions

2014-08-13 Thread Ronnie Sahlberg
Add a ref structure for backend methods. Start by adding method pointers
for the transaction functions.

Rename the existing transaction functions to files_* and make them static.
Add new transaction functions that just pass through to the appropriate
methods for the backend.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 54 +++
 refs.c| 68 +++
 refs.h| 35 ++
 3 files changed, 130 insertions(+), 27 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index aafc4c8..e7cea02 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -799,3 +799,57 @@ int check_refname_format(const char *refname, int flags)
return -1; /* Refname has only one component. */
return 0;
 }
+
+/* backend functions */
+struct ref_transaction *transaction_begin(struct strbuf *err)
+{
+   return refs-transaction_begin(err);
+}
+
+int transaction_update_sha1(struct ref_transaction *transaction,
+   const char *refname, const unsigned char *new_sha1,
+   const unsigned char *old_sha1, int flags,
+   int have_old, const char *msg, struct strbuf *err)
+{
+   return refs-transaction_update_sha1(transaction, refname, new_sha1,
+old_sha1, flags, have_old, msg,
+err);
+}
+
+int transaction_create_sha1(struct ref_transaction *transaction,
+   const char *refname, const unsigned char *new_sha1,
+   int flags, const char *msg, struct strbuf *err)
+{
+   return refs-transaction_create_sha1(transaction, refname, new_sha1,
+flags, msg, err);
+}
+int transaction_delete_sha1(struct ref_transaction *transaction,
+   const char *refname, const unsigned char *old_sha1,
+   int flags, int have_old, const char *msg,
+   struct strbuf *err)
+{
+   return refs-transaction_delete_sha1(transaction, refname, old_sha1,
+flags, have_old, msg, err);
+}
+
+int transaction_update_reflog(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ struct reflog_committer_info *ci,
+ const char *msg, int flags,
+ struct strbuf *err)
+{
+   return refs-transaction_update_reflog(transaction, refname, new_sha1,
+  old_sha1, ci, msg, flags, err);
+}
+
+int transaction_commit(struct ref_transaction *transaction, struct strbuf *err)
+{
+   return refs-transaction_commit(transaction, err);
+}
+
+void transaction_free(struct ref_transaction *transaction)
+{
+   return refs-transaction_free(transaction);
+}
diff --git a/refs.c b/refs.c
index e58a7e1..27eafd0 100644
--- a/refs.c
+++ b/refs.c
@@ -2777,12 +2777,12 @@ struct ref_transaction {
enum ref_transaction_state state;
 };
 
-struct ref_transaction *transaction_begin(struct strbuf *err)
+static struct ref_transaction *files_transaction_begin(struct strbuf *err)
 {
return xcalloc(1, sizeof(struct ref_transaction));
 }
 
-void transaction_free(struct ref_transaction *transaction)
+static void files_transaction_free(struct ref_transaction *transaction)
 {
int i;
 
@@ -2812,13 +2812,13 @@ static struct ref_update *add_update(struct 
ref_transaction *transaction,
return update;
 }
 
-int transaction_update_reflog(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- const unsigned char *old_sha1,
- struct reflog_committer_info *ci,
- const char *msg, int flags,
- struct strbuf *err)
+static int files_transaction_update_reflog(struct ref_transaction *transaction,
+  const char *refname,
+  const unsigned char *new_sha1,
+  const unsigned char *old_sha1,
+  struct reflog_committer_info *ci,
+  const char *msg, int flags,
+  struct strbuf *err)
 {
struct ref_update *update;
int i;
@@ -2865,12 +2865,13 @@ int transaction_update_reflog(struct ref_transaction 
*transaction,
return 0;
 }
 
-int transaction_update_sha1(struct ref_transaction *transaction,
-   const 

[PATCH v2 08/23] refs-common.c: move dwim and friend functions to refs common

2014-08-13 Thread Ronnie Sahlberg
These functions do not contain any backend specific code so we can move
them to the common code and share across all backends.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 202 ++
 refs.c| 202 --
 2 files changed, 202 insertions(+), 202 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index c40fa96..ac081e1 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -293,3 +293,205 @@ int ref_is_hidden(const char *refname)
}
return 0;
 }
+
+static const char *ref_rev_parse_rules[] = {
+   %.*s,
+   refs/%.*s,
+   refs/tags/%.*s,
+   refs/heads/%.*s,
+   refs/remotes/%.*s,
+   refs/remotes/%.*s/HEAD,
+   NULL
+};
+
+int refname_match(const char *abbrev_name, const char *full_name)
+{
+   const char **p;
+   const int abbrev_name_len = strlen(abbrev_name);
+
+   for (p = ref_rev_parse_rules; *p; p++) {
+   if (!strcmp(full_name, mkpath(*p, abbrev_name_len, 
abbrev_name))) {
+   return 1;
+   }
+   }
+
+   return 0;
+}
+
+/*
+ * *string and *len will only be substituted, and *string returned (for
+ * later free()ing) if the string passed in is a magic short-hand form
+ * to name a branch.
+ */
+static char *substitute_branch_name(const char **string, int *len)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int ret = interpret_branch_name(*string, *len, buf);
+
+   if (ret == *len) {
+   size_t size;
+   *string = strbuf_detach(buf, size);
+   *len = size;
+   return (char *)*string;
+   }
+
+   return NULL;
+}
+
+int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
+{
+   char *last_branch = substitute_branch_name(str, len);
+   const char **p, *r;
+   int refs_found = 0;
+
+   *ref = NULL;
+   for (p = ref_rev_parse_rules; *p; p++) {
+   char fullref[PATH_MAX];
+   unsigned char sha1_from_ref[20];
+   unsigned char *this_result;
+   int flag;
+
+   this_result = refs_found ? sha1_from_ref : sha1;
+   mksnpath(fullref, sizeof(fullref), *p, len, str);
+   r = resolve_ref_unsafe(fullref, this_result,
+  RESOLVE_REF_READING, flag);
+   if (r) {
+   if (!refs_found++)
+   *ref = xstrdup(r);
+   if (!warn_ambiguous_refs)
+   break;
+   } else if ((flag  REF_ISSYMREF)  strcmp(fullref, HEAD)) {
+   warning(ignoring dangling symref %s., fullref);
+   } else if ((flag  REF_ISBROKEN)  strchr(fullref, '/')) {
+   warning(ignoring broken ref %s., fullref);
+   }
+   }
+   free(last_branch);
+   return refs_found;
+}
+
+int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
+{
+   char *last_branch = substitute_branch_name(str, len);
+   const char **p;
+   int logs_found = 0;
+
+   *log = NULL;
+   for (p = ref_rev_parse_rules; *p; p++) {
+   unsigned char hash[20];
+   char path[PATH_MAX];
+   const char *ref, *it;
+
+   mksnpath(path, sizeof(path), *p, len, str);
+   ref = resolve_ref_unsafe(path, hash, RESOLVE_REF_READING, NULL);
+   if (!ref)
+   continue;
+   if (reflog_exists(path))
+   it = path;
+   else if (strcmp(ref, path)  reflog_exists(ref))
+   it = ref;
+   else
+   continue;
+   if (!logs_found++) {
+   *log = xstrdup(it);
+   hashcpy(sha1, hash);
+   }
+   if (!warn_ambiguous_refs)
+   break;
+   }
+   free(last_branch);
+   return logs_found;
+}
+
+char *shorten_unambiguous_ref(const char *refname, int strict)
+{
+   int i;
+   static char **scanf_fmts;
+   static int nr_rules;
+   char *short_name;
+
+   if (!nr_rules) {
+   /*
+* Pre-generate scanf formats from ref_rev_parse_rules[].
+* Generate a format suitable for scanf from a
+* ref_rev_parse_rules rule by interpolating %s at the
+* location of the %.*s.
+*/
+   size_t total_len = 0;
+   size_t offset = 0;
+
+   /* the rule list is NULL terminated, count them first */
+   for (nr_rules = 0; ref_rev_parse_rules[nr_rules]; nr_rules++)
+   /* -2 for strlen(%.*s) - strlen(%s); +1 for NUL */
+   total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 
+ 1;
+
+   

[PATCH v2 19/23] refs.c: add reflog backend methods

2014-08-13 Thread Ronnie Sahlberg
Add methods for the reflog functions.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 32 
 refs.c| 18 --
 refs.h| 17 +
 3 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index e7cea02..68152d6 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -853,3 +853,35 @@ void transaction_free(struct ref_transaction *transaction)
 {
return refs-transaction_free(transaction);
 }
+
+int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
+   void *cb_data)
+{
+   return refs-for_each_reflog_ent_reverse(refname, fn, cb_data);
+}
+
+int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
+   void *cb_data)
+{
+   return refs-for_each_reflog_ent(refname, fn, cb_data);
+}
+
+int for_each_reflog(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_reflog(fn, cb_data);
+}
+
+int reflog_exists(const char *refname)
+{
+   return refs-reflog_exists(refname);
+}
+
+int create_reflog(const char *refname)
+{
+   return refs-create_reflog(refname);
+}
+
+int delete_reflog(const char *refname)
+{
+   return refs-delete_reflog(refname);
+}
diff --git a/refs.c b/refs.c
index 27eafd0..699b548 100644
--- a/refs.c
+++ b/refs.c
@@ -2251,7 +2251,7 @@ static int copy_msg(char *buf, const char *msg)
 }
 
 /* This function must set a meaningful errno on failure */
-int create_reflog(const char *refname)
+static int files_create_reflog(const char *refname)
 {
int logfd, oflags = O_APPEND | O_WRONLY;
char logfile[PATH_MAX];
@@ -2516,7 +2516,7 @@ int create_symref(const char *ref_target, const char 
*refs_heads_master,
return 0;
 }
 
-int reflog_exists(const char *refname)
+static int files_reflog_exists(const char *refname)
 {
struct stat st;
 
@@ -2524,7 +2524,7 @@ int reflog_exists(const char *refname)
S_ISREG(st.st_mode);
 }
 
-int delete_reflog(const char *refname)
+static int files_delete_reflog(const char *refname)
 {
return remove_path(git_path(logs/%s, refname));
 }
@@ -2568,7 +2568,7 @@ static char *find_beginning_of_line(char *bob, char *scan)
return scan;
 }
 
-int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, 
void *cb_data)
+static int files_for_each_reflog_ent_reverse(const char *refname, 
each_reflog_ent_fn fn, void *cb_data)
 {
struct strbuf sb = STRBUF_INIT;
FILE *logfp;
@@ -2645,7 +2645,7 @@ int for_each_reflog_ent_reverse(const char *refname, 
each_reflog_ent_fn fn, void
return ret;
 }
 
-int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void 
*cb_data)
+static int files_for_each_reflog_ent(const char *refname, each_reflog_ent_fn 
fn, void *cb_data)
 {
FILE *logfp;
struct strbuf sb = STRBUF_INIT;
@@ -2706,7 +2706,7 @@ static int do_for_each_reflog(struct strbuf *name, 
each_ref_fn fn, void *cb_data
return retval;
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+static int files_for_each_reflog(each_ref_fn fn, void *cb_data)
 {
int retval;
struct strbuf name;
@@ -3295,6 +3295,12 @@ struct ref_be refs_files = {
files_transaction_update_reflog,
files_transaction_commit,
files_transaction_free,
+   files_for_each_reflog_ent,
+   files_for_each_reflog_ent_reverse,
+   files_for_each_reflog,
+   files_reflog_exists,
+   files_create_reflog,
+   files_delete_reflog,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.h b/refs.h
index 4b669f5..302eb03 100644
--- a/refs.h
+++ b/refs.h
@@ -373,6 +373,17 @@ typedef int (*transaction_commit_fn)(struct 
ref_transaction *transaction,
   struct strbuf *err);
 typedef void (*transaction_free_fn)(struct ref_transaction *transaction);
 
+typedef int (*for_each_reflog_ent_fn)(const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data);
+typedef int (*for_each_reflog_ent_reverse_fn)(const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data);
+typedef int (*for_each_reflog_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*reflog_exists_fn)(const char *refname);
+typedef int (*create_reflog_fn)(const char *refname);
+typedef int (*delete_reflog_fn)(const char *refname);
+
 struct ref_be {
transaction_begin_fn transaction_begin;
transaction_update_sha1_fn transaction_update_sha1;
@@ -381,6 +392,12 @@ struct ref_be {
transaction_update_reflog_fn transaction_update_reflog;
transaction_commit_fn transaction_commit;
transaction_free_fn transaction_free;
+   for_each_reflog_ent_fn for_each_reflog_ent;
+   for_each_reflog_ent_reverse_fn 

[PATCH v2 05/23] refs-common.c: move rename_ref to the common code

2014-08-13 Thread Ronnie Sahlberg
This change moves rename_ref() to the refs-common.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 92 +++
 refs.c| 92 ---
 2 files changed, 92 insertions(+), 92 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 71ad358..f99d83e 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -43,3 +43,95 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
transaction_free(transaction);
return 0;
 }
+
+struct rename_reflog_cb {
+   struct ref_transaction *transaction;
+   const char *refname;
+   struct strbuf *err;
+};
+
+static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+const char *id, unsigned long timestamp, int tz,
+const char *message, void *cb_data)
+{
+   struct rename_reflog_cb *cb = cb_data;
+   struct reflog_committer_info ci;
+
+   memset(ci, 0, sizeof(ci));
+   ci.id = id;
+   ci.timestamp = timestamp;
+   ci.tz = tz;
+   return transaction_update_reflog(cb-transaction, cb-refname,
+nsha1, osha1, ci, message, 0,
+cb-err);
+}
+
+int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
+{
+   unsigned char sha1[20];
+   int flag = 0, log;
+   struct ref_transaction *transaction = NULL;
+   struct strbuf err = STRBUF_INIT;
+   const char *symref = NULL;
+   struct rename_reflog_cb cb;
+   struct reflog_committer_info ci;
+
+   memset(ci, 0, sizeof(ci));
+   ci.committer_info = git_committer_info(0);
+
+   symref = resolve_ref_unsafe(oldrefname, sha1,
+   RESOLVE_REF_READING, flag);
+   if (flag  REF_ISSYMREF) {
+   error(refname %s is a symbolic ref, renaming it is not 
supported,
+   oldrefname);
+   return 1;
+   }
+   if (!symref) {
+   error(refname %s not found, oldrefname);
+   return 1;
+   }
+
+   if (!is_refname_available(newrefname, oldrefname, 1))
+   return 1;
+
+   log = reflog_exists(oldrefname);
+   transaction = transaction_begin(err);
+   if (!transaction)
+   goto fail;
+
+   if (strcmp(oldrefname, newrefname)) {
+   if (log  transaction_update_reflog(transaction, newrefname,
+sha1, sha1, ci, NULL,
+REFLOG_TRUNCATE, err))
+   goto fail;
+   cb.transaction = transaction;
+   cb.refname = newrefname;
+   cb.err = err;
+   if (log  for_each_reflog_ent(oldrefname, rename_reflog_ent,
+  cb))
+   goto fail;
+
+   if (transaction_delete_sha1(transaction, oldrefname, sha1,
+   REF_NODEREF,
+   1, NULL, err))
+   goto fail;
+   }
+   if (transaction_update_sha1(transaction, newrefname, sha1,
+   NULL, 0, 0, NULL, err))
+   goto fail;
+   if (log  transaction_update_reflog(transaction, newrefname, sha1,
+sha1, ci, logmsg,
+REFLOG_COMMITTER_INFO_IS_VALID,
+err))
+   goto fail;
+   if (transaction_commit(transaction, err))
+   goto fail;
+   transaction_free(transaction);
+   return 0;
+
+ fail:
+   error(rename_ref failed: %s, err.buf);
+   strbuf_release(err);
+   transaction_free(transaction);
+   return 1;
+}
diff --git a/refs.c b/refs.c
index faf794c..7d579be 100644
--- a/refs.c
+++ b/refs.c
@@ -2622,98 +2622,6 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-struct rename_reflog_cb {
-   struct ref_transaction *transaction;
-   const char *refname;
-   struct strbuf *err;
-};
-
-static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-const char *id, unsigned long timestamp, int tz,
-const char *message, void *cb_data)
-{
-   struct rename_reflog_cb *cb = cb_data;
-   struct reflog_committer_info ci;
-
-   memset(ci, 0, sizeof(ci));
-   ci.id = id;
-   ci.timestamp = timestamp;
-   ci.tz = tz;
-   return transaction_update_reflog(cb-transaction, cb-refname,
-nsha1, osha1, ci, message, 0,
-  

[PATCH v2 20/23] refs.c: add methods for misc ref operations

2014-08-13 Thread Ronnie Sahlberg
Add ref backend methods for:
resolve_ref_unsafe, is_refname_available, pack_refs, peel_ref,
create_symref, resolve_gitlink_ref.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 33 +
 refs.c| 22 +++---
 refs.h| 19 +++
 3 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 68152d6..d9688e2 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -885,3 +885,36 @@ int delete_reflog(const char *refname)
 {
return refs-delete_reflog(refname);
 }
+
+const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
+  int reading, int *flag)
+{
+   return refs-resolve_ref_unsafe(ref, sha1, reading, flag);
+}
+
+int is_refname_available(const char *refname, const char **skip, int skipnum)
+{
+   return refs-is_refname_available(refname, skip, skipnum);
+}
+
+int pack_refs(unsigned int flags, struct strbuf *err)
+{
+   return refs-pack_refs(flags, err);
+}
+
+int peel_ref(const char *refname, unsigned char *sha1)
+{
+   return refs-peel_ref(refname, sha1);
+}
+
+int create_symref(const char *ref_target, const char *refs_heads_master,
+ const char *logmsg)
+{
+   return refs-create_symref(ref_target, refs_heads_master, logmsg);
+}
+
+int resolve_gitlink_ref(const char *path, const char *refname,
+   unsigned char *sha1)
+{
+   return refs-resolve_gitlink_ref(path, refname, sha1);
+}
diff --git a/refs.c b/refs.c
index 699b548..9439809 100644
--- a/refs.c
+++ b/refs.c
@@ -1114,7 +1114,8 @@ static struct ref_dir *get_loose_refs(struct ref_cache 
*refs)
return get_ref_dir(refs-loose);
 }
 
-int is_refname_available(const char *refname, const char **skip, int skipnum)
+static int files_is_refname_available(const char *refname, const char **skip,
+ int skipnum)
 {
if (!is_refname_available_dir(refname, get_packed_refs(ref_cache),
  skip, skipnum))
@@ -1188,7 +1189,7 @@ static int resolve_gitlink_ref_recursive(struct ref_cache 
*refs,
return resolve_gitlink_ref_recursive(refs, p, sha1, recursion+1);
 }
 
-int resolve_gitlink_ref(const char *path, const char *refname, unsigned char 
*sha1)
+static int files_resolve_gitlink_ref(const char *path, const char *refname, 
unsigned char *sha1)
 {
int len = strlen(path), retval;
char *submodule;
@@ -1247,7 +1248,7 @@ static const char *handle_missing_loose_ref(const char 
*refname,
 }
 
 /* This function needs to return a meaningful errno on failure */
-const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int 
flags, int *ref_flag)
+static const char *files_resolve_ref_unsafe(const char *refname, unsigned char 
*sha1, int flags, int *ref_flag)
 {
int depth = MAXDEPTH;
ssize_t len;
@@ -1466,7 +1467,7 @@ static enum peel_status peel_entry(struct ref_entry 
*entry, int repeel)
return status;
 }
 
-int peel_ref(const char *refname, unsigned char *sha1)
+static int files_peel_ref(const char *refname, unsigned char *sha1)
 {
int flag;
unsigned char base[20];
@@ -2080,7 +2081,7 @@ static void prune_refs(struct ref_to_prune *r)
}
 }
 
-int pack_refs(unsigned int flags, struct strbuf *err)
+static int files_pack_refs(unsigned int flags, struct strbuf *err)
 {
struct pack_refs_cb_data cbdata;
 
@@ -2453,8 +2454,9 @@ static int write_ref_sha1(struct ref_lock *lock,
return 0;
 }
 
-int create_symref(const char *ref_target, const char *refs_heads_master,
- const char *logmsg)
+static int files_create_symref(const char *ref_target,
+  const char *refs_heads_master,
+  const char *logmsg)
 {
const char *lockpath;
char ref[1000];
@@ -3301,6 +3303,12 @@ struct ref_be refs_files = {
files_reflog_exists,
files_create_reflog,
files_delete_reflog,
+   files_resolve_ref_unsafe,
+   files_is_refname_available,
+   files_pack_refs,
+   files_peel_ref,
+   files_create_symref,
+   files_resolve_gitlink_ref,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.h b/refs.h
index 302eb03..ab120c5 100644
--- a/refs.h
+++ b/refs.h
@@ -384,6 +384,19 @@ typedef int (*reflog_exists_fn)(const char *refname);
 typedef int (*create_reflog_fn)(const char *refname);
 typedef int (*delete_reflog_fn)(const char *refname);
 
+typedef const char *(*resolve_ref_unsafe_fn)(const char *ref,
+   unsigned char *sha1, int reading, int *flag);
+
+typedef int (*is_refname_available_fn)(const char *refname, const char **skip,
+  int skipnum);
+typedef int (*pack_refs_fn)(unsigned int flags, struct strbuf *err);
+typedef int (*peel_ref_fn)(const char *refname, unsigned char *sha1);
+typedef int 

[PATCH v2 21/23] refs.c: add methods for head_ref*

2014-08-13 Thread Ronnie Sahlberg
Add methods for the head_ref* functions.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 10 ++
 refs.c|  6 --
 refs.h|  6 ++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index d9688e2..f19402b 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -918,3 +918,13 @@ int resolve_gitlink_ref(const char *path, const char 
*refname,
 {
return refs-resolve_gitlink_ref(path, refname, sha1);
 }
+
+int head_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-head_ref(fn, cb_data);
+}
+
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+   return refs-head_ref_submodule(submodule, fn, cb_data);
+}
diff --git a/refs.c b/refs.c
index 9439809..7ad07f1 100644
--- a/refs.c
+++ b/refs.c
@@ -1600,12 +1600,12 @@ static int do_head_ref(const char *submodule, 
each_ref_fn fn, void *cb_data)
return 0;
 }
 
-int head_ref(each_ref_fn fn, void *cb_data)
+static int files_head_ref(each_ref_fn fn, void *cb_data)
 {
return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+static int files_head_ref_submodule(const char *submodule, each_ref_fn fn, 
void *cb_data)
 {
return do_head_ref(submodule, fn, cb_data);
 }
@@ -3309,6 +3309,8 @@ struct ref_be refs_files = {
files_peel_ref,
files_create_symref,
files_resolve_gitlink_ref,
+   files_head_ref,
+   files_head_ref_submodule,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.h b/refs.h
index ab120c5..420e9b5 100644
--- a/refs.h
+++ b/refs.h
@@ -397,6 +397,10 @@ typedef int (*create_symref_fn)(const char *ref_target,
 typedef int (*resolve_gitlink_ref_fn)(const char *path, const char *refname,
  unsigned char *sha1);
 
+typedef int (*head_ref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*head_ref_submodule_fn)(const char *submodule, each_ref_fn fn,
+void *cb_data);
+
 struct ref_be {
transaction_begin_fn transaction_begin;
transaction_update_sha1_fn transaction_update_sha1;
@@ -417,6 +421,8 @@ struct ref_be {
peel_ref_fn peel_ref;
create_symref_fn create_symref;
resolve_gitlink_ref_fn resolve_gitlink_ref;
+   head_ref_fn head_ref;
+   head_ref_submodule_fn head_ref_submodule;
 };
 
 extern struct ref_be *refs;
-- 
2.0.1.556.g3edca4c

--
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/23] refs-common.c: move the hidden refs functions to the common code

2014-08-13 Thread Ronnie Sahlberg
This change moves the hidden refs functions to the refs-common.c file since
these functions do not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 44 
 refs.c| 43 ---
 2 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 3d7354e..c40fa96 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -1,6 +1,7 @@
 /* common code for all ref backends */
 #include cache.h
 #include refs.h
+#include string-list.h
 
 int update_ref(const char *action, const char *refname,
   const unsigned char *sha1, const unsigned char *oldval,
@@ -249,3 +250,46 @@ int read_ref_at(const char *refname, unsigned long 
at_time, int cnt,
 
return 1;
 }
+
+static struct string_list *hide_refs;
+
+int parse_hide_refs_config(const char *var, const char *value, const char 
*section)
+{
+   if (!strcmp(transfer.hiderefs, var) ||
+   /* NEEDSWORK: use parse_config_key() once both are merged */
+   (starts_with(var, section)  var[strlen(section)] == '.' 
+!strcmp(var + strlen(section), .hiderefs))) {
+   char *ref;
+   int len;
+
+   if (!value)
+   return config_error_nonbool(var);
+   ref = xstrdup(value);
+   len = strlen(ref);
+   while (len  ref[len - 1] == '/')
+   ref[--len] = '\0';
+   if (!hide_refs) {
+   hide_refs = xcalloc(1, sizeof(*hide_refs));
+   hide_refs-strdup_strings = 1;
+   }
+   string_list_append(hide_refs, ref);
+   }
+   return 0;
+}
+
+int ref_is_hidden(const char *refname)
+{
+   struct string_list_item *item;
+
+   if (!hide_refs)
+   return 0;
+   for_each_string_list_item(item, hide_refs) {
+   int len;
+   if (!starts_with(refname, item-string))
+   continue;
+   len = strlen(item-string);
+   if (!refname[len] || refname[len] == '/')
+   return 1;
+   }
+   return 0;
+}
diff --git a/refs.c b/refs.c
index 52ca0bb..6181edf 100644
--- a/refs.c
+++ b/refs.c
@@ -3796,46 +3796,3 @@ char *shorten_unambiguous_ref(const char *refname, int 
strict)
free(short_name);
return xstrdup(refname);
 }
-
-static struct string_list *hide_refs;
-
-int parse_hide_refs_config(const char *var, const char *value, const char 
*section)
-{
-   if (!strcmp(transfer.hiderefs, var) ||
-   /* NEEDSWORK: use parse_config_key() once both are merged */
-   (starts_with(var, section)  var[strlen(section)] == '.' 
-!strcmp(var + strlen(section), .hiderefs))) {
-   char *ref;
-   int len;
-
-   if (!value)
-   return config_error_nonbool(var);
-   ref = xstrdup(value);
-   len = strlen(ref);
-   while (len  ref[len - 1] == '/')
-   ref[--len] = '\0';
-   if (!hide_refs) {
-   hide_refs = xcalloc(1, sizeof(*hide_refs));
-   hide_refs-strdup_strings = 1;
-   }
-   string_list_append(hide_refs, ref);
-   }
-   return 0;
-}
-
-int ref_is_hidden(const char *refname)
-{
-   struct string_list_item *item;
-
-   if (!hide_refs)
-   return 0;
-   for_each_string_list_item(item, hide_refs) {
-   int len;
-   if (!starts_with(refname, item-string))
-   continue;
-   len = strlen(item-string);
-   if (!refname[len] || refname[len] == '/')
-   return 1;
-   }
-   return 0;
-}
-- 
2.0.1.556.g3edca4c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 17/23] refs-common.c: move head_ref_namespaced to the common file

2014-08-13 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 15 +++
 refs.c| 15 ---
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 3b20db3..aafc4c8 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -570,6 +570,21 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, 
const struct string_li
for_each_rawref(warn_if_dangling_symref, data);
 }
 
+int head_ref_namespaced(each_ref_fn fn, void *cb_data)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int ret = 0;
+   unsigned char sha1[20];
+   int flag;
+
+   strbuf_addf(buf, %sHEAD, get_git_namespace());
+   if (!read_ref_full(buf.buf, sha1, RESOLVE_REF_READING, flag))
+   ret = fn(buf.buf, sha1, flag, cb_data);
+   strbuf_release(buf);
+
+   return ret;
+}
+
 int read_ref_full(const char *refname, unsigned char *sha1, int flags, int 
*ref_flag)
 {
if (resolve_ref_unsafe(refname, sha1, flags, ref_flag))
diff --git a/refs.c b/refs.c
index 9aa88ef..e58a7e1 100644
--- a/refs.c
+++ b/refs.c
@@ -1635,21 +1635,6 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
 }
 
-int head_ref_namespaced(each_ref_fn fn, void *cb_data)
-{
-   struct strbuf buf = STRBUF_INIT;
-   int ret = 0;
-   unsigned char sha1[20];
-   int flag;
-
-   strbuf_addf(buf, %sHEAD, get_git_namespace());
-   if (!read_ref_full(buf.buf, sha1, RESOLVE_REF_READING, flag))
-   ret = fn(buf.buf, sha1, flag, cb_data);
-   strbuf_release(buf);
-
-   return ret;
-}
-
 int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
-- 
2.0.1.556.g3edca4c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 13/23] refs-common.c: move is_branch to the common code

2014-08-13 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 5 +
 refs.c| 5 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index f8b79e0..5f83d7e 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -3,6 +3,11 @@
 #include refs.h
 #include string-list.h
 
+int is_branch(const char *refname)
+{
+   return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
+}
+
 int update_ref(const char *action, const char *refname,
   const unsigned char *sha1, const unsigned char *oldval,
   int flags, struct strbuf *e)
diff --git a/refs.c b/refs.c
index 55bced9..70c034c 100644
--- a/refs.c
+++ b/refs.c
@@ -2483,11 +2483,6 @@ static int log_ref_write(const char *refname, const 
unsigned char *old_sha1,
return 0;
 }
 
-int is_branch(const char *refname)
-{
-   return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
-}
-
 static int write_sha1_update_reflog(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
 {
-- 
2.0.1.556.g3edca4c

--
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/23] refs-common.c: move warn_if_dangling_symref* to refs-common

2014-08-13 Thread Ronnie Sahlberg
These functions do not use any backend specific code so we can move
them to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 52 
 refs.c| 52 
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index ac081e1..ab3a118 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -495,3 +495,55 @@ char *shorten_unambiguous_ref(const char *refname, int 
strict)
free(short_name);
return xstrdup(refname);
 }
+
+struct warn_if_dangling_data {
+   FILE *fp;
+   const char *refname;
+   const struct string_list *refnames;
+   const char *msg_fmt;
+};
+
+static int warn_if_dangling_symref(const char *refname, const unsigned char 
*sha1,
+  int flags, void *cb_data)
+{
+   struct warn_if_dangling_data *d = cb_data;
+   const char *resolves_to;
+   unsigned char junk[20];
+
+   if (!(flags  REF_ISSYMREF))
+   return 0;
+
+   resolves_to = resolve_ref_unsafe(refname, junk, 0, NULL);
+   if (!resolves_to
+   || (d-refname
+   ? strcmp(resolves_to, d-refname)
+   : !string_list_has_string(d-refnames, resolves_to))) {
+   return 0;
+   }
+
+   fprintf(d-fp, d-msg_fmt, refname);
+   fputc('\n', d-fp);
+   return 0;
+}
+
+void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
+{
+   struct warn_if_dangling_data data;
+
+   data.fp = fp;
+   data.refname = refname;
+   data.refnames = NULL;
+   data.msg_fmt = msg_fmt;
+   for_each_rawref(warn_if_dangling_symref, data);
+}
+
+void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct 
string_list *refnames)
+{
+   struct warn_if_dangling_data data;
+
+   data.fp = fp;
+   data.refname = NULL;
+   data.refnames = refnames;
+   data.msg_fmt = msg_fmt;
+   for_each_rawref(warn_if_dangling_symref, data);
+}
diff --git a/refs.c b/refs.c
index 56e146f..40c329b 100644
--- a/refs.c
+++ b/refs.c
@@ -1667,58 +1667,6 @@ int peel_ref(const char *refname, unsigned char *sha1)
return peel_object(base, sha1);
 }
 
-struct warn_if_dangling_data {
-   FILE *fp;
-   const char *refname;
-   const struct string_list *refnames;
-   const char *msg_fmt;
-};
-
-static int warn_if_dangling_symref(const char *refname, const unsigned char 
*sha1,
-  int flags, void *cb_data)
-{
-   struct warn_if_dangling_data *d = cb_data;
-   const char *resolves_to;
-   unsigned char junk[20];
-
-   if (!(flags  REF_ISSYMREF))
-   return 0;
-
-   resolves_to = resolve_ref_unsafe(refname, junk, 0, NULL);
-   if (!resolves_to
-   || (d-refname
-   ? strcmp(resolves_to, d-refname)
-   : !string_list_has_string(d-refnames, resolves_to))) {
-   return 0;
-   }
-
-   fprintf(d-fp, d-msg_fmt, refname);
-   fputc('\n', d-fp);
-   return 0;
-}
-
-void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
-{
-   struct warn_if_dangling_data data;
-
-   data.fp = fp;
-   data.refname = refname;
-   data.refnames = NULL;
-   data.msg_fmt = msg_fmt;
-   for_each_rawref(warn_if_dangling_symref, data);
-}
-
-void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct 
string_list *refnames)
-{
-   struct warn_if_dangling_data data;
-
-   data.fp = fp;
-   data.refname = NULL;
-   data.refnames = refnames;
-   data.msg_fmt = msg_fmt;
-   for_each_rawref(warn_if_dangling_symref, data);
-}
-
 /*
  * Call fn for each reference in the specified ref_cache, omitting
  * references not in the containing_dir of base.  fn is called for all
-- 
2.0.1.556.g3edca4c

--
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/23] refs-common.c: move read_ref_at to the refs common file

2014-08-13 Thread Ronnie Sahlberg
This change moves read_ref_at() to the refs-common.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 114 ++
 refs.c| 114 --
 2 files changed, 114 insertions(+), 114 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index f99d83e..3d7354e 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -135,3 +135,117 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
transaction_free(transaction);
return 1;
 }
+
+struct read_ref_at_cb {
+   const char *refname;
+   unsigned long at_time;
+   int cnt;
+   int reccnt;
+   unsigned char *sha1;
+   int found_it;
+
+   unsigned char osha1[20];
+   unsigned char nsha1[20];
+   int tz;
+   unsigned long date;
+   char **msg;
+   unsigned long *cutoff_time;
+   int *cutoff_tz;
+   int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+   const char *id, unsigned long timestamp, int tz,
+   const char *message, void *cb_data)
+{
+   struct read_ref_at_cb *cb = cb_data;
+
+   cb-reccnt++;
+   cb-tz = tz;
+   cb-date = timestamp;
+
+   if (timestamp = cb-at_time || cb-cnt == 0) {
+   if (cb-msg)
+   *cb-msg = xstrdup(message);
+   if (cb-cutoff_time)
+   *cb-cutoff_time = timestamp;
+   if (cb-cutoff_tz)
+   *cb-cutoff_tz = tz;
+   if (cb-cutoff_cnt)
+   *cb-cutoff_cnt = cb-reccnt - 1;
+   /*
+* we have not yet updated cb-[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+   if (!is_null_sha1(cb-osha1)) {
+   hashcpy(cb-sha1, nsha1);
+   if (hashcmp(cb-osha1, nsha1))
+   warning(Log for ref %s has gap after %s.,
+   cb-refname, show_date(cb-date, 
cb-tz, DATE_RFC2822));
+   }
+   else if (cb-date == cb-at_time)
+   hashcpy(cb-sha1, nsha1);
+   else if (hashcmp(nsha1, cb-sha1))
+   warning(Log for ref %s unexpectedly ended on %s.,
+   cb-refname, show_date(cb-date, cb-tz,
+  DATE_RFC2822));
+   hashcpy(cb-osha1, osha1);
+   hashcpy(cb-nsha1, nsha1);
+   cb-found_it = 1;
+   return 1;
+   }
+   hashcpy(cb-osha1, osha1);
+   hashcpy(cb-nsha1, nsha1);
+   if (cb-cnt  0)
+   cb-cnt--;
+   return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *id, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+   struct read_ref_at_cb *cb = cb_data;
+
+   if (cb-msg)
+   *cb-msg = xstrdup(message);
+   if (cb-cutoff_time)
+   *cb-cutoff_time = timestamp;
+   if (cb-cutoff_tz)
+   *cb-cutoff_tz = tz;
+   if (cb-cutoff_cnt)
+   *cb-cutoff_cnt = cb-reccnt;
+   hashcpy(cb-sha1, osha1);
+   if (is_null_sha1(cb-sha1))
+   hashcpy(cb-sha1, nsha1);
+   /* We just want the first entry */
+   return 1;
+}
+
+int read_ref_at(const char *refname, unsigned long at_time, int cnt,
+   unsigned char *sha1, char **msg,
+   unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
+{
+   struct read_ref_at_cb cb;
+
+   memset(cb, 0, sizeof(cb));
+   cb.refname = refname;
+   cb.at_time = at_time;
+   cb.cnt = cnt;
+   cb.msg = msg;
+   cb.cutoff_time = cutoff_time;
+   cb.cutoff_tz = cutoff_tz;
+   cb.cutoff_cnt = cutoff_cnt;
+   cb.sha1 = sha1;
+
+   for_each_reflog_ent_reverse(refname, read_ref_at_ent, cb);
+
+   if (!cb.reccnt)
+   die(Log for %s is empty., refname);
+   if (cb.found_it)
+   return 0;
+
+   for_each_reflog_ent(refname, read_ref_at_ent_oldest, cb);
+
+   return 1;
+}
diff --git a/refs.c b/refs.c
index 7d579be..52ca0bb 100644
--- a/refs.c
+++ b/refs.c
@@ -2935,120 +2935,6 @@ int create_symref(const char *ref_target, const char 
*refs_heads_master,
return 0;
 }
 
-struct read_ref_at_cb {
-   const char *refname;
-   unsigned long at_time;
-   int cnt;
-   int reccnt;
-   unsigned char *sha1;
-   int found_it;
-
-   unsigned char osha1[20];
-   unsigned char nsha1[20];
-   int tz;
-   unsigned long date;
-   char **msg;
-   unsigned long *cutoff_time;
-  

[PATCH v2 11/23] refs-common.c: move resolve_refdup to common

2014-08-13 Thread Ronnie Sahlberg
This function can be shared across all refs backends so move it
to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 6 ++
 refs.c| 6 --
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 37d3d14..655a1a0 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -565,3 +565,9 @@ int ref_exists(const char *refname)
unsigned char sha1[20];
return !!resolve_ref_unsafe(refname, sha1, RESOLVE_REF_READING, NULL);
 }
+
+char *resolve_refdup(const char *ref, unsigned char *sha1, int flags, int 
*ref_flag)
+{
+   const char *ret = resolve_ref_unsafe(ref, sha1, flags, ref_flag);
+   return ret ? xstrdup(ret) : NULL;
+}
diff --git a/refs.c b/refs.c
index a94378e..ed7bc61 100644
--- a/refs.c
+++ b/refs.c
@@ -1501,12 +1501,6 @@ const char *resolve_ref_unsafe(const char *refname, 
unsigned char *sha1, int fla
}
 }
 
-char *resolve_refdup(const char *ref, unsigned char *sha1, int flags, int 
*ref_flag)
-{
-   const char *ret = resolve_ref_unsafe(ref, sha1, flags, ref_flag);
-   return ret ? xstrdup(ret) : NULL;
-}
-
 /* The argument to filter_refs */
 struct ref_filter {
const char *pattern;
-- 
2.0.1.556.g3edca4c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 15/23] refs-common.c: move prettify_refname to the common code

2014-08-13 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 9 +
 refs.c| 9 -
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 6eef80b..d8a295c 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -3,6 +3,15 @@
 #include refs.h
 #include string-list.h
 
+const char *prettify_refname(const char *name)
+{
+   return name + (
+   starts_with(name, refs/heads/) ? 11 :
+   starts_with(name, refs/tags/) ? 10 :
+   starts_with(name, refs/remotes/) ? 13 :
+   0);
+}
+
 int names_conflict(const char *refname1, const char *refname2)
 {
for (; *refname1  *refname1 == *refname2; refname1++, refname2++)
diff --git a/refs.c b/refs.c
index 6542969..fb9c614 100644
--- a/refs.c
+++ b/refs.c
@@ -1747,15 +1747,6 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
   DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }
 
-const char *prettify_refname(const char *name)
-{
-   return name + (
-   starts_with(name, refs/heads/) ? 11 :
-   starts_with(name, refs/tags/) ? 10 :
-   starts_with(name, refs/remotes/) ? 13 :
-   0);
-}
-
 static void unlock_ref(struct ref_lock *lock)
 {
/* Do not free lock-lk -- atexit() still looks at them */
-- 
2.0.1.556.g3edca4c

--
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/23] refs-common.c: move update_ref to refs-common.c

2014-08-13 Thread Ronnie Sahlberg
This change moves update_ref() to the refs-common.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 25 +
 refs.c| 23 ---
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 44d96d2..cb884b2 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -1,2 +1,27 @@
 /* common code for all ref backends */
+#include cache.h
+#include refs.h
+
+int update_ref(const char *action, const char *refname,
+  const unsigned char *sha1, const unsigned char *oldval,
+  int flags, struct strbuf *e)
+{
+   struct ref_transaction *t;
+   struct strbuf err = STRBUF_INIT;
+
+   t = transaction_begin(err);
+   if (!t ||
+   transaction_update_sha1(t, refname, sha1, oldval, flags,
+   !!oldval, action, err) ||
+   transaction_commit(t, err)) {
+   const char *str = update_ref failed for ref '%s': %s;
+
+   transaction_free(t);
+   if (e)
+   strbuf_addf(e, str, refname, err.buf);
+   strbuf_release(err);
+   return 1;
+   }
+   return 0;
+}
 
diff --git a/refs.c b/refs.c
index 4a22513..eb66cf7 100644
--- a/refs.c
+++ b/refs.c
@@ -3576,29 +3576,6 @@ int transaction_delete_sha1(struct ref_transaction 
*transaction,
  old_sha1, flags, have_old, msg, err);
 }
 
-int update_ref(const char *action, const char *refname,
-  const unsigned char *sha1, const unsigned char *oldval,
-  int flags, struct strbuf *e)
-{
-   struct ref_transaction *t;
-   struct strbuf err = STRBUF_INIT;
-
-   t = transaction_begin(err);
-   if (!t ||
-   transaction_update_sha1(t, refname, sha1, oldval, flags,
-   !!oldval, action, err) ||
-   transaction_commit(t, err)) {
-   const char *str = update_ref failed for ref '%s': %s;
-
-   transaction_free(t);
-   if (e)
-   strbuf_addf(e, str, refname, err.buf);
-   strbuf_release(err);
-   return 1;
-   }
-   return 0;
-}
-
 static int ref_update_compare(const void *r1, const void *r2)
 {
const struct ref_update * const *u1 = r1;
-- 
2.0.1.556.g3edca4c

--
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/23] refs.c: create a public function for is_refname_available

2014-08-13 Thread Ronnie Sahlberg
Export a generic is_refname_available() function. We will need this
as a public shared function later when we add additional refs backends
since we want to keep using the same rules for ref naming across
all backends.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 29 ++---
 refs.h |  6 ++
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/refs.c b/refs.c
index 7e13c0f..4a22513 100644
--- a/refs.c
+++ b/refs.c
@@ -830,9 +830,9 @@ static int name_conflict_fn(struct ref_entry *entry, void 
*cb_data)
  * operation). skip contains a list of refs we want to skip checking for
  * conflicts with.
  */
-static int is_refname_available(const char *refname,
-   struct ref_dir *dir,
-   const char **skip, int skipnum)
+static int is_refname_available_dir(const char *refname,
+   struct ref_dir *dir,
+   const char **skip, int skipnum)
 {
struct name_conflict_cb data;
data.refname = refname;
@@ -1238,6 +1238,18 @@ static struct ref_dir *get_loose_refs(struct ref_cache 
*refs)
return get_ref_dir(refs-loose);
 }
 
+int is_refname_available(const char *refname, const char **skip, int skipnum)
+{
+   if (!is_refname_available_dir(refname, get_packed_refs(ref_cache),
+ skip, skipnum))
+   return 0;
+
+   if (!is_refname_available_dir(refname, get_loose_refs(ref_cache),
+ skip, skipnum))
+   return 0;
+   return 1;
+}
+
 /* We allow recursive symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
 #define MAXREFLEN (1024)
@@ -2168,8 +2180,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 * name is a proper prefix of our refname.
 */
if (missing 
-!is_refname_available(refname, get_packed_refs(ref_cache),
-  skip, skipnum)) {
+!is_refname_available_dir(refname, get_packed_refs(ref_cache),
+  skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2676,12 +2688,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
return 1;
}
 
-   if (!is_refname_available(newrefname, get_packed_refs(ref_cache),
- oldrefname, 1))
-   return 1;
-
-   if (!is_refname_available(newrefname, get_loose_refs(ref_cache),
- oldrefname, 1))
+   if (!is_refname_available(newrefname, oldrefname, 1))
return 1;
 
log = reflog_exists(oldrefname);
diff --git a/refs.h b/refs.h
index f44b5c8..d526da0 100644
--- a/refs.h
+++ b/refs.h
@@ -131,6 +131,12 @@ extern int ref_exists(const char *);
 extern int is_branch(const char *refname);
 
 /*
+ * Check that a particular refname is available for creation. skip contains
+ * a list of refnames to exclude from the refname collision tests.
+ */
+int is_refname_available(const char *refname, const char **skip, int skipnum);
+
+/*
  * If refname is a non-symbolic reference that refers to a tag object,
  * and the tag can be (recursively) dereferenced to a non-tag object,
  * store the SHA1 of the referred-to object to sha1 and return 0.  If
-- 
2.0.1.556.g3edca4c

--
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 22/23] refs.c: add methods for the ref iterators

2014-08-13 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 36 
 refs.c| 21 ++---
 refs.h| 19 +++
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index f19402b..3df725d 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -928,3 +928,39 @@ int head_ref_submodule(const char *submodule, each_ref_fn 
fn, void *cb_data)
 {
return refs-head_ref_submodule(submodule, fn, cb_data);
 }
+ 
+int for_each_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_ref(fn, cb_data);
+}
+
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+{
+   return refs-for_each_ref_submodule(submodule, fn, cb_data);
+}
+
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_ref_in(prefix, fn, cb_data);
+}
+
+int for_each_ref_in_submodule(const char *submodule, const char *prefix,
+ each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_ref_in_submodule(submodule, prefix, fn, cb_data);
+}
+
+int for_each_rawref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_rawref(fn, cb_data);
+}
+
+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_namespaced_ref(fn, cb_data);
+}
+
+int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_replace_ref(fn, cb_data);
+}
diff --git a/refs.c b/refs.c
index 7ad07f1..dc0467a 100644
--- a/refs.c
+++ b/refs.c
@@ -1610,33 +1610,33 @@ static int files_head_ref_submodule(const char 
*submodule, each_ref_fn fn, void
return do_head_ref(submodule, fn, cb_data);
 }
 
-int for_each_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+static int files_for_each_ref_submodule(const char *submodule, each_ref_fn fn, 
void *cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+static int files_for_each_ref_in(const char *prefix, each_ref_fn fn, void 
*cb_data)
 {
return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
 }
 
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
+static int files_for_each_ref_in_submodule(const char *submodule, const char 
*prefix,
each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
 }
 
-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
int ret;
@@ -1646,7 +1646,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
return ret;
 }
 
-int for_each_rawref(each_ref_fn fn, void *cb_data)
+static int files_for_each_rawref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, , fn, 0,
   DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
@@ -3311,6 +3311,13 @@ struct ref_be refs_files = {
files_resolve_gitlink_ref,
files_head_ref,
files_head_ref_submodule,
+   files_for_each_ref,
+   files_for_each_ref_submodule,
+   files_for_each_ref_in,
+   files_for_each_ref_in_submodule,
+   files_for_each_rawref,
+   files_for_each_namespaced_ref,
+   files_for_each_replace_ref,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.h b/refs.h
index 420e9b5..249a14a 100644
--- a/refs.h
+++ b/refs.h
@@ -401,6 +401,18 @@ typedef int (*head_ref_fn)(each_ref_fn fn, void *cb_data);
 typedef int (*head_ref_submodule_fn)(const char *submodule, each_ref_fn fn,
 void *cb_data);
 
+typedef int (*for_each_ref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*for_each_ref_submodule_fn)(const char *submodule, each_ref_fn fn,
+void *cb_data);
+typedef int (*for_each_ref_in_fn)(const char *prefix, each_ref_fn fn,
+ void *cb_data);
+typedef int (*for_each_ref_in_submodule_fn)(const char *submodule,
+   const char *prefix,
+   each_ref_fn fn, void *cb_data);
+typedef int (*for_each_rawref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*for_each_namespaced_ref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*for_each_replace_ref_fn)(each_ref_fn fn, void *cb_data);
+
 

[PATCH v2 10/23] refs-common.c: move read_ref, read_ref_full and ref_exists to common

2014-08-13 Thread Ronnie Sahlberg
These functions do not depend on the backend implementation so we
can move them to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c | 18 ++
 refs.c| 18 --
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index ab3a118..37d3d14 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -547,3 +547,21 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, 
const struct string_li
data.msg_fmt = msg_fmt;
for_each_rawref(warn_if_dangling_symref, data);
 }
+
+int read_ref_full(const char *refname, unsigned char *sha1, int flags, int 
*ref_flag)
+{
+   if (resolve_ref_unsafe(refname, sha1, flags, ref_flag))
+   return 0;
+   return -1;
+}
+
+int read_ref(const char *refname, unsigned char *sha1)
+{
+   return read_ref_full(refname, sha1, RESOLVE_REF_READING, NULL);
+}
+
+int ref_exists(const char *refname)
+{
+   unsigned char sha1[20];
+   return !!resolve_ref_unsafe(refname, sha1, RESOLVE_REF_READING, NULL);
+}
diff --git a/refs.c b/refs.c
index 40c329b..a94378e 100644
--- a/refs.c
+++ b/refs.c
@@ -1514,24 +1514,6 @@ struct ref_filter {
void *cb_data;
 };
 
-int read_ref_full(const char *refname, unsigned char *sha1, int flags, int 
*ref_flag)
-{
-   if (resolve_ref_unsafe(refname, sha1, flags, ref_flag))
-   return 0;
-   return -1;
-}
-
-int read_ref(const char *refname, unsigned char *sha1)
-{
-   return read_ref_full(refname, sha1, RESOLVE_REF_READING, NULL);
-}
-
-int ref_exists(const char *refname)
-{
-   unsigned char sha1[20];
-   return !!resolve_ref_unsafe(refname, sha1, RESOLVE_REF_READING, NULL);
-}
-
 static int filter_refs(const char *refname, const unsigned char *sha1, int 
flags,
   void *data)
 {
-- 
2.0.1.556.g3edca4c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 14/23] refs-common.c: move names_conflict to the common code

2014-08-13 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-common.c |  8 
 refs.c| 14 --
 refs.h|  9 +
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/refs-common.c b/refs-common.c
index 5f83d7e..6eef80b 100644
--- a/refs-common.c
+++ b/refs-common.c
@@ -3,6 +3,14 @@
 #include refs.h
 #include string-list.h
 
+int names_conflict(const char *refname1, const char *refname2)
+{
+   for (; *refname1  *refname1 == *refname2; refname1++, refname2++)
+   ;
+   return (*refname1 == '\0'  *refname2 == '/')
+   || (*refname1 == '/'  *refname2 == '\0');
+}
+
 int is_branch(const char *refname)
 {
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
diff --git a/refs.c b/refs.c
index 70c034c..6542969 100644
--- a/refs.c
+++ b/refs.c
@@ -676,20 +676,6 @@ static void prime_ref_dir(struct ref_dir *dir)
prime_ref_dir(get_ref_dir(entry));
}
 }
-/*
- * Return true iff refname1 and refname2 conflict with each other.
- * Two reference names conflict if one of them exactly matches the
- * leading components of the other; e.g., foo/bar conflicts with
- * both foo and with foo/bar/baz but not with foo/bar or
- * foo/barbados.
- */
-static int names_conflict(const char *refname1, const char *refname2)
-{
-   for (; *refname1  *refname1 == *refname2; refname1++, refname2++)
-   ;
-   return (*refname1 == '\0'  *refname2 == '/')
-   || (*refname1 == '/'  *refname2 == '\0');
-}
 
 struct name_conflict_cb {
const char *refname;
diff --git a/refs.h b/refs.h
index d526da0..a14fc5d 100644
--- a/refs.h
+++ b/refs.h
@@ -128,6 +128,15 @@ int pack_refs(unsigned int flags, struct strbuf *err);
 
 extern int ref_exists(const char *);
 
+/*
+ * Return true iff refname1 and refname2 conflict with each other.
+ * Two reference names conflict if one of them exactly matches the
+ * leading components of the other; e.g., foo/bar conflicts with
+ * both foo and with foo/bar/baz but not with foo/bar or
+ * foo/barbados.
+ */
+int names_conflict(const char *refname1, const char *refname2);
+
 extern int is_branch(const char *refname);
 
 /*
-- 
2.0.1.556.g3edca4c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/6] sha1_file.c: do not die failing to malloc in unpack_compressed_entry

2014-08-13 Thread Junio C Hamano
Looks very sensible. Thanks.

On Wed, Aug 13, 2014 at 3:57 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 Fewer die() gives better control to the caller, provided that the
 caller _can_ handle it. And in unpack_compressed_entry() case, it can,
 because unpack_compressed_entry() already returns NULL if it fails to
 inflate data.

 A side effect from this is fsck continues to run when very large blobs
 are present (and do not fit in memory).

 Noticed-by: Dale R. Worley wor...@alum.mit.edu
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  sha1_file.c  | 4 +++-
  t/t1050-large.sh | 6 ++
  2 files changed, 9 insertions(+), 1 deletion(-)

 diff --git a/sha1_file.c b/sha1_file.c
 index 3f70b1d..330862b 100644
 --- a/sha1_file.c
 +++ b/sha1_file.c
 @@ -1923,7 +1923,9 @@ static void *unpack_compressed_entry(struct packed_git 
 *p,
 git_zstream stream;
 unsigned char *buffer, *in;

 -   buffer = xmallocz(size);
 +   buffer = xmallocz_gentle(size);
 +   if (!buffer)
 +   return NULL;
 memset(stream, 0, sizeof(stream));
 stream.next_out = buffer;
 stream.avail_out = size + 1;
 diff --git a/t/t1050-large.sh b/t/t1050-large.sh
 index aea4936..5642f84 100755
 --- a/t/t1050-large.sh
 +++ b/t/t1050-large.sh
 @@ -163,4 +163,10 @@ test_expect_success 'zip achiving, deflate' '
 git archive --format=zip HEAD /dev/null
  '

 +test_expect_success 'fsck' '
 +   test_must_fail git fsck 2err 
 +   n=$(grep error: attempting to allocate .* over limit err | wc -l) 
 +   test $n -gt 1
 +'
 +
  test_done
 --
 2.1.0.rc0.78.gc0d8480

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/23] backend-struct-db

2014-08-13 Thread Junio C Hamano
Ronnie Sahlberg sahlb...@google.com writes:

 15 of the patches, the refs-common.c patches, focuses on moving all backend
 agnostic refs functions to a common file. This file will contain all
 backend agnostic refs functions.

 The last 6 patches adds a backend structure with the methods we need to
 describe a pluggable backend. Currently we only have one built in backend,
 the current files based backend. These patches do not change any of the
 behavior other than that we now call the methods through backend specific
 wrapper functions rather than calling them directly.

 At this stage we now have a defined set of methods needed for a refs
 backend and we can start building and adding new types of ref backends
 to git.

Very nice ;-).

I would have expected that refs.c would be the generic one and refs-be-*
would be the backend specific ones, though; that way you do not have to
introduce a new file refs-common.c at all, no?

--
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/23] backend-struct-db

2014-08-13 Thread Ronnie Sahlberg
On Wed, Aug 13, 2014 at 2:18 PM, Junio C Hamano gits...@pobox.com wrote:
 Ronnie Sahlberg sahlb...@google.com writes:

 15 of the patches, the refs-common.c patches, focuses on moving all backend
 agnostic refs functions to a common file. This file will contain all
 backend agnostic refs functions.

 The last 6 patches adds a backend structure with the methods we need to
 describe a pluggable backend. Currently we only have one built in backend,
 the current files based backend. These patches do not change any of the
 behavior other than that we now call the methods through backend specific
 wrapper functions rather than calling them directly.

 At this stage we now have a defined set of methods needed for a refs
 backend and we can start building and adding new types of ref backends
 to git.

 Very nice ;-).

 I would have expected that refs.c would be the generic one and refs-be-*
 would be the backend specific ones, though; that way you do not have to
 introduce a new file refs-common.c at all, no?


Makes sense.
Let me do those changes and then I will re-post sometime next week
once I get additional feedback on it.


regards
Ronnie Sahlberg
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] read-cache.c: Ensure unmerged entries are removed

2014-08-13 Thread Jaime Soriano Pastor
On Tue, Aug 12, 2014 at 8:31 PM, Junio C Hamano gits...@pobox.com wrote:

 Jaime Soriano Pastor jsorianopas...@gmail.com writes:

  A file in the index can be left as merged and unmerged at the same time
  by some tools as libgit2, this causes some undesiderable behaviours in git.

 Well, doesn't it mean that libgit2 is broken?  Have you filed a bug
 over there yet?


Yes, exactly, I think libgit2 is broken but I wanted to double-check
that it was still happening in their master branch, and it is. I have
reported the bug after checking it.
https://github.com/libgit2/libgit2/issues/2515


 Having said that, protecting ourselves from insanity left by other
 people is always a good idea, provided that it can be done without
 bending overly backwards.


Yes, I think the most important thing in this case is to protect git
against this kind of inconsistencies.
--
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] read-cache.c: Ensure unmerged entries are removed

2014-08-13 Thread Jaime Soriano Pastor
On Tue, Aug 12, 2014 at 8:39 PM, Junio C Hamano gits...@pobox.com wrote:

 Jaime Soriano Pastor jsorianopas...@gmail.com writes:

  Wrong implementations of tools that modify the index can left
  some files as merged and unmerged at the same time. Avoid undesiderable
  behaviours by handling this situation.

 It is understandable that the way _you_ decided to handle the
 situation is so obvious to be spelled out to _you_, but that is the
 most important design decision that needs to be described here.  Do
 you silently remove higher-stage entries when an entry at stage 0
 exists?  Do you silently remove stage 0 entry when higher-stage
 entries exist?  Do you error out without doing neither?


Sorry, I didn't explain my decission enough, and my knowledge of git
internals is not so good.
The idea of my proposal is to remove higher stage entries when, after
replacing an existing entry at stage 0, there are still entries in
higher stages.

In the problematic cases I've seen (specially git add and git reset
--hard) the final state of both, merged and unmerged files, is that
only an entry in stage 0 exists.
Also, the current implementation of git checkout -f silently removes
higher stage entries in this case.


 Silently removing these at runtime may not be something we would
 want to do; after all, we do not know if the broken tool actually
 wanted to have the higher stage entries, or the merged entry.


Yes, I have to agree on that, the user should have the final decission
about what stage entry to use, although I'm not sure if in the
previously commented cases there could be such an additional loss as
the operations that can be modified are already intended to silently
remove stage entries.

 Ideally, I think we should error out and let the users figure out
 how to proceed (we may of course need to make sure they have the
 necessary tools to do so, e.g. git cat-file blob 0:$path to
 resurrect the contents and git update-index --cacheinfo to stuff
 the contents into the stages).


I have also tried a couple of implementations of this patch with die()
and warning().
The implementation with die() would have a message like There are
other staged versions for merged file, and maybe some recomendation
about how to see the blobs.
The warning implementation could return -1, what would prevent git add
to remove the higher-stage entries, but would still make git reset
--hard to clean the index as it seems that it does it anyway if it
manages to finish the call to read_index_unmerged.
Another option would be to print the deleted entries as a warning but
deleting them anyway.

Which option would be better? And what could be a good message?

BTW, I didn't know git cat-file blob 0:$path, but I only manage to
get Not a valid object name fatals. How is it supposed to be used?

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] read-cache.c: Ensure unmerged entries are removed

2014-08-13 Thread Junio C Hamano
Jaime Soriano Pastor jsorianopas...@gmail.com writes:

 In the problematic cases I've seen (specially git add and git reset
 --hard) the final state of both, merged and unmerged files, is that
 only an entry in stage 0 exists.
 Also, the current implementation of git checkout -f silently removes
 higher stage entries in this case.

 Silently removing these at runtime may not be something we would
 want to do; after all, we do not know if the broken tool actually
 wanted to have the higher stage entries, or the merged entry.


 Yes, I have to agree on that, the user should have the final decission
 about what stage entry to use, although I'm not sure if in the
 previously commented cases there could be such an additional loss as
 the operations that can be modified are already intended to silently
 remove stage entries.
 ...
 Which option would be better? And what could be a good message?

Being a conservative, I'd rather avoid doing any magic during
read_cache() time.  ls-files -s for example should show the four
stages so that the broken state can be inspected.

Instead, I suspect that the code paths with problematic iterations
over the index entries that assume that having stage #0 entry for a
path guarantees that there will not be any higher stage entry first
need to be identified (you already said add and reset may be
problematic, there may be others, or they may be the only ones, I
dunno), and then the most sensible one, which would be different
from case to case, among various possibilities need to be chosen as
a fix to each of them:

 (1) the loop may be fixed to ignore/skip unmerged entries;
 (2) the loop may be fixed to ignore/skip the merged entry;
 (3) the loop may be fixed not to spin indefinitely on a path with
 mixed entries; or
 (4) the command should error out.

Yes, it would be more work, but I'd feel safer if the following
worked:

$ git ls-files -s
100644 3cc58df83752123644fef39faab2393af643b1d2 0   conflict
100644 f70f10e4db19068f79bc43844b49f3eece45c4e8 1   conflict
100644 3cc58df83752123644fef39faab2393af643b1d2 2   conflict
100644 223b7836fb19fdf64ba2d3cd6173c6a283141f78 3   conflict
$ empty
$ git add empty
100644 3cc58df83752123644fef39faab2393af643b1d2 0   conflict
100644 f70f10e4db19068f79bc43844b49f3eece45c4e8 1   conflict
100644 3cc58df83752123644fef39faab2393af643b1d2 2   conflict
100644 223b7836fb19fdf64ba2d3cd6173c6a283141f78 3   conflict
100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0   empty
$ git cat-file blob :empty output
$ cmp empty output  echo OK
OK

which would be impossible to do if we nuked the problematic stages
whenever we read the index, I am afraid.

 BTW, I didn't know git cat-file blob 0:$path, but I only manage to
 get Not a valid object name fatals. How is it supposed to be used?

That was a typo of :$n:$path (where 0 = $n = 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


git fetch does not pass quiet setting to git c

2014-08-13 Thread Matthew Flaschen

git fetch does not pass the quiet flag (if applicable) to git gc.

I've reproduced this in 2.0.1, but it appears to be present in master. 
It looks like this line 
(https://github.com/git/git/blob/master/builtin/fetch.c#L1201) is 
calling git gc (which does support --quiet) without passing it.


Tested with:

git fetch -q origin master

I would expect to see no output for that.  Instead, I see:

Total 31 (delta 23), reused 31 (delta 23)

(or whatever the values are).

This will cause cronjobs, for example, to send output when the command 
completes normally.


Thanks,

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