Re: [PATCH 05/23] midx: write header information to lockfile
On 6/19/2018 10:59 AM, Duy Nguyen wrote:
On Tue, Jun 19, 2018 at 2:54 PM Derrick Stolee wrote:
On 6/12/2018 11:00 AM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 7:01 PM Derrick Stolee wrote:
diff --git a/midx.c b/midx.c
index 616af66b13..3e55422a21 100644
--- a/midx.c
+++ b/midx.c
@@ -1,9 +1,62 @@
#include "git-compat-util.h"
#include "cache.h"
#include "dir.h"
+#include "csum-file.h"
+#include "lockfile.h"
#include "midx.h"
+#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
+#define MIDX_VERSION 1
+#define MIDX_HASH_VERSION 1 /* SHA-1 */
...
+static size_t write_midx_header(struct hashfile *f,
+ unsigned char num_chunks,
+ uint32_t num_packs)
+{
+ char byte_values[4];
+ hashwrite_be32(f, MIDX_SIGNATURE);
+ byte_values[0] = MIDX_VERSION;
+ byte_values[1] = MIDX_HASH_VERSION;
Quoting from "State of NewHash work, future directions, and discussion" [1]
* If you need to serialize an algorithm identifier into your data
format, use the format_id field of struct git_hash_algo. It's
designed specifically for that purpose.
[1]
https://public-inbox.org/git/20180612024252.ga141...@aiede.svl.corp.google.com/T/#m5fdd09dcaf31266c45343fb6c0beaaa3e928bc60
Thanks! I'll also use the_hash_algo->rawsz to infer the length of the
hash function.
BTW, since you're the author of commit-graph.c and may notice it has
the same problem. Don't touch that code. Brian already has some WIP
changes [1]. We just make sure new code does not add extra work for
him. I expect he'll send all those patches out soon.
[1]
https://github.com/bk2204/git/commit/3f9031e06cfb21534eb7dfff7b54e7598ac1149f
Thanks for the link. It seems he is creating an oid_version() method
that returns a 1-byte version for the hash version instead of the 4-byte
signature of the_hash_algo->format_id. I look forward to incorporating
that into the MIDX format. I'll keep my macros for now, as we work out
the other details, and while Brain's patch is cooking.
Thanks,
-Stolee
Re: [PATCH 05/23] midx: write header information to lockfile
On Tue, Jun 19, 2018 at 2:54 PM Derrick Stolee wrote:
>
> On 6/12/2018 11:00 AM, Duy Nguyen wrote:
> > On Thu, Jun 7, 2018 at 7:01 PM Derrick Stolee wrote:
> >> diff --git a/midx.c b/midx.c
> >> index 616af66b13..3e55422a21 100644
> >> --- a/midx.c
> >> +++ b/midx.c
> >> @@ -1,9 +1,62 @@
> >> #include "git-compat-util.h"
> >> #include "cache.h"
> >> #include "dir.h"
> >> +#include "csum-file.h"
> >> +#include "lockfile.h"
> >> #include "midx.h"
> >>
> >> +#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
> >> +#define MIDX_VERSION 1
> >> +#define MIDX_HASH_VERSION 1 /* SHA-1 */
> > ...
> >> +static size_t write_midx_header(struct hashfile *f,
> >> + unsigned char num_chunks,
> >> + uint32_t num_packs)
> >> +{
> >> + char byte_values[4];
> >> + hashwrite_be32(f, MIDX_SIGNATURE);
> >> + byte_values[0] = MIDX_VERSION;
> >> + byte_values[1] = MIDX_HASH_VERSION;
> > Quoting from "State of NewHash work, future directions, and discussion" [1]
> >
> > * If you need to serialize an algorithm identifier into your data
> >format, use the format_id field of struct git_hash_algo. It's
> >designed specifically for that purpose.
> >
> > [1]
> > https://public-inbox.org/git/20180612024252.ga141...@aiede.svl.corp.google.com/T/#m5fdd09dcaf31266c45343fb6c0beaaa3e928bc60
>
> Thanks! I'll also use the_hash_algo->rawsz to infer the length of the
> hash function.
BTW, since you're the author of commit-graph.c and may notice it has
the same problem. Don't touch that code. Brian already has some WIP
changes [1]. We just make sure new code does not add extra work for
him. I expect he'll send all those patches out soon.
[1]
https://github.com/bk2204/git/commit/3f9031e06cfb21534eb7dfff7b54e7598ac1149f
--
Duy
Re: [PATCH 05/23] midx: write header information to lockfile
On 6/12/2018 11:00 AM, Duy Nguyen wrote:
On Thu, Jun 7, 2018 at 7:01 PM Derrick Stolee wrote:
diff --git a/midx.c b/midx.c
index 616af66b13..3e55422a21 100644
--- a/midx.c
+++ b/midx.c
@@ -1,9 +1,62 @@
#include "git-compat-util.h"
#include "cache.h"
#include "dir.h"
+#include "csum-file.h"
+#include "lockfile.h"
#include "midx.h"
+#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
+#define MIDX_VERSION 1
+#define MIDX_HASH_VERSION 1 /* SHA-1 */
...
+static size_t write_midx_header(struct hashfile *f,
+ unsigned char num_chunks,
+ uint32_t num_packs)
+{
+ char byte_values[4];
+ hashwrite_be32(f, MIDX_SIGNATURE);
+ byte_values[0] = MIDX_VERSION;
+ byte_values[1] = MIDX_HASH_VERSION;
Quoting from "State of NewHash work, future directions, and discussion" [1]
* If you need to serialize an algorithm identifier into your data
format, use the format_id field of struct git_hash_algo. It's
designed specifically for that purpose.
[1]
https://public-inbox.org/git/20180612024252.ga141...@aiede.svl.corp.google.com/T/#m5fdd09dcaf31266c45343fb6c0beaaa3e928bc60
Thanks! I'll also use the_hash_algo->rawsz to infer the length of the
hash function.
Re: [PATCH 05/23] midx: write header information to lockfile
On Thu, Jun 7, 2018 at 7:01 PM Derrick Stolee wrote:
> diff --git a/midx.c b/midx.c
> index 616af66b13..3e55422a21 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -1,9 +1,62 @@
> #include "git-compat-util.h"
> #include "cache.h"
> #include "dir.h"
> +#include "csum-file.h"
> +#include "lockfile.h"
> #include "midx.h"
>
> +#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
> +#define MIDX_VERSION 1
> +#define MIDX_HASH_VERSION 1 /* SHA-1 */
...
> +static size_t write_midx_header(struct hashfile *f,
> + unsigned char num_chunks,
> + uint32_t num_packs)
> +{
> + char byte_values[4];
> + hashwrite_be32(f, MIDX_SIGNATURE);
> + byte_values[0] = MIDX_VERSION;
> + byte_values[1] = MIDX_HASH_VERSION;
Quoting from "State of NewHash work, future directions, and discussion" [1]
* If you need to serialize an algorithm identifier into your data
format, use the format_id field of struct git_hash_algo. It's
designed specifically for that purpose.
[1]
https://public-inbox.org/git/20180612024252.ga141...@aiede.svl.corp.google.com/T/#m5fdd09dcaf31266c45343fb6c0beaaa3e928bc60
> + byte_values[2] = num_chunks;
> + byte_values[3] = 0; /* unused */
> + hashwrite(f, byte_values, sizeof(byte_values));
> + hashwrite_be32(f, num_packs);
> +
> + return MIDX_HEADER_SIZE;
> +}
--
Duy
Re: [PATCH 05/23] midx: write header information to lockfile
On Thu, Jun 7, 2018 at 4:03 PM, Derrick Stolee wrote:
> +static char *get_midx_filename(const char *object_dir)
> +{
> + struct strbuf midx_name = STRBUF_INIT;
> + strbuf_addstr(&midx_name, object_dir);
> + strbuf_addstr(&midx_name, "/pack/multi-pack-index");
> + return strbuf_detach(&midx_name, NULL);
> +}
I think this whole function can be written as
xstrfmt("%s/pack/multi-pack-index", object_dir);
> +
> +static size_t write_midx_header(struct hashfile *f,
> + unsigned char num_chunks,
> + uint32_t num_packs)
> +{
> + char byte_values[4];
unsigned char just to be on the safe side? 'char' is signed on ARM if
I remember correctly.
--
Duy
