Re: Git to use XDG Base Directory Standard

2012-08-23 Thread Lanoxx


On 23/08/12 04:57, David Aguilar wrote:

On Wed, Aug 22, 2012 at 2:44 PM, Carlos Martín Nieto c...@elego.de wrote:

Lanoxx lan...@gmx.net writes:


Hi,

Git and Gitk are currently using the ~/.gitconfig and ~/.gitk files in
the $HOME directory. It would be nice to use the XDG Base Directory
standard for the location instead, see [1] and [2]. Are there any
plans regarding this standard?

Git does this starting at 1.7.12. See the release notes, e.g. at
https://github.com/git/git/blob/master/Documentation/RelNotes/1.7.12.txt#L18-23

cmn

I do not recall whether gitk learned about it (I don't think so).

Like all big sweeping changes, these were done in a
backwards-compatible way that will allow users to switch over when
they are ready.  If you are interested in attacking this from the gitk
angle then you will want to follow a similar strategy.


I just checked it, the code is in 
https://github.com/git/git/blob/master/gitk-git/gitk:2710 and following. 
Currently it still saves it to ~/.gitk-new and then renames it to ~/.gitk


Unfortunately I do not know Tcl/Tk so I can't provide a patch. But if 
someone could provide a patch that would be really nice.

--
To unsubscribe from this list: send the line 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] Don't use curl_easy_strerror prior to curl-7.12.0

2012-08-23 Thread Joachim Schmitz
 From: Joachim Schmitz [mailto:j...@schmitz-digital.de]
 Sent: Wednesday, August 22, 2012 11:06 PM
 To: 'Junio C Hamano'
 Cc: 'git@vger.kernel.org'
 Subject: RE: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
 
  From: Junio C Hamano [mailto:gits...@pobox.com]
  Sent: Wednesday, August 22, 2012 11:02 PM
  To: Joachim Schmitz
  Cc: git@vger.kernel.org
  Subject: Re: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
 
  Joachim Schmitz j...@schmitz-digital.de writes:
 
diff --git a/http.c b/http.c
index b61ac85..18bc6bf 100644
--- a/http.c
+++ b/http.c
@@ -806,10 +806,12 @@ static int http_request(const char *url, void
*result, int target, int options)
  
   Likewrapped X-
  
   Any suggestion?
 
  Other than don't wrap (or get a real MUA and MTA X-), unfortunately no.
 
  I do not know if you have checked MUA specific hints section of
  Documentation/SubmittingPatches; there may be environment specific hints
  described for a MUA you may have at hand (or not).
 
 I checked, but Outlook isn't mentioned :-(

OK, Outlook inserts line breaks at position 76. I can't switch it off 
completely, but can set it to anything between 30 and 132. I
set it to 132 now, does that look OK now?
---

This reverts be22d92 (http: avoid empty error messages for some curl errors, 
2011-09-05) on platforms with older versions of
libcURL.

Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
---
 http.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/http.c b/http.c
index b61ac85..18bc6bf 100644
--- a/http.c
+++ b/http.c
@@ -806,10 +806,12 @@ static int http_request(const char *url, void *result, 
int target, int options)
ret = HTTP_REAUTH;
}
} else {
+#if LIBCURL_VERSION_NUM = 0x070c00
if (!curl_errorstr[0])
strlcpy(curl_errorstr,
curl_easy_strerror(results.curl_result),
sizeof(curl_errorstr));
+#endif
ret = HTTP_ERROR;
}
} else {
-- 
1.7.12


Bye, Jojo

--
To unsubscribe from this list: send the line 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: Re*: mergetool: support --tool-help option like difftool does

2012-08-23 Thread David Aguilar
On Wed, Aug 22, 2012 at 10:33 PM, Junio C Hamano ju...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 This way we do not have to risk the list of tools go out of sync
 between the implementation and the documentation.

 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
 Junio C Hamano gits...@pobox.com writes:

 +--tool-help::
 +   List the supported and available diff tools.

 This part is a good addition (but it already is mentioned in the
 description of --tool above, so it is more or less a Meh).

 I noticed that the main while loop has style violations in its
 case/esac, but I left it as-is.  Reindenting it would be a low
 hanging fruit janitor patch that would be a separate topic.

 After hinting a low-hanging-fruit that would be an easy way for new
 people to dip their toes, I saw no takers for one month, so I ended
 up doing it myself.

My bad, I didn't catch this and should have, especially so because
I had started on style fixing mergetool last week but ditched it;
I assumed it would be unwanted.  I will read more carefully.

Just please don't tell Charles about it ;-)

(I wanted to do this a long ago but at the time we didn't have
 the style guide for shell, and the time was perhaps not right...)

I am of course in favor of this patch.

While on the mergetool topic...

Would the ability to resolve the various merge situations using
the command-line be a wanted addition?

This would let a submodule or deleted/modified encountering
user do something like:

$ git mergetool --theirs -- submodule

...and not have to remember the various git commands that it runs.

This could touch just the parts of mergetool that require stdin,
but probably should work for everything.

It seems like exposing the guts of some of these mergetool functions
via command-line flags could be helpful, but I'm not sure if that's
overloading mergetool with too many features. What do you think?
-- 
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 01/17] t5500: add tests of error output for missing refs

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

If git fetch-pack is called with reference names that do not exist
on the remote, then it should emit an error message

error: no such remote ref refs/heads/xyzzy

This is currently broken if *only* missing references are passed to
git fetch-pack.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 t/t5500-fetch-pack.sh | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index e80a2af..3cc3346 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -397,4 +397,34 @@ test_expect_success 'test duplicate refs from stdin' '
test_cmp expect actual
 '
 
+test_expect_success 'set up tests of missing reference' '
+   cat expect-error -\EOF
+   error: no such remote ref refs/heads/xyzzy
+   EOF
+'
+
+test_expect_failure 'test lonely missing ref' '
+   (
+   cd client 
+   test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy
+   ) /dev/null 2error-m 
+   test_cmp expect-error error-m
+'
+
+test_expect_success 'test missing ref after existing' '
+   (
+   cd client 
+   test_must_fail git fetch-pack --no-progress .. refs/heads/A 
refs/heads/xyzzy
+   ) /dev/null 2error-em 
+   test_cmp expect-error error-em
+'
+
+test_expect_success 'test missing ref before existing' '
+   (
+   cd client 
+   test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy 
refs/heads/A
+   ) /dev/null 2error-me 
+   test_cmp expect-error error-me
+'
+
 test_done
-- 
1.7.11.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


[PATCH 15/17] cmd_fetch_pack(): simplify computation of return value

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

Set the final value at initialization rather than initializing it then
sometimes changing it.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 5b1e603..6c1a48e 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1027,17 +1027,16 @@ int cmd_fetch_pack(int argc, const char **argv, const 
char *prefix)
if (finish_connect(conn))
return 1;
 
-   ret = !ref;
-   if (nr_heads) {
-   /* If the heads to pull were given, we should have
-* consumed all of them by matching the remote.
-* Otherwise, 'git fetch remote no-such-ref' would
-* silently succeed without issuing an error.
-*/
-   for (i = 0; i  nr_heads; i++)
-   error(no such remote ref %s, heads[i]);
-   ret = 1;
-   }
+   ret = !ref || nr_heads;
+
+   /*
+* If the heads to pull were given, we should have consumed
+* all of them by matching the remote.  Otherwise, 'git fetch
+* remote no-such-ref' would silently succeed without issuing
+* an error.
+*/
+   for (i = 0; i  nr_heads; i++)
+   error(no such remote ref %s, heads[i]);
while (ref) {
printf(%s %s\n,
   sha1_to_hex(ref-old_sha1), ref-name);
-- 
1.7.11.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


[PATCH 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

There were various confusing things (and a couple of bugs) in the way
that fetch_pack() handled the list (nr_heads, heads) of references to
be sought from the remote:

* Different names were used for the list in different functions for no
  special reason.

* fetch_pack() modified the list in-place:

  * It moved entries around

  * It sometimes shrunk the list without informing the caller (which
could lead to spurious error messages)

  * It overwrote the first byte of matching entries with NUL, leaving
a sparse list that the caller had to interpret

  * Its interaction with the list was documented

* No error was reported if *all* requested references were missing
  from the remote.

I'm still suspicious about the logic related to args.fetch_all and
args.depth, but I don't think I've made anything worse.

This patch series applies to the merge between master and
jc/maint-push-refs-all, though the dependency on the latter is only
textual.

Michael Haggerty (17):
  t5500: add tests of error output for missing refs
  Rename static function fetch_pack() to http_fetch_pack()
  Fix formatting.
  Name local variables more consistently
  Do not check the same match_pos twice
  Let fetch_pack() inform caller about number of unique heads
  Pass nr_heads to do_pack_ref() by reference
  Pass nr_heads to everything_local() by reference
  Pass nr_heads to filter_refs() by reference
  Remove ineffective optimization
  filter_refs(): do not leave gaps in return_refs
  filter_refs(): compress unmatched refs in heads array
  cmd_fetch_pack: return early if finish_connect() returns an error
  Report missing refs even if no existing refs were received
  cmd_fetch_pack(): simplify computation of return value
  fetch_pack(): free matching heads
  fetch_refs(): simplify logic

 builtin/fetch-pack.c  | 128 --
 fetch-pack.h  |  19 +---
 http-walker.c |   4 +-
 t/t5500-fetch-pack.sh |  32 -
 transport.c   |   8 ++--
 5 files changed, 101 insertions(+), 90 deletions(-)

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


[PATCH 02/17] Rename static function fetch_pack() to http_fetch_pack()

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

Avoid confusion with the non-static function of the same name from
fetch-pack.h.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 http-walker.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/http-walker.c b/http-walker.c
index 51a906e..1516c5e 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -396,7 +396,7 @@ static int fetch_indices(struct walker *walker, struct 
alt_base *repo)
return ret;
 }
 
-static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned 
char *sha1)
+static int http_fetch_pack(struct walker *walker, struct alt_base *repo, 
unsigned char *sha1)
 {
struct packed_git *target;
int ret;
@@ -524,7 +524,7 @@ static int fetch(struct walker *walker, unsigned char *sha1)
if (!fetch_object(walker, altbase, sha1))
return 0;
while (altbase) {
-   if (!fetch_pack(walker, altbase, sha1))
+   if (!http_fetch_pack(walker, altbase, sha1))
return 0;
fetch_alternates(walker, data-alt-base);
altbase = altbase-next;
-- 
1.7.11.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


[PATCH 03/17] Fix formatting.

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu


Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c |  8 
 fetch-pack.h | 12 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 7912d2b..cc21047 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1062,10 +1062,10 @@ static int compare_heads(const void *a, const void *b)
 struct ref *fetch_pack(struct fetch_pack_args *my_args,
   int fd[], struct child_process *conn,
   const struct ref *ref,
-   const char *dest,
-   int nr_heads,
-   char **heads,
-   char **pack_lockfile)
+  const char *dest,
+  int nr_heads,
+  char **heads,
+  char **pack_lockfile)
 {
struct stat st;
struct ref *ref_cpy;
diff --git a/fetch-pack.h b/fetch-pack.h
index 7c2069c..1dbe90f 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -18,11 +18,11 @@ struct fetch_pack_args {
 };
 
 struct ref *fetch_pack(struct fetch_pack_args *args,
-   int fd[], struct child_process *conn,
-   const struct ref *ref,
-   const char *dest,
-   int nr_heads,
-   char **heads,
-   char **pack_lockfile);
+  int fd[], struct child_process *conn,
+  const struct ref *ref,
+  const char *dest,
+  int nr_heads,
+  char **heads,
+  char **pack_lockfile);
 
 #endif
-- 
1.7.11.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


[PATCH 05/17] Do not check the same match_pos twice

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

Once a match has been found at match_pos, the entry is zeroed and no
future attempts will match that entry.  So increment match_pos to
avoid checking against the zeroed-out entry during the next iteration.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index f11545e..a6406e7 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -561,8 +561,8 @@ static void filter_refs(struct ref **refs, int nr_heads, 
char **heads)
if (cmp  0) /* definitely do not have it */
break;
else if (cmp == 0) { /* definitely have it */
-   heads[match_pos][0] = '\0';
return_refs[match_pos] = ref;
+   heads[match_pos++][0] = '\0';
break;
}
else /* might have it; keep looking */
-- 
1.7.11.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


[PATCH 06/17] Let fetch_pack() inform caller about number of unique heads

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

fetch_pack() remotes duplicates from the list (nr_heads, heads),
thereby shrinking the list.  But previously, the caller was not
informed about the shrinkage.  This would cause a spurious error
message to be emitted by cmd_fetch_pack() if git fetch-pack is
called with duplicate refnames.

So change the signature of fetch_pack() to accept nr_heads by
reference, and if any duplicates were removed then modify it to
reflect the number of remaining references.

The last test of t5500 inexplicably *required* git fetch-pack to
fail when fetching a list of references that contains duplicates;
i.e., it insisted on the buggy behavior.  So change the test to expect
the correct behavior.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c  | 12 ++--
 fetch-pack.h  |  2 +-
 t/t5500-fetch-pack.sh |  2 +-
 transport.c   |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index a6406e7..3c93ec4 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1021,7 +1021,7 @@ int cmd_fetch_pack(int argc, const char **argv, const 
char *prefix)
get_remote_heads(fd[0], ref, 0, NULL);
 
ref = fetch_pack(args, fd, conn, ref, dest,
-   nr_heads, heads, pack_lockfile_ptr);
+   nr_heads, heads, pack_lockfile_ptr);
if (pack_lockfile) {
printf(lock %s\n, pack_lockfile);
fflush(stdout);
@@ -1062,7 +1062,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
   int fd[], struct child_process *conn,
   const struct ref *ref,
   const char *dest,
-  int nr_heads,
+  int *nr_heads,
   char **heads,
   char **pack_lockfile)
 {
@@ -1077,16 +1077,16 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
st.st_mtime = 0;
}
 
-   if (heads  nr_heads) {
-   qsort(heads, nr_heads, sizeof(*heads), compare_heads);
-   nr_heads = remove_duplicates(nr_heads, heads);
+   if (heads  *nr_heads) {
+   qsort(heads, *nr_heads, sizeof(*heads), compare_heads);
+   *nr_heads = remove_duplicates(*nr_heads, heads);
}
 
if (!ref) {
packet_flush(fd[1]);
die(no matching remote head);
}
-   ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
+   ref_cpy = do_fetch_pack(fd, ref, *nr_heads, heads, pack_lockfile);
 
if (args.depth  0) {
struct cache_time mtime;
diff --git a/fetch-pack.h b/fetch-pack.h
index 1dbe90f..a9d505b 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -21,7 +21,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
   int fd[], struct child_process *conn,
   const struct ref *ref,
   const char *dest,
-  int nr_heads,
+  int *nr_heads,
   char **heads,
   char **pack_lockfile);
 
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 3cc3346..0d4edcb 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -391,7 +391,7 @@ test_expect_success 'fetch mixed refs from cmdline and 
stdin' '
 test_expect_success 'test duplicate refs from stdin' '
(
cd client 
-   test_must_fail git fetch-pack --stdin --no-progress .. ../input.dup
+   git fetch-pack --stdin --no-progress .. ../input.dup
) output 
cut -d   -f 2 output | sort actual 
test_cmp expect actual
diff --git a/transport.c b/transport.c
index 1811b50..f7bbf58 100644
--- a/transport.c
+++ b/transport.c
@@ -548,7 +548,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 
refs = fetch_pack(args, data-fd, data-conn,
  refs_tmp ? refs_tmp : transport-remote_refs,
- dest, nr_heads, heads, transport-pack_lockfile);
+ dest, nr_heads, heads, transport-pack_lockfile);
close(data-fd[0]);
close(data-fd[1]);
if (finish_connect(data-conn))
-- 
1.7.11.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


[PATCH 13/17] cmd_fetch_pack: return early if finish_connect() returns an error

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

This simplifies the logic without changing the behavior.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cc9af80..4794839 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1025,10 +1025,10 @@ int cmd_fetch_pack(int argc, const char **argv, const 
char *prefix)
close(fd[0]);
close(fd[1]);
if (finish_connect(conn))
-   ref = NULL;
-   ret = !ref;
+   return 1;
 
-   if (!ret  nr_heads) {
+   ret = !ref;
+   if (ref  nr_heads) {
/* If the heads to pull were given, we should have
 * consumed all of them by matching the remote.
 * Otherwise, 'git fetch remote no-such-ref' would
-- 
1.7.11.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


[PATCH 04/17] Name local variables more consistently

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

Use the names (nr_heads, heads) consistently across functions, instead
of sometimes naming the same values (nr_match, match).

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cc21047..f11545e 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -521,7 +521,7 @@ static void mark_recent_complete_commits(unsigned long 
cutoff)
}
 }
 
-static void filter_refs(struct ref **refs, int nr_match, char **match)
+static void filter_refs(struct ref **refs, int nr_heads, char **heads)
 {
struct ref **return_refs;
struct ref *newlist = NULL;
@@ -530,12 +530,12 @@ static void filter_refs(struct ref **refs, int nr_match, 
char **match)
struct ref *fastarray[32];
int match_pos;
 
-   if (nr_match  !args.fetch_all) {
-   if (ARRAY_SIZE(fastarray)  nr_match)
-   return_refs = xcalloc(nr_match, sizeof(struct ref *));
+   if (nr_heads  !args.fetch_all) {
+   if (ARRAY_SIZE(fastarray)  nr_heads)
+   return_refs = xcalloc(nr_heads, sizeof(struct ref *));
else {
return_refs = fastarray;
-   memset(return_refs, 0, sizeof(struct ref *) * nr_match);
+   memset(return_refs, 0, sizeof(struct ref *) * nr_heads);
}
}
else
@@ -556,12 +556,12 @@ static void filter_refs(struct ref **refs, int nr_match, 
char **match)
}
else {
int cmp = -1;
-   while (match_pos  nr_match) {
-   cmp = strcmp(ref-name, match[match_pos]);
+   while (match_pos  nr_heads) {
+   cmp = strcmp(ref-name, heads[match_pos]);
if (cmp  0) /* definitely do not have it */
break;
else if (cmp == 0) { /* definitely have it */
-   match[match_pos][0] = '\0';
+   heads[match_pos][0] = '\0';
return_refs[match_pos] = ref;
break;
}
@@ -576,7 +576,7 @@ static void filter_refs(struct ref **refs, int nr_match, 
char **match)
 
if (!args.fetch_all) {
int i;
-   for (i = 0; i  nr_match; i++) {
+   for (i = 0; i  nr_heads; i++) {
ref = return_refs[i];
if (ref) {
*newtail = ref;
@@ -595,7 +595,7 @@ static void mark_alternate_complete(const struct ref *ref, 
void *unused)
mark_complete(NULL, ref-old_sha1, 0, NULL);
 }
 
-static int everything_local(struct ref **refs, int nr_match, char **match)
+static int everything_local(struct ref **refs, int nr_heads, char **heads)
 {
struct ref *ref;
int retval;
@@ -646,7 +646,7 @@ static int everything_local(struct ref **refs, int 
nr_match, char **match)
}
}
 
-   filter_refs(refs, nr_match, match);
+   filter_refs(refs, nr_heads, heads);
 
for (retval = 1, ref = *refs; ref ; ref = ref-next) {
const unsigned char *remote = ref-old_sha1;
@@ -777,8 +777,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
 
 static struct ref *do_fetch_pack(int fd[2],
const struct ref *orig_ref,
-   int nr_match,
-   char **match,
+   int nr_heads, char **heads,
char **pack_lockfile)
 {
struct ref *ref = copy_ref_list(orig_ref);
@@ -819,7 +818,7 @@ static struct ref *do_fetch_pack(int fd[2],
fprintf(stderr, Server supports ofs-delta\n);
} else
prefer_ofs_delta = 0;
-   if (everything_local(ref, nr_match, match)) {
+   if (everything_local(ref, nr_heads, heads)) {
packet_flush(fd[1]);
goto all_done;
}
-- 
1.7.11.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


[PATCH 10/17] Remove ineffective optimization

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

I cannot find a scenario in which this function is called any
significant number of times, so simplify the code by always allocating
an array for return_refs rather than trying to use a stack-allocated
array for small lists.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 0426cf4..9398059 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -527,17 +527,10 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
struct ref *newlist = NULL;
struct ref **newtail = newlist;
struct ref *ref, *next;
-   struct ref *fastarray[32];
int match_pos;
 
-   if (*nr_heads  !args.fetch_all) {
-   if (ARRAY_SIZE(fastarray)  *nr_heads)
-   return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
-   else {
-   return_refs = fastarray;
-   memset(return_refs, 0, sizeof(struct ref *) * 
*nr_heads);
-   }
-   }
+   if (*nr_heads  !args.fetch_all)
+   return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
else
return_refs = NULL;
 
@@ -584,8 +577,7 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
newtail = ref-next;
}
}
-   if (return_refs != fastarray)
-   free(return_refs);
+   free(return_refs);
}
*refs = newlist;
 }
-- 
1.7.11.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


[PATCH 11/17] filter_refs(): do not leave gaps in return_refs

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

It used to be that this function processed refnames in some arbitrary
order but wanted to return them in the order that they were requested,
not the order that they were processed.  Now, the refnames are
processed in sorted order, so there is no reason to go to the extra
effort.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 9398059..8366012 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -527,14 +527,13 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
struct ref *newlist = NULL;
struct ref **newtail = newlist;
struct ref *ref, *next;
-   int match_pos;
+   int match_pos = 0, matched = 0;
 
if (*nr_heads  !args.fetch_all)
return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
else
return_refs = NULL;
 
-   match_pos = 0;
for (ref = *refs; ref; ref = next) {
next = ref-next;
if (!memcmp(ref-name, refs/, 5) 
@@ -554,7 +553,7 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
if (cmp  0) /* definitely do not have it */
break;
else if (cmp == 0) { /* definitely have it */
-   return_refs[match_pos] = ref;
+   return_refs[matched++] = ref;
heads[match_pos++][0] = '\0';
break;
}
@@ -569,13 +568,11 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
 
if (!args.fetch_all) {
int i;
-   for (i = 0; i  *nr_heads; i++) {
+   for (i = 0; i  matched; i++) {
ref = return_refs[i];
-   if (ref) {
-   *newtail = ref;
-   ref-next = NULL;
-   newtail = ref-next;
-   }
+   *newtail = ref;
+   ref-next = NULL;
+   newtail = ref-next;
}
free(return_refs);
}
-- 
1.7.11.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


[PATCH 07/17] Pass nr_heads to do_pack_ref() by reference

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

This is the first of a few baby steps towards changing filter_refs()
to compress matched refs out of the list rather than overwriting the
first character of such references with '\0'.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 3c93ec4..d3ff166 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -777,7 +777,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
 
 static struct ref *do_fetch_pack(int fd[2],
const struct ref *orig_ref,
-   int nr_heads, char **heads,
+   int *nr_heads, char **heads,
char **pack_lockfile)
 {
struct ref *ref = copy_ref_list(orig_ref);
@@ -818,7 +818,7 @@ static struct ref *do_fetch_pack(int fd[2],
fprintf(stderr, Server supports ofs-delta\n);
} else
prefer_ofs_delta = 0;
-   if (everything_local(ref, nr_heads, heads)) {
+   if (everything_local(ref, *nr_heads, heads)) {
packet_flush(fd[1]);
goto all_done;
}
@@ -1086,7 +1086,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
packet_flush(fd[1]);
die(no matching remote head);
}
-   ref_cpy = do_fetch_pack(fd, ref, *nr_heads, heads, pack_lockfile);
+   ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
 
if (args.depth  0) {
struct cache_time mtime;
-- 
1.7.11.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


[PATCH 09/17] Pass nr_heads to filter_refs() by reference

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu


Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 5905dae..0426cf4 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -521,7 +521,7 @@ static void mark_recent_complete_commits(unsigned long 
cutoff)
}
 }
 
-static void filter_refs(struct ref **refs, int nr_heads, char **heads)
+static void filter_refs(struct ref **refs, int *nr_heads, char **heads)
 {
struct ref **return_refs;
struct ref *newlist = NULL;
@@ -530,12 +530,12 @@ static void filter_refs(struct ref **refs, int nr_heads, 
char **heads)
struct ref *fastarray[32];
int match_pos;
 
-   if (nr_heads  !args.fetch_all) {
-   if (ARRAY_SIZE(fastarray)  nr_heads)
-   return_refs = xcalloc(nr_heads, sizeof(struct ref *));
+   if (*nr_heads  !args.fetch_all) {
+   if (ARRAY_SIZE(fastarray)  *nr_heads)
+   return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
else {
return_refs = fastarray;
-   memset(return_refs, 0, sizeof(struct ref *) * nr_heads);
+   memset(return_refs, 0, sizeof(struct ref *) * 
*nr_heads);
}
}
else
@@ -556,7 +556,7 @@ static void filter_refs(struct ref **refs, int nr_heads, 
char **heads)
}
else {
int cmp = -1;
-   while (match_pos  nr_heads) {
+   while (match_pos  *nr_heads) {
cmp = strcmp(ref-name, heads[match_pos]);
if (cmp  0) /* definitely do not have it */
break;
@@ -576,7 +576,7 @@ static void filter_refs(struct ref **refs, int nr_heads, 
char **heads)
 
if (!args.fetch_all) {
int i;
-   for (i = 0; i  nr_heads; i++) {
+   for (i = 0; i  *nr_heads; i++) {
ref = return_refs[i];
if (ref) {
*newtail = ref;
@@ -646,7 +646,7 @@ static int everything_local(struct ref **refs, int 
*nr_heads, char **heads)
}
}
 
-   filter_refs(refs, *nr_heads, heads);
+   filter_refs(refs, nr_heads, heads);
 
for (retval = 1, ref = *refs; ref ; ref = ref-next) {
const unsigned char *remote = ref-old_sha1;
-- 
1.7.11.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


[PATCH 17/17] fetch_refs(): simplify logic

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

* Build linked list of return values as we go rather than recording
  them in a temporary array and linking them up later.

* Handle ref in a single if...else statement in the main loop, to make
  it clear that each ref has exactly two possible destinies.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 56 ++--
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 9650d04..90683ca 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -523,66 +523,48 @@ static void mark_recent_complete_commits(unsigned long 
cutoff)
 
 static void filter_refs(struct ref **refs, int *nr_heads, char **heads)
 {
-   struct ref **return_refs;
struct ref *newlist = NULL;
struct ref **newtail = newlist;
struct ref *ref, *next;
-   int match_pos = 0, matched = 0, unmatched = 0;
-
-   if (*nr_heads  !args.fetch_all)
-   return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
-   else
-   return_refs = NULL;
+   int match_pos = 0, unmatched = 0;
 
for (ref = *refs; ref; ref = next) {
+   int keep_ref = 0;
next = ref-next;
if (!memcmp(ref-name, refs/, 5) 
check_refname_format(ref-name, 0))
; /* trash */
else if (args.fetch_all 
-(!args.depth || prefixcmp(ref-name, refs/tags/) )) {
-   *newtail = ref;
-   ref-next = NULL;
-   newtail = ref-next;
-   continue;
-   }
-   else {
-   int cmp = -1;
+  (!args.depth || prefixcmp(ref-name, refs/tags/)))
+   keep_ref = 1;
+   else
while (match_pos  *nr_heads) {
-   cmp = strcmp(ref-name, heads[match_pos]);
-   if (cmp  0) /* definitely do not have it */
+   int cmp = strcmp(ref-name, heads[match_pos]);
+   if (cmp  0) { /* definitely do not have it */
break;
-   else if (cmp == 0) { /* definitely have it */
-   return_refs[matched++] = ref;
+   } else if (cmp == 0) { /* definitely have it */
free(heads[match_pos++]);
+   keep_ref = 1;
break;
-   }
-   else { /* might have it; keep looking */
+   } else { /* might have it; keep looking */
heads[unmatched++] = heads[match_pos++];
}
}
-   if (!cmp)
-   continue; /* we will link it later */
-   }
-   free(ref);
-   }
-
-   if (!args.fetch_all) {
-   int i;
 
-   /* copy remaining unmatched heads: */
-   while (match_pos  *nr_heads)
-   heads[unmatched++] = heads[match_pos++];
-   *nr_heads = unmatched;
-
-   for (i = 0; i  matched; i++) {
-   ref = return_refs[i];
+   if (keep_ref) {
*newtail = ref;
ref-next = NULL;
newtail = ref-next;
+   } else {
+   free(ref);
}
-   free(return_refs);
}
+
+   /* copy any remaining unmatched heads: */
+   while (match_pos  *nr_heads)
+   heads[unmatched++] = heads[match_pos++];
+   *nr_heads = unmatched;
+
*refs = newlist;
 }
 
-- 
1.7.11.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


[PATCH 08/17] Pass nr_heads to everything_local() by reference

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu


Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index d3ff166..5905dae 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -595,7 +595,7 @@ static void mark_alternate_complete(const struct ref *ref, 
void *unused)
mark_complete(NULL, ref-old_sha1, 0, NULL);
 }
 
-static int everything_local(struct ref **refs, int nr_heads, char **heads)
+static int everything_local(struct ref **refs, int *nr_heads, char **heads)
 {
struct ref *ref;
int retval;
@@ -646,7 +646,7 @@ static int everything_local(struct ref **refs, int 
nr_heads, char **heads)
}
}
 
-   filter_refs(refs, nr_heads, heads);
+   filter_refs(refs, *nr_heads, heads);
 
for (retval = 1, ref = *refs; ref ; ref = ref-next) {
const unsigned char *remote = ref-old_sha1;
@@ -818,7 +818,7 @@ static struct ref *do_fetch_pack(int fd[2],
fprintf(stderr, Server supports ofs-delta\n);
} else
prefer_ofs_delta = 0;
-   if (everything_local(ref, *nr_heads, heads)) {
+   if (everything_local(ref, nr_heads, heads)) {
packet_flush(fd[1]);
goto all_done;
}
-- 
1.7.11.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


[PATCH 16/17] fetch_pack(): free matching heads

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

fetch_pack() used to delete entries from the input list (*nr_heads,
heads) and drop them on the floor.  (Even earlier versions dropped
some names on the floor and modified others.)  This forced
fetch_refs_via_pack() to create a separate copy of the original list
so that it could free the all of the names.

Instead, teach fetch_pack() to free any names that it discards from
the list, and change fetch_refs_via_pack() to free only the remaining
(unmatched) names.

Document the change in the function comment in the header file.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---

This change forces callers to allocate names on the heap.  But the two
existing callers did so already, and the function already modified the
list, so I think the new style is no more intrusive than the old.

 builtin/fetch-pack.c | 4 +++-
 fetch-pack.h | 7 ---
 transport.c  | 9 +++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 6c1a48e..9650d04 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -554,7 +554,7 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
break;
else if (cmp == 0) { /* definitely have it */
return_refs[matched++] = ref;
-   match_pos++;
+   free(heads[match_pos++]);
break;
}
else { /* might have it; keep looking */
@@ -844,6 +844,8 @@ static int remove_duplicates(int nr_heads, char **heads)
for (src = dst = 1; src  nr_heads; src++)
if (strcmp(heads[src], heads[dst-1]))
heads[dst++] = heads[src];
+   else
+   free(heads[src]);
return dst;
 }
 
diff --git a/fetch-pack.h b/fetch-pack.h
index 915858e..d1860eb 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -19,9 +19,10 @@ struct fetch_pack_args {
 
 /*
  * (*nr_heads, heads) is an array of pointers to the full names of
- * remote references that should be updated from.  On return, both
- * will have been changed to list only the names that were not found
- * on the remote.
+ * remote references that should be updated from.  The names must have
+ * been allocated from the heap.  The names that are found in the
+ * remote will be removed from the list and freed; the others will be
+ * left in the front of the array with *nr_heads adjusted accordingly.
  */
 struct ref *fetch_pack(struct fetch_pack_args *args,
   int fd[], struct child_process *conn,
diff --git a/transport.c b/transport.c
index 3c38d44..f94b753 100644
--- a/transport.c
+++ b/transport.c
@@ -519,8 +519,6 @@ static int fetch_refs_via_pack(struct transport *transport,
 {
struct git_transport_data *data = transport-data;
char **heads = xmalloc(nr_heads * sizeof(*heads));
-   char **origh = xmalloc(nr_heads * sizeof(*origh));
-   int orig_nr_heads = nr_heads;
const struct ref *refs;
char *dest = xstrdup(transport-url);
struct fetch_pack_args args;
@@ -539,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.depth = data-options.depth;
 
for (i = 0; i  nr_heads; i++)
-   origh[i] = heads[i] = xstrdup(to_fetch[i]-name);
+   heads[i] = xstrdup(to_fetch[i]-name);
 
if (!data-got_remote_heads) {
connect_setup(transport, 0, 0);
@@ -559,9 +557,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 
free_refs(refs_tmp);
 
-   for (i = 0; i  orig_nr_heads; i++)
-   free(origh[i]);
-   free(origh);
+   for (i = 0; i  nr_heads; i++)
+   free(heads[i]);
free(heads);
free(dest);
return (refs ? 0 : -1);
-- 
1.7.11.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


[PATCH 12/17] filter_refs(): compress unmatched refs in heads array

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

Remove any references that were received from the remote from the list
(*nr_heads, heads) of requested references by squeezing them out of
the list (rather than overwriting their names with NUL characters, as
before).  On exit, *nr_heads is the number of requested references
that were not received.

Document this aspect of fetch_pack() in a comment in the header file.
(More documentation is obviously still needed.)

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c | 21 +
 fetch-pack.h |  6 ++
 transport.c  |  3 ++-
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 8366012..cc9af80 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -527,7 +527,7 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
struct ref *newlist = NULL;
struct ref **newtail = newlist;
struct ref *ref, *next;
-   int match_pos = 0, matched = 0;
+   int match_pos = 0, matched = 0, unmatched = 0;
 
if (*nr_heads  !args.fetch_all)
return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
@@ -554,11 +554,12 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
break;
else if (cmp == 0) { /* definitely have it */
return_refs[matched++] = ref;
-   heads[match_pos++][0] = '\0';
+   match_pos++;
break;
}
-   else /* might have it; keep looking */
-   match_pos++;
+   else { /* might have it; keep looking */
+   heads[unmatched++] = heads[match_pos++];
+   }
}
if (!cmp)
continue; /* we will link it later */
@@ -568,6 +569,12 @@ static void filter_refs(struct ref **refs, int *nr_heads, 
char **heads)
 
if (!args.fetch_all) {
int i;
+
+   /* copy remaining unmatched heads: */
+   while (match_pos  *nr_heads)
+   heads[unmatched++] = heads[match_pos++];
+   *nr_heads = unmatched;
+
for (i = 0; i  matched; i++) {
ref = return_refs[i];
*newtail = ref;
@@ -1028,10 +1035,8 @@ int cmd_fetch_pack(int argc, const char **argv, const 
char *prefix)
 * silently succeed without issuing an error.
 */
for (i = 0; i  nr_heads; i++)
-   if (heads[i]  heads[i][0]) {
-   error(no such remote ref %s, heads[i]);
-   ret = 1;
-   }
+   error(no such remote ref %s, heads[i]);
+   ret = 1;
}
while (ref) {
printf(%s %s\n,
diff --git a/fetch-pack.h b/fetch-pack.h
index a9d505b..915858e 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -17,6 +17,12 @@ struct fetch_pack_args {
stateless_rpc:1;
 };
 
+/*
+ * (*nr_heads, heads) is an array of pointers to the full names of
+ * remote references that should be updated from.  On return, both
+ * will have been changed to list only the names that were not found
+ * on the remote.
+ */
 struct ref *fetch_pack(struct fetch_pack_args *args,
   int fd[], struct child_process *conn,
   const struct ref *ref,
diff --git a/transport.c b/transport.c
index f7bbf58..3c38d44 100644
--- a/transport.c
+++ b/transport.c
@@ -520,6 +520,7 @@ static int fetch_refs_via_pack(struct transport *transport,
struct git_transport_data *data = transport-data;
char **heads = xmalloc(nr_heads * sizeof(*heads));
char **origh = xmalloc(nr_heads * sizeof(*origh));
+   int orig_nr_heads = nr_heads;
const struct ref *refs;
char *dest = xstrdup(transport-url);
struct fetch_pack_args args;
@@ -558,7 +559,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 
free_refs(refs_tmp);
 
-   for (i = 0; i  nr_heads; i++)
+   for (i = 0; i  orig_nr_heads; i++)
free(origh[i]);
free(origh);
free(heads);
-- 
1.7.11.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


[PATCH 14/17] Report missing refs even if no existing refs were received

2012-08-23 Thread mhagger
From: Michael Haggerty mhag...@alum.mit.edu

This fixes a test in t5500.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/fetch-pack.c  | 2 +-
 t/t5500-fetch-pack.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 4794839..5b1e603 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1028,7 +1028,7 @@ int cmd_fetch_pack(int argc, const char **argv, const 
char *prefix)
return 1;
 
ret = !ref;
-   if (ref  nr_heads) {
+   if (nr_heads) {
/* If the heads to pull were given, we should have
 * consumed all of them by matching the remote.
 * Otherwise, 'git fetch remote no-such-ref' would
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 0d4edcb..f78a118 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -403,7 +403,7 @@ test_expect_success 'set up tests of missing reference' '
EOF
 '
 
-test_expect_failure 'test lonely missing ref' '
+test_expect_success 'test lonely missing ref' '
(
cd client 
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy
-- 
1.7.11.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


Re: git on HP NonStop

2012-08-23 Thread Andreas Ericsson
On 08/22/2012 06:38 PM, Joachim Schmitz wrote:
 
 
 -Original Message-
 From: Junio C Hamano [mailto:gits...@pobox.com]
 Sent: Tuesday, August 21, 2012 4:06 AM
 To: Joachim Schmitz
 Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org
 Subject: Re: git on HP NonStop

 Joachim Schmitz j...@schmitz-digital.de writes:

 OK, so let's have a look at code, current git, builtin/cat-file.c,
 line 196:
  void *contents = contents;

 This variable is set later in an if branch (if (print_contents ==
 BATCH), but not in the else branch. It is later used always under the
 same condition as the one under which it is set.
 Apparently is is malloc_d storage (there a free(content);), so
 there's no harm al all in initializing it with NULL, even if it only
 appeases a stupid compiler.

 It actually is harmful.  See below.
 
 Harmful to initialize with NULL or to use that undefined behavoir?
 
 I checked what our compiler does here: after having warned about vlues us
 used before it is set: it actually dies seem to have initializes the value
 to 0 resp. NULL.
 So here there's no harm done in avoiding undefined behavior and set it to 0
 resp NULL in the first place.
 

There is harm in tricking future programmers into thinking that the
initialization actually means something, which some of them do.

It's unlikely that you're the one to maintain that code forever, and
the var = var idiom is used widely within git with a clear meaning
as a hint to programmers who read a bit of git code. If they aren't
used to that idiom, they usually investigate it in the code and
pretty quickly realize that what it means.


-- 
Andreas Ericsson   andreas.erics...@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
To unsubscribe from this list: send the line 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] specifying ranges: we did not mean to make .. an empty set

2012-08-23 Thread Jeff King
On Wed, Aug 22, 2012 at 03:59:43PM -0700, Junio C Hamano wrote:

 Date: Mon, 2 May 2011 13:39:16 -0700
 
 Either end of revision range operator can be omitted to default to HEAD,
 as in origin.. (what did I do since I forked) or ..origin (what did
 they do since I forked).  But the current parser interprets ..  as an
 empty range HEAD..HEAD, and worse yet, because .. does exist on the
 filesystem, we get this annoying output:
 
   $ cd Documentation/howto
   $ git log .. ;# give me recent commits that touch Documentation/ area.
   fatal: ambiguous argument '..': both revision and filename
   Use '--' to separate filenames from revisions
 
 Surely we could say git log ../ or even git log -- .. to disambiguate,
 but we shouldn't have to.
 
 Helped-by: Jeff King p...@peff.net
 Signed-off-by: Junio C Hamano gits...@pobox.com

Hmm, for some reason I had no recollection of the original thread at
all. And yet reading the archives, I apparently had quite a bit to say.
Reading again with fresh eyes, I still think this is sane.

I don't think assigning any revision magic to .. besides the empty
range makes sense at all for the reasons you gave in the original
thread. And the empty range is a pointless no-op. So I don't see any
real argument in favor of disambiguating towards the revision.

So the only reasonable choices are to leave it as an error, or to
disambiguate towards the pathspec.  I suppose some script could stupidly
be doing git log $a..$b and would prefer to error out when both are
blank. But that seems unlikely, and making .. work is actually useful.

 --- a/Documentation/revisions.txt
 +++ b/Documentation/revisions.txt
 @@ -213,6 +213,13 @@ of 'r1' and 'r2' and is defined as
  It is the set of commits that are reachable from either one of
  'r1' or 'r2' but not from both.
  
 +In these two shorthands, you can omit one end and let it default to HEAD.
 +For example, 'origin..' is a shorthand for 'origin..HEAD' and asks What
 +did I do since I forked from the origin branch?  Similarly, '..origin'
 +is a shorthand for 'HEAD..origin' and asks What did the origin do since
 +I forked from them?  Note that '..' would mean 'HEAD..HEAD' which is an
 +empty range that is both reachable and unreachable from HEAD.

This last sentence confuses me. Now we are documenting that yes, ..
really means HEAD..HEAD, which is the empty range. But isn't the point
of this patch to say sure, it would be the empty range, but because
that is stupid and pointless, we do not consider it valid and treat ..
as a pathspec?

I think that may be what you are trying to say with the would in that
sentence, but perhaps this would be a good point to expand and mention
that we special-case ...

 +test_expect_success 'dotdot is not an empty set' '
 + ( H=$(git rev-parse HEAD)  echo $H ; echo ^$H ) expect 

It almost certainly doesn't matter in practice, but the ';' here would
break the -chain from rev-parse.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/17] Name local variables more consistently

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 10:10:29AM +0200, mhag...@alum.mit.edu wrote:

 From: Michael Haggerty mhag...@alum.mit.edu
 
 Use the names (nr_heads, heads) consistently across functions, instead
 of sometimes naming the same values (nr_match, match).

I think this is fine, although:

 --- a/builtin/fetch-pack.c
 +++ b/builtin/fetch-pack.c
 @@ -521,7 +521,7 @@ static void mark_recent_complete_commits(unsigned long 
 cutoff)
   }
  }
  
 -static void filter_refs(struct ref **refs, int nr_match, char **match)
 +static void filter_refs(struct ref **refs, int nr_heads, char **heads)
  {
   struct ref **return_refs;
   struct ref *newlist = NULL;
 @@ -530,12 +530,12 @@ static void filter_refs(struct ref **refs, int 
 nr_match, char **match)
   struct ref *fastarray[32];
   int match_pos;

This match_pos is an index into the match array, which becomes head.
Should it become head_pos?

And then bits like this:

 - while (match_pos  nr_match) {
 - cmp = strcmp(ref-name, match[match_pos]);
 + while (match_pos  nr_heads) {
 + cmp = strcmp(ref-name, heads[match_pos]);

Would be:

  while (head_pos  nr_heads)

which makes more sense to me.

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


Re: [PATCH 05/17] Do not check the same match_pos twice

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 10:10:30AM +0200, mhag...@alum.mit.edu wrote:

 From: Michael Haggerty mhag...@alum.mit.edu
 
 Once a match has been found at match_pos, the entry is zeroed and no
 future attempts will match that entry.  So increment match_pos to
 avoid checking against the zeroed-out entry during the next iteration.

Good catch.

A subtle side effect of this zero-ing (not introduced by your patch, but
something I noticed while re-reading the code) is that we implicitly
eliminate duplicate entries from the list of remote refs. There
shouldn't generally be any duplicates, of course, but I think skipping
them is probably sane.

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


Re: [PATCH 06/17] Let fetch_pack() inform caller about number of unique heads

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 10:10:31AM +0200, mhag...@alum.mit.edu wrote:

 From: Michael Haggerty mhag...@alum.mit.edu
 
 fetch_pack() remotes duplicates from the list (nr_heads, heads),
 thereby shrinking the list.  But previously, the caller was not
 informed about the shrinkage.  This would cause a spurious error
 message to be emitted by cmd_fetch_pack() if git fetch-pack is
 called with duplicate refnames.
 
 So change the signature of fetch_pack() to accept nr_heads by
 reference, and if any duplicates were removed then modify it to
 reflect the number of remaining references.
 
 The last test of t5500 inexplicably *required* git fetch-pack to
 fail when fetching a list of references that contains duplicates;
 i.e., it insisted on the buggy behavior.  So change the test to expect
 the correct behavior.

Eek, yeah, the current behavior is obviously wrong. The
remove_duplicates code comes from 310b86d (fetch-pack: do not barf when
duplicate re patterns are given, 2006-11-25) and clearly meant for
fetch-pack to handle this case gracefully.

 diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
 index 3cc3346..0d4edcb 100755
 --- a/t/t5500-fetch-pack.sh
 +++ b/t/t5500-fetch-pack.sh
 @@ -391,7 +391,7 @@ test_expect_success 'fetch mixed refs from cmdline and 
 stdin' '
  test_expect_success 'test duplicate refs from stdin' '
   (
   cd client 
 - test_must_fail git fetch-pack --stdin --no-progress .. ../input.dup
 + git fetch-pack --stdin --no-progress .. ../input.dup
   ) output 
   cut -d   -f 2 output | sort actual 
   test_cmp expect actual

It's interesting that the output was the same before and after the fix.
I guess that is because the error comes at the very end, when we are
making sure all of the provided heads have been consumed.

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


Re: [PATCH 16/17] fetch_pack(): free matching heads

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 10:10:41AM +0200, mhag...@alum.mit.edu wrote:

 From: Michael Haggerty mhag...@alum.mit.edu
 
 fetch_pack() used to delete entries from the input list (*nr_heads,
 heads) and drop them on the floor.  (Even earlier versions dropped
 some names on the floor and modified others.)  This forced
 fetch_refs_via_pack() to create a separate copy of the original list
 so that it could free the all of the names.

Minor typo: s/the all/all/.

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


RE: git on HP NonStop

2012-08-23 Thread Joachim Schmitz
 From: Andreas Ericsson [mailto:a...@op5.se]
 Sent: Thursday, August 23, 2012 10:24 AM
 To: Joachim Schmitz
 Cc: 'Junio C Hamano'; 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org
 Subject: Re: git on HP NonStop
 
 On 08/22/2012 06:38 PM, Joachim Schmitz wrote:
 
 
  -Original Message-
  From: Junio C Hamano [mailto:gits...@pobox.com]
  Sent: Tuesday, August 21, 2012 4:06 AM
  To: Joachim Schmitz
  Cc: 'Johannes Sixt'; 'Jan Engelhardt'; git@vger.kernel.org
  Subject: Re: git on HP NonStop
 
  Joachim Schmitz j...@schmitz-digital.de writes:
 
  OK, so let's have a look at code, current git, builtin/cat-file.c,
  line 196:
   void *contents = contents;
 
  This variable is set later in an if branch (if (print_contents ==
  BATCH), but not in the else branch. It is later used always under the
  same condition as the one under which it is set.
  Apparently is is malloc_d storage (there a free(content);), so
  there's no harm al all in initializing it with NULL, even if it only
  appeases a stupid compiler.
 
  It actually is harmful.  See below.
 
  Harmful to initialize with NULL or to use that undefined behavoir?
 
  I checked what our compiler does here: after having warned about vlues us
  used before it is set: it actually dies seem to have initializes the value
  to 0 resp. NULL.
  So here there's no harm done in avoiding undefined behavior and set it to 0
  resp NULL in the first place.
 
 
 There is harm in tricking future programmers into thinking that the
 initialization actually means something, which some of them do.

Hmm, OK, I can agree to that.

 It's unlikely that you're the one to maintain that code forever, 

It is unlike for me to ever have to maintain this code.
Currently that's Junio's job and I won't apply for in ;-)

 and the var = var idiom is used widely within git 

This is overstating it a bit. I went thru the entire code and reported all 
places I could find in an earlier email.
I went back and counted: It is used in 11 files, at 15 places, for 21 
variables. 
OK, I may have missed  a few more that were in code paths my compiler didn't 
see, but still some 21+ isn't really much.

with a clear meaning

Only if you call undefined behavior a  'clear meaning!

 as a hint to programmers who read a bit of git code. If they aren't
 used to that idiom, they usually investigate it in the code and
 pretty quickly realize that what it means.

Whether I realize what it means, is irrelevant, my compiler does not and warns 
about it, and as per the ANSI/ICO C standard it
invokes undefined behavior.

If a proper initialization is meaningless for these cases, don't do them at 
all, let the stupid compiler complain about it and the
clever programmer check whether the warning is useful, but don't avoid a 
compiler warning on one compiler by introducing undefined
behavior and provoke a compiler warning on another.

Bye, Jojo

--
To unsubscribe from this list: send the line 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 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 10:10:25AM +0200, mhag...@alum.mit.edu wrote:

 There were various confusing things (and a couple of bugs) in the way
 that fetch_pack() handled the list (nr_heads, heads) of references to
 be sought from the remote:

Aside from the minor comments I made to individual patches, this all
looks good. As usual, thanks for breaking it down; I wish all series
were as easy to review as this.

 I'm still suspicious about the logic related to args.fetch_all and
 args.depth, but I don't think I've made anything worse.

I think the point of that is that when doing git fetch-pack --all
--depth=1, the meaning of --all is changed from all refs to
everything but tags.

Which I kind of see the point of, because you don't want to grab ancient
tags that will be expensive. But wouldn't it make more sense to limit it
only to the contents of refs/heads in that case? Surely you wouldn't
want refs/notes, refs/remotes, or other hierarchies.

I suspect this code is never even run at all these days. All of the
callers inside git should actually provide a real list of refs, not
--all. So it is really historical cruft for anybody who calls the
fetch-pack plumbing (I wonder if any third-party callers even exist;
this is such a deep part of the network infrastructure that any sane
scripts would probably just be calling fetch).

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


Re: [PATCH] l10n: preserve trailing spaces in Vietnamese translation

2012-08-23 Thread Nguyen Thai Ngoc Duy
2012/8/22 Nguyễn Thái Ngọc Duy pclo...@gmail.com:
 The trailing spaces in msgid can be used to separate the next
 word. Accidentally removing it means we may see On branchmaster
 instead of On branch master.

I don't speak Portugese but I think that translation has the same
problem. This is on git.git by the way, not git-l10n repository.

#: wt-status.c:737
msgid On branch 
msgstr Na rama

  #: wt-status.c:978
  msgid On branch 
 -msgstr Trên nhánh
 +msgstr Trên nhánh 
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Support generate poison .mo files for testing

2012-08-23 Thread Nguyen Thai Ngoc Duy
On Wed, Aug 22, 2012 at 11:22 PM, Junio C Hamano gits...@pobox.com wrote:
 Nguyen Thai Ngoc Duy pclo...@gmail.com writes:

 But a better way could be
 replacing tracked with t r a c k e d. We know the rule so we can
 recreate the that string from tracked in test_i18n*. Or reverse the
 upper/lower case, whichever is easier for the recreation by test_i18n*

 That does not make much sense to me, so either one of us must be
 slightly confused.  I thought the only purpose of testing with the
 poison was to find messages that must not be localized but were
 localized by mistake.  For that, we have to make sure that anything
 that uses test_i18n* is reading from Porcelain, in other words, we
 must use the byte-for-byte comparison without using test_i18n* when
 verifying the plumbing output.  And the primary requirement for this
 arrangement to work is that the expected output in C locale and the
 actual ouptut in the synthetic poison locale are reliably different.
 They do not have to be reversible (I was actually going to suggest
 rot13 of the original instead of cycling through the * gettext
 poison * in your patch --- prefixing with QQ would not work, as it
 is likely that the test with grep may not be anchored at the left
 end).

Yes, for verifying plumbing output that should be enough.

 Teaching test_i18n* to fuzzily match the expected output in C locale
 against the actual output in synthetic poison locale may or may not
 be doable, but spending any cycle working on that sounds like a
 total waste of time (even though it might be fun).  It does not test
 that we are translating Porcelain messages correctly.

Right. I was aiming at testing translated messages but obviously went
off track trying to test a fake locale instead. Thanks for stopping
me.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Support generate poison .mo files for testing

2012-08-23 Thread Nguyen Thai Ngoc Duy
On Wed, Aug 22, 2012 at 11:43 PM, Junio C Hamano gits...@pobox.com wrote:
 why are you special casing a run of non-blank letters that begin
 with a dollar sign (swapping two ints is done with %2$d %1$d, a
 percent still at the beginning, so there must be something else I am
 missing)?

'$' is for shell variables. I should separate it from C messages. All
these because now that we run the (fake) translation through gettext
toolchain, it'll catch us if we don't preserve dynamic parts
correctly.

 Also why do you stop at isspace()?  Isn't a   (space) a flag that
 means If the first character of a signed conversion is not a sign
 or if a signed conversion results in no characters, a space shall
 be prefixed to the result.

A hurry attempt to get past msgfmt. I should refine these code, either by...

 As the flags, min-width, precision, and length do not share the same
 character as the conversion that has to come at the end, I think you
 only want to do something like

 /*
  * conversion specifier characters, taken from:
  * 
 http://pubs.opengroup.org/onlinepubs/9699919799/functions/printf.html
  */
 static const char printf_conversion[] = diouxXfFeEgGaAcspnCS%;

 ...

 while (msg  end) {
 if (*msg == '%') {
 strbuf_addch(buf, *msg++);
 while (msg  end) {
 int ch = *msg++;
 strbuf_addch(buf, ch);
 if (strchr(printf_conversion, ch))
 break;
 }
 /* copied the printf part literally */
 continue;
 }
 ... keep \n ...
 ... muck with string ...
 }

 perhaps?

following this, or copying the matching logic from msgfmt source.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] specifying ranges: we did not mean to make .. an empty set

2012-08-23 Thread Thomas Rast
Jeff King p...@peff.net writes:

 On Wed, Aug 22, 2012 at 03:59:43PM -0700, Junio C Hamano wrote:

 Either end of revision range operator can be omitted to default to HEAD,
 as in origin.. (what did I do since I forked) or ..origin (what did
 they do since I forked).  But the current parser interprets ..  as an
 empty range HEAD..HEAD, and worse yet, because .. does exist on the
 filesystem, we get this annoying output:
 
   $ cd Documentation/howto
   $ git log .. ;# give me recent commits that touch Documentation/ area.
   fatal: ambiguous argument '..': both revision and filename
   Use '--' to separate filenames from revisions
 
 Surely we could say git log ../ or even git log -- .. to disambiguate,
 but we shouldn't have to.
 
 Helped-by: Jeff King p...@peff.net
 Signed-off-by: Junio C Hamano gits...@pobox.com

 Hmm, for some reason I had no recollection of the original thread at
 all. And yet reading the archives, I apparently had quite a bit to say.
 Reading again with fresh eyes, I still think this is sane.

 I don't think assigning any revision magic to .. besides the empty
 range makes sense at all for the reasons you gave in the original
 thread. And the empty range is a pointless no-op. So I don't see any
 real argument in favor of disambiguating towards the revision.

I don't think that .. is really a no-op.  It is true that HEAD..HEAD
does not itself result in any revisions, but it *could* be used as a
silly shorthand to introduce ^HEAD into the objects being walked.  This
can make a difference if it then excludes other objects, too.

I would argue that such use is misguided, and I am in favor of the
patch, but in theory it is possible.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


in_merge_bases() is too expensive for recent pu update

2012-08-23 Thread Nguyen Thai Ngoc Duy
I just did a git fetch. It took 19 seconds (measured with
gettimeofday) to finish in_merge_bases() in update_local_ref() in
fetch.c, just to print this line

 + a4f2db3...b95a282 pu - origin/pu  (forced update)

It's partly my fault because I'm on gcc -O0. But should it not take
that much time for one line? As a grumpy user, I'd be perfectly ok
with git saying probably forced update, check it yourself if the
spent time exceeds half a second. On the same token, if diffstat takes
too long for git-pull, then perhaps just stop and print do it
yourself.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: in_merge_bases() is too expensive for recent pu update

2012-08-23 Thread Thomas Rast
Nguyen Thai Ngoc Duy pclo...@gmail.com writes:

 I just did a git fetch. It took 19 seconds (measured with
 gettimeofday) to finish in_merge_bases() in update_local_ref() in
 fetch.c, just to print this line

  + a4f2db3...b95a282 pu - origin/pu  (forced update)

 It's partly my fault because I'm on gcc -O0. But should it not take
 that much time for one line? As a grumpy user, I'd be perfectly ok
 with git saying probably forced update, check it yourself if the
 spent time exceeds half a second. On the same token, if diffstat takes
 too long for git-pull, then perhaps just stop and print do it
 yourself.

I missed out on the a4f2db3 update, so I can't test exactly this, but my
reflog tells the I had before b95a282 was d3dd556.  Computing all
merge-bases takes very long indeed:

  $ time git merge-base --all d3dd556 b95a282 | wc -l
  51

  real0m4.877s
  user0m4.829s
  sys 0m0.026s

And I think the reason is that the algorithm is just doing too much.
From a quick glance, it appears to first compute the result list rslt[]
of merge bases, then for each pair attempts to eliminate one of them by
determining that rslt[i] or rslt[j] is also a merge base of rslt[j:].
It is thus spending quadratic time in the number of merge-bases, times
the size of history (ok, I can't actually come up with an example that
does the latter).

At the very least it should be possible to change in_merge_bases() to
not do any of the post-filtering; perhaps like the patch below.  It
passes the test suite.  The whole merge bases of A and a list of Bs
thing is blowing my overheated mind, though, so I'm not able to convince
myself that it is correct in all cases.

Even if this turns out to be flawed, we should also identify uses of
in_merge_bases() where the real question was is_descendant_of() [I
somewhat suspect that's all of them], and then replace is_descendant_of
with a much cheaper lookup.  This can be as simple as propagating a mark
from the candidate until it either goes beyond all possible ancestors,
or hits one of them.

By the way, the internal slowness of git-merge-base also affects the
A...B syntax.  For example,

  git rev-list --left-right --count @{upstream}...

is used by the __git_ps1 code to determine the prompt display, which
just got very slow for me today.  Again, it should be easy to figure out
the boundary of the symmetric difference simply by propagating two
marks.  I do not think that the result of A...B actually depends on
figuring out exactly what the merge bases are, simply excluding *any*
candidate without pruning is enough.


diff --git i/commit.c w/commit.c
index 65a8485..70427ab 100644
--- i/commit.c
+++ w/commit.c
@@ -837,10 +837,13 @@ int in_merge_bases(struct commit *commit, struct commit 
**reference, int num)
struct commit_list *bases, *b;
int ret = 0;
 
-   if (num == 1)
-   bases = get_merge_bases(commit, *reference, 1);
-   else
+   if (num != 1)
die(not yet);
+
+   bases = merge_bases_many(commit, 1, reference);
+   clear_commit_marks(commit, all_flags);
+   clear_commit_marks(*reference, all_flags);
+   
for (b = bases; b; b = b-next) {
if (!hashcmp(commit-object.sha1, b-item-object.sha1)) {
ret = 1;


-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line 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: a small git proposal

2012-08-23 Thread Hilco Wijbenga
On 23 August 2012 08:10, Catalin Pol catalin@gmail.com wrote:
 Hi everyone,

 This is my first email to this mailing list, so this may be somehow
 too straight forward... the idea is that I was thinking to develop a
 new feature in Git (although I'm kind of new to git myself).
 I wrote a small description of what I intend to do and I figured I
 could use some pointers (how I can improve it, any possible usage
 scenarios you can think for it and so on). Details are available at
 the gist link below or as attachment to this email (I zipped the text
 file since it was more it is larger than 10k and I didn't want it to
 get rejected by the email server... although it still might)

 gist link:https://gist.github.com/3437530

 I made the gist public, so feel free to edit it directly... or, if you
 prefer, just email me with any comments. I'm opened to any suggestion,
 so feel free to send me any kind of comment (maybe I'm trying to
 implement something that is already in git for example, and since I'm
 a bit of a newbie in the git world, I didn't notice any way to obtain
 my desired workflow).

 Thanks in advance for any feedback,

Have you looked at git notes?
--
To unsubscribe from this list: send the line 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 svn: Only follow first parents when populating svn:mergeinfo properties

2012-08-23 Thread Avishay Lavie
Subject: [PATCH] git svn: Only follow first parents when populating
svn:mergeinfo
 properties.

When svn.pushmergeinfo is set, git-svn tries to correctly populate mergeinfo
properties when encountering a merge commit. It does so by first aggregating
the mergeinfo property of the merged parent into the target, and then
adding to it the SVN revision number of any commit reachable from the
merged parent but not from the first (target) parent.

If a third branch was merged into the merged parent (e.g. X was merged into Y
and Y was then merged into Z), its revisions will be listed twice --
once as part
of aggregating Y's mergeinfo property into Z's, and once more when walking
the tree and finding X's commits reachable from Y's tip. While the first listing
correctly lists those revisions as merged from X, the second listing
will list them
as merged from Y, creating incorrect mergeinfo properties that later cause
unnecessary lookups and warnings when git-svn-fetching.

Adding '--first-parent' to the rev-list command fixes this by only walking the
part of the tree that's directly included in the merged branch (Y) and not any
branches merged into it (X).

Signed-off-by: Avishay Lavie avishay.la...@gmail.com
---
 git-svn.perl |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-svn.perl b/git-svn.perl
index 828b8f0..f69a4d6 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -728,7 +728,7 @@ sub populate_merge_info {

  next if $parent eq $parents[0]; # Skip first parent
  # Add new changes being placed in tree by merge
- my @cmd = (qw/rev-list --reverse/,
+ my @cmd = (qw/rev-list --first-parent --reverse/,
$parent, qw/--not/);
  foreach my $par (@parents) {
  unless ($par eq $parent) {
-- 
1.7.10.msysgit.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] contrib: GnomeKeyring support + generic helper implementation

2012-08-23 Thread Philipp A. Hartmann
All,

the following patch series proposes enhancements to the credential helper
implementations in the contrib section.  The detailed development history
can be found at GitHub [1].

  The first patch adds a GnomeKeyring credential backend.  The GnomeKeyring
specific parts are based on the work by John Szakmeister, who wrote the
helper originally for the initial, unpublished version of the credential
helper protocol.

  The second patch adds a generic implementation of a credential helper
based on a simplified credential API and common helper functions.  Helpers
based on this implementation do not need to worry about the credential
protocol and only need to implement callback functions for the different
credential operations.

  The third and fourth patches port the existing helpers to this generic
implementation.

Looking forward to your thoughts.

Greetings from Oldenburg,
  Philipp

[1] https://github.com/pah/git-credential-helper

--
To unsubscribe from this list: send the line 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] contrib: add credential helper for GnomeKeyring

2012-08-23 Thread Philipp A. Hartmann
From: Philipp A. Hartmann p...@qo.cx

With this installed in your $PATH, you can store
git-over-http passwords in your keyring by doing:

git config credential.helper gnome-keyring

The code is based in large part on the work of John Szakmeister
who wrote the helper originally for the initial, unpublished
version of the credential helper protocol.

This version will pass t0303 if you do:

  GIT_TEST_CREDENTIAL_HELPER=gnome-keyring \
  ./t0303-credential-external.sh

Signed-off-by: Philipp A. Hartmann p...@qo.cx
---
 contrib/credential/gnome-keyring/.gitignore|1 +
 contrib/credential/gnome-keyring/Makefile  |   24 ++
 .../gnome-keyring/git-credential-gnome-keyring.c   |  445 
 3 files changed, 470 insertions(+)
 create mode 100644 contrib/credential/gnome-keyring/.gitignore
 create mode 100644 contrib/credential/gnome-keyring/Makefile
 create mode 100644 
contrib/credential/gnome-keyring/git-credential-gnome-keyring.c

diff --git a/contrib/credential/gnome-keyring/.gitignore 
b/contrib/credential/gnome-keyring/.gitignore
new file mode 100644
index 000..88d8fcd
--- /dev/null
+++ b/contrib/credential/gnome-keyring/.gitignore
@@ -0,0 +1 @@
+git-credential-gnome-keyring
diff --git a/contrib/credential/gnome-keyring/Makefile 
b/contrib/credential/gnome-keyring/Makefile
new file mode 100644
index 000..e6561d8
--- /dev/null
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -0,0 +1,24 @@
+MAIN:=git-credential-gnome-keyring
+all:: $(MAIN)
+
+CC = gcc
+RM = rm -f
+CFLAGS = -g -O2 -Wall
+
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+INCS:=$(shell pkg-config --cflags gnome-keyring-1)
+LIBS:=$(shell pkg-config --libs gnome-keyring-1)
+
+SRCS:=$(MAIN).c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+   $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $
+
+$(MAIN): $(OBJS)
+   $(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
+
+clean:
+   @$(RM) $(MAIN) $(OBJS)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
new file mode 100644
index 000..41f61c5
--- /dev/null
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -0,0 +1,445 @@
+/*
+ * Copyright (C) 2011 John Szakmeister j...@szakmeister.net
+ *   2012 Philipp A. Hartmann p...@qo.cx
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * Credits:
+ * - GNOME Keyring API handling originally written by John Szakmeister
+ * - ported to credential helper API by Philipp A. Hartmann
+ */
+
+#include stdio.h
+#include string.h
+#include stdarg.h
+#include stdlib.h
+#include errno.h
+#include gnome-keyring.h
+
+/*
+ * This credential struct and API is simplified from git's credential.{h,c}
+ */
+struct credential
+{
+   char  *protocol;
+   char  *host;
+   unsigned short port;
+   char  *path;
+   char  *username;
+   char  *password;
+};
+
+#define CREDENTIAL_INIT \
+  { NULL,NULL,0,NULL,NULL,NULL }
+
+void credential_init(struct credential *c);
+void credential_clear(struct credential *c);
+int  credential_read(struct credential *c);
+void credential_write(const struct credential *c);
+
+typedef int (*credential_op_cb)(struct credential*);
+
+struct credential_operation
+{
+   char *name;
+   credential_op_cb op;
+};
+
+#define CREDENTIAL_OP_END \
+  { NULL,NULL }
+
+/*
+ * Table with operation callbacks is defined in concrete
+ * credential helper implementation and contains entries
+ * like { get, function_to_get_credential } terminated
+ * by CREDENTIAL_OP_END.
+ */
+struct credential_operation const credential_helper_ops[];
+
+/*  common helper functions - */
+
+static inline void free_password(char *password)
+{
+   char *c = password;
+   if (!password)
+   return;
+
+   while (*c) *c++ = '\0';
+   free(password);
+}
+
+static inline void warning(const char *fmt, ...)
+{
+   va_list ap;
+
+   va_start(ap, fmt);
+   fprintf(stderr, warning: );
+   vfprintf(stderr, fmt, ap);
+   fprintf(stderr, \n );
+   va_end(ap);
+}
+
+static inline void error(const char *fmt, ...)
+{
+   va_list ap;
+
+   va_start(ap, fmt);
+   

[PATCH 3/4] gnome-keyring: port to generic helper implementation

2012-08-23 Thread Philipp A. Hartmann
From: Philipp A. Hartmann p...@qo.cx

Use generic credential helper implementation in the
GnomeKeyring credential helper.

The GnomeKeyring helper has been using the generic implementation
internally already and therefore only drops the duplicate code.

Signed-off-by: Philipp A. Hartmann p...@qo.cx
---
 contrib/credential/gnome-keyring/Makefile  |6 +-
 .../gnome-keyring/git-credential-gnome-keyring.c   |  243 +---
 2 files changed, 6 insertions(+), 243 deletions(-)

diff --git a/contrib/credential/gnome-keyring/Makefile 
b/contrib/credential/gnome-keyring/Makefile
index e6561d8..7f3ec11 100644
--- a/contrib/credential/gnome-keyring/Makefile
+++ b/contrib/credential/gnome-keyring/Makefile
@@ -11,11 +11,15 @@ CFLAGS = -g -O2 -Wall
 INCS:=$(shell pkg-config --cflags gnome-keyring-1)
 LIBS:=$(shell pkg-config --libs gnome-keyring-1)
 
+HELPER:=../helper
+VPATH +=$(HELPER)
+
 SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
 OBJS:=$(SRCS:.c=.o)
 
 %.o: %.c
-   $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $
+   $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) $(INCS) -o $@ -c $
 
 $(MAIN): $(OBJS)
$(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c 
b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index 41f61c5..00244aa 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -23,114 +23,9 @@
  * - ported to credential helper API by Philipp A. Hartmann
  */
 
-#include stdio.h
-#include string.h
-#include stdarg.h
-#include stdlib.h
-#include errno.h
+#include credential_helper.h
 #include gnome-keyring.h
 
-/*
- * This credential struct and API is simplified from git's credential.{h,c}
- */
-struct credential
-{
-   char  *protocol;
-   char  *host;
-   unsigned short port;
-   char  *path;
-   char  *username;
-   char  *password;
-};
-
-#define CREDENTIAL_INIT \
-  { NULL,NULL,0,NULL,NULL,NULL }
-
-void credential_init(struct credential *c);
-void credential_clear(struct credential *c);
-int  credential_read(struct credential *c);
-void credential_write(const struct credential *c);
-
-typedef int (*credential_op_cb)(struct credential*);
-
-struct credential_operation
-{
-   char *name;
-   credential_op_cb op;
-};
-
-#define CREDENTIAL_OP_END \
-  { NULL,NULL }
-
-/*
- * Table with operation callbacks is defined in concrete
- * credential helper implementation and contains entries
- * like { get, function_to_get_credential } terminated
- * by CREDENTIAL_OP_END.
- */
-struct credential_operation const credential_helper_ops[];
-
-/*  common helper functions - */
-
-static inline void free_password(char *password)
-{
-   char *c = password;
-   if (!password)
-   return;
-
-   while (*c) *c++ = '\0';
-   free(password);
-}
-
-static inline void warning(const char *fmt, ...)
-{
-   va_list ap;
-
-   va_start(ap, fmt);
-   fprintf(stderr, warning: );
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n );
-   va_end(ap);
-}
-
-static inline void error(const char *fmt, ...)
-{
-   va_list ap;
-
-   va_start(ap, fmt);
-   fprintf(stderr, error: );
-   vfprintf(stderr, fmt, ap);
-   fprintf(stderr, \n );
-   va_end(ap);
-}
-
-static inline void die(const char *fmt, ...)
-{
-   va_list ap;
-
-   va_start(ap,fmt);
-   error(fmt, ap);
-   va_end(ap);
-   exit(EXIT_FAILURE);
-}
-
-static inline void die_errno(int err)
-{
-   error(%s, strerror(err));
-   exit(EXIT_FAILURE);
-}
-
-static inline char *xstrdup(const char *str)
-{
-   char *ret = strdup(str);
-   if (!ret)
-   die_errno(errno);
-
-   return ret;
-}
-
-/* - GNOME Keyring functions - */
-
 /* create a special keyring option string, if path is given */
 static char* keyring_object(struct credential *c)
 {
@@ -307,139 +202,3 @@ struct credential_operation const credential_helper_ops[] 
=
{ erase, keyring_erase },
CREDENTIAL_OP_END
 };
-
-/* -- credential functions -- */
-
-void credential_init(struct credential *c)
-{
-   memset(c, 0, sizeof(*c));
-}
-
-void credential_clear(struct credential *c)
-{
-   free(c-protocol);
-   free(c-host);
-   free(c-path);
-   free(c-username);
-   free_password(c-password);
-
-   credential_init(c);
-}
-
-int credential_read(struct credential *c)
-{
-   charbuf[1024];
-   ssize_t line_len = 0;
-   char   *key  = buf;
-   char   *value;
-
-   while (fgets(buf, sizeof(buf), stdin))
-   {
-   line_len = strlen(buf);
-
-   if(buf[line_len-1]=='\n')
-   buf[--line_len]='\0';
-
-   if(!line_len)
-  

[PATCH 4/4] osxkeychain: port to generic credential helper implementation

2012-08-23 Thread Philipp A. Hartmann
From: Philipp A. Hartmann p...@qo.cx

This reduces code duplication in the osxkeychain helper by
basing the implementation on the generic helper implementation.

Alongside, the return codes of the helper are tightened to be
more consistent in corner cases and the memory containing
cleartext passwords is explicitly cleared when possible.

Signed-off-by: Philipp A. Hartmann p...@qo.cx
---
 contrib/credential/helper/credential_helper.h  |9 +
 contrib/credential/osxkeychain/Makefile|   22 ++-
 .../osxkeychain/git-credential-osxkeychain.c   |  183 
 3 files changed, 96 insertions(+), 118 deletions(-)

diff --git a/contrib/credential/helper/credential_helper.h 
b/contrib/credential/helper/credential_helper.h
index 8266078..76b6e50 100644
--- a/contrib/credential/helper/credential_helper.h
+++ b/contrib/credential/helper/credential_helper.h
@@ -113,4 +113,13 @@ static inline char *xstrdup(const char *str)
return ret;
 }
 
+static inline char *xstrndup(const char *str, size_t len)
+{
+   char *ret = strndup(str,len);
+   if (!ret)
+   die_errno(errno);
+
+   return ret;
+}
+
 #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
diff --git a/contrib/credential/osxkeychain/Makefile 
b/contrib/credential/osxkeychain/Makefile
index 4b3a08a..64ee7c5 100644
--- a/contrib/credential/osxkeychain/Makefile
+++ b/contrib/credential/osxkeychain/Makefile
@@ -1,4 +1,5 @@
-all:: git-credential-osxkeychain
+MAIN:=git-credential-osxkeychain
+all:: $(MAIN)
 
 CC = gcc
 RM = rm -f
@@ -7,11 +8,20 @@ CFLAGS = -g -O2 -Wall
 -include ../../../config.mak.autogen
 -include ../../../config.mak
 
-git-credential-osxkeychain: git-credential-osxkeychain.o
-   $(CC) $(CFLAGS) -o $@ $ $(LDFLAGS) -Wl,-framework -Wl,Security
+INCS:=
+LIBS:=-Wl,-framework -Wl,Security
+HELPER:=../helper
+VPATH +=$(HELPER)
 
-git-credential-osxkeychain.o: git-credential-osxkeychain.c
-   $(CC) -c $(CFLAGS) $
+SRCS:=$(MAIN).c
+SRCS+=credential_helper.c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+   $(CC) $(CFLAGS) $(CPPFLAGS) -I$(HELPER) $(INCS) -o $@ -c $
+
+$(MAIN): $(OBJS)
+   $(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
 
 clean:
-   $(RM) git-credential-osxkeychain git-credential-osxkeychain.o
+   @$(RM) $(MAIN) $(OBJS)
diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c 
b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index 6beed12..60bd973 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -1,53 +1,40 @@
+
+#include credential_helper.h
+
 #include stdio.h
 #include string.h
 #include stdlib.h
 #include Security/Security.h
 
 static SecProtocolType protocol;
-static char *host;
-static char *path;
-static char *username;
-static char *password;
-static UInt16 port;
-
-static void die(const char *err, ...)
-{
-   char msg[4096];
-   va_list params;
-   va_start(params, err);
-   vsnprintf(msg, sizeof(msg), err, params);
-   fprintf(stderr, %s\n, msg);
-   va_end(params);
-   exit(1);
-}
-
-static void *xstrdup(const char *s1)
-{
-   void *ret = strdup(s1);
-   if (!ret)
-   die(Out of memory);
-   return ret;
-}
 
 #define KEYCHAIN_ITEM(x) (x ? strlen(x) : 0), x
-#define KEYCHAIN_ARGS \
+#define KEYCHAIN_ARGS(c) \
NULL, /* default keychain */ \
-   KEYCHAIN_ITEM(host), \
+   KEYCHAIN_ITEM(c-host), \
0, NULL, /* account domain */ \
-   KEYCHAIN_ITEM(username), \
-   KEYCHAIN_ITEM(path), \
-   port, \
+   KEYCHAIN_ITEM(c-username), \
+   KEYCHAIN_ITEM(c-path), \
+   (UInt16) c-port, \
protocol, \
kSecAuthenticationTypeDefault
 
-static void write_item(const char *what, const char *buf, int len)
+static int prepare_internet_password(struct credential *c)
 {
-   printf(%s=, what);
-   fwrite(buf, 1, len, stdout);
-   putchar('\n');
+   if (!c-protocol)
+   return -1;
+   else if (!strcmp(c-protocol, https))
+   protocol = kSecProtocolTypeHTTPS;
+   else if (!strcmp(c-protocol, http))
+   protocol = kSecProtocolTypeHTTP;
+   else /* we don't yet handle other protocols */
+   return -1;
+
+   return 0;
 }
 
-static void find_username_in_item(SecKeychainItemRef item)
+static void
+find_username_in_item(SecKeychainItemRef item, struct credential *c)
 {
SecKeychainAttributeList list;
SecKeychainAttribute attr;
@@ -59,27 +46,37 @@ static void find_username_in_item(SecKeychainItemRef item)
if (SecKeychainItemCopyContent(item, NULL, list, NULL, NULL))
return;
 
-   write_item(username, attr.data, attr.length);
+   free(c-username);
+   c-username = xstrndup(attr.data, attr.length);
+
SecKeychainItemFreeContent(list, NULL);
 }
 
-static void find_internet_password(void)
+static int find_internet_password(struct credential *c)
 {
  

[PATCH 2/4] contrib: add generic credential helper

2012-08-23 Thread Philipp A. Hartmann
From: Philipp A. Hartmann p...@qo.cx

This adds a generic implementation for credential helpers.

It provides a header file credential_helper.h containing
a simplified credential API and common helper functions.
The implementation in credential_helper.c already provides
a main() function and chooses the credential operation
from a function table:

  struct credential_operation const credential_helper_ops[] =
  {
  { get,   my_backend_get   },
  { store, my_backend_store },
  { erase, my_backend_erase },
  CREDENTIAL_OP_END
  };

With this, a specific credential helper backend usually only
needs to implement these operation callbacks.

Signed-off-by: Philipp A. Hartmann p...@qo.cx
---
 contrib/credential/helper/credential_helper.c |  149 +
 contrib/credential/helper/credential_helper.h |  116 +++
 2 files changed, 265 insertions(+)
 create mode 100644 contrib/credential/helper/credential_helper.c
 create mode 100644 contrib/credential/helper/credential_helper.h

diff --git a/contrib/credential/helper/credential_helper.c 
b/contrib/credential/helper/credential_helper.c
new file mode 100644
index 000..e99c2ec
--- /dev/null
+++ b/contrib/credential/helper/credential_helper.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright (C) 2012 Philipp A. Hartmann p...@qo.cx
+ *
+ * This file is licensed under the GPL v2, or a later version
+ * at the discretion of Linus.
+ *
+ * This credential struct and API is simplified from git's
+ * credential.{h,c} to be used within credential helper
+ * implementations.
+ */
+
+#include credential_helper.h
+
+void credential_init(struct credential *c)
+{
+   memset(c, 0, sizeof(*c));
+}
+
+void credential_clear(struct credential *c)
+{
+   free(c-protocol);
+   free(c-host);
+   free(c-path);
+   free(c-username);
+   free_password(c-password);
+
+   credential_init(c);
+}
+
+int credential_read(struct credential *c)
+{
+   charbuf[1024];
+   ssize_t line_len = 0;
+   char   *key  = buf;
+   char   *value;
+
+   while (fgets(buf, sizeof(buf), stdin))
+   {
+   line_len = strlen(buf);
+
+   if(buf[line_len-1]=='\n')
+   buf[--line_len]='\0';
+
+   if(!line_len)
+   break;
+
+   value = strchr(buf,'=');
+   if(!value) {
+   warning(invalid credential line: %s, key);
+   return -1;
+   }
+   *value++ = '\0';
+
+   if (!strcmp(key, protocol)) {
+   free(c-protocol);
+   c-protocol = xstrdup(value);
+   } else if (!strcmp(key, host)) {
+   free(c-host);
+   c-host = xstrdup(value);
+   value = strrchr(c-host,':');
+   if (value) {
+   *value++ = '\0';
+   c-port = atoi(value);
+   }
+   } else if (!strcmp(key, path)) {
+   free(c-path);
+   c-path = xstrdup(value);
+   } else if (!strcmp(key, username)) {
+   free(c-username);
+   c-username = xstrdup(value);
+   } else if (!strcmp(key, password)) {
+   free_password(c-password);
+   c-password = xstrdup(value);
+   while (*value) *value++ = '\0';
+   }
+   /*
+* Ignore other lines; we don't know what they mean, but
+* this future-proofs us when later versions of git do
+* learn new lines, and the helpers are updated to match.
+*/
+   }
+   return 0;
+}
+
+void credential_write_item(FILE *fp, const char *key, const char *value)
+{
+   if (!value)
+   return;
+   fprintf(fp, %s=%s\n, key, value);
+}
+
+void credential_write(const struct credential *c)
+{
+   /* only write username/password, if set */
+   credential_write_item(stdout, username, c-username);
+   credential_write_item(stdout, password, c-password);
+}
+
+static void usage(const char *name)
+{
+   struct credential_operation const *try_op = credential_helper_ops;
+   const char *basename = strrchr(name,'/');
+
+   basename = (basename) ? basename + 1 : name;
+   fprintf(stderr, Usage: %s , basename);
+   while(try_op-name) {
+   fprintf(stderr,%s,(try_op++)-name);
+   if(try_op-name)
+   fprintf(stderr,%s,|);
+   }
+   fprintf(stderr,%s,\n);
+}
+
+/*
+ * generic main function for credential helpers
+ */
+int main(int argc, char *argv[])
+{
+   int ret = EXIT_SUCCESS;
+
+   struct credential_operation const *try_op = credential_helper_ops;
+   struct credential  cred   = CREDENTIAL_INIT;
+
+   

Re: [PATCH] specifying ranges: we did not mean to make .. an empty set

2012-08-23 Thread Junio C Hamano
Thomas Rast tr...@student.ethz.ch writes:

 I don't think that .. is really a no-op.  It is true that HEAD..HEAD
 does not itself result in any revisions, but it *could* be used as a
 silly shorthand to introduce ^HEAD into the objects being walked.  This
 can make a difference if it then excludes other objects, too.

 I would argue that such use is misguided, and I am in favor of the
 patch, but in theory it is possible.

You could say log --all .. -- when you mean log --all ^HEAD,
because .. === HEAD..HEAD === ^HEAD HEAD and HEAD is already
contained in --all.  So it is a valid point.

You however cannot say log --all .. without double-dash to
disambiguate, as you would get:

$ git log --all ..
fatal: ambiguous argument '..': both revision and filename

Any existing practice that used to produce useful results couldn't
have been using .. without an explicit --, I think, and with the
disambiguation that favors to take .. as the parent directory,
log --all .. -- still means log --all ^HEAD.

So I think we are still OK.
--
To unsubscribe from this list: send the line 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: Existing utility to track compiled files in another sister repository, for rollouts

2012-08-23 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason ava...@gmail.com writes:

  5. Copy those to to software-generated.git, removing any that we
 didn't just create, adding any that are new
  6. Commit that, tag it with generated-deployment-MMDD-HHMMSS

  9. Do git clean -dxf on software.git remove old generated files
  10. Copy new generated files from generated-software.git to software.git

 For this I'll need to write some git snapshot-commit tool for #5 and
 #6 to commit whatever the current state of the directory is (with
 removed/added files), and hack up something to do #9-#10.

 This should all be relatively easy, I was just wondering if there was
 any prior art on this that I could use instead of hacking it up
 myself.

FWIW, dodoc script in my todo branch was hacked up to respond to my
push into k.org repository, pull 'master' into documentation build
repo at the k.org machine and check it out, build docs and then
removing any that we didn't create, adding any that are new to
update the html and man branches, and push it back to the main
repository, so in that sense, it has all the logic that was needed
(and more, as the rebuilt documentation from two revisions may only
differ from version stamp asciidoc-to-html/man toolchain adds, which
need to be filtered out when updating html/man branches).

There no longer is post-update hook at k.org, so I have to run it
manually at the end of day's integration cycle, just before pushing
things out.

I wrote it myself, not because I looked for suitable reusable code
and didn't find any, but because I was too lazy to look for existing
tools and evaluating them.
--
To unsubscribe from this list: send the line 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] vcs-svn: Fix 'fa/remote-svn' and 'fa/vcs-svn' in pu

2012-08-23 Thread Ramsay Jones


Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

Hi Florian,

The build on pu is currently broken:

CC remote-testsvn.o
LINK git-remote-testsvn
cc: vcs-svn/lib.a: No such file or directory
make: *** [git-remote-testsvn] Error 1

This is caused by a dependency missing from the git-remote-testsvn
link rule. The addition of the $(VCSSVN_LIB) dependency, which should
be squashed into commit ea1f4afb (Add git-remote-testsvn to Makefile,
20-08-2012), fixes the build.

However, this leads to a failure of test t9020.5 and (not unrelated)
compiler warnings:

CC vcs-svn/svndump.o
vcs-svn/svndump.c: In function ‘handle_node’:
vcs-svn/svndump.c:246: warning: left shift count = width of type
vcs-svn/svndump.c:345: warning: format ‘%lu’ expects type ‘long \
unsigned int’, but argument 3 has type ‘uintmax_t’

The fix for the shift count warning is to cast the lhs of the shift
expression to uintmax_t. The format warning is fixed by using the
PRIuMAX format macro. These fixes should be squashed into commit
78d9d4138 (vcs-svn/svndump: rewrite handle_node(), begin|end_revision(),
20-08-2012).

HTH

ATB,
Ramsay Jones

 Makefile  | 2 +-
 vcs-svn/svndump.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 9cede84..761ae05 100644
--- a/Makefile
+++ b/Makefile
@@ -2356,7 +2356,7 @@ git-http-push$X: revision.o http.o http-push.o 
GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
-git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS)
+git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(LIBS) \
$(VCSSVN_LIB)
 
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 28ce2aa..eb97e8e 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -243,7 +243,7 @@ static void handle_node(struct node_ctx_t *node)
const char *old_data = NULL;
uint32_t old_mode = REPO_MODE_BLB;
struct strbuf sb = STRBUF_INIT;
-   static uintmax_t blobmark = 1UL  (bitsizeof(uintmax_t) - 1);
+   static uintmax_t blobmark = (uintmax_t) 1UL  (bitsizeof(uintmax_t) - 
1);
 
 
if (have_text  type == REPO_MODE_DIR)
@@ -342,7 +342,7 @@ static void handle_node(struct node_ctx_t *node)
node-text_length, input);
}
 
-   strbuf_addf(sb, :%lu, blobmark);
+   strbuf_addf(sb, :%PRIuMAX, blobmark);
node-dataref = sb.buf;
}
}
-- 
1.7.12

--
To unsubscribe from this list: send the line 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: in_merge_bases() is too expensive for recent pu update

2012-08-23 Thread Junio C Hamano
Thomas Rast tr...@student.ethz.ch writes:

 At the very least it should be possible to change in_merge_bases() to
 not do any of the post-filtering; perhaps like the patch below.

I do not recall the details but the post-filtering was added after
the initial naive version without it that was posted to the list did
not work correctly in corner cases.  I wouldn't doubt that N*(N-1)
loop can be optimized to something with log(N), but I offhand find
it hard to believe to not do any could be correct without seeing
more detailed analysis.
--
To unsubscribe from this list: send the line 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] Make 'git submodule update --force' always check out submodules.

2012-08-23 Thread Jens Lehmann
Am 23.08.2012 03:43, schrieb Junio C Hamano:
 Stefan Zager sza...@google.com writes:
 
 Currently, it will only do a checkout if the sha1 registered in the 
 containing
 repository doesn't match the HEAD of the submodule, regardless of whether the
 submodule is dirty.  As discussed on the mailing list, the '--force' flag is 
 a
 strong indicator that the state of the submodule is suspect, and should be 
 reset
 to HEAD.

 Signed-off-by: Stefan Zager sza...@google.com
 ---
 
 Thanks for a reroll.  Will queue; looking good ;-)

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


[PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use

2012-08-23 Thread Carlos Martín Nieto
This interface is error prone, and a better one (--set-upstream-to)
exists. Add a message listing the alternatives and suggest how to fix
a --set-upstream invocation in case the user only gives one argument
which causes a local branch with the same name as a remote-tracking
one to be created. The typical case is

git branch --set-upstream origin/master

when the user meant

git branch --set-upstream master origin/master

assuming that the current branch is master. Show a message telling the
user how to undo their action and get what they wanted. For the
command above, the message would be

The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
Branch origin/master set up to track local branch master.

If you wanted to make 'master' track 'origin/master', do this:

git branch -d origin/master
git branch --set-upstream-to origin/master

Signed-off-by: Carlos Martín Nieto c...@elego.de
---

The 'track' in the message is still not great, but it does fit with
the one above. Maybe if we make it say If youw wanted [...] track the
remote-tracking branch 'origin/master' it would be clearer?

I've simplified and tightened the logic. Now it will only show the
undo message if the branch didn't exist locally and there is a
remote-tracking branch of the same name.

 builtin/branch.c  | 26 ++
 t/t3200-branch.sh | 34 ++
 2 files changed, 60 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 08068f7..a0302a2 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -877,10 +877,36 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_release(buf);
} else if (argc  0  argc = 2) {
+   struct branch *branch = branch_get(argv[0]);
+   int branch_existed = 0, remote_tracking = 0;
+   struct strbuf buf = STRBUF_INIT;
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make 
sense with a branch name));
+
+   if (track == BRANCH_TRACK_OVERRIDE)
+   fprintf(stderr, _(The --set-upstream flag is 
deprecated and will be removed. Consider using --track or 
--set-upstream-to\n));
+
+   strbuf_addf(buf, refs/remotes/%s, branch-name);
+   remote_tracking = ref_exists(buf.buf);
+   strbuf_release(buf);
+
+   branch_existed = ref_exists(branch-refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
  force_create, reflog, 0, quiet, track);
+
+   /*
+* We only show the instructions if the user gave us
+* one branch which doesn't exist locally, but is the
+* name of a remote-tracking branch.
+*/
+   if (argc == 1  track == BRANCH_TRACK_OVERRIDE 
+   !branch_existed  remote_tracking) {
+   fprintf(stderr, _(\nIf you wanted to make '%s' track 
'%s', do this:\n\n), head, branch-name);
+   fprintf(stderr, _(git branch -d %s\n), 
branch-name);
+   fprintf(stderr, _(git branch --set-upstream-to 
%s\n), branch-name);
+   }
+
} else
usage_with_options(builtin_branch_usage, options);
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 93e5d6e..e9e11cf 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -399,6 +399,40 @@ test_expect_success 'test --unset-upstream on a particular 
branch' \
  test_must_fail git config branch.my14.remote 
  test_must_fail git config branch.my14.merge'
 
+test_expect_success '--set-upstream shows message when creating a new branch 
that exists as remote-tracking' \
+'git update-ref refs/remotes/origin/master HEAD 
+ git branch --set-upstream origin/master 2actual 
+ test_when_finished git update-ref -d refs/remotes/origin/master 
+ test_when_finished git branch -d origin/master 
+ cat expected EOF 
+The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
+
+If you wanted to make ''master'' track ''origin/master'', do this:
+
+git branch -d origin/master
+git branch --set-upstream-to origin/master
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with two args only shows the deprecation 
message' \
+'git branch --set-upstream master my13 2actual 
+ test_when_finished git branch --unset-upstream master 
+ cat expected EOF 
+The --set-upstream flag is deprecated and will be removed. Consider using 
--track or --set-upstream-to
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success '--set-upstream with one arg only shows the deprecation 
message if the branch existed' \
+'git branch 

Re: [PATCH 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread Philip Oakley

From: Jeff King p...@peff.net
Sent: Thursday, August 23, 2012 10:26 AM

On Thu, Aug 23, 2012 at 10:10:25AM +0200, mhag...@alum.mit.edu wrote:


There were various confusing things (and a couple of bugs) in the way
that fetch_pack() handled the list (nr_heads, heads) of references to
be sought from the remote:


Aside from the minor comments I made to individual patches, this all
looks good. As usual, thanks for breaking it down; I wish all series
were as easy to review as this.


I'm still suspicious about the logic related to args.fetch_all and
args.depth, but I don't think I've made anything worse.


I think the point of that is that when doing git fetch-pack --all
--depth=1, the meaning of --all is changed from all refs to
everything but tags.



There is a comment in \git\Documentation\technical\shallow.txt that
- If you deepen a history, you'd want to get the tags of the
 newly stored (but older!) commits. This does not work right now.
which may be the source of this restriction. That is, how is the depth
of the tag fetching to be restricted to the requested depth count?
[assuming I've undestood the problem correctly]

It may be (?) that it is a good time to think about a 'datedepth' 
capability to bypass the current counted-depth shallow fetch that can 
cause so much trouble. With a date limited depth the relevant tags could 
also be fetched.


Which I kind of see the point of, because you don't want to grab 
ancient
tags that will be expensive. But wouldn't it make more sense to limit 
it

only to the contents of refs/heads in that case? Surely you wouldn't
want refs/notes, refs/remotes, or other hierarchies.

I suspect this code is never even run at all these days. All of the
callers inside git should actually provide a real list of refs, not
--all. So it is really historical cruft for anybody who calls the
fetch-pack plumbing (I wonder if any third-party callers even exist;
this is such a deep part of the network infrastructure that any sane
scripts would probably just be calling fetch).

-Peff
--



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


Re: [PATCH 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 08:13:29PM +0100, Philip Oakley wrote:

 I'm still suspicious about the logic related to args.fetch_all and
 args.depth, but I don't think I've made anything worse.
 
 I think the point of that is that when doing git fetch-pack --all
 --depth=1, the meaning of --all is changed from all refs to
 everything but tags.
 
 
 There is a comment in \git\Documentation\technical\shallow.txt that
 - If you deepen a history, you'd want to get the tags of the
  newly stored (but older!) commits. This does not work right now.
 which may be the source of this restriction. That is, how is the depth
 of the tag fetching to be restricted to the requested depth count?
 [assuming I've undestood the problem correctly]

I don't think this is about deepening, but rather about making sure we
remain shallow for the initial fetch. Remember that this is on the
fetch-pack --all code path, which used to be used by git clone when
it was a shell script (these days, clone is a C builtin and will
actually feed the list of refs to fetch-pack).

This code blames back to:

 commit 4bcb310c2539b66d535e87508d1b7a90fe29c083
 Author: Alexandre Julliard julli...@winehq.org
 Date:   Fri Nov 24 16:00:13 2006 +0100

fetch-pack: Do not fetch tags for shallow clones.

A better fix may be to only fetch tags that point to commits that we
are downloading, but git-clone doesn't have support for following
tags. This will happen automatically on the next git-fetch though.

So it is about making sure that git clone --depth=1 does not
accidentally pull a single commit from v1.0, v1.1, v1.2, and so forth,
losing the purpose of using --depth in the first place. These days it is
largely irrelevant, since this code path is not followed by clone, and
clone will automatically restrict its list of fetched refs to a single
branch if --depth is used.

The bug that shallow.txt talks about (and which is mentioned in that
commit message) is that we will not properly auto-follow tags during
such a clone (i.e., when we fetch a tag because it is pointing to a
commit that we already have or are already pulling). I'm not sure if
that is still the case or not. But assuming your workflow is something
like:

  [make an initial, cheap clone]
  git clone --depth=1 $repo

  [the next day, you do a regular fetch, which will just get new stuff
   on top of what you already have]
  git fetch

Then that second fetch will auto-follow the tags, anyway. And that is
what the commit message is pointing: it's a bug, but one you can work
around.

 It may be (?) that it is a good time to think about a 'datedepth'
 capability to bypass the current counted-depth shallow fetch that can
 cause so much trouble. With a date limited depth the relevant tags
 could also be fetched.

I don't have anything against such an idea, but I think it is orthogonal
to the issue being discussed here.

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


Re: [PATCH 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 03:56:48PM -0400, Jeff King wrote:

 This code blames back to:
 
  commit 4bcb310c2539b66d535e87508d1b7a90fe29c083
  Author: Alexandre Julliard julli...@winehq.org
  Date:   Fri Nov 24 16:00:13 2006 +0100
 
 fetch-pack: Do not fetch tags for shallow clones.
 
 A better fix may be to only fetch tags that point to commits that we
 are downloading, but git-clone doesn't have support for following
 tags. This will happen automatically on the next git-fetch though.
 
 So it is about making sure that git clone --depth=1 does not
 accidentally pull a single commit from v1.0, v1.1, v1.2, and so forth,
 losing the purpose of using --depth in the first place. These days it is
 largely irrelevant, since this code path is not followed by clone, and
 clone will automatically restrict its list of fetched refs to a single
 branch if --depth is used.

I think part of the confusion of this code is that inside the loop over
the refs it is sometimes checking aspects of the ref, and sometimes
checking invariants of the loop (like args.fetch_all). Splitting it into
separate loops makes it easier to see what is going on, like the patch
below (on top of Michael's series).

I'm not sure if it ends up being more readable, since the generic cut
down this linked list function has to operate through callbacks with a
void pointer. On the other hand, that function could also be used
elsewhere.

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 90683ca..877cf38 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -521,51 +521,80 @@ static void mark_recent_complete_commits(unsigned long 
cutoff)
}
 }
 
-static void filter_refs(struct ref **refs, int *nr_heads, char **heads)
+static void filter_refs_callback(struct ref **refs,
+int (*want)(struct ref *, void *),
+void *data)
 {
-   struct ref *newlist = NULL;
-   struct ref **newtail = newlist;
+   struct ref **tail = refs;
struct ref *ref, *next;
-   int match_pos = 0, unmatched = 0;
 
for (ref = *refs; ref; ref = next) {
-   int keep_ref = 0;
next = ref-next;
-   if (!memcmp(ref-name, refs/, 5) 
-   check_refname_format(ref-name, 0))
-   ; /* trash */
-   else if (args.fetch_all 
-  (!args.depth || prefixcmp(ref-name, refs/tags/)))
-   keep_ref = 1;
-   else
-   while (match_pos  *nr_heads) {
-   int cmp = strcmp(ref-name, heads[match_pos]);
-   if (cmp  0) { /* definitely do not have it */
-   break;
-   } else if (cmp == 0) { /* definitely have it */
-   free(heads[match_pos++]);
-   keep_ref = 1;
-   break;
-   } else { /* might have it; keep looking */
-   heads[unmatched++] = heads[match_pos++];
-   }
-   }
-
-   if (keep_ref) {
-   *newtail = ref;
-   ref-next = NULL;
-   newtail = ref-next;
-   } else {
+   if (want(ref, data))
+   tail = ref-next;
+   else {
free(ref);
+   *tail = next;
}
}
+}
 
-   /* copy any remaining unmatched heads: */
-   while (match_pos  *nr_heads)
-   heads[unmatched++] = heads[match_pos++];
-   *nr_heads = unmatched;
+static int ref_name_is_ok(struct ref *ref, void *data)
+{
+   return memcmp(ref-name, refs/, 5) ||
+   !check_refname_format(ref-name, 0);
+}
+
+static int ref_ok_for_shallow(struct ref *ref, void *data)
+{
+   return prefixcmp(ref-name, refs/tags/);
+}
 
-   *refs = newlist;
+struct filter_by_name_data {
+   char **heads;
+   int nr_heads;
+   int match_pos;
+   int unmatched;
+};
+
+static int want_ref_name(struct ref *ref, void *data)
+{
+   struct filter_by_name_data *f = data;
+
+   while (f-match_pos  f-nr_heads) {
+   int cmp = strcmp(ref-name, f-heads[f-match_pos]);
+   if (cmp  0) /* definitely do not have it */
+   return 0;
+   else if (cmp == 0) { /* definitely have it */
+   free(f-heads[f-match_pos++]);
+   return 1;
+   } else /* might have it; keep looking */
+   f-heads[f-unmatched++] = f-heads[f-match_pos++];
+   }
+   return 0;
+}
+
+static void filter_refs(struct ref **refs, int *nr_heads, char **heads)
+{
+   struct filter_by_name_data f;
+
+   

Re: in_merge_bases() is too expensive for recent pu update

2012-08-23 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Thomas Rast tr...@student.ethz.ch writes:

 At the very least it should be possible to change in_merge_bases() to
 not do any of the post-filtering; perhaps like the patch below.

 I do not recall the details but the post-filtering was added after
 the initial naive version without it that was posted to the list did
 not work correctly in corner cases.  I wouldn't doubt that N*(N-1)
 loop can be optimized to something with log(N), but I offhand find
 it hard to believe to not do any could be correct without seeing
 more detailed analysis.

If one on one side, many on the other side in merge_bases_many()
confuses any of you in the readers, you can just ignore many-ness.
Getting merge bases for one against many twos using
merge_bases_many() is exactly the same as getting merge bases for
one against a (fictitious) single commit you can make by merging
all twos.

So we can think about the degenerate case of merge base between two
commits A and B in the following discussion.

A merge-base between A and B is defined to be a commit that is a
common ancestor of both A and B, and that is not an ancestor of any
other merge-base between A and B.  In the simplest case (illustrated
below), 1 is a merge base between A and B, but 2 is not (even though
it is an ancestor of both A and B, it is an ancestor of 1 which is a
merge base).

  y---A
 /
 ---2---o---1---x---B

So, the thinking goes, in order to find all merge bases, first we
can traverse from A and B, following parent link from children,
and painting found parents in two colors.

Start from A and B.  Follow from B to find 'x' and paint it in blue,
follow from A to find 'y' and paint it in amber.  Follow from 'x' to
'1', paint it in blue.  Follow from 'y' to '1', paint it in amber
but notice that it already is painted in blue.  Stop the traversal
there and we found a candidate '1' that could be a merge base.  We
know digging from '1' will not find more merge bases, so we should
stop there (I do not recall offhand if the current code does stop
there, though) [*1*].

There may be other paths that are not depicted in the above
illustration that reach '2' from A and B without passing '1'
through.

o---o
   / \
  /   y---A
 /   /
 ---2---z---1---x---B
 \ /
  o---o

In such a history, we may stop after finding '1' and '2' in the
first pass of stop when a node is painted in both amber and blue.
This way, the first pass will find _all_ merge bases, but it also
may find common ancestors that are not merge bases.

So we need to notice that '1' and '2' have ancestry relation in
order to reject '2' as common but not merge-base.  One way of
doing so is not to stop at '1' and keep digging (and eventually we
find that '2' is what we could reach from '1' that already is a
merge base), but then we will be susceptible to the same kind of
clock skew issue as the revision traverser.  Instead, merge-base
traverser chose to do this by running the same stop traversing at
common traverser between the candidates (in this case, '1' and
'2').

The objective of this second traversal is very different from the
first one, though.  We do not need _all_ the merge bases between '1'
and '2'; we do not even need merge bases.

The only thing we need to prove that '1' is a merge base (i.e. not
an ancestor of any other merge base candidates the first round of
traversal between A and B found) is to do the first round of the
traversal for '1' as one and all the other ('2' in this case) as
twos; if the first round of such traversal finishes without
painting '1' in both colors, that would mean '1' is not reachable
from any other candidates of merge base between A and B, so we have
proven that '1' is a merge base.

So I suspect that the postprocess phase could be made from N*(N-1)
to N (run merge_bases_many() once for each of the common ancestor
the first round found).  You might also be able to stop the
traversal used in the first phase (i.e. get_merge_bases()) a bit
earlier, if we are digging through '1' to paint 'z' (and eventually
'2') in STALE (amber+blue) color, as digging further only to paint
things in STALE color is not necessary with the postprocess.


[Footnote]

*1* Digging through '2' down may find that other candidate merge
bases we reach by traversing other paths that may not be depicted in
the above illustration, and there may be such paths to reach '1'
from A and B that does not pass '2' through.  That is a possible
alternative way to reject common ancestor that is not merge-base.
--
To unsubscribe from this list: send the line 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: Existing utility to track compiled files in another sister repository, for rollouts

2012-08-23 Thread Ævar Arnfjörð Bjarmason
On Thu, Aug 23, 2012 at 6:28 PM, Ævar Arnfjörð Bjarmason
ava...@gmail.com wrote:
 I'm planning on using Git for a deployment process where the steps are
 basically:

  1. You log into a deployment host, cd into software.git, do git pull
  2. A tool runs make for you, creates a deployment-MMDD-HHMMSS tag
  3. That make step will create a bunch of generated (text) files
  4. Get a list of these with : git clean -dxfn
  5. Copy those to to software-generated.git, removing any that we
 didn't just create, adding any that are new
  6. Commit that, tag it with generated-deployment-MMDD-HHMMSS
  7. Push out both our generated software.git and
 software-generated.git tag to our servers
  8. git reset --hard both of those to our newly pushed out tags
  9. Do git clean -dxf on software.git remove old generated files
  10. Copy new generated files from generated-software.git to software.git
  11. Restart our application to pick up the new software

 For this I'll need to write some git snapshot-commit tool for #5 and
 #6 to commit whatever the current state of the directory is (with
 removed/added files), and hack up something to do #9-#10.

 This should all be relatively easy, I was just wondering if there was
 any prior art on this that I could use instead of hacking it up
 myself.

Here's a quick hack that does #4-6 but not #9-10 yet, although that
would be easy: https://gist.github.com/3440792

Suggestions for improvements welcome, particularly whether there's a
simpler way to do this to nuke existing files in a repo and replace it
with new files all staged for commit:

# Go to the target repository, nuke anything already there
chdir $to_repository;
system git reset --hard;
system git clean -dxf;
system git ls-tree --name-only HEAD -z | xargs -0 rm -rf;
system git add --update; # stage any removals

Followed by:

system tar xvf incoming.tar;
system rm incoming.tar;
system git add * .??* || :; # Might die if we empty the repo,
TODO: make this use status - add each file
system git commit -m'Bump copy from $from_repository to
$to_repository' || :; # We might have nothing to change!
--
To unsubscribe from this list: send the line 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: in_merge_bases() is too expensive for recent pu update

2012-08-23 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 The objective of this second traversal is very different from the
 first one, though.  We do not need _all_ the merge bases between '1'
 and '2'; we do not even need merge bases.

 The only thing we need to prove that '1' is a merge base (i.e. not
 an ancestor of any other merge base candidates the first round of
 traversal between A and B found) is to do the first round of the
 traversal for '1' as one and all the other ('2' in this case) as
 twos; if the first round of such traversal finishes without
 painting '1' in both colors, that would mean '1' is not reachable
 from any other candidates of merge base between A and B, so we have
 proven that '1' is a merge base.

 So I suspect that the postprocess phase could be made from N*(N-1)
 to N (run merge_bases_many() once for each of the common ancestor
 the first round found).  You might also be able to stop the
 traversal used in the first phase (i.e. get_merge_bases()) a bit
 earlier, if we are digging through '1' to paint 'z' (and eventually
 '2') in STALE (amber+blue) color, as digging further only to paint
 things in STALE color is not necessary with the postprocess.

As a corollary, the is pu@{0} a fast-forward of pu@{1}? check does
not need merge base computation at all.  The only thing it needs to
do is to prove pu@{1} is reachable from pu@{0}, and that can be done
the same way in which '1' can be proved unreachable from '2' in the
post processing phase as described above, i.e. it needs only the
first phase of running merge_bases_many() without postprocessing.


--
To unsubscribe from this list: send the line 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: [PATCHv2 3/3] branch: deprecate --set-upstream and show help if we detect possible mistaken use

2012-08-23 Thread Junio C Hamano
Carlos Martín Nieto c...@elego.de writes:

 The 'track' in the message is still not great, but it does fit with
 the one above. Maybe if we make it say If youw wanted [...] track the
 remote-tracking branch 'origin/master' it would be clearer?

The verb track in the phrase remote-tracking branch means keep
track of the branch at the remote, by storing a copy of the last
observed state of it.  In the same sentence, the verb track
elsewhere is used to describe what the branch B whose upstream is
set to B@{upstream} does against B@{upstream}, but that is not
keeping a copy---it is doing an entirely different thing.

If we say that the branch B whose upstream is set to B@{upstream} is
building on top of B@{upstream}, integrating with B@{upstream},
forked from B@{upstream}, etc., without using the verb track
that already means something else (i.e. keeps track of the copy of
last observed state), it would reduce the confusion, but I do not
think it would clarify anything if the verb track is used for
that.

As usual, because I am not the best source of phrasing, others may
find a better verb than builds, forks, or integrates, though.

 I've simplified and tightened the logic. Now it will only show the
 undo message if the branch didn't exist locally and there is a
 remote-tracking branch of the same name.

The updated and simplified logic reads quite straight-forward, and
looks good.  It is likely that the message will be reworded and also
localized in the future, so it would make sense to use test_i18ncmp
from the day one, though.

Thanks.  Will queue.
--
To unsubscribe from this list: send the line 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/3] branch: add --unset-upstream option

2012-08-23 Thread Junio C Hamano
Carlos Martín Nieto c...@elego.de writes:

 diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
 index e9019ac..93e5d6e 100755
 --- a/t/t3200-branch.sh
 +++ b/t/t3200-branch.sh
 @@ -383,6 +383,22 @@ test_expect_success 'use --set-upstream-to modify a 
 particular branch' \
   test $(git config branch.my13.remote) = . 
   test $(git config branch.my13.merge) = refs/heads/master'
  
 +test_expect_success 'test --unset-upstream on HEAD' \
 +'git branch my14
 + test_config branch.master.remote foo 
 + test_config branch.master.merge foo 
 + git branch --set-upstream-to my14 
 + git branch --unset-upstream 
 + test_must_fail git config branch.master.remote 
 + test_must_fail git config branch.master.merge'
 +
 +test_expect_success 'test --unset-upstream on a particular branch' \
 +'git branch my15
 + git branch --set-upstream-to master my14 
 + git branch --unset-upstream my14 
 + test_must_fail git config branch.my14.remote 
 + test_must_fail git config branch.my14.merge'
 +

What should happen when you say --unset-upstream on a branch B
that does not have B@{upstream}?  Should it fail?  Should it be
silently ignored?  Is it undefined that we do not want to test?
--
To unsubscribe from this list: send the line 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] Don't use curl_easy_strerror prior to curl-7.12.0

2012-08-23 Thread Junio C Hamano
Joachim Schmitz j...@schmitz-digital.de writes:

 From: Joachim Schmitz [mailto:j...@schmitz-digital.de]
 Sent: Wednesday, August 22, 2012 11:06 PM
 To: 'Junio C Hamano'
 Cc: 'git@vger.kernel.org'
 Subject: RE: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
 
  From: Junio C Hamano [mailto:gits...@pobox.com]
  Sent: Wednesday, August 22, 2012 11:02 PM
  To: Joachim Schmitz
  Cc: git@vger.kernel.org
  Subject: Re: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
 
  Joachim Schmitz j...@schmitz-digital.de writes:
 
diff --git a/http.c b/http.c
index b61ac85..18bc6bf 100644
--- a/http.c
+++ b/http.c
@@ -806,10 +806,12 @@ static int http_request(const char *url, void
*result, int target, int options)
  
   Likewrapped X-
  
   Any suggestion?
 
  Other than don't wrap (or get a real MUA and MTA X-), unfortunately 
  no.
 
  I do not know if you have checked MUA specific hints section of
  Documentation/SubmittingPatches; there may be environment specific hints
  described for a MUA you may have at hand (or not).
 
 I checked, but Outlook isn't mentioned :-(

 OK, Outlook inserts line breaks at position 76. I can't switch it off 
 completely, but can set it to anything between 30 and 132. I
 set it to 132 now, does that look OK now?

Yeah, modulo that with all the above noise and with --- before the
log message you inserted by hand before the real commit log message,
and also the log message is not properly line-wrapped, it is getting
closer to what is expected of a patch to be applied.

I've applied it by hand and fixed the log message up, so no need to
resend this particular one.  Thanks.

 ---

 This reverts be22d92 (http: avoid empty error messages for some curl errors, 
 2011-09-05) on platforms with older versions of
 libcURL.

 Signed-off-by: Joachim Schmitz j...@schmitz-digital.de
 ---
  http.c | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/http.c b/http.c
 index b61ac85..18bc6bf 100644
 --- a/http.c
 +++ b/http.c
 @@ -806,10 +806,12 @@ static int http_request(const char *url, void *result, 
 int target, int options)
   ret = HTTP_REAUTH;
   }
   } else {
 +#if LIBCURL_VERSION_NUM = 0x070c00
   if (!curl_errorstr[0])
   strlcpy(curl_errorstr,
   curl_easy_strerror(results.curl_result),
   sizeof(curl_errorstr));
 +#endif
   ret = HTTP_ERROR;
   }
   } else {
--
To unsubscribe from this list: send the line 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 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread Philip Oakley

From: Jeff King p...@peff.net
Sent: Thursday, August 23, 2012 8:56 PM

On Thu, Aug 23, 2012 at 08:13:29PM +0100, Philip Oakley wrote:


I'm still suspicious about the logic related to args.fetch_all and
args.depth, but I don't think I've made anything worse.

I think the point of that is that when doing git fetch-pack --all
--depth=1, the meaning of --all is changed from all refs to
everything but tags.


There is a comment in \git\Documentation\technical\shallow.txt that
- If you deepen a history, you'd want to get the tags of the
 newly stored (but older!) commits. This does not work right now.
which may be the source of this restriction. That is, how is the 
depth

of the tag fetching to be restricted to the requested depth count?
[assuming I've undestood the problem correctly]


I don't think this is about deepening, but rather about making sure we
remain shallow for the initial fetch. Remember that this is on the
fetch-pack --all code path, which used to be used by git clone 
when

it was a shell script (these days, clone is a C builtin and will
actually feed the list of refs to fetch-pack).


OK



This code blames back to:

commit 4bcb310c2539b66d535e87508d1b7a90fe29c083
Author: Alexandre Julliard julli...@winehq.org
Date:   Fri Nov 24 16:00:13 2006 +0100

   fetch-pack: Do not fetch tags for shallow clones.

   A better fix may be to only fetch tags that point to commits that 
we

   are downloading, but git-clone doesn't have support for following
   tags. This will happen automatically on the next git-fetch though.

So it is about making sure that git clone --depth=1 does not
accidentally pull a single commit from v1.0, v1.1, v1.2, and so forth,
losing the purpose of using --depth in the first place. These days it 
is

largely irrelevant, since this code path is not followed by clone, and
clone will automatically restrict its list of fetched refs to a single
branch if --depth is used.

The bug that shallow.txt talks about (and which is mentioned in that
commit message) is that we will not properly auto-follow tags during
such a clone (i.e., when we fetch a tag because it is pointing to a
commit that we already have or are already pulling). I'm not sure if
that is still the case or not. But assuming your workflow is something
like:

 [make an initial, cheap clone]
 git clone --depth=1 $repo

 [the next day, you do a regular fetch, which will just get new stuff
  on top of what you already have]
 git fetch

Then that second fetch will auto-follow the tags, anyway. And that is
what the commit message is pointing: it's a bug, but one you can work
around.


I hadn't appreciated that the fetch would limit itself to the original 
shallow

depth. I'd gained  the impression that one need to use the --depth to
control what was being fetched.




It may be (?) that it is a good time to think about a 'datedepth'
capability to bypass the current counted-depth shallow fetch that can
cause so much trouble. With a date limited depth the relevant tags
could also be fetched.


I don't have anything against such an idea, but I think it is 
orthogonal

to the issue being discussed here.

OK. I'll stick it on my slow-burner pile ;-)



-Peff


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


Re: [PATCH] specifying ranges: we did not mean to make .. an empty set

2012-08-23 Thread Jeff King
On Thu, Aug 23, 2012 at 02:40:19PM -0700, Junio C Hamano wrote:

  This last sentence confuses me. Now we are documenting that yes, ..
  really means HEAD..HEAD, which is the empty range. But isn't the point
  of this patch to say sure, it would be the empty range, but because
  that is stupid and pointless, we do not consider it valid and treat ..
  as a pathspec?
 
 No, we still allow .. as a short-hand for HEAD..HEAD when it is
 understood as a rev.  We also allow .. as a pathspec to match the
 parent directory when it is understood as a pathspec.
 
 The only thing the topic wanted to change was the disambiguation
 logic.  When a string S can name both rev and path, we ask the user
 to disambiguate, but when S is .., we do not have to (as one
 interpretation is meaningless).

Ah, right. OK, that makes more sense. I wasn't thinking that you could
still do:

  git log .. --

if you really wanted to. So yeah, it doesn't belong here...

 I think that documentation belongs to the section of disambiguation
 without --.  Usually you need to use --, but .. is taken as
 path even without --.

...but I agree it would be worth mentioning there. But I am not sure
where there is in the current documentation.

 An interesting side effect is that
 
   git log .. pu
 
 used to error out for .. being both rev and path, but it will
 error out for pu not being a path in the working tree.  This is
 because on a command line without -- disambiguation, once you
 start listing paths, you have to have nothing but paths after that
 point.

Hmm. Yeah, that's a slightly surprising emergent behavior. But I think
it is not a big deal since both cases led to errors anyway.

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


Reg:Git-ssh bug

2012-08-23 Thread Gokulramkumar Subramaniam
Hi,

I am new to Git and I am trying to add my machine with Git but it is failing 
through ssh method.

Error received:

$ ssh-add -l
2048 5f:6f:39:ed:b0:76:2e:d0:xx:xx:xx:xx:xx:xx:xx:xx id_rsa (RSA)

Gokul$ ssh -vT g...@github.com
OpenSSH_5.6p1, OpenSSL 0.9.8r 8 Feb 2011
debug1: Reading configuration data /etc/ssh_config
debug1: Applying options for *
debug1: Connecting to github.com [207.97.227.239] port 22.
debug1: Connection established.
debug1: identity file /Users/Gokul/.ssh/id_rsa type 1
debug1: identity file /Users/Gokul/.ssh/id_rsa-cert type -1
debug1: identity file /Users/Gokul/.ssh/id_dsa type -1
debug1: identity file /Users/Gokul/.ssh/id_dsa-cert type -1
debug1: Remote protocol version 2.0, remote software version OpenSSH_5.5p1 
Debian-6+squeeze1+github8
debug1: match: OpenSSH_5.5p1 Debian-6+squeeze1+github8 pat OpenSSH*
debug1: Enabling compatibility mode for protocol 2.0
debug1: Local version string SSH-2.0-OpenSSH_5.6
debug1: SSH2_MSG_KEXINIT sent
debug1: SSH2_MSG_KEXINIT received
debug1: kex: server-client aes128-ctr hmac-md5 none
debug1: kex: client-server aes128-ctr hmac-md5 none
debug1: SSH2_MSG_KEX_DH_GEX_REQUEST(102410248192) sent
debug1: expecting SSH2_MSG_KEX_DH_GEX_GROUP
debug1: SSH2_MSG_KEX_DH_GEX_INIT sent
debug1: expecting SSH2_MSG_KEX_DH_GEX_REPLY
debug1: Host 'github.com' is known and matches the RSA host key.
debug1: Found key in /Users/Gokul/.ssh/known_hosts:1
debug1: ssh_rsa_verify: signature correct
debug1: SSH2_MSG_NEWKEYS sent
debug1: expecting SSH2_MSG_NEWKEYS
debug1: SSH2_MSG_NEWKEYS received
debug1: Roaming not allowed by server
debug1: SSH2_MSG_SERVICE_REQUEST sent
debug1: SSH2_MSG_SERVICE_ACCEPT received
debug1: Authentications that can continue: publickey
debug1: Next authentication method: publickey
debug1: Offering RSA public key: id_rsa
debug1: Authentications that can continue: publickey
debug1: Offering RSA public key: /Users/Gokul/.ssh/id_rsa
debug1: Authentications that can continue: publickey
debug1: Trying private key: /Users/Gokul/.ssh/id_dsa
debug1: No more authentication methods to try.
Permission denied (publickey).

Please let me know, what I need to follow to get rid of this problem.

Regards,
Gokulramkumar Subramaniam--
To unsubscribe from this list: send the line 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 00/17] Clean up how fetch_pack() handles the heads list

2012-08-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 It may be (?) that it is a good time to think about a 'datedepth'
 capability to bypass the current counted-depth shallow fetch that can
 cause so much trouble. With a date limited depth the relevant tags
 could also be fetched.

 I don't have anything against such an idea, but I think it is orthogonal
 to the issue being discussed here.

Correct.

The biggest problem with the shallow hack is that the deepening
fetch counts from the tip of the refs at the time of deepening when
deepening the history (i.e. clone --depth, followed by number of
fetch, followed by fetch --depth).  If you start from a shallow
clone of depth 1, repeated fetch to keep up while the history grew
by 100, you would still have a connected history down to the initial
cauterization point, and fetch --depth=200 would give you a
history that is deeper than your original clone by 100 commits.  But
if you start from the same shallow clone of depth 1, did not do
anything while the history grew by 100, and then decide to fetch
again with fetch --depth=20, it does not grow.  It just makes
20-commit deep history from the updated tip, and leave the commit
from your original clone dangling.

The problem with depth does not have anything to do with how it is
specified at the UI level.  The end-user input is used to find out
at what set of commits the history is cauterized, and once they are
computed, the shallow logic solely works on is the history before
these cauterization points, or after, in topological terms. (and it
has to be that way to make sure we get reproducible results).  Even
if a new way to specify these cauterization points in terms of date
is introduced, it does not change anything and does not solve the
fundamental problem the code has when deepening.
--
To unsubscribe from this list: send the line 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] Don't use curl_easy_strerror prior to curl-7.12.0

2012-08-23 Thread Joachim Schmitz
 From: Junio C Hamano [mailto:gits...@pobox.com]
 Sent: Thursday, August 23, 2012 11:27 PM
 To: Joachim Schmitz
 Cc: git@vger.kernel.org
 Subject: Re: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
 
 Joachim Schmitz j...@schmitz-digital.de writes:
 
  From: Joachim Schmitz [mailto:j...@schmitz-digital.de]
  Sent: Wednesday, August 22, 2012 11:06 PM
  To: 'Junio C Hamano'
  Cc: 'git@vger.kernel.org'
  Subject: RE: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
 
   From: Junio C Hamano [mailto:gits...@pobox.com]
   Sent: Wednesday, August 22, 2012 11:02 PM
   To: Joachim Schmitz
   Cc: git@vger.kernel.org
   Subject: Re: [PATCH] Don't use curl_easy_strerror prior to curl-7.12.0
  
   Joachim Schmitz j...@schmitz-digital.de writes:
  
 diff --git a/http.c b/http.c
 index b61ac85..18bc6bf 100644
 --- a/http.c
 +++ b/http.c
 @@ -806,10 +806,12 @@ static int http_request(const char *url, void
 *result, int target, int options)
   
Likewrapped X-
   
Any suggestion?
  
   Other than don't wrap (or get a real MUA and MTA X-), unfortunately 
   no.
  
   I do not know if you have checked MUA specific hints section of
   Documentation/SubmittingPatches; there may be environment specific hints
   described for a MUA you may have at hand (or not).
 
  I checked, but Outlook isn't mentioned :-(
 
  OK, Outlook inserts line breaks at position 76. I can't switch it off 
  completely, but can set it to anything between 30 and 132.
I
  set it to 132 now, does that look OK now?
 
 Yeah, modulo that with all the above noise and with --- before the
 log message you inserted by hand before the real commit log message,

OK, those are not allowed then? Or would they need to look different?

 and also the log message is not properly line-wrapped, it is getting
 closer to what is expected of a patch to be applied.
 
 I've applied it by hand and fixed the log message up, so no need to
 resend this particular one.  Thanks.

Thanks

  This reverts be22d92 (http: avoid empty error messages for some curl 
  errors, 2011-09-05) on platforms with older versions of
  libcURL.

You meant that linewrap between of and libcURL?, Or well, that was the 132 
position... Or do you want these messages being wrapped
at =80?

Bye, Jojo

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


[PATCH 0/6] Gettext poison rework

2012-08-23 Thread Nguyễn Thái Ngọc Duy
Still WIP but I'm getting closer. I dropped test-poisongen and started
to use podebug [2] instead. Less code in git. podebug does not preserve
shell variables yet. I'll follow that up at upstream [1].

With this series, if you have translation toolkit installed, you could
do

make pseudo-locale L=your language code
make GETTEXT_POISON=$LANG test

podebug supports a few way of rewriting translations. Currently
unicode is used but you can change it via PODEBUG_OPTS

t9001 is not happy with $LANG != C though. May need to add some
prereq there.

[1] http://bugs.locamotion.org/show_bug.cgi?id=2450
[2] http://translate.sourceforge.net/wiki/toolkit/podebug

Nguyễn Thái Ngọc Duy (6):
  Makefile: do not mark strings for l10n from test programs
  Makefile: recreate git.pot if *.sh or *.perl changes
  Replace gettext poison implementation with pseudotranslation
generation
  Initialize gettext for test programs that may use it
  Support logging unmarked strings
  test-parse-options: mark parseopt help strings for pseudotranslation

 .gitignore|  1 +
 Makefile  | 48 +---
 gettext.c | 10 
 gettext.h | 10 +---
 git-sh-i18n.sh| 14 ---
 po/.gitignore |  1 +
 po/README | 15 +--
 t/.gitignore  |  1 +
 t/lib-gettext.sh  |  7 +-
 t/t0200-gettext-basic.sh  |  1 +
 t/t0205-gettext-poison.sh | 36 ---
 t/test-lib.sh |  2 +-
 test-date.c   |  1 +
 test-delta.c  |  1 +
 test-dump-cache-tree.c|  5 +++-
 test-index-version.c  |  1 +
 test-match-trees.c|  1 +
 test-mergesort.c  |  1 +
 test-parse-options.c  | 63 ---
 test-path-utils.c |  1 +
 test-revision-walking.c   |  1 +
 test-scrap-cache-tree.c   |  4 ++-
 test-sha1.c   |  1 +
 test-subprocess.c |  1 +
 wrap-for-bin.sh   | 16 +++-
 25 files changed, 111 insertions(+), 132 deletions(-)
 delete mode 100755 t/t0205-gettext-poison.sh

-- 
1.7.12.rc2

--
To unsubscribe from this list: send the line 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/6] Makefile: recreate git.pot if *.sh or *.perl changes

2012-08-23 Thread Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index ddeb04d..485978f 100644
--- a/Makefile
+++ b/Makefile
@@ -2409,7 +2409,7 @@ LOCALIZED_SH += t/t0200/test.sh
 LOCALIZED_PERL += t/t0200/test.perl
 endif
 
-po/git.pot: $(LOCALIZED_C)
+po/git.pot: $(LOCALIZED_C) $(LOCALIZED_SH) $(LOCALIZED_PERL)
$(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ $(XGETTEXT_FLAGS_C) $(LOCALIZED_C)
$(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ --join-existing $(XGETTEXT_FLAGS_SH) 
\
$(LOCALIZED_SH)
-- 
1.7.12.rc2

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


[PATCH 5/6] Support logging unmarked strings

2012-08-23 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Hard coding path obviously won't fly..

 .gitignore  | 1 +
 wrap-for-bin.sh | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/.gitignore b/.gitignore
index bb5c91e..f2d0fe5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -195,6 +195,7 @@
 /test-sigchain
 /test-subprocess
 /test-svn-fe
+/untranslated.log
 /common-cmds.h
 *.tar.gz
 *.dsc
diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index a2d9aef..4dbae80 100644
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -17,6 +17,12 @@ fi
 GITPERLLIB='@@BUILD_DIR@@/perl/blib/lib'
 if test -n $GIT_GETTEXT_POISON
 then
+   if test -x /usr/lib/preloadable_libintl.so
+   then
+   GETTEXT_LOG_UNTRANSLATED=@@BUILD_DIR@@/untranslated.log
+   LD_PRELOAD=/usr/lib/preloadable_libintl.so $LD_PRELOAD
+   export LD_PRELOAD GETTEXT_LOG_UNTRANSLATED
+   fi
GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/pseudo-locale'
LANG=$GIT_GETTEXT_POISON
unset LC_ALL
-- 
1.7.12.rc2

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


[PATCH 6/6] test-parse-options: mark parseopt help strings for pseudotranslation

2012-08-23 Thread Nguyễn Thái Ngọc Duy
This reduces the number of false positives in untranslated.log when we
check for unmarked strings.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 test-parse-options.c | 62 ++--
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/test-parse-options.c b/test-parse-options.c
index 4a94327..09ddf74 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -33,50 +33,50 @@ int main(int argc, const char **argv)
 {
const char *prefix = prefix/;
const char *usage[] = {
-   test-parse-options options,
+   N_(test-parse-options options),
NULL
};
struct option options[] = {
-   OPT_BOOL(0, yes, boolean, get a boolean),
-   OPT_BOOL('D', no-doubt, boolean, begins with 'no-'),
+   OPT_BOOL(0, yes, boolean, N_(get a boolean)),
+   OPT_BOOL('D', no-doubt, boolean, N_(begins with 'no-')),
{ OPTION_SET_INT, 'B', no-fear, boolean, NULL,
- be brave, PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
-   OPT_COUNTUP('b', boolean, boolean, increment by one),
+ N_(be brave), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+   OPT_COUNTUP('b', boolean, boolean, N_(increment by one)),
OPT_BIT('4', or4, boolean,
-   bitwise-or boolean with ...0100, 4),
-   OPT_NEGBIT(0, neg-or4, boolean, same as --no-or4, 4),
+   N_(bitwise-or boolean with ...0100), 4),
+   OPT_NEGBIT(0, neg-or4, boolean, N_(same as --no-or4), 4),
OPT_GROUP(),
-   OPT_INTEGER('i', integer, integer, get a integer),
-   OPT_INTEGER('j', NULL, integer, get a integer, too),
-   OPT_SET_INT(0, set23, integer, set integer to 23, 23),
-   OPT_DATE('t', NULL, timestamp, get timestamp of time),
-   OPT_CALLBACK('L', length, integer, str,
-   get length of str, length_callback),
-   OPT_FILENAME('F', file, file, set file to file),
-   OPT_GROUP(String options),
-   OPT_STRING('s', string, string, string, get a string),
-   OPT_STRING(0, string2, string, str, get another string),
-   OPT_STRING(0, st, string, st, get another string (pervert 
ordering)),
-   OPT_STRING('o', NULL, string, str, get another string),
+   OPT_INTEGER('i', integer, integer, N_(get a integer)),
+   OPT_INTEGER('j', NULL, integer, N_(get a integer, too)),
+   OPT_SET_INT(0, set23, integer, N_(set integer to 23), 23),
+   OPT_DATE('t', NULL, timestamp, N_(get timestamp of time)),
+   OPT_CALLBACK('L', length, integer, N_(str),
+   N_(get length of str), length_callback),
+   OPT_FILENAME('F', file, file, N_(set file to file)),
+   OPT_GROUP(N_(String options)),
+   OPT_STRING('s', string, string, N_(string), N_(get a 
string)),
+   OPT_STRING(0, string2, string, N_(str), N_(get another 
string)),
+   OPT_STRING(0, st, string, N_(st), N_(get another string 
(pervert ordering))),
+   OPT_STRING('o', NULL, string, N_(str), N_(get another 
string)),
OPT_NOOP_NOARG(0, obsolete),
OPT_SET_PTR(0, default-string, string,
-   set string to default, (unsigned long)default),
-   OPT_STRING_LIST(0, list, list, str, add str to list),
-   OPT_GROUP(Magic arguments),
-   OPT_ARGUMENT(quux, means --quux),
-   OPT_NUMBER_CALLBACK(integer, set integer to NUM,
+   N_(set string to default), (unsigned long)default),
+   OPT_STRING_LIST(0, list, list, N_(str), N_(add str to 
list)),
+   OPT_GROUP(N_(Magic arguments)),
+   OPT_ARGUMENT(quux, N_(means --quux)),
+   OPT_NUMBER_CALLBACK(integer, N_(set integer to NUM),
number_callback),
-   { OPTION_COUNTUP, '+', NULL, boolean, NULL, same as -b,
+   { OPTION_COUNTUP, '+', NULL, boolean, NULL, N_(same as -b),
  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },
{ OPTION_COUNTUP, 0, ambiguous, ambiguous, NULL,
- positive ambiguity, PARSE_OPT_NOARG | PARSE_OPT_NONEG },
+ N_(positive ambiguity), PARSE_OPT_NOARG | PARSE_OPT_NONEG },
{ OPTION_COUNTUP, 0, no-ambiguous, ambiguous, NULL,
- negative ambiguity, PARSE_OPT_NOARG | PARSE_OPT_NONEG },
-   OPT_GROUP(Standard options),
+ N_(negative ambiguity), PARSE_OPT_NOARG | PARSE_OPT_NONEG },
+   OPT_GROUP(N_(Standard options)),
OPT__ABBREV(abbrev),
-   OPT__VERBOSE(verbose, be verbose),
-