Re: Git to use XDG Base Directory Standard
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
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
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
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
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
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()
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/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
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
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
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
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
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
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
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
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
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
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
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
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
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
Æ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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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), -