Re: Fwd: [PATCH/RFC] upload-pack: ignore 'shallow' lines with unknown obj-ids

Subsystems: documentation, the rest

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: Fwd: [PATCH/RFC] upload-pack: ignore 'shallow' lines with unknown obj-ids

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:03

Junio C Hamano [off-list ref] writes:
OK, the change the patch text shows looks sensible to me.  Can you
resend it in full, with the log message, your sign-off, and Duy's
"Reviewed-by:", so that it can be applied?
I managed to reassemble what I _think_ is close to the original from
a few messages by dequoting and fixing MUA whitespace breakages in
them.

Here is with a slight tweak to the log message.  Please holler if I
grabbed a wrong version of the patch or made any silly mistakes.

Thanks.

-- >8 --
From: Michael Heemskerk <redacted>
Subject: [PATCH] upload-pack: ignore 'shallow' lines with unknown obj-ids

When the client sends a 'shallow' line for an object that the server does
not have, the server currently dies with the error: "did not find object
for shallow <obj-id>".  The client may have truncated the history at
the commit by fetching shallowly from a different server, or the commit
may have been garbage collected by the server. In either case, this
unknown commit is not relevant for calculating the pack that is to be
sent and can be safely ignored, and it is not used when recomputing where
the updated history of the client is cauterised.

The documentation in technical/pack-protocol.txt has been updated to
remove the restriction that "Clients MUST NOT mention an obj-id which it
does not know exists on the server". This requirement is not realistic
because clients cannot know whether an object has been garbage collected
by the server.

Signed-off-by: Michael Heemskerk <redacted>
Reviewed-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 Documentation/technical/pack-protocol.txt | 3 +--
 upload-pack.c                             | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index f1a51ed..b898e97 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -228,8 +228,7 @@ obtained through ref discovery.
 The client MUST write all obj-ids which it only has shallow copies
 of (meaning that it does not have the parents of a commit) as
 'shallow' lines so that the server is aware of the limitations of
-the client's history. Clients MUST NOT mention an obj-id which
-it does not know exists on the server.
+the client's history.
 
 The client now sends the maximum commit history depth it wants for
 this transaction, which is the number of commits it wants from the
diff --git a/upload-pack.c b/upload-pack.c
index bfa6279..127e59a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -592,7 +592,7 @@ static void receive_needs(void)
 				die("invalid shallow line: %s", line);
 			object = parse_object(sha1);
 			if (!object)
-				die("did not find object for %s", line);
+				continue;
 			if (object->type != OBJ_COMMIT)
 				die("invalid shallow object %s", sha1_to_hex(sha1));
 			if (!(object->flags & CLIENT_SHALLOW)) {
-- 
1.8.3-rc0-117-g5915a95

Re: Fwd: [PATCH/RFC] upload-pack: ignore 'shallow' lines with unknown obj-ids

From: Michael Heemskerk <hidden>
Date: 2016-06-15 22:57:03

Thanks Junio,

That looks fine to me. I'll try to find some time this week to create a
follow-up patch for removing pruned commits from the shallow file as
well.

Cheers,
Michael

On 29 April 2013 15:32, Junio C Hamano [off-list ref] wrote:
quoted hunk
Junio C Hamano [off-list ref] writes:
quoted
OK, the change the patch text shows looks sensible to me.  Can you
resend it in full, with the log message, your sign-off, and Duy's
"Reviewed-by:", so that it can be applied?
I managed to reassemble what I _think_ is close to the original from
a few messages by dequoting and fixing MUA whitespace breakages in
them.

Here is with a slight tweak to the log message.  Please holler if I
grabbed a wrong version of the patch or made any silly mistakes.

Thanks.

-- >8 --
From: Michael Heemskerk <redacted>
Subject: [PATCH] upload-pack: ignore 'shallow' lines with unknown obj-ids

When the client sends a 'shallow' line for an object that the server does
not have, the server currently dies with the error: "did not find object
for shallow <obj-id>".  The client may have truncated the history at
the commit by fetching shallowly from a different server, or the commit
may have been garbage collected by the server. In either case, this
unknown commit is not relevant for calculating the pack that is to be
sent and can be safely ignored, and it is not used when recomputing where
the updated history of the client is cauterised.

The documentation in technical/pack-protocol.txt has been updated to
remove the restriction that "Clients MUST NOT mention an obj-id which it
does not know exists on the server". This requirement is not realistic
because clients cannot know whether an object has been garbage collected
by the server.

Signed-off-by: Michael Heemskerk <redacted>
Reviewed-by: Nguyễn Thái Ngọc Duy <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 Documentation/technical/pack-protocol.txt | 3 +--
 upload-pack.c                             | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index f1a51ed..b898e97 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -228,8 +228,7 @@ obtained through ref discovery.
 The client MUST write all obj-ids which it only has shallow copies
 of (meaning that it does not have the parents of a commit) as
 'shallow' lines so that the server is aware of the limitations of
-the client's history. Clients MUST NOT mention an obj-id which
-it does not know exists on the server.
+the client's history.

 The client now sends the maximum commit history depth it wants for
 this transaction, which is the number of commits it wants from the
diff --git a/upload-pack.c b/upload-pack.c
index bfa6279..127e59a 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -592,7 +592,7 @@ static void receive_needs(void)
                                die("invalid shallow line: %s", line);
                        object = parse_object(sha1);
                        if (!object)
-                               die("did not find object for %s", line);
+                               continue;
                        if (object->type != OBJ_COMMIT)
                                die("invalid shallow object %s", sha1_to_hex(sha1));
                        if (!(object->flags & CLIENT_SHALLOW)) {
--
1.8.3-rc0-117-g5915a95
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help