Convert hash-object to struct object_id

PATCH In review Patryk Obara 3 participants 10 messages Aug 20, 2017 – Aug 21, 2017

lore

Share & tools

lore

Patryk Obara

author

2017-08-20 20:09 UTC #0 lore raw

cover Convert hash-object to struct object_id

This enabled conversion of few functions in sha1_file, which had almost all callers converted already.

I hope I'm not stepping on anyone's toes with this patch series. If I do - is there some email thread or document in which I can coordinate with other developers, regarding which code regions are being converted to struct object_id next?

I focused on this builtin in particular because it's probably the smallest functionality, that can be converted to different hashing algorithm, at least partly.

Patryk Obara (6):
builtin/hash-object: convert to struct object_id
read-cache: convert to struct object_id
sha1_file: convert index_path to struct object_id
sha1_file: convert index_fd to struct object_id
sha1_file: convert hash_sha1_file_literally to struct object_id
sha1_file: convert index_stream to struct object_id

 builtin/difftool.c     |  2 +-
 builtin/hash-object.c  | 12 ++++++------
 builtin/replace.c      |  2 +-
 builtin/update-index.c |  2 +-
 cache.h                |  6 +++---
 diff.c                 |  2 +-
 notes-merge.c          |  2 +-
 read-cache.c           |  8 ++++----
 sha1_file.c            | 34 +++++++++++++++++-----------------
 9 files changed, 35 insertions(+), 35 deletions(-)

Signature · 1 line

2.9.5

Patryk Obara

author

2017-08-20 20:09 UTC #1 lore raw

PATCH 1/6 builtin/hash-object: convert to struct object_id

Signed-off-by Patryk Obara <[email protected]>

---
 builtin/hash-object.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

builtin/hash-object.c +6 -6

index d04baf9..1c0f0f3 100644
@@ -16,7 +16,7 @@
  * needs to bypass the data conversion performed by, and the type
  * limitation imposed by, index_fd() and its callees.
  */
-static int hash_literally(unsigned char *sha1, int fd, const char *type, unsigned flags)
+static int hash_literally(struct object_id *oid, int fd, const char *type, unsigned flags)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;

@@ -24,7 +24,7 @@ static int hash_literally(unsigned char *sha1, int fd, const char *type, unsigne
 	if (strbuf_read(&buf, fd, 4096) < 0)
 		ret = -1;
 	else
-		ret = hash_sha1_file_literally(buf.buf, buf.len, type, sha1, flags);
+		ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid->hash, flags);
 	strbuf_release(&buf);
 	return ret;
 }

@@ -33,16 +33,16 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
 		    int literally)
 {
 	struct stat st;
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	if (fstat(fd, &st) < 0 ||
 	    (literally
-	     ? hash_literally(sha1, fd, type, flags)
-	     : index_fd(sha1, fd, &st, type_from_string(type), path, flags)))
+	     ? hash_literally(&oid, fd, type, flags)
+	     : index_fd(oid.hash, fd, &st, type_from_string(type), path, flags)))
 		die((flags & HASH_WRITE_OBJECT)
 		    ? "Unable to add %s to database"
 		    : "Unable to hash %s", path);
-	printf("%s\n", sha1_to_hex(sha1));
+	printf("%s\n", oid_to_hex(&oid));
 	maybe_flush_or_die(stdout, "hash to stdout");
 }

Signature · 1 line

2.9.5

Patryk Obara

author

2017-08-20 20:09 UTC #2 lore raw

PATCH 2/6 read-cache: convert to struct object_id

In reply to Patryk Obara

Replace hashcmp with oidcmp.

Signed-off-by Patryk Obara <[email protected]>

---
 read-cache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

read-cache.c +3 -3

index acfb028..7285608 100644
@@ -160,9 +160,9 @@ static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
 	int fd = git_open_cloexec(ce->name, O_RDONLY);
 
 	if (fd >= 0) {
-		unsigned char sha1[20];
-		if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
-			match = hashcmp(sha1, ce->oid.hash);
+		struct object_id oid;
+		if (!index_fd(oid.hash, fd, st, OBJ_BLOB, ce->name, 0))
+			match = oidcmp(&oid, &ce->oid);
 		/* index_fd() closed the file descriptor already */
 	}
 	return match;

Signature · 1 line

2.9.5

Patryk Obara

author

2017-08-20 20:09 UTC #3 lore raw

PATCH 3/6 sha1_file: convert index_path to struct object_id

In reply to Patryk Obara

Convert all remaining callers as well.

Signed-off-by Patryk Obara <[email protected]>

---
 builtin/update-index.c |  2 +-
 cache.h                |  2 +-
 diff.c                 |  2 +-
 notes-merge.c          |  2 +-
 read-cache.c           |  2 +-
 sha1_file.c            | 10 +++++-----
 6 files changed, 10 insertions(+), 10 deletions(-)

builtin/update-index.c +1 -1

index 56721cf..d562f2e 100644
@@ -280,7 +280,7 @@ static int add_one_path(const struct cache_entry *old, const char *path, int len
 	fill_stat_cache_info(ce, st);
 	ce->ce_mode = ce_mode_from_stat(old, st->st_mode);
 
-	if (index_path(ce->oid.hash, path, st,
+	if (index_path(&ce->oid, path, st,
 		       info_only ? 0 : HASH_WRITE_OBJECT)) {
 		free(ce);
 		return -1;

cache.h +1 -1

index 1c69d2a..380868d 100644
@@ -685,7 +685,7 @@ extern int ie_modified(const struct index_state *, const struct cache_entry *, s
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
-extern int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned flags);
+extern int index_path(struct object_id *oid, const char *path, struct stat *st, unsigned flags);
 
 /*
  * Record to sd the data from st that we use to check whether a file

diff.c +1 -1

index 9c38258..65f8d13 100644
@@ -3246,7 +3246,7 @@ static void diff_fill_oid_info(struct diff_filespec *one)
 			}
 			if (lstat(one->path, &st) < 0)
 				die_errno("stat '%s'", one->path);
-			if (index_path(one->oid.hash, one->path, &st, 0))
+			if (index_path(&one->oid, one->path, &st, 0))
 				die("cannot hash %s", one->path);
 		}
 	}

notes-merge.c +1 -1

index c12b354..744c685 100644
@@ -709,7 +709,7 @@ int notes_merge_commit(struct notes_merge_options *o,
 		/* write file as blob, and add to partial_tree */
 		if (stat(path.buf, &st))
 			die_errno("Failed to stat '%s'", path.buf);
-		if (index_path(blob_oid.hash, path.buf, &st, HASH_WRITE_OBJECT))
+		if (index_path(&blob_oid, path.buf, &st, HASH_WRITE_OBJECT))
 			die("Failed to write blob object from '%s'", path.buf);
 		if (add_note(partial_tree, &obj_oid, &blob_oid, NULL))
 			die("Failed to add resolved note '%s' to notes tree",

read-cache.c +1 -1

index 7285608..17f19a1 100644
@@ -689,7 +689,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		return 0;
 	}
 	if (!intent_only) {
-		if (index_path(ce->oid.hash, path, st, HASH_WRITE_OBJECT)) {
+		if (index_path(&ce->oid, path, st, HASH_WRITE_OBJECT)) {
 			free(ce);
 			return error("unable to index file %s", path);
 		}

sha1_file.c +5 -5

index b60ae15..6a2a48b 100644
@@ -3686,7 +3686,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
 	return ret;
 }
 
-int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned flags)
+int index_path(struct object_id *oid, const char *path, struct stat *st, unsigned flags)
 {
 	int fd;
 	struct strbuf sb = STRBUF_INIT;

@@ -3696,7 +3696,7 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned
 		fd = open(path, O_RDONLY);
 		if (fd < 0)
 			return error_errno("open(\"%s\")", path);
-		if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
+		if (index_fd(oid->hash, fd, st, OBJ_BLOB, path, flags) < 0)
 			return error("%s: failed to insert into database",
 				     path);
 		break;

@@ -3704,14 +3704,14 @@ int index_path(unsigned char *sha1, const char *path, struct stat *st, unsigned
 		if (strbuf_readlink(&sb, path, st->st_size))
 			return error_errno("readlink(\"%s\")", path);
 		if (!(flags & HASH_WRITE_OBJECT))
-			hash_sha1_file(sb.buf, sb.len, blob_type, sha1);
-		else if (write_sha1_file(sb.buf, sb.len, blob_type, sha1))
+			hash_sha1_file(sb.buf, sb.len, blob_type, oid->hash);
+		else if (write_sha1_file(sb.buf, sb.len, blob_type, oid->hash))
 			return error("%s: failed to insert into database",
 				     path);
 		strbuf_release(&sb);
 		break;
 	case S_IFDIR:
-		return resolve_gitlink_ref(path, "HEAD", sha1);
+		return resolve_gitlink_ref(path, "HEAD", oid->hash);
 	default:
 		return error("%s: unsupported file type", path);
 	}

Signature · 1 line

2.9.5

Patryk Obara

author

2017-08-20 20:10 UTC #4 lore raw

PATCH 4/6 sha1_file: convert index_fd to struct object_id

In reply to Patryk Obara

Convert all remaining callers as well.

Signed-off-by Patryk Obara <[email protected]>

---
 builtin/difftool.c    |  2 +-
 builtin/hash-object.c |  2 +-
 builtin/replace.c     |  2 +-
 cache.h               |  2 +-
 read-cache.c          |  2 +-
 sha1_file.c           | 14 +++++++-------
 6 files changed, 12 insertions(+), 12 deletions(-)

builtin/difftool.c +1 -1

index 8864d84..b2d3ba7 100644
@@ -111,7 +111,7 @@ static int use_wt_file(const char *workdir, const char *name,
 		int fd = open(buf.buf, O_RDONLY);
 
 		if (fd >= 0 &&
-		    !index_fd(wt_oid.hash, fd, &st, OBJ_BLOB, name, 0)) {
+		    !index_fd(&wt_oid, fd, &st, OBJ_BLOB, name, 0)) {
 			if (is_null_oid(oid)) {
 				oidcpy(oid, &wt_oid);
 				use = 1;

builtin/hash-object.c +1 -1

index 1c0f0f3..8a58ce0 100644
@@ -38,7 +38,7 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
 	if (fstat(fd, &st) < 0 ||
 	    (literally
 	     ? hash_literally(&oid, fd, type, flags)
-	     : index_fd(oid.hash, fd, &st, type_from_string(type), path, flags)))
+	     : index_fd(&oid, fd, &st, type_from_string(type), path, flags)))
 		die((flags & HASH_WRITE_OBJECT)
 		    ? "Unable to add %s to database"
 		    : "Unable to hash %s", path);

builtin/replace.c +1 -1

index f4a85a1..3e71a77 100644
@@ -269,7 +269,7 @@ static void import_object(struct object_id *oid, enum object_type type,
 
 		if (fstat(fd, &st) < 0)
 			die_errno("unable to fstat %s", filename);
-		if (index_fd(oid->hash, fd, &st, type, NULL, flags) < 0)
+		if (index_fd(oid, fd, &st, type, NULL, flags) < 0)
 			die("unable to write object to database");
 		/* index_fd close()s fd for us */
 	}

cache.h +1 -1

index 380868d..eaf3603 100644
@@ -684,7 +684,7 @@ extern int ie_modified(const struct index_state *, const struct cache_entry *, s
 
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
-extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
+extern int index_fd(struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
 extern int index_path(struct object_id *oid, const char *path, struct stat *st, unsigned flags);
 
 /*

read-cache.c +1 -1

index 17f19a1..9b41058 100644
@@ -161,7 +161,7 @@ static int ce_compare_data(const struct cache_entry *ce, struct stat *st)
 
 	if (fd >= 0) {
 		struct object_id oid;
-		if (!index_fd(oid.hash, fd, st, OBJ_BLOB, ce->name, 0))
+		if (!index_fd(&oid, fd, st, OBJ_BLOB, ce->name, 0))
 			match = oidcmp(&oid, &ce->oid);
 		/* index_fd() closed the file descriptor already */
 	}

sha1_file.c +7 -7

index 6a2a48b..11995e5 100644
@@ -3662,8 +3662,8 @@ static int index_stream(unsigned char *sha1, int fd, size_t size,
 	return index_bulk_checkin(sha1, fd, size, type, path, flags);
 }
 
-int index_fd(unsigned char *sha1, int fd, struct stat *st,
-	     enum object_type type, const char *path, unsigned flags)
+int index_fd(struct object_id *oid, int fd, struct stat *st,
+             enum object_type type, const char *path, unsigned flags)
 {
 	int ret;

@@ -3672,15 +3672,15 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
 	 * die() for large files.
 	 */
 	if (type == OBJ_BLOB && path && would_convert_to_git_filter_fd(path))
-		ret = index_stream_convert_blob(sha1, fd, path, flags);
+		ret = index_stream_convert_blob(oid->hash, fd, path, flags);
 	else if (!S_ISREG(st->st_mode))
-		ret = index_pipe(sha1, fd, type, path, flags);
+		ret = index_pipe(oid->hash, fd, type, path, flags);
 	else if (st->st_size <= big_file_threshold || type != OBJ_BLOB ||
 		 (path && would_convert_to_git(&the_index, path)))
-		ret = index_core(sha1, fd, xsize_t(st->st_size), type, path,
+		ret = index_core(oid->hash, fd, xsize_t(st->st_size), type, path,
 				 flags);
 	else
-		ret = index_stream(sha1, fd, xsize_t(st->st_size), type, path,
+		ret = index_stream(oid->hash, fd, xsize_t(st->st_size), type, path,
 				   flags);
 	close(fd);
 	return ret;

@@ -3696,7 +3696,7 @@ int index_path(struct object_id *oid, const char *path, struct stat *st, unsigne
 		fd = open(path, O_RDONLY);
 		if (fd < 0)
 			return error_errno("open(\"%s\")", path);
-		if (index_fd(oid->hash, fd, st, OBJ_BLOB, path, flags) < 0)
+		if (index_fd(oid, fd, st, OBJ_BLOB, path, flags) < 0)
 			return error("%s: failed to insert into database",
 				     path);
 		break;

Signature · 1 line

2.9.5

Patryk Obara

author

2017-08-20 20:09 UTC #5 lore raw

PATCH 5/6 sha1_file: convert hash_sha1_file_literally to struct object_id

In reply to Patryk Obara

Convert all remaining callers as well.

Signed-off-by Patryk Obara <[email protected]>

---
 builtin/hash-object.c | 2 +-
 cache.h               | 2 +-
 sha1_file.c           | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

builtin/hash-object.c +1 -1

index 8a58ce0..c532ff9 100644
@@ -24,7 +24,7 @@ static int hash_literally(struct object_id *oid, int fd, const char *type, unsig
 	if (strbuf_read(&buf, fd, 4096) < 0)
 		ret = -1;
 	else
-		ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid->hash, flags);
+		ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid, flags);
 	strbuf_release(&buf);
 	return ret;
 }

cache.h +1 -1

index eaf3603..237adb5 100644
@@ -1199,7 +1199,7 @@ static inline const unsigned char *lookup_replace_object(const unsigned char *sh
 extern int sha1_object_info(const unsigned char *, unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags);
+extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, struct object_id *oid, unsigned flags);
 extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);

sha1_file.c +4 -4

index 11995e5..3e2ef4e 100644
@@ -3437,7 +3437,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign
 }
 
 int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type,
-			     unsigned char *sha1, unsigned flags)
+                             struct object_id *oid, unsigned flags)
 {
 	char *header;
 	int hdrlen, status = 0;

@@ -3445,13 +3445,13 @@ int hash_sha1_file_literally(const void *buf, unsigned long len, const char *typ
 	/* type string, SP, %lu of the length plus NUL must fit this */
 	hdrlen = strlen(type) + 32;
 	header = xmalloc(hdrlen);
-	write_sha1_file_prepare(buf, len, type, sha1, header, &hdrlen);
+	write_sha1_file_prepare(buf, len, type, oid->hash, header, &hdrlen);
 
 	if (!(flags & HASH_WRITE_OBJECT))
 		goto cleanup;
-	if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
+	if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
 		goto cleanup;
-	status = write_loose_object(sha1, header, hdrlen, buf, len, 0);
+	status = write_loose_object(oid->hash, header, hdrlen, buf, len, 0);
 
 cleanup:
 	free(header);

Signature · 1 line

2.9.5

brian m. carlson

2017-08-20 20:37 UTC #6 lore raw

Re: [PATCH 5/6] sha1_file: convert hash_sha1_file_literally to struct object_id

Quoted 27 lines · Patryk Obara

Convert all remaining callers as well.

Signed-off-by Patryk Obara <[email protected]>

---
 builtin/hash-object.c | 2 +-
 cache.h               | 2 +-
 sha1_file.c           | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

builtin/hash-object.c +1 -1

index 8a58ce0..c532ff9 100644
@@ -24,7 +24,7 @@ static int hash_literally(struct object_id *oid, int fd, const char *type, unsig
 	if (strbuf_read(&buf, fd, 4096) < 0)
 		ret = -1;
 	else
-		ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid->hash, flags);
+		ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid, flags);
 	strbuf_release(&buf);
 	return ret;
 }

cache.h +0 -0

index eaf3603..237adb5 100644

cache.h static inline const unsigned char *lookup_replace_object(const unsigned char *sh · line 1202

Quoted 5 lines

cache.h +1 -1

@@ -1199,7 +1199,7 @@ static inline const unsigned char *lookup_replace_object(const unsigned char *sh
 extern int sha1_object_info(const unsigned char *, unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1);
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags);
+extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, struct object_id *oid, unsigned flags);

We probably want to rename this function, since it no longer handles exclusively SHA-1. When I've made changes to the "_sha1_file" functions, I've converted them to "_object_file" instead. However, if people like "_oid_file", we could do that instead.

Signature · 3 lines

brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

Patryk Obara

author

2017-08-20 20:09 UTC #7 lore raw

PATCH 6/6 sha1_file: convert index_stream to struct object_id

In reply to Patryk Obara

Signed-off-by Patryk Obara <[email protected]>

---
 sha1_file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

sha1_file.c +3 -3

index 3e2ef4e..8d6960a 100644
@@ -3655,11 +3655,11 @@ static int index_core(unsigned char *sha1, int fd, size_t size,
  * binary blobs, they generally do not want to get any conversion, and
  * callers should avoid this code path when filters are requested.
  */
-static int index_stream(unsigned char *sha1, int fd, size_t size,
+static int index_stream(struct object_id *oid, int fd, size_t size,
 			enum object_type type, const char *path,
 			unsigned flags)
 {
-	return index_bulk_checkin(sha1, fd, size, type, path, flags);
+	return index_bulk_checkin(oid->hash, fd, size, type, path, flags);
 }
 
 int index_fd(struct object_id *oid, int fd, struct stat *st,

@@ -3680,7 +3680,7 @@ int index_fd(struct object_id *oid, int fd, struct stat *st,
 		ret = index_core(oid->hash, fd, xsize_t(st->st_size), type, path,
 				 flags);
 	else
-		ret = index_stream(oid->hash, fd, xsize_t(st->st_size), type, path,
+		ret = index_stream(oid, fd, xsize_t(st->st_size), type, path,
 				   flags);
 	close(fd);
 	return ret;

Signature · 1 line

2.9.5

brian m. carlson

2017-08-20 20:25 UTC #8 lore raw

In reply to Patryk Obara

Quoted 7 lines · Patryk Obara

This enabled conversion of few functions in sha1_file, which had almost all callers converted already.
I hope I'm not stepping on anyone's toes with this patch series. If I do - is there some email thread or document in which I can coordinate with other developers, regarding which code regions are being converted to struct object_id next?

We don't have a coordinated thread at the moment. You can see what I'm working on at https://github.com/bk2204/git.git in the object-id-part10 and object-id-part11 branches (based on an older next).

However, having said that, I don't mind if you or others pick up various parts of the codebase. At worst, I drop a few patches for things others have already converted.

Signature · 3 lines

brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

Junio C Hamano

2017-08-21 05:00 UTC #9 lore raw

Quoted 9 lines · brian m. carlson

Quoted 7 lines · Patryk Obara
This enabled conversion of few functions in sha1_file, which had almost all callers converted already.
I hope I'm not stepping on anyone's toes with this patch series. If I do - is there some email thread or document in which I can coordinate with other developers, regarding which code regions are being converted to struct object_id next?

Quoted 7 lines

We don't have a coordinated thread at the moment. You can see what I'm working on at https://github.com/bk2204/git.git in the object-id-part10 and object-id-part11 branches (based on an older next).
However, having said that, I don't mind if you or others pick up various parts of the codebase. At worst, I drop a few patches for things others have already converted.

Thanks for working well together ;-)

I've scanned these patches and they looked OK. There still are places that dereferences oid->hash when making a call instead of passing a pointer to the whole oid, but that is not making things worse.

As to the naming (your comments on 5/6), I agree that we would need to switch s/sha1/oid/ in the names in the endgame. It may be OK to leave it to later rounds when we do use something like your hash algorithm abstraction throughout the codebase.