[RFC 0/3] Reflogs for deleted refs: fix breakage and suggest namespace change
From: Michael Haggerty mhag...@alum.mit.edu On 08/17/2012 01:29 AM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: I like the general direction. Perhaps a long distant future direction could be to also use the same trick in the ref namespace so that we can have 'next' branch itself, and 'next/foo', 'next/bar' forks that are based on the 'next' branch at the same time (it obviously is a totally unrelated topic)? I notice that I was responsible for making this topic veer in the wrong direction by bringing up a new feature having 'next' and 'next/bar' at the same time which nobody asked. Perhaps we can drop that for now to simplify the scope of the topic, to bring the log graveyard back on track? Given that a flag day would anyway be required to add a d/f-tolerant system, I could live with a separate graveyard namespace as originally proposed by Jeff. However, I still think that as long as we are making a jump, we could try to land closer to the ultimate destination. So here are some patches that apply on top of Jeff's to show what I mean. (Please also note that I made some technical comments about Jeff's patches in an earlier email.) The first two patches fix a breakage that I see when I apply Jeff's patches to master. The third changes the implementation of refname_to_graveyard_reflog() and graveyard_reflog_to_refname() and touches up some test cases. It changes the naming convention for dead references to $GIT_DIR/logs/refs~d/heads~d/foo~d/bar~d/baz~f I.e., the dead reflogs are stored closer to the living. It is not obvious whether the refs part of the name should be munged to refs~d as I have done, or left unmunged. The argument in favor of munging is that the algorithm is more uniform. On the other hand, extending the same scheme to loose references would produce filenames like $GIT_DIR/refs~d/heads~d/foo~d/bar~d/baz~f or maybe they should be nested inside of the refs directory like $GIT_DIR/refs/refs~d/heads~d/foo~d/bar~d/baz~f (which would also give a better place to store top-level reference names). I structured the patches to apply on top of Jeff's for presentation purposes, but if they are desired it would of course make more sense to squash his and mine together in the obvious way. I am a little bit worried that there are other test cases that use git prune in the belief that it will remove all commits that were referred to by deleted references. The test suite runs cleanly for me with these patches, but before they are integrated we should audit the places where the test suite calls to git prune to make sure that they are still testing what they think. Michael Haggerty (3): t9300: format test in modern style prior to modifying it Delete reflogs for dead references to allow pruning Change naming convention for the reflog graveyard refs.c | 31 --- t/t7701-repack-unpack-unreachable.sh | 4 ++-- t/t9300-fast-import.sh | 13 +++-- 3 files changed, 33 insertions(+), 15 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 0/2] Fix two minor problems in the docs for git-config
From: Michael Haggerty mhag...@alum.mit.edu This is just something I stumbled across. Michael Haggerty (2): git-config.txt: properly escape quotation marks in example git-config.txt: fix example Documentation/git-config.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 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 1/2] git-config.txt: properly escape quotation marks in example
From: Michael Haggerty mhag...@alum.mit.edu In the example line as written, gitproxy=proxy-command for kernel.org the quotation marks are eaten by the config-file parser. From the history, it looks like this example wanted to have quotation marks in the actual configured value. So quote them as required nowadays. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- The bigger question is whether this example is improved by including quotation marks, or whether they are just a distraction from the main point. I abstain. Documentation/git-config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 2d6ef32..46775fe 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -267,7 +267,7 @@ Given a .git/config like this: ; Proxy settings [core] - gitproxy=proxy-command for kernel.org + gitproxy=\proxy-command\ for kernel.org gitproxy=default-proxy ; for all the rest you can set the filemode to true with -- 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 2/2] git-config.txt: fix example
From: Michael Haggerty mhag...@alum.mit.edu The --add option is required to add a new value to a multivalued configuration entry. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/git-config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 46775fe..584ef51 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -342,7 +342,7 @@ To actually match only values with an exclamation mark, you have to To add a new proxy, without altering any of the existing ones, use -% git config core.gitproxy 'proxy-command for example.com' +% git config --add core.gitproxy 'proxy-command for example.com' An example to use customized color from the configuration in your -- 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 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
[PATCH v2 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 v2 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 | 35 +--
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cc21047..76288a2 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -521,27 +521,27 @@ 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;
struct ref **newtail = newlist;
struct ref *ref, *next;
struct ref *fastarray[32];
- int match_pos;
+ int head_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
return_refs = NULL;
- match_pos = 0;
+ head_pos = 0;
for (ref = *refs; ref; ref = next) {
next = ref-next;
if (!memcmp(ref-name, refs/, 5)
@@ -556,17 +556,17 @@ 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 (head_pos nr_heads) {
+ cmp = strcmp(ref-name, heads[head_pos]);
if (cmp 0) /* definitely do not have it */
break;
else if (cmp == 0) { /* definitely have it */
- match[match_pos][0] = '\0';
- return_refs[match_pos] = ref;
+ heads[head_pos][0] = '\0';
+ return_refs[head_pos] = ref;
break;
}
else /* might have it; keep looking */
- match_pos++;
+ head_pos++;
}
if (!cmp)
continue; /* we will link it later */
@@ -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
[PATCH v2 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 cf65998..fae4f7c 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 v2 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 c47090d..ca1ddd9 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 head_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 v2 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 5ba1cef..f04fd59 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 v2 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 f04fd59..00ac3b1 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
[PATCH v2 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 00ac3b1..c49dadf 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 v2 17/17] filter_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 1bc4599..db77ee6 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 head_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 head_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 (head_pos *nr_heads) {
- cmp = strcmp(ref-name, heads[head_pos]);
- if (cmp 0) /* definitely do not have it */
+ int cmp = strcmp(ref-name, heads[head_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[head_pos++]);
+ keep_ref = 1;
break;
- }
- else { /* might have it; keep looking */
+ } else { /* might have it; keep looking */
heads[unmatched++] = heads[head_pos++];
}
}
- if (!cmp)
- continue; /* we will link it later */
- }
- free(ref);
- }
-
- if (!args.fetch_all) {
- int i;
- /* copy remaining unmatched heads: */
- while (head_pos *nr_heads)
- heads[unmatched++] = heads[head_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 (head_pos *nr_heads)
+ heads[unmatched++] = heads[head_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 v2 00/17] Clean up how fetch_pack() handles the heads list
From: Michael Haggerty mhag...@alum.mit.edu Re-roll, incorporating Jeff's suggestions. Some commit messages have also been improved, but the only interdiff is that match_pos is renamed to head_pos in filter_refs(). 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 head_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 filter_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 v2 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 v2 05/17] Do not check the same head_pos twice
From: Michael Haggerty mhag...@alum.mit.edu
Once a match has been found at head_pos, the entry is zeroed and no
future attempts will match that entry. So increment head_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 76288a2..bda3c0c 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[head_pos][0] = '\0';
return_refs[head_pos] = ref;
+ heads[head_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 v2 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 a4bb0ff..c47090d 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 head_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 (head_pos nr_heads) {
+ while (head_pos *nr_heads) {
cmp = strcmp(ref-name, heads[head_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 v2 06/17] Let fetch_pack() inform caller about number of unique heads
From: Michael Haggerty mhag...@alum.mit.edu
fetch_pack() removes 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 bda3c0c..cf65998 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 v2 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 ca1ddd9..a995357 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 head_pos;
+ int head_pos = 0, matched = 0;
if (*nr_heads !args.fetch_all)
return_refs = xcalloc(*nr_heads, sizeof(struct ref *));
else
return_refs = NULL;
- head_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[head_pos] = ref;
+ return_refs[matched++] = ref;
heads[head_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 1/7] t0000: verify that absolute_path() fails if passed the empty string
From: Michael Haggerty mhag...@alum.mit.edu It doesn't, so mark the test as failing. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t-basic.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/t-basic.sh b/t/t-basic.sh index ccb5435..7347fb8 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -450,6 +450,10 @@ test_expect_success 'update-index D/F conflict' ' test $numpath0 = 1 ' +test_expect_failure 'absolute path rejects the empty string' ' + test_must_fail test-path-utils absolute_path +' + test_expect_success SYMLINKS 'real path works as expected' ' mkdir first ln -s ../.git first/.git -- 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 2/7] absolute_path(): reject the empty string
From: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
abspath.c| 4 +++-
t/t-basic.sh | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/abspath.c b/abspath.c
index f04ac18..5d62430 100644
--- a/abspath.c
+++ b/abspath.c
@@ -123,7 +123,9 @@ const char *absolute_path(const char *path)
{
static char buf[PATH_MAX + 1];
- if (is_absolute_path(path)) {
+ if (!*path) {
+ die(The empty string is not a valid path);
+ } else if (is_absolute_path(path)) {
if (strlcpy(buf, path, PATH_MAX) = PATH_MAX)
die(Too long path: %.*s, 60, path);
} else {
diff --git a/t/t-basic.sh b/t/t-basic.sh
index 7347fb8..5963a6c 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -450,7 +450,7 @@ test_expect_success 'update-index D/F conflict' '
test $numpath0 = 1
'
-test_expect_failure 'absolute path rejects the empty string' '
+test_expect_success 'absolute path rejects the empty string' '
test_must_fail test-path-utils absolute_path
'
--
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 0/7] Fix some bugs in abspath.c
From: Michael Haggerty mhag...@alum.mit.edu I really just wanted to tidy up filter_refs(), but I've been sucked into a cascade of recursive yak shaving. This is my first attempt to pop the yak stack. I want to use real_path() for making the handling of GIT_CEILING_DIRECTORIES more robust, but I noticed that it is broken for some cases that will be important in this context. This patch series adds some new tests of that function and fixes the ones that are broken, including a similar breakage in absolute_path(). It applies to the current master. Please note that both absolute_path() and real_path() used to return the current directory followed by a slash. I believe that this was a bug, and that it is more appropriate for both functions to reject the empty string. The evidence is as follows: * If this were intended behavior, presumably the return value would *not* have a trailing slash. * I couldn't find any callers that appeared to depend on the old behavior. * The test suite runs without errors after the change. But I didn't do a thorough code review to ensure that no callers ever rely on the old behavior. The most likely scenario would be if one of these functions were used to parse something like $PATH, where the empty string denotes the current directory, but I didn't see any callers that seemed to be doing anything like this. Michael Haggerty (7): t: verify that absolute_path() fails if passed the empty string absolute_path(): reject the empty string t: verify that real_path() fails if passed the empty string real_path(): reject the empty string t: verify that real_path() works correctly with absolute paths real_path(): properly handle nonexistent top-level paths t: verify that real_path() removes extra slashes abspath.c| 12 ++-- t/t-basic.sh | 31 ++- 2 files changed, 40 insertions(+), 3 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 7/7] t0000: verify that real_path() removes extra slashes
From: Michael Haggerty mhag...@alum.mit.edu These tests already pass, but make sure they don't break in the future. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- It would be great if somebody would check whether these tests pass on Windows, and if not, give me a tip about how to fix them. t/t-basic.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t-basic.sh b/t/t-basic.sh index d929578..3c75e97 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -468,6 +468,17 @@ test_expect_success 'real path works on absolute paths' ' test $d/$nopath = $(test-path-utils real_path $d/$nopath) ' +test_expect_success 'real path removes extra slashes' ' + nopath=hopefully-absent-path + test / = $(test-path-utils real_path ///) + test /$nopath = $(test-path-utils real_path ///$nopath) + # We need an existing top-level directory to use in the + # remaining tests. Use the top-level ancestor of $(pwd): + d=$(pwd -P | sed -e s|^\(/[^/]*\)/.*|\1|) + test $d = $(test-path-utils real_path //$d///) + test $d/$nopath = $(test-path-utils real_path //$d///$nopath) +' + test_expect_success SYMLINKS 'real path works on symlinks' ' mkdir first ln -s ../.git first/.git -- 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 5/7] t0000: verify that real_path() works correctly with absolute paths
From: Michael Haggerty mhag...@alum.mit.edu There is currently a bug: if passed an absolute top-level path that doesn't exist (e.g., /foo) it incorrectly interprets the path as a relative path (e.g., returns $(pwd)/foo). So mark the test as failing. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t-basic.sh | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index 1a51634..ad002ee 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -458,7 +458,17 @@ test_expect_success 'real path rejects the empty string' ' test_must_fail test-path-utils real_path ' -test_expect_success SYMLINKS 'real path works as expected' ' +test_expect_failure 'real path works on absolute paths' ' + nopath=hopefully-absent-path + test / = $(test-path-utils real_path /) + test /$nopath = $(test-path-utils real_path /$nopath) + # Find an existing top-level directory for the remaining tests: + d=$(pwd -P | sed -e s|^\(/[^/]*\)/.*|\1|) + test $d = $(test-path-utils real_path $d) + test $d/$nopath = $(test-path-utils real_path $d/$nopath) +' + +test_expect_success SYMLINKS 'real path works on symlinks' ' mkdir first ln -s ../.git first/.git mkdir second -- 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
