From c5aa6db64f6f43621568bb1fec9360c25dbd2749 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 11:53:57 +0200 Subject: [PATCH 01/12] argv_array: offer to split a string by whitespace This is a simple function that will interpret a string as a whitespace delimited list of values, and add those values into the array. Note: this function does not (yet) offer to split by arbitrary delimiters, or keep empty values in case of runs of whitespace, or de-quote Unix shell style. All fo this functionality can be added later, when and if needed. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- argv-array.c | 20 ++++++++++++++++++++ argv-array.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/argv-array.c b/argv-array.c index 5d370fa336..cb5bcd2c06 100644 --- a/argv-array.c +++ b/argv-array.c @@ -64,6 +64,26 @@ void argv_array_pop(struct argv_array *array) array->argc--; } +void argv_array_split(struct argv_array *array, const char *to_split) +{ + while (isspace(*to_split)) + to_split++; + for (;;) { + const char *p = to_split; + + if (!*p) + break; + + while (*p && !isspace(*p)) + p++; + argv_array_push_nodup(array, xstrndup(to_split, p - to_split)); + + while (isspace(*p)) + p++; + to_split = p; + } +} + void argv_array_clear(struct argv_array *array) { if (array->argv != empty_argv) { diff --git a/argv-array.h b/argv-array.h index 29056e49a1..750c30d2f2 100644 --- a/argv-array.h +++ b/argv-array.h @@ -19,6 +19,8 @@ LAST_ARG_MUST_BE_NULL void argv_array_pushl(struct argv_array *, ...); void argv_array_pushv(struct argv_array *, const char **); void argv_array_pop(struct argv_array *); +/* Splits by whitespace; does not handle quoted arguments! */ +void argv_array_split(struct argv_array *, const char *); void argv_array_clear(struct argv_array *); const char **argv_array_detach(struct argv_array *); From fef461ea5d53dd84c6d946f57a018ffc9f391a05 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 11:54:04 +0200 Subject: [PATCH 02/12] commit: Let the callback of for_each_mergetag return on error This is yet another patch to be filed under the keyword "libification". There is one subtle change in behavior here, where a `git log` that has been asked to show the mergetags would now stop reporting the mergetags upon the first failure, whereas previously, it would have continued to the next mergetag, if any. In practice, that change should not matter, as it is 1) uncommon to perform octopus merges using multiple tags as merge heads, and 2) when the user asks to be shown those tags, they really should be there. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/replace.c | 8 ++++---- commit.c | 8 +++++--- commit.h | 4 ++-- log-tree.c | 13 +++++++------ 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 935647be6b..245d3f4164 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -345,7 +345,7 @@ struct check_mergetag_data { const char **argv; }; -static void check_one_mergetag(struct commit *commit, +static int check_one_mergetag(struct commit *commit, struct commit_extra_header *extra, void *data) { @@ -368,20 +368,20 @@ static void check_one_mergetag(struct commit *commit, if (get_oid(mergetag_data->argv[i], &oid) < 0) die(_("Not a valid object name: '%s'"), mergetag_data->argv[i]); if (!oidcmp(&tag->tagged->oid, &oid)) - return; /* found */ + return 0; /* found */ } die(_("original commit '%s' contains mergetag '%s' that is discarded; " "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid)); } -static void check_mergetags(struct commit *commit, int argc, const char **argv) +static int check_mergetags(struct commit *commit, int argc, const char **argv) { struct check_mergetag_data mergetag_data; mergetag_data.argc = argc; mergetag_data.argv = argv; - for_each_mergetag(check_one_mergetag, commit, &mergetag_data); + return for_each_mergetag(check_one_mergetag, commit, &mergetag_data); } static int create_graft(int argc, const char **argv, int force) diff --git a/commit.c b/commit.c index ca474a7c11..2952ec987c 100644 --- a/commit.c +++ b/commit.c @@ -1288,17 +1288,19 @@ struct commit_extra_header *read_commit_extra_headers(struct commit *commit, return extra; } -void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data) +int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data) { struct commit_extra_header *extra, *to_free; + int res = 0; to_free = read_commit_extra_headers(commit, NULL); - for (extra = to_free; extra; extra = extra->next) { + for (extra = to_free; !res && extra; extra = extra->next) { if (strcmp(extra->key, "mergetag")) continue; /* not a merge tag */ - fn(commit, extra, data); + res = fn(commit, extra, data); } free_commit_extra_headers(to_free); + return res; } static inline int standard_header_field(const char *field, size_t len) diff --git a/commit.h b/commit.h index 0fb8271665..9000895ad9 100644 --- a/commit.h +++ b/commit.h @@ -291,10 +291,10 @@ extern const char *find_commit_header(const char *msg, const char *key, /* Find the end of the log message, the right place for a new trailer. */ extern int ignore_non_trailer(const char *buf, size_t len); -typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra, +typedef int (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra, void *cb_data); -extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data); +extern int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data); struct merge_remote_desc { struct object *obj; /* the named object, could be a tag */ diff --git a/log-tree.c b/log-tree.c index d1c0bedf24..f3a51a6e72 100644 --- a/log-tree.c +++ b/log-tree.c @@ -488,9 +488,9 @@ static int is_common_merge(const struct commit *commit) && !commit->parents->next->next); } -static void show_one_mergetag(struct commit *commit, - struct commit_extra_header *extra, - void *data) +static int show_one_mergetag(struct commit *commit, + struct commit_extra_header *extra, + void *data) { struct rev_info *opt = (struct rev_info *)data; struct object_id oid; @@ -502,7 +502,7 @@ static void show_one_mergetag(struct commit *commit, hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid); tag = lookup_tag(&oid); if (!tag) - return; /* error message already given */ + return -1; /* error message already given */ strbuf_init(&verify_message, 256); if (parse_tag_buffer(tag, extra->value, extra->len)) @@ -536,11 +536,12 @@ static void show_one_mergetag(struct commit *commit, show_sig_lines(opt, status, verify_message.buf); strbuf_release(&verify_message); + return 0; } -static void show_mergetag(struct rev_info *opt, struct commit *commit) +static int show_mergetag(struct rev_info *opt, struct commit *commit) { - for_each_mergetag(show_one_mergetag, commit, opt); + return for_each_mergetag(show_one_mergetag, commit, opt); } void show_log(struct rev_info *opt) From d398f2ea001ab4e639896c81fb616810cf14eb12 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 25 Apr 2018 11:54:06 +0200 Subject: [PATCH 03/12] replace: avoid using die() to indicate a bug We have the BUG() macro for that purpose. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/replace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/replace.c b/builtin/replace.c index 245d3f4164..e345a5a0f1 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -501,6 +501,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) return list_replace_refs(argv[0], format); default: - die("BUG: invalid cmdmode %d", (int)cmdmode); + BUG("invalid cmdmode %d", (int)cmdmode); } } From e24e871920ac04473a470645e4c4dc53b422dd75 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:44:26 +0200 Subject: [PATCH 04/12] replace: "libify" create_graft() and callees File this away as yet another patch in the "libification" category. As with all useful functions, in the next commit we want to use create_graft() from a higher-level function where it would be inconvenient if the called function simply die()s: if there is a problem, we want to let the user know how to proceed, and the callee simply has no way of knowing what to say. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/replace.c | 167 ++++++++++++++++++++++++++++++---------------- 1 file changed, 111 insertions(+), 56 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index e345a5a0f1..e57d3d187e 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -79,9 +79,9 @@ static int list_replace_refs(const char *pattern, const char *format) else if (!strcmp(format, "long")) data.format = REPLACE_FORMAT_LONG; else - die("invalid replace format '%s'\n" - "valid formats are 'short', 'medium' and 'long'\n", - format); + return error("invalid replace format '%s'\n" + "valid formats are 'short', 'medium' and 'long'\n", + format); for_each_replace_ref(show_reference, (void *)&data); @@ -134,7 +134,7 @@ static int delete_replace_ref(const char *name, const char *ref, return 0; } -static void check_ref_valid(struct object_id *object, +static int check_ref_valid(struct object_id *object, struct object_id *prev, struct strbuf *ref, int force) @@ -142,12 +142,13 @@ static void check_ref_valid(struct object_id *object, strbuf_reset(ref); strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object)); if (check_refname_format(ref->buf, 0)) - die("'%s' is not a valid ref name.", ref->buf); + return error("'%s' is not a valid ref name.", ref->buf); if (read_ref(ref->buf, prev)) oidclr(prev); else if (!force) - die("replace ref '%s' already exists", ref->buf); + return error("replace ref '%s' already exists", ref->buf); + return 0; } static int replace_object_oid(const char *object_ref, @@ -161,28 +162,33 @@ static int replace_object_oid(const char *object_ref, struct strbuf ref = STRBUF_INIT; struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; + int res = 0; obj_type = oid_object_info(object, NULL); repl_type = oid_object_info(repl, NULL); if (!force && obj_type != repl_type) - die("Objects must be of the same type.\n" - "'%s' points to a replaced object of type '%s'\n" - "while '%s' points to a replacement object of type '%s'.", - object_ref, type_name(obj_type), - replace_ref, type_name(repl_type)); + return error("Objects must be of the same type.\n" + "'%s' points to a replaced object of type '%s'\n" + "while '%s' points to a replacement object of " + "type '%s'.", + object_ref, type_name(obj_type), + replace_ref, type_name(repl_type)); - check_ref_valid(object, &prev, &ref, force); + if (check_ref_valid(object, &prev, &ref, force)) { + strbuf_release(&ref); + return -1; + } transaction = ref_transaction_begin(&err); if (!transaction || ref_transaction_update(transaction, ref.buf, repl, &prev, 0, NULL, &err) || ref_transaction_commit(transaction, &err)) - die("%s", err.buf); + res = error("%s", err.buf); ref_transaction_free(transaction); strbuf_release(&ref); - return 0; + return res; } static int replace_object(const char *object_ref, const char *replace_ref, int force) @@ -190,9 +196,11 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f struct object_id object, repl; if (get_oid(object_ref, &object)) - die("Failed to resolve '%s' as a valid ref.", object_ref); + return error("Failed to resolve '%s' as a valid ref.", + object_ref); if (get_oid(replace_ref, &repl)) - die("Failed to resolve '%s' as a valid ref.", replace_ref); + return error("Failed to resolve '%s' as a valid ref.", + replace_ref); return replace_object_oid(object_ref, &object, replace_ref, &repl, force); } @@ -202,7 +210,7 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f * If "raw" is true, then the object's raw contents are printed according to * "type". Otherwise, we pretty-print the contents for human editing. */ -static void export_object(const struct object_id *oid, enum object_type type, +static int export_object(const struct object_id *oid, enum object_type type, int raw, const char *filename) { struct child_process cmd = CHILD_PROCESS_INIT; @@ -210,7 +218,7 @@ static void export_object(const struct object_id *oid, enum object_type type, fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666); if (fd < 0) - die_errno("unable to open %s for writing", filename); + return error_errno("unable to open %s for writing", filename); argv_array_push(&cmd.args, "--no-replace-objects"); argv_array_push(&cmd.args, "cat-file"); @@ -223,7 +231,8 @@ static void export_object(const struct object_id *oid, enum object_type type, cmd.out = fd; if (run_command(&cmd)) - die("cat-file reported failure"); + return error("cat-file reported failure"); + return 0; } /* @@ -231,14 +240,14 @@ static void export_object(const struct object_id *oid, enum object_type type, * interpreting it as "type", and writing the result to the object database. * The sha1 of the written object is returned via sha1. */ -static void import_object(struct object_id *oid, enum object_type type, +static int import_object(struct object_id *oid, enum object_type type, int raw, const char *filename) { int fd; fd = open(filename, O_RDONLY); if (fd < 0) - die_errno("unable to open %s for reading", filename); + return error_errno("unable to open %s for reading", filename); if (!raw && type == OBJ_TREE) { const char *argv[] = { "mktree", NULL }; @@ -250,27 +259,40 @@ static void import_object(struct object_id *oid, enum object_type type, cmd.in = fd; cmd.out = -1; - if (start_command(&cmd)) - die("unable to spawn mktree"); + if (start_command(&cmd)) { + close(fd); + return error("unable to spawn mktree"); + } - if (strbuf_read(&result, cmd.out, 41) < 0) - die_errno("unable to read from mktree"); + if (strbuf_read(&result, cmd.out, 41) < 0) { + error_errno("unable to read from mktree"); + close(fd); + close(cmd.out); + return -1; + } close(cmd.out); - if (finish_command(&cmd)) - die("mktree reported failure"); - if (get_oid_hex(result.buf, oid) < 0) - die("mktree did not return an object name"); + if (finish_command(&cmd)) { + strbuf_release(&result); + return error("mktree reported failure"); + } + if (get_oid_hex(result.buf, oid) < 0) { + strbuf_release(&result); + return error("mktree did not return an object name"); + } strbuf_release(&result); } else { struct stat st; int flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT; - if (fstat(fd, &st) < 0) - die_errno("unable to fstat %s", filename); + if (fstat(fd, &st) < 0) { + error_errno("unable to fstat %s", filename); + close(fd); + return -1; + } if (index_fd(oid, fd, &st, type, NULL, flags) < 0) - die("unable to write object to database"); + return error("unable to write object to database"); /* index_fd close()s fd for us */ } @@ -278,30 +300,43 @@ static void import_object(struct object_id *oid, enum object_type type, * No need to close(fd) here; both run-command and index-fd * will have done it for us. */ + return 0; } static int edit_and_replace(const char *object_ref, int force, int raw) { - char *tmpfile = git_pathdup("REPLACE_EDITOBJ"); + char *tmpfile; enum object_type type; struct object_id old_oid, new_oid, prev; struct strbuf ref = STRBUF_INIT; if (get_oid(object_ref, &old_oid) < 0) - die("Not a valid object name: '%s'", object_ref); + return error("Not a valid object name: '%s'", object_ref); type = oid_object_info(&old_oid, NULL); if (type < 0) - die("unable to get object type for %s", oid_to_hex(&old_oid)); + return error("unable to get object type for %s", + oid_to_hex(&old_oid)); - check_ref_valid(&old_oid, &prev, &ref, force); + if (check_ref_valid(&old_oid, &prev, &ref, force)) { + strbuf_release(&ref); + return -1; + } strbuf_release(&ref); - export_object(&old_oid, type, raw, tmpfile); - if (launch_editor(tmpfile, NULL, NULL) < 0) - die("editing object file failed"); - import_object(&new_oid, type, raw, tmpfile); - + tmpfile = git_pathdup("REPLACE_EDITOBJ"); + if (export_object(&old_oid, type, raw, tmpfile)) { + free(tmpfile); + return -1; + } + if (launch_editor(tmpfile, NULL, NULL) < 0) { + free(tmpfile); + return error("editing object file failed"); + } + if (import_object(&new_oid, type, raw, tmpfile)) { + free(tmpfile); + return -1; + } free(tmpfile); if (!oidcmp(&old_oid, &new_oid)) @@ -310,7 +345,7 @@ static int edit_and_replace(const char *object_ref, int force, int raw) return replace_object_oid(object_ref, &old_oid, "replacement", &new_oid, force); } -static void replace_parents(struct strbuf *buf, int argc, const char **argv) +static int replace_parents(struct strbuf *buf, int argc, const char **argv) { struct strbuf new_parents = STRBUF_INIT; const char *parent_start, *parent_end; @@ -327,9 +362,15 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv) /* prepare new parents */ for (i = 0; i < argc; i++) { struct object_id oid; - if (get_oid(argv[i], &oid) < 0) - die(_("Not a valid object name: '%s'"), argv[i]); - lookup_commit_or_die(&oid, argv[i]); + if (get_oid(argv[i], &oid) < 0) { + strbuf_release(&new_parents); + return error(_("Not a valid object name: '%s'"), + argv[i]); + } + if (!lookup_commit_reference(&oid)) { + strbuf_release(&new_parents); + return error(_("could not parse %s"), argv[i]); + } strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid)); } @@ -338,6 +379,7 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv) new_parents.buf, new_parents.len); strbuf_release(&new_parents); + return 0; } struct check_mergetag_data { @@ -358,21 +400,23 @@ static int check_one_mergetag(struct commit *commit, hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &tag_oid); tag = lookup_tag(&tag_oid); if (!tag) - die(_("bad mergetag in commit '%s'"), ref); + return error(_("bad mergetag in commit '%s'"), ref); if (parse_tag_buffer(tag, extra->value, extra->len)) - die(_("malformed mergetag in commit '%s'"), ref); + return error(_("malformed mergetag in commit '%s'"), ref); /* iterate over new parents */ for (i = 1; i < mergetag_data->argc; i++) { struct object_id oid; if (get_oid(mergetag_data->argv[i], &oid) < 0) - die(_("Not a valid object name: '%s'"), mergetag_data->argv[i]); + return error(_("Not a valid object name: '%s'"), + mergetag_data->argv[i]); if (!oidcmp(&tag->tagged->oid, &oid)) return 0; /* found */ } - die(_("original commit '%s' contains mergetag '%s' that is discarded; " - "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid)); + return error(_("original commit '%s' contains mergetag '%s' that is " + "discarded; use --edit instead of --graft"), ref, + oid_to_hex(&tag_oid)); } static int check_mergetags(struct commit *commit, int argc, const char **argv) @@ -394,24 +438,35 @@ static int create_graft(int argc, const char **argv, int force) unsigned long size; if (get_oid(old_ref, &old_oid) < 0) - die(_("Not a valid object name: '%s'"), old_ref); - commit = lookup_commit_or_die(&old_oid, old_ref); + return error(_("Not a valid object name: '%s'"), old_ref); + commit = lookup_commit_reference(&old_oid); + if (!commit) + return error(_("could not parse %s"), old_ref); buffer = get_commit_buffer(commit, &size); strbuf_add(&buf, buffer, size); unuse_commit_buffer(commit, buffer); - replace_parents(&buf, argc - 1, &argv[1]); + if (replace_parents(&buf, argc - 1, &argv[1]) < 0) { + strbuf_release(&buf); + return -1; + } if (remove_signature(&buf)) { warning(_("the original commit '%s' has a gpg signature."), old_ref); warning(_("the signature will be removed in the replacement commit!")); } - check_mergetags(commit, argc, argv); + if (check_mergetags(commit, argc, argv)) { + strbuf_release(&buf); + return -1; + } - if (write_object_file(buf.buf, buf.len, commit_type, &new_oid)) - die(_("could not write replacement commit for: '%s'"), old_ref); + if (write_object_file(buf.buf, buf.len, commit_type, &new_oid)) { + strbuf_release(&buf); + return error(_("could not write replacement commit for: '%s'"), + old_ref); + } strbuf_release(&buf); From 041c98e22d88d963a92f1bac6ab348a0d4c6d228 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:44:32 +0200 Subject: [PATCH 05/12] replace: prepare create_graft() for converting graft files wholesale When converting all grafts in a graft file to replace refs, and one of them happens to leave the original commit's parents unchanged, we do not want to error out. Instead, we would like to issue a warning. Prepare the create_graft() function for such a use case by adding a `gentle` parameter. If set, we do not return an error when the replace ref is unchanged, but a mere warning. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/replace.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index e57d3d187e..64f5811270 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -428,7 +428,7 @@ static int check_mergetags(struct commit *commit, int argc, const char **argv) return for_each_mergetag(check_one_mergetag, commit, &mergetag_data); } -static int create_graft(int argc, const char **argv, int force) +static int create_graft(int argc, const char **argv, int force, int gentle) { struct object_id old_oid, new_oid; const char *old_ref = argv[0]; @@ -470,8 +470,13 @@ static int create_graft(int argc, const char **argv, int force) strbuf_release(&buf); - if (!oidcmp(&old_oid, &new_oid)) + if (!oidcmp(&old_oid, &new_oid)) { + if (gentle) { + warning("graft for '%s' unnecessary", oid_to_hex(&old_oid)); + return 0; + } return error("new commit is the same as the old one: '%s'", oid_to_hex(&old_oid)); + } return replace_object_oid(old_ref, &old_oid, "replacement", &new_oid, force); } @@ -547,7 +552,7 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc < 1) usage_msg_opt("-g needs at least one argument", git_replace_usage, options); - return create_graft(argc, argv, force); + return create_graft(argc, argv, force, 0); case MODE_LIST: if (argc > 1) From fb40429109723c0d8ec77ba81421f508ac2532a0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:44:35 +0200 Subject: [PATCH 06/12] replace: introduce --convert-graft-file This option is intended to help with the transition away from the now-deprecated graft file. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/git-replace.txt | 11 ++++++--- builtin/replace.c | 44 ++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index e5c57ae6ef..246dc9943c 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -11,6 +11,7 @@ SYNOPSIS 'git replace' [-f] 'git replace' [-f] --edit 'git replace' [-f] --graft [...] +'git replace' [-f] --convert-graft-file 'git replace' -d ... 'git replace' [--format=] [-l []] @@ -87,9 +88,13 @@ OPTIONS content as except that its parents will be [...] instead of 's parents. A replacement ref is then created to replace with the newly created - commit. See contrib/convert-grafts-to-replace-refs.sh for an - example script based on this option that can convert grafts to - replace refs. + commit. Use `--convert-graft-file` to convert a + `$GIT_DIR/info/grafts` file and use replace refs instead. + +--convert-graft-file:: + Creates graft commits for all entries in `$GIT_DIR/info/grafts` + and deletes that file upon success. The purpose is to help users + with transitioning off of the now-deprecated graft file. -l :: --list :: diff --git a/builtin/replace.c b/builtin/replace.c index 64f5811270..a87fca045b 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -20,6 +20,7 @@ static const char * const git_replace_usage[] = { N_("git replace [-f] "), N_("git replace [-f] --edit "), N_("git replace [-f] --graft [...]"), + N_("git replace [-f] --convert-graft-file"), N_("git replace -d ..."), N_("git replace [--format=] [-l []]"), NULL @@ -481,6 +482,38 @@ static int create_graft(int argc, const char **argv, int force, int gentle) return replace_object_oid(old_ref, &old_oid, "replacement", &new_oid, force); } +static int convert_graft_file(int force) +{ + const char *graft_file = get_graft_file(); + FILE *fp = fopen_or_warn(graft_file, "r"); + struct strbuf buf = STRBUF_INIT, err = STRBUF_INIT; + struct argv_array args = ARGV_ARRAY_INIT; + + if (!fp) + return -1; + + while (strbuf_getline(&buf, fp) != EOF) { + if (*buf.buf == '#') + continue; + + argv_array_split(&args, buf.buf); + if (args.argc && create_graft(args.argc, args.argv, force, 1)) + strbuf_addf(&err, "\n\t%s", buf.buf); + argv_array_clear(&args); + } + fclose(fp); + + strbuf_release(&buf); + + if (!err.len) + return unlink_or_warn(graft_file); + + warning(_("could not convert the following graft(s):\n%s"), err.buf); + strbuf_release(&err); + + return -1; +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; @@ -492,6 +525,7 @@ int cmd_replace(int argc, const char **argv, const char *prefix) MODE_DELETE, MODE_EDIT, MODE_GRAFT, + MODE_CONVERT_GRAFT_FILE, MODE_REPLACE } cmdmode = MODE_UNSPECIFIED; struct option options[] = { @@ -499,6 +533,7 @@ int cmd_replace(int argc, const char **argv, const char *prefix) OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"), MODE_DELETE), OPT_CMDMODE('e', "edit", &cmdmode, N_("edit existing object"), MODE_EDIT), OPT_CMDMODE('g', "graft", &cmdmode, N_("change a commit's parents"), MODE_GRAFT), + OPT_CMDMODE(0, "convert-graft-file", &cmdmode, N_("convert existing graft file"), MODE_CONVERT_GRAFT_FILE), OPT_BOOL_F('f', "force", &force, N_("replace the ref if it exists"), PARSE_OPT_NOCOMPLETE), OPT_BOOL(0, "raw", &raw, N_("do not pretty-print contents for --edit")), @@ -521,7 +556,8 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (force && cmdmode != MODE_REPLACE && cmdmode != MODE_EDIT && - cmdmode != MODE_GRAFT) + cmdmode != MODE_GRAFT && + cmdmode != MODE_CONVERT_GRAFT_FILE) usage_msg_opt("-f only makes sense when writing a replacement", git_replace_usage, options); @@ -554,6 +590,12 @@ int cmd_replace(int argc, const char **argv, const char *prefix) git_replace_usage, options); return create_graft(argc, argv, force, 0); + case MODE_CONVERT_GRAFT_FILE: + if (argc != 0) + usage_msg_opt("--convert-graft-file takes no argument", + git_replace_usage, options); + return !!convert_graft_file(force); + case MODE_LIST: if (argc > 1) usage_msg_opt("only one pattern can be given with -l", From 0115e030db499853a77c2b92dc8c6a51552009ef Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:44:42 +0200 Subject: [PATCH 07/12] Add a test for `git replace --convert-graft-file` The proof, as the saying goes, lies in the pudding. So here is a regression test that not only demonstrates what the option is supposed to accomplish, but also demonstrates that it does accomplish it. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t6050-replace.sh | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index c630aba657..d174bfed30 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -444,4 +444,32 @@ test_expect_success GPG '--graft on a commit with a mergetag' ' git replace -d $HASH10 ' +test_expect_success '--convert-graft-file' ' + git checkout -b with-graft-file && + test_commit root2 && + git reset --hard root2^ && + test_commit root1 && + test_commit after-root1 && + test_tick && + git merge -m merge-root2 root2 && + + : add and convert graft file && + printf "%s\n%s %s\n\n# comment\n%s\n" \ + $(git rev-parse HEAD^^ HEAD^ HEAD^^ HEAD^2) \ + >.git/info/grafts && + git replace --convert-graft-file && + test_path_is_missing .git/info/grafts && + + : verify that the history is now "grafted" && + git rev-list HEAD >out && + test_line_count = 4 out && + + : create invalid graft file and verify that it is not deleted && + test_when_finished "rm -f .git/info/grafts" && + echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts && + test_must_fail git replace --convert-graft-file 2>err && + test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" err && + test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts +' + test_done From f9f99b3f7d06716336b239bcf122eb5a7348bfd5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:44:44 +0200 Subject: [PATCH 08/12] Deprecate support for .git/info/grafts The grafts feature was a convenient way to "stitch together" ancient history to the fresh start of linux.git. Its implementation is, however, not up to Git's standards, as there are too many ways where it can lead to surprising and unwelcome behavior. For example, when pushing from a repository with active grafts, it is possible to miss commits that have been "grafted out", resulting in a broken state on the other side. Also, the grafts feature is limited to "rewriting" commits' list of parents, it cannot replace anything else. The much younger feature implemented as `git replace` set out to remedy those limitations and dangerous bugs. Seeing as `git replace` is pretty mature by now (since 4228e8bc98 (replace: add --graft option, 2014-07-19) it can perform the graft file's duties), it is time to deprecate support for the graft file, and to retire it eventually. Signed-off-by: Johannes Schindelin Reviewed-by: Stefan Beller Signed-off-by: Junio C Hamano --- advice.c | 2 ++ advice.h | 1 + commit.c | 10 ++++++++++ t/t6001-rev-list-graft.sh | 9 +++++++++ 4 files changed, 22 insertions(+) diff --git a/advice.c b/advice.c index 406efc183b..4411704fd4 100644 --- a/advice.c +++ b/advice.c @@ -19,6 +19,7 @@ int advice_rm_hints = 1; int advice_add_embedded_repo = 1; int advice_ignored_hook = 1; int advice_waiting_for_editor = 1; +int advice_graft_file_deprecated = 1; static struct { const char *name; @@ -42,6 +43,7 @@ static struct { { "addembeddedrepo", &advice_add_embedded_repo }, { "ignoredhook", &advice_ignored_hook }, { "waitingforeditor", &advice_waiting_for_editor }, + { "graftfiledeprecated", &advice_graft_file_deprecated }, /* make this an alias for backward compatibility */ { "pushnonfastforward", &advice_push_update_rejected } diff --git a/advice.h b/advice.h index 70568fa792..9f5064e82a 100644 --- a/advice.h +++ b/advice.h @@ -21,6 +21,7 @@ extern int advice_rm_hints; extern int advice_add_embedded_repo; extern int advice_ignored_hook; extern int advice_waiting_for_editor; +extern int advice_graft_file_deprecated; int git_default_advice_config(const char *var, const char *value); __attribute__((format (printf, 1, 2))) diff --git a/commit.c b/commit.c index 2952ec987c..451d3ce8df 100644 --- a/commit.c +++ b/commit.c @@ -12,6 +12,7 @@ #include "prio-queue.h" #include "sha1-lookup.h" #include "wt-status.h" +#include "advice.h" static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **); @@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file) struct strbuf buf = STRBUF_INIT; if (!fp) return -1; + if (advice_graft_file_deprecated) + advise(_("Support for /info/grafts is deprecated\n" + "and will be removed in a future Git version.\n" + "\n" + "Please use \"git replace --convert-graft-file\"\n" + "to convert the grafts into replace refs.\n" + "\n" + "Turn this message off by running\n" + "\"git config advice.graftFileDeprecated false\"")); while (!strbuf_getwholeline(&buf, fp, '\n')) { /* The format is just "Commit Parent1 Parent2 ...\n" */ struct commit_graft *graft = read_graft_line(&buf); diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh index 05ddc69cf2..7504ba4751 100755 --- a/t/t6001-rev-list-graft.sh +++ b/t/t6001-rev-list-graft.sh @@ -110,4 +110,13 @@ do " done + +test_expect_success 'show advice that grafts are deprecated' ' + git show HEAD 2>err && + test_i18ngrep "git replace" err && + test_config advice.graftFileDeprecated false && + git show HEAD 2>err && + test_i18ngrep ! "git replace" err +' + test_done From e2d65c1ea810f34681c3f3aba03c58b6e8484b3f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:44:53 +0200 Subject: [PATCH 09/12] filter-branch: stop suggesting to use grafts The graft file is deprecated now, so let's use replace refs in the example in filter-branch's man page instead. Suggested-by: Eric Sunshine Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/git-filter-branch.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt index b634043183..1d4d2f8604 100644 --- a/Documentation/git-filter-branch.txt +++ b/Documentation/git-filter-branch.txt @@ -288,7 +288,7 @@ git filter-branch --parent-filter \ or even simpler: ----------------------------------------------- -echo "$commit-id $graft-id" >> .git/info/grafts +git replace --graft $commit-id $graft-id git filter-branch $graft-id..HEAD ----------------------------------------------- From 8d0d81a9caf612b479b96bc474815e2f38c4746b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:45:13 +0200 Subject: [PATCH 10/12] technical/shallow: stop referring to grafts Now that grafts are deprecated, we should start to assume that readers have no idea what grafts are. So it makes more sense to make the description of the "shallow" feature stand on its own. Suggested-by: Eric Sunshine Helped-by: Junio Hamano Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/technical/shallow.txt | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Documentation/technical/shallow.txt b/Documentation/technical/shallow.txt index 5183b15422..4ec721335d 100644 --- a/Documentation/technical/shallow.txt +++ b/Documentation/technical/shallow.txt @@ -8,15 +8,10 @@ repo, and therefore grafts are introduced pretending that these commits have no parents. ********************************************************* -The basic idea is to write the SHA-1s of shallow commits into -$GIT_DIR/shallow, and handle its contents like the contents -of $GIT_DIR/info/grafts (with the difference that shallow -cannot contain parent information). - -This information is stored in a new file instead of grafts, or -even the config, since the user should not touch that file -at all (even throughout development of the shallow clone, it -was never manually edited!). +$GIT_DIR/shallow lists commit object names and tells Git to +pretend as if they are root commits (e.g. "git log" traversal +stops after showing them; "git fsck" does not complain saying +the commits listed on their "parent" lines do not exist). Each line contains exactly one SHA-1. When read, a commit_graft will be constructed, which has nr_parent < 0 to make it easier From f42fa470b0d87fcb36475f78f2366bba6864895a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:45:21 +0200 Subject: [PATCH 11/12] technical/shallow: describe why shallow cannot use replace refs It is tempting to do away with commit_graft altogether (in the long haul), now that grafts are deprecated. However, the shallow feature needs a couple of things that the replace refs cannot fulfill. Let's point that out in the documentation. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/technical/shallow.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/technical/shallow.txt b/Documentation/technical/shallow.txt index 4ec721335d..01dedfe9ff 100644 --- a/Documentation/technical/shallow.txt +++ b/Documentation/technical/shallow.txt @@ -17,6 +17,13 @@ Each line contains exactly one SHA-1. When read, a commit_graft will be constructed, which has nr_parent < 0 to make it easier to discern from user provided grafts. +Note that the shallow feature could not be changed easily to +use replace refs: a commit containing a `mergetag` is not allowed +to be replaced, not even by a root commit. Such a commit can be +made shallow, though. Also, having a `shallow` file explicitly +listing all the commits made shallow makes it a *lot* easier to +do shallow-specific things such as to deepen the history. + Since fsck-objects relies on the library to read the objects, it honours shallow commits automatically. From a3694d949fcd39f9a909cf762f698df8ce83215c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 29 Apr 2018 00:45:22 +0200 Subject: [PATCH 12/12] Remove obsolete script to convert grafts to replace refs The functionality is now implemented as `git replace --convert-graft-file`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/convert-grafts-to-replace-refs.sh | 28 ----------------------- 1 file changed, 28 deletions(-) delete mode 100755 contrib/convert-grafts-to-replace-refs.sh diff --git a/contrib/convert-grafts-to-replace-refs.sh b/contrib/convert-grafts-to-replace-refs.sh deleted file mode 100755 index 0cbc917b8c..0000000000 --- a/contrib/convert-grafts-to-replace-refs.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/bin/sh - -# You should execute this script in the repository where you -# want to convert grafts to replace refs. - -GRAFTS_FILE="${GIT_DIR:-.git}/info/grafts" - -. $(git --exec-path)/git-sh-setup - -test -f "$GRAFTS_FILE" || die "Could not find graft file: '$GRAFTS_FILE'" - -grep '^[^# ]' "$GRAFTS_FILE" | -while read definition -do - if test -n "$definition" - then - echo "Converting: $definition" - git replace --graft $definition || - die "Conversion failed for: $definition" - fi -done - -mv "$GRAFTS_FILE" "$GRAFTS_FILE.bak" || - die "Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'" - -echo "Success!" -echo "All the grafts in '$GRAFTS_FILE' have been converted to replace refs!" -echo "The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'"