* [PATCH v2 0/8] object_id part 4
@ 2016-06-18 22:13 brian m. carlson
2016-06-18 22:14 ` [PATCH v2 1/8] Add basic Coccinelle transforms brian m. carlson
` (9 more replies)
0 siblings, 10 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:13 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
This series is part 4 in a series of conversions to replace instances of
unsigned char [20] with struct object_id. Most of this series touches
the merge-recursive code.
New in this series is the use of Coccinelle (http://coccinelle.lip6.fr/)
semantic patches. These semantic patches can make automatic
transformations to C source code for cleanup or refactoring reasons.
This series introduces a set of transforms for the struct object_id
transition, cleans up some existing code with them, and applies a small
number of semantic patches to transform parts of the merge-recursive
code. Some manual refactoring work follows.
Note that in the patches created with the semantic patches, the only manual
change was the definition of the struct member. Opinions on whether this is a
viable technique for further work to ease both the creation and review of
patches are of course welcomed.
The testsuite continues to pass at each step, and this series rebases
cleanly on both pu and next.
I moved the Coccinelle transforms to contrib/examples/coccinelle, but if
it's decided that the Coccinelle transforms simply don't belong in the
repository, it's fine to simply drop the first patch (and maybe fix up
the commit messages). I can create a GitHub Gist and let reviewers
refer to that at their convenience.
Changes from v1:
* Move the object ID transformations to contrib/examples/coccinelle.
* Add a README to that folder explaining what they are.
* Adjust the Coccinelle patches to transform plain structs before
pointers to structs to avoid misconversions. This addresses the issue
that Peff caught originally.
brian m. carlson (8):
Add basic Coccinelle transforms.
Apply object_id Coccinelle transformations.
Convert struct diff_filespec to struct object_id
Rename struct diff_filespec's sha1_valid member.
merge-recursive: convert struct stage_data to use object_id
merge-recursive: convert struct merge_file_info to object_id
merge-recursive: convert leaf functions to use struct object_id
merge-recursive: convert merge_recursive_generic to object_id
bisect.c | 2 +-
builtin/blame.c | 6 +-
builtin/fast-export.c | 10 +-
builtin/merge-recursive.c | 20 +-
builtin/merge.c | 13 +-
builtin/reset.c | 4 +-
combine-diff.c | 14 +-
contrib/examples/coccinelle/README | 2 +
contrib/examples/coccinelle/object_id.cocci | 83 ++++++++
diff.c | 95 +++++----
diffcore-break.c | 4 +-
diffcore-rename.c | 16 +-
diffcore.h | 4 +-
line-log.c | 12 +-
merge-recursive.c | 310 ++++++++++++++--------------
merge-recursive.h | 6 +-
notes-merge.c | 42 ++--
refs/files-backend.c | 4 +-
submodule.c | 4 +-
wt-status.c | 3 +-
20 files changed, 378 insertions(+), 276 deletions(-)
create mode 100644 contrib/examples/coccinelle/README
create mode 100644 contrib/examples/coccinelle/object_id.cocci
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/8] Add basic Coccinelle transforms.
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 2/8] Apply object_id Coccinelle transformations brian m. carlson
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Coccinelle (http://coccinelle.lip6.fr/) is a program which performs
mechanical transformations on C programs using semantic patches. These
semantic patches can be used to implement automatic refactoring and
maintenance tasks.
Add a set of basic semantic patches to convert common patterns related
to the struct object_id transformation, as well as a README.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
contrib/examples/coccinelle/README | 2 +
contrib/examples/coccinelle/object_id.cocci | 83 +++++++++++++++++++++++++++++
2 files changed, 85 insertions(+)
create mode 100644 contrib/examples/coccinelle/README
create mode 100644 contrib/examples/coccinelle/object_id.cocci
diff --git a/contrib/examples/coccinelle/README b/contrib/examples/coccinelle/README
new file mode 100644
index 00000000..9c2f8879
--- /dev/null
+++ b/contrib/examples/coccinelle/README
@@ -0,0 +1,2 @@
+This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)
+semantic patches that might be useful to developers.
diff --git a/contrib/examples/coccinelle/object_id.cocci b/contrib/examples/coccinelle/object_id.cocci
new file mode 100644
index 00000000..0f068252
--- /dev/null
+++ b/contrib/examples/coccinelle/object_id.cocci
@@ -0,0 +1,83 @@
+@@
+expression E1;
+@@
+- is_null_sha1(E1.hash)
++ is_null_oid(&E1)
+
+@@
+expression E1;
+@@
+- is_null_sha1(E1->hash)
++ is_null_oid(E1)
+
+@@
+expression E1;
+@@
+- sha1_to_hex(E1.hash)
++ oid_to_hex(&E1)
+
+@@
+expression E1;
+@@
+- sha1_to_hex(E1->hash)
++ oid_to_hex(E1)
+
+@@
+expression E1;
+@@
+- hashclr(E1.hash)
++ oidclr(&E1)
+
+@@
+expression E1;
+@@
+- hashclr(E1->hash)
++ oidclr(E1)
+
+@@
+expression E1, E2;
+@@
+- hashcmp(E1.hash, E2.hash)
++ oidcmp(&E1, &E2)
+
+@@
+expression E1, E2;
+@@
+- hashcmp(E1->hash, E2->hash)
++ oidcmp(E1, E2)
+
+@@
+expression E1, E2;
+@@
+- hashcmp(E1->hash, E2.hash)
++ oidcmp(E1, &E2)
+
+@@
+expression E1, E2;
+@@
+- hashcmp(E1.hash, E2->hash)
++ oidcmp(&E1, E2)
+
+@@
+expression E1, E2;
+@@
+- hashcpy(E1.hash, E2.hash)
++ oidcpy(&E1, &E2)
+
+@@
+expression E1, E2;
+@@
+- hashcpy(E1->hash, E2->hash)
++ oidcpy(E1, E2)
+
+@@
+expression E1, E2;
+@@
+- hashcpy(E1->hash, E2.hash)
++ oidcpy(E1, &E2)
+
+@@
+expression E1, E2;
+@@
+- hashcpy(E1.hash, E2->hash)
++ oidcpy(&E1, E2)
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/8] Apply object_id Coccinelle transformations.
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
2016-06-18 22:14 ` [PATCH v2 1/8] Add basic Coccinelle transforms brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-21 21:36 ` Junio C Hamano
2016-06-18 22:14 ` [PATCH v2 3/8] Convert struct diff_filespec to struct object_id brian m. carlson
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Apply the set of semantic patches from contrib/examples/coccinelle to
convert some leftover places using struct object_id's hash member to
instead use the wrapper functions that take struct object_id natively.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
bisect.c | 2 +-
builtin/merge.c | 13 ++++++-------
refs/files-backend.c | 4 ++--
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/bisect.c b/bisect.c
index 6d93edbc..ff147589 100644
--- a/bisect.c
+++ b/bisect.c
@@ -754,7 +754,7 @@ static void handle_bad_merge_base(void)
static void handle_skipped_merge_base(const unsigned char *mb)
{
char *mb_hex = sha1_to_hex(mb);
- char *bad_hex = sha1_to_hex(current_bad_oid->hash);
+ char *bad_hex = oid_to_hex(current_bad_oid);
char *good_hex = join_sha1_array_hex(&good_revs, ' ');
warning("the merge base between %s and [%s] "
diff --git a/builtin/merge.c b/builtin/merge.c
index b555a1bf..1e7be852 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -501,7 +501,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
if (ref_exists(truname.buf)) {
strbuf_addf(msg,
"%s\t\tbranch '%s'%s of .\n",
- sha1_to_hex(remote_head->object.oid.hash),
+ oid_to_hex(&remote_head->object.oid),
truname.buf + 11,
(early ? " (early part)" : ""));
strbuf_release(&truname);
@@ -515,7 +515,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
desc = merge_remote_util(remote_head);
if (desc && desc->obj && desc->obj->type == OBJ_TAG) {
strbuf_addf(msg, "%s\t\t%s '%s'\n",
- sha1_to_hex(desc->obj->oid.hash),
+ oid_to_hex(&desc->obj->oid),
typename(desc->obj->type),
remote);
goto cleanup;
@@ -523,7 +523,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
}
strbuf_addf(msg, "%s\t\tcommit '%s'\n",
- sha1_to_hex(remote_head->object.oid.hash), remote);
+ oid_to_hex(&remote_head->object.oid), remote);
cleanup:
strbuf_release(&buf);
strbuf_release(&bname);
@@ -1366,7 +1366,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
for (p = remoteheads; p; p = p->next) {
struct commit *commit = p->item;
strbuf_addf(&buf, "GITHEAD_%s",
- sha1_to_hex(commit->object.oid.hash));
+ oid_to_hex(&commit->object.oid));
setenv(buf.buf, merge_remote_util(commit)->name, 1);
strbuf_reset(&buf);
if (fast_forward != FF_ONLY &&
@@ -1425,7 +1425,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
goto done;
} else if (fast_forward != FF_NO && !remoteheads->next &&
!common->next &&
- !hashcmp(common->item->object.oid.hash, head_commit->object.oid.hash)) {
+ !oidcmp(&common->item->object.oid, &head_commit->object.oid)) {
/* Again the most common case of merging one remote. */
struct strbuf msg = STRBUF_INIT;
struct commit *commit;
@@ -1499,8 +1499,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* HEAD^^" would be missed.
*/
common_one = get_merge_bases(head_commit, j->item);
- if (hashcmp(common_one->item->object.oid.hash,
- j->item->object.oid.hash)) {
+ if (oidcmp(&common_one->item->object.oid, &j->item->object.oid)) {
up_to_date = 0;
break;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1f380764..dac3a222 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1725,14 +1725,14 @@ static int verify_lock(struct ref_lock *lock,
errno = save_errno;
return -1;
} else {
- hashclr(lock->old_oid.hash);
+ oidclr(&lock->old_oid);
return 0;
}
}
if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
strbuf_addf(err, "ref %s is at %s but expected %s",
lock->ref_name,
- sha1_to_hex(lock->old_oid.hash),
+ oid_to_hex(&lock->old_oid),
sha1_to_hex(old_sha1));
errno = EBUSY;
return -1;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/8] Convert struct diff_filespec to struct object_id
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
2016-06-18 22:14 ` [PATCH v2 1/8] Add basic Coccinelle transforms brian m. carlson
2016-06-18 22:14 ` [PATCH v2 2/8] Apply object_id Coccinelle transformations brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-21 22:22 ` Junio C Hamano
2016-06-18 22:14 ` [PATCH v2 4/8] Rename struct diff_filespec's sha1_valid member brian m. carlson
` (6 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Convert struct diff_filespec's sha1 member to use a struct object_id
called "oid" instead. The following Coccinelle semantic patch was used
to implement this, followed by the transformations in object_id.cocci:
@@
struct diff_filespec o;
@@
- o.sha1
+ o.oid.hash
@@
struct diff_filespec *p;
@@
- p->sha1
+ p->oid.hash
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/blame.c | 6 +--
builtin/fast-export.c | 10 ++---
builtin/reset.c | 4 +-
combine-diff.c | 10 ++---
diff.c | 69 +++++++++++++++++---------------
diffcore-break.c | 2 +-
diffcore-rename.c | 14 ++++---
diffcore.h | 2 +-
line-log.c | 2 +-
merge-recursive.c | 107 +++++++++++++++++++++++++++-----------------------
notes-merge.c | 42 ++++++++++----------
submodule.c | 4 +-
wt-status.c | 3 +-
13 files changed, 147 insertions(+), 128 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 21f42b0b..04bc4e0e 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -599,7 +599,7 @@ static struct origin *find_origin(struct scoreboard *sb,
p->status);
case 'M':
porigin = get_origin(sb, parent, origin->path);
- hashcpy(porigin->blob_sha1, p->one->sha1);
+ hashcpy(porigin->blob_sha1, p->one->oid.hash);
porigin->mode = p->one->mode;
break;
case 'A':
@@ -645,7 +645,7 @@ static struct origin *find_rename(struct scoreboard *sb,
if ((p->status == 'R' || p->status == 'C') &&
!strcmp(p->two->path, origin->path)) {
porigin = get_origin(sb, parent, p->one->path);
- hashcpy(porigin->blob_sha1, p->one->sha1);
+ hashcpy(porigin->blob_sha1, p->one->oid.hash);
porigin->mode = p->one->mode;
break;
}
@@ -1309,7 +1309,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
continue;
norigin = get_origin(sb, parent, p->one->path);
- hashcpy(norigin->blob_sha1, p->one->sha1);
+ hashcpy(norigin->blob_sha1, p->one->oid.hash);
norigin->mode = p->one->mode;
fill_origin_blob(&sb->revs->diffopt, norigin, &file_p);
if (!file_p.ptr)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 8164b581..c0652a7e 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -368,7 +368,7 @@ static void show_filemodify(struct diff_queue_struct *q,
print_path(spec->path);
putchar('\n');
- if (!hashcmp(ospec->sha1, spec->sha1) &&
+ if (!oidcmp(&ospec->oid, &spec->oid) &&
ospec->mode == spec->mode)
break;
/* fallthrough */
@@ -383,10 +383,10 @@ static void show_filemodify(struct diff_queue_struct *q,
if (no_data || S_ISGITLINK(spec->mode))
printf("M %06o %s ", spec->mode,
sha1_to_hex(anonymize ?
- anonymize_sha1(spec->sha1) :
- spec->sha1));
+ anonymize_sha1(spec->oid.hash) :
+ spec->oid.hash));
else {
- struct object *object = lookup_object(spec->sha1);
+ struct object *object = lookup_object(spec->oid.hash);
printf("M %06o :%d ", spec->mode,
get_object_mark(object));
}
@@ -572,7 +572,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
/* Export the referenced blobs, and remember the marks. */
for (i = 0; i < diff_queued_diff.nr; i++)
if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))
- export_blob(diff_queued_diff.queue[i]->two->sha1);
+ export_blob(diff_queued_diff.queue[i]->two->oid.hash);
refname = commit->util;
if (anonymize) {
diff --git a/builtin/reset.c b/builtin/reset.c
index 092c3a53..cce64b53 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -121,7 +121,7 @@ static void update_index_from_diff(struct diff_queue_struct *q,
for (i = 0; i < q->nr; i++) {
struct diff_filespec *one = q->queue[i]->one;
- int is_missing = !(one->mode && !is_null_sha1(one->sha1));
+ int is_missing = !(one->mode && !is_null_oid(&one->oid));
struct cache_entry *ce;
if (is_missing && !intent_to_add) {
@@ -129,7 +129,7 @@ static void update_index_from_diff(struct diff_queue_struct *q,
continue;
}
- ce = make_cache_entry(one->mode, one->sha1, one->path,
+ ce = make_cache_entry(one->mode, one->oid.hash, one->path,
0, 0);
if (!ce)
die(_("make_cache_entry failed for path '%s'"),
diff --git a/combine-diff.c b/combine-diff.c
index 8f2313d5..3537209c 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -44,9 +44,9 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
memset(p->parent, 0,
sizeof(p->parent[0]) * num_parent);
- hashcpy(p->oid.hash, q->queue[i]->two->sha1);
+ oidcpy(&p->oid, &q->queue[i]->two->oid);
p->mode = q->queue[i]->two->mode;
- hashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);
+ oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);
p->parent[n].mode = q->queue[i]->one->mode;
p->parent[n].status = q->queue[i]->status;
*tail = p;
@@ -77,7 +77,7 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,
continue;
}
- hashcpy(p->parent[n].oid.hash, q->queue[i]->one->sha1);
+ oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid);
p->parent[n].mode = q->queue[i]->one->mode;
p->parent[n].status = q->queue[i]->status;
@@ -1268,7 +1268,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,
for (i = 0; i < num_parent; i++) {
pair->one[i].path = p->path;
pair->one[i].mode = p->parent[i].mode;
- hashcpy(pair->one[i].sha1, p->parent[i].oid.hash);
+ oidcpy(&pair->one[i].oid, &p->parent[i].oid);
pair->one[i].sha1_valid = !is_null_oid(&p->parent[i].oid);
pair->one[i].has_more_entries = 1;
}
@@ -1276,7 +1276,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,
pair->two->path = p->path;
pair->two->mode = p->mode;
- hashcpy(pair->two->sha1, p->oid.hash);
+ oidcpy(&pair->two->oid, &p->oid);
pair->two->sha1_valid = !is_null_oid(&p->oid);
return pair;
}
diff --git a/diff.c b/diff.c
index fa78fc18..5a6d8654 100644
--- a/diff.c
+++ b/diff.c
@@ -1934,7 +1934,7 @@ static void show_dirstat(struct diff_options *options)
name = p->two->path ? p->two->path : p->one->path;
if (p->one->sha1_valid && p->two->sha1_valid)
- content_changed = hashcmp(p->one->sha1, p->two->sha1);
+ content_changed = oidcmp(&p->one->oid, &p->two->oid);
else
content_changed = 1;
@@ -2306,7 +2306,8 @@ static void builtin_diff(const char *name_a,
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path : two->path,
line_prefix,
- one->sha1, two->sha1, two->dirty_submodule,
+ one->oid.hash, two->oid.hash,
+ two->dirty_submodule,
meta, del, add, reset);
return;
}
@@ -2384,7 +2385,7 @@ static void builtin_diff(const char *name_a,
if (!one->data && !two->data &&
S_ISREG(one->mode) && S_ISREG(two->mode) &&
!DIFF_OPT_TST(o, BINARY)) {
- if (!hashcmp(one->sha1, two->sha1)) {
+ if (!oidcmp(&one->oid, &two->oid)) {
if (must_show_header)
fprintf(o->file, "%s", header.buf);
goto free_ab_and_return;
@@ -2505,7 +2506,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
return;
}
- same_contents = !hashcmp(one->sha1, two->sha1);
+ same_contents = !oidcmp(&one->oid, &two->oid);
if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
data->is_binary = 1;
@@ -2638,7 +2639,7 @@ void fill_filespec(struct diff_filespec *spec, const unsigned char *sha1,
{
if (mode) {
spec->mode = canon_mode(mode);
- hashcpy(spec->sha1, sha1);
+ hashcpy(spec->oid.hash, sha1);
spec->sha1_valid = sha1_valid;
}
}
@@ -2721,7 +2722,8 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
if (s->dirty_submodule)
dirty = "-dirty";
- strbuf_addf(&buf, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty);
+ strbuf_addf(&buf, "Subproject commit %s%s\n",
+ oid_to_hex(&s->oid), dirty);
s->size = buf.len;
if (size_only) {
s->data = NULL;
@@ -2765,7 +2767,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
return diff_populate_gitlink(s, size_only);
if (!s->sha1_valid ||
- reuse_worktree_file(s->path, s->sha1, 0)) {
+ reuse_worktree_file(s->path, s->oid.hash, 0)) {
struct strbuf buf = STRBUF_INIT;
struct stat st;
int fd;
@@ -2822,9 +2824,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
else {
enum object_type type;
if (size_only || (flags & CHECK_BINARY)) {
- type = sha1_object_info(s->sha1, &s->size);
+ type = sha1_object_info(s->oid.hash, &s->size);
if (type < 0)
- die("unable to read %s", sha1_to_hex(s->sha1));
+ die("unable to read %s",
+ oid_to_hex(&s->oid));
if (size_only)
return 0;
if (s->size > big_file_threshold && s->is_binary == -1) {
@@ -2832,9 +2835,9 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
return 0;
}
}
- s->data = read_sha1_file(s->sha1, &type, &s->size);
+ s->data = read_sha1_file(s->oid.hash, &type, &s->size);
if (!s->data)
- die("unable to read %s", sha1_to_hex(s->sha1));
+ die("unable to read %s", oid_to_hex(&s->oid));
s->should_free = 1;
}
return 0;
@@ -2913,7 +2916,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (!S_ISGITLINK(one->mode) &&
(!one->sha1_valid ||
- reuse_worktree_file(name, one->sha1, 1))) {
+ reuse_worktree_file(name, one->oid.hash, 1))) {
struct stat st;
if (lstat(name, &st) < 0) {
if (errno == ENOENT)
@@ -2926,7 +2929,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
die_errno("readlink(%s)", name);
prep_temp_blob(name, temp, sb.buf, sb.len,
(one->sha1_valid ?
- one->sha1 : null_sha1),
+ one->oid.hash : null_sha1),
(one->sha1_valid ?
one->mode : S_IFLNK));
strbuf_release(&sb);
@@ -2937,7 +2940,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (!one->sha1_valid)
sha1_to_hex_r(temp->hex, null_sha1);
else
- sha1_to_hex_r(temp->hex, one->sha1);
+ sha1_to_hex_r(temp->hex, one->oid.hash);
/* Even though we may sometimes borrow the
* contents from the work tree, we always want
* one->mode. mode is trustworthy even when
@@ -2952,7 +2955,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (diff_populate_filespec(one, 0))
die("cannot read data blob for %s", one->path);
prep_temp_blob(name, temp, one->data, one->size,
- one->sha1, one->mode);
+ one->oid.hash, one->mode);
}
return temp;
}
@@ -3065,7 +3068,7 @@ static void fill_metainfo(struct strbuf *msg,
default:
*must_show_header = 0;
}
- if (one && two && hashcmp(one->sha1, two->sha1)) {
+ if (one && two && oidcmp(&one->oid, &two->oid)) {
int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
if (DIFF_OPT_TST(o, BINARY)) {
@@ -3075,8 +3078,8 @@ static void fill_metainfo(struct strbuf *msg,
abbrev = 40;
}
strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
- find_unique_abbrev(one->sha1, abbrev));
- strbuf_addstr(msg, find_unique_abbrev(two->sha1, abbrev));
+ find_unique_abbrev(one->oid.hash, abbrev));
+ strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
if (one->mode == two->mode)
strbuf_addf(msg, " %06o", one->mode);
strbuf_addf(msg, "%s\n", reset);
@@ -3134,17 +3137,17 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
if (!one->sha1_valid) {
struct stat st;
if (one->is_stdin) {
- hashcpy(one->sha1, null_sha1);
+ hashcpy(one->oid.hash, null_sha1);
return;
}
if (lstat(one->path, &st) < 0)
die_errno("stat '%s'", one->path);
- if (index_path(one->sha1, one->path, &st, 0))
+ if (index_path(one->oid.hash, one->path, &st, 0))
die("cannot hash %s", one->path);
}
}
else
- hashclr(one->sha1);
+ oidclr(&one->oid);
}
static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
@@ -4118,8 +4121,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
fprintf(opt->file, "%s", diff_line_prefix(opt));
if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,
- diff_unique_abbrev(p->one->sha1, opt->abbrev));
- fprintf(opt->file, "%s ", diff_unique_abbrev(p->two->sha1, opt->abbrev));
+ diff_unique_abbrev(p->one->oid.hash, opt->abbrev));
+ fprintf(opt->file, "%s ",
+ diff_unique_abbrev(p->two->oid.hash, opt->abbrev));
}
if (p->score) {
fprintf(opt->file, "%c%03d%c", p->status, similarity_index(p),
@@ -4169,7 +4173,7 @@ int diff_unmodified_pair(struct diff_filepair *p)
* dealing with a change.
*/
if (one->sha1_valid && two->sha1_valid &&
- !hashcmp(one->sha1, two->sha1) &&
+ !oidcmp(&one->oid, &two->oid) &&
!one->dirty_submodule && !two->dirty_submodule)
return 1; /* no change */
if (!one->sha1_valid && !two->sha1_valid)
@@ -4233,7 +4237,7 @@ void diff_debug_filespec(struct diff_filespec *s, int x, const char *one)
s->path,
DIFF_FILE_VALID(s) ? "valid" : "invalid",
s->mode,
- s->sha1_valid ? sha1_to_hex(s->sha1) : "");
+ s->sha1_valid ? oid_to_hex(&s->oid) : "");
fprintf(stderr, "queue[%d] %s size %lu\n",
x, one ? one : "",
s->size);
@@ -4303,11 +4307,11 @@ static void diff_resolve_rename_copy(void)
else
p->status = DIFF_STATUS_RENAMED;
}
- else if (hashcmp(p->one->sha1, p->two->sha1) ||
+ else if (oidcmp(&p->one->oid, &p->two->oid) ||
p->one->mode != p->two->mode ||
p->one->dirty_submodule ||
p->two->dirty_submodule ||
- is_null_sha1(p->one->sha1))
+ is_null_oid(&p->one->oid))
p->status = DIFF_STATUS_MODIFIED;
else {
/* This is a "no-change" entry and should not
@@ -4523,8 +4527,10 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
if (diff_filespec_is_binary(p->one) ||
diff_filespec_is_binary(p->two)) {
- git_SHA1_Update(&ctx, sha1_to_hex(p->one->sha1), 40);
- git_SHA1_Update(&ctx, sha1_to_hex(p->two->sha1), 40);
+ git_SHA1_Update(&ctx, oid_to_hex(&p->one->oid),
+ 40);
+ git_SHA1_Update(&ctx, oid_to_hex(&p->two->oid),
+ 40);
continue;
}
@@ -5113,7 +5119,8 @@ size_t fill_textconv(struct userdiff_driver *driver,
die("BUG: fill_textconv called with non-textconv driver");
if (driver->textconv_cache && df->sha1_valid) {
- *outbuf = notes_cache_get(driver->textconv_cache, df->sha1,
+ *outbuf = notes_cache_get(driver->textconv_cache,
+ df->oid.hash,
&size);
if (*outbuf)
return size;
@@ -5125,7 +5132,7 @@ size_t fill_textconv(struct userdiff_driver *driver,
if (driver->textconv_cache && df->sha1_valid) {
/* ignore errors, as we might be in a readonly repository */
- notes_cache_put(driver->textconv_cache, df->sha1, *outbuf,
+ notes_cache_put(driver->textconv_cache, df->oid.hash, *outbuf,
size);
/*
* we could save up changes and flush them all at the end,
diff --git a/diffcore-break.c b/diffcore-break.c
index 5473493e..a3e79608 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -58,7 +58,7 @@ static int should_break(struct diff_filespec *src,
}
if (src->sha1_valid && dst->sha1_valid &&
- !hashcmp(src->sha1, dst->sha1))
+ !oidcmp(&src->oid, &dst->oid))
return 0; /* they are the same */
if (diff_populate_filespec(src, 0) || diff_populate_filespec(dst, 0))
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 7f03eb5a..22b239a4 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -60,7 +60,8 @@ static int add_rename_dst(struct diff_filespec *two)
memmove(rename_dst + first + 1, rename_dst + first,
(rename_dst_nr - first - 1) * sizeof(*rename_dst));
rename_dst[first].two = alloc_filespec(two->path);
- fill_filespec(rename_dst[first].two, two->sha1, two->sha1_valid, two->mode);
+ fill_filespec(rename_dst[first].two, two->oid.hash, two->sha1_valid,
+ two->mode);
rename_dst[first].pair = NULL;
return 0;
}
@@ -263,9 +264,10 @@ static unsigned int hash_filespec(struct diff_filespec *filespec)
if (!filespec->sha1_valid) {
if (diff_populate_filespec(filespec, 0))
return 0;
- hash_sha1_file(filespec->data, filespec->size, "blob", filespec->sha1);
+ hash_sha1_file(filespec->data, filespec->size, "blob",
+ filespec->oid.hash);
}
- return sha1hash(filespec->sha1);
+ return sha1hash(filespec->oid.hash);
}
static int find_identical_files(struct hashmap *srcs,
@@ -287,7 +289,7 @@ static int find_identical_files(struct hashmap *srcs,
struct diff_filespec *source = p->filespec;
/* False hash collision? */
- if (hashcmp(source->sha1, target->sha1))
+ if (oidcmp(&source->oid, &target->oid))
continue;
/* Non-regular files? If so, the modes must match! */
if (!S_ISREG(source->mode) || !S_ISREG(target->mode)) {
@@ -466,7 +468,7 @@ void diffcore_rename(struct diff_options *options)
strcmp(options->single_follow, p->two->path))
continue; /* not interested */
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
- is_empty_blob_sha1(p->two->sha1))
+ is_empty_blob_sha1(p->two->oid.hash))
continue;
else if (add_rename_dst(p->two) < 0) {
warning("skipping rename detection, detected"
@@ -476,7 +478,7 @@ void diffcore_rename(struct diff_options *options)
}
}
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
- is_empty_blob_sha1(p->one->sha1))
+ is_empty_blob_sha1(p->one->oid.hash))
continue;
else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {
/*
diff --git a/diffcore.h b/diffcore.h
index 33ea2de3..c40e5b6c 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -25,7 +25,7 @@
struct userdiff_driver;
struct diff_filespec {
- unsigned char sha1[20];
+ struct object_id oid;
char *path;
void *data;
void *cnt_data;
diff --git a/line-log.c b/line-log.c
index bbe31ed6..02938639 100644
--- a/line-log.c
+++ b/line-log.c
@@ -520,7 +520,7 @@ static void fill_line_ends(struct diff_filespec *spec, long *lines,
char *data = NULL;
if (diff_populate_filespec(spec, 0))
- die("Cannot read blob %s", sha1_to_hex(spec->sha1));
+ die("Cannot read blob %s", oid_to_hex(&spec->oid));
ALLOC_ARRAY(ends, size);
ends[cur++] = 0;
diff --git a/merge-recursive.c b/merge-recursive.c
index 65cb5d6c..1e802097 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -134,11 +134,13 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,
int ostage2 = ostage1 ^ 1;
ci->ren1_other.path = pair1->one->path;
- hashcpy(ci->ren1_other.sha1, src_entry1->stages[ostage1].sha);
+ hashcpy(ci->ren1_other.oid.hash,
+ src_entry1->stages[ostage1].sha);
ci->ren1_other.mode = src_entry1->stages[ostage1].mode;
ci->ren2_other.path = pair2->one->path;
- hashcpy(ci->ren2_other.sha1, src_entry2->stages[ostage2].sha);
+ hashcpy(ci->ren2_other.oid.hash,
+ src_entry2->stages[ostage2].sha);
ci->ren2_other.mode = src_entry2->stages[ostage2].mode;
}
}
@@ -552,13 +554,13 @@ static int update_stages(const char *path, const struct diff_filespec *o,
if (remove_file_from_cache(path))
return -1;
if (o)
- if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))
+ if (add_cacheinfo(o->mode, o->oid.hash, path, 1, 0, options))
return -1;
if (a)
- if (add_cacheinfo(a->mode, a->sha1, path, 2, 0, options))
+ if (add_cacheinfo(a->mode, a->oid.hash, path, 2, 0, options))
return -1;
if (b)
- if (add_cacheinfo(b->mode, b->sha1, path, 3, 0, options))
+ if (add_cacheinfo(b->mode, b->oid.hash, path, 3, 0, options))
return -1;
return 0;
}
@@ -572,9 +574,9 @@ static void update_entry(struct stage_data *entry,
entry->stages[1].mode = o->mode;
entry->stages[2].mode = a->mode;
entry->stages[3].mode = b->mode;
- hashcpy(entry->stages[1].sha, o->sha1);
- hashcpy(entry->stages[2].sha, a->sha1);
- hashcpy(entry->stages[3].sha, b->sha1);
+ hashcpy(entry->stages[1].sha, o->oid.hash);
+ hashcpy(entry->stages[2].sha, a->oid.hash);
+ hashcpy(entry->stages[3].sha, b->oid.hash);
}
static int remove_file(struct merge_options *o, int clean,
@@ -871,9 +873,9 @@ static int merge_3way(struct merge_options *o,
name2 = mkpathdup("%s", branch2);
}
- read_mmblob(&orig, one->sha1);
- read_mmblob(&src1, a->sha1);
- read_mmblob(&src2, b->sha1);
+ read_mmblob(&orig, one->oid.hash);
+ read_mmblob(&src1, a->oid.hash);
+ read_mmblob(&src2, b->oid.hash);
merge_status = ll_merge(result_buf, a->path, &orig, base_name,
&src1, name1, &src2, name2, &ll_opts);
@@ -902,13 +904,13 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
result.clean = 0;
if (S_ISREG(a->mode)) {
result.mode = a->mode;
- hashcpy(result.sha, a->sha1);
+ hashcpy(result.sha, a->oid.hash);
} else {
result.mode = b->mode;
- hashcpy(result.sha, b->sha1);
+ hashcpy(result.sha, b->oid.hash);
}
} else {
- if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
+ if (!sha_eq(a->oid.hash, one->oid.hash) && !sha_eq(b->oid.hash, one->oid.hash))
result.merge = 1;
/*
@@ -924,10 +926,10 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
}
}
- if (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, one->sha1))
- hashcpy(result.sha, b->sha1);
- else if (sha_eq(b->sha1, one->sha1))
- hashcpy(result.sha, a->sha1);
+ if (sha_eq(a->oid.hash, b->oid.hash) || sha_eq(a->oid.hash, one->oid.hash))
+ hashcpy(result.sha, b->oid.hash);
+ else if (sha_eq(b->oid.hash, one->oid.hash))
+ hashcpy(result.sha, a->oid.hash);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
int merge_status;
@@ -947,13 +949,15 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
result.clean = (merge_status == 0);
} else if (S_ISGITLINK(a->mode)) {
result.clean = merge_submodule(result.sha,
- one->path, one->sha1,
- a->sha1, b->sha1,
+ one->path,
+ one->oid.hash,
+ a->oid.hash,
+ b->oid.hash,
!o->call_depth);
} else if (S_ISLNK(a->mode)) {
- hashcpy(result.sha, a->sha1);
+ hashcpy(result.sha, a->oid.hash);
- if (!sha_eq(a->sha1, b->sha1))
+ if (!sha_eq(a->oid.hash, b->oid.hash))
result.clean = 0;
} else {
die(_("unsupported object type in the tree"));
@@ -1000,11 +1004,11 @@ static struct merge_file_info merge_file_one(struct merge_options *o,
struct diff_filespec one, a, b;
one.path = a.path = b.path = (char *)path;
- hashcpy(one.sha1, o_sha);
+ hashcpy(one.oid.hash, o_sha);
one.mode = o_mode;
- hashcpy(a.sha1, a_sha);
+ hashcpy(a.oid.hash, a_sha);
a.mode = a_mode;
- hashcpy(b.sha1, b_sha);
+ hashcpy(b.oid.hash, b_sha);
b.mode = b_mode;
return merge_file_1(o, &one, &a, &b, branch1, branch2);
}
@@ -1079,16 +1083,16 @@ static void conflict_rename_delete(struct merge_options *o,
int b_mode = 0;
if (rename_branch == o->branch1) {
- a_sha = dest->sha1;
+ a_sha = dest->oid.hash;
a_mode = dest->mode;
} else {
- b_sha = dest->sha1;
+ b_sha = dest->oid.hash;
b_mode = dest->mode;
}
handle_change_delete(o,
o->call_depth ? orig->path : dest->path,
- orig->sha1, orig->mode,
+ orig->oid.hash, orig->mode,
a_sha, a_mode,
b_sha, b_mode,
_("rename"), _("renamed"));
@@ -1111,7 +1115,7 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
unsigned mode = entry->stages[stage].mode;
if (mode == 0 || is_null_sha1(sha))
return NULL;
- hashcpy(target->sha1, sha);
+ hashcpy(target->oid.hash, sha);
target->mode = mode;
return target;
}
@@ -1140,7 +1144,7 @@ static void handle_file(struct merge_options *o,
add = filespec_from_entry(&other, dst_entry, stage ^ 1);
if (add) {
char *add_name = unique_path(o, rename->path, other_branch);
- update_file(o, 0, add->sha1, add->mode, add_name);
+ update_file(o, 0, add->oid.hash, add->mode, add_name);
remove_file(o, 0, rename->path, 0);
dst_name = unique_path(o, rename->path, cur_branch);
@@ -1151,7 +1155,7 @@ static void handle_file(struct merge_options *o,
rename->path, other_branch, dst_name);
}
}
- update_file(o, 0, rename->sha1, rename->mode, dst_name);
+ update_file(o, 0, rename->oid.hash, rename->mode, dst_name);
if (stage == 2)
update_stages(rename->path, NULL, rename, add);
else
@@ -1180,9 +1184,9 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
struct diff_filespec other;
struct diff_filespec *add;
mfi = merge_file_one(o, one->path,
- one->sha1, one->mode,
- a->sha1, a->mode,
- b->sha1, b->mode,
+ one->oid.hash, one->mode,
+ a->oid.hash, a->mode,
+ b->oid.hash, b->mode,
ci->branch1, ci->branch2);
/*
* FIXME: For rename/add-source conflicts (if we could detect
@@ -1202,12 +1206,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
*/
add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1);
if (add)
- update_file(o, 0, add->sha1, add->mode, a->path);
+ update_file(o, 0, add->oid.hash, add->mode, a->path);
else
remove_file_from_cache(a->path);
add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1);
if (add)
- update_file(o, 0, add->sha1, add->mode, b->path);
+ update_file(o, 0, add->oid.hash, add->mode, b->path);
else
remove_file_from_cache(b->path);
} else {
@@ -1421,13 +1425,15 @@ static int process_renames(struct merge_options *o,
remove_file(o, 1, ren1_src,
renamed_stage == 2 || !was_tracked(ren1_src));
- hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);
+ hashcpy(src_other.oid.hash,
+ ren1->src_entry->stages[other_stage].sha);
src_other.mode = ren1->src_entry->stages[other_stage].mode;
- hashcpy(dst_other.sha1, ren1->dst_entry->stages[other_stage].sha);
+ hashcpy(dst_other.oid.hash,
+ ren1->dst_entry->stages[other_stage].sha);
dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
try_merge = 0;
- if (sha_eq(src_other.sha1, null_sha1)) {
+ if (sha_eq(src_other.oid.hash, null_sha1)) {
setup_rename_conflict_info(RENAME_DELETE,
ren1->pair,
NULL,
@@ -1439,7 +1445,7 @@ static int process_renames(struct merge_options *o,
NULL,
NULL);
} else if ((dst_other.mode == ren1->pair->two->mode) &&
- sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
+ sha_eq(dst_other.oid.hash, ren1->pair->two->oid.hash)) {
/*
* Added file on the other side identical to
* the file being renamed: clean merge.
@@ -1449,12 +1455,12 @@ static int process_renames(struct merge_options *o,
* update_file().
*/
update_file_flags(o,
- ren1->pair->two->sha1,
+ ren1->pair->two->oid.hash,
ren1->pair->two->mode,
ren1_dst,
1, /* update_cache */
0 /* update_wd */);
- } else if (!sha_eq(dst_other.sha1, null_sha1)) {
+ } else if (!sha_eq(dst_other.oid.hash, null_sha1)) {
clean_merge = 0;
try_merge = 1;
output(o, 1, _("CONFLICT (rename/add): Rename %s->%s in %s. "
@@ -1464,8 +1470,10 @@ static int process_renames(struct merge_options *o,
if (o->call_depth) {
struct merge_file_info mfi;
mfi = merge_file_one(o, ren1_dst, null_sha1, 0,
- ren1->pair->two->sha1, ren1->pair->two->mode,
- dst_other.sha1, dst_other.mode,
+ ren1->pair->two->oid.hash,
+ ren1->pair->two->mode,
+ dst_other.oid.hash,
+ dst_other.mode,
branch1, branch2);
output(o, 1, _("Adding merged %s"), ren1_dst);
update_file(o, 0, mfi.sha, mfi.mode, ren1_dst);
@@ -1473,7 +1481,8 @@ static int process_renames(struct merge_options *o,
} else {
char *new_path = unique_path(o, ren1_dst, branch2);
output(o, 1, _("Adding as %s instead"), new_path);
- update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
+ update_file(o, 0, dst_other.oid.hash,
+ dst_other.mode, new_path);
free(new_path);
}
} else
@@ -1599,11 +1608,11 @@ static int merge_content(struct merge_options *o,
o_sha = (unsigned char *)null_sha1;
}
one.path = a.path = b.path = (char *)path;
- hashcpy(one.sha1, o_sha);
+ hashcpy(one.oid.hash, o_sha);
one.mode = o_mode;
- hashcpy(a.sha1, a_sha);
+ hashcpy(a.oid.hash, a_sha);
a.mode = a_mode;
- hashcpy(b.sha1, b_sha);
+ hashcpy(b.oid.hash, b_sha);
b.mode = b_mode;
if (rename_conflict_info) {
@@ -1664,7 +1673,7 @@ static int merge_content(struct merge_options *o,
else {
int file_from_stage2 = was_tracked(path);
struct diff_filespec merged;
- hashcpy(merged.sha1, mfi.sha);
+ hashcpy(merged.oid.hash, mfi.sha);
merged.mode = mfi.mode;
update_stages(path, NULL,
diff --git a/notes-merge.c b/notes-merge.c
index 34bfac0c..62c23d8a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -41,14 +41,14 @@ static int verify_notes_filepair(struct diff_filepair *p, unsigned char *sha1)
switch (p->status) {
case DIFF_STATUS_MODIFIED:
assert(p->one->mode == p->two->mode);
- assert(!is_null_sha1(p->one->sha1));
- assert(!is_null_sha1(p->two->sha1));
+ assert(!is_null_oid(&p->one->oid));
+ assert(!is_null_oid(&p->two->oid));
break;
case DIFF_STATUS_ADDED:
- assert(is_null_sha1(p->one->sha1));
+ assert(is_null_oid(&p->one->oid));
break;
case DIFF_STATUS_DELETED:
- assert(is_null_sha1(p->two->sha1));
+ assert(is_null_oid(&p->two->oid));
break;
default:
return -1;
@@ -142,27 +142,27 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
if (verify_notes_filepair(p, obj)) {
trace_printf("\t\tCannot merge entry '%s' (%c): "
"%.7s -> %.7s. Skipping!\n", p->one->path,
- p->status, sha1_to_hex(p->one->sha1),
- sha1_to_hex(p->two->sha1));
+ p->status, oid_to_hex(&p->one->oid),
+ oid_to_hex(&p->two->oid));
continue;
}
mp = find_notes_merge_pair_pos(changes, len, obj, 1, &occupied);
if (occupied) {
/* We've found an addition/deletion pair */
assert(!hashcmp(mp->obj, obj));
- if (is_null_sha1(p->one->sha1)) { /* addition */
+ if (is_null_oid(&p->one->oid)) { /* addition */
assert(is_null_sha1(mp->remote));
- hashcpy(mp->remote, p->two->sha1);
- } else if (is_null_sha1(p->two->sha1)) { /* deletion */
+ hashcpy(mp->remote, p->two->oid.hash);
+ } else if (is_null_oid(&p->two->oid)) { /* deletion */
assert(is_null_sha1(mp->base));
- hashcpy(mp->base, p->one->sha1);
+ hashcpy(mp->base, p->one->oid.hash);
} else
assert(!"Invalid existing change recorded");
} else {
hashcpy(mp->obj, obj);
- hashcpy(mp->base, p->one->sha1);
+ hashcpy(mp->base, p->one->oid.hash);
hashcpy(mp->local, uninitialized);
- hashcpy(mp->remote, p->two->sha1);
+ hashcpy(mp->remote, p->two->oid.hash);
len++;
}
trace_printf("\t\tStored remote change for %s: %.7s -> %.7s\n",
@@ -203,21 +203,21 @@ static void diff_tree_local(struct notes_merge_options *o,
if (verify_notes_filepair(p, obj)) {
trace_printf("\t\tCannot merge entry '%s' (%c): "
"%.7s -> %.7s. Skipping!\n", p->one->path,
- p->status, sha1_to_hex(p->one->sha1),
- sha1_to_hex(p->two->sha1));
+ p->status, oid_to_hex(&p->one->oid),
+ oid_to_hex(&p->two->oid));
continue;
}
mp = find_notes_merge_pair_pos(changes, len, obj, 0, &match);
if (!match) {
trace_printf("\t\tIgnoring local-only change for %s: "
"%.7s -> %.7s\n", sha1_to_hex(obj),
- sha1_to_hex(p->one->sha1),
- sha1_to_hex(p->two->sha1));
+ oid_to_hex(&p->one->oid),
+ oid_to_hex(&p->two->oid));
continue;
}
assert(!hashcmp(mp->obj, obj));
- if (is_null_sha1(p->two->sha1)) { /* deletion */
+ if (is_null_oid(&p->two->oid)) { /* deletion */
/*
* Either this is a true deletion (1), or it is part
* of an A/D pair (2), or D/A pair (3):
@@ -229,7 +229,7 @@ static void diff_tree_local(struct notes_merge_options *o,
*/
if (!hashcmp(mp->local, uninitialized))
hashclr(mp->local);
- } else if (is_null_sha1(p->one->sha1)) { /* addition */
+ } else if (is_null_oid(&p->one->oid)) { /* addition */
/*
* Either this is a true addition (1), or it is part
* of an A/D pair (2), or D/A pair (3):
@@ -240,16 +240,16 @@ static void diff_tree_local(struct notes_merge_options *o,
*/
assert(is_null_sha1(mp->local) ||
!hashcmp(mp->local, uninitialized));
- hashcpy(mp->local, p->two->sha1);
+ hashcpy(mp->local, p->two->oid.hash);
} else { /* modification */
/*
* This is a true modification. p->one->sha1 shall
* match mp->base, and mp->local shall be uninitialized.
* Set mp->local to p->two->sha1.
*/
- assert(!hashcmp(p->one->sha1, mp->base));
+ assert(!hashcmp(p->one->oid.hash, mp->base));
assert(!hashcmp(mp->local, uninitialized));
- hashcpy(mp->local, p->two->sha1);
+ hashcpy(mp->local, p->two->oid.hash);
}
trace_printf("\t\tStored local change for %s: %.7s -> %.7s\n",
sha1_to_hex(mp->obj), sha1_to_hex(mp->base),
diff --git a/submodule.c b/submodule.c
index 4532b11d..cdccf65c 100644
--- a/submodule.c
+++ b/submodule.c
@@ -445,7 +445,7 @@ static void collect_submodules_from_diff(struct diff_queue_struct *q,
struct diff_filepair *p = q->queue[i];
if (!S_ISGITLINK(p->two->mode))
continue;
- if (submodule_needs_pushing(p->two->path, p->two->sha1))
+ if (submodule_needs_pushing(p->two->path, p->two->oid.hash))
string_list_insert(needs_pushing, p->two->path);
}
}
@@ -577,7 +577,7 @@ static void submodule_collect_changed_cb(struct diff_queue_struct *q,
* being moved around. */
struct string_list_item *path;
path = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);
- if (!path && !is_submodule_commit_present(p->two->path, p->two->sha1))
+ if (!path && !is_submodule_commit_present(p->two->path, p->two->oid.hash))
string_list_append(&changed_submodule_paths, xstrdup(p->two->path));
} else {
/* Submodule is new or was moved here */
diff --git a/wt-status.c b/wt-status.c
index 4f27bd62..7c2fa8cd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -432,7 +432,8 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
d->worktree_status = p->status;
d->dirty_submodule = p->two->dirty_submodule;
if (S_ISGITLINK(p->two->mode))
- d->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);
+ d->new_submodule_commits = !!oidcmp(&p->one->oid,
+ &p->two->oid);
}
}
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/8] Rename struct diff_filespec's sha1_valid member.
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (2 preceding siblings ...)
2016-06-18 22:14 ` [PATCH v2 3/8] Convert struct diff_filespec to struct object_id brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 5/8] merge-recursive: convert struct stage_data to use object_id brian m. carlson
` (5 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Now that this struct's sha1 member is called "oid", update the comment
and the sha1_valid member to be called "oid_valid" instead. The
following Coccinelle semantic patch was used to implement this, followed
by the transformations in object_id.cocci:
@@
struct diff_filespec o;
@@
- o.sha1_valid
+ o.oid_valid
@@
struct diff_filespec *p;
@@
- p->sha1_valid
+ p->oid_valid
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
combine-diff.c | 4 ++--
diff.c | 28 ++++++++++++++--------------
diffcore-break.c | 2 +-
diffcore-rename.c | 4 ++--
diffcore.h | 2 +-
line-log.c | 10 +++++-----
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 3537209c..5940dc87 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1269,7 +1269,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,
pair->one[i].path = p->path;
pair->one[i].mode = p->parent[i].mode;
oidcpy(&pair->one[i].oid, &p->parent[i].oid);
- pair->one[i].sha1_valid = !is_null_oid(&p->parent[i].oid);
+ pair->one[i].oid_valid = !is_null_oid(&p->parent[i].oid);
pair->one[i].has_more_entries = 1;
}
pair->one[num_parent - 1].has_more_entries = 0;
@@ -1277,7 +1277,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,
pair->two->path = p->path;
pair->two->mode = p->mode;
oidcpy(&pair->two->oid, &p->oid);
- pair->two->sha1_valid = !is_null_oid(&p->oid);
+ pair->two->oid_valid = !is_null_oid(&p->oid);
return pair;
}
diff --git a/diff.c b/diff.c
index 5a6d8654..a7a553b8 100644
--- a/diff.c
+++ b/diff.c
@@ -1933,7 +1933,7 @@ static void show_dirstat(struct diff_options *options)
name = p->two->path ? p->two->path : p->one->path;
- if (p->one->sha1_valid && p->two->sha1_valid)
+ if (p->one->oid_valid && p->two->oid_valid)
content_changed = oidcmp(&p->one->oid, &p->two->oid);
else
content_changed = 1;
@@ -2640,7 +2640,7 @@ void fill_filespec(struct diff_filespec *spec, const unsigned char *sha1,
if (mode) {
spec->mode = canon_mode(mode);
hashcpy(spec->oid.hash, sha1);
- spec->sha1_valid = sha1_valid;
+ spec->oid_valid = sha1_valid;
}
}
@@ -2766,7 +2766,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
if (S_ISGITLINK(s->mode))
return diff_populate_gitlink(s, size_only);
- if (!s->sha1_valid ||
+ if (!s->oid_valid ||
reuse_worktree_file(s->path, s->oid.hash, 0)) {
struct strbuf buf = STRBUF_INIT;
struct stat st;
@@ -2915,7 +2915,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
}
if (!S_ISGITLINK(one->mode) &&
- (!one->sha1_valid ||
+ (!one->oid_valid ||
reuse_worktree_file(name, one->oid.hash, 1))) {
struct stat st;
if (lstat(name, &st) < 0) {
@@ -2928,16 +2928,16 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (strbuf_readlink(&sb, name, st.st_size) < 0)
die_errno("readlink(%s)", name);
prep_temp_blob(name, temp, sb.buf, sb.len,
- (one->sha1_valid ?
+ (one->oid_valid ?
one->oid.hash : null_sha1),
- (one->sha1_valid ?
+ (one->oid_valid ?
one->mode : S_IFLNK));
strbuf_release(&sb);
}
else {
/* we can borrow from the file in the work tree */
temp->name = name;
- if (!one->sha1_valid)
+ if (!one->oid_valid)
sha1_to_hex_r(temp->hex, null_sha1);
else
sha1_to_hex_r(temp->hex, one->oid.hash);
@@ -3134,7 +3134,7 @@ static void run_diff_cmd(const char *pgm,
static void diff_fill_sha1_info(struct diff_filespec *one)
{
if (DIFF_FILE_VALID(one)) {
- if (!one->sha1_valid) {
+ if (!one->oid_valid) {
struct stat st;
if (one->is_stdin) {
hashcpy(one->oid.hash, null_sha1);
@@ -4172,11 +4172,11 @@ int diff_unmodified_pair(struct diff_filepair *p)
/* both are valid and point at the same path. that is, we are
* dealing with a change.
*/
- if (one->sha1_valid && two->sha1_valid &&
+ if (one->oid_valid && two->oid_valid &&
!oidcmp(&one->oid, &two->oid) &&
!one->dirty_submodule && !two->dirty_submodule)
return 1; /* no change */
- if (!one->sha1_valid && !two->sha1_valid)
+ if (!one->oid_valid && !two->oid_valid)
return 1; /* both look at the same file on the filesystem. */
return 0;
}
@@ -4237,7 +4237,7 @@ void diff_debug_filespec(struct diff_filespec *s, int x, const char *one)
s->path,
DIFF_FILE_VALID(s) ? "valid" : "invalid",
s->mode,
- s->sha1_valid ? oid_to_hex(&s->oid) : "");
+ s->oid_valid ? oid_to_hex(&s->oid) : "");
fprintf(stderr, "queue[%d] %s size %lu\n",
x, one ? one : "",
s->size);
@@ -4822,7 +4822,7 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
*/
if (!DIFF_FILE_VALID(p->one) || /* (1) */
!DIFF_FILE_VALID(p->two) ||
- (p->one->sha1_valid && p->two->sha1_valid) ||
+ (p->one->oid_valid && p->two->oid_valid) ||
(p->one->mode != p->two->mode) ||
diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||
diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||
@@ -5118,7 +5118,7 @@ size_t fill_textconv(struct userdiff_driver *driver,
if (!driver->textconv)
die("BUG: fill_textconv called with non-textconv driver");
- if (driver->textconv_cache && df->sha1_valid) {
+ if (driver->textconv_cache && df->oid_valid) {
*outbuf = notes_cache_get(driver->textconv_cache,
df->oid.hash,
&size);
@@ -5130,7 +5130,7 @@ size_t fill_textconv(struct userdiff_driver *driver,
if (!*outbuf)
die("unable to read files to diff");
- if (driver->textconv_cache && df->sha1_valid) {
+ if (driver->textconv_cache && df->oid_valid) {
/* ignore errors, as we might be in a readonly repository */
notes_cache_put(driver->textconv_cache, df->oid.hash, *outbuf,
size);
diff --git a/diffcore-break.c b/diffcore-break.c
index a3e79608..881a74f2 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -57,7 +57,7 @@ static int should_break(struct diff_filespec *src,
return 1; /* even their types are different */
}
- if (src->sha1_valid && dst->sha1_valid &&
+ if (src->oid_valid && dst->oid_valid &&
!oidcmp(&src->oid, &dst->oid))
return 0; /* they are the same */
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 22b239a4..58ac0a53 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -60,7 +60,7 @@ static int add_rename_dst(struct diff_filespec *two)
memmove(rename_dst + first + 1, rename_dst + first,
(rename_dst_nr - first - 1) * sizeof(*rename_dst));
rename_dst[first].two = alloc_filespec(two->path);
- fill_filespec(rename_dst[first].two, two->oid.hash, two->sha1_valid,
+ fill_filespec(rename_dst[first].two, two->oid.hash, two->oid_valid,
two->mode);
rename_dst[first].pair = NULL;
return 0;
@@ -261,7 +261,7 @@ struct file_similarity {
static unsigned int hash_filespec(struct diff_filespec *filespec)
{
- if (!filespec->sha1_valid) {
+ if (!filespec->oid_valid) {
if (diff_populate_filespec(filespec, 0))
return 0;
hash_sha1_file(filespec->data, filespec->size, "blob",
diff --git a/diffcore.h b/diffcore.h
index c40e5b6c..c11b8465 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -33,7 +33,7 @@ struct diff_filespec {
int count; /* Reference count */
int rename_used; /* Count of rename users */
unsigned short mode; /* file mode */
- unsigned sha1_valid : 1; /* if true, use sha1 and trust mode;
+ unsigned oid_valid : 1; /* if true, use oid and trust mode;
* if false, use the name and read from
* the filesystem.
*/
diff --git a/line-log.c b/line-log.c
index 02938639..93407baa 100644
--- a/line-log.c
+++ b/line-log.c
@@ -894,14 +894,14 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
if (!pair || !diff)
return;
- if (pair->one->sha1_valid)
+ if (pair->one->oid_valid)
fill_line_ends(pair->one, &p_lines, &p_ends);
fill_line_ends(pair->two, &t_lines, &t_ends);
printf("%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, pair->one->path, pair->two->path, c_reset);
printf("%s%s--- %s%s%s\n", prefix, c_meta,
- pair->one->sha1_valid ? "a/" : "",
- pair->one->sha1_valid ? pair->one->path : "/dev/null",
+ pair->one->oid_valid ? "a/" : "",
+ pair->one->oid_valid ? pair->one->path : "/dev/null",
c_reset);
printf("%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, c_reset);
for (i = 0; i < range->ranges.nr; i++) {
@@ -1011,12 +1011,12 @@ static int process_diff_filepair(struct rev_info *rev,
if (rg->ranges.nr == 0)
return 0;
- assert(pair->two->sha1_valid);
+ assert(pair->two->oid_valid);
diff_populate_filespec(pair->two, 0);
file_target.ptr = pair->two->data;
file_target.size = pair->two->size;
- if (pair->one->sha1_valid) {
+ if (pair->one->oid_valid) {
diff_populate_filespec(pair->one, 0);
file_parent.ptr = pair->one->data;
file_parent.size = pair->one->size;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/8] merge-recursive: convert struct stage_data to use object_id
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (3 preceding siblings ...)
2016-06-18 22:14 ` [PATCH v2 4/8] Rename struct diff_filespec's sha1_valid member brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 6/8] merge-recursive: convert struct merge_file_info to object_id brian m. carlson
` (4 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Convert the anonymous struct within struct stage_data to use struct
object_id. The following Coccinelle semantic patch was used to
implement this, followed by the transformations in object_id.cocci:
@@
struct stage_data o;
expression E1;
@@
- o.stages[E1].sha
+ o.stages[E1].oid.hash
@@
struct stage_data *p;
expression E1;
@@
- p->stages[E1].sha
+ p->stages[E1].oid.hash
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
merge-recursive.c | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 1e802097..a07050cd 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -90,7 +90,7 @@ struct rename_conflict_info {
struct stage_data {
struct {
unsigned mode;
- unsigned char sha[20];
+ struct object_id oid;
} stages[4];
struct rename_conflict_info *rename_conflict_info;
unsigned processed:1;
@@ -134,13 +134,11 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,
int ostage2 = ostage1 ^ 1;
ci->ren1_other.path = pair1->one->path;
- hashcpy(ci->ren1_other.oid.hash,
- src_entry1->stages[ostage1].sha);
+ oidcpy(&ci->ren1_other.oid, &src_entry1->stages[ostage1].oid);
ci->ren1_other.mode = src_entry1->stages[ostage1].mode;
ci->ren2_other.path = pair2->one->path;
- hashcpy(ci->ren2_other.oid.hash,
- src_entry2->stages[ostage2].sha);
+ oidcpy(&ci->ren2_other.oid, &src_entry2->stages[ostage2].oid);
ci->ren2_other.mode = src_entry2->stages[ostage2].mode;
}
}
@@ -316,11 +314,11 @@ static struct stage_data *insert_stage_data(const char *path,
struct string_list_item *item;
struct stage_data *e = xcalloc(1, sizeof(struct stage_data));
get_tree_entry(o->object.oid.hash, path,
- e->stages[1].sha, &e->stages[1].mode);
+ e->stages[1].oid.hash, &e->stages[1].mode);
get_tree_entry(a->object.oid.hash, path,
- e->stages[2].sha, &e->stages[2].mode);
+ e->stages[2].oid.hash, &e->stages[2].mode);
get_tree_entry(b->object.oid.hash, path,
- e->stages[3].sha, &e->stages[3].mode);
+ e->stages[3].oid.hash, &e->stages[3].mode);
item = string_list_insert(entries, path);
item->util = e;
return e;
@@ -351,7 +349,7 @@ static struct string_list *get_unmerged(void)
}
e = item->util;
e->stages[ce_stage(ce)].mode = ce->ce_mode;
- hashcpy(e->stages[ce_stage(ce)].sha, ce->sha1);
+ hashcpy(e->stages[ce_stage(ce)].oid.hash, ce->sha1);
}
return unmerged;
@@ -574,9 +572,9 @@ static void update_entry(struct stage_data *entry,
entry->stages[1].mode = o->mode;
entry->stages[2].mode = a->mode;
entry->stages[3].mode = b->mode;
- hashcpy(entry->stages[1].sha, o->oid.hash);
- hashcpy(entry->stages[2].sha, a->oid.hash);
- hashcpy(entry->stages[3].sha, b->oid.hash);
+ oidcpy(&entry->stages[1].oid, &o->oid);
+ oidcpy(&entry->stages[2].oid, &a->oid);
+ oidcpy(&entry->stages[3].oid, &b->oid);
}
static int remove_file(struct merge_options *o, int clean,
@@ -1111,7 +1109,7 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
struct stage_data *entry,
int stage)
{
- unsigned char *sha = entry->stages[stage].sha;
+ unsigned char *sha = entry->stages[stage].oid.hash;
unsigned mode = entry->stages[stage].mode;
if (mode == 0 || is_null_sha1(sha))
return NULL;
@@ -1425,11 +1423,11 @@ static int process_renames(struct merge_options *o,
remove_file(o, 1, ren1_src,
renamed_stage == 2 || !was_tracked(ren1_src));
- hashcpy(src_other.oid.hash,
- ren1->src_entry->stages[other_stage].sha);
+ oidcpy(&src_other.oid,
+ &ren1->src_entry->stages[other_stage].oid);
src_other.mode = ren1->src_entry->stages[other_stage].mode;
- hashcpy(dst_other.oid.hash,
- ren1->dst_entry->stages[other_stage].sha);
+ oidcpy(&dst_other.oid,
+ &ren1->dst_entry->stages[other_stage].oid);
dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
try_merge = 0;
@@ -1703,9 +1701,9 @@ static int process_entry(struct merge_options *o,
unsigned o_mode = entry->stages[1].mode;
unsigned a_mode = entry->stages[2].mode;
unsigned b_mode = entry->stages[3].mode;
- unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
- unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
- unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
+ unsigned char *o_sha = stage_sha(entry->stages[1].oid.hash, o_mode);
+ unsigned char *a_sha = stage_sha(entry->stages[2].oid.hash, a_mode);
+ unsigned char *b_sha = stage_sha(entry->stages[3].oid.hash, b_mode);
entry->processed = 1;
if (entry->rename_conflict_info) {
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/8] merge-recursive: convert struct merge_file_info to object_id
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (4 preceding siblings ...)
2016-06-18 22:14 ` [PATCH v2 5/8] merge-recursive: convert struct stage_data to use object_id brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 7/8] merge-recursive: convert leaf functions to use struct object_id brian m. carlson
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Convert struct merge_file_info to use struct object_id. The following
Coccinelle semantic patch was used to implement this, followed by the
transformations in object_id.cocci:
@@
struct merge_file_info o;
@@
- o.sha
+ o.oid.hash
@@
struct merge_file_info *p;
@@
- p->sha
+ p->oid.hash
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
merge-recursive.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index a07050cd..bc455491 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -819,7 +819,7 @@ static void update_file(struct merge_options *o,
/* Low level file merging, update and removal */
struct merge_file_info {
- unsigned char sha[20];
+ struct object_id oid;
unsigned mode;
unsigned clean:1,
merge:1;
@@ -902,10 +902,10 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
result.clean = 0;
if (S_ISREG(a->mode)) {
result.mode = a->mode;
- hashcpy(result.sha, a->oid.hash);
+ oidcpy(&result.oid, &a->oid);
} else {
result.mode = b->mode;
- hashcpy(result.sha, b->oid.hash);
+ oidcpy(&result.oid, &b->oid);
}
} else {
if (!sha_eq(a->oid.hash, one->oid.hash) && !sha_eq(b->oid.hash, one->oid.hash))
@@ -925,9 +925,9 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
}
if (sha_eq(a->oid.hash, b->oid.hash) || sha_eq(a->oid.hash, one->oid.hash))
- hashcpy(result.sha, b->oid.hash);
+ oidcpy(&result.oid, &b->oid);
else if (sha_eq(b->oid.hash, one->oid.hash))
- hashcpy(result.sha, a->oid.hash);
+ oidcpy(&result.oid, &a->oid);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
int merge_status;
@@ -939,21 +939,21 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
die(_("Failed to execute internal merge"));
if (write_sha1_file(result_buf.ptr, result_buf.size,
- blob_type, result.sha))
+ blob_type, result.oid.hash))
die(_("Unable to add %s to database"),
a->path);
free(result_buf.ptr);
result.clean = (merge_status == 0);
} else if (S_ISGITLINK(a->mode)) {
- result.clean = merge_submodule(result.sha,
+ result.clean = merge_submodule(result.oid.hash,
one->path,
one->oid.hash,
a->oid.hash,
b->oid.hash,
!o->call_depth);
} else if (S_ISLNK(a->mode)) {
- hashcpy(result.sha, a->oid.hash);
+ oidcpy(&result.oid, &a->oid);
if (!sha_eq(a->oid.hash, b->oid.hash))
result.clean = 0;
@@ -1192,7 +1192,7 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
* pathname and then either rename the add-source file to that
* unique path, or use that unique path instead of src here.
*/
- update_file(o, 0, mfi.sha, mfi.mode, one->path);
+ update_file(o, 0, mfi.oid.hash, mfi.mode, one->path);
/*
* Above, we put the merged content at the merge-base's
@@ -1255,16 +1255,16 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
* again later for the non-recursive merge.
*/
remove_file(o, 0, path, 0);
- update_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path);
- update_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path);
+ update_file(o, 0, mfi_c1.oid.hash, mfi_c1.mode, a->path);
+ update_file(o, 0, mfi_c2.oid.hash, mfi_c2.mode, b->path);
} else {
char *new_path1 = unique_path(o, path, ci->branch1);
char *new_path2 = unique_path(o, path, ci->branch2);
output(o, 1, _("Renaming %s to %s and %s to %s instead"),
a->path, new_path1, b->path, new_path2);
remove_file(o, 0, path, 0);
- update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1);
- update_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2);
+ update_file(o, 0, mfi_c1.oid.hash, mfi_c1.mode, new_path1);
+ update_file(o, 0, mfi_c2.oid.hash, mfi_c2.mode, new_path2);
free(new_path2);
free(new_path1);
}
@@ -1474,7 +1474,8 @@ static int process_renames(struct merge_options *o,
dst_other.mode,
branch1, branch2);
output(o, 1, _("Adding merged %s"), ren1_dst);
- update_file(o, 0, mfi.sha, mfi.mode, ren1_dst);
+ update_file(o, 0, mfi.oid.hash,
+ mfi.mode, ren1_dst);
try_merge = 0;
} else {
char *new_path = unique_path(o, ren1_dst, branch2);
@@ -1634,7 +1635,7 @@ static int merge_content(struct merge_options *o,
o->branch2, path2);
if (mfi.clean && !df_conflict_remains &&
- sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) {
+ sha_eq(mfi.oid.hash, a_sha) && mfi.mode == a_mode) {
int path_renamed_outside_HEAD;
output(o, 3, _("Skipped %s (merged same as existing)"), path);
/*
@@ -1645,7 +1646,7 @@ static int merge_content(struct merge_options *o,
*/
path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
if (!path_renamed_outside_HEAD) {
- add_cacheinfo(mfi.mode, mfi.sha, path,
+ add_cacheinfo(mfi.mode, mfi.oid.hash, path,
0, (!o->call_depth), 0);
return mfi.clean;
}
@@ -1671,7 +1672,7 @@ static int merge_content(struct merge_options *o,
else {
int file_from_stage2 = was_tracked(path);
struct diff_filespec merged;
- hashcpy(merged.oid.hash, mfi.sha);
+ oidcpy(&merged.oid, &mfi.oid);
merged.mode = mfi.mode;
update_stages(path, NULL,
@@ -1682,11 +1683,11 @@ static int merge_content(struct merge_options *o,
}
new_path = unique_path(o, path, rename_conflict_info->branch1);
output(o, 1, _("Adding as %s instead"), new_path);
- update_file(o, 0, mfi.sha, mfi.mode, new_path);
+ update_file(o, 0, mfi.oid.hash, mfi.mode, new_path);
free(new_path);
mfi.clean = 0;
} else {
- update_file(o, mfi.clean, mfi.sha, mfi.mode, path);
+ update_file(o, mfi.clean, mfi.oid.hash, mfi.mode, path);
}
return mfi.clean;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 7/8] merge-recursive: convert leaf functions to use struct object_id
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (5 preceding siblings ...)
2016-06-18 22:14 ` [PATCH v2 6/8] merge-recursive: convert struct merge_file_info to object_id brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 8/8] merge-recursive: convert merge_recursive_generic to object_id brian m. carlson
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Convert all but two of the static functions in this file to use struct
object_id.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
merge-recursive.c | 236 +++++++++++++++++++++++++++---------------------------
1 file changed, 118 insertions(+), 118 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index bc455491..7bbd4aea 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -56,11 +56,11 @@ static struct commit *make_virtual_commit(struct tree *tree, const char *comment
* Since we use get_tree_entry(), which does not put the read object into
* the object pool, we cannot rely on a == b.
*/
-static int sha_eq(const unsigned char *a, const unsigned char *b)
+static int oid_eq(const struct object_id *a, const struct object_id *b)
{
if (!a && !b)
return 2;
- return a && b && hashcmp(a, b) == 0;
+ return a && b && oidcmp(a, b) == 0;
}
enum rename_type {
@@ -198,11 +198,11 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)
}
}
-static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
+static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
const char *path, int stage, int refresh, int options)
{
struct cache_entry *ce;
- ce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage,
+ ce = make_cache_entry(mode, oid ? oid->hash : null_sha1, path, stage,
(refresh ? (CE_MATCH_REFRESH |
CE_MATCH_IGNORE_MISSING) : 0 ));
if (!ce)
@@ -552,13 +552,13 @@ static int update_stages(const char *path, const struct diff_filespec *o,
if (remove_file_from_cache(path))
return -1;
if (o)
- if (add_cacheinfo(o->mode, o->oid.hash, path, 1, 0, options))
+ if (add_cacheinfo(o->mode, &o->oid, path, 1, 0, options))
return -1;
if (a)
- if (add_cacheinfo(a->mode, a->oid.hash, path, 2, 0, options))
+ if (add_cacheinfo(a->mode, &a->oid, path, 2, 0, options))
return -1;
if (b)
- if (add_cacheinfo(b->mode, b->oid.hash, path, 3, 0, options))
+ if (add_cacheinfo(b->mode, &b->oid, path, 3, 0, options))
return -1;
return 0;
}
@@ -736,7 +736,7 @@ static int make_room_for_path(struct merge_options *o, const char *path)
}
static void update_file_flags(struct merge_options *o,
- const unsigned char *sha,
+ const struct object_id *oid,
unsigned mode,
const char *path,
int update_cache,
@@ -760,11 +760,11 @@ static void update_file_flags(struct merge_options *o,
goto update_index;
}
- buf = read_sha1_file(sha, &type, &size);
+ buf = read_sha1_file(oid->hash, &type, &size);
if (!buf)
- die(_("cannot read object %s '%s'"), sha1_to_hex(sha), path);
+ die(_("cannot read object %s '%s'"), oid_to_hex(oid), path);
if (type != OBJ_BLOB)
- die(_("blob expected for %s '%s'"), sha1_to_hex(sha), path);
+ die(_("blob expected for %s '%s'"), oid_to_hex(oid), path);
if (S_ISREG(mode)) {
struct strbuf strbuf = STRBUF_INIT;
if (convert_to_working_tree(path, buf, size, &strbuf)) {
@@ -799,21 +799,21 @@ static void update_file_flags(struct merge_options *o,
free(lnk);
} else
die(_("do not know what to do with %06o %s '%s'"),
- mode, sha1_to_hex(sha), path);
+ mode, oid_to_hex(oid), path);
free(buf);
}
update_index:
if (update_cache)
- add_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
+ add_cacheinfo(mode, oid, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);
}
static void update_file(struct merge_options *o,
int clean,
- const unsigned char *sha,
+ const struct object_id *oid,
unsigned mode,
const char *path)
{
- update_file_flags(o, sha, mode, path, o->call_depth || clean, !o->call_depth);
+ update_file_flags(o, oid, mode, path, o->call_depth || clean, !o->call_depth);
}
/* Low level file merging, update and removal */
@@ -908,7 +908,7 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
oidcpy(&result.oid, &b->oid);
}
} else {
- if (!sha_eq(a->oid.hash, one->oid.hash) && !sha_eq(b->oid.hash, one->oid.hash))
+ if (!oid_eq(&a->oid, &one->oid) && !oid_eq(&b->oid, &one->oid))
result.merge = 1;
/*
@@ -924,9 +924,9 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
}
}
- if (sha_eq(a->oid.hash, b->oid.hash) || sha_eq(a->oid.hash, one->oid.hash))
+ if (oid_eq(&a->oid, &b->oid) || oid_eq(&a->oid, &one->oid))
oidcpy(&result.oid, &b->oid);
- else if (sha_eq(b->oid.hash, one->oid.hash))
+ else if (oid_eq(&b->oid, &one->oid))
oidcpy(&result.oid, &a->oid);
else if (S_ISREG(a->mode)) {
mmbuffer_t result_buf;
@@ -955,7 +955,7 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
} else if (S_ISLNK(a->mode)) {
oidcpy(&result.oid, &a->oid);
- if (!sha_eq(a->oid.hash, b->oid.hash))
+ if (!oid_eq(&a->oid, &b->oid))
result.clean = 0;
} else {
die(_("unsupported object type in the tree"));
@@ -993,34 +993,34 @@ merge_file_special_markers(struct merge_options *o,
static struct merge_file_info merge_file_one(struct merge_options *o,
const char *path,
- const unsigned char *o_sha, int o_mode,
- const unsigned char *a_sha, int a_mode,
- const unsigned char *b_sha, int b_mode,
+ const struct object_id *o_oid, int o_mode,
+ const struct object_id *a_oid, int a_mode,
+ const struct object_id *b_oid, int b_mode,
const char *branch1,
const char *branch2)
{
struct diff_filespec one, a, b;
one.path = a.path = b.path = (char *)path;
- hashcpy(one.oid.hash, o_sha);
+ oidcpy(&one.oid, o_oid);
one.mode = o_mode;
- hashcpy(a.oid.hash, a_sha);
+ oidcpy(&a.oid, a_oid);
a.mode = a_mode;
- hashcpy(b.oid.hash, b_sha);
+ oidcpy(&b.oid, b_oid);
b.mode = b_mode;
return merge_file_1(o, &one, &a, &b, branch1, branch2);
}
static void handle_change_delete(struct merge_options *o,
const char *path,
- const unsigned char *o_sha, int o_mode,
- const unsigned char *a_sha, int a_mode,
- const unsigned char *b_sha, int b_mode,
+ const struct object_id *o_oid, int o_mode,
+ const struct object_id *a_oid, int a_mode,
+ const struct object_id *b_oid, int b_mode,
const char *change, const char *change_past)
{
char *renamed = NULL;
if (dir_in_way(path, !o->call_depth)) {
- renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
+ renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
}
if (o->call_depth) {
@@ -1030,20 +1030,20 @@ static void handle_change_delete(struct merge_options *o,
* them, simply reuse the base version for virtual merge base.
*/
remove_file_from_cache(path);
- update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);
- } else if (!a_sha) {
+ update_file(o, 0, o_oid, o_mode, renamed ? renamed : path);
+ } else if (!a_oid) {
if (!renamed) {
output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
"and %s in %s. Version %s of %s left in tree."),
change, path, o->branch1, change_past,
o->branch2, o->branch2, path);
- update_file(o, 0, b_sha, b_mode, path);
+ update_file(o, 0, b_oid, b_mode, path);
} else {
output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
"and %s in %s. Version %s of %s left in tree at %s."),
change, path, o->branch1, change_past,
o->branch2, o->branch2, path, renamed);
- update_file(o, 0, b_sha, b_mode, renamed);
+ update_file(o, 0, b_oid, b_mode, renamed);
}
} else {
if (!renamed) {
@@ -1056,7 +1056,7 @@ static void handle_change_delete(struct merge_options *o,
"and %s in %s. Version %s of %s left in tree at %s."),
change, path, o->branch2, change_past,
o->branch1, o->branch1, path, renamed);
- update_file(o, 0, a_sha, a_mode, renamed);
+ update_file(o, 0, a_oid, a_mode, renamed);
}
/*
* No need to call update_file() on path when !renamed, since
@@ -1075,24 +1075,24 @@ static void conflict_rename_delete(struct merge_options *o,
{
const struct diff_filespec *orig = pair->one;
const struct diff_filespec *dest = pair->two;
- const unsigned char *a_sha = NULL;
- const unsigned char *b_sha = NULL;
+ const struct object_id *a_oid = NULL;
+ const struct object_id *b_oid = NULL;
int a_mode = 0;
int b_mode = 0;
if (rename_branch == o->branch1) {
- a_sha = dest->oid.hash;
+ a_oid = &dest->oid;
a_mode = dest->mode;
} else {
- b_sha = dest->oid.hash;
+ b_oid = &dest->oid;
b_mode = dest->mode;
}
handle_change_delete(o,
o->call_depth ? orig->path : dest->path,
- orig->oid.hash, orig->mode,
- a_sha, a_mode,
- b_sha, b_mode,
+ &orig->oid, orig->mode,
+ a_oid, a_mode,
+ b_oid, b_mode,
_("rename"), _("renamed"));
if (o->call_depth) {
@@ -1109,11 +1109,11 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
struct stage_data *entry,
int stage)
{
- unsigned char *sha = entry->stages[stage].oid.hash;
+ struct object_id *oid = &entry->stages[stage].oid;
unsigned mode = entry->stages[stage].mode;
- if (mode == 0 || is_null_sha1(sha))
+ if (mode == 0 || is_null_oid(oid))
return NULL;
- hashcpy(target->oid.hash, sha);
+ oidcpy(&target->oid, oid);
target->mode = mode;
return target;
}
@@ -1142,7 +1142,7 @@ static void handle_file(struct merge_options *o,
add = filespec_from_entry(&other, dst_entry, stage ^ 1);
if (add) {
char *add_name = unique_path(o, rename->path, other_branch);
- update_file(o, 0, add->oid.hash, add->mode, add_name);
+ update_file(o, 0, &add->oid, add->mode, add_name);
remove_file(o, 0, rename->path, 0);
dst_name = unique_path(o, rename->path, cur_branch);
@@ -1153,7 +1153,7 @@ static void handle_file(struct merge_options *o,
rename->path, other_branch, dst_name);
}
}
- update_file(o, 0, rename->oid.hash, rename->mode, dst_name);
+ update_file(o, 0, &rename->oid, rename->mode, dst_name);
if (stage == 2)
update_stages(rename->path, NULL, rename, add);
else
@@ -1182,9 +1182,9 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
struct diff_filespec other;
struct diff_filespec *add;
mfi = merge_file_one(o, one->path,
- one->oid.hash, one->mode,
- a->oid.hash, a->mode,
- b->oid.hash, b->mode,
+ &one->oid, one->mode,
+ &a->oid, a->mode,
+ &b->oid, b->mode,
ci->branch1, ci->branch2);
/*
* FIXME: For rename/add-source conflicts (if we could detect
@@ -1192,7 +1192,7 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
* pathname and then either rename the add-source file to that
* unique path, or use that unique path instead of src here.
*/
- update_file(o, 0, mfi.oid.hash, mfi.mode, one->path);
+ update_file(o, 0, &mfi.oid, mfi.mode, one->path);
/*
* Above, we put the merged content at the merge-base's
@@ -1204,12 +1204,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
*/
add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1);
if (add)
- update_file(o, 0, add->oid.hash, add->mode, a->path);
+ update_file(o, 0, &add->oid, add->mode, a->path);
else
remove_file_from_cache(a->path);
add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1);
if (add)
- update_file(o, 0, add->oid.hash, add->mode, b->path);
+ update_file(o, 0, &add->oid, add->mode, b->path);
else
remove_file_from_cache(b->path);
} else {
@@ -1255,16 +1255,16 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
* again later for the non-recursive merge.
*/
remove_file(o, 0, path, 0);
- update_file(o, 0, mfi_c1.oid.hash, mfi_c1.mode, a->path);
- update_file(o, 0, mfi_c2.oid.hash, mfi_c2.mode, b->path);
+ update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path);
+ update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, b->path);
} else {
char *new_path1 = unique_path(o, path, ci->branch1);
char *new_path2 = unique_path(o, path, ci->branch2);
output(o, 1, _("Renaming %s to %s and %s to %s instead"),
a->path, new_path1, b->path, new_path2);
remove_file(o, 0, path, 0);
- update_file(o, 0, mfi_c1.oid.hash, mfi_c1.mode, new_path1);
- update_file(o, 0, mfi_c2.oid.hash, mfi_c2.mode, new_path2);
+ update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1);
+ update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, new_path2);
free(new_path2);
free(new_path1);
}
@@ -1431,7 +1431,7 @@ static int process_renames(struct merge_options *o,
dst_other.mode = ren1->dst_entry->stages[other_stage].mode;
try_merge = 0;
- if (sha_eq(src_other.oid.hash, null_sha1)) {
+ if (oid_eq(&src_other.oid, &null_oid)) {
setup_rename_conflict_info(RENAME_DELETE,
ren1->pair,
NULL,
@@ -1443,7 +1443,7 @@ static int process_renames(struct merge_options *o,
NULL,
NULL);
} else if ((dst_other.mode == ren1->pair->two->mode) &&
- sha_eq(dst_other.oid.hash, ren1->pair->two->oid.hash)) {
+ oid_eq(&dst_other.oid, &ren1->pair->two->oid)) {
/*
* Added file on the other side identical to
* the file being renamed: clean merge.
@@ -1453,12 +1453,12 @@ static int process_renames(struct merge_options *o,
* update_file().
*/
update_file_flags(o,
- ren1->pair->two->oid.hash,
+ &ren1->pair->two->oid,
ren1->pair->two->mode,
ren1_dst,
1, /* update_cache */
0 /* update_wd */);
- } else if (!sha_eq(dst_other.oid.hash, null_sha1)) {
+ } else if (!oid_eq(&dst_other.oid, &null_oid)) {
clean_merge = 0;
try_merge = 1;
output(o, 1, _("CONFLICT (rename/add): Rename %s->%s in %s. "
@@ -1467,20 +1467,20 @@ static int process_renames(struct merge_options *o,
ren1_dst, branch2);
if (o->call_depth) {
struct merge_file_info mfi;
- mfi = merge_file_one(o, ren1_dst, null_sha1, 0,
- ren1->pair->two->oid.hash,
+ mfi = merge_file_one(o, ren1_dst, &null_oid, 0,
+ &ren1->pair->two->oid,
ren1->pair->two->mode,
- dst_other.oid.hash,
+ &dst_other.oid,
dst_other.mode,
branch1, branch2);
output(o, 1, _("Adding merged %s"), ren1_dst);
- update_file(o, 0, mfi.oid.hash,
+ update_file(o, 0, &mfi.oid,
mfi.mode, ren1_dst);
try_merge = 0;
} else {
char *new_path = unique_path(o, ren1_dst, branch2);
output(o, 1, _("Adding as %s instead"), new_path);
- update_file(o, 0, dst_other.oid.hash,
+ update_file(o, 0, &dst_other.oid,
dst_other.mode, new_path);
free(new_path);
}
@@ -1519,30 +1519,30 @@ static int process_renames(struct merge_options *o,
return clean_merge;
}
-static unsigned char *stage_sha(const unsigned char *sha, unsigned mode)
+static struct object_id *stage_oid(const struct object_id *oid, unsigned mode)
{
- return (is_null_sha1(sha) || mode == 0) ? NULL: (unsigned char *)sha;
+ return (is_null_oid(oid) || mode == 0) ? NULL: (struct object_id *)oid;
}
-static int read_sha1_strbuf(const unsigned char *sha1, struct strbuf *dst)
+static int read_oid_strbuf(const struct object_id *oid, struct strbuf *dst)
{
void *buf;
enum object_type type;
unsigned long size;
- buf = read_sha1_file(sha1, &type, &size);
+ buf = read_sha1_file(oid->hash, &type, &size);
if (!buf)
- return error(_("cannot read object %s"), sha1_to_hex(sha1));
+ return error(_("cannot read object %s"), oid_to_hex(oid));
if (type != OBJ_BLOB) {
free(buf);
- return error(_("object %s is not a blob"), sha1_to_hex(sha1));
+ return error(_("object %s is not a blob"), oid_to_hex(oid));
}
strbuf_attach(dst, buf, size, size + 1);
return 0;
}
-static int blob_unchanged(const unsigned char *o_sha,
+static int blob_unchanged(const struct object_id *o_oid,
unsigned o_mode,
- const unsigned char *a_sha,
+ const struct object_id *a_oid,
unsigned a_mode,
int renormalize, const char *path)
{
@@ -1552,13 +1552,13 @@ static int blob_unchanged(const unsigned char *o_sha,
if (a_mode != o_mode)
return 0;
- if (sha_eq(o_sha, a_sha))
+ if (oid_eq(o_oid, a_oid))
return 1;
if (!renormalize)
return 0;
- assert(o_sha && a_sha);
- if (read_sha1_strbuf(o_sha, &o) || read_sha1_strbuf(a_sha, &a))
+ assert(o_oid && a_oid);
+ if (read_oid_strbuf(o_oid, &o) || read_oid_strbuf(a_oid, &a))
goto error_return;
/*
* Note: binary | is used so that both renormalizations are
@@ -1577,23 +1577,23 @@ error_return:
static void handle_modify_delete(struct merge_options *o,
const char *path,
- unsigned char *o_sha, int o_mode,
- unsigned char *a_sha, int a_mode,
- unsigned char *b_sha, int b_mode)
+ struct object_id *o_oid, int o_mode,
+ struct object_id *a_oid, int a_mode,
+ struct object_id *b_oid, int b_mode)
{
handle_change_delete(o,
path,
- o_sha, o_mode,
- a_sha, a_mode,
- b_sha, b_mode,
+ o_oid, o_mode,
+ a_oid, a_mode,
+ b_oid, b_mode,
_("modify"), _("modified"));
}
static int merge_content(struct merge_options *o,
const char *path,
- unsigned char *o_sha, int o_mode,
- unsigned char *a_sha, int a_mode,
- unsigned char *b_sha, int b_mode,
+ struct object_id *o_oid, int o_mode,
+ struct object_id *a_oid, int a_mode,
+ struct object_id *b_oid, int b_mode,
struct rename_conflict_info *rename_conflict_info)
{
const char *reason = _("content");
@@ -1602,16 +1602,16 @@ static int merge_content(struct merge_options *o,
struct diff_filespec one, a, b;
unsigned df_conflict_remains = 0;
- if (!o_sha) {
+ if (!o_oid) {
reason = _("add/add");
- o_sha = (unsigned char *)null_sha1;
+ o_oid = (struct object_id *)&null_oid;
}
one.path = a.path = b.path = (char *)path;
- hashcpy(one.oid.hash, o_sha);
+ oidcpy(&one.oid, o_oid);
one.mode = o_mode;
- hashcpy(a.oid.hash, a_sha);
+ oidcpy(&a.oid, a_oid);
a.mode = a_mode;
- hashcpy(b.oid.hash, b_sha);
+ oidcpy(&b.oid, b_oid);
b.mode = b_mode;
if (rename_conflict_info) {
@@ -1635,7 +1635,7 @@ static int merge_content(struct merge_options *o,
o->branch2, path2);
if (mfi.clean && !df_conflict_remains &&
- sha_eq(mfi.oid.hash, a_sha) && mfi.mode == a_mode) {
+ oid_eq(&mfi.oid, a_oid) && mfi.mode == a_mode) {
int path_renamed_outside_HEAD;
output(o, 3, _("Skipped %s (merged same as existing)"), path);
/*
@@ -1646,7 +1646,7 @@ static int merge_content(struct merge_options *o,
*/
path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
if (!path_renamed_outside_HEAD) {
- add_cacheinfo(mfi.mode, mfi.oid.hash, path,
+ add_cacheinfo(mfi.mode, &mfi.oid, path,
0, (!o->call_depth), 0);
return mfi.clean;
}
@@ -1683,11 +1683,11 @@ static int merge_content(struct merge_options *o,
}
new_path = unique_path(o, path, rename_conflict_info->branch1);
output(o, 1, _("Adding as %s instead"), new_path);
- update_file(o, 0, mfi.oid.hash, mfi.mode, new_path);
+ update_file(o, 0, &mfi.oid, mfi.mode, new_path);
free(new_path);
mfi.clean = 0;
} else {
- update_file(o, mfi.clean, mfi.oid.hash, mfi.mode, path);
+ update_file(o, mfi.clean, &mfi.oid, mfi.mode, path);
}
return mfi.clean;
@@ -1702,9 +1702,9 @@ static int process_entry(struct merge_options *o,
unsigned o_mode = entry->stages[1].mode;
unsigned a_mode = entry->stages[2].mode;
unsigned b_mode = entry->stages[3].mode;
- unsigned char *o_sha = stage_sha(entry->stages[1].oid.hash, o_mode);
- unsigned char *a_sha = stage_sha(entry->stages[2].oid.hash, a_mode);
- unsigned char *b_sha = stage_sha(entry->stages[3].oid.hash, b_mode);
+ struct object_id *o_oid = stage_oid(&entry->stages[1].oid, o_mode);
+ struct object_id *a_oid = stage_oid(&entry->stages[2].oid, a_mode);
+ struct object_id *b_oid = stage_oid(&entry->stages[3].oid, b_mode);
entry->processed = 1;
if (entry->rename_conflict_info) {
@@ -1713,7 +1713,7 @@ static int process_entry(struct merge_options *o,
case RENAME_NORMAL:
case RENAME_ONE_FILE_TO_ONE:
clean_merge = merge_content(o, path,
- o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
+ o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
conflict_info);
break;
case RENAME_DELETE:
@@ -1734,45 +1734,45 @@ static int process_entry(struct merge_options *o,
entry->processed = 0;
break;
}
- } else if (o_sha && (!a_sha || !b_sha)) {
+ } else if (o_oid && (!a_oid || !b_oid)) {
/* Case A: Deleted in one */
- if ((!a_sha && !b_sha) ||
- (!b_sha && blob_unchanged(o_sha, o_mode, a_sha, a_mode, normalize, path)) ||
- (!a_sha && blob_unchanged(o_sha, o_mode, b_sha, b_mode, normalize, path))) {
+ if ((!a_oid && !b_oid) ||
+ (!b_oid && blob_unchanged(o_oid, o_mode, a_oid, a_mode, normalize, path)) ||
+ (!a_oid && blob_unchanged(o_oid, o_mode, b_oid, b_mode, normalize, path))) {
/* Deleted in both or deleted in one and
* unchanged in the other */
- if (a_sha)
+ if (a_oid)
output(o, 2, _("Removing %s"), path);
/* do not touch working file if it did not exist */
- remove_file(o, 1, path, !a_sha);
+ remove_file(o, 1, path, !a_oid);
} else {
/* Modify/delete; deleted side may have put a directory in the way */
clean_merge = 0;
- handle_modify_delete(o, path, o_sha, o_mode,
- a_sha, a_mode, b_sha, b_mode);
+ handle_modify_delete(o, path, o_oid, o_mode,
+ a_oid, a_mode, b_oid, b_mode);
}
- } else if ((!o_sha && a_sha && !b_sha) ||
- (!o_sha && !a_sha && b_sha)) {
+ } else if ((!o_oid && a_oid && !b_oid) ||
+ (!o_oid && !a_oid && b_oid)) {
/* Case B: Added in one. */
/* [nothing|directory] -> ([nothing|directory], file) */
const char *add_branch;
const char *other_branch;
unsigned mode;
- const unsigned char *sha;
+ const struct object_id *oid;
const char *conf;
- if (a_sha) {
+ if (a_oid) {
add_branch = o->branch1;
other_branch = o->branch2;
mode = a_mode;
- sha = a_sha;
+ oid = a_oid;
conf = _("file/directory");
} else {
add_branch = o->branch2;
other_branch = o->branch1;
mode = b_mode;
- sha = b_sha;
+ oid = b_oid;
conf = _("directory/file");
}
if (dir_in_way(path, !o->call_depth)) {
@@ -1781,22 +1781,22 @@ static int process_entry(struct merge_options *o,
output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s"),
conf, path, other_branch, path, new_path);
- update_file(o, 0, sha, mode, new_path);
+ update_file(o, 0, oid, mode, new_path);
if (o->call_depth)
remove_file_from_cache(path);
free(new_path);
} else {
output(o, 2, _("Adding %s"), path);
/* do not overwrite file if already present */
- update_file_flags(o, sha, mode, path, 1, !a_sha);
+ update_file_flags(o, oid, mode, path, 1, !a_oid);
}
- } else if (a_sha && b_sha) {
+ } else if (a_oid && b_oid) {
/* Case C: Added in both (check for same permissions) and */
/* case D: Modified in both, but differently. */
clean_merge = merge_content(o, path,
- o_sha, o_mode, a_sha, a_mode, b_sha, b_mode,
+ o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,
NULL);
- } else if (!o_sha && !a_sha && !b_sha) {
+ } else if (!o_oid && !a_oid && !b_oid) {
/*
* this entry was deleted altogether. a_mode == 0 means
* we had that path and want to actively remove it.
@@ -1821,7 +1821,7 @@ int merge_trees(struct merge_options *o,
common = shift_tree_object(head, common, o->subtree_shift);
}
- if (sha_eq(common->object.oid.hash, merge->object.oid.hash)) {
+ if (oid_eq(&common->object.oid, &merge->object.oid)) {
output(o, 0, _("Already up-to-date!"));
*result = head;
return 1;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 8/8] merge-recursive: convert merge_recursive_generic to object_id
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (6 preceding siblings ...)
2016-06-18 22:14 ` [PATCH v2 7/8] merge-recursive: convert leaf functions to use struct object_id brian m. carlson
@ 2016-06-18 22:14 ` brian m. carlson
2016-06-19 8:50 ` [PATCH v2 0/8] object_id part 4 Johannes Sixt
2016-06-21 21:22 ` Junio C Hamano
9 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-18 22:14 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Convert this function and the git merge-recursive subcommand to use
struct object_id.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/merge-recursive.c | 20 ++++++++++----------
merge-recursive.c | 14 +++++++-------
merge-recursive.h | 6 +++---
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index 491efd55..fd2c4556 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -9,10 +9,10 @@ static const char builtin_merge_recursive_usage[] =
static const char *better_branch_name(const char *branch)
{
- static char githead_env[8 + 40 + 1];
+ static char githead_env[8 + GIT_SHA1_HEXSZ + 1];
char *name;
- if (strlen(branch) != 40)
+ if (strlen(branch) != GIT_SHA1_HEXSZ)
return branch;
xsnprintf(githead_env, sizeof(githead_env), "GITHEAD_%s", branch);
name = getenv(githead_env);
@@ -21,10 +21,10 @@ static const char *better_branch_name(const char *branch)
int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
{
- const unsigned char *bases[21];
+ const struct object_id *bases[21];
unsigned bases_count = 0;
int i, failed;
- unsigned char h1[20], h2[20];
+ struct object_id h1, h2;
struct merge_options o;
struct commit *result;
@@ -46,10 +46,10 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
continue;
}
if (bases_count < ARRAY_SIZE(bases)-1) {
- unsigned char *sha = xmalloc(20);
- if (get_sha1(argv[i], sha))
+ struct object_id *oid = xmalloc(sizeof(struct object_id));
+ if (get_oid(argv[i], oid))
die("Could not parse object '%s'", argv[i]);
- bases[bases_count++] = sha;
+ bases[bases_count++] = oid;
}
else
warning("Cannot handle more than %d bases. "
@@ -62,9 +62,9 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
o.branch1 = argv[++i];
o.branch2 = argv[++i];
- if (get_sha1(o.branch1, h1))
+ if (get_oid(o.branch1, &h1))
die("Could not resolve ref '%s'", o.branch1);
- if (get_sha1(o.branch2, h2))
+ if (get_oid(o.branch2, &h2))
die("Could not resolve ref '%s'", o.branch2);
o.branch1 = better_branch_name(o.branch1);
@@ -73,7 +73,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
if (o.verbosity >= 3)
printf("Merging %s with %s\n", o.branch1, o.branch2);
- failed = merge_recursive_generic(&o, h1, h2, bases_count, bases, &result);
+ failed = merge_recursive_generic(&o, &h1, &h2, bases_count, bases, &result);
if (failed < 0)
return 128; /* die() error code */
return failed;
diff --git a/merge-recursive.c b/merge-recursive.c
index 7bbd4aea..48fe7e73 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1982,11 +1982,11 @@ int merge_recursive(struct merge_options *o,
return clean;
}
-static struct commit *get_ref(const unsigned char *sha1, const char *name)
+static struct commit *get_ref(const struct object_id *oid, const char *name)
{
struct object *object;
- object = deref_tag(parse_object(sha1), name, strlen(name));
+ object = deref_tag(parse_object(oid->hash), name, strlen(name));
if (!object)
return NULL;
if (object->type == OBJ_TREE)
@@ -1999,10 +1999,10 @@ static struct commit *get_ref(const unsigned char *sha1, const char *name)
}
int merge_recursive_generic(struct merge_options *o,
- const unsigned char *head,
- const unsigned char *merge,
+ const struct object_id *head,
+ const struct object_id *merge,
int num_base_list,
- const unsigned char **base_list,
+ const struct object_id **base_list,
struct commit **result)
{
int clean;
@@ -2015,9 +2015,9 @@ int merge_recursive_generic(struct merge_options *o,
int i;
for (i = 0; i < num_base_list; ++i) {
struct commit *base;
- if (!(base = get_ref(base_list[i], sha1_to_hex(base_list[i]))))
+ if (!(base = get_ref(base_list[i], oid_to_hex(base_list[i]))))
return error(_("Could not parse object '%s'"),
- sha1_to_hex(base_list[i]));
+ oid_to_hex(base_list[i]));
commit_list_insert(base, &ca);
}
}
diff --git a/merge-recursive.h b/merge-recursive.h
index 52f0201f..d415724a 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -49,10 +49,10 @@ int merge_trees(struct merge_options *o,
* virtual commits and call merge_recursive() proper.
*/
int merge_recursive_generic(struct merge_options *o,
- const unsigned char *head,
- const unsigned char *merge,
+ const struct object_id *head,
+ const struct object_id *merge,
int num_ca,
- const unsigned char **ca,
+ const struct object_id **ca,
struct commit **result);
void init_merge_options(struct merge_options *o);
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (7 preceding siblings ...)
2016-06-18 22:14 ` [PATCH v2 8/8] merge-recursive: convert merge_recursive_generic to object_id brian m. carlson
@ 2016-06-19 8:50 ` Johannes Sixt
2016-06-19 9:24 ` Jeff King
2016-06-21 21:22 ` Junio C Hamano
9 siblings, 1 reply; 20+ messages in thread
From: Johannes Sixt @ 2016-06-19 8:50 UTC (permalink / raw)
To: brian m. carlson
Cc: git, Elijah Newren, Junio C Hamano, Stefan Beller, Jeff King
Am 19.06.2016 um 00:13 schrieb brian m. carlson:
> * Adjust the Coccinelle patches to transform plain structs before
> pointers to structs to avoid misconversions. This addresses the issue
> that Peff caught originally.
To avoid future mistakes, can you write down how "transform plain
structs before pointers to structs" looks like? Is it a particular order
of Coccinelle rules? Which part of the interdiff between the previous
round and this round makes the difference?
On a tangent, I wondered recently, why we need oidcpy() and oidclr().
After all, in place of, e.g.,
oidcpy(&pair->two->oid, &p->oid);
oidclr(&one->oid);
we can write
pair->two->oid = p->oid;
one->oid = null_oid;
Is there a particular reason *not* to make this transition? I find the
latter less cluttered with equal clarity.
-- Hannes
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-19 8:50 ` [PATCH v2 0/8] object_id part 4 Johannes Sixt
@ 2016-06-19 9:24 ` Jeff King
2016-06-19 17:25 ` brian m. carlson
2016-06-20 7:01 ` Johannes Schindelin
0 siblings, 2 replies; 20+ messages in thread
From: Jeff King @ 2016-06-19 9:24 UTC (permalink / raw)
To: Johannes Sixt
Cc: brian m. carlson, git, Elijah Newren, Junio C Hamano, Stefan Beller
On Sun, Jun 19, 2016 at 10:50:14AM +0200, Johannes Sixt wrote:
> Am 19.06.2016 um 00:13 schrieb brian m. carlson:
> > * Adjust the Coccinelle patches to transform plain structs before
> > pointers to structs to avoid misconversions. This addresses the issue
> > that Peff caught originally.
>
> To avoid future mistakes, can you write down how "transform plain structs
> before pointers to structs" looks like? Is it a particular order of
> Coccinelle rules? Which part of the interdiff between the previous round and
> this round makes the difference?
Yeah, I'd also like a better understanding of what went wrong in the
original (just to be able to better evaluate the tool).
Also, for the record, the issue was noticed by Johannes originally, not
me, as indicated above (I just found a similar case after seeing his
report).
> On a tangent, I wondered recently, why we need oidcpy() and oidclr(). After
> all, in place of, e.g.,
>
> oidcpy(&pair->two->oid, &p->oid);
> oidclr(&one->oid);
>
> we can write
>
> pair->two->oid = p->oid;
> one->oid = null_oid;
>
> Is there a particular reason *not* to make this transition? I find the
> latter less cluttered with equal clarity.
I think traditionally we've avoided struct assignment because some
ancient compilers didn't do it. But it's in C89, and I suspect it's
crept into the code base anyway over the years without anyone
complaining.
-Peff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-19 9:24 ` Jeff King
@ 2016-06-19 17:25 ` brian m. carlson
2016-06-20 7:01 ` Johannes Schindelin
1 sibling, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-19 17:25 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Sixt, git, Elijah Newren, Junio C Hamano, Stefan Beller
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]
On Sun, Jun 19, 2016 at 05:24:48AM -0400, Jeff King wrote:
> On Sun, Jun 19, 2016 at 10:50:14AM +0200, Johannes Sixt wrote:
> > To avoid future mistakes, can you write down how "transform plain structs
> > before pointers to structs" looks like? Is it a particular order of
> > Coccinelle rules? Which part of the interdiff between the previous round and
> > this round makes the difference?
>
> Yeah, I'd also like a better understanding of what went wrong in the
> original (just to be able to better evaluate the tool).
I'm not completely clear on why Coccinelle made the transform that it
did. If you look at the commit messages, the new patch is this:
@@
struct diff_filespec o;
@@
- o.sha1
+ o.oid.hash
@@
struct diff_filespec *p;
@@
- p->sha1
+ p->oid.hash
whereas the two pieces were reversed before. This fixes the problem
because it's never possible for the second transform to match after the
first has transformed a given piece of code. The functional interdiff
shows that only the issues the two of you found are changed.
> Also, for the record, the issue was noticed by Johannes originally, not
> me, as indicated above (I just found a similar case after seeing his
> report).
I apologize for the misattribution.
> > On a tangent, I wondered recently, why we need oidcpy() and oidclr(). After
> > all, in place of, e.g.,
> >
> > oidcpy(&pair->two->oid, &p->oid);
> > oidclr(&one->oid);
> >
> > we can write
> >
> > pair->two->oid = p->oid;
> > one->oid = null_oid;
> >
> > Is there a particular reason *not* to make this transition? I find the
> > latter less cluttered with equal clarity.
>
> I think traditionally we've avoided struct assignment because some
> ancient compilers didn't do it. But it's in C89, and I suspect it's
> crept into the code base anyway over the years without anyone
> complaining.
I asked about this earlier and Junio felt that struct assignment was
less desirable.
Also, if we do struct assignment, if we add a new hash, we may copy more
than necessary (e.g. 64 bytes when we're only using SHA-1); however,
this may be offset by the ability of the compiler to compute the offset
at compile time.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-19 9:24 ` Jeff King
2016-06-19 17:25 ` brian m. carlson
@ 2016-06-20 7:01 ` Johannes Schindelin
2016-06-20 10:05 ` Jeff King
1 sibling, 1 reply; 20+ messages in thread
From: Johannes Schindelin @ 2016-06-20 7:01 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Sixt, brian m. carlson, git, Elijah Newren,
Junio C Hamano, Stefan Beller
Hi Peff,
On Sun, 19 Jun 2016, Jeff King wrote:
> I think traditionally we've avoided struct assignment because some
> ancient compilers didn't do it. But it's in C89, and I suspect it's
> crept into the code base anyway over the years without anyone
> complaining.
I fear that's my fault, at least partially, seeing as merge-recursive.c
even *returns* structs (see 6d297f81; I plan to fix that as part of the
cleaned-up am-3-merge-recursive-direct patch series).
Sorry,
Dscho
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-20 7:01 ` Johannes Schindelin
@ 2016-06-20 10:05 ` Jeff King
2016-06-20 15:53 ` Junio C Hamano
0 siblings, 1 reply; 20+ messages in thread
From: Jeff King @ 2016-06-20 10:05 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Johannes Sixt, brian m. carlson, git, Elijah Newren,
Junio C Hamano, Stefan Beller
On Mon, Jun 20, 2016 at 09:01:30AM +0200, Johannes Schindelin wrote:
> On Sun, 19 Jun 2016, Jeff King wrote:
>
> > I think traditionally we've avoided struct assignment because some
> > ancient compilers didn't do it. But it's in C89, and I suspect it's
> > crept into the code base anyway over the years without anyone
> > complaining.
>
> I fear that's my fault, at least partially, seeing as merge-recursive.c
> even *returns* structs (see 6d297f81; I plan to fix that as part of the
> cleaned-up am-3-merge-recursive-direct patch series).
Heh, that commit is quite old. If nobody has complained about it, then I
think there is nothing to be sorry about. If struct assignment (and
returns) work everywhere, and they make the code easier to read, we
should be using them.
I am on the fence regarding oidcpy/oidclr. I agree they _could_ be
struct assignments, but it is also convenient to have concept wrapped up
in a function, in case we ever want to do anything more complicated.
-Peff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-20 10:05 ` Jeff King
@ 2016-06-20 15:53 ` Junio C Hamano
0 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2016-06-20 15:53 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, Johannes Sixt, brian m. carlson, git,
Elijah Newren, Stefan Beller
Jeff King <peff@peff.net> writes:
> I am on the fence regarding oidcpy/oidclr. I agree they _could_ be
> struct assignments, but it is also convenient to have concept wrapped up
> in a function, in case we ever want to do anything more complicated.
Also dedicated functions have documenation value. There are some
things that currently use the same 40-hex that is the result of
running SHA-1 but are not object names (e.g. patch id, and rerere
id). They use the same hashcpy()/hashcmp() helpers as object names
do, but the code that use them probably do not want to be converted
to struct oid and oidcpy().
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
` (8 preceding siblings ...)
2016-06-19 8:50 ` [PATCH v2 0/8] object_id part 4 Johannes Sixt
@ 2016-06-21 21:22 ` Junio C Hamano
2016-06-22 18:44 ` brian m. carlson
9 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2016-06-21 21:22 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Elijah Newren, Stefan Beller, Jeff King
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> This series is part 4 in a series of conversions to replace instances of
> unsigned char [20] with struct object_id. Most of this series touches
> the merge-recursive code.
I've queued them in 'pu', but please read contrib/examples/README
;-) Tentatively, I used contrib/coccinelle instead, but something
even shorter (e.g. contrib/cocci) might be sufficient.
> Note that in the patches created with the semantic patches, the only manual
> change was the definition of the struct member. Opinions on whether this is a
> viable technique for further work to ease both the creation and review of
> patches are of course welcomed.
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/8] Apply object_id Coccinelle transformations.
2016-06-18 22:14 ` [PATCH v2 2/8] Apply object_id Coccinelle transformations brian m. carlson
@ 2016-06-21 21:36 ` Junio C Hamano
0 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2016-06-21 21:36 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Elijah Newren, Stefan Beller, Jeff King
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> Apply the set of semantic patches from contrib/examples/coccinelle to
> convert some leftover places using struct object_id's hash member to
> instead use the wrapper functions that take struct object_id natively.
It is somewhat curious how these 'some leftover places' are chosen.
Especially, this hunk was interesting.
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 1f380764..dac3a222 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1725,14 +1725,14 @@ static int verify_lock(struct ref_lock *lock,
> errno = save_errno;
> return -1;
> } else {
> - hashclr(lock->old_oid.hash);
> + oidclr(&lock->old_oid);
> return 0;
> }
> }
> if (old_sha1 && hashcmp(lock->old_oid.hash, old_sha1)) {
> strbuf_addf(err, "ref %s is at %s but expected %s",
> lock->ref_name,
> - sha1_to_hex(lock->old_oid.hash),
> + oid_to_hex(&lock->old_oid),
> sha1_to_hex(old_sha1));
> errno = EBUSY;
> return -1;
The function gets old_sha1 which is the old-world "const unsigned
char *" that is passed via lock_ref_sha1_basic() from public entry
points like rename_ref() and ref_transaction_commit(). As this
codepath and the functions involved have not be converted to oid,
we end up seeing oid_to_hex() next to sha1_to_hex() ;-)
Not a complaint; this is how we make progress incrementally.
It was just I noticed this function is left in a somewhat funny
intermediate state after this step.
Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/8] Convert struct diff_filespec to struct object_id
2016-06-18 22:14 ` [PATCH v2 3/8] Convert struct diff_filespec to struct object_id brian m. carlson
@ 2016-06-21 22:22 ` Junio C Hamano
2016-06-24 15:27 ` brian m. carlson
0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2016-06-21 22:22 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Elijah Newren, Stefan Beller, Jeff King
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
I was trying to make sure there is no misconversion, but some lines
that got wrapped were distracting. For example:
> @@ -2721,7 +2722,8 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
> if (s->dirty_submodule)
> dirty = "-dirty";
>
> - strbuf_addf(&buf, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty);
> + strbuf_addf(&buf, "Subproject commit %s%s\n",
> + oid_to_hex(&s->oid), dirty);
This would have been
> - strbuf_addf(&buf, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty);
> + strbuf_addf(&buf, "Subproject commit %s%s\n", oid_to_hex(&s->oid), dirty);
which the conversion made the line _shorter_. If the original's
line length was acceptable, there is no reason to wrap the result.
> - die("unable to read %s", sha1_to_hex(s->sha1));
> + die("unable to read %s",
> + oid_to_hex(&s->oid));
Likewise.
> @@ -2937,7 +2940,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
> if (!one->sha1_valid)
> sha1_to_hex_r(temp->hex, null_sha1);
> else
> - sha1_to_hex_r(temp->hex, one->sha1);
> + sha1_to_hex_r(temp->hex, one->oid.hash);
This suggests that oid_to_hex_r() is needed, perhaps?
> @@ -2952,7 +2955,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
> if (diff_populate_filespec(one, 0))
> die("cannot read data blob for %s", one->path);
> prep_temp_blob(name, temp, one->data, one->size,
> - one->sha1, one->mode);
> + one->oid.hash, one->mode);
prep_temp_blob() is a file-scope static with only two callers. It
probably would not take too much effort to make it take &one->oid
instead?
> @@ -3075,8 +3078,8 @@ static void fill_metainfo(struct strbuf *msg,
> abbrev = 40;
> }
> strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
> - find_unique_abbrev(one->sha1, abbrev));
> - strbuf_addstr(msg, find_unique_abbrev(two->sha1, abbrev));
> + find_unique_abbrev(one->oid.hash, abbrev));
> + strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
Good. As more and more callers of find_unique_abbrev() is converted
to pass oid.hash to it, eventually we will have only a handful of
callers that have a raw "const unsigned char sha1[20]" to pass it,
and we can eventually make the function take &oid. It seems we are
not quite there yet, by the looks of 'git grep find_unique_abbrev'
output, but we are getting closer.
> @@ -3134,17 +3137,17 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
> if (!one->sha1_valid) {
> struct stat st;
> if (one->is_stdin) {
> - hashcpy(one->sha1, null_sha1);
> + hashcpy(one->oid.hash, null_sha1);
> return;
> }
oidclr()?
Perhaps a preparatory step of
unsigned char *E1;
-hashcpy(E1, null_sha1);
+hashclr(E1)
would help?
> @@ -902,13 +904,13 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
> result.clean = 0;
> if (S_ISREG(a->mode)) {
> result.mode = a->mode;
> - hashcpy(result.sha, a->sha1);
> + hashcpy(result.sha, a->oid.hash);
> } else {
> result.mode = b->mode;
> - hashcpy(result.sha, b->sha1);
> + hashcpy(result.sha, b->oid.hash);
merge_file_info is a file-scope-static type, and it shouldn't take
too much effort to replace its sha1 with an oid, I would think.
> - if (!sha_eq(a->sha1, one->sha1) && !sha_eq(b->sha1, one->sha1))
> + if (!sha_eq(a->oid.hash, one->oid.hash) && !sha_eq(b->oid.hash, one->oid.hash))
sha_eq() knows that either of its two parameters could be NULL, but
a->sha1 is diff_filespec.sha1 which cannot be NULL.
So !sha_eq() here can become oidcmp(). There are some calls to
sha_eq() that could pass NULL (e.g. a_sha could be NULL in
blob_unchanged()), but many other sha_eq() can become !oidcmp().
Am I reading the conversion correctly?
I'll stop here for now and will come back to the remainder of this
patch sometime later. Thanks.
> diff --git a/notes-merge.c b/notes-merge.c
> index 34bfac0c..62c23d8a 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/8] object_id part 4
2016-06-21 21:22 ` Junio C Hamano
@ 2016-06-22 18:44 ` brian m. carlson
0 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-22 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Elijah Newren, Stefan Beller, Jeff King
[-- Attachment #1: Type: text/plain, Size: 847 bytes --]
On Tue, Jun 21, 2016 at 02:22:50PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > This series is part 4 in a series of conversions to replace instances of
> > unsigned char [20] with struct object_id. Most of this series touches
> > the merge-recursive code.
>
> I've queued them in 'pu', but please read contrib/examples/README
> ;-) Tentatively, I used contrib/coccinelle instead, but something
> even shorter (e.g. contrib/cocci) might be sufficient.
That seems fine with me. I did read contrib/examples/README, but
figured it might be okay nevertheless. I think contrib/coccinelle is
better, though.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/8] Convert struct diff_filespec to struct object_id
2016-06-21 22:22 ` Junio C Hamano
@ 2016-06-24 15:27 ` brian m. carlson
0 siblings, 0 replies; 20+ messages in thread
From: brian m. carlson @ 2016-06-24 15:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Elijah Newren, Stefan Beller, Jeff King
[-- Attachment #1: Type: text/plain, Size: 4875 bytes --]
On Tue, Jun 21, 2016 at 03:22:04PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> I was trying to make sure there is no misconversion, but some lines
> that got wrapped were distracting. For example:
>
> > @@ -2721,7 +2722,8 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
> > if (s->dirty_submodule)
> > dirty = "-dirty";
> >
> > - strbuf_addf(&buf, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty);
> > + strbuf_addf(&buf, "Subproject commit %s%s\n",
> > + oid_to_hex(&s->oid), dirty);
>
> This would have been
>
> > - strbuf_addf(&buf, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty);
> > + strbuf_addf(&buf, "Subproject commit %s%s\n", oid_to_hex(&s->oid), dirty);
>
> which the conversion made the line _shorter_. If the original's
> line length was acceptable, there is no reason to wrap the result.
I do tend to agree. Coccinelle wrapped the line automatically, but I'm
not sure why it did that. I can see if there's an option that disables
that if you'd like. I'm a bit reticent to manually fix up the line
wrapping as I want others to be able to run Coccinelle themselves and
get identical output.
> > @@ -2937,7 +2940,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
> > if (!one->sha1_valid)
> > sha1_to_hex_r(temp->hex, null_sha1);
> > else
> > - sha1_to_hex_r(temp->hex, one->sha1);
> > + sha1_to_hex_r(temp->hex, one->oid.hash);
>
> This suggests that oid_to_hex_r() is needed, perhaps?
Probably so. I can add that in a reroll.
> > @@ -2952,7 +2955,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
> > if (diff_populate_filespec(one, 0))
> > die("cannot read data blob for %s", one->path);
> > prep_temp_blob(name, temp, one->data, one->size,
> > - one->sha1, one->mode);
> > + one->oid.hash, one->mode);
>
> prep_temp_blob() is a file-scope static with only two callers. It
> probably would not take too much effort to make it take &one->oid
> instead?
I can certainly do that in a reroll.
> > @@ -3075,8 +3078,8 @@ static void fill_metainfo(struct strbuf *msg,
> > abbrev = 40;
> > }
> > strbuf_addf(msg, "%s%sindex %s..", line_prefix, set,
> > - find_unique_abbrev(one->sha1, abbrev));
> > - strbuf_addstr(msg, find_unique_abbrev(two->sha1, abbrev));
> > + find_unique_abbrev(one->oid.hash, abbrev));
> > + strbuf_addstr(msg, find_unique_abbrev(two->oid.hash, abbrev));
>
> Good. As more and more callers of find_unique_abbrev() is converted
> to pass oid.hash to it, eventually we will have only a handful of
> callers that have a raw "const unsigned char sha1[20]" to pass it,
> and we can eventually make the function take &oid. It seems we are
> not quite there yet, by the looks of 'git grep find_unique_abbrev'
> output, but we are getting closer.
Yes, I'd noticed that as well. One thing I observed from these
conversions is that it sometimes takes a huge amount of work to get all
the callers to change. Often, it's only one or two call sites that end
up being a bunch of work.
> > @@ -3134,17 +3137,17 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
> > if (!one->sha1_valid) {
> > struct stat st;
> > if (one->is_stdin) {
> > - hashcpy(one->sha1, null_sha1);
> > + hashcpy(one->oid.hash, null_sha1);
> > return;
> > }
>
> oidclr()?
>
> Perhaps a preparatory step of
>
> unsigned char *E1;
>
> -hashcpy(E1, null_sha1);
> +hashclr(E1)
>
> would help?
Sure. I can do that.
> > @@ -902,13 +904,13 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
> > result.clean = 0;
> > if (S_ISREG(a->mode)) {
> > result.mode = a->mode;
> > - hashcpy(result.sha, a->sha1);
> > + hashcpy(result.sha, a->oid.hash);
> > } else {
> > result.mode = b->mode;
> > - hashcpy(result.sha, b->sha1);
> > + hashcpy(result.sha, b->oid.hash);
>
> merge_file_info is a file-scope-static type, and it shouldn't take
> too much effort to replace its sha1 with an oid, I would think.
That's one of the following patches.
> sha_eq() knows that either of its two parameters could be NULL, but
> a->sha1 is diff_filespec.sha1 which cannot be NULL.
>
> So !sha_eq() here can become oidcmp(). There are some calls to
> sha_eq() that could pass NULL (e.g. a_sha could be NULL in
> blob_unchanged()), but many other sha_eq() can become !oidcmp().
>
> Am I reading the conversion correctly?
I think that's accurate. I'll make that change.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-06-24 15:28 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-18 22:13 [PATCH v2 0/8] object_id part 4 brian m. carlson
2016-06-18 22:14 ` [PATCH v2 1/8] Add basic Coccinelle transforms brian m. carlson
2016-06-18 22:14 ` [PATCH v2 2/8] Apply object_id Coccinelle transformations brian m. carlson
2016-06-21 21:36 ` Junio C Hamano
2016-06-18 22:14 ` [PATCH v2 3/8] Convert struct diff_filespec to struct object_id brian m. carlson
2016-06-21 22:22 ` Junio C Hamano
2016-06-24 15:27 ` brian m. carlson
2016-06-18 22:14 ` [PATCH v2 4/8] Rename struct diff_filespec's sha1_valid member brian m. carlson
2016-06-18 22:14 ` [PATCH v2 5/8] merge-recursive: convert struct stage_data to use object_id brian m. carlson
2016-06-18 22:14 ` [PATCH v2 6/8] merge-recursive: convert struct merge_file_info to object_id brian m. carlson
2016-06-18 22:14 ` [PATCH v2 7/8] merge-recursive: convert leaf functions to use struct object_id brian m. carlson
2016-06-18 22:14 ` [PATCH v2 8/8] merge-recursive: convert merge_recursive_generic to object_id brian m. carlson
2016-06-19 8:50 ` [PATCH v2 0/8] object_id part 4 Johannes Sixt
2016-06-19 9:24 ` Jeff King
2016-06-19 17:25 ` brian m. carlson
2016-06-20 7:01 ` Johannes Schindelin
2016-06-20 10:05 ` Jeff King
2016-06-20 15:53 ` Junio C Hamano
2016-06-21 21:22 ` Junio C Hamano
2016-06-22 18:44 ` brian m. carlson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.