Re: [RFC PATCH 08/12] commit-graph: convert to using the_hash_algo

2018-09-03 Thread brian m. carlson
On Wed, Aug 29, 2018 at 08:41:36AM -0400, Derrick Stolee wrote:
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index 6aee861f78..676c1a9ae0 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -333,7 +333,7 @@ test_expect_success 'git commit-graph verify' '
> 
>  NUM_COMMITS=9
>  NUM_OCTOPUS_EDGES=2
> -HASH_LEN=20
> +HASH_LEN="$(test_oid rawsz)"
>  GRAPH_BYTE_VERSION=4
>  GRAPH_BYTE_HASH=5
>  GRAPH_BYTE_CHUNK_COUNT=6

I dropped this at the end of my hash-independent fixes series and I
slipped in a use of test_oid_init, which is now required.  Thanks for
the patch.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: [RFC PATCH 08/12] commit-graph: convert to using the_hash_algo

2018-08-29 Thread brian m. carlson
On Wed, Aug 29, 2018 at 08:41:36AM -0400, Derrick Stolee wrote:
> On 8/28/2018 8:58 PM, brian m. carlson wrote:
> > Instead of using hard-coded constants for object sizes, use
> > the_hash_algo to look them up.  In addition, use a function call to look
> > up the object ID version and produce the correct value.
> 
> The C code in this patch looks good to me. The only issue is that I predict
> failure in the 'git commit-graph verify' tests in t5318-commit-graph.sh.
> Squashing in this commit should help (assuming that test_oid works, it
> doesn't at my current branch):

Yeah, this is a separate series not based on the other one.  If I
finally submit this after the other series lands, I'll squash that
change in.

Thanks for the patch.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: [RFC PATCH 08/12] commit-graph: convert to using the_hash_algo

2018-08-29 Thread Derrick Stolee

On 8/28/2018 8:58 PM, brian m. carlson wrote:

Instead of using hard-coded constants for object sizes, use
the_hash_algo to look them up.  In addition, use a function call to look
up the object ID version and produce the correct value.


The C code in this patch looks good to me. The only issue is that I 
predict failure in the 'git commit-graph verify' tests in 
t5318-commit-graph.sh. Squashing in this commit should help (assuming 
that test_oid works, it doesn't at my current branch):


-->8--

Subject: [PATCH] t5318-commit-graph.sh: use test_oid for HASH_LEN

Signed-off-by: Derrick Stolee 
---
 t/t5318-commit-graph.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 6aee861f78..676c1a9ae0 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -333,7 +333,7 @@ test_expect_success 'git commit-graph verify' '

 NUM_COMMITS=9
 NUM_OCTOPUS_EDGES=2
-HASH_LEN=20
+HASH_LEN="$(test_oid rawsz)"
 GRAPH_BYTE_VERSION=4
 GRAPH_BYTE_HASH=5
 GRAPH_BYTE_CHUNK_COUNT=6
--
2.19.0.rc1


[RFC PATCH 08/12] commit-graph: convert to using the_hash_algo

2018-08-28 Thread brian m. carlson
Instead of using hard-coded constants for object sizes, use
the_hash_algo to look them up.  In addition, use a function call to look
up the object ID version and produce the correct value.

Signed-off-by: brian m. carlson 
---
 commit-graph.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index 8a1bec7b8a..29356d84a2 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -25,11 +25,6 @@
 #define GRAPH_VERSION_1 0x1
 #define GRAPH_VERSION GRAPH_VERSION_1
 
-#define GRAPH_OID_VERSION_SHA1 1
-#define GRAPH_OID_LEN_SHA1 GIT_SHA1_RAWSZ
-#define GRAPH_OID_VERSION GRAPH_OID_VERSION_SHA1
-#define GRAPH_OID_LEN GRAPH_OID_LEN_SHA1
-
 #define GRAPH_OCTOPUS_EDGES_NEEDED 0x8000
 #define GRAPH_PARENT_MISSING 0x7fff
 #define GRAPH_EDGE_LAST_MASK 0x7fff
@@ -41,13 +36,18 @@
 #define GRAPH_FANOUT_SIZE (4 * 256)
 #define GRAPH_CHUNKLOOKUP_WIDTH 12
 #define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
-   + GRAPH_FANOUT_SIZE + GRAPH_OID_LEN)
+   + GRAPH_FANOUT_SIZE + the_hash_algo->rawsz)
 
 char *get_commit_graph_filename(const char *obj_dir)
 {
return xstrfmt("%s/info/commit-graph", obj_dir);
 }
 
+static uint8_t oid_version(void)
+{
+   return 1;
+}
+
 static struct commit_graph *alloc_commit_graph(void)
 {
struct commit_graph *g = xcalloc(1, sizeof(*g));
@@ -100,15 +100,15 @@ struct commit_graph *load_commit_graph_one(const char 
*graph_file)
}
 
hash_version = *(unsigned char*)(data + 5);
-   if (hash_version != GRAPH_OID_VERSION) {
+   if (hash_version != oid_version()) {
error(_("hash version %X does not match version %X"),
- hash_version, GRAPH_OID_VERSION);
+ hash_version, oid_version());
goto cleanup_fail;
}
 
graph = alloc_commit_graph();
 
-   graph->hash_len = GRAPH_OID_LEN;
+   graph->hash_len = the_hash_algo->rawsz;
graph->num_chunks = *(unsigned char*)(data + 6);
graph->graph_fd = fd;
graph->data = graph_map;
@@ -124,7 +124,7 @@ struct commit_graph *load_commit_graph_one(const char 
*graph_file)
 
chunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH;
 
-   if (chunk_offset > graph_size - GIT_MAX_RAWSZ) {
+   if (chunk_offset > graph_size - the_hash_algo->rawsz) {
error(_("improper chunk offset %08x%08x"), 
(uint32_t)(chunk_offset >> 32),
  (uint32_t)chunk_offset);
goto cleanup_fail;
@@ -692,6 +692,7 @@ void write_commit_graph(const char *obj_dir,
int num_chunks;
int num_extra_edges;
struct commit_list *parent;
+   const unsigned hashsz = the_hash_algo->rawsz;
 
oids.nr = 0;
oids.alloc = approximate_object_count() / 4;
@@ -812,7 +813,7 @@ void write_commit_graph(const char *obj_dir,
hashwrite_be32(f, GRAPH_SIGNATURE);
 
hashwrite_u8(f, GRAPH_VERSION);
-   hashwrite_u8(f, GRAPH_OID_VERSION);
+   hashwrite_u8(f, oid_version());
hashwrite_u8(f, num_chunks);
hashwrite_u8(f, 0); /* unused padding byte */
 
@@ -827,8 +828,8 @@ void write_commit_graph(const char *obj_dir,
 
chunk_offsets[0] = 8 + (num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH;
chunk_offsets[1] = chunk_offsets[0] + GRAPH_FANOUT_SIZE;
-   chunk_offsets[2] = chunk_offsets[1] + GRAPH_OID_LEN * commits.nr;
-   chunk_offsets[3] = chunk_offsets[2] + (GRAPH_OID_LEN + 16) * commits.nr;
+   chunk_offsets[2] = chunk_offsets[1] + hashsz * commits.nr;
+   chunk_offsets[3] = chunk_offsets[2] + (hashsz + 16) * commits.nr;
chunk_offsets[4] = chunk_offsets[3] + 4 * num_extra_edges;
 
for (i = 0; i <= num_chunks; i++) {
@@ -841,8 +842,8 @@ void write_commit_graph(const char *obj_dir,
}
 
write_graph_chunk_fanout(f, commits.list, commits.nr);
-   write_graph_chunk_oids(f, GRAPH_OID_LEN, commits.list, commits.nr);
-   write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr);
+   write_graph_chunk_oids(f, hashsz, commits.list, commits.nr);
+   write_graph_chunk_data(f, hashsz, commits.list, commits.nr);
write_graph_chunk_large_edges(f, commits.list, commits.nr);
 
close_commit_graph();