From: Michael Haggerty <redacted>
It used to be that alternate references were not considered "complete"
when fetching via fetch-pack. This failure was not so obvious because
the big benefit of alternates is seen when cloning, and clone used a
different data path: it put the alternate references into extra refs
(which makes them look like references within the local repository).
This patch series teaches fetch-pack to treat objects that are
available via alternates as "complete".
Once that is fixed, clone doesn't need to use the special extra_refs
kludge, so change that.
And once that is changed, the extra_refs API is no longer needed at
all, so remove it.
Michael Haggerty (7):
t5700: document a failure of alternates to affect fetch
clone.c: move more code into the "if (refs)" conditional
fetch-pack.c: rename some parameters from "path" to "refname"
fetch-pack.c: inline insert_alternate_refs()
everything_local(): mark alternate refs as complete
clone: do not add alternate references to extra_refs
refs: remove the extra_refs API
builtin/clone.c | 51 +++++++++++++++++--------------------------
builtin/fetch-pack.c | 23 ++++++++++---------
refs.c | 23 +-------------------
refs.h | 8 -------
t/t5700-clone-reference.sh | 34 ++++++++++++++++++++++++++--
5 files changed, 64 insertions(+), 75 deletions(-)
--
1.7.9
From: Michael Haggerty <redacted>
If an alternate supplies some, but not all, of the objects needed for
a fetch, fetch-pack nevertheless generates "want" lines for the
alternate objects that are present. Demonstrate this problem via a
failing test.
Signed-off-by: Michael Haggerty <redacted>
---
t/t5700-clone-reference.sh | 34 +++++++++++++++++++++++++++++++---
1 files changed, 31 insertions(+), 3 deletions(-)
From: Michael Haggerty <redacted>
The bahavior of a bunch of code before the "if (refs)" statement also
depends on whether refs is set, so make the logic clearer by shifting
this code into the if statement.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/clone.c | 39 ++++++++++++++++++++-------------------
1 files changed, 20 insertions(+), 19 deletions(-)
@@ -852,6 +852,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)}else{warning(_("You appear to have cloned an empty repository."));+mapped_refs=NULL;our_head_points_at=NULL;remote_head_points_at=NULL;remote_head=NULL;
From: Michael Haggerty <redacted>
The parameters denote reference names, which are no longer 1:1 with
filesystem paths.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/fetch-pack.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
From: Michael Haggerty <redacted>
Alternate references are directly (and now, correctly) handled by
fetch-pack, so there is no need to inform fetch-pack about them via
the extra_refs back channel.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/clone.c | 12 ------------
1 files changed, 0 insertions(+), 12 deletions(-)
From: Michael Haggerty <redacted>
The extra_refs provided a kludgy way to create fake references at a
global level in the hope that they would only affect some particular
code path. The last user of this API been rewritten, so strip this
stuff out before somebody else gets the bad idea of using it.
Signed-off-by: Michael Haggerty <redacted>
---
refs.c | 23 +----------------------
refs.h | 8 --------
2 files changed, 1 insertions(+), 30 deletions(-)
From: Michael Haggerty <redacted>
Objects in an alternate object database are already available to the
local repository and therefore don't need to be fetched. So mark them
as complete in everything_local().
This fixes a test in t5700.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/fetch-pack.c | 6 ++++++
t/t5700-clone-reference.sh | 2 +-
2 files changed, 7 insertions(+), 1 deletions(-)
From: Michael Haggerty <redacted>
The logic of the (single) caller is clearer without encapsulating this
one line in a function.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/fetch-pack.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
From: Junio C Hamano <hidden> Date: 2016-06-15 22:53:03
mhagger@alum.mit.edu writes:
+test_expect_success 'prepare branched repository' '
+ git clone A J &&
+ (
+ cd J &&
+ git checkout -b other master^ &&
+ echo other > otherfile &&
s/ > / >/; but that is nothing I cannot fix locally.
+test_expect_failure 'fetch with incomplete alternates' '
I am assuming that this "incomplete" means "this alternate helps reducing
the number of objects we need to fetch from the remote, but it does not
have everything objects we need, and we still need to fetch some from the
remote". Am I correct?
I do not think you meant the alternate repository is in some way corrupt,
but I am just making sure, because I found the phrasing a bit odd.
From: Junio C Hamano <hidden> Date: 2016-06-15 22:53:03
mhagger@alum.mit.edu writes:
From: Michael Haggerty <redacted>
The bahavior of a bunch of code before the "if (refs)" statement also
depends on whether refs is set, so make the logic clearer by shifting
this code into the if statement.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/clone.c | 39 ++++++++++++++++++++-------------------
1 files changed, 20 insertions(+), 19 deletions(-)
From: Junio C Hamano <hidden> Date: 2016-06-15 22:53:03
mhagger@alum.mit.edu writes:
From: Michael Haggerty <redacted>
The parameters denote reference names, which are no longer 1:1 with
filesystem paths.
These three functions are only used as callback from for_each_ref() so
they always get the full refname and nothing else (like a partial refname
like tags/v1.7.9), so calling them refname makes perfect sense.
Even though I generally try to stay away from this kind of naming churn
patches, but hopefully there is nothing in flight to cause horrible
conflict with it.
Thanks.
From: Junio C Hamano <hidden> Date: 2016-06-15 22:53:03
mhagger@alum.mit.edu writes:
From: Michael Haggerty <redacted>
Alternate references are directly (and now, correctly) handled by
fetch-pack, so there is no need to inform fetch-pack about them via
the extra_refs back channel.
Signed-off-by: Michael Haggerty <redacted>
---
builtin/clone.c | 12 ------------
1 files changed, 0 insertions(+), 12 deletions(-)
Very nice (the real niceness primarily comes from the previous step ;-).
Thanks.
From: Michael Haggerty <hidden> Date: 2016-06-15 22:53:03
On 02/13/2012 04:35 AM, Junio C Hamano wrote:
mhagger@alum.mit.edu writes:
quoted
+test_expect_success 'prepare branched repository' '
+ git clone A J &&
+ (
+ cd J &&
+ git checkout -b other master^ &&
+ echo other > otherfile &&
s/ > / >/; but that is nothing I cannot fix locally.
quoted
+test_expect_failure 'fetch with incomplete alternates' '
I am assuming that this "incomplete" means "this alternate helps reducing
the number of objects we need to fetch from the remote, but it does not
have everything objects we need, and we still need to fetch some from the
remote". Am I correct?
Correct. Feel free to improve the description if you can think of a
better way to describe it.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
From: Jeff King <hidden> Date: 2016-06-15 22:53:04
On Sat, Feb 11, 2012 at 07:20:54AM +0100, mhagger@alum.mit.edu wrote:
It used to be that alternate references were not considered "complete"
when fetching via fetch-pack. This failure was not so obvious because
the big benefit of alternates is seen when cloning, and clone used a
different data path: it put the alternate references into extra refs
(which makes them look like references within the local repository).
This patch series teaches fetch-pack to treat objects that are
available via alternates as "complete".
Once that is fixed, clone doesn't need to use the special extra_refs
kludge, so change that.
And once that is changed, the extra_refs API is no longer needed at
all, so remove it.
Michael Haggerty (7):
t5700: document a failure of alternates to affect fetch
clone.c: move more code into the "if (refs)" conditional
fetch-pack.c: rename some parameters from "path" to "refname"
fetch-pack.c: inline insert_alternate_refs()
everything_local(): mark alternate refs as complete
clone: do not add alternate references to extra_refs
refs: remove the extra_refs API
From my reading, all of these patches look good. Thanks for a