git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 00/10] pack v4 UI support

From
Nicolas Pitre <nico@fluxnic.net>
Date
Sep 27, 2013, 02:59 UTC
Message-ID
<alpine.LFD.2.03.1309262150190.312@syhkavp.arg>
In-Reply-To
<CACsJy8De9mMvSpnzHuq+XG+Pfi5QD1fY44uHT8hNQZN6sU-hgg@mail.gmail.com>
On Thu, 26 Sep 2013, Duy Nguyen wrote:
Show 11 quoted lines
> On Thu, Sep 26, 2013 at 11:51 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> >> Multi-base tree support is not part of "packv4" capability. Let's see
> >> if such support comes before the series is merged to master. If so we
> >> can drop that line from protocol-capabilities.txt. Otherwise a new
> >> capability can be added for multi-base trees.
> >
> > What is that for?  Multi-base trees are not created yet, but the code to
> > parse them is already there.  So I don't see the point of having a
> > capability in the protocol for this.
> 
> pv4_get_tree() can. index-pack cannot.
Hmmm... right.

I think this ought to be part of the "packv4" capability nevertheless. This is not going into the official git branch right away so there is still time to implement it.

Show 17 quoted lines
> >> Another capability could be added for sending the actual number of
> >> objects in a thin pack for more accurate display in index-pack. Low
> >> priority in my opinion.
> >
> > That just cannot be communicated during capability exchange.  This
> > number is known only after object enumeration.  Hence my suggestion of a
> > ['T', 'H', 'I', 'N', htonl(<number_of_sent_objects>)] special header
> > prepended to the actual pack on the wire.  And that has to be decided
> > before formalizing the pack v4 capability.  That makes it a somewhat
> > higher priority.
> 
> The capability is to let the server know the client understands ['T',
> 'H', 'I', 'N' ..]. The server can choose not to send it later, but
> that's ok. Hence the new capability. I'm somewhat reluctant to do it
> because of peeking code in fetch-pack and receive-pack and some
> refactoring may be needed first. But I could certainly put it on
> higher priority.
Here's what I'm suggesting as a start (untested):
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3eb5fc..04e5ae1 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -792,9 +792,10 @@ static struct command *read_head_info(void)
 	return commands;
 }
 
-static const char *parse_pack_header(struct pack_header *hdr)
+static const char *parse_pack_header(struct pack_header *hdr,
+				     struct thin_header *thin_hdr)
 {
-	switch (read_pack_header(0, hdr)) {
+	switch (read_pack_header(0, hdr, thin_hdr)) {
 	case PH_ERROR_EOF:
 		return "eof before pack header was fully read";
 
@@ -817,23 +818,25 @@ static const char *pack_lockfile;
 static const char *unpack(int err_fd)
 {
 	struct pack_header hdr;
+	struct thin_header thin_hdr;
 	const char *hdr_err;
-	char hdr_arg[38];
+	char hdr_arg[50];
 	int fsck_objects = (receive_fsck_objects >= 0
 			    ? receive_fsck_objects
 			    : transfer_fsck_objects >= 0
 			    ? transfer_fsck_objects
 			    : 0);
 
-	hdr_err = parse_pack_header(&hdr);
+	hdr_err = parse_pack_header(&hdr, &thin_hdr);
 	if (hdr_err) {
 		if (err_fd > 0)
 			close(err_fd);
 		return hdr_err;
 	}
 	snprintf(hdr_arg, sizeof(hdr_arg),
-			"--pack_header=%"PRIu32",%"PRIu32,
-			ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
+			"--pack_header=%"PRIu32",%"PRIu32",%"PRIu32,
+			ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries),
+			ntohl(thin_hdr.hdr_entries));
 
 	if (ntohl(hdr.hdr_entries) < unpack_limit) {
 		int code, i = 0;
diff --git a/fetch-pack.c b/fetch-pack.c
index 6684348..d86e5d1 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -715,12 +715,14 @@ static int get_pack(struct fetch_pack_args *args,
 	*hdr_arg = 0;
 	if (!args->keep_pack && unpack_limit) {
 		struct pack_header header;
+		struct thin_header thin_hdr;
 
-		if (read_pack_header(demux.out, &header))
+		if (read_pack_header(demux.out, &header, &thin_hdr))
 			die("protocol error: bad pack header");
 		snprintf(hdr_arg, sizeof(hdr_arg),
-			 "--pack_header=%"PRIu32",%"PRIu32,
-			 ntohl(header.hdr_version), ntohl(header.hdr_entries));
+			 "--pack_header=%"PRIu32",%"PRIu32",%"PRIu32,
+			 ntohl(header.hdr_version), ntohl(header.hdr_entries),
+			 ntohl(thin_hdr.hdr_entries));
 		if (ntohl(header.hdr_entries) < unpack_limit)
 			do_keep = 0;
 		else
diff --git a/pack.h b/pack.h
index ccefdbe..974b860 100644
--- a/pack.h
+++ b/pack.h
@@ -16,6 +16,18 @@ struct pack_header {
 };
 
 /*
+ * Pack v4 header prefix for thin packs, indicating the actual number
+ * of objects being transmitted.  Expected before the actual pack header
+ * on the wire and not stored on disk upon reception.  This is not
+ * included in the computation of the pack trailing SHA1.
+ */
+#define THIN_SIGNATURE 0x5448494e	/* "THIN" */
+struct thin_header {
+	uint32_t hdr_signature;
+	uint32_t hdr_entries;
+};
+
+/*
  * The first four bytes of index formats later than version 1 should
  * start with this signature, as all older git binaries would find this
  * value illegal and abort reading the file.
@@ -88,7 +100,7 @@ extern int pv4_encode_object_header(enum object_type, uintmax_t, unsigned char *
 #define PH_ERROR_EOF		(-1)
 #define PH_ERROR_PACK_SIGNATURE	(-2)
 #define PH_ERROR_PROTOCOL	(-3)
-extern int read_pack_header(int fd, struct pack_header *);
+extern int read_pack_header(int fd, struct pack_header *, struct thin_header *);
 
 extern struct sha1file *create_tmp_packfile(char **pack_tmp_name);
 extern void finish_tmp_packfile(char *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
diff --git a/sha1_file.c b/sha1_file.c
index ef6ecc8..5f00c15 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3180,12 +3180,26 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned
 	return 0;
 }
 
-int read_pack_header(int fd, struct pack_header *header)
+int read_pack_header(int fd, struct pack_header *header,
+		     struct thin_header *thin_hdr)
 {
 	if (read_in_full(fd, header, sizeof(*header)) < sizeof(*header))
 		/* "eof before pack header was fully read" */
 		return PH_ERROR_EOF;
 
+	if (header->hdr_signature == htonl(THIN_SIGNATURE)) {
+		char *thin_hdr_end = (char *)header + sizeof(*thin_hdr);
+		int leftover = sizeof(*header) - sizeof(*thin_hdr);
+		if (!thin_hdr)
+			return PH_ERROR_PROTOCOL;
+		memcpy(thin_hdr, header, sizeof(*thin_hdr));
+		memcpy(header, thin_hdr_end, leftover);
+		if (read_in_full(fd, thin_hdr_end, sizeof(*header) - leftover)
+				< sizeof(*header) - leftover)
+			return PH_ERROR_EOF;
+	} else if (thin_hdr)
+		memset(thin_hdr, 0, sizeof(*thin_hdr));
+
 	if (header->hdr_signature != htonl(PACK_SIGNATURE))
 		/* "protocol error (pack signature mismatch detected)" */
 		return PH_ERROR_PACK_SIGNATURE;


Nicolas
Previous: Duy Nguyen
Message 20 of 20 in “pack v4 UI support”
  1. 00/10 pack v4 UI supportNguyễn Thái Ngọc Duy, Sep 26, 2013
  2. 01/10 test-dump: new test program to examine binary dataNguyễn Thái Ngọc Duy, Sep 26, 2013
  3. 02/10 config: add core.preferredPackVersionNguyễn Thái Ngọc Duy, Sep 26, 2013
  4. 03/10 upload-pack: new capability to send pack v4Nguyễn Thái Ngọc Duy, Sep 26, 2013
  5. 04/10 fetch: new option to set preferred pack version for transferNguyễn Thái Ngọc Duy, Sep 26, 2013
  6. 05/10 clone: new option to set preferred pack version for transferNguyễn Thái Ngọc Duy, Sep 26, 2013
  7. 06/10 fetch: pack v4 support on smart httpNguyễn Thái Ngọc Duy, Sep 26, 2013
  8. 07/10 receive-pack: request for packv4 if it's the preferred versionNguyễn Thái Ngọc Duy, Sep 26, 2013
  9. Junio C HamanoOct 17, 2013
  10. 08/10 send-pack: support pack v4Nguyễn Thái Ngọc Duy, Sep 26, 2013
  11. 09/10 repack: add --pack-version and fall back to core.preferredPackVersionNguyễn Thái Ngọc Duy, Sep 26, 2013
  12. repack: Add --version parameterStefan Beller, Sep 26, 2013
  13. Felipe ContrerasSep 26, 2013
  14. Stefan BellerSep 28, 2013
  15. Duy NguyenSep 26, 2013
  16. Stefan BellerSep 28, 2013
  17. 10/10 count-objects: report pack v4 usageNguyễn Thái Ngọc Duy, Sep 26, 2013
  18. Nicolas PitreSep 26, 2013
  19. Duy NguyenSep 26, 2013
  20. Nicolas PitreSep 27, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.