Git development
 help / color / mirror / Atom feed
* [PATCH 12/17] reflog-walk: convert struct reflog_info to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Convert struct reflog_info to use struct object_id by changing the
structure definition and applying the following semantic patch:

@@
struct reflog_info E1;
@@
- E1.osha1
+ E1.ooid.hash

@@
struct reflog_info *E1;
@@
- E1->osha1
+ E1->ooid.hash

@@
struct reflog_info E1;
@@
- E1.nsha1
+ E1.noid.hash

@@
struct reflog_info *E1;
@@
- E1->nsha1
+ E1->noid.hash

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 reflog-walk.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index f98748e2a..fe5be4147 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -10,7 +10,7 @@ struct complete_reflogs {
 	char *ref;
 	const char *short_ref;
 	struct reflog_info {
-		unsigned char osha1[20], nsha1[20];
+		struct object_id ooid, noid;
 		char *email;
 		unsigned long timestamp;
 		int tz;
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 
 	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
-	hashcpy(item->osha1, osha1);
-	hashcpy(item->nsha1, nsha1);
+	hashcpy(item->ooid.hash, osha1);
+	hashcpy(item->noid.hash, nsha1);
 	item->email = xstrdup(email);
 	item->timestamp = timestamp;
 	item->tz = tz;
@@ -238,13 +238,13 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	do {
 		reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 		commit_reflog->recno--;
-		logobj = parse_object(reflog->osha1);
+		logobj = parse_object(reflog->ooid.hash);
 	} while (commit_reflog->recno && (logobj && logobj->type != OBJ_COMMIT));
 
-	if (!logobj && commit_reflog->recno >= 0 && is_null_sha1(reflog->osha1)) {
+	if (!logobj && commit_reflog->recno >= 0 && is_null_sha1(reflog->ooid.hash)) {
 		/* a root commit, but there are still more entries to show */
 		reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
-		logobj = parse_object(reflog->nsha1);
+		logobj = parse_object(reflog->noid.hash);
 	}
 
 	if (!logobj || logobj->type != OBJ_COMMIT) {
-- 
2.11.0


^ permalink raw reply related

* [PATCH 10/17] Convert remaining callers of resolve_refdup to object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

There are a few leaf functions in various files that call
resolve_refdup.  Convert these functions to use struct object_id
internally to prepare for transitioning resolve_refdup itself.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/notes.c        | 18 +++++++++---------
 builtin/receive-pack.c |  4 ++--
 ref-filter.c           |  4 ++--
 reflog-walk.c          | 12 ++++++------
 transport.c            |  4 ++--
 wt-status.c            |  4 ++--
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 5248a9bad..8c569a49a 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -693,7 +693,7 @@ static int merge_abort(struct notes_merge_options *o)
 static int merge_commit(struct notes_merge_options *o)
 {
 	struct strbuf msg = STRBUF_INIT;
-	unsigned char sha1[20], parent_sha1[20];
+	struct object_id oid, parent_oid;
 	struct notes_tree *t;
 	struct commit *partial;
 	struct pretty_print_context pretty_ctx;
@@ -705,27 +705,27 @@ static int merge_commit(struct notes_merge_options *o)
 	 * and target notes ref from .git/NOTES_MERGE_REF.
 	 */
 
-	if (get_sha1("NOTES_MERGE_PARTIAL", sha1))
+	if (get_oid("NOTES_MERGE_PARTIAL", &oid))
 		die(_("failed to read ref NOTES_MERGE_PARTIAL"));
-	else if (!(partial = lookup_commit_reference(sha1)))
+	else if (!(partial = lookup_commit_reference(oid.hash)))
 		die(_("could not find commit from NOTES_MERGE_PARTIAL."));
 	else if (parse_commit(partial))
 		die(_("could not parse commit from NOTES_MERGE_PARTIAL."));
 
 	if (partial->parents)
-		hashcpy(parent_sha1, partial->parents->item->object.oid.hash);
+		oidcpy(&parent_oid, &partial->parents->item->object.oid);
 	else
-		hashclr(parent_sha1);
+		oidclr(&parent_oid);
 
 	t = xcalloc(1, sizeof(struct notes_tree));
 	init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
 
 	o->local_ref = local_ref_to_free =
-		resolve_refdup("NOTES_MERGE_REF", 0, sha1, NULL);
+		resolve_refdup("NOTES_MERGE_REF", 0, oid.hash, NULL);
 	if (!o->local_ref)
 		die(_("failed to resolve NOTES_MERGE_REF"));
 
-	if (notes_merge_commit(o, t, partial, sha1))
+	if (notes_merge_commit(o, t, partial, oid.hash))
 		die(_("failed to finalize notes merge"));
 
 	/* Reuse existing commit message in reflog message */
@@ -733,8 +733,8 @@ static int merge_commit(struct notes_merge_options *o)
 	format_commit_message(partial, "%s", &msg, &pretty_ctx);
 	strbuf_trim(&msg);
 	strbuf_insert(&msg, 0, "notes: ", 7);
-	update_ref(msg.buf, o->local_ref, sha1,
-		   is_null_sha1(parent_sha1) ? NULL : parent_sha1,
+	update_ref(msg.buf, o->local_ref, oid.hash,
+		   is_null_oid(&parent_oid) ? NULL : parent_oid.hash,
 		   0, UPDATE_REFS_DIE_ON_ERR);
 
 	free_notes(t);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6b97cbdbe..b24644242 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1414,7 +1414,7 @@ static void execute_commands(struct command *commands,
 {
 	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	struct command *cmd;
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct iterate_data data;
 	struct async muxer;
 	int err_fd = 0;
@@ -1471,7 +1471,7 @@ static void execute_commands(struct command *commands,
 	check_aliased_updates(commands);
 
 	free(head_name_to_free);
-	head_name = head_name_to_free = resolve_refdup("HEAD", 0, sha1, NULL);
+	head_name = head_name_to_free = resolve_refdup("HEAD", 0, oid.hash, NULL);
 
 	if (use_atomic)
 		execute_commands_atomic(commands, si);
diff --git a/ref-filter.c b/ref-filter.c
index 1a978405e..a759bf67d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -961,9 +961,9 @@ static void populate_value(struct ref_array_item *ref)
 	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
 	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
-		unsigned char unused1[20];
+		struct object_id unused1;
 		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
-					     unused1, NULL);
+					     unused1.hash, NULL);
 		if (!ref->symref)
 			ref->symref = "";
 	}
diff --git a/reflog-walk.c b/reflog-walk.c
index a246af276..f98748e2a 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -45,11 +45,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 	reflogs->ref = xstrdup(ref);
 	for_each_reflog_ent(ref, read_one_reflog, reflogs);
 	if (reflogs->nr == 0) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		const char *name;
 		void *name_to_free;
 		name = name_to_free = resolve_refdup(ref, RESOLVE_REF_READING,
-						     sha1, NULL);
+						     oid.hash, NULL);
 		if (name) {
 			for_each_reflog_ent(name, read_one_reflog, reflogs);
 			free(name_to_free);
@@ -172,18 +172,18 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
 		reflogs = item->util;
 	else {
 		if (*branch == '\0') {
-			unsigned char sha1[20];
+			struct object_id oid;
 			free(branch);
-			branch = resolve_refdup("HEAD", 0, sha1, NULL);
+			branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 			if (!branch)
 				die ("No current branch");
 
 		}
 		reflogs = read_complete_reflog(branch);
 		if (!reflogs || reflogs->nr == 0) {
-			unsigned char sha1[20];
+			struct object_id oid;
 			char *b;
-			if (dwim_log(branch, strlen(branch), sha1, &b) == 1) {
+			if (dwim_log(branch, strlen(branch), oid.hash, &b) == 1) {
 				if (reflogs) {
 					free(reflogs->ref);
 					free(reflogs);
diff --git a/transport.c b/transport.c
index 04e5d6623..be217609f 100644
--- a/transport.c
+++ b/transport.c
@@ -467,11 +467,11 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 {
 	struct ref *ref;
 	int n = 0;
-	unsigned char head_sha1[20];
+	struct object_id head_oid;
 	char *head;
 	int summary_width = transport_summary_width(refs);
 
-	head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL);
 
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
diff --git a/wt-status.c b/wt-status.c
index a715e7190..f0d750880 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -121,7 +121,7 @@ static void status_printf_more(struct wt_status *s, const char *color,
 
 void wt_status_prepare(struct wt_status *s)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	memset(s, 0, sizeof(*s));
 	memcpy(s->color_palette, default_wt_status_colors,
@@ -129,7 +129,7 @@ void wt_status_prepare(struct wt_status *s)
 	s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
 	s->use_color = -1;
 	s->relative_paths = 1;
-	s->branch = resolve_refdup("HEAD", 0, sha1, NULL);
+	s->branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 	s->reference = "HEAD";
 	s->fp = stdout;
 	s->index_file = get_index_file();
-- 
2.11.0


^ permalink raw reply related

* [PATCH 13/17] refs: convert each_reflog_ent_fn to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Make each_reflog_ent_fn take two struct object_id pointers instead of
two pointers to unsigned char.  Convert the various callbacks to use
struct object_id as well.  Also, rename fsck_handle_reflog_sha1 to
fsck_handle_reflog_oid.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/fsck.c       | 16 ++++++++--------
 builtin/merge-base.c |  6 +++---
 builtin/reflog.c     |  2 +-
 reflog-walk.c        |  6 +++---
 refs.c               | 24 ++++++++++++------------
 refs.h               |  2 +-
 refs/files-backend.c | 24 ++++++++++++------------
 revision.c           | 12 ++++++------
 sha1_name.c          |  2 +-
 wt-status.c          |  6 +++---
 10 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index f01b81eeb..8d6e98399 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -393,13 +393,13 @@ static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
 
 static int default_refs;
 
-static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
+static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,
 	unsigned long timestamp)
 {
 	struct object *obj;
 
-	if (!is_null_sha1(sha1)) {
-		obj = lookup_object(sha1);
+	if (!is_null_oid(oid)) {
+		obj = lookup_object(oid->hash);
 		if (obj) {
 			if (timestamp && name_objects)
 				add_decoration(fsck_walk_options.object_names,
@@ -408,13 +408,13 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
 			obj->used = 1;
 			mark_object_reachable(obj);
 		} else {
-			error("%s: invalid reflog entry %s", refname, sha1_to_hex(sha1));
+			error("%s: invalid reflog entry %s", refname, oid_to_hex(oid));
 			errors_found |= ERROR_REACHABLE;
 		}
 	}
 }
 
-static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -422,10 +422,10 @@ static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 
 	if (verbose)
 		fprintf(stderr, "Checking reflog %s->%s\n",
-			sha1_to_hex(osha1), sha1_to_hex(nsha1));
+			oid_to_hex(ooid), oid_to_hex(noid));
 
-	fsck_handle_reflog_sha1(refname, osha1, 0);
-	fsck_handle_reflog_sha1(refname, nsha1, timestamp);
+	fsck_handle_reflog_oid(refname, ooid, 0);
+	fsck_handle_reflog_oid(refname, noid, timestamp);
 	return 0;
 }
 
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index b572a37c2..db95bc29c 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -131,7 +131,7 @@ static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
 	commit->object.flags |= TMP_MARK;
 }
 
-static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int collect_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 				  const char *ident, unsigned long timestamp,
 				  int tz, const char *message, void *cbdata)
 {
@@ -139,9 +139,9 @@ static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 
 	if (revs->initial) {
 		revs->initial = 0;
-		add_one_commit(osha1, revs);
+		add_one_commit(ooid->hash, revs);
 	}
-	add_one_commit(nsha1, revs);
+	add_one_commit(noid->hash, revs);
 	return 0;
 }
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7a7136e53..747277577 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -615,7 +615,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
-static int count_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
diff --git a/reflog-walk.c b/reflog-walk.c
index fe5be4147..99679f582 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -19,7 +19,7 @@ struct complete_reflogs {
 	int nr, alloc;
 };
 
-static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
+static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 
 	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
-	hashcpy(item->ooid.hash, osha1);
-	hashcpy(item->noid.hash, nsha1);
+	oidcpy(&item->ooid, ooid);
+	oidcpy(&item->noid, noid);
 	item->email = xstrdup(email);
 	item->timestamp = timestamp;
 	item->tz = tz;
diff --git a/refs.c b/refs.c
index 9bd0bc177..4bc924790 100644
--- a/refs.c
+++ b/refs.c
@@ -669,7 +669,7 @@ struct read_ref_at_cb {
 	int *cutoff_cnt;
 };
 
-static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -693,30 +693,30 @@ static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
 		 * hold the values for the previous record.
 		 */
 		if (!is_null_sha1(cb->osha1)) {
-			hashcpy(cb->sha1, nsha1);
-			if (hashcmp(cb->osha1, nsha1))
+			hashcpy(cb->sha1, noid->hash);
+			if (hashcmp(cb->osha1, noid->hash))
 				warning("Log for ref %s has gap after %s.",
 					cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));
 		}
 		else if (cb->date == cb->at_time)
-			hashcpy(cb->sha1, nsha1);
-		else if (hashcmp(nsha1, cb->sha1))
+			hashcpy(cb->sha1, noid->hash);
+		else if (hashcmp(noid->hash, cb->sha1))
 			warning("Log for ref %s unexpectedly ended on %s.",
 				cb->refname, show_date(cb->date, cb->tz,
 						       DATE_MODE(RFC2822)));
-		hashcpy(cb->osha1, osha1);
-		hashcpy(cb->nsha1, nsha1);
+		hashcpy(cb->osha1, ooid->hash);
+		hashcpy(cb->nsha1, noid->hash);
 		cb->found_it = 1;
 		return 1;
 	}
-	hashcpy(cb->osha1, osha1);
-	hashcpy(cb->nsha1, nsha1);
+	hashcpy(cb->osha1, ooid->hash);
+	hashcpy(cb->nsha1, noid->hash);
 	if (cb->cnt > 0)
 		cb->cnt--;
 	return 0;
 }
 
-static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,
 				  const char *email, unsigned long timestamp,
 				  int tz, const char *message, void *cb_data)
 {
@@ -730,9 +730,9 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
 		*cb->cutoff_tz = tz;
 	if (cb->cutoff_cnt)
 		*cb->cutoff_cnt = cb->reccnt;
-	hashcpy(cb->sha1, osha1);
+	hashcpy(cb->sha1, ooid->hash);
 	if (is_null_sha1(cb->sha1))
-		hashcpy(cb->sha1, nsha1);
+		hashcpy(cb->sha1, noid->hash);
 	/* We just want the first entry */
 	return 1;
 }
diff --git a/refs.h b/refs.h
index 694784391..ce35c4019 100644
--- a/refs.h
+++ b/refs.h
@@ -290,7 +290,7 @@ int delete_reflog(const char *refname);
 
 /* iterate over reflog entries */
 typedef int each_reflog_ent_fn(
-		unsigned char *old_sha1, unsigned char *new_sha1,
+		struct object_id *old_oid, struct object_id *new_oid,
 		const char *committer, unsigned long timestamp,
 		int tz, const char *msg, void *cb_data);
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f9023939d..3da3141ee 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3113,15 +3113,15 @@ static int files_delete_reflog(struct ref_store *ref_store,
 
 static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
 {
-	unsigned char osha1[20], nsha1[20];
+	struct object_id ooid, noid;
 	char *email_end, *message;
 	unsigned long timestamp;
 	int tz;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
 	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
-	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
-	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
+	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
+	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
 	    !(email_end = strchr(sb->buf + 82, '>')) ||
 	    email_end[1] != ' ' ||
 	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
@@ -3136,7 +3136,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 		message += 6;
 	else
 		message += 7;
-	return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data);
+	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);
 }
 
 static char *find_beginning_of_line(char *bob, char *scan)
@@ -3936,10 +3936,10 @@ struct expire_reflog_cb {
 	reflog_expiry_should_prune_fn *should_prune_fn;
 	void *policy_cb;
 	FILE *newlog;
-	unsigned char last_kept_sha1[20];
+	struct object_id last_kept_oid;
 };
 
-static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 			     const char *email, unsigned long timestamp, int tz,
 			     const char *message, void *cb_data)
 {
@@ -3947,9 +3947,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	struct expire_reflog_policy_cb *policy_cb = cb->policy_cb;
 
 	if (cb->flags & EXPIRE_REFLOGS_REWRITE)
-		osha1 = cb->last_kept_sha1;
+		ooid = &cb->last_kept_oid;
 
-	if ((*cb->should_prune_fn)(osha1, nsha1, email, timestamp, tz,
+	if ((*cb->should_prune_fn)(ooid->hash, noid->hash, email, timestamp, tz,
 				   message, policy_cb)) {
 		if (!cb->newlog)
 			printf("would prune %s", message);
@@ -3958,9 +3958,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	} else {
 		if (cb->newlog) {
 			fprintf(cb->newlog, "%s %s %s %lu %+05d\t%s",
-				sha1_to_hex(osha1), sha1_to_hex(nsha1),
+				oid_to_hex(ooid), oid_to_hex(noid),
 				email, timestamp, tz, message);
-			hashcpy(cb->last_kept_sha1, nsha1);
+			oidcpy(&cb->last_kept_oid, noid);
 		}
 		if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
 			printf("keep %s", message);
@@ -4047,14 +4047,14 @@ static int files_reflog_expire(struct ref_store *ref_store,
 		 */
 		int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
 			!(type & REF_ISSYMREF) &&
-			!is_null_sha1(cb.last_kept_sha1);
+			!is_null_oid(&cb.last_kept_oid);
 
 		if (close_lock_file(&reflog_lock)) {
 			status |= error("couldn't write %s: %s", log_file,
 					strerror(errno));
 		} else if (update &&
 			   (write_in_full(get_lock_file_fd(lock->lk),
-				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
+				oid_to_hex(&cb.last_kept_oid), 40) != 40 ||
 			    write_str_in_full(get_lock_file_fd(lock->lk), "\n") != 1 ||
 			    close_ref(lock) < 0)) {
 			status |= error("couldn't write %s",
diff --git a/revision.c b/revision.c
index b37dbec37..d9fe73318 100644
--- a/revision.c
+++ b/revision.c
@@ -1196,11 +1196,11 @@ static void handle_refs(const char *submodule, struct rev_info *revs, unsigned f
 	for_each(submodule, handle_one_ref, &cb);
 }
 
-static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
+static void handle_one_reflog_commit(struct object_id *oid, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
-	if (!is_null_sha1(sha1)) {
-		struct object *o = parse_object(sha1);
+	if (!is_null_oid(oid)) {
+		struct object *o = parse_object(oid->hash);
 		if (o) {
 			o->flags |= cb->all_flags;
 			/* ??? CMDLINEFLAGS ??? */
@@ -1214,12 +1214,12 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 	}
 }
 
-static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
-	handle_one_reflog_commit(osha1, cb_data);
-	handle_one_reflog_commit(nsha1, cb_data);
+	handle_one_reflog_commit(ooid, cb_data);
+	handle_one_reflog_commit(noid, cb_data);
 	return 0;
 }
 
diff --git a/sha1_name.c b/sha1_name.c
index 73a915ff1..744e9f884 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1051,7 +1051,7 @@ struct grab_nth_branch_switch_cbdata {
 	struct strbuf buf;
 };
 
-static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+static int grab_nth_branch_switch(struct object_id *ooid, struct object_id *noid,
 				  const char *email, unsigned long timestamp, int tz,
 				  const char *message, void *cb_data)
 {
diff --git a/wt-status.c b/wt-status.c
index f0d750880..08a4d0bd3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1367,7 +1367,7 @@ struct grab_1st_switch_cbdata {
 	unsigned char nsha1[20];
 };
 
-static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
 			   const char *email, unsigned long timestamp, int tz,
 			   const char *message, void *cb_data)
 {
@@ -1381,13 +1381,13 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
 		return 0;
 	target += strlen(" to ");
 	strbuf_reset(&cb->buf);
-	hashcpy(cb->nsha1, nsha1);
+	hashcpy(cb->nsha1, noid->hash);
 	end = strchrnul(target, '\n');
 	strbuf_add(&cb->buf, target, end - target);
 	if (!strcmp(cb->buf.buf, "HEAD")) {
 		/* HEAD is relative. Resolve it to the right reflog entry. */
 		strbuf_reset(&cb->buf);
-		strbuf_add_unique_abbrev(&cb->buf, nsha1, DEFAULT_ABBREV);
+		strbuf_add_unique_abbrev(&cb->buf, noid->hash, DEFAULT_ABBREV);
 	}
 	return 1;
 }
-- 
2.11.0


^ permalink raw reply related

* [PATCH 15/17] Convert object iteration callbacks to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Convert each_loose_object_fn and each_packed_object_fn to take a pointer
to struct object_id.  Update the various callbacks.  Convert several
40-based constants to use GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/cat-file.c      |  8 ++++----
 builtin/count-objects.c |  4 ++--
 builtin/fsck.c          | 10 +++++-----
 builtin/pack-objects.c  |  6 +++---
 builtin/prune-packed.c  |  4 ++--
 builtin/prune.c         |  8 ++++----
 cache.h                 |  4 ++--
 reachable.c             | 30 +++++++++++++++---------------
 sha1_file.c             | 12 ++++++------
 9 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 30383e9eb..8b85cb8cf 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -409,20 +409,20 @@ static int batch_object_cb(const unsigned char sha1[20], void *vdata)
 	return 0;
 }
 
-static int batch_loose_object(const unsigned char *sha1,
+static int batch_loose_object(const struct object_id *oid,
 			      const char *path,
 			      void *data)
 {
-	sha1_array_append(data, sha1);
+	sha1_array_append(data, oid->hash);
 	return 0;
 }
 
-static int batch_packed_object(const unsigned char *sha1,
+static int batch_packed_object(const struct object_id *oid,
 			       struct packed_git *pack,
 			       uint32_t pos,
 			       void *data)
 {
-	sha1_array_append(data, sha1);
+	sha1_array_append(data, oid->hash);
 	return 0;
 }
 
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a04b4f2ef..acb05940f 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -53,7 +53,7 @@ static void loose_garbage(const char *path)
 		report_garbage(PACKDIR_FILE_GARBAGE, path);
 }
 
-static int count_loose(const unsigned char *sha1, const char *path, void *data)
+static int count_loose(const struct object_id *oid, const char *path, void *data)
 {
 	struct stat st;
 
@@ -62,7 +62,7 @@ static int count_loose(const unsigned char *sha1, const char *path, void *data)
 	else {
 		loose_size += on_disk_bytes(st);
 		loose++;
-		if (verbose && has_sha1_pack(sha1))
+		if (verbose && has_sha1_pack(oid->hash))
 			packed_loose++;
 	}
 	return 0;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8d6e98399..90da0928a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -362,13 +362,13 @@ static int fsck_obj(struct object *obj)
 	return 0;
 }
 
-static int fsck_sha1(const unsigned char *sha1)
+static int fsck_oid(const struct object_id *oid)
 {
-	struct object *obj = parse_object(sha1);
+	struct object *obj = parse_object(oid->hash);
 	if (!obj) {
 		errors_found |= ERROR_OBJECT;
 		return error("%s: object corrupt or missing",
-			     sha1_to_hex(sha1));
+			     oid_to_hex(oid));
 	}
 	obj->flags |= HAS_OBJ;
 	return fsck_obj(obj);
@@ -488,9 +488,9 @@ static void get_default_heads(void)
 	}
 }
 
-static int fsck_loose(const unsigned char *sha1, const char *path, void *data)
+static int fsck_loose(const struct object_id *oid, const char *path, void *data)
 {
-	if (fsck_sha1(sha1))
+	if (fsck_oid(oid))
 		errors_found |= ERROR_OBJECT;
 	return 0;
 }
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0fd52bd6b..ef5b8e66d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2546,17 +2546,17 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
 	free(in_pack.array);
 }
 
-static int add_loose_object(const unsigned char *sha1, const char *path,
+static int add_loose_object(const struct object_id *oid, const char *path,
 			    void *data)
 {
-	enum object_type type = sha1_object_info(sha1, NULL);
+	enum object_type type = sha1_object_info(oid->hash, NULL);
 
 	if (type < 0) {
 		warning("loose object at %s could not be examined", path);
 		return 0;
 	}
 
-	add_object_entry(sha1, type, "", 0);
+	add_object_entry(oid->hash, type, "", 0);
 	return 0;
 }
 
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index 7cf900ea0..c026299e7 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -19,12 +19,12 @@ static int prune_subdir(int nr, const char *path, void *data)
 	return 0;
 }
 
-static int prune_object(const unsigned char *sha1, const char *path,
+static int prune_object(const struct object_id *oid, const char *path,
 			 void *data)
 {
 	int *opts = data;
 
-	if (!has_sha1_pack(sha1))
+	if (!has_sha1_pack(oid->hash))
 		return 0;
 
 	if (*opts & PRUNE_PACKED_DRY_RUN)
diff --git a/builtin/prune.c b/builtin/prune.c
index 8f4f05228..42633e0c6 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -30,7 +30,7 @@ static int prune_tmp_file(const char *fullpath)
 	return 0;
 }
 
-static int prune_object(const unsigned char *sha1, const char *fullpath,
+static int prune_object(const struct object_id *oid, const char *fullpath,
 			void *data)
 {
 	struct stat st;
@@ -39,7 +39,7 @@ static int prune_object(const unsigned char *sha1, const char *fullpath,
 	 * Do we know about this object?
 	 * It must have been reachable
 	 */
-	if (lookup_object(sha1))
+	if (lookup_object(oid->hash))
 		return 0;
 
 	if (lstat(fullpath, &st)) {
@@ -50,8 +50,8 @@ static int prune_object(const unsigned char *sha1, const char *fullpath,
 	if (st.st_mtime > expire)
 		return 0;
 	if (show_only || verbose) {
-		enum object_type type = sha1_object_info(sha1, NULL);
-		printf("%s %s\n", sha1_to_hex(sha1),
+		enum object_type type = sha1_object_info(oid->hash, NULL);
+		printf("%s %s\n", oid_to_hex(oid),
 		       (type > 0) ? typename(type) : "unknown");
 	}
 	if (!show_only)
diff --git a/cache.h b/cache.h
index b1ed1c7ae..8c01a8ea2 100644
--- a/cache.h
+++ b/cache.h
@@ -1582,7 +1582,7 @@ extern int unpack_object_header(struct packed_git *, struct pack_window **, off_
  * scratch buffer, but restored to its original contents before
  * the function returns.
  */
-typedef int each_loose_object_fn(const unsigned char *sha1,
+typedef int each_loose_object_fn(const struct object_id *oid,
 				 const char *path,
 				 void *data);
 typedef int each_loose_cruft_fn(const char *basename,
@@ -1608,7 +1608,7 @@ int for_each_loose_file_in_objdir_buf(struct strbuf *path,
  * LOCAL_ONLY flag is set).
  */
 #define FOR_EACH_OBJECT_LOCAL_ONLY 0x1
-typedef int each_packed_object_fn(const unsigned char *sha1,
+typedef int each_packed_object_fn(const struct object_id *oid,
 				  struct packed_git *pack,
 				  uint32_t pos,
 				  void *data);
diff --git a/reachable.c b/reachable.c
index d0199cace..a8a979bd4 100644
--- a/reachable.c
+++ b/reachable.c
@@ -58,7 +58,7 @@ struct recent_data {
 	unsigned long timestamp;
 };
 
-static void add_recent_object(const unsigned char *sha1,
+static void add_recent_object(const struct object_id *oid,
 			      unsigned long mtime,
 			      struct recent_data *data)
 {
@@ -75,37 +75,37 @@ static void add_recent_object(const unsigned char *sha1,
 	 * later processing, and the revision machinery expects
 	 * commits and tags to have been parsed.
 	 */
-	type = sha1_object_info(sha1, NULL);
+	type = sha1_object_info(oid->hash, NULL);
 	if (type < 0)
-		die("unable to get object info for %s", sha1_to_hex(sha1));
+		die("unable to get object info for %s", oid_to_hex(oid));
 
 	switch (type) {
 	case OBJ_TAG:
 	case OBJ_COMMIT:
-		obj = parse_object_or_die(sha1, NULL);
+		obj = parse_object_or_die(oid->hash, NULL);
 		break;
 	case OBJ_TREE:
-		obj = (struct object *)lookup_tree(sha1);
+		obj = (struct object *)lookup_tree(oid->hash);
 		break;
 	case OBJ_BLOB:
-		obj = (struct object *)lookup_blob(sha1);
+		obj = (struct object *)lookup_blob(oid->hash);
 		break;
 	default:
 		die("unknown object type for %s: %s",
-		    sha1_to_hex(sha1), typename(type));
+		    oid_to_hex(oid), typename(type));
 	}
 
 	if (!obj)
-		die("unable to lookup %s", sha1_to_hex(sha1));
+		die("unable to lookup %s", oid_to_hex(oid));
 
 	add_pending_object(data->revs, obj, "");
 }
 
-static int add_recent_loose(const unsigned char *sha1,
+static int add_recent_loose(const struct object_id *oid,
 			    const char *path, void *data)
 {
 	struct stat st;
-	struct object *obj = lookup_object(sha1);
+	struct object *obj = lookup_object(oid->hash);
 
 	if (obj && obj->flags & SEEN)
 		return 0;
@@ -119,22 +119,22 @@ static int add_recent_loose(const unsigned char *sha1,
 		 */
 		if (errno == ENOENT)
 			return 0;
-		return error_errno("unable to stat %s", sha1_to_hex(sha1));
+		return error_errno("unable to stat %s", oid_to_hex(oid));
 	}
 
-	add_recent_object(sha1, st.st_mtime, data);
+	add_recent_object(oid, st.st_mtime, data);
 	return 0;
 }
 
-static int add_recent_packed(const unsigned char *sha1,
+static int add_recent_packed(const struct object_id *oid,
 			     struct packed_git *p, uint32_t pos,
 			     void *data)
 {
-	struct object *obj = lookup_object(sha1);
+	struct object *obj = lookup_object(oid->hash);
 
 	if (obj && obj->flags & SEEN)
 		return 0;
-	add_recent_object(sha1, p->mtime, data);
+	add_recent_object(oid, p->mtime, data);
 	return 0;
 }
 
diff --git a/sha1_file.c b/sha1_file.c
index d27d1521b..8db91b387 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3660,15 +3660,15 @@ static int for_each_file_in_obj_subdir(int subdir_nr,
 		strbuf_setlen(path, baselen);
 		strbuf_addf(path, "/%s", de->d_name);
 
-		if (strlen(de->d_name) == 38)  {
-			char hex[41];
-			unsigned char sha1[20];
+		if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2)  {
+			char hex[GIT_SHA1_HEXSZ+1];
+			struct object_id oid;
 
 			snprintf(hex, sizeof(hex), "%02x%s",
 				 subdir_nr, de->d_name);
-			if (!get_sha1_hex(hex, sha1)) {
+			if (!get_oid_hex(hex, &oid)) {
 				if (obj_cb) {
-					r = obj_cb(sha1, path->buf, data);
+					r = obj_cb(&oid, path->buf, data);
 					if (r)
 						break;
 				}
@@ -3780,7 +3780,7 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
 			return error("unable to get sha1 of object %u in %s",
 				     i, p->pack_name);
 
-		r = cb(oid->hash, p, i, data);
+		r = cb(oid, p, i, data);
 		if (r)
 			break;
 	}
-- 
2.11.0


^ permalink raw reply related

* [PATCH 14/17] sha1_file: introduce an nth_packed_object_oid function
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

There are places in the code where we would like to provide a struct
object_id *, yet read the hash directly from the pack.  Provide an
nth_packed_object_oid function that mirrors the nth_packed_object_sha1
function.

The required cast is questionable, but should be safe on all known
platforms.  The alternative of allocating an object as an intermediate
would be too inefficient and cause memory leaks.  If necessary, an
intermediate union could be used; this practice is allowed by GCC and
explicitly sanctioned by C11.  However, such a change will likely not be
necessary, and can be made if and when it is.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 cache.h     |  1 +
 sha1_file.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index a50a61a19..b1ed1c7ae 100644
--- a/cache.h
+++ b/cache.h
@@ -1540,6 +1540,7 @@ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
  * error.
  */
 extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
+extern const struct object_id *nth_packed_object_oid(struct packed_git *, uint32_t n);
 
 /*
  * Return the offset of the nth object within the specified packfile.
diff --git a/sha1_file.c b/sha1_file.c
index 117307185..d27d1521b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2608,6 +2608,12 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
 	}
 }
 
+const struct object_id *nth_packed_object_oid(struct packed_git *p,
+					      uint32_t n)
+{
+	return (const struct object_id *)nth_packed_object_sha1(p, n);
+}
+
 void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 {
 	const unsigned char *ptr = vptr;
@@ -3768,13 +3774,13 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
 	int r = 0;
 
 	for (i = 0; i < p->num_objects; i++) {
-		const unsigned char *sha1 = nth_packed_object_sha1(p, i);
+		const struct object_id *oid = nth_packed_object_oid(p, i);
 
-		if (!sha1)
+		if (!oid)
 			return error("unable to get sha1 of object %u in %s",
 				     i, p->pack_name);
 
-		r = cb(sha1, p, i, data);
+		r = cb(oid->hash, p, i, data);
 		if (r)
 			break;
 	}
-- 
2.11.0


^ permalink raw reply related

* [PATCH 08/17] builtin/clone: convert to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/clone.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 5ef81927a..e0916e5f3 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -681,7 +681,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
 
 static int checkout(int submodule_progress)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *head;
 	struct lock_file *lock_file;
 	struct unpack_trees_options opts;
@@ -692,7 +692,7 @@ static int checkout(int submodule_progress)
 	if (option_no_checkout)
 		return 0;
 
-	head = resolve_refdup("HEAD", RESOLVE_REF_READING, sha1, NULL);
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING, oid.hash, NULL);
 	if (!head) {
 		warning(_("remote HEAD refers to nonexistent ref, "
 			  "unable to checkout.\n"));
@@ -700,7 +700,7 @@ static int checkout(int submodule_progress)
 	}
 	if (!strcmp(head, "HEAD")) {
 		if (advice_detached_head)
-			detach_advice(sha1_to_hex(sha1));
+			detach_advice(oid_to_hex(&oid));
 	} else {
 		if (!starts_with(head, "refs/heads/"))
 			die(_("HEAD not found below refs/heads!"));
@@ -721,7 +721,7 @@ static int checkout(int submodule_progress)
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 
-	tree = parse_tree_indirect(sha1);
+	tree = parse_tree_indirect(oid.hash);
 	parse_tree(tree);
 	init_tree_desc(&t, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts) < 0)
@@ -731,7 +731,7 @@ static int checkout(int submodule_progress)
 		die(_("unable to write new index file"));
 
 	err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
-			   sha1_to_hex(sha1), "1", NULL);
+			   oid_to_hex(&oid), "1", NULL);
 
 	if (!err && option_recursive) {
 		struct argv_array args = ARGV_ARRAY_INIT;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 16/17] builtin/merge-base: convert to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Convert the remaining uses of unsigned char [20] to struct object_id.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/merge-base.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index db95bc29c..cfe2a796f 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -36,12 +36,12 @@ static const char * const merge_base_usage[] = {
 
 static struct commit *get_commit_reference(const char *arg)
 {
-	unsigned char revkey[20];
+	struct object_id revkey;
 	struct commit *r;
 
-	if (get_sha1(arg, revkey))
+	if (get_oid(arg, &revkey))
 		die("Not a valid object name %s", arg);
-	r = lookup_commit_reference(revkey);
+	r = lookup_commit_reference(revkey.hash);
 	if (!r)
 		die("Not a valid commit name %s", arg);
 
@@ -113,14 +113,14 @@ struct rev_collect {
 	unsigned int initial : 1;
 };
 
-static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
+static void add_one_commit(struct object_id *oid, struct rev_collect *revs)
 {
 	struct commit *commit;
 
-	if (is_null_sha1(sha1))
+	if (is_null_oid(oid))
 		return;
 
-	commit = lookup_commit(sha1);
+	commit = lookup_commit(oid->hash);
 	if (!commit ||
 	    (commit->object.flags & TMP_MARK) ||
 	    parse_commit(commit))
@@ -139,15 +139,15 @@ static int collect_one_reflog_ent(struct object_id *ooid, struct object_id *noid
 
 	if (revs->initial) {
 		revs->initial = 0;
-		add_one_commit(ooid->hash, revs);
+		add_one_commit(ooid, revs);
 	}
-	add_one_commit(noid->hash, revs);
+	add_one_commit(noid, revs);
 	return 0;
 }
 
 static int handle_fork_point(int argc, const char **argv)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *refname;
 	const char *commitname;
 	struct rev_collect revs;
@@ -155,7 +155,7 @@ static int handle_fork_point(int argc, const char **argv)
 	struct commit_list *bases;
 	int i, ret = 0;
 
-	switch (dwim_ref(argv[0], strlen(argv[0]), sha1, &refname)) {
+	switch (dwim_ref(argv[0], strlen(argv[0]), oid.hash, &refname)) {
 	case 0:
 		die("No such ref: '%s'", argv[0]);
 	case 1:
@@ -165,16 +165,16 @@ static int handle_fork_point(int argc, const char **argv)
 	}
 
 	commitname = (argc == 2) ? argv[1] : "HEAD";
-	if (get_sha1(commitname, sha1))
+	if (get_oid(commitname, &oid))
 		die("Not a valid object name: '%s'", commitname);
 
-	derived = lookup_commit_reference(sha1);
+	derived = lookup_commit_reference(oid.hash);
 	memset(&revs, 0, sizeof(revs));
 	revs.initial = 1;
 	for_each_reflog_ent(refname, collect_one_reflog_ent, &revs);
 
-	if (!revs.nr && !get_sha1(refname, sha1))
-		add_one_commit(sha1, &revs);
+	if (!revs.nr && !get_oid(refname, &oid))
+		add_one_commit(&oid, &revs);
 
 	for (i = 0; i < revs.nr; i++)
 		revs.commit[i]->object.flags &= ~TMP_MARK;
-- 
2.11.0


^ permalink raw reply related

* [PATCH 11/17] builtin/replace: convert to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Convert various uses of unsigned char [20] to struct object_id.  Rename
replace_object_sha1 to rename_object_oid.  Finally, specify a constant
in terms of GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/replace.c | 112 +++++++++++++++++++++++++++---------------------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index b58c714cb..f7716a547 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -88,78 +88,78 @@ static int list_replace_refs(const char *pattern, const char *format)
 }
 
 typedef int (*each_replace_name_fn)(const char *name, const char *ref,
-				    const unsigned char *sha1);
+				    const struct object_id *oid);
 
 static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 {
 	const char **p, *full_hex;
 	char ref[PATH_MAX];
 	int had_error = 0;
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	for (p = argv; *p; p++) {
-		if (get_sha1(*p, sha1)) {
+		if (get_oid(*p, &oid)) {
 			error("Failed to resolve '%s' as a valid ref.", *p);
 			had_error = 1;
 			continue;
 		}
-		full_hex = sha1_to_hex(sha1);
+		full_hex = oid_to_hex(&oid);
 		snprintf(ref, sizeof(ref), "%s%s", git_replace_ref_base, full_hex);
 		/* read_ref() may reuse the buffer */
 		full_hex = ref + strlen(git_replace_ref_base);
-		if (read_ref(ref, sha1)) {
+		if (read_ref(ref, oid.hash)) {
 			error("replace ref '%s' not found.", full_hex);
 			had_error = 1;
 			continue;
 		}
-		if (fn(full_hex, ref, sha1))
+		if (fn(full_hex, ref, &oid))
 			had_error = 1;
 	}
 	return had_error;
 }
 
 static int delete_replace_ref(const char *name, const char *ref,
-			      const unsigned char *sha1)
+			      const struct object_id *oid)
 {
-	if (delete_ref(ref, sha1, 0))
+	if (delete_ref(ref, oid->hash, 0))
 		return 1;
 	printf("Deleted replace ref '%s'\n", name);
 	return 0;
 }
 
-static void check_ref_valid(unsigned char object[20],
-			    unsigned char prev[20],
+static void check_ref_valid(struct object_id *object,
+			    struct object_id *prev,
 			    char *ref,
 			    int ref_size,
 			    int force)
 {
 	if (snprintf(ref, ref_size,
 		     "%s%s", git_replace_ref_base,
-		     sha1_to_hex(object)) > ref_size - 1)
+		     oid_to_hex(object)) > ref_size - 1)
 		die("replace ref name too long: %.*s...", 50, ref);
 	if (check_refname_format(ref, 0))
 		die("'%s' is not a valid ref name.", ref);
 
-	if (read_ref(ref, prev))
-		hashclr(prev);
+	if (read_ref(ref, prev->hash))
+		oidclr(prev);
 	else if (!force)
 		die("replace ref '%s' already exists", ref);
 }
 
-static int replace_object_sha1(const char *object_ref,
-			       unsigned char object[20],
+static int replace_object_oid(const char *object_ref,
+			       struct object_id *object,
 			       const char *replace_ref,
-			       unsigned char repl[20],
+			       struct object_id *repl,
 			       int force)
 {
-	unsigned char prev[20];
+	struct object_id prev;
 	enum object_type obj_type, repl_type;
 	char ref[PATH_MAX];
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 
-	obj_type = sha1_object_info(object, NULL);
-	repl_type = sha1_object_info(repl, NULL);
+	obj_type = sha1_object_info(object->hash, NULL);
+	repl_type = sha1_object_info(repl->hash, NULL);
 	if (!force && obj_type != repl_type)
 		die("Objects must be of the same type.\n"
 		    "'%s' points to a replaced object of type '%s'\n"
@@ -167,11 +167,11 @@ static int replace_object_sha1(const char *object_ref,
 		    object_ref, typename(obj_type),
 		    replace_ref, typename(repl_type));
 
-	check_ref_valid(object, prev, ref, sizeof(ref), force);
+	check_ref_valid(object, &prev, ref, sizeof(ref), force);
 
 	transaction = ref_transaction_begin(&err);
 	if (!transaction ||
-	    ref_transaction_update(transaction, ref, repl, prev,
+	    ref_transaction_update(transaction, ref, repl->hash, prev.hash,
 				   0, NULL, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
@@ -182,14 +182,14 @@ static int replace_object_sha1(const char *object_ref,
 
 static int replace_object(const char *object_ref, const char *replace_ref, int force)
 {
-	unsigned char object[20], repl[20];
+	struct object_id object, repl;
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
-	if (get_sha1(replace_ref, repl))
+	if (get_oid(replace_ref, &repl))
 		die("Failed to resolve '%s' as a valid ref.", replace_ref);
 
-	return replace_object_sha1(object_ref, object, replace_ref, repl, force);
+	return replace_object_oid(object_ref, &object, replace_ref, &repl, force);
 }
 
 /*
@@ -197,7 +197,7 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f
  * If "raw" is true, then the object's raw contents are printed according to
  * "type". Otherwise, we pretty-print the contents for human editing.
  */
-static void export_object(const unsigned char *sha1, enum object_type type,
+static void export_object(const struct object_id *oid, enum object_type type,
 			  int raw, const char *filename)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
@@ -213,7 +213,7 @@ static void export_object(const unsigned char *sha1, enum object_type type,
 		argv_array_push(&cmd.args, typename(type));
 	else
 		argv_array_push(&cmd.args, "-p");
-	argv_array_push(&cmd.args, sha1_to_hex(sha1));
+	argv_array_push(&cmd.args, oid_to_hex(oid));
 	cmd.git_cmd = 1;
 	cmd.out = fd;
 
@@ -226,7 +226,7 @@ static void export_object(const unsigned char *sha1, enum object_type type,
  * interpreting it as "type", and writing the result to the object database.
  * The sha1 of the written object is returned via sha1.
  */
-static void import_object(unsigned char *sha1, enum object_type type,
+static void import_object(struct object_id *oid, enum object_type type,
 			  int raw, const char *filename)
 {
 	int fd;
@@ -254,7 +254,7 @@ static void import_object(unsigned char *sha1, enum object_type type,
 
 		if (finish_command(&cmd))
 			die("mktree reported failure");
-		if (get_sha1_hex(result.buf, sha1) < 0)
+		if (get_oid_hex(result.buf, oid) < 0)
 			die("mktree did not return an object name");
 
 		strbuf_release(&result);
@@ -264,7 +264,7 @@ static void import_object(unsigned char *sha1, enum object_type type,
 
 		if (fstat(fd, &st) < 0)
 			die_errno("unable to fstat %s", filename);
-		if (index_fd(sha1, fd, &st, type, NULL, flags) < 0)
+		if (index_fd(oid->hash, fd, &st, type, NULL, flags) < 0)
 			die("unable to write object to database");
 		/* index_fd close()s fd for us */
 	}
@@ -279,29 +279,29 @@ static int edit_and_replace(const char *object_ref, int force, int raw)
 {
 	char *tmpfile = git_pathdup("REPLACE_EDITOBJ");
 	enum object_type type;
-	unsigned char old[20], new[20], prev[20];
+	struct object_id old, new, prev;
 	char ref[PATH_MAX];
 
-	if (get_sha1(object_ref, old) < 0)
+	if (get_oid(object_ref, &old) < 0)
 		die("Not a valid object name: '%s'", object_ref);
 
-	type = sha1_object_info(old, NULL);
+	type = sha1_object_info(old.hash, NULL);
 	if (type < 0)
-		die("unable to get object type for %s", sha1_to_hex(old));
+		die("unable to get object type for %s", oid_to_hex(&old));
 
-	check_ref_valid(old, prev, ref, sizeof(ref), force);
+	check_ref_valid(&old, &prev, ref, sizeof(ref), force);
 
-	export_object(old, type, raw, tmpfile);
+	export_object(&old, type, raw, tmpfile);
 	if (launch_editor(tmpfile, NULL, NULL) < 0)
 		die("editing object file failed");
-	import_object(new, type, raw, tmpfile);
+	import_object(&new, type, raw, tmpfile);
 
 	free(tmpfile);
 
-	if (!hashcmp(old, new))
-		return error("new object is the same as the old one: '%s'", sha1_to_hex(old));
+	if (!oidcmp(&old, &new))
+		return error("new object is the same as the old one: '%s'", oid_to_hex(&old));
 
-	return replace_object_sha1(object_ref, old, "replacement", new, force);
+	return replace_object_oid(object_ref, &old, "replacement", &new, force);
 }
 
 static void replace_parents(struct strbuf *buf, int argc, const char **argv)
@@ -312,7 +312,7 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 
 	/* find existing parents */
 	parent_start = buf->buf;
-	parent_start += 46; /* "tree " + "hex sha1" + "\n" */
+	parent_start += GIT_SHA1_HEXSZ + 6; /* "tree " + "hex sha1" + "\n" */
 	parent_end = parent_start;
 
 	while (starts_with(parent_end, "parent "))
@@ -320,11 +320,11 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 
 	/* prepare new parents */
 	for (i = 0; i < argc; i++) {
-		unsigned char sha1[20];
-		if (get_sha1(argv[i], sha1) < 0)
+		struct object_id oid;
+		if (get_oid(argv[i], &oid) < 0)
 			die(_("Not a valid object name: '%s'"), argv[i]);
-		lookup_commit_or_die(sha1, argv[i]);
-		strbuf_addf(&new_parents, "parent %s\n", sha1_to_hex(sha1));
+		lookup_commit_or_die(oid.hash, argv[i]);
+		strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid));
 	}
 
 	/* replace existing parents with new ones */
@@ -345,12 +345,12 @@ static void check_one_mergetag(struct commit *commit,
 {
 	struct check_mergetag_data *mergetag_data = (struct check_mergetag_data *)data;
 	const char *ref = mergetag_data->argv[0];
-	unsigned char tag_sha1[20];
+	struct object_id tag_oid;
 	struct tag *tag;
 	int i;
 
-	hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_sha1);
-	tag = lookup_tag(tag_sha1);
+	hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_oid.hash);
+	tag = lookup_tag(tag_oid.hash);
 	if (!tag)
 		die(_("bad mergetag in commit '%s'"), ref);
 	if (parse_tag_buffer(tag, extra->value, extra->len))
@@ -366,7 +366,7 @@ static void check_one_mergetag(struct commit *commit,
 	}
 
 	die(_("original commit '%s' contains mergetag '%s' that is discarded; "
-	      "use --edit instead of --graft"), ref, sha1_to_hex(tag_sha1));
+	      "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid));
 }
 
 static void check_mergetags(struct commit *commit, int argc, const char **argv)
@@ -380,16 +380,16 @@ static void check_mergetags(struct commit *commit, int argc, const char **argv)
 
 static int create_graft(int argc, const char **argv, int force)
 {
-	unsigned char old[20], new[20];
+	struct object_id old, new;
 	const char *old_ref = argv[0];
 	struct commit *commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *buffer;
 	unsigned long size;
 
-	if (get_sha1(old_ref, old) < 0)
+	if (get_oid(old_ref, &old) < 0)
 		die(_("Not a valid object name: '%s'"), old_ref);
-	commit = lookup_commit_or_die(old, old_ref);
+	commit = lookup_commit_or_die(old.hash, old_ref);
 
 	buffer = get_commit_buffer(commit, &size);
 	strbuf_add(&buf, buffer, size);
@@ -404,15 +404,15 @@ static int create_graft(int argc, const char **argv, int force)
 
 	check_mergetags(commit, argc, argv);
 
-	if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+	if (write_sha1_file(buf.buf, buf.len, commit_type, new.hash))
 		die(_("could not write replacement commit for: '%s'"), old_ref);
 
 	strbuf_release(&buf);
 
-	if (!hashcmp(old, new))
-		return error("new commit is the same as the old one: '%s'", sha1_to_hex(old));
+	if (!oidcmp(&old, &new))
+		return error("new commit is the same as the old one: '%s'", oid_to_hex(&old));
 
-	return replace_object_sha1(old_ref, old, "replacement", new, force);
+	return replace_object_oid(old_ref, &old, "replacement", &new, force);
 }
 
 int cmd_replace(int argc, const char **argv, const char *prefix)
-- 
2.11.0


^ permalink raw reply related

* [PATCH 09/17] builtin/merge: convert to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Additionally convert several uses of the constant 40 into
GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/merge.c | 136 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 0070bf255..30bb05ec4 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -240,7 +240,7 @@ static void drop_save(void)
 	unlink(git_path_merge_mode());
 }
 
-static int save_state(unsigned char *stash)
+static int save_state(struct object_id *stash)
 {
 	int len;
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -261,7 +261,7 @@ static int save_state(unsigned char *stash)
 	else if (!len)		/* no changes */
 		return -1;
 	strbuf_setlen(&buffer, buffer.len-1);
-	if (get_sha1(buffer.buf, stash))
+	if (get_oid(buffer.buf, stash))
 		die(_("not a valid object: %s"), buffer.buf);
 	return 0;
 }
@@ -301,18 +301,18 @@ static void reset_hard(unsigned const char *sha1, int verbose)
 		die(_("read-tree failed"));
 }
 
-static void restore_state(const unsigned char *head,
-			  const unsigned char *stash)
+static void restore_state(const struct object_id *head,
+			  const struct object_id *stash)
 {
 	struct strbuf sb = STRBUF_INIT;
 	const char *args[] = { "stash", "apply", NULL, NULL };
 
-	if (is_null_sha1(stash))
+	if (is_null_oid(stash))
 		return;
 
-	reset_hard(head, 1);
+	reset_hard(head->hash, 1);
 
-	args[2] = sha1_to_hex(stash);
+	args[2] = oid_to_hex(stash);
 
 	/*
 	 * It is OK to ignore error here, for example when there was
@@ -372,10 +372,10 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead
 
 static void finish(struct commit *head_commit,
 		   struct commit_list *remoteheads,
-		   const unsigned char *new_head, const char *msg)
+		   const struct object_id *new_head, const char *msg)
 {
 	struct strbuf reflog_message = STRBUF_INIT;
-	const unsigned char *head = head_commit->object.oid.hash;
+	const struct object_id *head = &head_commit->object.oid;
 
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
@@ -393,7 +393,7 @@ static void finish(struct commit *head_commit,
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 			update_ref(reflog_message.buf, "HEAD",
-				new_head, head, 0,
+				new_head->hash, head->hash, 0,
 				UPDATE_REFS_DIE_ON_ERR);
 			/*
 			 * We ignore errors in 'gc --auto', since the
@@ -412,7 +412,7 @@ static void finish(struct commit *head_commit,
 			DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
 		opts.detect_rename = DIFF_DETECT_RENAME;
 		diff_setup_done(&opts);
-		diff_tree_sha1(head, new_head, "", &opts);
+		diff_tree_sha1(head->hash, new_head->hash, "", &opts);
 		diffcore_std(&opts);
 		diff_flush(&opts);
 	}
@@ -427,7 +427,7 @@ static void finish(struct commit *head_commit,
 static void merge_name(const char *remote, struct strbuf *msg)
 {
 	struct commit *remote_head;
-	unsigned char branch_head[20];
+	struct object_id branch_head;
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf bname = STRBUF_INIT;
 	const char *ptr;
@@ -437,25 +437,25 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	strbuf_branchname(&bname, remote);
 	remote = bname.buf;
 
-	memset(branch_head, 0, sizeof(branch_head));
+	memset(&branch_head, 0, sizeof(branch_head));
 	remote_head = get_merge_parent(remote);
 	if (!remote_head)
 		die(_("'%s' does not point to a commit"), remote);
 
-	if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {
+	if (dwim_ref(remote, strlen(remote), branch_head.hash, &found_ref) > 0) {
 		if (starts_with(found_ref, "refs/heads/")) {
 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 		if (starts_with(found_ref, "refs/tags/")) {
 			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 		if (starts_with(found_ref, "refs/remotes/")) {
 			strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 	}
@@ -586,8 +586,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	return git_diff_ui_config(k, v, cb);
 }
 
-static int read_tree_trivial(unsigned char *common, unsigned char *head,
-			     unsigned char *one)
+static int read_tree_trivial(struct object_id *common, struct object_id *head,
+			     struct object_id *one)
 {
 	int i, nr_trees = 0;
 	struct tree *trees[MAX_UNPACK_TREES];
@@ -602,13 +602,13 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	opts.verbose_update = 1;
 	opts.trivial_merges_only = 1;
 	opts.merge = 1;
-	trees[nr_trees] = parse_tree_indirect(common);
+	trees[nr_trees] = parse_tree_indirect(common->hash);
 	if (!trees[nr_trees++])
 		return -1;
-	trees[nr_trees] = parse_tree_indirect(head);
+	trees[nr_trees] = parse_tree_indirect(head->hash);
 	if (!trees[nr_trees++])
 		return -1;
-	trees[nr_trees] = parse_tree_indirect(one);
+	trees[nr_trees] = parse_tree_indirect(one->hash);
 	if (!trees[nr_trees++])
 		return -1;
 	opts.fn = threeway_merge;
@@ -622,9 +622,9 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	return 0;
 }
 
-static void write_tree_trivial(unsigned char *sha1)
+static void write_tree_trivial(struct object_id *oid)
 {
-	if (write_cache_as_tree(sha1, 0, NULL))
+	if (write_cache_as_tree(oid->hash, 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 }
 
@@ -777,7 +777,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 
 static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
-	unsigned char result_tree[20], result_commit[20];
+	struct object_id result_tree, result_commit;
 	struct commit_list *parents, **pptr = &parents;
 	static struct lock_file lock;
 
@@ -788,15 +788,15 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 		return error(_("Unable to write index."));
 	rollback_lock_file(&lock);
 
-	write_tree_trivial(result_tree);
+	write_tree_trivial(&result_tree);
 	printf(_("Wonderful.\n"));
 	pptr = commit_list_append(head, pptr);
 	pptr = commit_list_append(remoteheads->item, pptr);
 	prepare_to_commit(remoteheads);
-	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
+			result_commit.hash, NULL, sign_commit))
 		die(_("failed to write commit object"));
-	finish(head, remoteheads, result_commit, "In-index merge");
+	finish(head, remoteheads, &result_commit, "In-index merge");
 	drop_save();
 	return 0;
 }
@@ -805,12 +805,12 @@ static int finish_automerge(struct commit *head,
 			    int head_subsumed,
 			    struct commit_list *common,
 			    struct commit_list *remoteheads,
-			    unsigned char *result_tree,
+			    struct object_id *result_tree,
 			    const char *wt_strategy)
 {
 	struct commit_list *parents = NULL;
 	struct strbuf buf = STRBUF_INIT;
-	unsigned char result_commit[20];
+	struct object_id result_commit;
 
 	free_commit_list(common);
 	parents = remoteheads;
@@ -818,11 +818,11 @@ static int finish_automerge(struct commit *head,
 		commit_list_insert(head, &parents);
 	strbuf_addch(&merge_msg, '\n');
 	prepare_to_commit(remoteheads);
-	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, parents,
+			result_commit.hash, NULL, sign_commit))
 		die(_("failed to write commit object"));
 	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
-	finish(head, remoteheads, result_commit, buf.buf);
+	finish(head, remoteheads, &result_commit, buf.buf);
 	strbuf_release(&buf);
 	drop_save();
 	return 0;
@@ -850,18 +850,18 @@ static int suggest_conflicts(void)
 }
 
 static struct commit *is_old_style_invocation(int argc, const char **argv,
-					      const unsigned char *head)
+					      const struct object_id *head)
 {
 	struct commit *second_token = NULL;
 	if (argc > 2) {
-		unsigned char second_sha1[20];
+		struct object_id second_oid;
 
-		if (get_sha1(argv[1], second_sha1))
+		if (get_oid(argv[1], &second_oid))
 			return NULL;
-		second_token = lookup_commit_reference_gently(second_sha1, 0);
+		second_token = lookup_commit_reference_gently(second_oid.hash, 0);
 		if (!second_token)
 			die(_("'%s' is not a commit"), argv[1]);
-		if (hashcmp(second_token->object.oid.hash, head))
+		if (oidcmp(&second_token->object.oid, head))
 			return NULL;
 	}
 	return second_token;
@@ -1034,7 +1034,7 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
 		die_errno(_("could not close '%s'"), filename);
 
 	for (pos = 0; pos < merge_names->len; pos = npos) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		char *ptr;
 		struct commit *commit;
 
@@ -1044,16 +1044,16 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
 		else
 			npos = merge_names->len;
 
-		if (npos - pos < 40 + 2 ||
-		    get_sha1_hex(merge_names->buf + pos, sha1))
+		if (npos - pos < GIT_SHA1_HEXSZ + 2 ||
+		    get_oid_hex(merge_names->buf + pos, &oid))
 			commit = NULL; /* bad */
-		else if (memcmp(merge_names->buf + pos + 40, "\t\t", 2))
+		else if (memcmp(merge_names->buf + pos + GIT_SHA1_HEXSZ, "\t\t", 2))
 			continue; /* not-for-merge */
 		else {
-			char saved = merge_names->buf[pos + 40];
-			merge_names->buf[pos + 40] = '\0';
+			char saved = merge_names->buf[pos + GIT_SHA1_HEXSZ];
+			merge_names->buf[pos + GIT_SHA1_HEXSZ] = '\0';
 			commit = get_merge_parent(merge_names->buf + pos);
-			merge_names->buf[pos + 40] = saved;
+			merge_names->buf[pos + GIT_SHA1_HEXSZ] = saved;
 		}
 		if (!commit) {
 			if (ptr)
@@ -1113,9 +1113,9 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
-	unsigned char result_tree[20];
-	unsigned char stash[20];
-	unsigned char head_sha1[20];
+	struct object_id result_tree;
+	struct object_id stash;
+	struct object_id head_oid;
 	struct commit *head_commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *head_arg;
@@ -1133,13 +1133,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
 	 * current branch.
 	 */
-	branch = branch_to_free = resolve_refdup("HEAD", 0, head_sha1, NULL);
+	branch = branch_to_free = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
 	if (branch && starts_with(branch, "refs/heads/"))
 		branch += 11;
-	if (!branch || is_null_sha1(head_sha1))
+	if (!branch || is_null_oid(&head_oid))
 		head_commit = NULL;
 	else
-		head_commit = lookup_commit_or_die(head_sha1, "HEAD");
+		head_commit = lookup_commit_or_die(head_oid.hash, "HEAD");
 
 	init_diff_ui_defaults();
 	git_config(git_merge_config, NULL);
@@ -1217,7 +1217,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to forbid "git merge" into a branch yet to be born.
 		 * We do the same for "git pull".
 		 */
-		unsigned char *remote_head_sha1;
+		struct object_id *remote_head_oid;
 		if (squash)
 			die(_("Squash commit into empty head not supported yet"));
 		if (fast_forward == FF_NO)
@@ -1229,9 +1229,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die(_("%s - not something we can merge"), argv[0]);
 		if (remoteheads->next)
 			die(_("Can merge only exactly one commit into empty head"));
-		remote_head_sha1 = remoteheads->item->object.oid.hash;
-		read_empty(remote_head_sha1, 0);
-		update_ref("initial pull", "HEAD", remote_head_sha1,
+		remote_head_oid = &remoteheads->item->object.oid;
+		read_empty(remote_head_oid->hash, 0);
+		update_ref("initial pull", "HEAD", remote_head_oid->hash,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 		goto done;
 	}
@@ -1245,7 +1245,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * additional safety measure to check for it.
 	 */
 	if (!have_message &&
-	    is_old_style_invocation(argc, argv, head_commit->object.oid.hash)) {
+	    is_old_style_invocation(argc, argv, &head_commit->object.oid)) {
 		warning("old-style 'git merge <msg> HEAD <commit>' is deprecated.");
 		strbuf_addstr(&merge_msg, argv[0]);
 		head_arg = argv[1];
@@ -1397,7 +1397,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			goto done;
 		}
 
-		finish(head_commit, remoteheads, commit->object.oid.hash, msg.buf);
+		finish(head_commit, remoteheads, &commit->object.oid, msg.buf);
 		drop_save();
 		goto done;
 	} else if (!remoteheads->next && common->next)
@@ -1416,9 +1416,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			/* See if it is really trivial. */
 			git_committer_info(IDENT_STRICT);
 			printf(_("Trying really trivial in-index merge...\n"));
-			if (!read_tree_trivial(common->item->object.oid.hash,
-					       head_commit->object.oid.hash,
-					       remoteheads->item->object.oid.hash)) {
+			if (!read_tree_trivial(&common->item->object.oid,
+					       &head_commit->object.oid,
+					       &remoteheads->item->object.oid)) {
 				ret = merge_trivial(head_commit, remoteheads);
 				goto done;
 			}
@@ -1470,14 +1470,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	    /*
 	     * Stash away the local changes so that we can try more than one.
 	     */
-	    save_state(stash))
-		hashclr(stash);
+	    save_state(&stash))
+		oidclr(&stash);
 
 	for (i = 0; i < use_strategies_nr; i++) {
 		int ret;
 		if (i) {
 			printf(_("Rewinding the tree to pristine...\n"));
-			restore_state(head_commit->object.oid.hash, stash);
+			restore_state(&head_commit->object.oid, &stash);
 		}
 		if (use_strategies_nr != 1)
 			printf(_("Trying merge strategy %s...\n"),
@@ -1522,7 +1522,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		/* Automerge succeeded. */
-		write_tree_trivial(result_tree);
+		write_tree_trivial(&result_tree);
 		automerge_was_ok = 1;
 		break;
 	}
@@ -1534,7 +1534,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (automerge_was_ok) {
 		ret = finish_automerge(head_commit, head_subsumed,
 				       common, remoteheads,
-				       result_tree, wt_strategy);
+				       &result_tree, wt_strategy);
 		goto done;
 	}
 
@@ -1543,7 +1543,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * it up.
 	 */
 	if (!best_strategy) {
-		restore_state(head_commit->object.oid.hash, stash);
+		restore_state(&head_commit->object.oid, &stash);
 		if (use_strategies_nr > 1)
 			fprintf(stderr,
 				_("No merge strategy handled the merge.\n"));
@@ -1556,7 +1556,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		; /* We already have its result in the working tree. */
 	else {
 		printf(_("Rewinding the tree to pristine...\n"));
-		restore_state(head_commit->object.oid.hash, stash);
+		restore_state(&head_commit->object.oid, &stash);
 		printf(_("Using the %s to prepare resolving by hand.\n"),
 			best_strategy);
 		try_merge_strategy(best_strategy, common, remoteheads,
-- 
2.11.0


^ permalink raw reply related

* [PATCH 07/17] builtin/branch: convert to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/branch.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 9d30f55b0..faf472ff8 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -32,7 +32,7 @@ static const char * const builtin_branch_usage[] = {
 };
 
 static const char *head;
-static unsigned char head_sha1[20];
+static struct object_id head_oid;
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -117,13 +117,13 @@ static int branch_merged(int kind, const char *name,
 	if (kind == FILTER_REFS_BRANCHES) {
 		struct branch *branch = branch_get(name);
 		const char *upstream = branch_get_upstream(branch, NULL);
-		unsigned char sha1[20];
+		struct object_id oid;
 
 		if (upstream &&
 		    (reference_name = reference_name_to_free =
 		     resolve_refdup(upstream, RESOLVE_REF_READING,
-				    sha1, NULL)) != NULL)
-			reference_rev = lookup_commit_reference(sha1);
+				    oid.hash, NULL)) != NULL)
+			reference_rev = lookup_commit_reference(oid.hash);
 	}
 	if (!reference_rev)
 		reference_rev = head_rev;
@@ -153,10 +153,10 @@ static int branch_merged(int kind, const char *name,
 }
 
 static int check_branch_commit(const char *branchname, const char *refname,
-			       const unsigned char *sha1, struct commit *head_rev,
+			       const struct object_id *oid, struct commit *head_rev,
 			       int kinds, int force)
 {
-	struct commit *rev = lookup_commit_reference(sha1);
+	struct commit *rev = lookup_commit_reference(oid->hash);
 	if (!rev) {
 		error(_("Couldn't look up commit object for '%s'"), refname);
 		return -1;
@@ -183,7 +183,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			   int quiet)
 {
 	struct commit *head_rev = NULL;
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *name = NULL;
 	const char *fmt;
 	int i;
@@ -207,7 +207,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	}
 
 	if (!force) {
-		head_rev = lookup_commit_reference(head_sha1);
+		head_rev = lookup_commit_reference(head_oid.hash);
 		if (!head_rev)
 			die(_("Couldn't look up commit object for HEAD"));
 	}
@@ -235,7 +235,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 					RESOLVE_REF_READING
 					| RESOLVE_REF_NO_RECURSE
 					| RESOLVE_REF_ALLOW_BAD_NAME,
-					sha1, &flags);
+					oid.hash, &flags);
 		if (!target) {
 			error(remote_branch
 			      ? _("remote-tracking branch '%s' not found.")
@@ -245,13 +245,13 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		}
 
 		if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
-		    check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
+		    check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
 					force)) {
 			ret = 1;
 			goto next;
 		}
 
-		if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
+		if (delete_ref(name, is_null_oid(&oid) ? NULL : oid.hash,
 			       REF_NODEREF)) {
 			error(remote_branch
 			      ? _("Error deleting remote-tracking branch '%s'")
@@ -267,7 +267,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			       bname.buf,
 			       (flags & REF_ISBROKEN) ? "broken"
 			       : (flags & REF_ISSYMREF) ? target
-			       : find_unique_abbrev(sha1, DEFAULT_ABBREV));
+			       : find_unique_abbrev(oid.hash, DEFAULT_ABBREV));
 		}
 		delete_branch_config(bname.buf);
 
@@ -693,7 +693,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	track = git_branch_track;
 
-	head = resolve_refdup("HEAD", 0, head_sha1, NULL);
+	head = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
 	if (!head)
 		die(_("Failed to resolve HEAD as a valid ref."));
 	if (!strcmp(head, "HEAD"))
-- 
2.11.0


^ permalink raw reply related

* [PATCH 03/17] builtin/describe: convert to struct object_id
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty
In-Reply-To: <20170101191847.564741-1-sandals@crustytoothpaste.net>

Convert the functions in this file and struct commit_name  to struct
object_id.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/describe.c | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157..738e68f95 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -39,11 +39,11 @@ static const char *diff_index_args[] = {
 
 struct commit_name {
 	struct hashmap_entry entry;
-	unsigned char peeled[20];
+	struct object_id peeled;
 	struct tag *tag;
 	unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
 	unsigned name_checked:1;
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *path;
 };
 
@@ -54,17 +54,17 @@ static const char *prio_names[] = {
 static int commit_name_cmp(const struct commit_name *cn1,
 		const struct commit_name *cn2, const void *peeled)
 {
-	return hashcmp(cn1->peeled, peeled ? peeled : cn2->peeled);
+	return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
 }
 
-static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+static inline struct commit_name *find_commit_name(const struct object_id *peeled)
 {
-	return hashmap_get_from_hash(&names, sha1hash(peeled), peeled);
+	return hashmap_get_from_hash(&names, sha1hash(peeled->hash), peeled->hash);
 }
 
 static int replace_name(struct commit_name *e,
 			       int prio,
-			       const unsigned char *sha1,
+			       const struct object_id *oid,
 			       struct tag **tag)
 {
 	if (!e || e->prio < prio)
@@ -77,13 +77,13 @@ static int replace_name(struct commit_name *e,
 		struct tag *t;
 
 		if (!e->tag) {
-			t = lookup_tag(e->sha1);
+			t = lookup_tag(e->oid.hash);
 			if (!t || parse_tag(t))
 				return 1;
 			e->tag = t;
 		}
 
-		t = lookup_tag(sha1);
+		t = lookup_tag(oid->hash);
 		if (!t || parse_tag(t))
 			return 0;
 		*tag = t;
@@ -96,24 +96,24 @@ static int replace_name(struct commit_name *e,
 }
 
 static void add_to_known_names(const char *path,
-			       const unsigned char *peeled,
+			       const struct object_id *peeled,
 			       int prio,
-			       const unsigned char *sha1)
+			       const struct object_id *oid)
 {
 	struct commit_name *e = find_commit_name(peeled);
 	struct tag *tag = NULL;
-	if (replace_name(e, prio, sha1, &tag)) {
+	if (replace_name(e, prio, oid, &tag)) {
 		if (!e) {
 			e = xmalloc(sizeof(struct commit_name));
-			hashcpy(e->peeled, peeled);
-			hashmap_entry_init(e, sha1hash(peeled));
+			oidcpy(&e->peeled, peeled);
+			hashmap_entry_init(e, sha1hash(peeled->hash));
 			hashmap_add(&names, e);
 			e->path = NULL;
 		}
 		e->tag = tag;
 		e->prio = prio;
 		e->name_checked = 0;
-		hashcpy(e->sha1, sha1);
+		oidcpy(&e->oid, oid);
 		free(e->path);
 		e->path = xstrdup(path);
 	}
@@ -154,7 +154,7 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
 	else
 		prio = 0;
 
-	add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, oid->hash);
+	add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid);
 	return 0;
 }
 
@@ -212,7 +212,7 @@ static unsigned long finish_depth_computation(
 static void display_name(struct commit_name *n)
 {
 	if (n->prio == 2 && !n->tag) {
-		n->tag = lookup_tag(n->sha1);
+		n->tag = lookup_tag(n->oid.hash);
 		if (!n->tag || parse_tag(n->tag))
 			die(_("annotated tag %s not available"), n->path);
 	}
@@ -230,14 +230,14 @@ static void display_name(struct commit_name *n)
 		printf("%s", n->path);
 }
 
-static void show_suffix(int depth, const unsigned char *sha1)
+static void show_suffix(int depth, const struct object_id *oid)
 {
-	printf("-%d-g%s", depth, find_unique_abbrev(sha1, abbrev));
+	printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
 }
 
 static void describe(const char *arg, int last_one)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct commit *cmit, *gave_up_on = NULL;
 	struct commit_list *list;
 	struct commit_name *n;
@@ -246,20 +246,20 @@ static void describe(const char *arg, int last_one)
 	unsigned long seen_commits = 0;
 	unsigned int unannotated_cnt = 0;
 
-	if (get_sha1(arg, sha1))
+	if (get_oid(arg, &oid))
 		die(_("Not a valid object name %s"), arg);
-	cmit = lookup_commit_reference(sha1);
+	cmit = lookup_commit_reference(oid.hash);
 	if (!cmit)
 		die(_("%s is not a valid '%s' object"), arg, commit_type);
 
-	n = find_commit_name(cmit->object.oid.hash);
+	n = find_commit_name(&cmit->object.oid);
 	if (n && (tags || all || n->prio == 2)) {
 		/*
 		 * Exact match to an existing ref.
 		 */
 		display_name(n);
 		if (longformat)
-			show_suffix(0, n->tag ? n->tag->tagged->oid.hash : sha1);
+			show_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);
 		if (dirty)
 			printf("%s", dirty);
 		printf("\n");
@@ -276,7 +276,7 @@ static void describe(const char *arg, int last_one)
 		struct commit *c;
 		struct commit_name *n = hashmap_iter_first(&names, &iter);
 		for (; n; n = hashmap_iter_next(&iter)) {
-			c = lookup_commit_reference_gently(n->peeled, 1);
+			c = lookup_commit_reference_gently(n->peeled.hash, 1);
 			if (c)
 				c->util = n;
 		}
@@ -380,7 +380,7 @@ static void describe(const char *arg, int last_one)
 
 	display_name(all_matches[0].name);
 	if (abbrev)
-		show_suffix(all_matches[0].depth, cmit->object.oid.hash);
+		show_suffix(all_matches[0].depth, &cmit->object.oid);
 	if (dirty)
 		printf("%s", dirty);
 	printf("\n");
-- 
2.11.0


^ permalink raw reply related

* [PATCH 00/17] object_id part 6
From: brian m. carlson @ 2017-01-01 19:18 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty

This is another series in the continuing conversion to struct object_id.

This series converts more of the builtin directory and some of the
refs code to use struct object_id. Additionally, it implements an
nth_packed_object_oid function which provides a struct object_id
version of the nth_packed_object function.

There is a small known conflict with next, but it can easily be fixed up.

brian m. carlson (17):
  builtin/commit: convert to struct object_id
  builtin/diff-tree: convert to struct object_id
  builtin/describe: convert to struct object_id
  builtin/fast-export: convert to struct object_id
  builtin/fmt-merge-message: convert to struct object_id
  builtin/grep: convert to struct object_id
  builtin/branch: convert to struct object_id
  builtin/clone: convert to struct object_id
  builtin/merge: convert to struct object_id
  Convert remaining callers of resolve_refdup to object_id
  builtin/replace: convert to struct object_id
  reflog-walk: convert struct reflog_info to struct object_id
  refs: convert each_reflog_ent_fn to struct object_id
  sha1_file: introduce an nth_packed_object_oid function
  Convert object iteration callbacks to struct object_id
  builtin/merge-base: convert to struct object_id
  wt-status: convert to struct object_id

 builtin/branch.c        |  26 ++++-----
 builtin/cat-file.c      |   8 +--
 builtin/clone.c         |  10 ++--
 builtin/commit.c        |  46 ++++++++--------
 builtin/count-objects.c |   4 +-
 builtin/describe.c      |  50 +++++++++---------
 builtin/diff-tree.c     |  38 +++++++-------
 builtin/fast-export.c   |  58 ++++++++++-----------
 builtin/fmt-merge-msg.c |  70 ++++++++++++-------------
 builtin/fsck.c          |  26 ++++-----
 builtin/grep.c          |  24 ++++-----
 builtin/merge-base.c    |  30 +++++------
 builtin/merge.c         | 136 ++++++++++++++++++++++++------------------------
 builtin/notes.c         |  18 +++----
 builtin/pack-objects.c  |   6 +--
 builtin/prune-packed.c  |   4 +-
 builtin/prune.c         |   8 +--
 builtin/receive-pack.c  |   4 +-
 builtin/reflog.c        |   2 +-
 builtin/replace.c       | 112 +++++++++++++++++++--------------------
 cache.h                 |   5 +-
 reachable.c             |  30 +++++------
 ref-filter.c            |   4 +-
 reflog-walk.c           |  26 ++++-----
 refs.c                  |  24 ++++-----
 refs.h                  |   2 +-
 refs/files-backend.c    |  24 ++++-----
 revision.c              |  12 ++---
 sha1_file.c             |  22 +++++---
 sha1_name.c             |   2 +-
 transport.c             |   4 +-
 wt-status.c             |  52 +++++++++---------
 32 files changed, 448 insertions(+), 439 deletions(-)

-- 
2.11.0


^ permalink raw reply

* Re: [PATCH v15 15/27] bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
From: Pranit Bauva @ 2017-01-01 17:41 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Git List
In-Reply-To: <cdf41dcc-bdc5-a3a7-8d39-8b85e01cda85@gmx.net>

Hey Stephan,

On Sun, Jan 1, 2017 at 9:57 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
>>>>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>>> @@ -643,6 +794,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>>>                        N_("print out the bisect terms"), BISECT_TERMS),
>>>>               OPT_CMDMODE(0, "bisect-start", &cmdmode,
>>>>                        N_("start the bisect session"), BISECT_START),
>>>> +             OPT_CMDMODE(0, "bisect-next", &cmdmode,
>>>> +                      N_("find the next bisection commit"), BISECT_NEXT),
>>>> +             OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
>>>> +                      N_("verify the next bisection state then find the next bisection state"), BISECT_AUTO_NEXT),
>>>
>>> The next bisection *state* is found?
>>
>> checkout is more appropriate. I don't remember why I used "find".
>
> "checkout the next bisection commit" maybe?

Seems better. Thanks!

Regards,
Pranit Bauva

^ permalink raw reply

* [PATCH] read-cache: mark a file-local symbol with static
From: Ramsay Jones @ 2017-01-01 17:10 UTC (permalink / raw)
  To: Christian Couder; +Cc: Junio C Hamano, GIT Mailing-list


Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Christian,

If you need to re-roll your 'cc/split-index-config' branch, could
you please squash this into the relevant patch (commit 8a7e3ef9a6,
"read-cache: touch shared index files when used", 26-12-2016).

Thanks!

ATB,
Ramsay Jones

 read-cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index 119701bf0..17b9f2461 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1682,7 +1682,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
  * for some time. It's ok to fail to update the mtime if we are on a
  * read only file system.
  */
-void freshen_shared_index(char *base_sha1_hex)
+static void freshen_shared_index(char *base_sha1_hex)
 {
 	const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
 	check_and_freshen_file(shared_index, 1);
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v15 15/27] bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
From: Stephan Beyer @ 2017-01-01 16:27 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPPtF5P5nGp+=btHtwNm1unTJ7qo1khJHCqLvNn1=RYAUQ@mail.gmail.com>

Hi Pranit,

On 12/31/2016 11:43 AM, Pranit Bauva wrote:
>>> +
>>> +static int bisect_auto_next(struct bisect_terms *terms, const char *prefix)
>>> +{
>>> +     if (!bisect_next_check(terms, NULL))
>>> +             return bisect_next(terms, prefix);
>>> +
>>> +     return 0;
>>> +}
>>
>> Hmm, the handling of the return values is a little confusing. However,
>> if I understand the sh source correctly, it always returns success, no
>> matter if bisect_next failed or not. I do not know if you had something
>> special in mind here.
> 
> Umm. Shell code used to die() and thus exit with an error code.

The invoked bisect_next shell code called "exit", right... you had to
replace this by passing return values. I get it. Thank you!

>>>  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>> @@ -643,6 +794,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>>>                        N_("print out the bisect terms"), BISECT_TERMS),
>>>               OPT_CMDMODE(0, "bisect-start", &cmdmode,
>>>                        N_("start the bisect session"), BISECT_START),
>>> +             OPT_CMDMODE(0, "bisect-next", &cmdmode,
>>> +                      N_("find the next bisection commit"), BISECT_NEXT),
>>> +             OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
>>> +                      N_("verify the next bisection state then find the next bisection state"), BISECT_AUTO_NEXT),
>>
>> The next bisection *state* is found?
> 
> checkout is more appropriate. I don't remember why I used "find".

"checkout the next bisection commit" maybe?

Thanks,
  Stephan

^ permalink raw reply

* Re: [PATCH] don't use test_must_fail with grep
From: Luke Diamand @ 2017-01-01 15:24 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pranit Bauva, Git Users, Stefan Beller
In-Reply-To: <285ed013-5c59-0b98-7dc0-8f729587a313@kdbg.org>

On 1 January 2017 at 14:50, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 01.01.2017 um 15:23 schrieb Luke Diamand:
>>
>> On 31 December 2016 at 11:44, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>>
>>> diff --git a/t/t9813-git-p4-preserve-users.sh
>>> b/t/t9813-git-p4-preserve-users.sh
>>> index 0fe231280..2384535a7 100755
>>> --- a/t/t9813-git-p4-preserve-users.sh
>>> +++ b/t/t9813-git-p4-preserve-users.sh
>>> @@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed
>>> authorship' '
>>>                 grep "git author charlie@example.com does not match" &&
>>>
>>>                 make_change_by_user usernamefile3 alice alice@example.com
>>> &&
>>> -               git p4 commit |\
>>> -               test_must_fail grep "git author.*does not match" &&
>>> +               ! git p4 commit |\
>>> +               grep "git author.*does not match" &&
>>
>>
>> Would it be clearer to use this?
>>
>>     git p4 commit |\
>>     grep -q -v "git author.*does not match" &&
>>
>> With your original change, I think that if "git p4 commit" fails, then
>> that expression will be treated as a pass.
>
>
> No. The exit code of the upstream in a pipe is ignored. For this reason,
> having a git invocation as the upstream of a pipe *anywhere* in the test
> suite is frowned upon. Hence, a better rewrite would be
>
>         git p4 commit >actual &&
>         ! grep "git author.*does not match" actual &&
>
> which makes me wonder: Is the message that we do expect not to occur
> actually printed on stdout? It sounds much more like an error message, i.e.,
> text that is printed on stderr. Wouldn't we need this?
>
>         git p4 commit >actual 2>&1 &&
>         ! grep "git author.*does not match" actual &&

The message is actually part of a template presented to the user via
their chosen editor. For this test, we set the editor to be "cat", so
it comes out on stdout.

Your first suggestion would therefore be fine (and similarly for the
other cases).

^ permalink raw reply

* Re: [PATCH] don't use test_must_fail with grep
From: Johannes Sixt @ 2017-01-01 14:50 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Pranit Bauva, Git Users, Stefan Beller
In-Reply-To: <CAE5ih7-7e+ZLUbE7iquWV2=qP4ofzAHUC2ZPg3b-ivSpCo4eRw@mail.gmail.com>

Am 01.01.2017 um 15:23 schrieb Luke Diamand:
> On 31 December 2016 at 11:44, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
>> index 0fe231280..2384535a7 100755
>> --- a/t/t9813-git-p4-preserve-users.sh
>> +++ b/t/t9813-git-p4-preserve-users.sh
>> @@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '
>>                 grep "git author charlie@example.com does not match" &&
>>
>>                 make_change_by_user usernamefile3 alice alice@example.com &&
>> -               git p4 commit |\
>> -               test_must_fail grep "git author.*does not match" &&
>> +               ! git p4 commit |\
>> +               grep "git author.*does not match" &&
>
> Would it be clearer to use this?
>
>     git p4 commit |\
>     grep -q -v "git author.*does not match" &&
>
> With your original change, I think that if "git p4 commit" fails, then
> that expression will be treated as a pass.

No. The exit code of the upstream in a pipe is ignored. For this reason, 
having a git invocation as the upstream of a pipe *anywhere* in the test 
suite is frowned upon. Hence, a better rewrite would be

	git p4 commit >actual &&
	! grep "git author.*does not match" actual &&

which makes me wonder: Is the message that we do expect not to occur 
actually printed on stdout? It sounds much more like an error message, 
i.e., text that is printed on stderr. Wouldn't we need this?

	git p4 commit >actual 2>&1 &&
	! grep "git author.*does not match" actual &&

-- Hannes


^ permalink raw reply

* Re: [PATCH] don't use test_must_fail with grep
From: Luke Diamand @ 2017-01-01 14:23 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: Git Users, Stefan Beller
In-Reply-To: <20161231114412.23439-1-pranit.bauva@gmail.com>

On 31 December 2016 at 11:44, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> test_must_fail should only be used for testing git commands. To test the
> failure of other commands use `!`.
>
> Reported-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
>  t/t3510-cherry-pick-sequence.sh  |  6 +++---
>  t/t5504-fetch-receive-strict.sh  |  2 +-
>  t/t5516-fetch-push.sh            |  2 +-
>  t/t5601-clone.sh                 |  2 +-
>  t/t6030-bisect-porcelain.sh      |  2 +-
>  t/t7610-mergetool.sh             |  2 +-
>  t/t9001-send-email.sh            |  2 +-
>  t/t9117-git-svn-init-clone.sh    | 12 ++++++------
>  t/t9813-git-p4-preserve-users.sh |  8 ++++----
>  t/t9814-git-p4-rename.sh         |  6 +++---
>  10 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
> index 372307c21..0acf4b146 100755
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -385,7 +385,7 @@ test_expect_success '--continue respects opts' '
>         git cat-file commit HEAD~1 >picked_msg &&
>         git cat-file commit HEAD~2 >unrelatedpick_msg &&
>         git cat-file commit HEAD~3 >initial_msg &&
> -       test_must_fail grep "cherry picked from" initial_msg &&
> +       ! grep "cherry picked from" initial_msg &&
>         grep "cherry picked from" unrelatedpick_msg &&
>         grep "cherry picked from" picked_msg &&
>         grep "cherry picked from" anotherpick_msg
> @@ -426,9 +426,9 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'
>         git cat-file commit HEAD~1 >picked_msg &&
>         git cat-file commit HEAD~2 >unrelatedpick_msg &&
>         git cat-file commit HEAD~3 >initial_msg &&
> -       test_must_fail grep "Signed-off-by:" initial_msg &&
> +       ! grep "Signed-off-by:" initial_msg &&
>         grep "Signed-off-by:" unrelatedpick_msg &&
> -       test_must_fail grep "Signed-off-by:" picked_msg &&
> +       ! grep "Signed-off-by:" picked_msg &&
>         grep "Signed-off-by:" anotherpick_msg
>  '
>
> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
> index 9b19cff72..49d3621a9 100755
> --- a/t/t5504-fetch-receive-strict.sh
> +++ b/t/t5504-fetch-receive-strict.sh
> @@ -152,7 +152,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
>         git --git-dir=dst/.git config --add \
>                 receive.fsck.badDate warn &&
>         git push --porcelain dst bogus >act 2>&1 &&
> -       test_must_fail grep "missingEmail" act
> +       ! grep "missingEmail" act
>  '
>
>  test_expect_success \
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 26b2cafc4..0fc5a7c59 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1004,7 +1004,7 @@ test_expect_success 'push --porcelain' '
>  test_expect_success 'push --porcelain bad url' '
>         mk_empty testrepo &&
>         test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/master:refs/remotes/origin/master &&
> -       test_must_fail grep -q Done .git/bar
> +       ! grep -q Done .git/bar
>  '
>
>  test_expect_success 'push --porcelain rejected' '
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index a43339420..4241ea5b3 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -151,7 +151,7 @@ test_expect_success 'clone --mirror does not repeat tags' '
>         git clone --mirror src mirror2 &&
>         (cd mirror2 &&
>          git show-ref 2> clone.err > clone.out) &&
> -       test_must_fail grep Duplicate mirror2/clone.err &&
> +       ! grep Duplicate mirror2/clone.err &&
>         grep some-tag mirror2/clone.out
>
>  '
> diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
> index 5e5370feb..8c2c6eaef 100755
> --- a/t/t6030-bisect-porcelain.sh
> +++ b/t/t6030-bisect-porcelain.sh
> @@ -407,7 +407,7 @@ test_expect_success 'good merge base when good and bad are siblings' '
>         test_i18ngrep "merge base must be tested" my_bisect_log.txt &&
>         grep $HASH4 my_bisect_log.txt &&
>         git bisect good > my_bisect_log.txt &&
> -       test_must_fail grep "merge base must be tested" my_bisect_log.txt &&
> +       ! grep "merge base must be tested" my_bisect_log.txt &&
>         grep $HASH6 my_bisect_log.txt &&
>         git bisect reset
>  '
> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
> index 63d36fb28..0fe7e58cf 100755
> --- a/t/t7610-mergetool.sh
> +++ b/t/t7610-mergetool.sh
> @@ -602,7 +602,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
>         test_config mergetool.myecho.trustExitCode true &&
>         test_must_fail git merge master &&
>         git mergetool --no-prompt --tool myecho -- both >actual &&
> -       test_must_fail grep ^\./both_LOCAL_ actual >/dev/null &&
> +       ! grep ^\./both_LOCAL_ actual >/dev/null &&
>         grep /both_LOCAL_ actual >/dev/null &&
>         git reset --hard master >/dev/null 2>&1
>  '
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 3dc4a3454..0f398dd16 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -50,7 +50,7 @@ test_no_confirm () {
>                 --smtp-server="$(pwd)/fake.sendmail" \
>                 $@ \
>                 $patches >stdout &&
> -               test_must_fail grep "Send this email" stdout &&
> +               ! grep "Send this email" stdout &&
>                 >no_confirm_okay
>  }
>
> diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
> index 69a675052..044f65e91 100755
> --- a/t/t9117-git-svn-init-clone.sh
> +++ b/t/t9117-git-svn-init-clone.sh
> @@ -55,7 +55,7 @@ test_expect_success 'clone to target directory with --stdlayout' '
>  test_expect_success 'init without -s/-T/-b/-t does not warn' '
>         test ! -d trunk &&
>         git svn init "$svnrepo"/project/trunk trunk 2>warning &&
> -       test_must_fail grep -q prefix warning &&
> +       ! grep -q prefix warning &&
>         rm -rf trunk &&
>         rm -f warning
>         '
> @@ -63,7 +63,7 @@ test_expect_success 'init without -s/-T/-b/-t does not warn' '
>  test_expect_success 'clone without -s/-T/-b/-t does not warn' '
>         test ! -d trunk &&
>         git svn clone "$svnrepo"/project/trunk 2>warning &&
> -       test_must_fail grep -q prefix warning &&
> +       ! grep -q prefix warning &&
>         rm -rf trunk &&
>         rm -f warning
>         '
> @@ -86,7 +86,7 @@ EOF
>  test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '
>         test ! -d project &&
>         git svn init -s "$svnrepo"/project project 2>warning &&
> -       test_must_fail grep -q prefix warning &&
> +       ! grep -q prefix warning &&
>         test_svn_configured_prefix "origin/" &&
>         rm -rf project &&
>         rm -f warning
> @@ -95,7 +95,7 @@ test_expect_success 'init with -s/-T/-b/-t assumes --prefix=origin/' '
>  test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
>         test ! -d project &&
>         git svn clone -s "$svnrepo"/project 2>warning &&
> -       test_must_fail grep -q prefix warning &&
> +       ! grep -q prefix warning &&
>         test_svn_configured_prefix "origin/" &&
>         rm -rf project &&
>         rm -f warning
> @@ -104,7 +104,7 @@ test_expect_success 'clone with -s/-T/-b/-t assumes --prefix=origin/' '
>  test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
>         test ! -d project &&
>         git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
> -       test_must_fail grep -q prefix warning &&
> +       ! grep -q prefix warning &&
>         test_svn_configured_prefix "" &&
>         rm -rf project &&
>         rm -f warning
> @@ -113,7 +113,7 @@ test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
>  test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
>         test ! -d project &&
>         git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
> -       test_must_fail grep -q prefix warning &&
> +       ! grep -q prefix warning &&
>         test_svn_configured_prefix "" &&
>         rm -rf project &&
>         rm -f warning
> diff --git a/t/t9813-git-p4-preserve-users.sh b/t/t9813-git-p4-preserve-users.sh
> index 0fe231280..2384535a7 100755
> --- a/t/t9813-git-p4-preserve-users.sh
> +++ b/t/t9813-git-p4-preserve-users.sh
> @@ -126,13 +126,13 @@ test_expect_success 'not preserving user with mixed authorship' '
>                 grep "git author charlie@example.com does not match" &&
>
>                 make_change_by_user usernamefile3 alice alice@example.com &&
> -               git p4 commit |\
> -               test_must_fail grep "git author.*does not match" &&
> +               ! git p4 commit |\
> +               grep "git author.*does not match" &&

Would it be clearer to use this?

    git p4 commit |\
    grep -q -v "git author.*does not match" &&

With your original change, I think that if "git p4 commit" fails, then
that expression will be treated as a pass. What we want is for "git p4
commit" to pass, but the string to be missing.

(I would have used "--invert-match" rather than "-v", but it seems
that's not supported on Solaris).

Luke

^ permalink raw reply

* Re: [PATCH v3 00/23] Delete directories left empty after ref deletion
From: Philip Oakley @ 2017-01-01 12:43 UTC (permalink / raw)
  To: Jacob Keller, Junio C Hamano
  Cc: Jeff King, Michael Haggerty, Git mailing list, David Turner
In-Reply-To: <CA+P7+xqqVFvDKSCTrGVVdpZB_VHwGdZ3gFQzo+RQqCu0FpOsAQ@mail.gmail.com>

From: "Jacob Keller" <jacob.keller@gmail.com>
Sent: Sunday, January 01, 2017 9:24 AM
> On Sat, Dec 31, 2016 at 6:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:
>>
>>> On Sat, Dec 31, 2016 at 04:12:40AM +0100, Michael Haggerty wrote:
>>>
>>>> This is a re-roll of an old patch series. v1 [1] got some feedback,
>>>> which I think was all addressed in v2 [2]. But it seems that v2 fell
>>>> on the floor, and I didn't bother following up because it was in the
>>>> same area of code that was undergoing heavy changes due to the
>>>> pluggable reference backend work. Sorry for the long delay before
>>>> getting back to it.
>>>
>>> I've read through the whole thing, and aside from a few very minor nits
>>> (that I am not even sure are worth a re-roll), I didn't see anything
>>> wrong. And the overall goal and approach seem obviously sound.
>>>
>>>> Michael Haggerty (23):
>>>
>>> I'll admit to being daunted by the number of patches, but it was quite a
>>> pleasant and easy read. Thanks.
>>>
>>> -Peff
>>
>> Thanks, both.  These patches indeed were pleasant.
>
> I do have one comment regarding this series. Is it ever possible for
> an older version of git to be running a process while a new version of
> git which cleans up dirs runs? Is this expected? I just want to make
> sure we don't need to worry about that scenario since otherwise it
> makes it much more challenge.

It is easily possible in the Windows environment where the install philosphy 
is different, and some external vendor tools may even bring in their own 
copy of Git as well. There is also the Portable Git version, so the 
possibility of multiple versions running concurrently is there, though it is 
on Windows...

I certainly have a Git-for-Windows published version, and a recent SDK 
version on my home machines.

>
> My thought as far as I understand it is that it is possible, because a
> user COULD choose to run both this and an older version, but that it
> is unlikely in practice outside of a few developer boxes who
> periodically switch between versions of git, and are unlikely to
> actually run multiple versions at exactly the same time.
>
> Thanks,
> Jake
>
Philip 


^ permalink raw reply

* Re: [PATCH v3 13/23] log_ref_setup(): pass the open file descriptor back to the caller
From: Junio C Hamano @ 2017-01-01 10:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Michael Haggerty, git, David Turner
In-Reply-To: <20161231175808.cvm54nmk3x7zoipo@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sat, Dec 31, 2016 at 08:58:43AM +0100, Michael Haggerty wrote:
>
>> > The return value is always "0" or "-1". It seems like it would be
>> > simpler to just return the descriptor instead of 0.
>> > 
>> > I guess that makes it hard to identify the case when we chose not to
>> > create a descriptor. I wonder if more "normal" semantics would be:
>> > 
>> >   1. ret >= 0: file existed or was created, and ret is the descriptor
>> > 
>> >   2. ret < 0, err is empty: we chose not to create
>> > 
>> >   3. ret < 0, err is non-empty: a real error
>> 
>> I don't like making callers read err to find out whether the call was
>> successful, and I think we've been able to avoid that pattern so far.
>
> I guess my mental model is that case 2 _is_ a failure, because we didn't
> open the reflog. It's just one that callers may want to distinguish from
> case 3, because it's probably a silent failure, not one we want to
> complain to the user about.
>
> But whether that's accurate would depend on the callers. Looking at the
> callers, I think the immediate callers would be happier with this, but
> you probably would want to end up converting case 3 back to "return 0"
> out of files_log_ref_write().
>
>> > I dunno. This may just be bikeshedding, and I can live with it either
>> > way (especially because you documented it!).
>> 
>> Let's see if anybody has a strong opinion about it; otherwise I'd rather
>> leave it as is.
>
> Sounds good.

FWIW, in my mental model, 2. is not a failure ("we returned without
creating new log because that is what was asked by the user").  A
true failure case is "we wanted to open but couldn't".

The caller needs to be able to differentiate between these two cases
because we get no usable fd out of the function in either case.

I agree that we could cram the error status and the file descriptor
into a single return value as you two discussed, but I think what
the patch chose to do is easier to use from the callers' point of
view.  The caller can switch between the codepath to give an error
message and the non-error codepath based on the return value, and in
the non-error codepath can choose what to do based on the value of
logfd.

I do not mind "all negative values mean there is no fd" plus "some
negative values are more special than others" convention, and if a
patch did that from the beginning, I certainly would not suggest to
rewrite it to use the "error status comes as a return value, file
descriptor is an out parameter" convention; i.e. I personally do not
see much difference either way, so...

^ permalink raw reply

* Re: Test failures when Git is built with libpcre and grep is built without it
From: Torsten Bögershausen @ 2017-01-01 10:19 UTC (permalink / raw)
  To: A. Wilcox, git
In-Reply-To: <58688C9F.4000605@adelielinux.org>

On 01.01.17 05:59, A. Wilcox wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Hello!
> 
> I'm attempting to package Git for our new Linux distribution and I
> have run in to a failure on our PowerPC builder while running the test
> suite.
> 
> The PowerPC builder runs a tiny version of grep(1) that was not built
> with PCRE.  As such, grep -P returns 2 and prints:
> 
> grep: support for the -P option is not compiled into this
> - --disable-perl-regexp binary
> 
> However, our Git build *does* link against libpcre.  This causes a
> tests numbered 142 and 143 to fail in t7810-grep.sh.
> 
> I am not sure the best way to handle this but I felt it would be
> prudent to inform you of this issue.  I will be happy to provide any
> other information you may require.

The first thing you can do is to run the test with debug and verbose:

debug=t verbose=t ./t7810-grep.sh  
and post the output of these 2 test cases here:
(mine looks like this)

expecting success: 
	echo "ab:a+bc" >expected &&
	git \
		-c grep.patterntype=extended \
		-c grep.patterntype=fixed \
		-c grep.patterntype=basic \
		grep "a+b*c" ab >actual &&
	test_cmp expected actual

ok 142 - grep pattern with grep.patternType=extended, =fixed, =basic

expecting success: 
	echo "ab:abc" >expected &&
	git grep -F -G -E "a+b*c" ab >actual &&
	test_cmp expected actual

ok 143 - grep -F -G -E pattern


^ permalink raw reply

* Re: [PATCH v3 00/23] Delete directories left empty after ref deletion
From: Jacob Keller @ 2017-01-01  9:26 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Michael Haggerty, Git mailing list, David Turner
In-Reply-To: <CA+P7+xqqVFvDKSCTrGVVdpZB_VHwGdZ3gFQzo+RQqCu0FpOsAQ@mail.gmail.com>

On Sun, Jan 1, 2017 at 1:24 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Sat, Dec 31, 2016 at 6:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:
>>
>>> On Sat, Dec 31, 2016 at 04:12:40AM +0100, Michael Haggerty wrote:
>>>
>>>> This is a re-roll of an old patch series. v1 [1] got some feedback,
>>>> which I think was all addressed in v2 [2]. But it seems that v2 fell
>>>> on the floor, and I didn't bother following up because it was in the
>>>> same area of code that was undergoing heavy changes due to the
>>>> pluggable reference backend work. Sorry for the long delay before
>>>> getting back to it.
>>>
>>> I've read through the whole thing, and aside from a few very minor nits
>>> (that I am not even sure are worth a re-roll), I didn't see anything
>>> wrong. And the overall goal and approach seem obviously sound.
>>>
>>>> Michael Haggerty (23):
>>>
>>> I'll admit to being daunted by the number of patches, but it was quite a
>>> pleasant and easy read. Thanks.
>>>
>>> -Peff
>>
>> Thanks, both.  These patches indeed were pleasant.
>
> I do have one comment regarding this series. Is it ever possible for
> an older version of git to be running a process while a new version of
> git which cleans up dirs runs? Is this expected? I just want to make
> sure we don't need to worry about that scenario since otherwise it
> makes it much more challenge.
>
> My thought as far as I understand it is that it is possible, because a
> user COULD choose to run both this and an older version, but that it
> is unlikely in practice outside of a few developer boxes who
> periodically switch between versions of git, and are unlikely to
> actually run multiple versions at exactly the same time.
>
> Thanks,
> Jake

To add to this, if it is possible, it might be worth merging the "make
ourselves safer against a race" first, and then waiting some time
before merging the "we are now safe to delete directories". I am not
yet convinced that it is necessary, but wanted to point it out so that
someone more knowledgeable could explain why it is safe to do so.

Regards,
Jake

^ permalink raw reply

* Re: [PATCH v3 00/23] Delete directories left empty after ref deletion
From: Jacob Keller @ 2017-01-01  9:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Michael Haggerty, Git mailing list, David Turner
In-Reply-To: <xmqqr34n4ii8.fsf@gitster.mtv.corp.google.com>

On Sat, Dec 31, 2016 at 6:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> On Sat, Dec 31, 2016 at 04:12:40AM +0100, Michael Haggerty wrote:
>>
>>> This is a re-roll of an old patch series. v1 [1] got some feedback,
>>> which I think was all addressed in v2 [2]. But it seems that v2 fell
>>> on the floor, and I didn't bother following up because it was in the
>>> same area of code that was undergoing heavy changes due to the
>>> pluggable reference backend work. Sorry for the long delay before
>>> getting back to it.
>>
>> I've read through the whole thing, and aside from a few very minor nits
>> (that I am not even sure are worth a re-roll), I didn't see anything
>> wrong. And the overall goal and approach seem obviously sound.
>>
>>> Michael Haggerty (23):
>>
>> I'll admit to being daunted by the number of patches, but it was quite a
>> pleasant and easy read. Thanks.
>>
>> -Peff
>
> Thanks, both.  These patches indeed were pleasant.

I do have one comment regarding this series. Is it ever possible for
an older version of git to be running a process while a new version of
git which cleans up dirs runs? Is this expected? I just want to make
sure we don't need to worry about that scenario since otherwise it
makes it much more challenge.

My thought as far as I understand it is that it is possible, because a
user COULD choose to run both this and an older version, but that it
is unlikely in practice outside of a few developer boxes who
periodically switch between versions of git, and are unlikely to
actually run multiple versions at exactly the same time.

Thanks,
Jake

^ permalink raw reply

* Re: [PATCH v3 11/23] log_ref_setup(): separate code for create vs non-create
From: Michael Haggerty @ 2017-01-01  8:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, David Turner
In-Reply-To: <xmqqful34fww.fsf@gitster.mtv.corp.google.com>

On 01/01/2017 04:28 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> +			if (errno == ENOENT || errno == EISDIR) {
>> +				/*
>> +				 * The logfile doesn't already exist,
>> +				 * but that is not an error; it only
>> +				 * means that we won't write log
>> +				 * entries to it.
>> +				 */
>> +			} else {
> 
> It may be valid C, but an
> 
> 	{
> 		/*
> 		 * an empty block without any statement,
> 		 * not even a null statement.
> 		 */
> 	}
> 
> always makes me a bit nervous.  Have a line with a semicolon without
> anything else (other than the indent) at the end and it will read
> nicer, at least to me.

That's a form of superstition that I haven't encountered before ;-) but
I'll change it.

Michael


^ permalink raw reply

* Re: Rebasing multiple branches at once
From: Johannes Sixt @ 2017-01-01  8:42 UTC (permalink / raw)
  To: Mike Hommey; +Cc: git, Johannes Schindelin
In-Reply-To: <20161231081433.3zo6lrsjsu2qho4u@glandium.org>

Am 31.12.2016 um 09:14 schrieb Mike Hommey:
> Hi,
>
> I've had this kind of things to do more than once, and had to do it a
> lot today, so I figured it would be worth discussing whether git-rebase
> should be enhanced to support this, or if this should go in a separate
> tool or whatever.
>
> So here is what I'm trying to do in a not-too painful way:
>
> I'm starting with something like this:
> A---B---C---D---E
>             \---F
>
> where A is master, and E and F are two local topics with a common set of
> things on top of master.
>
> The next thing that happens is that master is updated, and I want to
> rebase both topics on top of the new master.
>
> So I now have:
>
> A---G
> \---B---C---D---E
>             \---F
>
> If I do the dumb thing, which is to do `git rebase master E` and `git
> rebase master F`, I end up with:
>
> A---G---B'---C'---D'---E'
>     \---B"---C"---D"---F'
>
> That is, I just lost the fast that E and F had common history.

Git garden shears, perhaps?

https://public-inbox.org/git/alpine.DEB.2.20.1609111027330.129229@virtualbox/

-- Hannes


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox